TextureCache.get(image, frame): framewidth/frameheight refinement is a no-op for every atlas format

[obiot]

Summary

TextureCache.get(image, { framewidth, frameheight }) in packages/melonjs/src/video/texture/cache.js:166 documents itself as picking the atlas under image whose dimensions match the requested frame size, useful when an image has multiple atlases registered (e.g. a cropped variant alongside the full-image default). In practice the refinement loop is a no-op against every atlas format the engine produces — it has been dormant for as long as the parser has been unified.

Surfaced while writing adversarial tests for #1448; not a live bug today, but worth pinning before it bites.

How it's silent

this.cache.forEach((value, key) => {
    const _atlas = value.getAtlas();
    if (
        key === image &&
        _atlas.width === atlas.framewidth &&   // ← always undefined === number
        _atlas.height === atlas.frameheight
    ) {
        entry = value;
    }
});

• getAtlas() returns this.atlases.get(this.activeAtlas).
• this.atlases is populated by parseTexturePacker(atlas, this) for every format — TexturePacker, Aseprite, ShoeBox, AND the "melonJS" format produced by createAtlas() (the pattern path) all route through the same parser.
• parseTexturePacker (packages/melonjs/src/video/texture/parser/texturepacker.js:41) builds a frame-keyed dict: atlas[frame.filename] = { width: s.w, height: s.h, ... }. So _atlas is { default: { width: 64, height: 64, ... } }, not { width: 64, height: 64 }.
• _atlas.width is always undefined. undefined === framewidth is always false. The loop never updates entry. The function always returns cache.get(image)[0].

Why nothing observably breaks today

• Single-atlas-per-image is the common case (sprites, sprite sheets, tilesets). Refining "the only one" → "still the only one" is no-op anyway.
• The existing test texture.spec.js > should retrieve specific atlas by frame dimensions only asserts toBeDefined() — which holds even when the refinement no-ops, so the test passes against the broken behavior.
• The WebGL: createPattern cache-key collision invalidates earlier pattern handles for the same image #1448 fix makes multi-atlas-per-image legitimate (one atlas per repeat mode), but those atlases share dimensions (the source image's), so the framewidth refinement still wouldn't have discriminated even with a working comparison.

Options

A. Fix the comparison to use the right path. Compare _atlas[Object.keys(_atlas)[0]].width / height or, more semantically, peek at the underlying meta.size.w / .h if accessible. Real behavioral change — could surface latent bugs in any caller that's accidentally been relying on always getting cache.get(image)[0].

B. Expose top-level .width / .height on the parsed atlas dict. Tweak parseTexturePacker (and the createAtlas "melonJS"-format equivalent) to also set top-level .width and .height on the dict so the existing comparison works as intended.

C. Drop the framewidth/frameheight argument entirely. If no caller in the engine actually relies on it (grep suggests none do), removing the dead branch keeps the API smaller and prevents future confusion. The existing test would need updating.

Recommendation

Defer to the #1410 (TextureCache + Batcher renderer-agnostic refactor) reviewer / author — they'll touch this whole API surface anyway, and the right answer (which of A/B/C) depends on whether the framewidth refinement is intentional API or a vestige.

Cross-refs

• WebGL: createPattern cache-key collision invalidates earlier pattern handles for the same image #1448 (surfaced while writing tests for the createPattern cache-key fix).
• Refactor TextureCache and Batcher base class for renderer-agnostic abstraction #1410 (TextureCache refactor where this should land).

[obiot]

Re-assessed — this is a latent correctness bug, not just dead code. Targeting 19.8.

Verified the consequence with a throwaway repro. The dead refinement loop isn't merely cosmetic: it masks a real (if rare) bug.

Repro

const cache = video.renderer.cache;
const src = new CanvasTexture(128, 128).canvas;

// consumer A: 64×64 cells
const a = cache.get(src, { framewidth: 64, frameheight: 64 });
// consumer B: 32×32 cells on the SAME image
const b = cache.get(src, { framewidth: 32, frameheight: 32 });

Output:

[A] frame size: 64 x 64
[B] frame size: 64 x 64    ← should be 32 x 32
[same atlas instance?] true

Two consumers sharing one image with different cell sizes — the second silently inherits the first's frame grid. The cache.get(image, frame) refinement loop was designed to pick the matching atlas per frame size, but the _atlas.width === atlas.framewidth comparison is always undefined === number (frame dimensions live at _atlas[frameName].width, not top-level), so it never fires and cache.get(image)[0] always wins.

Real callers passing a frame arg

• renderable/sprite.js:236 — cache.get(this.image, settings) (sprite cell size)
• renderable/mesh.js:37 — cache.get(image, { framewidth: image.width, frameheight: image.height })
• level/tiled/TMXTileset.js:441 — cache.get(this.image, { ... })

They work today only because the common case is one cell-size per image.

Why deferred to 19.8 (not 19.7)

A correct fix is invasive and squarely #1410 territory:

1. TextureAtlas doesn't expose its frame dimensions today — needs a new accessor.
2. The refinement only makes semantic sense for fixed-cell spritesheets; a multi-frame TexturePacker atlas has no single "framewidth" to match against.
3. The real fix changes the cache from "one atlas per image" to "multiple atlases per image keyed by cell size" — touching the cache.get hot path AND interacting with the WebGL: createPattern cache-key collision invalidates earlier pattern handles for the same image #1448 (source, repeat) unit-keying just landed in 19.7.

Doing that on the renderer's hottest path at the 19.7 ship line was the wrong risk/reward (rare scenario, no live report). Scheduled for 19.8, ideally folded into the #1410 TextureCache refactor.

[Huaian666]

been messing around with this for a game jam. the entity system is really intuitive