diff --git a/packages/melonjs/CHANGELOG.md b/packages/melonjs/CHANGELOG.md index fdd191650..1fe90edb4 100644 --- a/packages/melonjs/CHANGELOG.md +++ b/packages/melonjs/CHANGELOG.md @@ -65,6 +65,8 @@ The old path scales linearly with vertex count; the new one is flat, because no - **the cost of `antiAlias: true` under post effects, quantified** ([#1556](https://github.com/melonjs/melonJS/issues/1556)) — MSAA composing through effect chains (see *Added*) is paid for in memory and bandwidth, and the price is worth knowing. Arithmetic, not measurement: a 4× capture target keeps 4 color + 4 depth-stencil samples per pixel next to its 1× resolve texture — roughly **28 extra bytes per pixel on WebGL (~55 MB of GPU memory at 1080p)** and **~16 bytes per pixel on WebGPU (~32 MB at 1080p)**, where the multisampled depth attachment is shared with the canvas rather than per-target; both scale linearly with resolution. Per frame it adds one resolve blit per effect bracket, and draws inside the bracket write up to 4 samples per covered pixel — bandwidth, not shading cost, since fragment shaders still run once per pixel under MSAA. Only scene **capture** targets pay any of this (ping-pong intermediates stay 1×), and with `antiAlias: false` — the default — no multisampled storage exists at all, so nothing changes ### Fixed +- **destroyed renderers stayed subscribed to global events forever** — `WebGLRenderer` subscribed to `GAME_RESET`, `ONCONTEXT_RESTORED` and `CANVAS_ONRESIZE`, and `CanvasRenderer` to `GAME_RESET`, all as **inline anonymous handlers** — which cannot be passed to `off()`, so nothing could ever unregister them. `CanvasRenderer` had no `destroy()` at all, inheriting the base no-op. Two consequences, both silent: a destroyed renderer kept reacting to those events, and — worse — each handler closes over the renderer, so the subscription pinned the renderer, its batchers and its GPU objects against garbage collection. Releasing the GL context alone did not help, because the JS graph was still reachable from the event bus. The same shape was in the scene graph: the **root `Container`** subscribed to `CANVAS_ONRESIZE` with an inline arrow, and **`World`** to `GAME_RESET` (with a context) and `LEVEL_LOADED` (inline), none of them ever removed — and `World` had no `destroy()` of its own, so a torn-down world kept resetting itself and clearing a broadphase nobody read. All of these are now per-instance fields and `destroy()` unregisters them, matching what the WebGPU backend already did. Any application that tears down and rebuilds — an SPA moving between scenes, a level reload — stops accumulating them +- **`Application.destroy()` leaked the WebGL context** — teardown deleted every GL object the renderer owned and removed the canvas from the DOM, but never handed back the **context** itself. A canvas keeps its context until the canvas is garbage-collected, which is non-deterministic and routinely delayed, so each destroyed application left a live context behind. Browsers cap how many they keep — around 16 on Chromium — and force-lose the oldest past that, which means a long-lived page that builds and tears down several applications accumulates dead-but-unfreed contexts until an unrelated later `getContext` stalls or comes back already lost. That hits any single-page app that moves between scenes or unmounts a game view (the examples gallery does exactly this on every navigation), and it was also making unrelated test suites time out in CI. `destroy()` now releases the context through `WEBGL_lose_context`. It stays idempotent, and since `destroy()` is already terminal — `Application.init()` refuses to run again afterwards — losing the context forecloses nothing that was previously possible. Renderers whose driver does not expose the extension are unaffected - **the 3D broadphase silently dropped collisions between bodies at different depths** — under `Camera3d` the world's broadphase is an `Octree`, and `retrieve()` — the candidate feed for SAT collision, pointer picking, the 2D raycast and `adapter.queryAABB` — descended only into the octant the query item itself classified into. But every one of those consumers decides overlap in the **XY plane**: two bodies at different z that overlap in XY genuinely collide, and were never offered to each other as candidates. Whether a given pair was tested came down to which side of an octant boundary each happened to fall on. Measured on a randomized 300-body scene, **12 of 20 genuinely overlapping pairs were never surfaced**. `retrieve()` is now depth-blind: it classifies on x/y only and walks both depth halves of that quadrant, so x/y pruning still applies at every level and in both halves (an item lying wholly inside a different x/y quadrant cannot overlap, and midpoint-straddling items already live at the parent level). On a 600-body scene this costs nothing at all when the bodies share a gameplay plane — the candidate count is unchanged — and on a depth-spread scene it settles at the same candidate count as the flat one, which is the point: with depth no longer part of the decision, the candidate set depends only on the x/y distribution. The genuinely 3D queries are unaffected and still prune on depth: `queryAABB`, `querySphere`, `queryRay` and `queryFrustum` have their own entry points, and each is now pinned by a differential test against a brute-force scan. Note this removes the incidental "parallax at a distant z drops out of collision for free" behaviour that the 2.5D documentation described as best-effort — it was this defect seen from its good side. Exclude parallax deliberately instead, with `isKinematic = true` or `collisionType` / `collisionMask`, which is what the 2D path has always done - **an entire 2.5D gameplay plane sat unpartitioned at the root of the octree** — `getIndex` returned −1 (meaning "straddles a midpoint, keep at this level") for an item sitting *exactly* on one. On x and y that is at least defensible, since an item there may genuinely span the boundary; on z it never is, because items are point-z in the broadphase and a point cannot straddle anything. It mattered because the root box is origin-centred, so its midpoints are `(0, 0, 0)` — the default `pos` of every renderable, and the shared gameplay z that the 2.5D recipe prescribes. Measured: 200 bodies on a `z = 0` plane all stayed at the root and `retrieve()` returned **200 of 200**, degrading the broadphase to a linear scan for exactly the layer holding the most bodies; the same 200 spread across z left only 10 at the root. Classification is now exact on all three axes — a midpoint belongs to the far/right/bottom child, and an item whose far edge merely touches one still counts as wholly inside the near side. Genuine straddlers and out-of-bounds items still stay at the parent, both under regression test. This is invisible to 2D games, which use a `QuadTree` and never construct an `Octree` - **a mesh marked `lit` with no usable normals rendered solid black** — normalizing a zero-length normal yields NaN, which the shader turned into black fragments rather than something recognisable. That happens whenever `lit: true` meets geometry with no normals, and on the 2D-camera path generally, where world normals are never written. Such a mesh now degrades to **unlit** on both GPU backends: wrong, but recognisably the model instead of a hole in the scene. Note this makes the failure legible, it does not make a `Camera2d` mesh light — populating world normals on that path is tracked separately as [#1576](https://github.com/melonjs/melonJS/issues/1576) and remains open diff --git a/packages/melonjs/src/physics/world.js b/packages/melonjs/src/physics/world.js index 8df6ba713..9327a54d3 100644 --- a/packages/melonjs/src/physics/world.js +++ b/packages/melonjs/src/physics/world.js @@ -4,6 +4,7 @@ import { emit, GAME_RESET, LEVEL_LOADED, + off, on, WORLD_STEP, } from "../system/event.ts"; @@ -160,13 +161,33 @@ export default class World extends Container { // clears contents because the Octree root is a fixed // origin-centred box that doesn't depend on the level's 2D // extent. - on(LEVEL_LOADED, () => { + // Held as a field, not an inline arrow, so `destroy()` can pass it to + // `off()` — an anonymous handler is unremovable, and its closure keeps + // this world (and its whole child tree and broadphase) reachable from + // the event bus for the life of the page. + this.onLevelLoaded = () => { if (this._sortOn === "depth") { this.broadphase.clear(); } else { this.broadphase.clear(this.getBounds().clone()); } - }); + }; + on(LEVEL_LOADED, this.onLevelLoaded); + } + + /** + * Release this world's global event subscriptions before handing off to + * the container teardown. Without this a destroyed world stays on the + * bus: it keeps resetting itself on `GAME_RESET` and clearing a + * broadphase nobody reads on `LEVEL_LOADED`, and it cannot be garbage + * collected because both handlers close over it. + * @ignore + */ + destroy() { + off(GAME_RESET, this.reset, this); + off(LEVEL_LOADED, this.onLevelLoaded); + this.onLevelLoaded = undefined; + super.destroy(...arguments); } /** diff --git a/packages/melonjs/src/renderable/container.js b/packages/melonjs/src/renderable/container.js index f2a347a9e..52f65ff6a 100644 --- a/packages/melonjs/src/renderable/container.js +++ b/packages/melonjs/src/renderable/container.js @@ -1,7 +1,7 @@ import { colorPool } from "../math/color.ts"; import Body from "../physics/builtin/body.js"; import state from "../state/state.ts"; -import { CANVAS_ONRESIZE, on } from "../system/event.ts"; +import { CANVAS_ONRESIZE, off, on } from "../system/event.ts"; import pool from "../system/legacy_pool.js"; import { defer } from "../utils/function"; import { createGUID } from "../utils/utils"; @@ -244,14 +244,19 @@ export default class Container extends Renderable { // subscribe on the canvas resize event if (this.root === true) { + // Held as a field, not an inline arrow, so `destroy()` can pass it + // to `off()` — an anonymous handler is unremovable, and its closure + // would keep this container (and its whole child tree) reachable + // from the event bus for the life of the page. // Workaround for not updating container child-bounds automatically (it's expensive!) - on(CANVAS_ONRESIZE, () => { + this.onCanvasResize = () => { // temporarly enable the enableChildBoundsUpdate flag // this.enableChildBoundsUpdate === true; // update bounds this.updateBounds(); // this.enableChildBoundsUpdate === false; - }); + }; + on(CANVAS_ONRESIZE, this.onCanvasResize); } } @@ -1098,6 +1103,16 @@ export default class Container extends Renderable { * @ignore */ destroy() { + // drop the root container's resize subscription before anything else — + // the handler closes over `this`, so leaving it registered keeps the + // container and its entire child tree reachable from the event bus, + // and a destroyed container would still try to update its bounds on + // every canvas resize. Only root containers ever subscribe. + if (this.onCanvasResize) { + off(CANVAS_ONRESIZE, this.onCanvasResize); + this.onCanvasResize = undefined; + } + // empty the container this.reset(); // call the parent destroy method, spreading the actual arguments — diff --git a/packages/melonjs/src/video/canvas/canvas_renderer.js b/packages/melonjs/src/video/canvas/canvas_renderer.js index dd637c1e6..a85153c84 100644 --- a/packages/melonjs/src/video/canvas/canvas_renderer.js +++ b/packages/melonjs/src/video/canvas/canvas_renderer.js @@ -4,6 +4,7 @@ import { GAME_RESET, ONCONTEXT_LOST, ONCONTEXT_RESTORED, + off, on, } from "../../system/event.ts"; import { Gradient } from "./../gradient.js"; @@ -74,10 +75,26 @@ export default class CanvasRenderer extends Renderer { false, ); - // reset the renderer on game reset - on(GAME_RESET, () => { + // Held as a bound field, not an inline arrow, so `destroy()` can + // unregister it — an anonymous handler cannot be passed to `off()`, + // and the closure would otherwise pin this renderer forever. + this.onGameReset = () => { this.reset(); - }); + }; + + // reset the renderer on game reset + on(GAME_RESET, this.onGameReset); + } + + /** + * Release the resources held by this renderer. The Canvas backend owns + * no GPU objects, but it does hold an event subscription whose closure + * keeps the renderer (and its canvas) reachable — so a torn-down + * application would otherwise leak one listener per teardown and keep + * reacting to `GAME_RESET` after it was destroyed. + */ + destroy() { + off(GAME_RESET, this.onGameReset); } /** diff --git a/packages/melonjs/src/video/webgl/webgl_renderer.js b/packages/melonjs/src/video/webgl/webgl_renderer.js index 228150271..3f05431e5 100644 --- a/packages/melonjs/src/video/webgl/webgl_renderer.js +++ b/packages/melonjs/src/video/webgl/webgl_renderer.js @@ -8,6 +8,7 @@ import { GAME_RESET, ONCONTEXT_LOST, ONCONTEXT_RESTORED, + off, on, RENDER_TARGET_CHANGED, } from "../../system/event.ts"; @@ -342,10 +343,28 @@ export default class WebGLRenderer extends Renderer { false, ); - // reset the renderer on game reset - on(GAME_RESET, () => { + // Held as bound fields rather than inline arrows so `destroy()` can + // actually unregister them. An anonymous handler cannot be passed to + // `off()`, so every torn-down renderer used to leave its listeners on + // the bus forever — and each closure pins the renderer, its batchers + // and its GL objects against garbage collection, so releasing the GL + // context alone was not enough to let the canvas go. + this.onGameReset = () => { this.reset(); - }); + }; + this.onContextRestoredInvalidate = (renderer) => { + if (renderer === this) { + this.currentProgram = undefined; + } + }; + this.onCanvasResize = (width, height) => { + this.flush(); + this.setViewport(0, 0, width, height); + // FBOs are lazily resized in beginPostEffect via get() → resize() + }; + + // reset the renderer on game reset + on(GAME_RESET, this.onGameReset); // Every live GLShader recompiles on this event, and each recompile // binds its own program to replay its uniform snapshot — so the @@ -358,18 +377,10 @@ export default class WebGLRenderer extends Renderer { // Light2dBlock — a foreign mesh-lit program parks its block at // default binding point 0, whose 2D buffer is now too small, and a // plain sprite flush dies with INVALID_OPERATION. - on(ONCONTEXT_RESTORED, (renderer) => { - if (renderer === this) { - this.currentProgram = undefined; - } - }); + on(ONCONTEXT_RESTORED, this.onContextRestoredInvalidate); // register to the CANVAS resize channel - on(CANVAS_ONRESIZE, (width, height) => { - this.flush(); - this.setViewport(0, 0, width, height); - // FBOs are lazily resized in beginPostEffect via get() → resize() - }); + on(CANVAS_ONRESIZE, this.onCanvasResize); } /** @@ -474,6 +485,15 @@ export default class WebGLRenderer extends Renderer { } destroy() { + // Unregister first: every handler closes over `this`, so leaving them + // on the bus keeps the renderer (and transitively its batchers and GL + // objects) reachable forever — a destroyed renderer would also still + // react to GAME_RESET and CANVAS_ONRESIZE. Matches what the WebGPU + // backend already does. + off(GAME_RESET, this.onGameReset); + off(ONCONTEXT_RESTORED, this.onContextRestoredInvalidate); + off(CANVAS_ONRESIZE, this.onCanvasResize); + // the shared ground-shadow quads (#1515) hold retained GPU geometry // keyed off this renderer's batchers — released before those go releaseShadowQuads(this); @@ -489,6 +509,24 @@ export default class WebGLRenderer extends Renderer { this.gl.deleteBuffer(this.vertexBuffer); this.vertexBuffer = null; } + + // Release the GL context itself. Dropping every GL object above and + // removing the canvas from the DOM does NOT do this: a canvas keeps + // its context until the canvas is garbage-collected, which is + // non-deterministic and routinely delayed. Browsers cap how many live + // contexts they keep (Chromium ~16) and force-lose the oldest past + // that, so a long-lived page that creates and tears down several + // applications — an SPA moving between scenes, or a test session + // sharing one page across spec files — accumulates dead-but-unfreed + // contexts until an unrelated later `getContext` stalls or fails. + // + // `WEBGL_lose_context` is the only way to hand a context back + // deterministically. `destroy()` is terminal (`Application.destroy` + // refuses a subsequent `init`), so losing it here forecloses nothing. + // The extension is absent on some drivers, hence the optional call. + if (this.isContextValid !== false) { + this.gl.getExtension("WEBGL_lose_context")?.loseContext(); + } } reset() { diff --git a/packages/melonjs/tests/application_lifecycle.spec.js b/packages/melonjs/tests/application_lifecycle.spec.js new file mode 100644 index 000000000..8810d70e0 --- /dev/null +++ b/packages/melonjs/tests/application_lifecycle.spec.js @@ -0,0 +1,143 @@ +import { beforeAll, describe, expect, it } from "vitest"; +import { + Application, + boot, + CanvasRenderer, + video, + WebGLRenderer, +} from "../src/index.js"; + +/** + * Application create / destroy lifecycle. + * + * The failure this guards against is quiet: a renderer subscribes to global + * events in its constructor, and if `destroy()` cannot unregister them the + * handlers stay on the bus for the life of the page. Each closure captures + * the renderer, so it pins the renderer, its batchers and its GPU objects + * against garbage collection — and a destroyed renderer keeps reacting to + * `GAME_RESET` / `CANVAS_ONRESIZE` long after its application is gone. + * + * Nothing about that is visible in a normal run; it surfaces as an unrelated + * suite timing out much later, once enough teardowns have piled up. That is + * exactly how it reached CI. + * + * ## What these can and cannot assert + * + * Two stronger tests were attempted and abandoned, recorded here so the dead + * ends are not re-walked: + * + * 1. "destroy, then `emit(GAME_RESET)`, assert the dead renderer did not + * react". `emit` reaches EVERY listener in the shared browser session, + * including ones left behind by other spec files whose state is long gone, + * and throws partway through. Wrapping it in try/catch would let the test + * pass without ever reaching the handler under test — a silent pass. + * 2. "spy on `event.off` and assert it was called with the registered + * handler". Vitest browser mode cannot spy on ESM exports (module + * namespaces are not configurable). + * + * So these assert the STRUCTURAL property that made the bug possible: the + * handlers must be retrievable per-instance references, because an inline + * anonymous arrow can never be passed to `off()` at all. That is necessary + * but not sufficient — it would not catch a `destroy()` that simply forgot + * to call `off`. A listener-count assertion would be strictly better and + * needs a test-visible way to inspect the bus. + */ +describe("Application lifecycle: renderer event handlers are unregisterable", () => { + let hasWebGL; + + beforeAll(async () => { + await boot(); + const probe = new Application(32, 32, { + parent: "screen", + renderer: video.WEBGL, + failIfMajorPerformanceCaveat: false, + consoleHeader: false, + }); + await probe.init(); + hasWebGL = probe.renderer instanceof WebGLRenderer; + probe.destroy(); + }); + + const mk = async (renderer) => { + const app = new Application(64, 64, { + parent: "screen", + renderer, + failIfMajorPerformanceCaveat: false, + consoleHeader: false, + }); + await app.init(); + return app; + }; + + it("CanvasRenderer defines destroy() and a retrievable GAME_RESET handler", async () => { + const app = await mk(video.CANVAS); + expect(app.renderer).toBeInstanceOf(CanvasRenderer); + // CanvasRenderer had no destroy() at all — it inherited the base + // no-op, so its GAME_RESET subscription outlived every application + expect( + Object.hasOwn(CanvasRenderer.prototype, "destroy"), + "CanvasRenderer must define its own destroy()", + ).toBe(true); + expect(typeof app.renderer.onGameReset).toBe("function"); + app.destroy(); + }); + + it("WebGLRenderer exposes all three subscriptions as retrievable handlers", async (ctx) => { + if (!hasWebGL) { + ctx.skip("WebGL renderer not available in this environment"); + } + const app = await mk(video.WEBGL); + const r = app.renderer; + // inline arrows here would be unremovable; these must be fields + expect(typeof r.onGameReset).toBe("function"); + expect(typeof r.onContextRestoredInvalidate).toBe("function"); + expect(typeof r.onCanvasResize).toBe("function"); + app.destroy(); + }); + + it("the root Container's CANVAS_ONRESIZE handler is retrievable and cleared", async () => { + const app = await mk(video.CANVAS); + const world = app.world; + // only ROOT containers subscribe (`if (this.root === true)`), so this + // is one per world rather than one per node in the scene graph + expect(world.root).toBe(true); + expect(typeof world.onCanvasResize).toBe("function"); + + app.destroy(); + // cleared on teardown, which is also what makes a second destroy a + // no-op rather than a double `off` + expect(world.onCanvasResize).toBeUndefined(); + }); + + it("World unregisters both of its subscriptions on destroy", async () => { + const app = await mk(video.CANVAS); + const world = app.world; + // GAME_RESET is registered with (handler, context) and LEVEL_LOADED + // was an inline arrow — the latter was unremovable + expect(typeof world.onLevelLoaded).toBe("function"); + expect(typeof world.reset).toBe("function"); + + app.destroy(); + expect(world.onLevelLoaded).toBeUndefined(); + }); + + it("handlers are per-instance, so each teardown removes its own", async (ctx) => { + if (!hasWebGL) { + ctx.skip("WebGL renderer not available in this environment"); + } + // Guards the drift case: were the handler shared on the prototype, + // cycle N would unregister cycle 0's closure and leave every later + // renderer on the bus. + const seen = new Set(); + for (let i = 0; i < 6; i++) { + const app = await mk(video.WEBGL); + const handler = app.renderer.onGameReset; + expect(seen.has(handler), `cycle ${i} reused a previous handler`).toBe( + false, + ); + seen.add(handler); + app.destroy(); + } + expect(seen.size).toBe(6); + }); +}); diff --git a/packages/melonjs/tests/bezier.spec.js b/packages/melonjs/tests/bezier.spec.js index 2b93a0cf4..c1430466c 100644 --- a/packages/melonjs/tests/bezier.spec.js +++ b/packages/melonjs/tests/bezier.spec.js @@ -1,6 +1,6 @@ -import { beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; import Path2D from "../src/geometries/path2d.ts"; -import { Application } from "../src/index.js"; +import { Application, video } from "../src/index.js"; describe("Bezier Curves", () => { let app; @@ -9,10 +9,21 @@ describe("Bezier Curves", () => { app = new Application(128, 128, { parent: "screen", scale: "auto", + // Canvas: this suite exercises path geometry, not GL. Defaulting to + // AUTO took a WebGL context and never released it, and the browser + // caps how many it keeps — see webgl_vao_teardown.spec.js. + renderer: video.CANVAS, }); await app.init(); }); + afterAll(() => { + // tear the application down rather than leaving its canvas, + // listeners and timers live in the shared browser session for + // the rest of the run + app?.destroy(); + }); + describe("quadraticCurveTo", () => { it("should not throw when drawing a quadratic curve", () => { expect(() => { diff --git a/packages/melonjs/tests/camera3d_integration.spec.js b/packages/melonjs/tests/camera3d_integration.spec.js index a633c07a3..cebd050d5 100644 --- a/packages/melonjs/tests/camera3d_integration.spec.js +++ b/packages/melonjs/tests/camera3d_integration.spec.js @@ -37,7 +37,10 @@ describe("Camera3d × Stage × Application integration", () => { const app = new Application(800, 600, { parent: "screen", scale: "auto", - renderer: video.AUTO, + // Canvas: this app exists only to reset global state for later + // spec files. Under AUTO it took a WebGL context and was never + // destroyed — see webgl_vao_teardown.spec.js. + renderer: video.CANVAS, }); await app.init(); }); diff --git a/packages/melonjs/tests/canvas-cliprect-transform.spec.js b/packages/melonjs/tests/canvas-cliprect-transform.spec.js index 97f550872..bd3beb44c 100644 --- a/packages/melonjs/tests/canvas-cliprect-transform.spec.js +++ b/packages/melonjs/tests/canvas-cliprect-transform.spec.js @@ -37,7 +37,10 @@ describe("CanvasRenderer clipRect vs transforms", () => { try { const app = new Application(64, 64, { parent: "screen", - renderer: video.AUTO, + // Canvas: this app exists only to reset global state for later + // spec files. Under AUTO it took a WebGL context and was never + // destroyed — see webgl_vao_teardown.spec.js. + renderer: video.CANVAS, }); await app.init(); } catch { diff --git a/packages/melonjs/tests/depth.spec.js b/packages/melonjs/tests/depth.spec.js index 4292164af..77033caa5 100644 --- a/packages/melonjs/tests/depth.spec.js +++ b/packages/melonjs/tests/depth.spec.js @@ -99,7 +99,10 @@ describe("Renderer.setDepth", () => { const app = new Application(800, 600, { parent: "screen", scale: "auto", - renderer: video.AUTO, + // Canvas: reset-only app — it exists to restore global defaults + // for later spec files, not to render. Under AUTO it took a + // WebGL context and was never destroyed. + renderer: video.CANVAS, }); await app.init(); }); @@ -152,7 +155,10 @@ describe("Renderable.preDraw forwards depth", () => { const app = new Application(800, 600, { parent: "screen", scale: "auto", - renderer: video.AUTO, + // Canvas: reset-only app — it exists to restore global defaults + // for later spec files, not to render. Under AUTO it took a + // WebGL context and was never destroyed. + renderer: video.CANVAS, }); await app.init(); }); @@ -224,7 +230,10 @@ describe("WebGL batchers carry depth as vec3 aVertex (PR A)", () => { const app = new Application(800, 600, { parent: "screen", scale: "auto", - renderer: video.AUTO, + // Canvas: reset-only app — it exists to restore global defaults + // for later spec files, not to render. Under AUTO it took a + // WebGL context and was never destroyed. + renderer: video.CANVAS, }); await app.init(); }); diff --git a/packages/melonjs/tests/glcore-audit.spec.js b/packages/melonjs/tests/glcore-audit.spec.js index 1084143c3..bec9bda43 100644 --- a/packages/melonjs/tests/glcore-audit.spec.js +++ b/packages/melonjs/tests/glcore-audit.spec.js @@ -55,7 +55,10 @@ describe("video/GL core audit reproductions", () => { try { const app = new Application(64, 64, { parent: "screen", - renderer: video.AUTO, + // Canvas: reset-only app — it exists to restore global defaults + // for later spec files, not to render. Under AUTO it took a + // WebGL context and was never destroyed. + renderer: video.CANVAS, }); await app.init(); } catch { diff --git a/packages/melonjs/tests/gltf_model.spec.js b/packages/melonjs/tests/gltf_model.spec.js index a03ea8775..c16024ac6 100644 --- a/packages/melonjs/tests/gltf_model.spec.js +++ b/packages/melonjs/tests/gltf_model.spec.js @@ -121,7 +121,10 @@ describe("GLTFModel", () => { const app = new Application(800, 600, { parent: "screen", scale: "auto", - renderer: video.AUTO, + // Canvas: this app exists only to reset global state for later + // spec files. Under AUTO it took a WebGL context and was never + // destroyed — see webgl_vao_teardown.spec.js. + renderer: video.CANVAS, }); await app.init(); }); diff --git a/packages/melonjs/tests/lighting3d.spec.js b/packages/melonjs/tests/lighting3d.spec.js index db5169eb9..bc3e58fd9 100644 --- a/packages/melonjs/tests/lighting3d.spec.js +++ b/packages/melonjs/tests/lighting3d.spec.js @@ -88,7 +88,10 @@ describe("Light3d ↔ Stage registration", () => { const app = new Application(800, 600, { parent: "screen", scale: "auto", - renderer: video.AUTO, + // Canvas: this app exists only to reset global state for later + // spec files. Under AUTO it took a WebGL context and was never + // destroyed — see webgl_vao_teardown.spec.js. + renderer: video.CANVAS, }); await app.init(); }); diff --git a/packages/melonjs/tests/linedash.spec.js b/packages/melonjs/tests/linedash.spec.js index fb291fa87..0bf3337ea 100644 --- a/packages/melonjs/tests/linedash.spec.js +++ b/packages/melonjs/tests/linedash.spec.js @@ -1,5 +1,5 @@ -import { beforeAll, describe, expect, it } from "vitest"; -import { Application } from "../src/index.js"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { Application, video } from "../src/index.js"; describe("LineDash", () => { let app; @@ -8,10 +8,19 @@ describe("LineDash", () => { app = new Application(64, 64, { parent: "screen", scale: "auto", + // Canvas: dash state is a 2D-context concern; no GL needed here. + renderer: video.CANVAS, }); await app.init(); }); + afterAll(() => { + // tear the application down rather than leaving its canvas, + // listeners and timers live in the shared browser session for + // the rest of the run + app?.destroy(); + }); + describe("setLineDash / getLineDash", () => { it("should default to an empty array (solid line)", () => { expect(app.renderer.getLineDash()).toEqual([]); diff --git a/packages/melonjs/tests/mesh.spec.js b/packages/melonjs/tests/mesh.spec.js index ef03a2ffd..59ebfea2b 100644 --- a/packages/melonjs/tests/mesh.spec.js +++ b/packages/melonjs/tests/mesh.spec.js @@ -1015,7 +1015,10 @@ describe("Mesh × Camera3d world-space path", () => { const app = new Application(800, 600, { parent: "screen", scale: "auto", - renderer: video.AUTO, + // Canvas: this app exists only to reset global state for later + // spec files. Under AUTO it took a WebGL context and was never + // destroyed — see webgl_vao_teardown.spec.js. + renderer: video.CANVAS, }); await app.init(); }); diff --git a/packages/melonjs/tests/octree_adversarial.spec.js b/packages/melonjs/tests/octree_adversarial.spec.js index 31663dc11..2401da116 100644 --- a/packages/melonjs/tests/octree_adversarial.spec.js +++ b/packages/melonjs/tests/octree_adversarial.spec.js @@ -11,8 +11,7 @@ * one that genuinely overlaps. A false negative is a silently missed * collision, which is the failure mode that does not announce itself. */ -import { beforeAll, beforeEach, describe, expect, it } from "vitest"; -import { Application, boot, video, World } from "../src/index.js"; +import { beforeEach, describe, expect, it } from "vitest"; import { AABB3d } from "../src/physics/broadphase/aabb3d.ts"; import Octree from "../src/physics/broadphase/octree.ts"; @@ -58,23 +57,27 @@ function overlaps2d(a, b) { ); } +/** + * The Octree only ever touches `world` to reach + * `world.app.viewport.localToWorld` on the `isFloating` branch, and nothing + * here is floating. So this stays a pure unit test: no `boot()`, no + * `Application`, no canvas. + * + * That is deliberate rather than incidental. Every spec that stands up an + * Application leaves a live canvas/context in the shared browser session for + * the rest of the run, and `helpers/webgl-context.js` documents the + * consequence — some unrelated spec's `beforeAll` times out later, blaming + * whichever file happened to run late. A test that does not need one should + * not create one. + */ +const stubWorld = {}; + describe("Octree — adversarial", () => { - let world; /** the ±10000 origin-centred root that `createBroadphase()` actually builds */ let octree; - beforeAll(async () => { - boot(); - const app = new Application(800, 600, { - parent: "screen", - scale: "auto", - renderer: video.CANVAS, - }); - await app.init(); - }); - beforeEach(() => { - world = new World(0, 0, 800, 600); + const world = stubWorld; const bounds = new AABB3d(); bounds.setMinMax(-10000, -10000, -10000, 10000, 10000, 10000); octree = new Octree(world, bounds, 4, 4, 0); diff --git a/packages/melonjs/tests/timer.spec.js b/packages/melonjs/tests/timer.spec.js index 1c284f93f..82eb3774c 100644 --- a/packages/melonjs/tests/timer.spec.js +++ b/packages/melonjs/tests/timer.spec.js @@ -1,12 +1,31 @@ -import { beforeAll, describe, expect, onTestFinished, test, vi } from "vitest"; -import { Application, timer } from "../src/index.js"; +import { + afterAll, + beforeAll, + describe, + expect, + onTestFinished, + test, + vi, +} from "vitest"; +import { Application, timer, video } from "../src/index.js"; describe("Timer", () => { + let app; + beforeAll(async () => { - const app = new Application(100, 100); + // Canvas: this suite only drives the timer; an unspecified renderer + // resolved to WebGL and held a context for the whole session. + app = new Application(100, 100, { renderer: video.CANVAS }); await app.init(); }); + afterAll(() => { + // tear the application down rather than leaving its canvas, + // listeners and timers live in the shared browser session for + // the rest of the run + app?.destroy(); + }); + describe("setTimeout", () => { test("calls the provided function when enough time have elapsed", async () => { const fn = vi.fn(); diff --git a/packages/melonjs/tests/webgl_save_restore.spec.js b/packages/melonjs/tests/webgl_save_restore.spec.js index 1b4efe621..c140832d1 100644 --- a/packages/melonjs/tests/webgl_save_restore.spec.js +++ b/packages/melonjs/tests/webgl_save_restore.spec.js @@ -37,7 +37,10 @@ describe("WebGL Renderer save/restore", () => { const app = new Application(800, 600, { parent: "screen", scale: "auto", - renderer: video.AUTO, + // Canvas: reset-only app — it exists to restore global defaults + // for later spec files, not to render. Under AUTO it took a + // WebGL context and was never destroyed. + renderer: video.CANVAS, }); await app.init(); }); diff --git a/packages/melonjs/tests/webgl_vao_teardown.spec.js b/packages/melonjs/tests/webgl_vao_teardown.spec.js index bec95bb77..7dbaf55fa 100644 --- a/packages/melonjs/tests/webgl_vao_teardown.spec.js +++ b/packages/melonjs/tests/webgl_vao_teardown.spec.js @@ -72,7 +72,7 @@ describe("WebGL batcher teardown releases GL objects", () => { expect(renderer.vertexBuffer).toBe(null); }); - it("destroy() is idempotent and does not queue GL errors", async (ctx) => { + it("destroy() releases the GL context, and is idempotent", async (ctx) => { requireWebGL(ctx); const app = new Application(48, 48, { parent: "screen", @@ -85,8 +85,33 @@ describe("WebGL batcher teardown releases GL objects", () => { while (gl.getError() !== gl.NO_ERROR) { /* drain */ } + app.destroy(); - app.renderer.destroy(); - expect(gl.getError()).toBe(gl.NO_ERROR); + + // `destroy()` now hands the context back via `WEBGL_lose_context`. + // Dropping the GL objects and removing the canvas from the DOM does + // NOT do this on its own — the context survives until the canvas is + // garbage-collected, and browsers force-lose the oldest once past + // their live-context cap (~16 on Chromium). A page that builds and + // tears down several applications therefore used to accumulate + // dead-but-unfreed contexts until an unrelated later `getContext` + // stalled. This assertion is what keeps that from regressing. + expect(gl.isContextLost()).toBe(true); + + // still idempotent: a second teardown on the now-lost context must + // not throw. GL calls on a lost context are no-ops by spec. + expect(() => { + app.renderer.destroy(); + }).not.toThrow(); }); }); + +// NOTE — a "create/destroy N applications past the browser's context cap" +// test was written here and REMOVED, because it passed identically with and +// without the fix. On a machine with a real GPU the cap is never reached at +// any N a unit test can afford, and eviction does not surface as a +// newly-created context reporting `isContextLost()` — it surfaces as the +// OLDEST context dying, and as `getContext` getting slower, neither of which +// is assertable cheaply or deterministically. The `isContextLost()` check +// above is the honest regression guard: it fails without the fix and passes +// with it. Left as a comment so nobody re-derives the dead end.