From a5a2c42c20c47ed92395c0325a9f7e0a3d2cb652 Mon Sep 17 00:00:00 2001 From: Olivier Biot Date: Mon, 7 Sep 2026 16:44:06 +0800 Subject: [PATCH] Renderer: two caches that never hit, one per backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both were doing the same work on every frame, forever, for an answer that could not change. WebGL — `UniformBlock#bindTo` asked the driver for the light block's index on every call. That is a blocking round-trip, and `bindTo` is reached per draw: `_bindLightBlock` guards with a single-slot `program === this._lightBlockProgram` check, which thrashes whenever the program alternates (lit and unlit, fog and no fog). The index is a fixed property of a linked program, so it is now memoized in a WeakMap keyed on the program. On the instanced forest example that is 1358 queries for 4 distinct answers, now 4, and about 50% less engine CPU per frame. Keying on the program object is what makes it safe across a lost context: every program is recompiled on restore, so the new object misses the cache while the old one becomes collectable — and `WebGLRenderer.reset()` rebuilds the UniformBlock itself anyway. The engine never re-links a program in place: `compileProgram` is the single link site and it creates the program it links. `extractUniforms` already relies on the same fact for its uniform locations. WebGPU — a mesh texture whose largest dimension is 1 was re-created every frame. A full mip chain for a 1x1 image is one level, but the test for whether a resident texture already had a chain was `mipLevelCount === 1`, which for that size can never come true: rebuilt with one level, asked again next draw, never settling. Every mesh without an alpha map binds a 1x1 filler through this path. Comparing against the chain the size can actually hold takes 2066 texture creations down to 3, and the bind groups rebuilt alongside them from 3114 to 21 — about 17% less engine CPU per frame. Neither fix changes what is drawn. `uniformBlockBinding` still runs on every `bindTo`; only the query is skipped. The 1x1 texture is reused rather than recreated with identical contents. The four lighting-sensitive static examples render byte-identical before and after, on both backends. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N --- packages/melonjs/CHANGELOG.md | 4 + .../src/video/webgl/buffer/uniformblock.js | 41 ++++- .../melonjs/src/video/webgpu/texture/store.js | 29 +++- .../melonjs/tests/webgl_uniformblock.spec.js | 163 ++++++++++++++++++ .../melonjs/tests/webgpu_compressed.spec.js | 11 ++ packages/melonjs/tests/webgpu_mipmaps.spec.js | 91 +++++++++- 6 files changed, 328 insertions(+), 11 deletions(-) diff --git a/packages/melonjs/CHANGELOG.md b/packages/melonjs/CHANGELOG.md index 28d38d813..1068f9616 100644 --- a/packages/melonjs/CHANGELOG.md +++ b/packages/melonjs/CHANGELOG.md @@ -11,6 +11,10 @@ - Mesh: normals are generated from the geometry when a `lit` mesh is built without them, so a hand-built mesh no longer has to write the same accumulate-and-normalize loop first. Flat versus smooth is decided by the geometry rather than a flag: face normals accumulate into their vertices weighted by area, so shared vertices average into smooth shading while a triangle soup resolves to the face normal and shades flat. An explicit `settings.normals` still wins, and an unlit mesh gets none - Docs: the API reference carries the engine's own identity — logo, brand palette and favicon — and the header links out to the site, the wiki, the repository and Discord. A **Copy page** control hands the page you are reading to an assistant: it copies the page as Markdown with its canonical URL attached, or opens it directly in a chat. The landing page also gained a short section on using the reference with an AI assistant +### Performance +- Lit meshes and sprites (WebGL): the light block's `getUniformBlockIndex` is memoized per program, rather than re-queried — a blocking driver call — on every program switch. **About 50% less engine CPU per frame** on the instanced forest example +- WebGPU: the 1×1 filler texture every mesh without an alpha map binds was re-created every frame, its one-level mip chain never counting as complete. **About 17% less engine CPU per frame**, and 2066 texture creations down to 3 + ### Fixed - Level: `level.reload()` was documented as returning `object` — "the current level" — but it returns whatever `level.load()` returns, which is `true`. The declared type has been wrong for the method's whole life: the 2011 original returned nothing at all. `getCurrentLevel()` is the call that hands back the level object. This corrects the emitted type from `object` to `boolean`, so a `const lvl: object = level.reload()` that compiled while receiving `true` now fails to compile, at the site that was already wrong - Lit meshes: specular highlights sat in the wrong place under a scaled ancestor ([#1636](https://github.com/melonjs/melonJS/issues/1636)). The camera position was derived from the view as `-Rᵀ·t`, which is only the right point when the upper 3×3 is orthonormal — and `Container.draw` folds every ancestor into that matrix. It is now the translation column of the view's inverse diff --git a/packages/melonjs/src/video/webgl/buffer/uniformblock.js b/packages/melonjs/src/video/webgl/buffer/uniformblock.js index d1b71c448..ae2b233f0 100644 --- a/packages/melonjs/src/video/webgl/buffer/uniformblock.js +++ b/packages/melonjs/src/video/webgl/buffer/uniformblock.js @@ -65,6 +65,25 @@ export default class UniformBlock { /** @type {number} */ this.bindingPoint = bindingPoint; + /** + * `getUniformBlockIndex` answers for programs already asked about. + * + * The index is a static property of a linked program, but the query is + * a driver round-trip, and `bindTo` is reached per draw — whenever the + * current program changes. A scene alternating between two shader + * variants therefore re-asks the driver the same question several times + * a frame, forever. + * + * A WeakMap keyed on the program is exactly right for the context-loss + * story above: a restored context brings NEW program objects, absent + * from the map and so re-queried, while the old ones become unreachable + * and collectable. There is still nothing to ask the driver. + * @type {WeakMap>} + * @ignore + * @internal + */ + this._blockIndices = new WeakMap(); + /** @type {WebGLBuffer|null} */ this.buffer = gl.createBuffer(); @@ -77,16 +96,30 @@ export default class UniformBlock { /** * Point a program's named block at this buffer's binding point. * - * Safe to call repeatedly — after a context restore the program is a new - * object and must be re-bound, and there is no way to ask whether it - * already was. + * Safe to call repeatedly, and cheap to: the block index is memoized per + * program, so a repeat call costs a map lookup rather than a driver + * round-trip. After a context restore the program is a new object and must + * be re-bound — there is no way to ask whether it already was — and that + * new object misses the cache, so it is re-queried. * @param {WebGLProgram} program - the linked program * @param {string} blockName - the block's name in GLSL * @returns {boolean} `false` when the program does not declare the block */ bindTo(program, blockName) { const gl = this.gl; - const index = gl.getUniformBlockIndex(program, blockName); + let byName = this._blockIndices.get(program); + if (byName === undefined) { + byName = new Map(); + this._blockIndices.set(program, byName); + } + let index = byName.get(blockName); + if (index === undefined) { + index = gl.getUniformBlockIndex(program, blockName); + // INVALID_INDEX is cached too: a program that does not declare the + // block will never grow one, and re-asking every draw is the same + // round-trip for the same answer + byName.set(blockName, index); + } if (index === gl.INVALID_INDEX) { // the shader compiled without the block — either it does not use // lighting, or the declaration was optimised out because nothing diff --git a/packages/melonjs/src/video/webgpu/texture/store.js b/packages/melonjs/src/video/webgpu/texture/store.js index 3a9488d83..f997a09f0 100644 --- a/packages/melonjs/src/video/webgpu/texture/store.js +++ b/packages/melonjs/src/video/webgpu/texture/store.js @@ -3,6 +3,25 @@ import { TextureStore } from "../../gpu/texturestore.js"; import mipblitWGSL from "../shaders/mipblit.wgsl"; import { COMPRESSED_FORMATS, uploadCompressedTexture } from "./compressed.js"; +/** + * How many mip levels a full chain for this size has. + * + * The reason this is a function rather than an inline expression: a source + * whose largest dimension is 1 has a COMPLETE chain at one level. Testing + * "does this record have mips" as `mipLevelCount === 1` therefore never comes + * true for it — the texture is rebuilt with one level, the next draw asks the + * same question, and a 1x1 filler (every mesh without an alpha map binds one) + * is re-created and retired several times a frame for the life of the scene. + * @param {number} width - source width in pixels + * @param {number} height - source height in pixels + * @returns {number} levels in a full chain, at least 1 + * @ignore + * @internal + */ +function fullMipLevelCount(width, height) { + return Math.floor(Math.log2(Math.max(width, height, 1))) + 1; +} + /** * Renderer-owned GPU texture store for the WebGPU backend — the counterpart * of `MaterialBatcher`'s createTexture2D/bindTexture2D/deleteTexture2D tier, @@ -123,7 +142,8 @@ export default class WebGPUTextureStore extends TextureStore { // same source, but the texture must be rebuilt with a chain (options.mipmaps === true && record.compressed !== true && - (record.mipLevelCount ?? 1) === 1) + (record.mipLevelCount ?? 1) < + fullMipLevelCount(record.width ?? 1, record.height ?? 1)) ) { // compressed sources (parsed dds/ktx/pvr/pkm) carry pre-encoded // block data: a dedicated createTexture + per-mip writeTexture @@ -206,7 +226,8 @@ export default class WebGPUTextureStore extends TextureStore { // level-0-only texture to a full chain — never the reverse: // flat consumers of a mipped record sample level 0 via their // lod-clamped sampler - (options.mipmaps === true && (record.mipLevelCount ?? 1) === 1) || + (options.mipmaps === true && + (record.mipLevelCount ?? 1) < fullMipLevelCount(width, height)) || // a recycled unit whose resident texture came from the // compressed path cannot adopt an image source: its format // is non-renderable and copyExternalImageToTexture would @@ -221,9 +242,7 @@ export default class WebGPUTextureStore extends TextureStore { } // full chain down to 1×1 when the mesh path asks for mips const mipLevelCount = - options.mipmaps === true - ? Math.floor(Math.log2(Math.max(width, height))) + 1 - : 1; + options.mipmaps === true ? fullMipLevelCount(width, height) : 1; const gpuTexture = this.device.createTexture({ label: "melonJS texture", size: [width, height], diff --git a/packages/melonjs/tests/webgl_uniformblock.spec.js b/packages/melonjs/tests/webgl_uniformblock.spec.js index b0838a408..0a837a188 100644 --- a/packages/melonjs/tests/webgl_uniformblock.spec.js +++ b/packages/melonjs/tests/webgl_uniformblock.spec.js @@ -1,6 +1,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { event } from "../src/index.js"; import UniformBlock from "../src/video/webgl/buffer/uniformblock.js"; +import { compileProgram } from "../src/video/webgl/utils/program.js"; import { getWebGLRenderer, releaseWebGLRenderer, @@ -144,6 +145,168 @@ void main(void) { fragColor = first + second; }`; } }); + /** count driver queries while `fn` runs */ + const countQueries = (fn) => { + const original = gl.getUniformBlockIndex.bind(gl); + let calls = 0; + gl.getUniformBlockIndex = (...args) => { + calls += 1; + return original(...args); + }; + try { + fn(); + } finally { + gl.getUniformBlockIndex = original; + } + return calls; + }; + + it("asks the driver once per program, however often it is called", (ctx) => { + requireWebGL(ctx, renderer); + // `bindTo` is reached per draw — whenever the current program + // changes — so a scene alternating between two shader variants + // re-asks several times a frame. The index is a static property of + // a linked program, but the query is a driver round-trip, and on + // a real scene those round-trips measured 1358 calls for 4 answers. + const block = new UniformBlock(gl, 8, 2); + const program = buildProgram(); + try { + const queries = countQueries(() => { + for (let i = 0; i < 50; i++) { + expect(block.bindTo(program, "TestBlock")).toBe(true); + } + }); + expect(queries).toBe(1); + // and the binding is still actually established + const index = gl.getUniformBlockIndex(program, "TestBlock"); + expect( + gl.getActiveUniformBlockParameter( + program, + index, + gl.UNIFORM_BLOCK_BINDING, + ), + ).toBe(2); + expect(gl.getError()).toBe(gl.NO_ERROR); + } finally { + gl.deleteProgram(program); + block.destroy(); + } + }); + + it("caches the miss as well as the hit", (ctx) => { + requireWebGL(ctx, renderer); + // a program that does not declare the block will never grow one, + // so re-asking is the same round-trip for the same answer + const block = new UniformBlock(gl, 8, 2); + const program = buildProgram(); + try { + const queries = countQueries(() => { + for (let i = 0; i < 10; i++) { + expect(block.bindTo(program, "NoSuchBlock")).toBe(false); + } + }); + expect(queries).toBe(1); + } finally { + gl.deleteProgram(program); + block.destroy(); + } + }); + + it("keeps a separate answer per block name on the same program", (ctx) => { + requireWebGL(ctx, renderer); + // The cache is keyed by (program, name) because the index belongs to + // the pair. Keyed on the program alone, the second name would be + // handed the first one's index — a bind pointing at the wrong block, + // or a `true` for a block the shader never declared. + const block = new UniformBlock(gl, 8, 2); + const program = buildProgram(); + try { + const queries = countQueries(() => { + for (let i = 0; i < 10; i++) { + expect(block.bindTo(program, "TestBlock")).toBe(true); + expect(block.bindTo(program, "NoSuchBlock")).toBe(false); + } + }); + expect(queries).toBe(2); + } finally { + gl.deleteProgram(program); + block.destroy(); + } + }); + + it("the engine never re-links a program in place, which is what makes the cache safe", (ctx) => { + requireWebGL(ctx, renderer); + // A block index belongs to a LINKED program: re-linking the same + // object changes it while the object identity stays put, which no + // cache keyed on identity can notice. That is safe here only + // because `compileProgram` is the single link site in the backend + // and it creates the program it links — so every link yields a + // fresh object, which misses the cache by construction. + // + // This is NOT a private assumption of this cache: `extractUniforms` + // resolves `getUniformLocation` once per shader, so an in-place + // re-link would already send every uniform to a stale location. If + // this test ever fails, both caches need revisiting, not just this + // one. + const first = compileProgram(gl, VERT, FRAG, {}); + const second = compileProgram(gl, VERT, FRAG, {}); + try { + expect(second).not.toBe(first); + + const block = new UniformBlock(gl, 8, 2); + try { + const queries = countQueries(() => { + block.bindTo(first, "TestBlock"); + block.bindTo(first, "TestBlock"); + block.bindTo(second, "TestBlock"); + }); + // one per program, and the second was never served the + // first's answer + expect(queries).toBe(2); + } finally { + block.destroy(); + } + } finally { + gl.deleteProgram(first); + gl.deleteProgram(second); + } + }); + + it("re-queries a program it has not seen — which is what a context restore brings", (ctx) => { + requireWebGL(ctx, renderer); + // The cache is keyed on the program OBJECT, which is what makes it + // safe across a lost context: every program is recompiled on + // restore, so the new one misses the cache and is re-queried, while + // the dead one becomes unreachable. A cache keyed on anything + // weaker would hand back an index for a program that no longer + // exists, and the shader would read an unbound block — black, with + // no GL error. + const block = new UniformBlock(gl, 8, 2); + const first = buildProgram(); + const restored = buildProgram(); + try { + const queries = countQueries(() => { + block.bindTo(first, "TestBlock"); + block.bindTo(first, "TestBlock"); + block.bindTo(restored, "TestBlock"); + block.bindTo(restored, "TestBlock"); + }); + expect(queries).toBe(2); + const index = gl.getUniformBlockIndex(restored, "TestBlock"); + expect( + gl.getActiveUniformBlockParameter( + restored, + index, + gl.UNIFORM_BLOCK_BINDING, + ), + ).toBe(2); + } finally { + gl.deleteProgram(first); + gl.deleteProgram(restored); + block.destroy(); + } + }); + it("is idempotent", (ctx) => { requireWebGL(ctx, renderer); // re-bound after every context restore, with no way to ask whether diff --git a/packages/melonjs/tests/webgpu_compressed.spec.js b/packages/melonjs/tests/webgpu_compressed.spec.js index a65e5aa4b..5e4defaf1 100644 --- a/packages/melonjs/tests/webgpu_compressed.spec.js +++ b/packages/melonjs/tests/webgpu_compressed.spec.js @@ -210,6 +210,17 @@ describe("WebGPU compressed textures", () => { expect(writes).toHaveLength(2); expect(created).toHaveLength(1); + // and it STAYS at one across frames. The authored chain is SHORTER + // than a full one (2 levels for 8×8, which would be 4), so a + // "does this already have its mips?" test that compares against a + // full chain wants to rebuild it — every frame, forever. + for (let frame = 2; frame <= 5; frame++) { + renderer.frameId = frame; + store.getBinding(atlas, { mipmaps: true }); + } + expect(created).toHaveLength(1); + expect(writes).toHaveLength(2); + // a recycled unit must NOT adopt a same-size image source into the // compressed-format texture (non-renderable format — the copy would // fail validation while the stale pixels kept serving): it recreates diff --git a/packages/melonjs/tests/webgpu_mipmaps.spec.js b/packages/melonjs/tests/webgpu_mipmaps.spec.js index ffa15896d..c6eca71d6 100644 --- a/packages/melonjs/tests/webgpu_mipmaps.spec.js +++ b/packages/melonjs/tests/webgpu_mipmaps.spec.js @@ -34,7 +34,7 @@ describe("WebGPUTextureStore mipmaps", () => { beforeEach(() => { createdTextures = []; samplers = []; - mipgen = { submits: 0, passes: [], draws: 0 }; + mipgen = { submits: 0, passes: [], draws: 0, copies: 0 }; const device = { createTexture(descriptor) { const texture = { @@ -90,7 +90,12 @@ describe("WebGPUTextureStore mipmaps", () => { }; }, queue: { - copyExternalImageToTexture() {}, + copyExternalImageToTexture() { + // counted: a texture that is not re-CREATED can still be + // re-UPLOADED every frame through the adoption path, which + // is the same pathology one layer down + mipgen.copies++; + }, submit() { mipgen.submits++; }, @@ -200,6 +205,88 @@ describe("WebGPUTextureStore mipmaps", () => { expect(mipgen.submits).toBe(0); }); + it("a 1×1 source is uploaded ONCE, not re-created every frame", () => { + // A full chain for 1×1 is one level, so a record holding one level is + // already complete. Testing "has it got mips?" as `mipLevelCount === 1` + // can never come true here: the texture was rebuilt with one level, the + // next frame asked again, and the answer never changed. Every mesh + // without an alpha map binds a 1×1 filler through this path, so this + // churned ~4 GPU textures per frame for the life of the scene. + const source = makeSource(1, 1); + for (let frame = 0; frame < 5; frame++) { + renderer.frameId = frame + 1; + store.getBinding(makeAtlas(source), { mipmaps: true }); + } + expect(createdTextures).toHaveLength(1); + expect(createdTextures[0].destroyed).toBe(false); + expect(mipgen.submits).toBe(0); + // Uploaded once, too. Without this the outer guard can be reverted on + // its own and every test still passes: the record stops being + // re-created but starts being re-adopted, one image copy per frame. + expect(mipgen.copies).toBe(1); + }); + + it("a mipped record is not re-created every frame either", () => { + // the same guard, read the other way: once a record holds the full + // chain its size allows, nothing further is owed + const source = makeSource(32, 32); + renderer.frameId = 1; + store.getBinding(makeAtlas(source), { mipmaps: true }); + expect(createdTextures).toHaveLength(1); + expect(createdTextures[0].mipLevelCount).toBe(6); + + for (let frame = 2; frame <= 5; frame++) { + renderer.frameId = frame; + store.getBinding(makeAtlas(source), { mipmaps: true }); + } + expect(createdTextures).toHaveLength(1); + // and the chain was generated once, not once per frame + expect(mipgen.submits).toBe(1); + }); + + it("upgrades a flat record to a chain across frames, then settles", () => { + // The in-place upgrade test above runs both binds in ONE frame, where + // `record.frameId === renderer.frameId` decides the rebuild before the + // mip clause is even evaluated. Crossing a frame boundary is what makes + // the mip comparison itself the deciding term. + const source = makeSource(32, 32); + renderer.frameId = 1; + store.getBinding(makeAtlas(source)); + expect(createdTextures).toHaveLength(1); + expect(createdTextures[0].mipLevelCount).toBe(1); + + renderer.frameId = 2; + store.getBinding(makeAtlas(source), { mipmaps: true }); + expect(createdTextures).toHaveLength(2); + expect(createdTextures[1].mipLevelCount).toBe(6); + + // and then stops: the chain is as long as the size allows + for (let frame = 3; frame <= 6; frame++) { + renderer.frameId = frame; + store.getBinding(makeAtlas(source), { mipmaps: true }); + } + expect(createdTextures).toHaveLength(2); + expect(mipgen.submits).toBe(1); + }); + + it("a non-power-of-two source settles after one upload", () => { + // the chain length is floor(log2(max)) + 1, so 100×100 gets 7 levels — + // the guard has to compare against THAT, not against a power-of-two + // assumption, or the record never looks complete + const source = makeSource(100, 100); + renderer.frameId = 1; + store.getBinding(makeAtlas(source), { mipmaps: true }); + expect(createdTextures).toHaveLength(1); + expect(createdTextures[0].mipLevelCount).toBe(7); + + for (let frame = 2; frame <= 5; frame++) { + renderer.frameId = frame; + store.getBinding(makeAtlas(source), { mipmaps: true }); + } + expect(createdTextures).toHaveLength(1); + expect(mipgen.submits).toBe(1); + }); + it("plain 2D uploads are unchanged (no chain, no submits — regression pin)", () => { store.getBinding(makeAtlas(makeSource(128, 128))); expect(createdTextures[0].mipLevelCount).toBe(1);