fix(clipping): correct Container.clipping under nested transforms (closes #1349) by obiot · Pull Request #1435 · melonjs/melonJS

obiot · 2026-05-07T23:22:13Z

Summary

Fix Container.clipping misalignment when nested inside a translated, scaled, or rotated parent (closes clipRect: potential issues with transformed containers and scissor state cleanup #1349).
Fix a pre-existing latent bug where WebGLRenderer.restore() reverted the GL scissor without flushing pending PrimitiveBatcher vertices, so vertices queued inside a deeper clip could leak past restore() and flush later under a more permissive scissor.
Add a Clipping example to the gallery — first example to actually exercise Container.clipping, which is why both bugs went unnoticed.
Extend clipRect.spec.js from 0 to 19 tests across WebGL + Canvas + a RenderState.peekScissor unit suite.

What changed

Coord-space fix at the call site — Container.draw now applies its own translate(pos.x, pos.y) before calling clipRect, then passes container-local (0, 0, width, height). The renderer's transform stack drives the screen-space conversion. As a deliberate side effect the clip is now the container's own rect rather than the union bounds it used to be (which silently expanded to include overflowing children, defeating the clip).

Matrix-aware AABB on WebGL — WebGLRenderer.clipRect previously only honored currentTransform.tx/ty, ignoring scale and rotation. It now transforms all four input corners through currentTransform and uses the screen-space AABB as the scissor box. Rotation collapses to the AABB by hardware contract (gl.scissor is axis-aligned). Canvas's context.rect was already matrix-aware.

Transform-aware "no clip" fast path — clipRect short-circuits to "scissor disabled" when the resulting AABB ≥ canvas, and bails out early when currentTransform.tx/ty are non-finite (the ColorLayer Infinity-anchor poison case).

Flush ordering in restore() — new RenderState.peekScissor() lets WebGLRenderer.restore() compare the to-be-restored scissor against the current one without allocating, and flush only when the box actually changes. Vertices then drain under the correct (current) GL scissor before the box reverts.

Test plan

pnpm vitest run — 2928 tests pass (87 files), including 19 new clipRect specs covering: identity / translation / scale / rotation under WebGL + Canvas, nested-Container call site, full-canvas no-clip fast path, Infinity-translate poison, AABB-covers-canvas under non-identity transform, primitive draw-mode switch (fillRect → strokeLine) under a clip, batcher swap (fillRect ↔ drawImage) under a clip, pre-clip / post-clip vertex drain at clipRect entry, nested save/restore scissor preservation, and a RenderState.peekScissor unit suite.
pnpm build — 0 errors.
Visual: load the new Clipping example in WebGL and Canvas modes — concentric color rings on the left, pulsing scaled clip on the right, identical between renderers.

🤖 Generated with Claude Code

…arent transform (closes #1349) Two bugs were producing the same wrong screen rect via different mechanisms whenever a clipping `Container` lived under a translated, scaled, or rotated parent: 1. **Call-site coord-space mismatch (both renderers).** `Container.draw` was passing world-space `bounds.left/top` to a `clipRect` API that interpreted its input as local-to-current-transform. The parent's own `translate(this.pos.x, this.pos.y)` (line 948) had already been pushed onto `currentTransform` by the time the inner container's `clipRect` ran, so the world-space input got double-counted and the clip landed offset by exactly the parent's `pos`. 2. **WebGL `clipRect` ignored scale and rotation.** The implementation converted input to GL bottom-left scissor space by adding only `currentTransform.tx/ty`. A directly-nested clipping container under a scaled or rotated parent produced a wrong-sized scissor box (or in rotation's case, a non-rotated rect at the wrong place). Fixes: - `Container.draw` now applies its own `translate(pos)` *before* calling `clipRect`, and passes container-local `(0, 0, width, height)`. The renderer's transform stack handles the screen-space conversion; no manual coord-space juggling at the call site. - `WebGLRenderer.clipRect` now transforms the four input corners through `currentTransform` and uses the AABB as the screen-space scissor box. Scale and rotation are honored (rotation collapses to the rect's screen AABB — `gl.scissor` is axis-aligned). Preserves the original "input matches canvas size = no-clip" fast path so callers like `ColorLayer.draw` (which has `Infinity` host bounds and a 0.5-anchor, leaving `currentTransform.tx/ty = -Infinity`) keep working. - `WebGLRenderer.restore` reads `currentScissor` as screen-space coords directly — `clipRect` now stores the post-transform rect so restore doesn't need to re-run the math against a possibly-different `currentTransform`. Canvas's `context.rect` + `context.clip()` was already matrix-aware, so only the call-site fix applied there. Tests (`tests/clipRect.spec.js`, 10 cases): - `clipRect` baseline: identity, translation, save/restore (all pass). - WebGL scale + rotation: fixed (was demonstrably wrong on master). - Container nested in translated wrapper: fixed for both WebGL and Canvas (was the visible #1349 symptom). - Full-canvas no-clip fast path: regression guard for the `ColorLayer`-shaped contract — calling `clipRect(0, 0, canvasW, canvasH)` under a `-Infinity`-translated `currentTransform` must disable the scissor without running the transform math. Example (`packages/examples/src/examples/clipping/ExampleClipping.tsx`): visual reproducer / regression demo. Stacks three layers via z-index: - z=0: `ColorLayer` filling the whole canvas (full-canvas `clipRect + clearColor`) - z=1: `PartialColorLayer` (subclass) filling a horizontal band via the same `clipRect + clearColor` mechanism but with a sub-rect input - z=2..4: two clipping `Container`s side-by-side — left directly under the world, right nested inside a `pos = (280, 80)` wrapper. Both clip identically inside their green outlines after the fix; pre-fix the right one was offset by exactly the wrapper's `pos`. CHANGELOG entry under `[19.3.0] - _unreleased_ → ### Fixed`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…rer.restore `WebGLRenderer.restore()` was reverting the GL scissor box without flushing pending PrimitiveBatcher vertices, so vertices queued inside a deeper clipping container could leak past the restore and flush later under a more permissive scissor — visible only with deeply nested `Container.clipping`, latent on master because no example exercised it before #1349. Also: tighten `WebGLRenderer.clipRect` to disable the scissor when the screen-space AABB >= canvas (transform-aware "no clip" fast path), and explicitly short-circuit when `currentTransform.tx/ty` are non-finite (`ColorLayer`'s `Infinity`-anchor poison case). Add `peekScissor()` on `RenderState` so `restore()` can compare the to-be-restored scissor against the current one without allocating, and only flush when the box actually changes. Tests: extend `clipRect.spec.js` with 8 adversarial guards covering the flush-ordering fix, primitive draw-mode switches, batcher swaps mid-clip, the pre-clip vertex drain at `clipRect` entry, the canvas-covering AABB fast path, and a unit test for `peekScissor`. Example: switch the clipping demo to `video.AUTO` so Canvas users see the same result, and trim its comment block now that the underlying mechanics are stable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Fixes Container.clipping behavior under nested transforms by aligning clipRect coordinate space usage and making WebGL scissor computation transform-aware, while also addressing a WebGL scissor restore/flush ordering bug. Adds regression tests and a new gallery example to exercise the clipping path end-to-end.

Changes:

Update Container.draw to translate into container-local space before applying clipRect(0,0,width,height).
Update WebGL clipRect to compute a screen-space AABB via the current transform and update restore() to flush when scissor state changes (via new RenderState.peekScissor()).
Add a new Clipping example plus expanded WebGL/Canvas/unit test coverage and changelog entries.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/melonjs/tests/clipRect.spec.js	New regression suite covering clip/scissor behavior across WebGL + Canvas and save/restore semantics
packages/melonjs/src/video/webgl/webgl_renderer.js	Transform-aware scissor computation in `clipRect` and scissor-change-aware flushing in `restore()`
packages/melonjs/src/video/renderstate.js	Adds `peekScissor()` to inspect the next restore’s scissor without mutating stack state
packages/melonjs/src/renderable/container.js	Adjusts clipping call site to use container-local coordinates (after applying container translation)
packages/melonjs/CHANGELOG.md	Documents the clipping/scissor fixes
packages/examples/src/main.tsx	Registers the new Clipping gallery example
packages/examples/src/examples/clipping/ExampleClipping.tsx	New example demonstrating nested and animated container clipping

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Container.draw: also require finite this.width/this.height before calling clipRect. A container can have aggregate `bounds.isFinite()` via finite-child enableChildBoundsUpdate while still carrying Infinity self-dimensions; passing Infinity to clipRect would NaN-poison the WebGL scissor math. - WebGLRenderer.clipRect: flush pending vertices before disabling the scissor in the non-finite-transform fast path. Pre-fix, queued vertices could leak past the disable and flush later under no scissor (same class as the restore-ordering fix). - WebGLRenderer.clipRect: stop allocating four point literals per call. Reuse a single instance-level scratch point across the four corner transforms; capture each result into numeric locals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Replaces the inline 4-corner transform + manual min/max with the existing `Bounds.addFrame(x0, y0, x1, y1, matrix)` utility, which already encapsulates this pattern (used by Renderable bounds). The renderer keeps a single instance-level `Bounds` as a scratch AABB so clipRect still does not allocate per call. Net: ~25 lines of inline geometry replaced with 5 lines that read as "add this rect's 4 transformed corners to a fresh AABB, take the min/max". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

`WebGLRenderer.clipRect` now uses `Bounds.addFrame(x0, y0, x1, y1, m)` to derive the screen-space scissor AABB, where `m` is the renderer's `currentTransform` (a `Matrix3d`). Existing coverage exercised `addFrame` only with `Matrix2d` rotation; add unit guards for the shapes the renderer actually feeds it: - Matrix3d identity (sanity) - Matrix3d translation - Matrix3d non-uniform scale - Matrix3d 45° rotation produces √2-scaled AABB - Composed Matrix3d (translate + rotate + scale) matches the manual 4-corner application Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Remove the empty `it("works", () => {})` placeholder at the top of the bounds describe block. Pure scaffolding, never asserted anything. - Reword `Bounds.addFrame` JSDoc: the previous wording said it adds "the given quad coordinates", but with a non-identity matrix the result is the AABB of the transformed quad's four corners, not the transformed quad itself. Make that explicit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

`addFrame` runs once per renderable per frame (Renderable bounds, Text/BitmapText bounds, Entity bounds, the new clipRect AABB) — its per-call `pointPool.get()`/`release()` is small but cumulative. Two changes inside addFrame, no API change: - No-matrix path: fold the four corners' min/max directly into the AABB without going through `addPoint`/`Point` at all. Mirrors `addPoint`'s Math.min/max behavior so swapped corners (x1 < x0 etc.) still produce the same AABB. - Matrix path: reuse a module-level scratch `Point` instead of pulling one out of `pointPool` each call. Safe under JS's single threaded execution; `addPoint` cannot reenter `addFrame`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- bounds.spec.ts: pass `Vector2d` instances (not plain `{x, y}` literals) to `Matrix3d.apply` in the manual-application cross-check — fixes the TS2345 that broke CI lint. - WebGLRenderer.enableScissor: walk 4 corners through `currentTransform` and store screen-space coords in `currentScissor`, matching the new convention `clipRect` and `restore` use. The previous impl applied only `currentTransform.tx/ty` to the GL call while caching the untransformed (local) rect, which would desync save/restore for any external caller of `enableScissor`. - examples/Clipping: `PartialColorLayer` now calls `resetTransform()` after `save()` before `clipRect`. `ColorLayer` inherits `Renderable(0, 0, Infinity, Infinity)` with the default 0.5 anchor, so `Renderable.preDraw` translates by `-Infinity` — under the new non-finite-transform guard `clipRect` would no-op and `clearColor` would clear the full canvas instead of the band. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

…cissor) Copilot caught: `RenderState` is re-exported from the package entrypoint, so `peekScissor()` was unintentionally part of the public surface. Returning a live `Int32Array` reference into `_scissorStack` let external callers mutate stack contents and corrupt subsequent `restore()` calls. The method exists purely as an internal hand-off between `RenderState` and `WebGLRenderer.restore()`; no allocation is acceptable on the hot path. Underscore-prefixed to match other internal `_*Stack` / `_stackDepth` fields and `@ignore`d so it stops appearing in the generated docs / public types. Documented the read-only contract on the live-ref return. Test + renderer call site updated to the new name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ract The previous commit underscore-prefixed + `@ignore`d `peekScissor` to hide it from the public surface. Reverting that — `@ignore` alone already keeps it out of generated docs, and a public, documented "inspect the scissor that the next restore() will install" method is useful for any custom renderer / extension that needs the same flush-before-revert decision the engine's `WebGLRenderer.restore()` makes. Replaces the rename with a clear JSDoc body describing the live-ref contract and the read-only requirement, so the tradeoff is obvious to anyone who calls it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Keeps the method out of generated public docs while leaving the documented JSDoc body in source for anyone reading the file. Stays public + callable; `@ignore` only hides from the doc generator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

+		aabb.clear();
+		aabb.addFrame(x, y, x + width, y + height, this.currentTransform);


+		const aabb = this._clipAABB;
+		aabb.clear();
+		aabb.addFrame(x, y, x + width, y + height, m);
+		const sx = Math.floor(aabb.min.x);
+		const sy = Math.floor(aabb.min.y);
+		const sw = Math.ceil(aabb.max.x - sx);
+		const sh = Math.ceil(aabb.max.y - sy);


+		const wrapper = new Container(
+			RIGHT_CENTER_X,
+			RIGHT_CENTER_Y,
+			RIGHT_W,
+			RIGHT_H,


Three small follow-ups that landed in Copilot review after the squash merge of #1435: - Bounds.addFrame: short-circuit the 4-corner walk when the matrix is identity. The renderer's clipRect/enableScissor unconditionally pass `currentTransform` and don't filter identity themselves, so they were running 4× `Matrix3d.apply` per scissor change for the common identity case. Existing matrix-passing callers (Renderable, Spine plugin) already gate on `isIdentity()` so they're unaffected. - examples/Clipping: the right-side wrapper was positioned at its visual center (`pos = (RIGHT_CENTER_X, RIGHT_CENTER_Y)`) with a child clip offset by `(-W/2, -H/2)`. Container's anchorPoint is always `(0, 0)`, so its bounds (and viewport-cull rect) didn't match what was actually drawn — could disappear near edges or on resize. Wrapper now sits at top-left with `pos = (RIGHT_X, RIGHT_Y)` and the inner clip at `(0, 0)`; the per-frame pulse scales around the visual center via `translate(cx, cy) → scale → translate(-cx, -cy)` on `currentTransform`. Bounds and visual now agree. - bounds.spec.ts: add a sentinel test that spies on `m.apply` and asserts the identity short-circuit avoids the corner walk. Guards against a future refactor that drops the `isIdentity()` check. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

obiot and others added 2 commits May 7, 2026 15:03

Copilot AI review requested due to automatic review settings May 7, 2026 23:22

Copilot started reviewing on behalf of obiot May 7, 2026 23:22 View session

Copilot AI reviewed May 7, 2026

View reviewed changes

Comment thread packages/melonjs/src/video/webgl/webgl_renderer.js Outdated

Comment thread packages/melonjs/src/video/webgl/webgl_renderer.js Outdated

Comment thread packages/melonjs/src/renderable/container.js Outdated

obiot and others added 2 commits May 8, 2026 07:33

Copilot AI review requested due to automatic review settings May 7, 2026 23:36

Copilot started reviewing on behalf of obiot May 7, 2026 23:37 View session

obiot and others added 2 commits May 8, 2026 07:38

Copilot AI reviewed May 7, 2026

View reviewed changes

Comment thread packages/melonjs/src/video/webgl/webgl_renderer.js

Comment thread packages/examples/src/examples/clipping/ExampleClipping.tsx

obiot and others added 2 commits May 8, 2026 07:43

Copilot AI review requested due to automatic review settings May 7, 2026 23:48

Copilot started reviewing on behalf of obiot May 7, 2026 23:48 View session

obiot mentioned this pull request May 7, 2026

ColorLayer silently ignores pos/scale/parent transform #1436

Open

Copilot AI reviewed May 7, 2026

View reviewed changes

Comment thread packages/melonjs/src/video/renderstate.js

obiot and others added 2 commits May 8, 2026 07:58

Copilot AI review requested due to automatic review settings May 8, 2026 00:32

Copilot started reviewing on behalf of obiot May 8, 2026 00:33 View session

obiot merged commit 06c94b5 into master May 8, 2026
6 checks passed

obiot deleted the fix/cliprect-1349 branch May 8, 2026 00:38

Copilot AI reviewed May 8, 2026

View reviewed changes

obiot mentioned this pull request May 8, 2026

fix(clipping): post-merge Copilot review on #1435 #1437

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(clipping): correct Container.clipping under nested transforms (closes #1349)#1435

fix(clipping): correct Container.clipping under nested transforms (closes #1349)#1435
obiot merged 11 commits intomasterfrom
fix/cliprect-1349

obiot commented May 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		aabb.clear();
		aabb.addFrame(x, y, x + width, y + height, this.currentTransform);

Uh oh!

Conversation

obiot commented May 7, 2026

Summary

What changed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants