From d6171fd38638de8fda5c51dfec4fb2150194a296 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Thu, 1 Oct 2026 12:02:12 +0200 Subject: [PATCH] Preview pixel art and curve fields once per frame too (#274) The previous commit left two findings open: in the pixel art tab the palette colour and the opacity of a layer still told the editor about every input, and so did the place fields of the curve tab. They now follow the same rule as the inspector rows. - PixelArtSide writes colour and opacity into the model on every input and requests the preview through a FramePreview. Both commits cancel a waiting preview first. - CurveCanvas.movePicked does the same for the t and v fields of the picked point. Showing another curve cancels a waiting preview. - Also fixed: when the colour picker ended on the colour it started from, the model was put back but nobody was told, so the previewed colour stayed on screen. commitColor now says so, as the opacity already did. The gain was not measured for either tab, only the behaviour is tested. Tests: - Gate: 198 files, 2911 tests (2907 + 4), exit 0 - e2e 47,48,53: 117 checks passed - Four new unit tests in tests/editor/frame-preview.test.ts: colour, opacity, curve fields and the return to the starting colour - Seven counter-probes fail, one test each. They ran on stein2, not in the container. - No new e2e check, the sum stays at 1616. Neither the full suite nor the smokes ran for this state, and there was no counter-probe in the browser. Co-Authored-By: Claude Opus 5.5 --- src/editor/curve/CurveCanvas.ts | 10 ++- src/editor/pixelart/PixelArtSide.ts | 16 ++-- tests/editor/frame-preview.test.ts | 132 +++++++++++++++++++++++++++- 3 files changed, 150 insertions(+), 8 deletions(-) diff --git a/src/editor/curve/CurveCanvas.ts b/src/editor/curve/CurveCanvas.ts index 02175f98..f15afb8b 100644 --- a/src/editor/curve/CurveCanvas.ts +++ b/src/editor/curve/CurveCanvas.ts @@ -15,6 +15,7 @@ import { curveExtent, curveValue, sanitizedCurve } from '../../core/model/curve' import { findAssetOf } from '../../core/model/Project'; import type { Vec2 } from '../../core/model/Properties'; import { cancelFrameWork, onFrame, pressCount, Theme, textStyle } from '../../ui'; +import { FramePreview } from '../framePreview'; import { DRAG_THRESHOLD, drawPointGrip, POINT_SIZE } from '../pointGrips'; import { type PointKind, @@ -85,10 +86,12 @@ export class CurveCanvas extends SheetCanvas { private snap = false; /** The curve before the header's fields started moving the picked point – their undo step's other side. */ private typed: VectorPoint[] | null = null; + private readonly typedPreview: FramePreview; private readonly frameKey = `curve:${(instances += 1)}`; constructor(private readonly state: EditorState) { super(); + this.typedPreview = new FramePreview(state, 'curve-field'); this.content.addChild(this.guides, this.line, this.overlay, ...this.labels); for (const label of this.labels) label.eventMode = 'none'; state.on('project', () => this.redraw()); @@ -104,6 +107,7 @@ export class CurveCanvas extends SheetCanvas { this.assetId = assetId; this.needsFit = true; this.typed = null; + this.typedPreview.cancel(); this.pick(-1); this.redraw(); } @@ -185,7 +189,8 @@ export class CurveCanvas extends SheetCanvas { /** * Puts the picked point at `to` – the header's `t` and `v` fields. Until `commit` it only shows - * the change, so a scrub on a field is one undo step, as a drag on the canvas is. + * the change, so a scrub on a field is one undo step, as a drag on the canvas is. The curve is + * written on every input, the preview event waits for the frame (#274). */ movePicked(to: Vec2, commit: boolean): void { const asset = this.asset(); @@ -194,9 +199,10 @@ export class CurveCanvas extends SheetCanvas { const points = sanitizedCurve(withCurvePointSet(before, this.picked, to)); if (!commit) { asset.points = points; - this.state.preview(); + this.typedPreview.request(); return; } + this.typedPreview.cancel(); asset.points = before; this.typed = null; if (SetCurvePointsCommand.changes(asset, points)) diff --git a/src/editor/pixelart/PixelArtSide.ts b/src/editor/pixelart/PixelArtSide.ts index 655271b7..a65a1ca3 100644 --- a/src/editor/pixelart/PixelArtSide.ts +++ b/src/editor/pixelart/PixelArtSide.ts @@ -28,6 +28,7 @@ import { type Widget, } from '../../ui'; import { pixelCels } from '../../viewport/PixelCels'; +import { FramePreview } from '../framePreview'; import type { EditorState } from '../state'; import { PaletteGrid } from './PaletteGrid'; import { type PixelArtCanvas, PIXEL_TOOL_LABELS, PIXEL_TOOLS, type PixelTool } from './PixelArtCanvas'; @@ -85,12 +86,15 @@ export class PixelArtSide extends ScrollContainer { readonly tilesBox = new Checkbox(false); readonly preview: PixelPreview; readonly fpsField = new NumberField(8, { min: 1, max: 60, step: 1, decimals: 0 }); + /** Colour and opacity are written on every input, the preview event waits for the frame (#274). */ + private readonly perFrame: FramePreview; constructor( private readonly state: EditorState, private readonly canvas: PixelArtCanvas, ) { super(); + this.perFrame = new FramePreview(state, 'pixelart-side'); this.preview = new PixelPreview(state); const c = this.column; @@ -277,17 +281,18 @@ export class PixelArtSide extends ScrollContainer { if (!palette || index < 1 || index > palette.colors.length) return; this.colorBefore ??= palette.colors[index - 1]!; palette.colors[index - 1] = color; - this.state.preview(); + this.perFrame.request(); } private commitColor(color: number): void { + this.perFrame.cancel(); const palette = this.paletteAsset(); const index = this.canvas.color; if (!palette || index < 1 || index > palette.colors.length) return; - const before = this.colorBefore ?? palette.colors[index - 1]!; - palette.colors[index - 1] = before; - setPaletteColor(this.state, palette, index, color); + const previewed = this.colorBefore !== null; + palette.colors[index - 1] = this.colorBefore ?? palette.colors[index - 1]!; this.colorBefore = null; + if (!setPaletteColor(this.state, palette, index, color) && previewed) this.state.preview(); } /** The colour the picked swatch had before a preview changed it – what the commit puts back first. */ @@ -341,10 +346,11 @@ export class PixelArtSide extends ScrollContainer { if (!layer) return; if (!this.opacityBefore.has(layerId)) this.opacityBefore.set(layerId, layer.opacity); layer.opacity = opacity; - this.state.preview(); + this.perFrame.request(); } private commitOpacity(layerId: string, opacity: number): void { + this.perFrame.cancel(); const asset = this.canvas.asset(); const layer = asset?.layers.find((l) => l.id === layerId); const before = this.opacityBefore.get(layerId); diff --git a/tests/editor/frame-preview.test.ts b/tests/editor/frame-preview.test.ts index 794976df..9c9e3be3 100644 --- a/tests/editor/frame-preview.test.ts +++ b/tests/editor/frame-preview.test.ts @@ -6,14 +6,26 @@ import { type Application, CanvasTextMetrics } from 'pixi.js'; import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { buildAsset } from '../../src/core/commands/assetCommands'; import type { Command } from '../../src/core/commands/Command'; -import type { GraphAsset, VectorArtAsset, VectorGradient } from '../../src/core/model/Asset'; +import type { + CurveAsset, + GraphAsset, + PaletteAsset, + PixelArtAsset, + VectorArtAsset, + VectorGradient, +} from '../../src/core/model/Asset'; +import { PALETTE_KEY } from '../../src/core/model/pixelart'; import { createEmptyProject } from '../../src/core/model/Project'; +import { CurveCanvas } from '../../src/editor/curve/CurveCanvas'; import { FramePreview } from '../../src/editor/framePreview'; import { GraphDrags } from '../../src/editor/graph/graphDrags'; import type { GraphCanvasHost } from '../../src/editor/graph/graphHost'; import type { InspectorHost } from '../../src/editor/inspector/host'; import { previewingHandlers } from '../../src/editor/inspector/previewHandlers'; import { buildShapeSection, currentGradientBar } from '../../src/editor/inspector/shapeSection'; +import { PixelArtCanvas } from '../../src/editor/pixelart/PixelArtCanvas'; +import { PixelArtSide } from '../../src/editor/pixelart/PixelArtSide'; +import { PixelThumbs } from '../../src/editor/pixelart/PixelThumbs'; import { EditorState } from '../../src/editor/state'; import { VStack } from '../../src/ui'; import { initRenderLoop, resetRenderLoop } from '../../src/ui/render'; @@ -237,3 +249,121 @@ describe('dragging a colour stop of a gradient (#274)', () => { expect(offsets()[0]).toBe(0); }); }); + +describe('the side panel of the pixel art tab (#274)', () => { + beforeAll(() => { + CanvasTextMetrics.measureText = (() => ({ width: 10, height: 10, lines: [], lineWidths: [] })) as never; + // Canvas and thumbnails paint onto browser canvases, which Node has none of. + PixelArtCanvas.prototype.redraw = () => undefined; + PixelThumbs.prototype.paint = () => undefined; + }); + beforeEach(() => resetRenderLoop()); + + function rig() { + const state = new EditorState(); + state.loadProject(createEmptyProject('p', 'P'), 1); + const palette = buildAsset('palette', 'Palette') as PaletteAsset; + const art = buildAsset('pixelart', 'Art') as PixelArtAsset; + art.props[PALETTE_KEY] = palette.id; + state.project.assets.push(palette, art); + const canvas = new PixelArtCanvas(state); + const side = new PixelArtSide(state, canvas); + canvas.show(art.id); + canvas.setColor(1); + const layer = art.layers[0]!; + // The row's number field calls these two; the list keeps them to itself. + const opacity = side as unknown as { + previewOpacity(layerId: string, opacity: number): void; + commitOpacity(layerId: string, opacity: number): void; + }; + return { state, palette, art, layer, side, opacity, preview: vi.spyOn(state, 'preview') }; + } + + it('writes every colour under the picker into the palette and previews once a frame', () => { + const tick = frames(); + const { state, palette, side, preview } = rig(); + const first = palette.colors[0]!; + for (const color of [0x111111, 0x222222, 0x333333]) { + side.colorField.onInput?.(color); + expect(palette.colors[0]).toBe(color); + } + expect(preview).not.toHaveBeenCalled(); + tick(); + expect(preview).toHaveBeenCalledTimes(1); + side.colorField.onInput?.(0x444444); + side.colorField.onChange?.(0x555555); + tick(); + expect(preview).toHaveBeenCalledTimes(1); + expect(palette.colors[0]).toBe(0x555555); + state.undo(); + expect(palette.colors[0]).toBe(first); + }); + + it('says so when the picker ends on the colour it started from', () => { + const tick = frames(); + const { palette, side, preview } = rig(); + const first = palette.colors[0]!; + side.colorField.onInput?.(0x111111); + tick(); + side.colorField.onChange?.(first); + expect(palette.colors[0]).toBe(first); + expect(preview).toHaveBeenCalledTimes(2); + }); + + it('writes every step of an opacity scrub into the layer and previews once a frame', () => { + const tick = frames(); + const { state, art, layer, opacity, preview } = rig(); + for (const value of [0.9, 0.8, 0.7]) { + opacity.previewOpacity(layer.id, value); + expect(layer.opacity).toBe(value); + } + expect(preview).not.toHaveBeenCalled(); + tick(); + expect(preview).toHaveBeenCalledTimes(1); + opacity.previewOpacity(layer.id, 0.6); + opacity.commitOpacity(layer.id, 0.5); + tick(); + expect(preview).toHaveBeenCalledTimes(1); + expect(art.layers[0]!.opacity).toBe(0.5); + state.undo(); + expect(art.layers[0]!.opacity).toBe(1); + }); +}); + +describe('the place fields of the curve tab (#274)', () => { + beforeAll(() => { + CanvasTextMetrics.measureText = (() => ({ width: 10, height: 10, lines: [], lineWidths: [] })) as never; + }); + beforeEach(() => resetRenderLoop()); + + function rig() { + const state = new EditorState(); + state.loadProject(createEmptyProject('p', 'P'), 1); + const curve = buildAsset('curve', 'Ease') as CurveAsset; + state.project.assets.push(curve); + const canvas = new CurveCanvas(state); + canvas.show(curve.id); + (canvas as unknown as { picked: number }).picked = 1; + return { state, curve, canvas, preview: vi.spyOn(state, 'preview') }; + } + + it('moves the picked point with every input and previews once a frame', () => { + const tick = frames(); + const { state, curve, canvas, preview } = rig(); + const before = curve.points[1]!.y; + for (const v of [0.9, 0.8, 0.7]) { + canvas.movePicked({ x: 1, y: v }, false); + expect(canvas.pickedPlace?.y).toBeCloseTo(v); + } + expect(preview).not.toHaveBeenCalled(); + tick(); + expect(preview).toHaveBeenCalledTimes(1); + canvas.movePicked({ x: 1, y: 0.6 }, false); + canvas.movePicked({ x: 1, y: 0.5 }, true); + tick(); + expect(preview).toHaveBeenCalledTimes(1); + expect(curve.points[1]!.y).toBeCloseTo(0.5); + state.undo(); + expect(curve.points[1]!.y).toBe(before); + }); +});