Skip to content

Conversation

@silenaker
Copy link

@silenaker silenaker commented Jul 24, 2025

Description of change
  1. The AlphaMaskPipe calculates the bounds of the mask using world coordinates instead of the local coordinates of the cache-as-texture render group.
  2. getFastGlobalBounds should calculate local bounds only for render groups; otherwise, it may cause unpredictable bugs.
  3. The FilterSystem applies clipToViewport with the screen root viewport instead of the cache-as-texture render group's viewport, causing unexpected bugs.
  4. The Pool does not account for duplicate object returns, leading to erratic bugs and unit test failures.
  5. The TexturePool#getOptimalTexture method does not dispatch texture update event when the texture frame is updated. As a result, the TextureMatrix that depends on it fails to timely update its state (TextureMatrix#mapCoord), leading to errors when calculating uFilterMatrix uniform in MaskFilter#apply.
  6. Unlike StencilMask, AlphaMask should not have addBounds and addLocalBounds methods, as they lead to incorrect bounds calculation on masked objects. No changes were committed since users can implement a new AlphaMask class to fix this.
Pre-Merge Checklist
  • Tests and/or benchmarks are included
  • Documentation is changed or added
  • Lint process passed (npm run lint)
  • Tests passed (npm run test)

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jul 24, 2025

pixi.js-basepixi.js-bunny-mark

npm i https://pkg.pr.new/pixijs/pixijs/pixi.js@11577

commit: 438aefd

@silenaker silenaker marked this pull request as ready for review July 24, 2025 08:48
@silenaker silenaker changed the title Alpha mask filter broken with render group cache-as-texture fix: alpha mask filter broken with render group cache-as-texture Jul 24, 2025
@Zyie
Copy link
Member

Zyie commented Jul 24, 2025

hey @silenaker are you able to take a look at the failing visual tests?

@silenaker silenaker force-pushed the dev branch 4 times, most recently from adbe59a to d850e65 Compare July 27, 2025 10:02
Copy link
Collaborator

@mayakwd mayakwd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this fix appears to be a solution for a specific case involving one filter. Of course, the mask is a specific case to begin with, so there's nothing wrong with that.

However, it may be worth revisiting this pull request and considering all filters and bounds calculations for RenderGroup.

Currently, filters “break” in unexpected places when the user uses cacheAsTexture.

this._calculateFilterBounds(filterData, renderer.renderTarget.rootViewPort, rootAntialias, rootResolution, 1);
let viewport = renderer.renderTarget.rootViewPort;

const renderGroup = instruction.container.renderGroup || instruction.container.parentRenderGroup;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instruction.container is not always available. This line will break execution and produce "TypeError".
FilterInstruction contains either container or renderables.

@silenaker silenaker force-pushed the dev branch 2 times, most recently from 0862077 to 9690b53 Compare July 27, 2025 15:13
@silenaker
Copy link
Author

Overall, this fix appears to be a solution for a specific case involving one filter. Of course, the mask is a specific case to begin with, so there's nothing wrong with that.

This isn’t a fix targeting a specific filter. The FilterSystem already takes cache-as-texture render groups into account. For it to work correctly in such cases, two conditions must be met. The first—applying the inverse transform to global bounds—has already been handled. However, it missed a subtle but essential detail: the root viewport also needs to be adjusted accordingly. This PR addresses that missing piece.

However, it may be worth revisiting this pull request and considering all filters and bounds calculations for RenderGroup.

On the surface, this PR fixes an issue where alpha mask doesn’t work properly inside cache-as-texture render groups. But while addressing that, I encountered several deeper issues in core components like getFastGlobalBounds, TexturePool, and Pool, which had hard-to-detect bugs that were difficult to test. The FilterSystem was not my only focus—it just happens to be one of the affected components. Given its importance, I took the time to review its implementation as well.

During the review, I noticed several inconsistencies in how it handles nested filters, resolution, and antialias settings. The logic there isn't entirely clear and contains some conflicting intentions. I attempted to address these issues too, but the changes were too extensive and out of scope for this PR.

As a result, I’ve only included the minimal changes necessary to keep this PR focused and stable and I'll create new PRs to address these not directly related issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants