From cdcbecb2d1f6925b189ff9c70d81acb46dce8941 Mon Sep 17 00:00:00 2001 From: snowyukitty <270071858+snowyukitty@users.noreply.github.com> Date: Mon, 12 Oct 2026 02:57:10 +0900 Subject: [PATCH] fix(texture): keep coordinate UVs out of atlas regions --- packages/melonjs/CHANGELOG.md | 1 + packages/melonjs/src/video/texture/atlas.js | 107 ++++++--- .../melonjs/src/video/webgl/webgl_renderer.js | 2 +- .../src/video/webgpu/webgpu_renderer.js | 2 +- .../tests/texture-atlas-parsers.spec.js | 7 +- .../melonjs/tests/texture-atlas-uvs.spec.js | 217 ++++++++++++++++++ 6 files changed, 299 insertions(+), 37 deletions(-) create mode 100644 packages/melonjs/tests/texture-atlas-uvs.spec.js diff --git a/packages/melonjs/CHANGELOG.md b/packages/melonjs/CHANGELOG.md index 1566448a29..d2589dac99 100644 --- a/packages/melonjs/CHANGELOG.md +++ b/packages/melonjs/CHANGELOG.md @@ -19,6 +19,7 @@ - **`createPool` is exported**, along with the `Pool` and `CreatePoolOptions` types it names, so a pool for one of your own classes no longer needs a deep import and a field holding one can be declared. `getPool()` only reaches the pools the engine ships, so this is what a game moving off `pool.register(name, Class, true)` needs ### Changed +- TextureAtlas: source-rectangle UVs are cached separately from named frames, so cropped image and pattern draws no longer add regions to the animation dictionary. `getUVs(name)` now throws for an unknown name instead of parsing it as coordinates; use `getUVs(x, y, width, height)` for a source rectangle, or define a named region with `addRegion()` before looking it up ([#1281](https://github.com/melonjs/melonJS/issues/1281)). - `audio.playTrack()` stops the track it replaces. It only ever assigned the current-track slot and played, so the previous track kept looping underneath and, with the slot already moved on, `stopTrack()`, `pauseTrack()` and `resumeTrack()` could no longer reach it: two beds playing at once, and the older one stuck for the life of the page. There is one current track, and now only one is ever playing. Calling it with the track already playing restarts that track rather than stacking a second copy of it, which is what `play()` does for every other clip; a scene that wants the bed to carry on across a screen change should not call it again. An unknown clip name still throws, and now throws before anything is stopped, so a typo cannot leave the game silent - `pool.push(obj)` reports a refusal by returning `false` rather than throwing. It threw by default for any class that was never registered, or registered without recycling, and nothing at the call site made that visible: the exception aborted whatever was running, which in both of the engine's own uses removed in 20.7.0 was a `destroy()` that had already recycled other state, leaving a half torn down object to die later somewhere unrelated. Pass `pool.push(obj, true)` where a missed registration is a bug you want to hear about immediately ([#1688](https://github.com/melonjs/melonJS/issues/1688)) - Tiled tile images, atlas sprites and Tiled image layers are built from their classes directly rather than looked up by name. `TextureAtlas#createSpriteFromName` and the Tiled image layer loader used to resolve their class through `pool.pull("me.Sprite")` and `pool.pull("ImageLayer")`, which never pooled anything: neither class is registered for recycling, so the call was a plain construction behind a string key. Re-registering one of those BUILT-IN names to substitute your own class, as in `pool.register("Sprite", MySprite)`, therefore no longer changes what they produce. Registering your own names is unaffected, and so is substituting a built-in for Tiled OBJECTS, which goes through the Tiled object factory ([#1688](https://github.com/melonjs/melonJS/issues/1688)) diff --git a/packages/melonjs/src/video/texture/atlas.js b/packages/melonjs/src/video/texture/atlas.js index 43d84a4983..5d390e4816 100644 --- a/packages/melonjs/src/video/texture/atlas.js +++ b/packages/melonjs/src/video/texture/atlas.js @@ -180,11 +180,18 @@ export class TextureAtlas extends Texture2d { this.activeAtlas = undefined; /** - * UV lookup cache to avoid per-frame string allocation in drawImage + * Source-rectangle UVs, separate from the named regions in each atlas. * @ignore * @internal */ - this._uvCache = { sx: -1, sy: -1, sw: -1, sh: -1, uvs: null }; + this._uvsByAtlas = new WeakMap(); + + /** + * Last numeric lookup, scoped to its atlas page. + * @ignore + * @internal + */ + this._uvCache = { atlas: null, sx: -1, sy: -1, sw: -1, sh: -1, uvs: null }; // parse given atlas(es) paremeters if (typeof atlases !== "undefined") { @@ -445,19 +452,23 @@ export class TextureAtlas extends Texture2d { } /** - * return the uvs mapping for the given region + * Return precomputed UVs for a named region, or cached UVs for a numeric + * source rectangle. Numeric lookups do not add regions to the atlas. * @param {string|number} name - region (or frame) name, or sx when using numeric parameters * @param {number} [sy] - source y coordinate (when using numeric parameters) * @param {number} [sw] - source width (when using numeric parameters) * @param {number} [sh] - source height (when using numeric parameters) * @returns {Float32Array} region Uvs + * @throws {Error} if the named region does not exist */ getUVs(name, sy, sw, sh) { if (typeof name === "number") { const sx = name; + const atlas = this.getAtlas(); const cache = this._uvCache; // fast path: return cached UVs if coordinates match last lookup if ( + atlas === cache.atlas && sx === cache.sx && sy === cache.sy && sw === cache.sw && @@ -465,31 +476,72 @@ export class TextureAtlas extends Texture2d { ) { return cache.uvs; } - // cache miss: fall back to string-keyed region lookup - const key = sx + "," + sy + "," + sw + "," + sh; - let region = this.getRegion(key); - if (typeof region === "undefined") { - region = this.addRegion(key, sx, sy, sw, sh); + const rectangles = this._getUVCache(atlas, sx, sy, sw); + let uvs = rectangles.get(sh); + if (typeof uvs === "undefined") { + const source = this.getTexture(); + const width = source.width || source.videoWidth; + const height = source.height || source.videoHeight; + uvs = new Float32Array([ + sx / width, + sy / height, + (sx + sw) / width, + (sy + sh) / height, + ]); + rectangles.set(sh, uvs); } + cache.atlas = atlas; cache.sx = sx; cache.sy = sy; cache.sw = sw; cache.sh = sh; - cache.uvs = region.uvs; - return region.uvs; + cache.uvs = uvs; + return uvs; } // string-based lookup (named regions) - let region = this.getRegion(name); + const region = this.getRegion(name); - if (typeof region === "undefined") { - // see https://github.com/melonjs/melonJS/issues/1281 - const keys = name.split(","); - region = this.addRegion(name, +keys[0], +keys[1], +keys[2], +keys[3]); + if (typeof region?.uvs === "undefined") { + throw new Error("Texture - region for " + name + " not found"); } return region.uvs; } + /** + * Locate a rectangle's height-keyed UV map without constructing string keys. + * @param {object} atlas - atlas dictionary + * @param {number} x - source x coordinate + * @param {number} y - source y coordinate + * @param {number} width - source width + * @returns {Map} cached UVs by source height + * @ignore + * @internal + */ + _getUVCache(atlas, x, y, width) { + let byX = this._uvsByAtlas.get(atlas); + if (typeof byX === "undefined") { + byX = new Map(); + this._uvsByAtlas.set(atlas, byX); + } + let byY = byX.get(x); + if (typeof byY === "undefined") { + byY = new Map(); + byX.set(x, byY); + } + let byWidth = byY.get(y); + if (typeof byWidth === "undefined") { + byWidth = new Map(); + byY.set(y, byWidth); + } + let byHeight = byWidth.get(width); + if (typeof byHeight === "undefined") { + byHeight = new Map(); + byWidth.set(width, byHeight); + } + return byHeight; + } + /** * add uvs mapping for the given region * @param {object} atlas - the atlas dictionnary where the region is define @@ -510,17 +562,11 @@ export class TextureAtlas extends Texture2d { (s.x + sw) / w, // u1 (right) (s.y + sh) / h, // v1 (bottom) ]); - // Cache source coordinates - // see https://github.com/melonjs/melonJS/issues/1281 - // keyed by the REGION rect (x, y, width, height) — the same key shape - // `getUVs(sx, sy, sw, sh)` builds for coordinate lookups. (Keying by - // the texture dimensions instead made the alias unmatchable for any - // sub-region AND poisoned full-image lookups whenever a frame sat at - // (0,0): "0,0,texW,texH" pointed at that frame, not the whole image.) - const key = `${s.x},${s.y},${sw},${sh}`; - if (key !== name) { - atlas[key] = atlas[name]; - } + // Numeric draws reuse the frame's precomputed UVs without exposing + // coordinate aliases to animation consumers of the atlas dictionary. + this._getUVCache(atlas, s.x, s.y, sw).set(sh, atlas[name].uvs); + // A newly defined region may replace the UVs from the last lookup. + this._uvCache.atlas = null; return atlas[name].uvs; } @@ -639,9 +685,8 @@ export class TextureAtlas extends Texture2d { const isArray = Array.isArray(names); const name = isArray ? names[i] : i; // in dictionary mode, only iterate real regions: a region's `name` - // matches its dictionary key. This skips both the coordinate-alias - // keys added by `addUVs` (#1281 — they'd duplicate every frame) and - // non-region entries like the aseprite parser's `anims` dictionary + // matches its dictionary key. This skips non-region entries like + // the aseprite parser's `anims` dictionary // (whose value has no `name`, and which `getRegion` would otherwise // happily return as a NaN-sized pseudo-region). if (!isArray && (names[i] == null || names[i].name !== i)) { @@ -704,8 +749,8 @@ export class TextureAtlas extends Texture2d { for (const i in names) { const isArray = Array.isArray(names); const name = isArray ? names[i] : i; - // dictionary mode: skip coordinate-alias keys and non-region - // entries (see getAnimationSettings for the full rationale) + // dictionary mode: skip non-region entries (see + // getAnimationSettings for the full rationale) if (!isArray && (names[i] == null || names[i].name !== i)) { continue; } diff --git a/packages/melonjs/src/video/webgl/webgl_renderer.js b/packages/melonjs/src/video/webgl/webgl_renderer.js index 73311aa25d..e3883453cc 100644 --- a/packages/melonjs/src/video/webgl/webgl_renderer.js +++ b/packages/melonjs/src/video/webgl/webgl_renderer.js @@ -2430,7 +2430,7 @@ export default class WebGLRenderer extends Renderer { * @see WebGLRenderer#createPattern */ drawPattern(pattern, x, y, width, height) { - const uvs = pattern.getUVs("0,0," + width + "," + height); + const uvs = pattern.getUVs(0, 0, width, height); this.setBatcher("quad"); this.currentBatcher.addQuad( pattern, diff --git a/packages/melonjs/src/video/webgpu/webgpu_renderer.js b/packages/melonjs/src/video/webgpu/webgpu_renderer.js index 2585eb0643..700f9aa917 100644 --- a/packages/melonjs/src/video/webgpu/webgpu_renderer.js +++ b/packages/melonjs/src/video/webgpu/webgpu_renderer.js @@ -2759,7 +2759,7 @@ export default class WebGPURenderer extends Renderer { * @param {number} height - height of the pattern */ drawPattern(pattern, x, y, width, height) { - const uvs = pattern.getUVs("0,0," + width + "," + height); + const uvs = pattern.getUVs(0, 0, width, height); this.setBatcher("quad"); this.currentBatcher.addQuad( pattern, diff --git a/packages/melonjs/tests/texture-atlas-parsers.spec.js b/packages/melonjs/tests/texture-atlas-parsers.spec.js index 0a729aded2..ed32d71ff1 100644 --- a/packages/melonjs/tests/texture-atlas-parsers.spec.js +++ b/packages/melonjs/tests/texture-atlas-parsers.spec.js @@ -146,7 +146,7 @@ describe("no-arg atlas iteration (getAnimationSettings)", () => { }); }); -describe("UV coordinate-alias cache (#1281 workaround)", () => { +describe("UV source-rectangle cache", () => { it("a full-image lookup is not poisoned by a frame at (0,0)", () => { const atlas = new TextureAtlas( { framewidth: 32, frameheight: 32 }, @@ -159,15 +159,14 @@ describe("UV coordinate-alias cache (#1281 workaround)", () => { expect(Array.from(uvs)).toEqual([0, 0, 1, 1]); }); - it("a sub-region lookup hits the alias instead of duplicating the region", () => { + it("a sub-region lookup reuses the frame's precomputed UVs", () => { const atlas = new TextureAtlas( { framewidth: 32, frameheight: 32 }, fakeImage(64, 64, "sheet.png"), false, ); const frame1uvs = atlas.getAtlas()["1"].uvs; - // frame "1" is at (32,0,32,32) — the coordinate lookup must resolve - // to the SAME uvs array (cache hit), not an ad-hoc duplicate region + // frame "1" is at (32,0,32,32); numeric lookup reuses its UV array. expect(atlas.getUVs(32, 0, 32, 32)).toBe(frame1uvs); }); }); diff --git a/packages/melonjs/tests/texture-atlas-uvs.spec.js b/packages/melonjs/tests/texture-atlas-uvs.spec.js new file mode 100644 index 0000000000..96bef63d3a --- /dev/null +++ b/packages/melonjs/tests/texture-atlas-uvs.spec.js @@ -0,0 +1,217 @@ +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { + Application, + boot, + Matrix3d, + TextureAtlas, + video, + WebGLRenderer, +} from "../src/index.js"; +import WebGPURenderer from "../src/video/webgpu/webgpu_renderer.js"; +import { readPixel } from "./helpers/pixels.js"; + +const sheet = () => { + return new TextureAtlas( + { framewidth: 16, frameheight: 16 }, + { width: 32, height: 32 }, + false, + ); +}; + +describe("TextureAtlas UV lookup without coordinate regions", () => { + it("keeps packed UVs axis-aligned for rotated and trimmed frames", () => { + const atlas = new TextureAtlas( + { + meta: { + app: "texturepacker", + image: "packed.png", + size: { w: 64, h: 64 }, + }, + frames: [ + { + filename: "packed", + frame: { x: 8, y: 8, w: 16, h: 8 }, + rotated: true, + trimmed: true, + spriteSourceSize: { x: 2, y: 1, w: 8, h: 16 }, + sourceSize: { w: 12, h: 20 }, + }, + ], + }, + { width: 64, height: 64 }, + false, + ); + const region = atlas.getRegion("packed"); + expect(region.angle).not.toBe(0); + expect(region.trimmed).toBe(true); + expect(Array.from(atlas.getUVs(8, 8, 16, 8))).toEqual([ + 0.125, 0.125, 0.375, 0.25, + ]); + expect(atlas.getUVs(8, 8, 16, 8)).toBe(region.uvs); + }); + + it("keeps only authored frames in the atlas dictionary", () => { + const atlas = sheet(); + expect(Object.keys(atlas.getAtlas())).toEqual(["0", "1", "2", "3"]); + }); + + it("does not register arbitrary drawImage source rectangles as frames", () => { + const atlas = sheet(); + const names = Object.keys(atlas.getAtlas()); + expect(Array.from(atlas.getUVs(4, 8, 8, 16))).toEqual([ + 0.125, 0.25, 0.375, 0.75, + ]); + expect(Object.keys(atlas.getAtlas())).toEqual(names); + expect(Object.keys(atlas.getAnimationSettings().atlasIndices)).toEqual([ + "0", + "1", + "2", + "3", + ]); + }); + + it("reuses precomputed frame UVs and caches alternating arbitrary rectangles", () => { + const atlas = sheet(); + expect(atlas.getUVs(16, 0, 16, 16)).toBe(atlas.getRegion("1").uvs); + const first = atlas.getUVs(4, 8, 8, 16); + const second = atlas.getUVs(8, 4, 16, 8); + expect(atlas.getUVs(4, 8, 8, 16)).toBe(first); + expect(atlas.getUVs(8, 4, 16, 8)).toBe(second); + expect(Array.from(first)).toEqual([0.125, 0.25, 0.375, 0.75]); + }); + + it("rejects missing names and coordinate strings without creating regions", () => { + const atlas = sheet(); + const names = Object.keys(atlas.getAtlas()); + expect(() => { + return atlas.getUVs("missing"); + }).toThrow(/region.*missing.*not found/i); + expect(() => { + return atlas.getUVs("4,8,8,16"); + }).toThrow(/not found/i); + expect(Object.keys(atlas.getAtlas())).toEqual(names); + }); + + it("keeps an explicitly named coordinate string distinct from a numeric rectangle", () => { + const atlas = sheet(); + const named = atlas.addRegion("4,8,8,16", 0, 0, 32, 32); + expect(atlas.getUVs("4,8,8,16")).toBe(named.uvs); + expect(Array.from(atlas.getUVs(4, 8, 8, 16))).toEqual([ + 0.125, 0.25, 0.375, 0.75, + ]); + expect(atlas.getRegion("4,8,8,16")).toBe(named); + }); + + it("scopes identical source rectangles to the active atlas page", () => { + const atlas = sheet(); + atlas.sources.set("large", { width: 64, height: 64 }); + atlas.atlases.set("large", {}); + const small = atlas.getUVs(0, 0, 16, 16); + atlas.activeAtlas = "large"; + const large = atlas.getUVs(0, 0, 16, 16); + expect(Array.from(large)).toEqual([0, 0, 0.25, 0.25]); + expect(large).not.toBe(small); + atlas.activeAtlas = "default"; + expect(atlas.getUVs(0, 0, 16, 16)).toBe(small); + expect(Object.keys(atlas.getAtlas("large"))).toEqual([]); + }); + + it("refreshes a cached rectangle when addRegion supplies precomputed UVs", () => { + const atlas = sheet(); + const previous = atlas.getUVs(4, 8, 8, 16); + const region = atlas.addRegion("clip", 4, 8, 8, 16); + expect(atlas.getUVs(4, 8, 8, 16)).toBe(region.uvs); + expect(region.uvs).not.toBe(previous); + }); + + it("uses intrinsic video dimensions for anonymous numeric rectangles", () => { + const atlas = sheet(); + atlas.sources.set("default", { + width: 0, + height: 0, + videoWidth: 64, + videoHeight: 128, + }); + expect(Array.from(atlas.getUVs(8, 16, 16, 32))).toEqual([ + 0.125, 0.125, 0.375, 0.375, + ]); + }); +}); + +describe.each([ + ["WebGL", WebGLRenderer], + ["WebGPU", WebGPURenderer], +])("%s pattern UV consumer", (_name, Renderer) => { + it("uses numeric UV lookup while preserving repeat coordinates", () => { + const pattern = sheet(); + const getUVs = vi.spyOn(pattern, "getUVs"); + const batcher = { addQuad: vi.fn() }; + const renderer = { + currentBatcher: batcher, + setBatcher: vi.fn(), + currentTint: { + toUint32: () => { + return 0xffffffff; + }, + }, + getGlobalAlpha: () => { + return 1; + }, + }; + Renderer.prototype.drawPattern.call(renderer, pattern, 2, 3, 64, 96); + expect(getUVs).toHaveBeenCalledWith(0, 0, 64, 96); + expect(batcher.addQuad.mock.calls[0].slice(5, 9)).toEqual([0, 0, 2, 3]); + expect(Object.keys(pattern.getAtlas())).toEqual(["0", "1", "2", "3"]); + }); +}); + +describe("WebGL source rectangles and repeating patterns", () => { + let app; + let renderer; + let image; + beforeAll(async () => { + await boot(); + app = new Application(32, 32, { + parent: "screen", + renderer: video.WEBGL, + antiAlias: false, + failIfMajorPerformanceCaveat: false, + }); + await app.init(); + renderer = app.renderer; + expect(renderer).toBeInstanceOf(WebGLRenderer); + const projection = new Matrix3d(); + projection.ortho(0, 32, 32, 0, -1000, 1000); + renderer.setProjection(projection); + image = document.createElement("canvas"); + image.width = image.height = 8; + const ctx = image.getContext("2d"); + ctx.fillStyle = "red"; + ctx.fillRect(0, 0, 4, 8); + ctx.fillStyle = "blue"; + ctx.fillRect(4, 0, 4, 8); + }); + afterAll(() => { + return app?.destroy(); + }); + + it("draws cropped pixels without adding animation frames", async () => { + const atlas = renderer.cache.get(image); + const names = Object.keys(atlas.getAtlas()); + renderer.clearColor("black"); + renderer.drawImage(image, 4, 0, 4, 8, 0, 0, 16, 16); + expect(await readPixel(renderer, 8, 8)).toEqual([0, 0, 255, 255]); + expect(Object.keys(atlas.getAtlas())).toEqual(names); + }); + + it("repeats the full image without adding pattern-sized regions", async () => { + const pattern = renderer.createPattern(image, "repeat"); + const names = Object.keys(pattern.getAtlas()); + renderer.clearColor("black"); + renderer.drawPattern(pattern, 0, 0, 24, 24); + expect(await readPixel(renderer, 1, 4)).toEqual([255, 0, 0, 255]); + expect(await readPixel(renderer, 5, 4)).toEqual([0, 0, 255, 255]); + expect(await readPixel(renderer, 9, 4)).toEqual([255, 0, 0, 255]); + expect(Object.keys(pattern.getAtlas())).toEqual(names); + }); +});