adminandClaude Opus 5.5
9e869df063
Editing a vector drawing made Pixi warn "destroyed while still bound
to a shader", twice with every change, once it had begun. It began
where an object wears the drawing through a shader asset. Each
change draws the picture anew and destroyed the one before it, but
the mesh of that object keeps its picture bound until it syncs. The
issue also asks for the button "Add shape" to go: a shape without
points cannot be given any, the Pen draws a new one.
- GeneratedPictures takes two more callbacks. inUse says that a
picture cannot be freed yet, soon runs a retry once whoever asked
is done.
- A replaced picture that is still in use waits in a list of its
own. It is freed right after the redraw that asked, as a rule. On
a later ask where its wearer was not part of that redraw.
- TextureLibrary tells a bound picture by the listeners for change
on its source. Under WebGL the bind groups are the only ones that
listen there. The retry is a microtask.
- The rule is asked for every drawn picture alike, whoever wears it.
- The inspector of a drawing and of a shape has no "Add shape" any
more. "Delete shapes" for several selected shapes stays.
Decided here:
- Under the list of shapes a second note says "The Pen draws a new
shape." while only the asset is selected.
Found on the way:
- Reproduced on a throwaway copy of the project the report came
from: 6 warnings for 3 changes with the shader override on the
object, none without it. After the change none, and no replaced
picture is left waiting.
- The fixture project of the tests never showed the warning. No
object there wore a drawing through a shader.
Tests:
- Gate on melektaus-atx: 211 files, 3219 tests (3209 + 10), exit 0
- e2e 47,48 on melektaus-atx: 113 checks, all pass
- e2e 28,30,31,39,40,47,48: 350 checks, all pass, run before the
wording of the new note changed
- The first gate was red: an existing test read the wording of the
note that had changed. Only three test files had been run after
the change.
- No full run and no smokes. The sum of 1668 (1667 + 1) is reckoned,
1667 is what CI counted for ec2fd90.
- Ten unit tests, five of them against the real bind group of Pixi
in tests/viewport/bound-pictures.test.ts
- One e2e check in block 47b-ii of module 47: a drawing worn through
a shader, three changes, no warning. The check on "Add shape" now
reads that there is no such button.
- Nine unit counter-probes and two browser counter-probes all fail
their tests. Without the change the new check counts 6 warnings.
- Not checked: a tilemap or a particle emitter holding a picture
bound the same way. Nothing was looked at after the build, the
new note included.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
235 lines
8.3 KiB
TypeScript
235 lines
8.3 KiB
TypeScript
import type { Texture } from 'pixi.js';
|
|
import { afterEach, describe, expect, it } from 'vitest';
|
|
import { CreateAssetCommand } from '../../src/core/commands/assetCommands';
|
|
import { registerPictureSource } from '../../src/core/model/generated';
|
|
import {
|
|
GeneratedPictures,
|
|
projectPictureFactory,
|
|
registerPictureGenerator,
|
|
} from '../../src/viewport/generatedPictures';
|
|
import { makeSpriteScene } from '../helpers';
|
|
|
|
/** The pictures are plain strings here: what is measured is who is kept and who is let go. */
|
|
function library() {
|
|
const freed: string[] = [];
|
|
const drawn: string[] = [];
|
|
const pictures = new GeneratedPictures<string>((picture) => freed.push(picture));
|
|
let answer: (assetId: string) => string | null = (assetId) => assetId;
|
|
pictures.setFactory((assetId) => {
|
|
drawn.push(assetId);
|
|
return answer(assetId);
|
|
});
|
|
return {
|
|
pictures,
|
|
freed,
|
|
drawn,
|
|
answerWith: (fn: (assetId: string) => string | null) => {
|
|
answer = fn;
|
|
},
|
|
};
|
|
}
|
|
|
|
describe('generated pictures kept in memory', () => {
|
|
it('draws a key once and hands the same picture out again', () => {
|
|
const f = library();
|
|
f.answerWith(() => 'first');
|
|
expect(f.pictures.get('gen:a@1')).toBe('first');
|
|
expect(f.pictures.get('gen:a@1')).toBe('first');
|
|
expect(f.drawn).toEqual(['a']);
|
|
expect(f.freed).toEqual([]);
|
|
});
|
|
|
|
it('lets the old revision go the moment the new one is drawn', () => {
|
|
const f = library();
|
|
f.answerWith((id) => `${id}-one`);
|
|
f.pictures.get('gen:a@1');
|
|
f.answerWith((id) => `${id}-two`);
|
|
expect(f.pictures.get('gen:a@2')).toBe('a-two');
|
|
expect(f.freed).toEqual(['a-one']);
|
|
expect(f.pictures.keys()).toEqual(['gen:a@2']);
|
|
// Only that asset's own older picture goes; a neighbour keeps hers.
|
|
f.answerWith((id) => `${id}-b`);
|
|
f.pictures.get('gen:b@1');
|
|
f.pictures.get('gen:a@3');
|
|
expect(f.freed).toEqual(['a-one', 'a-two']);
|
|
expect(f.pictures.keys()).toEqual(['gen:b@1', 'gen:a@3']);
|
|
});
|
|
|
|
it('asks again for a picture that could not be drawn yet', () => {
|
|
const f = library();
|
|
f.answerWith(() => null);
|
|
expect(f.pictures.get('gen:a@1')).toBeNull();
|
|
expect(f.pictures.peek('gen:a@1')).toBeNull();
|
|
f.answerWith(() => 'ready');
|
|
expect(f.pictures.get('gen:a@1')).toBe('ready');
|
|
expect(f.drawn).toEqual(['a', 'a']);
|
|
});
|
|
|
|
it('draws nothing for a name that is not a key', () => {
|
|
const f = library();
|
|
expect(f.pictures.get(`${'ab'.repeat(32)}.png`)).toBeNull();
|
|
expect(f.pictures.get('gen:a')).toBeNull();
|
|
expect(f.drawn).toEqual([]);
|
|
});
|
|
|
|
it('gives everything back when another session takes over', () => {
|
|
const f = library();
|
|
f.pictures.get('gen:a@1');
|
|
f.pictures.get('gen:b@1');
|
|
f.pictures.setFactory(null);
|
|
expect(f.freed).toEqual(['a', 'b']);
|
|
expect(f.pictures.keys()).toEqual([]);
|
|
expect(f.pictures.get('gen:a@1')).toBeNull();
|
|
});
|
|
|
|
describe('a picture something still draws with (#293)', () => {
|
|
/** Like `library`, with pictures that can be held on to. */
|
|
function held() {
|
|
const freed: string[] = [];
|
|
const busy = new Set<string>();
|
|
const pictures = new GeneratedPictures<string>(
|
|
(picture, key) => freed.push(`${picture} ${key}`),
|
|
(picture) => busy.has(picture),
|
|
);
|
|
let revision = 0;
|
|
pictures.setFactory((assetId) => `${assetId}${++revision}`);
|
|
return { pictures, freed, busy };
|
|
}
|
|
|
|
it('keeps the old revision while it is in use and frees it on the next ask after', () => {
|
|
const f = held();
|
|
f.pictures.get('gen:a@1');
|
|
f.busy.add('a1');
|
|
expect(f.pictures.get('gen:a@2')).toBe('a2');
|
|
// Gone from what is handed out, but not freed.
|
|
expect(f.pictures.keys()).toEqual(['gen:a@2']);
|
|
expect(f.pictures.peek('gen:a@1')).toBeNull();
|
|
expect(f.freed).toEqual([]);
|
|
expect(f.pictures.waitingKeys()).toEqual(['gen:a@1']);
|
|
// Still held: asking again changes nothing.
|
|
f.pictures.get('gen:a@2');
|
|
expect(f.freed).toEqual([]);
|
|
// Let go: any ask frees it, also one for a picture that is already there.
|
|
f.busy.delete('a1');
|
|
f.pictures.get('gen:a@2');
|
|
expect(f.freed).toEqual(['a1 gen:a@1']);
|
|
expect(f.pictures.waitingKeys()).toEqual([]);
|
|
// And only once.
|
|
f.pictures.get('gen:a@2');
|
|
f.pictures.get('gen:a@3');
|
|
expect(f.freed).toEqual(['a1 gen:a@1', 'a2 gen:a@2']);
|
|
});
|
|
|
|
it('frees what waits in the order it was replaced, and leaves the one still held', () => {
|
|
const f = held();
|
|
f.pictures.get('gen:a@1');
|
|
f.busy.add('a1');
|
|
f.pictures.get('gen:a@2');
|
|
f.busy.add('a2');
|
|
f.pictures.get('gen:a@3');
|
|
expect(f.pictures.waitingKeys()).toEqual(['gen:a@1', 'gen:a@2']);
|
|
f.busy.delete('a2');
|
|
f.pictures.get('gen:a@3');
|
|
expect(f.freed).toEqual(['a2 gen:a@2']);
|
|
expect(f.pictures.waitingKeys()).toEqual(['gen:a@1']);
|
|
});
|
|
|
|
it('frees a picture nobody holds right away', () => {
|
|
const f = held();
|
|
f.pictures.get('gen:a@1');
|
|
f.pictures.get('gen:a@2');
|
|
expect(f.freed).toEqual(['a1 gen:a@1']);
|
|
expect(f.pictures.waitingKeys()).toEqual([]);
|
|
});
|
|
|
|
it('retries once whoever asked is done, and asks for that only once per wait', () => {
|
|
const freed: string[] = [];
|
|
const busy = new Set<string>();
|
|
const later: (() => void)[] = [];
|
|
const pictures = new GeneratedPictures<string>(
|
|
(picture) => freed.push(picture),
|
|
(picture) => busy.has(picture),
|
|
(retry) => later.push(retry),
|
|
);
|
|
let revision = 0;
|
|
pictures.setFactory((assetId) => `${assetId}${++revision}`);
|
|
pictures.get('gen:a@1');
|
|
pictures.get('gen:b@1');
|
|
busy.add('a1').add('b2');
|
|
pictures.get('gen:a@2');
|
|
pictures.get('gen:b@2');
|
|
expect(later).toHaveLength(1);
|
|
// The wearer of a took the new picture during the redraw, the one of b did not.
|
|
busy.delete('a1');
|
|
later.shift()!();
|
|
expect(freed).toEqual(['a1']);
|
|
expect(pictures.waitingKeys()).toEqual(['gen:b@1']);
|
|
// What still waits is not retried on a timer: the next ask does that.
|
|
expect(later).toHaveLength(0);
|
|
busy.delete('b2');
|
|
pictures.get('gen:b@2');
|
|
expect(freed).toEqual(['a1', 'b2']);
|
|
// A new wait asks for a retry again.
|
|
busy.add('a3');
|
|
pictures.get('gen:a@3');
|
|
expect(later).toHaveLength(1);
|
|
});
|
|
|
|
it('lets a held picture wait through a change of session too', () => {
|
|
const f = held();
|
|
f.pictures.get('gen:a@1');
|
|
f.pictures.get('gen:b@1');
|
|
f.busy.add('a1');
|
|
f.pictures.setFactory((assetId) => `${assetId}-next`);
|
|
expect(f.freed).toEqual(['b2 gen:b@1']);
|
|
expect(f.pictures.keys()).toEqual([]);
|
|
f.busy.delete('a1');
|
|
expect(f.pictures.get('gen:c@1')).toBe('c-next');
|
|
expect(f.freed).toEqual(['b2 gen:b@1', 'a1 gen:a@1']);
|
|
});
|
|
});
|
|
|
|
it('answers peek only with what has been drawn', () => {
|
|
const f = library();
|
|
expect(f.pictures.peek('gen:a@1')).toBeNull();
|
|
f.pictures.get('gen:a@1');
|
|
expect(f.pictures.peek('gen:a@1')).toBe('a');
|
|
expect(f.drawn).toEqual(['a']);
|
|
});
|
|
});
|
|
|
|
describe('the factory the editor runs with', () => {
|
|
const registered: (() => void)[] = [];
|
|
afterEach(() => {
|
|
for (const undo of registered.splice(0)) undo();
|
|
});
|
|
|
|
it('hands an asset to the generator of its type and nothing else to anyone', () => {
|
|
const f = makeSpriteScene();
|
|
const sound = new CreateAssetCommand('sound', 'Beep');
|
|
f.stack.execute(sound);
|
|
registered.push(registerPictureSource('sound', () => ({ width: 8, height: 8, revision: 1 })));
|
|
const seen: string[] = [];
|
|
const picture = {} as Texture;
|
|
registered.push(
|
|
registerPictureGenerator('sound', (asset) => {
|
|
seen.push(asset.name);
|
|
return picture;
|
|
}),
|
|
);
|
|
|
|
const factory = projectPictureFactory(() => f.project);
|
|
expect(factory(sound.asset.id)).toBe(picture);
|
|
expect(seen).toEqual(['Beep']);
|
|
// A sprite has no generator, and an id that is gone has no asset.
|
|
expect(factory(f.spriteId)).toBeNull();
|
|
expect(factory('nobody')).toBeNull();
|
|
expect(seen).toEqual(['Beep']);
|
|
});
|
|
|
|
it('draws nothing while no project is open', () => {
|
|
const factory = projectPictureFactory(() => null);
|
|
expect(factory('a')).toBeNull();
|
|
});
|
|
});
|