Skip to content

fix(tests): WebGL save/restore assertions for the actual 4x4 matrix layout (#1481) #1483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
obiot merged 1 commit into from
Jun 1, 2026
+55 −73
Merged

fix(tests): WebGL save/restore assertions for the actual 4x4 matrix layout (#1481) #1483

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 55 additions & 73 deletions packages/melonjs/tests/webgl_save_restore.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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();

Expand All @@ -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();

Expand All @@ -76,30 +83,25 @@ 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);
renderer.rotate(Math.PI / 4);
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);
Expand All @@ -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");

Expand All @@ -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();
Expand All @@ -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);

Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand All @@ -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);
Expand All @@ -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");
Expand All @@ -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();
Expand All @@ -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);
Expand Down
Loading