From 3f57cf731dc472017c6dbf50186ebfaf29ed3850 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Sun, 4 Oct 2026 00:30:00 +0200 Subject: [PATCH] Keep a pixel frame worn through a shader alive while it is drawn Changing a palette colour of pixel art flooded the console with Pixi warnings: "[BindGroup] a 'textureSource' was destroyed while still bound to a shader", and the same for its sampler. #293 had stopped this for a drawing, but a frame of pixel art is worn as a cut of the sheet. The cut has a source of its own, so the sheet looked unbound, was freed at once and took the cut with it under the shader. - The check whether a replaced picture is still in use gets its key too. TextureLibrary then asks the picture and every cut taken from it. - A sheet with a bound cut waits like a bound picture did since #293, and both go together once the shader has taken the new frame. Found on the way: - The project the report came from has a sprite wearing a frame of an eleven-frame pixel art through a shader asset. - A nine-sliced sprite wearing the frame showed no warning, even without the change. The first try of the e2e check used it. Tests: - Gate on melektaus-atx: 236 files, 3577 tests (+1) - e2e 48: 45 checks (+1, sum 1741). 48l-iii puts a shader on the object wearing the second frame, changes a palette colour three times and counts the warnings. - Counter-checks bite: without the cuts in the check the unit test fails, and 48l-iii counts 6 warnings for 3 changes. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/viewport/TextureLibrary.ts | 11 ++++++-- src/viewport/generatedPictures.ts | 6 ++-- tests/viewport/bound-pictures.test.ts | 40 +++++++++++++++++++++++++++ tools/e2e/48-pixelart.mjs | 31 +++++++++++++++++++++ 5 files changed, 84 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 4bc76e23..8b1e2646 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ npm run build # writes dist/ and regenerates index.html; the folder is wh The browser suite and the three example games need a machine with a GPU: ```bash -node tools/seed-e2e.mjs && node tools/e2e.mjs # the editor, 1740 checks (GM_STEPS=20 for one step) +node tools/seed-e2e.mjs && node tools/e2e.mjs # the editor, 1741 checks (GM_STEPS=20 for one step) # every tool signs in first – see the Accounts page node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs node tools/seed-tetris.mjs && node tools/smoke-tetris.mjs diff --git a/src/viewport/TextureLibrary.ts b/src/viewport/TextureLibrary.ts index 9b3e2796..9ea7ac02 100644 --- a/src/viewport/TextureLibrary.ts +++ b/src/viewport/TextureLibrary.ts @@ -36,7 +36,9 @@ export class TextureLibrary extends FileLibrary { this.dropCropsOf(key); texture.destroy(true); }, - boundToShader, + // A frame of pixel art is worn as a cut, which has a source of its own: the sheet itself is + // then bound nowhere, and freeing it destroyed the cut under the shader. + (texture, key) => boundToShader(texture) || this.cropsOf(key).some(boundToShader), // After the redraw that asked: every mesh it synced wears the new picture by then. (retry) => queueMicrotask(retry), ); @@ -152,7 +154,12 @@ export class TextureLibrary extends FileLibrary { return Assets.load(url); } - /** Cuts taken from a picture that has moved on to a newer revision; the key is their prefix. */ + /** The cuts taken from one picture; the file is the prefix of their key. */ + private cropsOf(file: string): Texture[] { + return [...this.crops].filter(([key]) => key.startsWith(`${file}|`)).map(([, texture]) => texture); + } + + /** Cuts taken from a picture that has moved on to a newer revision. */ private dropCropsOf(file: string): void { for (const [key, texture] of this.crops) if (key.startsWith(`${file}|`)) { diff --git a/src/viewport/generatedPictures.ts b/src/viewport/generatedPictures.ts index 55706056..eea044f4 100644 --- a/src/viewport/generatedPictures.ts +++ b/src/viewport/generatedPictures.ts @@ -34,7 +34,7 @@ export class GeneratedPictures { */ constructor( private readonly free: (picture: T, key: string) => void, - private readonly inUse: (picture: T) => boolean = () => false, + private readonly inUse: (picture: T, key: string) => boolean = () => false, private readonly soon: (retry: () => void) => void = () => {}, ) {} @@ -80,7 +80,7 @@ export class GeneratedPictures { private freeWaiting(): void { this.waiting = this.waiting.filter((held) => { - if (this.inUse(held.picture)) return true; + if (this.inUse(held.picture, held.key)) return true; this.free(held.picture, held.key); return false; }); @@ -95,7 +95,7 @@ export class GeneratedPictures { const held = this.kept.get(key); if (!held) return; this.kept.delete(key); - if (!this.inUse(held.picture)) return this.free(held.picture, key); + if (!this.inUse(held.picture, key)) return this.free(held.picture, key); this.waiting.push({ key, picture: held.picture }); if (this.asked) return; this.asked = true; diff --git a/tests/viewport/bound-pictures.test.ts b/tests/viewport/bound-pictures.test.ts index 26ca9992..b5862282 100644 --- a/tests/viewport/bound-pictures.test.ts +++ b/tests/viewport/bound-pictures.test.ts @@ -97,6 +97,46 @@ describe('a drawn picture a shader still names (#293)', () => { expect(r.warnings()).toEqual([]); }); + it('waits while a shader names a frame cut from it, as pixel art is worn', () => { + // A frame is cut onto a canvas of its own, which needs an image to cut from. + vi.stubGlobal( + 'OffscreenCanvas', + class { + getContext = () => ({ drawImage: () => {} }); + }, + ); + const library = new TextureLibrary(); + library.setPictureFactory( + () => new Texture({ source: new TextureSource({ resource: {}, width: 8, height: 4 }) }), + ); + const warned = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const frameOf = (file: string) => + library.get({ + file, + rect: { x: 4, y: 0, w: 4, h: 4 }, + smooth: false, + sourceWidth: 8, + sourceHeight: 4, + slice: null, + })!; + library.load('gen:a@1'); + const cut = frameOf('gen:a@1'); + const shader = new BindGroup({ 0: cut.source, 1: cut.source.style }); + + // A colour of the palette changes: a new revision, and the cut is still in the shader. + library.load('gen:a@2'); + expect(cut.destroyed).toBe(false); + expect(library.waitingPictures()).toEqual(['gen:a@1']); + + shader.setResource(frameOf('gen:a@2').source, 0); + shader.setResource(Texture.WHITE.source.style, 1); + library.load('gen:a@2'); + expect(cut.destroyed).toBe(true); + expect(library.waitingPictures()).toEqual([]); + expect(warned.mock.calls.filter((call) => call.join(' ').includes('BindGroup'))).toEqual([]); + vi.unstubAllGlobals(); + }); + it('is destroyed right away when no shader names it', () => { const r = rig(); const first = r.library.load('gen:a@1')!; diff --git a/tools/e2e/48-pixelart.mjs b/tools/e2e/48-pixelart.mjs index c3ddc6bf..808b4d43 100644 --- a/tools/e2e/48-pixelart.mjs +++ b/tools/e2e/48-pixelart.mjs @@ -705,6 +705,37 @@ export async function pixelArt(t) { delete art.props.sliceScale;`); await settle(); + // 48l-iii. The object wears the frame through a shader asset: its mesh names a cut of the sheet, + // which has a source of its own. A palette colour changed under it freed the sheet at + // once and the cut with it, and Pixi warned for the source and its sampler every time. + await mutate(` + Object.assign(__gm.state.activeScene.objects.find((o) => o.id === 'pa-object').overrides, + { shader: 'adjust-colors', saturation: 0 })`); + await settle(); + await sleep(300); + const boundLogFrom = t.consoleLog.length; + const waiting = []; + for (const color of [0x3050a0, 0x70b030, 0xff004d]) { + await mutate(` + const p = __gm.state.project; + const art = p.assets.find((a) => a.id === ${JSON.stringify(id)}); + p.assets.find((a) => a.id === art.props.palette).colors[${RED - 1}] = ${color};`); + await settle(); + await sleep(200); + waiting.push((await json('__gm.picturesWaiting()')).length); + } + const boundWarnings = t.consoleLog.slice(boundLogFrom).filter((line) => line.includes('BindGroup')); + check( + 'a palette colour changed under a frame worn through a shader warns of no destroyed texture', + boundWarnings.length === 0 && waiting.every((n) => n === 0), + JSON.stringify({ warnings: boundWarnings.length, waiting }), + ); + await mutate(` + const o = __gm.state.activeScene.objects.find((o) => o.id === 'pa-object'); + delete o.overrides.shader; + delete o.overrides.saturation;`); + await settle(); + const textureRow = await json("__gm.inspectorField('Texture', true)"); await click(textureRow.x, textureRow.y); await settle();