Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses rendering regressions around filtered container layers (issue #274), particularly when sprites contain transformed shapes or cacheAsBitmap descendants, and aligns related caching/binding behaviors across WebGL/WebGPU paths.
Changes:
- Introduce
layerScalepropagation in the display render-queue to allocate higher-resolution filter layers for descendants’ cacheAsBitmap scales (issue #274), plus new E2E pages/snapshots to lock in behavior. - Improve WebGPU atlas bind-group reuse by caching bind groups per atlas texture view (WeakMap) with sampler/layout invalidation.
- Fix/adjust several low-level rendering utilities (rotation edge-case handling, GL bound-texture cleanup on delete, buffer growth copy optimization, JSON-deserialized gradient matrix handling).
Reviewed changes
Copilot reviewed 23 out of 66 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/webgpu/src/Context/usecase/ContextDrawArraysInstancedUseCase.ts | Cache WebGPU bind groups per atlas texture view via WeakMap; invalidate on sampler/layout change. |
| packages/webgpu/src/Context/usecase/ContextContainerEndLayerUseCase.ts | Adds layer scale parameters (currently TODO for WebGPU-specific scaling behavior). |
| packages/webgpu/src/Context.ts | Plumbs layer scale through containerEndLayer call site. |
| packages/webgpu/src/AtlasManager.ts | Simplifies atlas attachment/transfer-bounds retrieval logic. |
| packages/webgl/src/TextureManager/usecase/TextureManagerReleaseTextureObjectUseCase.ts | Clears cached bound-texture references before deleting a texture resource. |
| packages/webgl/src/TextureManager/service/TextureManagerBindService.ts | Refactors binding cache checks; consolidates filter parameter updates. |
| packages/webgl/src/Mesh.ts | Avoid copying unused buffer regions when growing the fill buffer. |
| packages/webgl/src/Context/usecase/ContextContainerEndLayerUseCase.ts | Accepts layer scale args (documented as unused in WebGL path). |
| packages/webgl/src/Context.ts | Plumbs layer scale through containerEndLayer call site. |
| packages/webgl/src/AtlasManager.ts | Simplifies atlas attachment existence checks and transfer-bounds retrieval. |
| packages/renderer/src/DisplayObjectContainer/usecase/DisplayObjectContainerRenderUseCase.ts | Updates render-queue parsing to account for newly-pushed layerScale values. |
| packages/display/src/Graphics/service/GraphicsToNumberArrayService.ts | Handles JSON-deserialized gradient matrices by index access instead of spread. |
| packages/display/src/DisplayObjectContainer/usecase/DisplayObjectContainerGetLayerScaleUseCase.ts | New: computes max descendant cacheAsBitmap scale for filter-layer sizing. |
| packages/display/src/DisplayObjectContainer/usecase/DisplayObjectContainerGenerateRenderQueueUseCase.ts | Computes/pushes layerScaleX/Y, scales filter-layer matrix to prevent cropping (issue #274). |
| packages/display/src/DisplayObject/usecase/DisplayObjectSetRotationUseCase.ts | Adjusts rotation matrix update to avoid false “cos==0” detection. |
| packages/display/src/DisplayObject/usecase/DisplayObjectSetRotationUseCase.test.ts | Adds regression test covering the scaleX×sin(theta)==1 false-positive case (issue #274). |
| package.json | Version bump + dependency bumps (tooling/test stack). |
| e2e/tests/filter.spec.ts | Adds new regression E2E cases for transformed shapes + cacheAsBitmap child under filters (issue #274). |
| e2e/snapshots/webgpu/textfield.spec.ts-snapshots/textfield-auto-font-size-webgpu-darwin.png | Updated WebGPU snapshot output. |
| e2e/snapshots/webgpu/shape.spec.ts-snapshots/cache-as-bitmap-webgpu-darwin.png | Updated WebGPU snapshot output. |
| e2e/snapshots/webgpu/filter.spec.ts-snapshots/filter-modes-webgpu-darwin.png | Updated WebGPU snapshot output. |
| e2e/snapshots/webgpu/filter.spec.ts-snapshots/filter-convolution-webgpu-darwin.png | Updated WebGPU snapshot output. |
| e2e/snapshots/webgpu/filter.spec.ts-snapshots/filter-color-matrix-cacheasbitmap-child-webgpu-darwin.png | New/updated WebGPU snapshot for cacheAsBitmap-child filter regression. |
| e2e/snapshots/webgl/filter.spec.ts-snapshots/filter-modes-webgl-darwin.png | Updated WebGL snapshot output. |
| e2e/snapshots/webgl/filter.spec.ts-snapshots/filter-convolution-webgl-darwin.png | Updated WebGL snapshot output. |
| e2e/snapshots/webgl/filter.spec.ts-snapshots/filter-color-matrix-cacheasbitmap-child-webgl-darwin.png | New/updated WebGL snapshot for cacheAsBitmap-child filter regression. |
| e2e/pages/filter/filters-sprite-transformed.html | New E2E page: all filters on transformed-shapes sprite (issue #274 regression). |
| e2e/pages/filter/filters-cacheasbitmap-child.html | New E2E page: all filters on parent with cacheAsBitmap child (issue #274 regression). |
| e2e/pages/filter/color-matrix-sprite-transformed.html | New E2E page: ColorMatrixFilter on transformed-shapes sprite (issue #274). |
| e2e/pages/filter/color-matrix-cacheasbitmap-child.html | New E2E page: ColorMatrixFilter parent with cacheAsBitmap child (issue #274). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+58
to
+66
| const sinX = Math.sin(radianX); | ||
| const cosX = Math.cos(radianX); | ||
| matrix.b = scaleX * sinX; | ||
| matrix.a = Math.abs(sinX) === 1 ? 0 : scaleX * cosX; | ||
|
|
||
| matrix.c = -scaleY * Math.sin(radianY); | ||
| if (matrix.c === 1 || matrix.c === -1) { | ||
| matrix.d = 0; | ||
| } else { | ||
| matrix.d = scaleY * Math.cos(radianY); | ||
| } | ||
| const sinY = Math.sin(radianY); | ||
| const cosY = Math.cos(radianY); | ||
| matrix.c = -scaleY * sinY; | ||
| matrix.d = Math.abs(sinY) === 1 ? 0 : scaleY * cosY; |
There was a problem hiding this comment.
Math.abs(sinX) === 1 / Math.abs(sinY) === 1 relies on exact floating-point equality and can fail to clear a/d when cos is effectively 0 (e.g., due to rounding). Consider using an epsilon check (e.g. Math.abs(cosX) < eps) or Math.abs(1 - Math.abs(sinX)) < eps to reliably detect the 90°/270° cases.
Comment on lines
+385
to
+387
| // 子のcacheScale倍テクスチャが端まで収まる (issue #274 発展対応)。 | ||
| const layerScale = new Float32Array([1, 1]); | ||
| displayObjectContainerGetLayerScaleUseCase( |
There was a problem hiding this comment.
layerScale is recomputed by traversing descendants and allocating new Float32Array([1, 1]) on every render, even when the filter cache HIT path returns early. This can add significant per-frame overhead for filtered containers; consider reusing a scratch buffer and/or caching the computed layerScale (recompute only when descendants / cacheAsBitmap matrices change).
Comment on lines
+33
to
+37
| const boundTexture = $boundTextures[index]; | ||
| if (boundTexture === texture_object) { | ||
| // Same reference, already bound to this unit. | ||
| return; | ||
| } |
There was a problem hiding this comment.
The early return on boundTexture === texture_object prevents updating texture parameters when the same texture stays bound but the requested smooth flag changes. If callers can toggle smooth per draw, this will leave TEXTURE_MIN/MAG_FILTER stale; consider only returning early when texture_object is null or texture_object.smooth already matches smooth (or move the smooth update check before returning).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.