diff --git a/packages/melonjs/CHANGELOG.md b/packages/melonjs/CHANGELOG.md index e9a680f84..47572848f 100644 --- a/packages/melonjs/CHANGELOG.md +++ b/packages/melonjs/CHANGELOG.md @@ -26,6 +26,12 @@ - **`moveTowards()` never reached its target — and returned the target instead of `this`** (all four vector classes) — the arrival test compared the remaining distance against `step * step` (wrong units: snapped way too early for steps above 1, dithered around the target forever for fractional steps), and on "arrival" it returned the **caller's target vector** without moving `this` at all — so the vector never landed, and chaining onto the result silently mutated the target. It now lands exactly on the target and always returns `this`, per its own documentation; negative-step flee semantics are unchanged. - **Every preloaded video was fetched with forced anonymous CORS** — the video parser unconditionally stamped the `crossorigin` attribute with the loader's `crossOrigin` setting, which defaults to `undefined` — and per the HTML spec an *invalid* value (the string `"undefined"`) maps to `anonymous`, unlike a *missing* attribute (no CORS). Cross-origin videos served without CORS headers failed to load outright when they would have played fine untainted. The attribute is now only set when `crossOrigin` is actually configured (the empty string remains a valid, honored value). - **Video preloading hung forever on autoplay-restricted browsers (e.g. iOS Safari) unless `autoplay: false` was spelled out** — the parser waited for `canplay`, which those browsers never fire for a video that isn't allowed to play, and only an *explicit* `autoplay: false` opted into the reliable `loadedmetadata` path — so the common manifest shape (no `autoplay` key) stalled the whole preloader and the game never started. Preloading now completes on `loadedmetadata` unless `autoplay: true` explicitly asks to wait for `canplay`. +- **Compressed-texture loading crashed on any device missing one compression family** — six `WEBKIT_`-prefixed extension fallbacks in `getSupportedCompressedTextureFormats()` referenced `this._gl`, a field that is never assigned (the context lives in `this.gl`). The fallback only evaluates when the primary extension is absent, so the whole feature *worked on ANGLE/Metal Macs* (which expose every family) *and threw a TypeError everywhere else* — Windows (no ASTC), iOS (no S3TC), software renderers — killing every `.dds`/`.pvr`/`.pkm`/`.ktx`/`.ktx2` load. +- **Nested multi-effect post-effects corrupted the render-target pool and crashed every frame** — a renderable with 2+ `postEffects` drawn inside a container with 2+ `postEffects` hit a half-guarded nesting path: the inner pass cleared the outer pass's capture target, its `end` popped the outer pass's pool slot, and the outer `endPostEffect` then dereferenced `undefined` (per-frame TypeError; with camera effects active it silently blitted the wrong texture instead). The pool now tracks a proper stack of passes — each nesting level gets its own lazily-allocated capture/ping-pong pair — and the saved effect projection is a per-pass stack too, so nested post-effect passes compose correctly. +- **Gradient shape fills (`fillEllipse`/`fillPolygon`/`fillArc`/`fillRoundRect`) broke inverted and nested masks** — the internal stencil pass gated its write phase on the *hidden* region of an inverted `setMask` (painting the gradient inside the cutout instead of the visible area) and restored a `NOTEQUAL` parity test instead of the mask's actual test — inverting the mask for every subsequent draw, and leaking outside-all-masks pixels for nested masks. The pass now tags the shape's visible pixels with a high stencil bit (collision-free with mask levels), clips the gradient to the tag, and re-installs the exact stencil test `setMask` had established. (`fillRect` gradients were unaffected — they render through a textured quad.) +- **`disableScissor()` didn't flush, so sprites batched inside the scissor escaped it** — GL scissor applies at draw time, not at batch time; every other scissor mutation (`enableScissor`, `clipRect`, `restore`) flushes pending vertices first, but `disableScissor` didn't — so the common `enableScissor → drawImage → disableScissor` pattern clipped nothing: the quads were still queued when the test was turned off. +- **WebGL `clearRect()` painted opaque black instead of erasing** — its JSDoc (and the Canvas renderer) promise transparent black, but the internal `clearColor()` default (`"#000000"`) parses with alpha 1. Visible on `transparent: true` canvases and inside post-effect render targets, where the "erased" region composited as a solid black box. +- **Canvas `clipRect()` skip-optimizations ignored the active transform** — the "full-viewport no-op" and "same-as-last-clip" early-outs compared the rect's raw *local* values, so a canvas-sized clipped container positioned off-origin got no clip at all, and a clipped container nested inside a same-sized clipped container skipped its own clip via the stale cache — both Canvas-only divergences from the transform-aware WebGL scissor path (the #1349 fix). The skip logic now reasons in device space (and always clips under rotation/skew, where an axis-aligned cache can't). - **`Path2D.arc()`/`arcTo()`/`ellipse()` appended to an open path started a new sub-path instead of connecting** — the native Path2D spec joins an arc to the current point with a straight line (`arcTo`'s own documentation even promises it), but all three called `moveTo()` unconditionally. Under this release's new multi-sub-path fill semantics that stray sub-path is treated as a **hole**, so an arc appended to a filled outline punched a hole in the shape. They now connect per the spec; the canonical circle-hole idiom (explicit `moveTo` before `arc()`) still registers a hole, and a virgin-path `arc()` still starts cleanly. - **`Rect.right`/`Rect.bottom` returned the width/height when the edge sat exactly at coordinate 0** — the getters computed `this.left + w || w`, so an edge landing precisely on 0 (falsy) fell back to the size. Bounds and collision math around the origin came out wrong for such rectangles. - **`Rect.toPolygon()` handed out the rectangle's live vertices** — `Polygon.setVertices` stores a `Vector2d[]` by reference, so the "new" polygon shared the rect's actual points array: transforming or mutating the polygon silently corrupted the rectangle. The vertices are now cloned. diff --git a/packages/melonjs/src/video/canvas/canvas_renderer.js b/packages/melonjs/src/video/canvas/canvas_renderer.js index 5614cd726..75c24233c 100644 --- a/packages/melonjs/src/video/canvas/canvas_renderer.js +++ b/packages/melonjs/src/video/canvas/canvas_renderer.js @@ -1255,31 +1255,67 @@ export default class CanvasRenderer extends Renderer { * @param {number} height */ clipRect(x, y, width, height) { + const context = this.getContext(); + const t = context.getTransform(); + + // The two skip-optimizations below (full-viewport no-op and + // same-as-last-clip) must reason in DEVICE space — the rect arrives + // in LOCAL coordinates (e.g. Container clipping translates first, + // then clips at (0, 0)), so comparing raw values against the canvas + // box or the cache is wrong under any transform. The WebGL renderer + // transforms the rect the same way (see the #1349 fix). + if (t.b !== 0 || t.c !== 0) { + // rotated/skewed: the axis-aligned skip logic can't reason about + // the clipped region — always clip, and poison the cache so a + // following axis-aligned call can't false-match. The cache is an + // Int32Array (NaN would coerce to 0!) and widths are normalized + // to >= 0 below, so -1 in the width slot can never match. + context.beginPath(); + context.rect(x, y, width, height); + context.clip(); + this.currentScissor[2] = -1; + return; + } + + // axis-aligned device-space box (scale can be negative — normalize), + // floored/ceiled to integers so the Int32Array cache is exact under + // fractional transforms (same discipline as the WebGL clipRect) + let dx = x * t.a + t.e; + let dy = y * t.d + t.f; + let dw = width * t.a; + let dh = height * t.d; + if (dw < 0) { + dx += dw; + dw = -dw; + } + if (dh < 0) { + dy += dh; + dh = -dh; + } + const ix = Math.floor(dx); + const iy = Math.floor(dy); + const iw = Math.ceil(dx + dw) - ix; + const ih = Math.ceil(dy + dh) - iy; + const canvas = this.getCanvas(); - // if requested box is different from the current canvas size; - if ( - x !== 0 || - y !== 0 || - width !== canvas.width || - height !== canvas.height - ) { + // if the requested box is different from the full canvas; + if (ix !== 0 || iy !== 0 || iw !== canvas.width || ih !== canvas.height) { const currentScissor = this.currentScissor; // if different from the current scissor box if ( - currentScissor[0] !== x || - currentScissor[1] !== y || - currentScissor[2] !== width || - currentScissor[3] !== height + currentScissor[0] !== ix || + currentScissor[1] !== iy || + currentScissor[2] !== iw || + currentScissor[3] !== ih ) { - const context = this.getContext(); context.beginPath(); context.rect(x, y, width, height); context.clip(); - // save the new currentScissor box - currentScissor[0] = x; - currentScissor[1] = y; - currentScissor[2] = width; - currentScissor[3] = height; + // save the new scissor box in device space + currentScissor[0] = ix; + currentScissor[1] = iy; + currentScissor[2] = iw; + currentScissor[3] = ih; } } } diff --git a/packages/melonjs/src/video/rendertarget/render_target_pool.js b/packages/melonjs/src/video/rendertarget/render_target_pool.js index 232f45b56..81c4b96ac 100644 --- a/packages/melonjs/src/video/rendertarget/render_target_pool.js +++ b/packages/melonjs/src/video/rendertarget/render_target_pool.js @@ -7,8 +7,11 @@ * Renderer-agnostic — the actual RenderTarget creation is delegated to a * factory function provided by the renderer (WebGL, WebGPU, etc.). * - * Camera effects use pool indices 0 and 1 (capture + ping-pong), - * sprite effects use indices 2 and 3. + * Camera effects use pool indices 0 and 1 (capture + ping-pong); sprite + * effects use indices 2 and 3, and each NESTED sprite pass (a multi-effect + * renderable drawn inside another multi-effect renderable) gets its own pair + * above that (4/5, 6/7, …), tracked by a stack of active bases so begin/end + * pairs compose like save/restore. * Render targets are lazily created and resized to match the required dimensions. * @ignore */ @@ -21,10 +24,23 @@ export default class RenderTargetPool { this._factory = factory; /** @type {RenderTarget[]} */ this._pool = []; - /** @type {number} */ - this._activeBase = -1; - /** @type {number} */ - this._previousBase = -1; + /** + * active pass bases, innermost last — a STACK, so nested begin/end + * pairs unwind correctly (two scalars silently corrupted the pool on + * nested passes: the inner end() popped the outer pass's slot) + * @type {number[]} + */ + this._baseStack = []; + } + + /** + * The base index of the innermost active pass, or -1 when none. + * @returns {number} + */ + get activeBase() { + return this._baseStack.length > 0 + ? this._baseStack[this._baseStack.length - 1] + : -1; } /** @@ -53,16 +69,24 @@ export default class RenderTargetPool { * @returns {RenderTarget} the capture target (ready to bind) */ begin(isCamera, effectCount, width, height) { - const newBase = isCamera ? 0 : 2; - // guard against nested sprite post-effect passes (not yet supported) - if (!isCamera && this._activeBase === newBase) { - return this.get(this._activeBase, width, height); + let newBase; + if (isCamera) { + newBase = 0; + } else { + // each nested sprite pass gets its own capture/ping-pong pair + // (2/3, then 4/5, 6/7, … — lazily allocated) + let depth = 0; + for (const base of this._baseStack) { + if (base >= 2) { + depth++; + } + } + newBase = 2 + depth * 2; } - this._previousBase = this._activeBase; - this._activeBase = newBase; - const rt = this.get(this._activeBase, width, height); + this._baseStack.push(newBase); + const rt = this.get(newBase, width, height); if (effectCount > 1) { - this.get(this._activeBase + 1, width, height); + this.get(newBase + 1, width, height); } return rt; } @@ -72,10 +96,11 @@ export default class RenderTargetPool { * @returns {RenderTarget|undefined} the capture target, or undefined if no active pass */ getCaptureTarget() { - if (this._activeBase < 0) { + const base = this.activeBase; + if (base < 0) { return undefined; } - return this._pool[this._activeBase]; + return this._pool[base]; } /** @@ -83,10 +108,11 @@ export default class RenderTargetPool { * @returns {RenderTarget|undefined} the ping-pong target, or undefined if no active pass */ getPingPongTarget() { - if (this._activeBase < 0) { + const base = this.activeBase; + if (base < 0) { return undefined; } - return this._pool[this._activeBase + 1]; + return this._pool[base + 1]; } /** @@ -95,10 +121,10 @@ export default class RenderTargetPool { * @returns {RenderTarget|null} the parent target, or null if returning to screen */ end() { - this._activeBase = this._previousBase; - this._previousBase = -1; - if (this._activeBase >= 0 && this._pool[this._activeBase]) { - return this._pool[this._activeBase]; + this._baseStack.pop(); + const base = this.activeBase; + if (base >= 0 && this._pool[base]) { + return this._pool[base]; } return null; } @@ -126,7 +152,6 @@ export default class RenderTargetPool { } } this._pool.length = 0; - this._activeBase = -1; - this._previousBase = -1; + this._baseStack.length = 0; } } diff --git a/packages/melonjs/src/video/webgl/webgl_renderer.js b/packages/melonjs/src/video/webgl/webgl_renderer.js index 30de7c5e4..a052b6764 100644 --- a/packages/melonjs/src/video/webgl/webgl_renderer.js +++ b/packages/melonjs/src/video/webgl/webgl_renderer.js @@ -104,7 +104,11 @@ export default class WebGLRenderer extends Renderer { * @type {Matrix3d} * @ignore */ - this._savedEffectProjection = new Matrix3d(); + // projections saved by beginPostEffect, one per (possibly nested) + // active pass — preallocated slots + a depth counter (zero-alloc + // steady state), matching RenderTargetPool's base stack + this._effectProjectionStack = []; + this._effectPassDepth = 0; /** * sets or returns the thickness of lines for shape drawing @@ -162,6 +166,12 @@ export default class WebGLRenderer extends Renderer { // current gradient state (null when using solid color) this._currentGradient = null; + // the stencil value of currently-VISIBLE pixels under the innermost + // active mask, as installed by setMask (0 for an inverted mask, + // maskLevel otherwise) — the single source #gradientMask gates and + // restores against + this._maskVisibleRef = 0; + /** * The current transformation matrix used for transformations on the overall scene * (alias to renderState.currentTransform for backward compatibility) @@ -314,22 +324,22 @@ export default class WebGLRenderer extends Renderer { supportedCompressedTextureFormats = { astc: gl.getExtension("WEBGL_compressed_texture_astc") || - this._gl.getExtension("WEBKIT_WEBGL_compressed_texture_astc"), + gl.getExtension("WEBKIT_WEBGL_compressed_texture_astc"), bptc: gl.getExtension("EXT_texture_compression_bptc") || - this._gl.getExtension("WEBKIT_EXT_texture_compression_bptc"), + gl.getExtension("WEBKIT_EXT_texture_compression_bptc"), s3tc: gl.getExtension("WEBGL_compressed_texture_s3tc") || - this._gl.getExtension("WEBKIT_WEBGL_compressed_texture_s3tc"), + gl.getExtension("WEBKIT_WEBGL_compressed_texture_s3tc"), s3tc_srgb: gl.getExtension("WEBGL_compressed_texture_s3tc_srgb") || - this._gl.getExtension("WEBKIT_WEBGL_compressed_texture_s3tc_srgb"), + gl.getExtension("WEBKIT_WEBGL_compressed_texture_s3tc_srgb"), pvrtc: gl.getExtension("WEBGL_compressed_texture_pvrtc") || - this._gl.getExtension("WEBKIT_WEBGL_compressed_texture_pvrtc"), + gl.getExtension("WEBKIT_WEBGL_compressed_texture_pvrtc"), etc1: gl.getExtension("WEBGL_compressed_texture_etc1") || - this._gl.getExtension("WEBKIT_WEBGL_compressed_texture_etc1"), + gl.getExtension("WEBKIT_WEBGL_compressed_texture_etc1"), etc2: gl.getExtension("WEBGL_compressed_texture_etc") || gl.getExtension("WEBKIT_WEBGL_compressed_texture_etc") || @@ -388,6 +398,12 @@ export default class WebGLRenderer extends Renderer { reset() { super.reset(); + // drop any pass state orphaned by a mid-pass exception — a stale + // depth would misalign every later pass and inflate the pool's + // nesting forever (the pool's own stack is cleared via its + // destroy() below; the preallocated slots are kept for reuse) + this._effectPassDepth = 0; + // clear gl context this.clear(); @@ -819,8 +835,16 @@ export default class WebGLRenderer extends Renderer { // since FBO construction temporarily changes GL framebuffer bindings this.flush(); this.save(); - // save the current projection (not part of the render state stack) - this._savedEffectProjection.copy(this.projectionMatrix); + // save the current projection (not part of the render state stack) — + // one preallocated slot per nesting depth, so nested passes each + // restore their own without per-frame allocation + let savedProjection = this._effectProjectionStack[this._effectPassDepth]; + if (typeof savedProjection === "undefined") { + savedProjection = this._effectProjectionStack[this._effectPassDepth] = + new Matrix3d(); + } + savedProjection.copy(this.projectionMatrix); + this._effectPassDepth++; const rt = this._renderTargetPool.begin(isCamera, effects.length, w, h); // FBO creation/resize uses TEXTURE0 — invalidate the batcher's cache for that unit @@ -931,7 +955,10 @@ export default class WebGLRenderer extends Renderer { // restore renderer state and projection saved in beginPostEffect this.restore(); - this.projectionMatrix.copy(this._savedEffectProjection); + this._effectPassDepth--; + this.projectionMatrix.copy( + this._effectProjectionStack[this._effectPassDepth], + ); this.currentBatcher.setProjection(this.projectionMatrix); } @@ -1057,6 +1084,13 @@ export default class WebGLRenderer extends Renderer { * Disable the scissor test, allowing rendering to the full viewport. */ disableScissor() { + if (this._scissorActive === true) { + // drain quads batched under the scissor BEFORE turning the test + // off — GL scissor applies at draw time, not at batch time + // (mirrors enableScissor / clipRect / restore, which all flush + // for exactly this reason) + this.flush(); + } this.gl.disable(this.gl.SCISSOR_TEST); this._scissorActive = false; } @@ -1126,7 +1160,10 @@ export default class WebGLRenderer extends Renderer { clearRect(x, y, width, height) { this.save(); this.clipRect(x, y, width, height); - this.clearColor(); + // actual transparent black, per the JSDoc above and the Canvas + // renderer's native clearRect — the bare clearColor() default + // ("#000000") parses with alpha 1 and would paint OPAQUE black + this.clearColor("rgba(0,0,0,0)"); this.restore(); } @@ -2260,7 +2297,6 @@ export default class WebGLRenderer extends Renderer { const gl = this.gl; const grad = this._currentGradient; const hasMask = this.maskLevel > 0; - const stencilRef = hasMask ? this.maskLevel + 1 : 1; this._currentGradient = null; this.flush(); @@ -2268,10 +2304,21 @@ export default class WebGLRenderer extends Renderer { gl.enable(gl.STENCIL_TEST); gl.colorMask(false, false, false, false); + // The stencil value of currently-VISIBLE pixels, as installed by + // setMask (its single source of truth). The shape's visible pixels + // are tagged with a high-bit marker (mask levels only use the low 7 + // bits, so the marker can never collide with one), the gradient is + // clipped to the marker, then the marker is cleared and setMask's + // exact render test is re-installed. + const visibleRef = this._maskVisibleRef; + let markRef = 1; + if (hasMask) { - // nest within existing mask level - gl.stencilFunc(gl.EQUAL, this.maskLevel, 0xff); - gl.stencilOp(gl.KEEP, gl.KEEP, gl.INCR); + markRef = 0x80 | visibleRef; + // tag: only pixels that are visible under the active mask + // (low bits == visibleRef) get the marker written + gl.stencilFunc(gl.EQUAL, markRef, 0x7f); + gl.stencilOp(gl.KEEP, gl.KEEP, gl.REPLACE); } else { gl.clear(gl.STENCIL_BUFFER_BIT); gl.stencilFunc(gl.ALWAYS, 1, 0xff); @@ -2281,9 +2328,9 @@ export default class WebGLRenderer extends Renderer { drawShape(); this.flush(); - // use stencil to clip gradient + // use stencil to clip the gradient to the marked pixels gl.colorMask(true, true, true, true); - gl.stencilFunc(gl.EQUAL, stencilRef, 0xff); + gl.stencilFunc(gl.EQUAL, markRef, 0xff); gl.stencilOp(gl.KEEP, gl.KEEP, gl.KEEP); this._currentGradient = grad; @@ -2291,16 +2338,18 @@ export default class WebGLRenderer extends Renderer { this.flush(); if (hasMask) { - // restore the parent mask level by decrementing stencil + // clear the marker: write the visible value back on the shape's + // pixels (REPLACE writes the func ref — a no-op on unmarked + // visible pixels, and it strips the high bit from marked ones) this._currentGradient = null; gl.colorMask(false, false, false, false); - gl.stencilFunc(gl.EQUAL, stencilRef, 0xff); - gl.stencilOp(gl.KEEP, gl.KEEP, gl.DECR); + gl.stencilFunc(gl.EQUAL, visibleRef, 0x7f); + gl.stencilOp(gl.KEEP, gl.KEEP, gl.REPLACE); drawShape(); this.flush(); gl.colorMask(true, true, true, true); - // restore parent mask stencil test - gl.stencilFunc(gl.NOTEQUAL, this.maskLevel + 1, 1); + // re-install the exact render test setMask had established + gl.stencilFunc(gl.EQUAL, visibleRef, 0xff); gl.stencilOp(gl.KEEP, gl.KEEP, gl.KEEP); this._currentGradient = grad; } else { @@ -2494,6 +2543,18 @@ export default class WebGLRenderer extends Renderer { } this.maskLevel++; + if (this.maskLevel > 0x7f) { + // mask levels live in the stencil's low 7 bits — the high bit is + // reserved as #gradientMask's temporary marker, and an 8-bit + // stencil couldn't represent deeper nesting anyway + this.maskLevel = 0x7f; + if (this._maskDepthWarned !== true) { + this._maskDepthWarned = true; + console.warn( + "melonJS: setMask nesting deeper than 127 — mask level clamped", + ); + } + } // Write phase: increment stencil for the drawn shape's pixels so each // setMask call adds +1 to stencil inside the shape. This lets chained @@ -2524,6 +2585,7 @@ export default class WebGLRenderer extends Renderer { gl.stencilFunc(gl.EQUAL, this.maskLevel, 0xff); } gl.stencilOp(gl.KEEP, gl.KEEP, gl.KEEP); + this._maskVisibleRef = invert === true ? 0 : this.maskLevel; } /** @@ -2535,6 +2597,7 @@ export default class WebGLRenderer extends Renderer { // flush the batcher this.flush(); this.maskLevel = 0; + this._maskVisibleRef = 0; this.gl.disable(this.gl.STENCIL_TEST); } } diff --git a/packages/melonjs/tests/canvas-cliprect-transform.spec.js b/packages/melonjs/tests/canvas-cliprect-transform.spec.js new file mode 100644 index 000000000..fdcad3344 --- /dev/null +++ b/packages/melonjs/tests/canvas-cliprect-transform.spec.js @@ -0,0 +1,108 @@ +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { boot, CanvasRenderer, video } from "../src/index.js"; + +/** + * Reproductions for the Canvas clipRect skip-logic findings (2026-07-08 + * video/GL-core audit), written failing-first. + * + * CanvasRenderer.clipRect makes both of its skip decisions on the RAW local + * rect values, ignoring the active transform: + * (a) a canvas-sized rect is treated as "full viewport → no-op" even when a + * translate has moved it, so a canvas-sized clipped container positioned + * off-origin gets no clip at all; + * (b) the currentScissor cache is only reset by restore(), not save(), so a + * clipped container nested inside another clipped container with the + * same dimensions matches the stale cache and skips its own clip(). + * The WebGL renderer transforms the rect before deciding (the #1349 fix) — + * these are Canvas-only divergences on identical public API. + */ +describe("CanvasRenderer clipRect vs transforms", () => { + let renderer; + let ctx2d; + + beforeAll(async () => { + await boot(); + video.init(64, 64, { parent: "screen", renderer: video.CANVAS }); + renderer = video.renderer; + ctx2d = renderer.getContext(); + expect(renderer).toBeInstanceOf(CanvasRenderer); + }); + + afterAll(() => { + try { + video.init(64, 64, { parent: "screen", renderer: video.AUTO }); + } catch { + // ignore + } + }); + + const pixel = (x, y) => { + return Array.from(ctx2d.getImageData(x, y, 1, 1).data); + }; + + const paintBackground = () => { + ctx2d.save(); + ctx2d.setTransform(1, 0, 0, 1, 0, 0); + ctx2d.fillStyle = "#000000"; + ctx2d.fillRect(0, 0, 64, 64); + ctx2d.restore(); + }; + + it("a canvas-sized clip under a translate still clips (not a full-viewport no-op)", () => { + paintBackground(); + renderer.save(); + renderer.translate(32, 0); + // local rect = full canvas size, but the translate makes it x ∈ [32, 96] + renderer.clipRect(0, 0, 64, 64); + renderer.setColor("#ff0000"); + renderer.fillRect(-32, 0, 128, 64); // covers the whole canvas locally + renderer.restore(); + + // left of the translated clip region must stay black + expect(pixel(5, 5)[0]).toBeLessThan(50); + // inside the clip region the fill landed + expect(pixel(40, 5)[0]).toBeGreaterThan(200); + }); + + it("an axis-aligned clip after a ROTATED clip is not skipped via a stale cache", () => { + // the rotated path can't cache an axis-aligned box, so it must poison + // the cache — an inert poison (e.g. NaN into an Int32Array, which + // coerces to 0) leaves a frankenstein box behind that a later + // axis-aligned clip can false-match, skipping its own clip() + paintBackground(); + renderer.save(); + renderer.clipRect(2, 2, 60, 60); // cache [2,2,60,60], x ∈ [2,62] + renderer.rotate(0.3); + renderer.clipRect(-20, -20, 120, 120); // rotated: huge box, ≈ no-op clip + renderer.rotate(-0.3); + // broken poison → cache [0,2,60,60]; this request matches it exactly + // and gets skipped, leaving x clipped to 62 instead of 60 + renderer.clipRect(0, 2, 60, 60); + renderer.setColor("#ff0000"); + renderer.fillRect(0, 0, 64, 64); + renderer.restore(); + + // x ∈ (60, 62] must be clipped by the last requested box + expect(pixel(61, 30)[0]).toBeLessThan(50); + // inside every clip: painted + expect(pixel(30, 30)[0]).toBeGreaterThan(200); + }); + + it("a nested same-size clip is not skipped via the stale scissor cache", () => { + paintBackground(); + renderer.save(); + renderer.clipRect(0, 0, 32, 32); // outer clip [0,32]² + renderer.save(); + renderer.translate(16, 16); + renderer.clipRect(0, 0, 32, 32); // inner clip → [16,48]² ∩ outer = [16,32]² + renderer.setColor("#ff0000"); + renderer.fillRect(-16, -16, 64, 64); // covers the whole canvas locally + renderer.restore(); + renderer.restore(); + + // inside outer but OUTSIDE inner: must stay black (inner clip applies) + expect(pixel(8, 8)[0]).toBeLessThan(50); + // inside both clips: painted + expect(pixel(20, 20)[0]).toBeGreaterThan(200); + }); +}); diff --git a/packages/melonjs/tests/glcore-audit.spec.js b/packages/melonjs/tests/glcore-audit.spec.js new file mode 100644 index 000000000..4268dfc77 --- /dev/null +++ b/packages/melonjs/tests/glcore-audit.spec.js @@ -0,0 +1,262 @@ +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { + boot, + Matrix3d, + Rect, + ShaderEffect, + video, + WebGLRenderer, +} from "../src/index.js"; + +/** + * Reproductions for the video/GL-core audit findings (2026-07-08), written + * failing-first: every test below demonstrates its bug on the unfixed code. + * + * 1. getSupportedCompressedTextureFormats references the never-assigned + * `this._gl` in its WEBKIT fallbacks — throws on any device missing at + * least one compression family (i.e. everything except ANGLE/Metal Macs). + * 2. WebGL clearRect "erases" with OPAQUE black (its JSDoc promises + * transparent black; the Canvas renderer genuinely erases). + * 3. disableScissor() doesn't flush, so quads batched inside the scissor + * render unclipped after it's turned off. + * 4. #gradientMask mis-gates its stencil phases under an INVERTED mask and + * restores a parity test that inverts/leaks subsequent draws. + * 5. Nested multi-effect post-effect passes corrupt the RenderTargetPool: + * the parent's endPostEffect dereferences undefined and throws. + */ +describe("video/GL core audit reproductions", () => { + let renderer; + let gl; + + beforeAll(async () => { + await boot(); + try { + video.init(64, 64, { + parent: "screen", + renderer: video.WEBGL, + // alpha readback for the clearRect test needs a transparent + // backbuffer; SwiftShader needs the caveat opt-out + transparent: true, + failIfMajorPerformanceCaveat: false, + }); + } catch { + // no WebGL at all — tests skip below + } + if (video.renderer instanceof WebGLRenderer) { + renderer = video.renderer; + gl = renderer.gl; + } + }); + + afterAll(() => { + try { + video.init(64, 64, { parent: "screen", renderer: video.AUTO }); + } catch { + // ignore + } + }); + + const requireWebGL = (ctx) => { + if (renderer === undefined) { + ctx.skip("WebGL renderer not available in this environment"); + } + }; + + const setupProjection = () => { + const proj = new Matrix3d(); + proj.ortho(0, 64, 64, 0, -1000, 1000); + renderer.setProjection(proj); + }; + + // canvas-space pixel read (readPixels is y-up) + const readPixel = (x, y) => { + const px = new Uint8Array(4); + gl.finish(); + gl.readPixels(x, 64 - 1 - y, 1, 1, gl.RGBA, gl.UNSIGNED_BYTE, px); + return px; + }; + + const whiteTex = () => { + const c = document.createElement("canvas"); + c.width = 4; + c.height = 4; + const ctx2d = c.getContext("2d"); + ctx2d.fillStyle = "#ffffff"; + ctx2d.fillRect(0, 0, 4, 4); + return c; + }; + + it("getSupportedCompressedTextureFormats survives a missing extension family (the this._gl fallbacks)", (ctx) => { + requireWebGL(ctx); + // force at least one family's primary lookup to miss, so the WEBKIT + // fallback path actually evaluates — on the unfixed code that path + // dereferences the never-assigned `this._gl` and throws + const orig = gl.getExtension.bind(gl); + const spy = vi.spyOn(gl, "getExtension").mockImplementation((name) => { + if (name === "WEBGL_compressed_texture_astc") { + return null; + } + return orig(name); + }); + try { + let formats; + expect(() => { + formats = renderer.getSupportedCompressedTextureFormats(); + }).not.toThrow(); + expect(typeof formats).toBe("object"); + } finally { + spy.mockRestore(); + } + }); + + it("clearRect erases to TRANSPARENT black, per its own JSDoc (parity with Canvas)", (ctx) => { + requireWebGL(ctx); + setupProjection(); + renderer.setColor("#ff0000"); + renderer.fillRect(10, 10, 20, 20); + renderer.flush(); + + renderer.clearRect(12, 12, 4, 4); + + // inside the cleared area: must be fully transparent, not opaque black + const cleared = readPixel(13, 13); + expect(cleared[3]).toBe(0); + // outside the cleared area: the red fill is untouched + const kept = readPixel(25, 25); + expect(kept[0]).toBeGreaterThan(200); + expect(kept[3]).toBe(255); + }); + + it("disableScissor flushes pending quads so they can't escape the scissor", (ctx) => { + requireWebGL(ctx); + setupProjection(); + // black background + renderer.setColor("#000000"); + renderer.fillRect(0, 0, 64, 64); + renderer.flush(); + + // draw a sprite fully OUTSIDE the scissor box, then disable the + // scissor while the quad is still batched — it must NOT render + renderer.enableScissor(0, 0, 8, 8); + renderer.drawImage(whiteTex(), 40, 40); + renderer.disableScissor(); + renderer.flush(); + + const px = readPixel(42, 42); + expect(px[0]).toBeLessThan(50); // stays black — the quad was clipped + }); + + it("gradient shape fills respect an INVERTED mask, and later draws keep the mask test", (ctx) => { + requireWebGL(ctx); + setupProjection(); + // black background + renderer.setColor("#000000"); + renderer.fillRect(0, 0, 64, 64); + renderer.flush(); + + // inverted mask: visible OUTSIDE the center box + renderer.setMask(new Rect(16, 16, 32, 32), true); + + // a solid-red gradient (color constant → assertions are exact). + // NOTE: fillEllipse, not fillRect — the rect fill renders gradients + // as a plain textured quad (no #gradientMask stencil pass); the + // shape fills (ellipse/polygon/arc/roundRect) are the stencil path. + const grad = renderer.createLinearGradient(0, 0, 64, 0); + grad.addColorStop(0, "#ff0000").addColorStop(1, "#ff0000"); + renderer.setColor(grad); + renderer.fillEllipse(32, 32, 30, 30); + renderer.flush(); + + // the gradient ellipse must land in the VISIBLE (outside) region only + const outside = readPixel(6, 32); // inside ellipse, outside mask box + const inside = readPixel(32, 32); // inside ellipse AND mask box + expect(outside[0]).toBeGreaterThan(200); // red outside the cutout + expect(inside[0]).toBeLessThan(50); // cutout interior untouched + + // draws AFTER the gradient must still obey the inverted mask + renderer.setColor("#00ff00"); + renderer.fillRect(0, 0, 64, 64); + renderer.flush(); + const outside2 = readPixel(4, 4); + const inside2 = readPixel(32, 32); + expect(outside2[1]).toBeGreaterThan(200); // green outside + expect(inside2[1]).toBeLessThan(50); // interior still masked + + renderer.clearMask(); + }); + + // LAST on purpose: on the unfixed code this corrupts renderer/pool state + it("nested multi-effect post-effect passes don't corrupt the render-target pool", (ctx) => { + requireWebGL(ctx); + const fx = () => { + const effect = new ShaderEffect( + renderer, + "vec4 apply(vec4 color, vec2 uv) { return color; }", + ); + return effect; + }; + // duck-typed renderables — beginPostEffect/endPostEffect read exactly + // these two fields (same shape Renderable.preDraw/postDraw provide) + const parent = { + postEffects: [fx(), fx()], + _postEffectManaged: false, + }; + const child = { + postEffects: [fx(), fx()], + _postEffectManaged: false, + }; + + // the exact call order Container.draw produces for a 2-effect + // container holding a 2-effect child + renderer.beginPostEffect(parent); + renderer.beginPostEffect(child); + renderer.endPostEffect(child); + expect(() => { + renderer.endPostEffect(parent); + }).not.toThrow(); + + for (const r of [parent, child]) { + for (const effect of r.postEffects) { + effect.destroy(); + } + } + }); + + it("mask nesting depth is clamped below the gradient marker bit", (ctx) => { + requireWebGL(ctx); + // #gradientMask reserves stencil bit 0x80 as its temporary marker — + // mask levels must stay in the low 7 bits or the marker can collide + // (an 8-bit stencil couldn't represent deeper nesting anyway) + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + for (let i = 0; i < 130; i++) { + renderer.setMask(new Rect(0, 0, 4, 4)); + } + expect(renderer.maskLevel).toBeLessThan(0x80); + } finally { + renderer.clearMask(); + warnSpy.mockRestore(); + } + }); + + it("reset() clears the effect-projection stack (mid-pass exception recovery)", (ctx) => { + requireWebGL(ctx); + // simulate a draw throwing between begin and end: the pass never + // ends, leaving a stale entry — reset() (GAME_RESET / context + // restore) must clear it or every later pass leaks a matrix and + // the pool's nesting depth inflates forever + const effect = new ShaderEffect( + renderer, + "vec4 apply(vec4 color, vec2 uv) { return color; }", + ); + const orphan = { + postEffects: [effect, effect], + _postEffectManaged: false, + }; + renderer.beginPostEffect(orphan); + expect(renderer._effectPassDepth).toBe(1); + renderer.reset(); + expect(renderer._effectPassDepth).toBe(0); + effect.destroy(); + }); +}); diff --git a/packages/melonjs/tests/renderTargetPool.spec.js b/packages/melonjs/tests/renderTargetPool.spec.js index 6de90e974..561f8910e 100644 --- a/packages/melonjs/tests/renderTargetPool.spec.js +++ b/packages/melonjs/tests/renderTargetPool.spec.js @@ -30,19 +30,19 @@ describe("RenderTargetPool", () => { }); it("should default activeBase to -1", () => { - expect(pool._activeBase).toEqual(-1); + expect(pool.activeBase).toEqual(-1); }); }); describe("begin()", () => { it("should set activeBase to 0 for camera", () => { pool.begin(true, 1, 100, 100); - expect(pool._activeBase).toEqual(0); + expect(pool.activeBase).toEqual(0); }); it("should set activeBase to 2 for sprites", () => { pool.begin(false, 1, 100, 100); - expect(pool._activeBase).toEqual(2); + expect(pool.activeBase).toEqual(2); }); it("should create render target via factory", () => { @@ -73,7 +73,7 @@ describe("RenderTargetPool", () => { }); it("should return the correct pool index for camera", () => { - pool._activeBase = 0; + pool._baseStack.push(0); pool._pool[0] = { id: "capture-cam" }; pool._pool[1] = { id: "pingpong-cam" }; @@ -82,7 +82,7 @@ describe("RenderTargetPool", () => { }); it("should return the correct pool index for sprite", () => { - pool._activeBase = 2; + pool._baseStack.push(2); pool._pool[2] = { id: "capture-sprite" }; pool._pool[3] = { id: "pingpong-sprite" }; @@ -96,26 +96,57 @@ describe("RenderTargetPool", () => { pool._pool[2] = { id: "spr0" }; pool._pool[3] = { id: "spr1" }; - pool._activeBase = 0; + pool._baseStack.push(0); expect(pool.getCaptureTarget().id).toEqual("cam0"); - pool._activeBase = 2; + pool._baseStack.push(2); expect(pool.getCaptureTarget().id).toEqual("spr0"); // camera targets still intact - pool._activeBase = 0; + pool._baseStack.push(0); expect(pool.getCaptureTarget().id).toEqual("cam0"); }); }); + describe("nested sprite passes (the 2026-07 GL-core audit fix)", () => { + it("allocates a distinct pair per nesting level and unwinds like a stack", () => { + // pre-fix, the pool tracked two scalars: a nested sprite begin() + // early-returned without pushing, the inner end() popped the + // OUTER pass's slot, and the outer endPostEffect dereferenced + // undefined — a per-frame crash for a 2-effect renderable drawn + // inside a 2-effect container + const cam = pool.begin(true, 2, 100, 100); + const outer = pool.begin(false, 2, 100, 100); + expect(pool.activeBase).toEqual(2); + const inner = pool.begin(false, 2, 100, 100); + expect(pool.activeBase).toEqual(4); // its own pair, above the outer + expect(inner).not.toBe(outer); + + expect(pool.end()).toBe(outer); // inner end → back to the outer pass + expect(pool.activeBase).toEqual(2); + expect(pool.end()).toBe(cam); // outer end → back to the camera pass + expect(pool.end()).toBeNull(); // camera end → back to the screen + expect(pool.activeBase).toEqual(-1); + }); + + it("getCaptureTarget/getPingPongTarget follow the innermost pass", () => { + pool.begin(false, 2, 100, 100); + const outerCapture = pool.getCaptureTarget(); + pool.begin(false, 2, 100, 100); + expect(pool.getCaptureTarget()).not.toBe(outerCapture); + pool.end(); + expect(pool.getCaptureTarget()).toBe(outerCapture); + }); + }); + describe("end()", () => { it("should restore previous activeBase", () => { pool.begin(true, 1, 100, 100); pool.begin(false, 1, 100, 100); - expect(pool._activeBase).toEqual(2); + expect(pool.activeBase).toEqual(2); const parent = pool.end(); - expect(pool._activeBase).toEqual(0); + expect(pool.activeBase).toEqual(0); expect(parent).toBe(pool._pool[0]); });