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>
362 lines
15 KiB
TypeScript
362 lines
15 KiB
TypeScript
/**
|
||
* The inspector side of vector art (#129, stage 4): the arithmetic of the shape list, which rows a
|
||
* shape shows, and what editing one of them writes into the model. The rows are driven through the
|
||
* widgets' own callbacks – that is the path a click takes, `setValue` only shows a value.
|
||
*/
|
||
import { beforeAll, describe, expect, it } from 'vitest';
|
||
import { CanvasTextMetrics } from 'pixi.js';
|
||
import {
|
||
AddVectorShapeCommand,
|
||
RemoveVectorShapeCommand,
|
||
SetVectorShapeStyleCommand,
|
||
} from '../../src/core/commands/vectorArtCommands';
|
||
import type { VectorArtAsset, VectorGradient } from '../../src/core/model/Asset';
|
||
import { buildAsset } from '../../src/core/commands/assetCommands';
|
||
import { createEmptyProject, findAssetOf } from '../../src/core/model/Project';
|
||
import {
|
||
Button,
|
||
Checkbox,
|
||
ColorField,
|
||
Label,
|
||
NumberField,
|
||
TextField,
|
||
toHex,
|
||
VStack,
|
||
type Widget,
|
||
} from '../../src/ui';
|
||
import type { InspectorHost } from '../../src/editor/inspector/host';
|
||
import {
|
||
buildShapeListSection,
|
||
buildShapeSection,
|
||
currentShapeList,
|
||
refreshShapeSection,
|
||
setFillTexture,
|
||
setPaintKind,
|
||
shapeSignature,
|
||
} from '../../src/editor/inspector/shapeSection';
|
||
import { EditorState, shapeSelection } from '../../src/editor/state';
|
||
import { PaintCache } from '../../src/viewport/vectorArtPicture';
|
||
import { allShapes } from '../../src/core/model/vectorTree';
|
||
|
||
/**
|
||
* These rows really are built, and a widget repaints itself one microtask after a value goes into
|
||
* it – text width included, which Pixi measures on a canvas of the browser. The suite runs on node,
|
||
* so the measure is answered here instead of the whole test avoiding the widgets.
|
||
*/
|
||
beforeAll(() => {
|
||
CanvasTextMetrics.measureText = (() => ({ width: 10, height: 10, lines: [], lineWidths: [] })) as never;
|
||
// A gradient (#186) paints itself onto a browser canvas too; the rows under test do not need it.
|
||
PaintCache.prototype.gradient = (() => null) as never;
|
||
});
|
||
|
||
function makeHost(): InspectorHost {
|
||
const state = new EditorState();
|
||
state.loadProject(createEmptyProject('p', 'P'), 1);
|
||
return {
|
||
state,
|
||
body: new VStack(),
|
||
fields: new Map(),
|
||
nameField: null,
|
||
project: () => state.project,
|
||
liveVariables: () => null,
|
||
};
|
||
}
|
||
|
||
/** A vector art asset with `count` triangles in it, and a host to build its rows into. */
|
||
function withShapes(count: number): { host: InspectorHost; assetId: string; ids: string[] } {
|
||
const host = makeHost();
|
||
const asset = buildAsset('vectorart', 'Drawing') as VectorArtAsset;
|
||
host.state.project.assets.push(asset);
|
||
const ids: string[] = [];
|
||
for (let i = 0; i < count; i++) {
|
||
const command = new AddVectorShapeCommand(asset.id, [
|
||
{
|
||
closed: true,
|
||
points: [
|
||
{ x: i, y: 0 },
|
||
{ x: i + 20, y: 0 },
|
||
{ x: i, y: 20 },
|
||
],
|
||
},
|
||
]);
|
||
host.state.execute(command);
|
||
ids.push(command.shapeId);
|
||
}
|
||
return { host, assetId: asset.id, ids };
|
||
}
|
||
|
||
const shapeOf = (host: InspectorHost, assetId: string, index = 0) =>
|
||
allShapes(findAssetOf(host.state.project, assetId, 'vectorart')!)[index]!;
|
||
|
||
function widgets<T extends Widget>(host: InspectorHost, type: new (...args: never[]) => T): T[] {
|
||
const found: T[] = [];
|
||
const walk = (widget: Widget): void => {
|
||
if (widget instanceof type) found.push(widget);
|
||
for (const child of widget.children) walk(child as Widget);
|
||
};
|
||
walk(host.body);
|
||
return found;
|
||
}
|
||
|
||
/** The TRANSFORM rows (#291) stand over the shape's own, five number fields ahead of every other. */
|
||
const FRAME_KEYS = ['frameX', 'frameY', 'frameWidth', 'frameHeight', 'frameTurn'];
|
||
const numberFields = (host: InspectorHost): NumberField[] =>
|
||
widgets(host, NumberField).slice(FRAME_KEYS.length);
|
||
|
||
const texts = (host: InspectorHost): string[] => widgets(host, Label).map((label) => label.text);
|
||
|
||
describe('shape rows', () => {
|
||
it('shows fill, opacity and an outline switch, and no line rows without an outline', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
expect([...host.fields.keys()]).toEqual([
|
||
...FRAME_KEYS,
|
||
'name',
|
||
'fillType',
|
||
'fill',
|
||
'opacity',
|
||
'outline',
|
||
]);
|
||
expect(texts(host)).toContain('Without an outline only the fill is drawn.');
|
||
});
|
||
|
||
it('writes a fill as one undo step, a dragged colour previewing without one', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
const before = shapeOf(host, assetId).fill;
|
||
const fill = widgets(host, ColorField)[0]!;
|
||
fill.onInput!(0xff0000);
|
||
expect(shapeOf(host, assetId).fill).toBe(0xff0000);
|
||
fill.onChange!(0x00ff00);
|
||
expect(shapeOf(host, assetId).fill).toBe(0x00ff00);
|
||
host.state.undo();
|
||
expect(shapeOf(host, assetId).fill).toBe(before);
|
||
});
|
||
|
||
it('writes the opacity, clamped to what an opacity can be', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
// A NumberField reads text: its own `onChange` is the one a typed value goes through.
|
||
const opacity = numberFields(host)[0]!;
|
||
opacity.onChange!('0.4');
|
||
expect(shapeOf(host, assetId).opacity).toBeCloseTo(0.4);
|
||
opacity.onChange!('5');
|
||
expect(shapeOf(host, assetId).opacity).toBe(1);
|
||
});
|
||
|
||
it('the outline switch adds a line and takes it away again', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
widgets(host, Checkbox)[0]!.toggle();
|
||
expect(shapeOf(host, assetId).stroke).toEqual({ color: 0x000000, width: 2 });
|
||
host.state.undo();
|
||
expect(shapeOf(host, assetId).stroke).toBeUndefined();
|
||
});
|
||
|
||
it('shows the line rows once there is a line, and a width of nothing is not one', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
shapeOf(host, assetId).stroke = { color: 0x112233, width: 3 };
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
expect([...host.fields.keys()]).toEqual([
|
||
...FRAME_KEYS,
|
||
'name',
|
||
'fillType',
|
||
'fill',
|
||
'opacity',
|
||
'outline',
|
||
'lineType',
|
||
'strokeColor',
|
||
'strokeWidth',
|
||
]);
|
||
const width = numberFields(host)[1]!;
|
||
// The command refuses a line of no width, so the row never sends one.
|
||
width.onChange!('0');
|
||
expect(shapeOf(host, assetId).stroke?.width).toBe(0.1);
|
||
widgets(host, ColorField)[1]!.onChange!(0xabcdef);
|
||
expect(shapeOf(host, assetId).stroke).toEqual({ color: 0xabcdef, width: 0.1 });
|
||
});
|
||
|
||
it('names the shapes and the outline in its signature, so both rebuild the rows', () => {
|
||
const { host, assetId, ids } = withShapes(2);
|
||
const before = shapeSignature(host.state.project, assetId, ids[0]!, [ids[0]!]);
|
||
shapeOf(host, assetId).name = 'Hat';
|
||
const renamed = shapeSignature(host.state.project, assetId, ids[0]!, [ids[0]!]);
|
||
expect(renamed).not.toBe(before);
|
||
shapeOf(host, assetId).stroke = { color: 0, width: 1 };
|
||
expect(shapeSignature(host.state.project, assetId, ids[0]!, [ids[0]!])).not.toBe(renamed);
|
||
});
|
||
|
||
it('a shape that is gone asks for a rebuild instead of refreshing', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
expect(refreshShapeSection(host, assetId, ids[0]!)).toBe(true);
|
||
expect(refreshShapeSection(host, assetId, 'gone')).toBe(false);
|
||
expect(shapeSignature(host.state.project, assetId, 'gone', ['gone'])).toBe('missing');
|
||
});
|
||
|
||
it('refreshing pushes the model values back into the rows', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
const shape = shapeOf(host, assetId);
|
||
shape.fill = 0x123456;
|
||
shape.name = 'Cloud';
|
||
refreshShapeSection(host, assetId, ids[0]!);
|
||
// The name field first, then the hex text inside the fill row – a name is a field, not a label.
|
||
const fields = widgets(host, TextField).slice(FRAME_KEYS.length);
|
||
expect(fields[0]!.value).toBe('Cloud');
|
||
expect(fields[1]!.value).toBe(toHex(0x123456));
|
||
});
|
||
|
||
it('the asset itself shows the list and the way to add to it', () => {
|
||
const { host, assetId } = withShapes(2);
|
||
buildShapeListSection(host, findAssetOf(host.state.project, assetId, 'vectorart')!);
|
||
// The list carries its "SHAPES" header itself.
|
||
expect(currentShapeList()?.shownIds).toHaveLength(2);
|
||
// The way to add is the Pen, said in the note: there is no button for it (#293).
|
||
expect(texts(host)).toContain('The top row is drawn over the ones below it.');
|
||
expect(texts(host)).toContain('The Pen draws a new shape.');
|
||
});
|
||
|
||
it('an empty drawing says so instead of showing an empty list', () => {
|
||
const { host, assetId } = withShapes(0);
|
||
buildShapeListSection(host, findAssetOf(host.state.project, assetId, 'vectorart')!);
|
||
expect(texts(host).some((t) => t.startsWith('Nothing drawn yet'))).toBe(true);
|
||
});
|
||
});
|
||
|
||
describe('combining from the inspector (#187)', () => {
|
||
const button = (host: InspectorHost, text: string) => widgets(host, Button).find((b) => b.text === text)!;
|
||
|
||
it('Ctrl+click adds a shape and takes it out again; emptied, the drawing stays selected', () => {
|
||
const { host, assetId, ids } = withShapes(2);
|
||
host.state.select(shapeSelection(assetId, [ids[0]!]));
|
||
host.state.toggleShape(assetId, ids[1]!);
|
||
expect(host.state.selection).toMatchObject({ shapeId: ids[1], shapeIds: [ids[0], ids[1]] });
|
||
host.state.toggleShape(assetId, ids[1]!);
|
||
host.state.toggleShape(assetId, ids[0]!);
|
||
expect(host.state.selection).toEqual({ kind: 'asset', assetId });
|
||
});
|
||
|
||
it('switches the four buttons off for one shape and says how to get a second', () => {
|
||
const { host, assetId, ids } = withShapes(2);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
expect(['Union', 'Subtract', 'Intersect', 'Exclude'].map((t) => button(host, t).eventMode)).toEqual([
|
||
'none',
|
||
'none',
|
||
'none',
|
||
'none',
|
||
]);
|
||
expect(texts(host)).toContain('Ctrl+click a second shape to combine.');
|
||
});
|
||
|
||
it('combines the selected shapes into the lowest one and selects it', () => {
|
||
const { host, assetId, ids } = withShapes(3);
|
||
host.state.select(shapeSelection(assetId, [ids[2]!, ids[1]!]));
|
||
buildShapeSection(host, assetId, ids[1]!, [ids[2]!, ids[1]!]);
|
||
expect(texts(host)).toContain('The result keeps "Shape 2", the lowest.');
|
||
button(host, 'Union').onClick!();
|
||
expect(allShapes(findAssetOf(host.state.project, assetId, 'vectorart')!).map((s) => s.id)).toEqual([
|
||
ids[0],
|
||
ids[1],
|
||
]);
|
||
expect(host.state.selection).toMatchObject({ shapeId: ids[1], shapeIds: [ids[1]] });
|
||
});
|
||
|
||
it('drops a selected shape that is gone and keeps the rest', () => {
|
||
const { host, assetId, ids } = withShapes(3);
|
||
host.state.select(shapeSelection(assetId, [ids[0]!, ids[2]!]));
|
||
host.state.execute(new RemoveVectorShapeCommand(assetId, ids[2]!));
|
||
expect(host.state.selection).toMatchObject({ shapeId: ids[0], shapeIds: [ids[0]] });
|
||
});
|
||
|
||
it('names the selected shapes in the signature, so a Ctrl+click rebuilds the rows', () => {
|
||
const { host, assetId, ids } = withShapes(2);
|
||
const project = host.state.project;
|
||
expect(shapeSignature(project, assetId, ids[0]!, [ids[0]!])).not.toBe(
|
||
shapeSignature(project, assetId, ids[0]!, [ids[1]!, ids[0]!]),
|
||
);
|
||
});
|
||
});
|
||
|
||
describe('fill and line types (#186)', () => {
|
||
const button = (host: InspectorHost, text: string) => widgets(host, Button).find((b) => b.text === text)!;
|
||
const rebuild = (host: InspectorHost, assetId: string, shapeId: string) => {
|
||
host.body.removeChildren();
|
||
host.fields.clear();
|
||
buildShapeSection(host, assetId, shapeId, [shapeId]);
|
||
};
|
||
|
||
it('switches the fill to a gradient in one undo step, and the colour row makes way for the stops', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
setPaintKind(host.state, assetId, ids[0]!, 'fill', 'linear');
|
||
expect(shapeOf(host, assetId).paint).toMatchObject({
|
||
type: 'linear',
|
||
stops: [{ offset: 0 }, { offset: 1 }],
|
||
});
|
||
const signature = shapeSignature(host.state.project, assetId, ids[0]!, [ids[0]!]);
|
||
expect(signature).toContain('linear/none');
|
||
rebuild(host, assetId, ids[0]!);
|
||
expect([...host.fields.keys()]).toEqual([
|
||
...FRAME_KEYS,
|
||
'name',
|
||
'fillType',
|
||
'fillStopColor',
|
||
'fillStopAlpha',
|
||
'fillStopOffset',
|
||
'opacity',
|
||
'outline',
|
||
]);
|
||
host.state.undo();
|
||
expect(shapeOf(host, assetId).paint).toBeUndefined();
|
||
});
|
||
|
||
it('edits the picked stop, and "Remove stop" only works while three are left', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
setPaintKind(host.state, assetId, ids[0]!, 'fill', 'radial');
|
||
rebuild(host, assetId, ids[0]!);
|
||
// The stop's colour is the only colour row left; opacity of the stop is the first number row.
|
||
widgets(host, ColorField)[0]!.onChange!(0xff0000);
|
||
numberFields(host)[0]!.onChange!('0.5');
|
||
const paint = shapeOf(host, assetId).paint as VectorGradient;
|
||
expect(paint.stops[0]).toEqual({ offset: 0, color: 0xff0000, alpha: 0.5 });
|
||
expect(button(host, 'Remove stop').eventMode).toBe('none');
|
||
|
||
host.state.execute(
|
||
new SetVectorShapeStyleCommand(assetId, ids[0]!, {
|
||
paint: { ...paint, stops: [...paint.stops, { offset: 0.5, color: 0, alpha: 1 }] },
|
||
}),
|
||
);
|
||
rebuild(host, assetId, ids[0]!);
|
||
expect(button(host, 'Remove stop').eventMode).not.toBe('none');
|
||
button(host, 'Remove stop').onClick!();
|
||
expect((shapeOf(host, assetId).paint as VectorGradient).stops.map((s) => s.offset)).toEqual([1, 0.5]);
|
||
});
|
||
|
||
it('gives the outline a type of its own, with the line colour as its first stop', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
shapeOf(host, assetId).stroke = { color: 0x112233, width: 3 };
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
setPaintKind(host.state, assetId, ids[0]!, 'stroke', 'linear');
|
||
expect(shapeOf(host, assetId).stroke).toMatchObject({
|
||
color: 0x112233,
|
||
width: 3,
|
||
paint: { type: 'linear' },
|
||
});
|
||
expect((shapeOf(host, assetId).stroke!.paint as VectorGradient).stops[0]?.color).toBe(0x112233);
|
||
setPaintKind(host.state, assetId, ids[0]!, 'stroke', 'solid');
|
||
expect(shapeOf(host, assetId).stroke).toEqual({ color: 0x112233, width: 3 });
|
||
});
|
||
|
||
it('a texture fill shows the picker, and picking writes the id into the paint', () => {
|
||
const { host, assetId, ids } = withShapes(1);
|
||
buildShapeSection(host, assetId, ids[0]!, [ids[0]!]);
|
||
setPaintKind(host.state, assetId, ids[0]!, 'fill', 'texture');
|
||
rebuild(host, assetId, ids[0]!);
|
||
expect(host.fields.has('fillTexture')).toBe(true);
|
||
setFillTexture(host.state, assetId, ids[0]!, 't1');
|
||
expect(shapeOf(host, assetId).paint).toMatchObject({ type: 'texture', texture: 't1' });
|
||
});
|
||
});
|