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);