fix(webgl): createPattern cache-key collision + Canvas/WebGL no-arg parity (#1448)

[obiot]

Closes #1448.

The bug

Two createPattern(image, repeat) calls on the same source image with different repeat modes used to silently share one GL texture unit. The second call's wrap state trampled the first pattern's handle:

const horiz = renderer.createPattern(sky, "repeat-x");
const vert  = renderer.createPattern(sky, "repeat-y");
renderer.drawPattern(horiz, 0, 0, 800, 64);   // ← actually sampled "repeat-y"

Same outcome whenever two ImageLayers shared a source image with different repeat modes.

Root cause (deeper than the issue suggested)

It wasn't just the createPattern cache-key — the actual collision was in TextureCache.getUnit(texture):

// cache.js — before
getUnit(texture) {
    const source = texture.sources.get(texture.activeAtlas);
    if (!this.units.has(source)) {
        this.units.set(source, this.allocateTextureUnit());
    }
    return this.units.get(source);
}

units keyed by source alone → two TextureAtlas instances built from the same image (but different wrap modes) collided on the same unit, shared the same GL texture, last upload won.

The cache.has(image) / deleteTexture2D(...) band-aid in createPattern was added for #1278's leak; it made #1448 worse by actively freeing the still-live first-pattern texture before the new upload.

Fix

Key the units map by (source, repeat) via a nested Map<source, Map<repeat, unit>>.

// cache.js — after
_unitKey(texture) {
    return {
        source: texture.sources.get(texture.activeAtlas),
        repeat: texture.repeat || "no-repeat",
    };
}

getUnit(texture) {
    const { source, repeat } = this._unitKey(texture);
    let perRepeat = this.units.get(source);
    if (perRepeat === undefined) {
        perRepeat = new Map();
        this.units.set(source, perRepeat);
    }
    if (!perRepeat.has(repeat)) {
        perRepeat.set(repeat, this.allocateTextureUnit());
    }
    return perRepeat.get(repeat);
}

Same external API (getUnit(texture) in, unit number out) — only the internal map structure changes. freeTextureUnit and peekUnit get the same treatment.

The band-aid in createPattern is removed:

• Same-(source, repeat) re-uploads short-circuit via uploadTexture's existing boundTextures[unit] check — no leak, no allocation.
• Different-repeat calls each get their own unit — no collision.

Side effect: Canvas/WebGL parity on the no-arg form

renderer.createPattern(image) (no repeat) used to throw a DOM TypeError under Canvas while WebGL silently defaulted to "no-repeat" via TextureAtlas's internal fallback. Both renderers now default repeat = "no-repeat" at the engine entry point. JSDoc updated to [repeat="no-repeat"].

What's NOT touched

• Public createPattern(image, repeat) / drawPattern(...) signatures.
• TextureAtlas shape — still exposes .image, .repeat, .sources, etc.
• getUnit / peekUnit / freeTextureUnit external signatures (still take a texture).
• All existing TextureAtlas users (sprites, atlases, tilemaps, lights, meshes use repeat = "no-repeat" → identical composite key → identical unit allocation).

Tests

• New tests/createPattern_repeat_parity.spec.js (4 tests) — runs the same scenario under each renderer with explicit video.init(..., { renderer: video.<MODE> }). Asserts user-visible invariants: distinct handles, usedUnits.size grows by 1 per call on WebGL, no-arg form tolerated identically on both.
• Flipped tests/texture.spec.js "should clean up previous pattern when repeat mode changes" → "allocates a separate texture unit per (image, repeat) pair (WebGL: createPattern cache-key collision invalidates earlier pattern handles for the same image #1448)". The old assertion (usedUnits.size stays flat) was actively pinning the bug in place. Kept as a focused regression guard.

Test plan

• `pnpm test:types` clean
• `pnpm vitest run` — 3979 / 13 skipped / 0 failed (+4 new parity tests)
• `pnpm build` — lint + types clean

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a WebGL texture-unit collision affecting createPattern(image, repeat) when the same source image is used with multiple repeat modes, and aligns Canvas/WebGL behavior for the no-argument createPattern(image) form.

Changes:

• Update TextureCache texture-unit allocation to key by (source, repeat) instead of source alone, preventing wrap-mode collisions.
• Default repeat to "no-repeat" in both Canvas and WebGL renderers for createPattern(image, repeat?), matching behavior across renderers.
• Add/adjust regression tests to cover repeat-mode independence and no-arg parity; document the fix in the changelog.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/melonjs/src/video/texture/cache.js	Switch texture-unit mapping to nested (source → repeat → unit) allocation.
packages/melonjs/src/video/webgl/webgl_renderer.js	Default repeat param and remove prior delete-and-replace cleanup behavior.
packages/melonjs/src/video/canvas/canvas_renderer.js	Default repeat param to avoid DOM TypeError and ensure parity.
packages/melonjs/tests/texture.spec.js	Update regression expectation for unit allocation across repeat modes.
packages/melonjs/tests/createPattern_repeat_parity.spec.js	Add parity/regression coverage for repeat collisions and no-arg behavior.
packages/melonjs/CHANGELOG.md	Document the collision fix and the Canvas/WebGL parity change.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/melonjs/src/video/texture/cache.js:219

• TextureCache.delete(image) now frees texture units for every cached atlas under image, but it still only updates cache bookkeeping. When called from MaterialBatcher.deleteTexture2D(TextureAtlas), the batcher deletes/unbinds only the one GL texture for the passed-in atlas and then calls cache.delete(texture.getTexture()), which will now free additional repeat-mode units without deleting/unbinding their corresponding boundTextures[unit]. That can leave orphaned GL textures and stale boundTextures entries for freed units (later re-use can bind the wrong texture or skip upload).

	delete(image) {
		if (this.cache.has(image)) {
			// Free every atlas registered under this image, not just the
			// first one. Post-#1448 (units keyed by (source, repeat))
			// multiple atlases can coexist for one image — freeing only
			// `cache.get(image)[0]` would leak the remaining repeats'
			// texture units after `this.cache.delete(image)` wipes the
			// entire multimap bucket.
			for (const texture of this.cache.get(image)) {
				this.freeTextureUnit(texture);
			}
			this.cache.delete(image);
		}

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.