diff --git a/packages/melonjs/tests/webgl_save_restore.spec.js b/packages/melonjs/tests/webgl_save_restore.spec.js index 02d6419df..dbb3d6255 100644 --- a/packages/melonjs/tests/webgl_save_restore.spec.js +++ b/packages/melonjs/tests/webgl_save_restore.spec.js @@ -9,6 +9,12 @@ import { boot, video, WebGLRenderer } from "../src/index.js"; * and edge cases. These tests should catch regressions if the internal * stack implementation is changed (e.g. from pool-based cloning to * pre-allocated index-based stacks). + * + * Each test calls `ctx.skip("WebGL not available")` (not bare `return`) + * when the test environment doesn't expose a WebGL context — the bare + * return pattern silently registered as `passed` in the reporter, which + * is how the broken-since-day-one matrix-index assertions in #1481 + * slipped past CI for months. A real "skipped" status surfaces the gap. */ describe("WebGL Renderer save/restore", () => { let renderer; @@ -20,7 +26,7 @@ describe("WebGL Renderer save/restore", () => { parent: "screen", scale: "auto", renderer: video.AUTO, - failIfMajorPerformanceCaveat: true, + failIfMajorPerformanceCaveat: false, }); renderer = video.renderer; isWebGL = renderer instanceof WebGLRenderer; @@ -31,16 +37,19 @@ describe("WebGL Renderer save/restore", () => { parent: "screen", scale: "auto", renderer: video.AUTO, - failIfMajorPerformanceCaveat: true, }); }); - // ---- Color state (available on all renderers) ---- - - it("should preserve color across save/restore", () => { + const requireWebGL = (ctx) => { if (!isWebGL) { - return; + ctx.skip("WebGL renderer not available in this environment"); } + }; + + // ---- Color state (available on all renderers) ---- + + it("should preserve color across save/restore", (ctx) => { + requireWebGL(ctx); renderer.setColor("#ff0000"); const before = renderer.currentColor.toArray().slice(); @@ -56,10 +65,8 @@ describe("WebGL Renderer save/restore", () => { expect(after[3]).toBeCloseTo(before[3], 5); }); - it("should preserve color alpha across save/restore", () => { - if (!isWebGL) { - return; - } + it("should preserve color alpha across save/restore", (ctx) => { + requireWebGL(ctx); renderer.setColor("rgba(100, 200, 50, 0.5)"); const before = renderer.currentColor.toArray().slice(); @@ -76,10 +83,8 @@ describe("WebGL Renderer save/restore", () => { // ---- Transform state (WebGL only — Canvas delegates to native context) ---- - it("should preserve transform across save/restore", () => { - if (!isWebGL) { - return; - } + it("should preserve transform across save/restore", (ctx) => { + requireWebGL(ctx); renderer.currentTransform.identity(); renderer.save(); renderer.translate(100, 200); @@ -87,19 +92,16 @@ describe("WebGL Renderer save/restore", () => { renderer.scale(2, 3); renderer.restore(); - const val = renderer.currentTransform.val; - expect(val[0]).toBeCloseTo(1, 5); - expect(val[1]).toBeCloseTo(0, 5); - expect(val[3]).toBeCloseTo(0, 5); - expect(val[4]).toBeCloseTo(1, 5); - expect(val[6]).toBeCloseTo(0, 5); - expect(val[7]).toBeCloseTo(0, 5); + // `Matrix3d` is a 4x4 column-major matrix (16 floats). The + // original assertions on this line indexed as if it were a + // 3x3 (val[4] = 1 for identity), which was silently wrong for + // years — see #1481 for the post-mortem. `isIdentity()` is + // layout-agnostic and the actual property we care about. + expect(renderer.currentTransform.isIdentity()).toBe(true); }); - it("should preserve translate across save/restore", () => { - if (!isWebGL) { - return; - } + it("should preserve translate across save/restore", (ctx) => { + requireWebGL(ctx); renderer.currentTransform.identity(); renderer.translate(50, 75); const before = Float64Array.from(renderer.currentTransform.val); @@ -116,10 +118,8 @@ describe("WebGL Renderer save/restore", () => { // ---- Blend mode state ---- - it("should preserve blend mode across save/restore", () => { - if (!isWebGL) { - return; - } + it("should preserve blend mode across save/restore", (ctx) => { + requireWebGL(ctx); renderer.setBlendMode("normal"); expect(renderer.getBlendMode()).toBe("normal"); @@ -131,10 +131,8 @@ describe("WebGL Renderer save/restore", () => { expect(renderer.getBlendMode()).toBe("normal"); }); - it("should preserve blend mode through multiple modes", () => { - if (!isWebGL) { - return; - } + it("should preserve blend mode through multiple modes", (ctx) => { + requireWebGL(ctx); renderer.setBlendMode("additive"); renderer.save(); @@ -151,10 +149,8 @@ describe("WebGL Renderer save/restore", () => { // ---- Scissor state (WebGL only) ---- - it("should preserve scissor across save/restore", () => { - if (!isWebGL) { - return; - } + it("should preserve scissor across save/restore", (ctx) => { + requireWebGL(ctx); renderer.clipRect(10, 20, 100, 200); const before = Int32Array.from(renderer.currentScissor); @@ -173,10 +169,8 @@ describe("WebGL Renderer save/restore", () => { // ---- Nested save/restore ---- - it("should handle nested save/restore correctly", () => { - if (!isWebGL) { - return; - } + it("should handle nested save/restore correctly", (ctx) => { + requireWebGL(ctx); // set initial state renderer.currentTransform.identity(); renderer.setColor("#ff0000"); @@ -223,20 +217,16 @@ describe("WebGL Renderer save/restore", () => { expect(renderer.currentColor.g).toBe(0); expect(renderer.getBlendMode()).toBe("normal"); - // transform should be identity again - const val = renderer.currentTransform.val; - expect(val[0]).toBeCloseTo(1, 5); - expect(val[4]).toBeCloseTo(1, 5); - expect(val[6]).toBeCloseTo(0, 5); - expect(val[7]).toBeCloseTo(0, 5); + // transform should be identity again — see #1481 for why we use + // the layout-agnostic `isIdentity()` here instead of indexing + // hardcoded matrix slots. + expect(renderer.currentTransform.isIdentity()).toBe(true); }); // ---- Deep nesting (stress test) ---- - it("should handle deep nesting (20 levels)", () => { - if (!isWebGL) { - return; - } + it("should handle deep nesting (20 levels)", (ctx) => { + requireWebGL(ctx); const depth = 20; renderer.currentTransform.identity(); renderer.setColor("#ff0000"); @@ -256,19 +246,15 @@ describe("WebGL Renderer save/restore", () => { expect(renderer.currentColor.g).toBe(0); expect(renderer.currentColor.b).toBe(0); - const val = renderer.currentTransform.val; - expect(val[0]).toBeCloseTo(1, 5); - expect(val[4]).toBeCloseTo(1, 5); - expect(val[6]).toBeCloseTo(0, 5); - expect(val[7]).toBeCloseTo(0, 5); + // transform should be identity again — see #1481 for why we use + // `isIdentity()` instead of hardcoded matrix-slot indices. + expect(renderer.currentTransform.isIdentity()).toBe(true); }); // ---- Edge cases ---- - it("should handle restore with no matching save (no-op)", () => { - if (!isWebGL) { - return; - } + it("should handle restore with no matching save (no-op)", (ctx) => { + requireWebGL(ctx); renderer.setColor("#ff0000"); const colorBefore = renderer.currentColor.toArray().slice(); const transformBefore = Float64Array.from(renderer.currentTransform.val); @@ -286,10 +272,8 @@ describe("WebGL Renderer save/restore", () => { } }); - it("should handle save/restore with no state changes in between", () => { - if (!isWebGL) { - return; - } + it("should handle save/restore with no state changes in between", (ctx) => { + requireWebGL(ctx); renderer.currentTransform.identity(); renderer.setColor("#abcdef"); renderer.setBlendMode("normal"); @@ -311,10 +295,8 @@ describe("WebGL Renderer save/restore", () => { expect(renderer.getBlendMode()).toBe("normal"); }); - it("should isolate state between sequential save/restore pairs", () => { - if (!isWebGL) { - return; - } + it("should isolate state between sequential save/restore pairs", (ctx) => { + requireWebGL(ctx); // first pair: red + translated renderer.setColor("#ff0000"); renderer.currentTransform.identity(); @@ -335,15 +317,15 @@ describe("WebGL Renderer save/restore", () => { expect(renderer.currentColor.r).toBe(255); expect(renderer.currentColor.g).toBe(0); expect(renderer.currentColor.b).toBe(0); - expect(renderer.currentTransform.val[6]).toBeCloseTo(0, 5); + // transform should be back to identity — see #1481 for why we + // use `isIdentity()` instead of probing one matrix slot. + expect(renderer.currentTransform.isIdentity()).toBe(true); }); // ---- Combined state integrity ---- - it("should restore ALL state properties simultaneously", () => { - if (!isWebGL) { - return; - } + it("should restore ALL state properties simultaneously", (ctx) => { + requireWebGL(ctx); // set a known initial state renderer.currentTransform.identity(); renderer.translate(42, 84);