diff --git a/packages/melonjs/CHANGELOG.md b/packages/melonjs/CHANGELOG.md index e02dfa6f8..5ffabe657 100644 --- a/packages/melonjs/CHANGELOG.md +++ b/packages/melonjs/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## [20.10.0] (melonJS 2) - _unreleased_ + +### Fixed +- 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)). (thanks @snowyukitty) + ## [20.9.0] (melonJS 2) - _2026-10-18_ ### Added diff --git a/packages/melonjs/src/video/texture/atlas.js b/packages/melonjs/src/video/texture/atlas.js index 43d84a498..5d390e481 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 73311aa25..e3883453c 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 2585eb064..700f9aa91 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 0a729aded..ed32d71ff 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 000000000..96bef63d3 --- /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); + }); +});