From 4c7861b5b8c454d85ffb74a2a302b7d96501cbca Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Wed, 7 Oct 2026 16:54:59 +0200 Subject: [PATCH] Keep the frame rate up while painting a big tilemap (#347) Painting on the 60x60 map of a project with fifty objects dropped the editor to 20 to 39 fps. A profile of a brush stroke showed four causes, none of them the brush itself. - Each frame of a stroke previews the model. The asset and hierarchy panels then built every row anew, so every label became a new text and a new upload to the GPU. One run spent 46 % of the main thread in those uploads. ListView.setItems now keeps a row whose item draws the same (sameListItem) and builds only the rows that changed. - The brush drew the whole scene on every pointer event, on top of the preview. Now the preview draws what the stroke changed, once a frame. The rectangle tool asks for one drawing a frame for its box. - An animated tile made the view draw in the tick and the preview drew it again in the same frame. animate now asks just before the frame is drawn and stays out when the view was drawn since the tick began. - layoutSymbols ran on every drawing and costs up to the cube of the symbols. SymbolLayoutMemo keeps the spots while no anchor moves. - A painted cell made the map read every "x,y" key again, and once more for each of the eight neighbours of every cell. layerCells reads a layer once per content into an index by place. layerTiles and the blocking cells take their neighbours from there (neighboursIn). parseCellKey reads a key without split, with the same results. The test hook sceneRedraws counts how often the scene view was drawn. Not done: a painted cell still works out the tiles and blocking cells of the whole map again, so that part grows with the map. Tests: - Brush stroke on a copy of the project (stein2, Chrome): 20 to 39 fps before, 47.5 after, 55 to 57 without painting. Labels drawn again during the stroke 2012 before, none after. Scene drawings per frame 1.32 before, 1 after. - One painted cell, headless: 28 to 16 ms on 60x60, 109 to 49 ms on the same map doubled to 120x120. - frame-preview: 2 new, red with the old pointer redraw. - tilemap: 3 new (key reading, index, refresh), list-ping 1 new, symbol-layout 1 new. 5 single reversals turned each red. - e2e 39,40 (melektaus-atx): ALL PASSED, 107 checks (1 new, a fast stroke: 6 drawings in 6 frames, no label drawn again). With the row reuse and the pointer redraw reverted it fails: 24 drawings in 3 frames, and the panels draw their labels again. - Gate (melektaus-atx): CHECK EXIT 0, 260 files, 4038 tests. Co-Authored-By: Claude Opus 5.5 --- src/core/model/tilemap.ts | 81 +++++++++++++++++++++----- src/core/model/tilemapCollision.ts | 24 ++++---- src/editor/debug/sceneHooks.ts | 2 + src/editor/viewport/SceneViewport.ts | 7 ++- src/editor/viewport/tilemapPainting.ts | 10 +++- src/ui/widgets/ListView.ts | 41 +++++++++++-- src/viewport/WorldViewport.ts | 13 ++++- src/viewport/symbolLayout.ts | 24 ++++++++ tests/core/tilemap.test.ts | 61 +++++++++++++++++++ tests/editor/frame-preview.test.ts | 81 +++++++++++++++++++++++++- tests/ui/list-ping.test.ts | 26 ++++++++- tests/viewport/symbol-layout.test.ts | 32 +++++++++- tools/e2e/40-tilemap.mjs | 39 +++++++++++++ 13 files changed, 397 insertions(+), 44 deletions(-) diff --git a/src/core/model/tilemap.ts b/src/core/model/tilemap.ts index a09b0371..326fb6f3 100644 --- a/src/core/model/tilemap.ts +++ b/src/core/model/tilemap.ts @@ -60,9 +60,14 @@ export function cellKey(x: number, y: number): string { return `${x},${y}`; } +/** Read without `split`: a map reads every key of a layer again after each painted cell (#347). */ export function parseCellKey(key: string): Cell | null { - const [x, y] = key.split(',').map(Number); - return Number.isInteger(x) && Number.isInteger(y) ? { x: x as number, y: y as number } : null; + const comma = key.indexOf(','); + if (comma < 0) return null; + const end = key.indexOf(',', comma + 1); + const x = Number(key.slice(0, comma)); + const y = Number(end < 0 ? key.slice(comma + 1) : key.slice(comma + 1, end)); + return Number.isInteger(x) && Number.isInteger(y) ? { x, y } : null; } export function cellAt(layer: TilemapLayer, x: number, y: number): string { @@ -127,6 +132,61 @@ export function neighboursOf( }; } +/** + * A layer's painted cells inside the grid, read once per content and grid size: in reading order, + * and by place. Reading the `x,y` keys again for every cell and each of its neighbours was most of + * what one painted cell cost a big map (#347). + */ +export interface LayerCells { + columns: number; + rows: number; + list: readonly { x: number; y: number; tileId: string }[]; + /** Tile id by `y * columns + x`. */ + at: ReadonlyMap; +} + +const indexed = new WeakMap, LayerCells & { version: number }>(); + +export function layerCells(props: PropertyBag, layer: TilemapLayer): LayerCells { + const columns = tilemapColumns(props); + const rows = tilemapRows(props); + const version = cellsVersion(layer.cells); + const held = indexed.get(layer.cells); + if (held && held.version === version && held.columns === columns && held.rows === rows) return held; + const at = new Map(); + for (const [key, tileId] of Object.entries(layer.cells)) { + const cell = parseCellKey(key); + if (!tileId || !cell || cell.x < 0 || cell.y < 0 || cell.x >= columns || cell.y >= rows) continue; + at.set(cell.y * columns + cell.x, tileId); + } + const list = [...at.keys()] + .sort((a, b) => a - b) + .map((i) => ({ x: i % columns, y: Math.floor(i / columns), tileId: at.get(i)! })); + const fresh = { columns, rows, list, at, version }; + indexed.set(layer.cells, fresh); + return fresh; +} + +/** `neighboursOf`, read from the index. */ +export function neighboursIn(cells: LayerCells, x: number, y: number, tileId: string): Neighbours { + const { columns, rows, at } = cells; + const same = (dx: number, dy: number): boolean => { + const nx = x + dx; + const ny = y + dy; + return nx >= 0 && ny >= 0 && nx < columns && ny < rows && at.get(ny * columns + nx) === tileId; + }; + return { + n: same(0, -1), + e: same(1, 0), + s: same(0, 1), + w: same(-1, 0), + ne: same(1, -1), + se: same(1, 1), + sw: same(-1, 1), + nw: same(-1, -1), + }; +} + /** One cell as it is drawn: where it sits, which tile it is and which quarters build it. */ export interface TileDraw { x: number; @@ -162,21 +222,14 @@ function workOutLayerTiles( layer: TilemapLayer, kinds: Map, ): TileDraw[] { - const kindOf = (id: string): TileKind | null => kinds.get(id) ?? null; + const cells = layerCells(props, layer); const tiles: TileDraw[] = []; - for (const [key, tileId] of Object.entries(layer.cells)) { - const at = parseCellKey(key); - if (!at || !tileId || !inGrid(props, at.x, at.y)) continue; - const kind = kindOf(tileId); + for (const { x, y, tileId } of cells.list) { + const kind = kinds.get(tileId); if (!kind) continue; - tiles.push({ - x: at.x, - y: at.y, - tileId, - quadrants: tileQuadrants(kind, neighboursOf(props, layer, at.x, at.y, tileId)), - }); + tiles.push({ x, y, tileId, quadrants: tileQuadrants(kind, neighboursIn(cells, x, y, tileId)) }); } - return tiles.sort((a, b) => a.y - b.y || a.x - b.x); + return tiles; } /** diff --git a/src/core/model/tilemapCollision.ts b/src/core/model/tilemapCollision.ts index c81089bf..c2398130 100644 --- a/src/core/model/tilemapCollision.ts +++ b/src/core/model/tilemapCollision.ts @@ -45,9 +45,8 @@ import { } from './tile'; import { cellOrigin, - inGrid, - neighboursOf, - parseCellKey, + layerCells, + neighboursIn, tilemapColumns, tilemapRows, tilemapSize, @@ -334,18 +333,15 @@ function workOutBlockingCells( asset: TilemapAsset, tiles: Map, ): CellBlock[] { - const tileOf = (id: string) => tiles.get(id) ?? null; - const found = new Map(); + const found = new Map(); for (const layer of asset.layers) { - for (const [key, tileId] of Object.entries(layer.cells)) { - const tile = tileId ? tileOf(tileId) : null; + const cells = layerCells(props, layer); + for (const { x, y, tileId } of cells.list) { + const tile = tiles.get(tileId); if (!tile) continue; - let cell = found.get(key); - if (!cell) { - const at = parseCellKey(key); - if (!at || !inGrid(props, at.x, at.y)) continue; - found.set(key, (cell = { ...at, box: null, polygons: [] })); - } + const index = y * cells.columns + x; + let cell = found.get(index); + if (!cell) found.set(index, (cell = { x, y, box: null, polygons: [] })); const { shape, scale } = tile; if (!shape) { cell.box = Math.max(cell.box ?? 0, scale); @@ -355,7 +351,7 @@ function workOutBlockingCells( for (const outline of shape.outlines) cell.polygons.push(placed(outline, cell.x + at, cell.y + at, scale)); } else { - const around = neighboursOf(props, layer, cell.x, cell.y, tileId); + const around = neighboursIn(cells, cell.x, cell.y, tileId); const quarters = tileQuadrants(shape.tileKind, around).map( (q) => shape.quarters.get(`${q.x},${q.y}`) ?? [], ); diff --git a/src/editor/debug/sceneHooks.ts b/src/editor/debug/sceneHooks.ts index 3c16043a..28b1499b 100644 --- a/src/editor/debug/sceneHooks.ts +++ b/src/editor/debug/sceneHooks.ts @@ -23,6 +23,8 @@ export function sceneHooks({ state, editor }: HookContext) { viewportCenter: () => centreOf(viewport), /** Pans the Scene tab to a world point – a step's own lane, away from what earlier steps left. */ lookAt: (x: number, y: number) => viewport.lookAt(x, y), + /** How often the Scene tab was drawn so far – a gesture should cost one per frame (#347). */ + sceneRedraws: () => viewport.redrawCount, /** Its drawn box – a step that wants an empty spot must stay inside what the console left over. */ viewportRect: () => { const p = viewport.getGlobalPosition(); diff --git a/src/editor/viewport/SceneViewport.ts b/src/editor/viewport/SceneViewport.ts index d3e2ab38..f030032b 100644 --- a/src/editor/viewport/SceneViewport.ts +++ b/src/editor/viewport/SceneViewport.ts @@ -53,7 +53,7 @@ import { dashedLine } from '../../viewport/symbols'; import { drawSymbolIcon, SYMBOL_GRID, SYMBOL_OUTLINE, type SymbolIcon } from '../../viewport/symbolIcons'; import { type JointLink, jointLinkOf } from '../../viewport/jointLinks'; import { type LabelBox, stackLabels, type WantedLabel } from '../../viewport/labelRows'; -import { layoutSymbols, type SymbolSpot } from '../../viewport/symbolLayout'; +import { SymbolLayoutMemo, type SymbolSpot } from '../../viewport/symbolLayout'; import { type ActiveCamera, cameraOfItems } from '../../viewport/activeCamera'; import { freeViewOfCamera, screenToWorld, type View, worldToScreen } from '../../viewport/view'; import { type CastRay, RAY_SHOWN, type World } from '../../runtime/World'; @@ -135,7 +135,8 @@ export class SceneViewport extends FreeViewport { private readonly plates = new Graphics(); private readonly iconLayer = new Container(); /** Where each icon stood at the last redraw, in world units (see `layoutSymbols`). */ - private spots = new Map(); + private spots: ReadonlyMap = new Map(); + private readonly symbolLayout = new SymbolLayoutMemo(); /** The plates of the names shown at the last redraw, in world units (test hook). */ private lastLabelBoxes: (WantedLabel & { left: boolean })[] = []; private readonly solids = new TilemapSolidOutlines(this.overlay); @@ -919,7 +920,7 @@ export class SceneViewport extends FreeViewport { const anchors = items .filter((item) => item.enabled && hasSceneIcon(item.type) && this.shows(item, 'symbol')) .map((item) => ({ id: item.id, x: item.x, y: item.y })); - this.spots = layoutSymbols(anchors, SYMBOL_SPACING * this.symbolScale()); + this.spots = this.symbolLayout.layout(anchors, SYMBOL_SPACING * this.symbolScale()); for (const [id, spot] of this.spots) this.icons.get(id)?.position.set(spot.x, spot.y); this.placeLabels(items, scale); } diff --git a/src/editor/viewport/tilemapPainting.ts b/src/editor/viewport/tilemapPainting.ts index 2836b2ac..12174fae 100644 --- a/src/editor/viewport/tilemapPainting.ts +++ b/src/editor/viewport/tilemapPainting.ts @@ -21,6 +21,7 @@ import { tilemapTileSize, } from '../../core/model/tilemap'; import { Theme } from '../../ui'; +import { cancelFrameWork, onFrame } from '../../ui/render'; import type { DrawItem } from '../../viewport/items'; import { type View, worldToScreen } from '../../viewport/view'; import { FramePreview } from '../framePreview'; @@ -54,10 +55,13 @@ interface PaintStroke { to: Vec2; } +let painters = 0; + /** The tilemap tools of the scene view: brush, eraser, rectangle, fill and pick. */ export class TilemapPainter { private stroke: PaintStroke | null = null; private readonly perFrame: FramePreview; + private readonly redrawKey = `tile-rect:${(painters += 1)}`; constructor(private readonly host: SceneViewHost) { this.perFrame = new FramePreview(host.state, 'tile-stroke'); @@ -110,9 +114,10 @@ export class TilemapPainter { const from = stroke.last ?? stroke.from; stroke.last = at; stroke.to = at; - // A pointer jumps between events, so the cells in between are filled in as well. + // A pointer jumps between events, so the cells in between are filled in as well. What they + // change is drawn by the preview, once a frame rather than once a pointer event (#347). if (stroke.tool !== 'rect') this.paintCells(item, cellsAlong(from, at)); - this.host.redraw(); + else onFrame(this.redrawKey, () => this.host.redraw()); return true; } @@ -216,6 +221,7 @@ export class TilemapPainter { const stroke = this.stroke; this.stroke = null; this.perFrame.cancel(); + cancelFrameWork(this.redrawKey); if (!stroke) return; const layer = this.strokeLayer(stroke); if (layer) { diff --git a/src/ui/widgets/ListView.ts b/src/ui/widgets/ListView.ts index 53ed2a47..ec3cbc96 100644 --- a/src/ui/widgets/ListView.ts +++ b/src/ui/widgets/ListView.ts @@ -50,6 +50,25 @@ export function pingAlpha(elapsed: number): number { return PING_ALPHA * (1 - (elapsed - hold) / (PING_MS - hold)); } +/** Whether a row drawn for `a` shows `b` just the same. */ +export function sameListItem(a: ListItem, b: ListItem): boolean { + const marks = a.marks ?? []; + const others = b.marks ?? []; + return ( + a.id === b.id && + a.label === b.label && + a.hint === b.hint && + (a.depth ?? 0) === (b.depth ?? 0) && + a.expanded === b.expanded && + Boolean(a.overridden) === Boolean(b.overridden) && + a.icon === b.icon && + Boolean(a.iconAtEnd) === Boolean(b.iconAtEnd) && + Boolean(a.heading) === Boolean(b.heading) && + marks.length === others.length && + marks.every((mark, i) => mark === others[i]) + ); +} + let instances = 0; /** Modifier keys held while a row was clicked. */ @@ -133,19 +152,29 @@ export class ListView extends Widget { } /** - * Rebuilds all rows. The selection survives when its id is still present and is dropped - * silently otherwise; highlights are kept as they are (the owner refreshes them). + * Rebuilds the rows that changed and keeps the others: a new row draws its texts again, and the + * panels set their items on every model event – each frame of a drag (#347). The selection + * survives when its id is still present and is dropped silently otherwise; highlights are kept as + * they are (the owner refreshes them). */ setItems(items: ListItem[]): void { this.items = items; - for (const row of this.rows) row.destroy({ children: true }); + const held = new Map(this.rows.map((row) => [row.item.id, row])); this.rows = items.map((item, index) => { - const row = new ListRow(item, this.rowHeight, Boolean(this.onToggle)); - row.on('pointerdown', (e: FederatedPointerEvent) => this.onRowPress(row, e)); + const kept = held.get(item.id); + let row: ListRow; + if (kept && sameListItem(kept.item, item)) { + held.delete(item.id); + row = kept; + } else { + row = new ListRow(item, this.rowHeight, Boolean(this.onToggle)); + row.on('pointerdown', (e: FederatedPointerEvent) => this.onRowPress(row, e)); + this.addChild(row); + } row.position.set(0, index * this.rowHeight); - this.addChild(row); return row; }); + for (const row of held.values()) row.destroy({ children: true }); this.addChild(this.pingMarker, this.dropMarker); // keep the markers above the rows if (this.selectedId && !items.some((i) => i.id === this.selectedId)) this.selectedId = null; if (this.contextId && !items.some((i) => i.id === this.contextId)) this.contextId = null; diff --git a/src/viewport/WorldViewport.ts b/src/viewport/WorldViewport.ts index b5eb31be..95eb8cd4 100644 --- a/src/viewport/WorldViewport.ts +++ b/src/viewport/WorldViewport.ts @@ -6,7 +6,7 @@ import { footprintExtent, footprintOf, placementOf } from '../core/properties/bo import { cameraFrame, rectsIntersect, type WorldRect } from '../core/properties/camera'; import type { ClipRegion } from '../core/properties/clip'; import type { Project } from '../core/model/Project'; -import { type Size, ui, Widget } from '../ui'; +import { cancelFrameWork, onFrame, type Size, ui, Widget } from '../ui'; import { fontLibrary } from './FontLibrary'; import { drawSlots } from './drawOrder'; import type { DrawItem } from './items'; @@ -30,6 +30,8 @@ import { cameraSees } from '../core/model/renderLayers'; /** How strongly the cut-off part of an object shows while the scene view opens its clip area (#260). */ const CLIP_GHOST_ALPHA = 0.3; +let views = 0; + /** * What a view draws from: the project and word of when it changed. The editor's state is one; a * released game (#266), which has no editor, hands over its own. @@ -93,6 +95,7 @@ export abstract class WorldViewport extends W } | null = null; /** Full redraws and camera-only moves since start (test hook). */ private redraws = 0; + private readonly animateKey = `animate:${(views += 1)}`; private cameraMoves = 0; private readonly stopFiles: (() => void)[]; /** One view per overlay camera (#302), by camera id, laid over this one in camera order. */ @@ -124,6 +127,7 @@ export abstract class WorldViewport extends W /** The libraries outlive a signed-out editor; a stale listener would redraw destroyed graphics. */ override destroy(options?: Parameters[0]): void { for (const stop of this.stopFiles) stop(); + cancelFrameWork(this.animateKey); for (const overlay of this.overlays.values()) overlay.destroy(); this.unshadeCamera(); this.offstage.destroy(); @@ -297,9 +301,14 @@ export abstract class WorldViewport extends W * Called every tick: redraws while the last redraw found a Time shader on screen. A view drawn * since the tick began, at `drawnBefore`, already shows this frame – a run draws its views * itself, and drawing them again cost a second collect of the whole world each frame (#284). + * Asked just before the frame is drawn rather than now: a gesture's preview redraws there too, + * and painting a tilemap drew the scene twice a frame (#347). */ animate(drawnBefore: number): void { - if (this.visible && this.animated && this.redraws === drawnBefore) this.redraw(); + if (!this.visible || !this.animated) return; + onFrame(this.animateKey, () => { + if (!this.destroyed && this.redraws === drawnBefore) this.redraw(); + }); } layerOf(objectId: string): 'objects' | 'overlay' | null { diff --git a/src/viewport/symbolLayout.ts b/src/viewport/symbolLayout.ts index 99402a49..a712739d 100644 --- a/src/viewport/symbolLayout.ts +++ b/src/viewport/symbolLayout.ts @@ -80,3 +80,27 @@ export function layoutSymbols(anchors: readonly SymbolAnchor[], spacing: number) } return spots; } + +/** + * `layoutSymbols` for a view that asks on every redraw: while the anchors stay where they were, the + * last answer stands. Merging rings costs up to the cube of the symbols, and a scene with fifty of + * them spent a tenth of every frame on it (#347). + */ +export class SymbolLayoutMemo { + private anchors: readonly SymbolAnchor[] = []; + private spacing = Number.NaN; + private spots = new Map(); + + layout(anchors: readonly SymbolAnchor[], spacing: number): ReadonlyMap { + if (spacing !== this.spacing || !sameAnchors(this.anchors, anchors)) { + this.spots = layoutSymbols(anchors, spacing); + this.anchors = anchors; + this.spacing = spacing; + } + return this.spots; + } +} + +function sameAnchors(a: readonly SymbolAnchor[], b: readonly SymbolAnchor[]): boolean { + return a.length === b.length && a.every((p, i) => p.id === b[i]!.id && p.x === b[i]!.x && p.y === b[i]!.y); +} diff --git a/tests/core/tilemap.test.ts b/tests/core/tilemap.test.ts index 4fbc2944..a06b545f 100644 --- a/tests/core/tilemap.test.ts +++ b/tests/core/tilemap.test.ts @@ -30,8 +30,11 @@ import { cellsInRect, floodFill, inGrid, + layerCells, layerTiles, + neighboursIn, neighboursOf, + parseCellKey, TILEMAP_COLUMNS_KEY, TILEMAP_ROWS_KEY, TILEMAP_TILE_KEY, @@ -39,6 +42,7 @@ import { } from '../../src/core/model/tilemap'; import { DIMMED_LAYER_ALPHA, layerAlpha } from '../../src/viewport/tilemap'; import { cellsPainted } from '../../src/core/model/usages'; +import { cellsChanged } from '../../src/core/model/tilemapMemo'; import { groupByTile, tileMesh } from '../../src/viewport/tilemapMesh'; import { makeProject, roundTrip } from '../helpers'; @@ -332,6 +336,63 @@ describe('what a painted cell shows', () => { }); }); +describe('the cells of a layer, read once per content (#347)', () => { + it('reads a key the way it always was read', () => { + const before = (key: string) => { + const [x, y] = key.split(',').map(Number); + return Number.isInteger(x) && Number.isInteger(y) ? { x, y } : null; + }; + for (const key of ['3,4', '-1,12', '0,0', '1,2,3', ',5', '5,', '1.5,2', 'a,1', '1', '', ' 2,3']) + expect(parseCellKey(key)).toEqual(before(key)); + }); + + it('holds the cells inside the grid in reading order, and knows the same neighbours', () => { + const f = makeTilemap(); + const sand = new CreateAssetCommand('tile', 'Sand'); + f.stack.execute(sand); + f.stack.execute(new SetAssetPropertyCommand(f.tilemapId, TILEMAP_COLUMNS_KEY, 5)); + f.stack.execute(new SetAssetPropertyCommand(f.tilemapId, TILEMAP_ROWS_KEY, 4)); + f.paint([ + { x: 4, y: 3 }, + { x: 0, y: 0 }, + { x: 1, y: 0 }, + { x: 1, y: 1 }, + { x: 2, y: 2 }, + { x: 3, y: 3 }, + ]); + f.paint([{ x: 2, y: 1 }], sand.asset.id); + const asset = f.assetOf(); + const layer = asset.layers[0]!; + // Outside the grid: kept in the file, but neither drawn nor anyone's neighbour. + layer.cells['5,0'] = f.tileId; + layer.cells['-1,1'] = f.tileId; + cellsChanged(layer.cells); + const cells = layerCells(asset.props, layer); + expect(cells.list.map((c) => `${c.x},${c.y}`)).toEqual(['0,0', '1,0', '1,1', '2,1', '2,2', '3,3', '4,3']); + expect(cells.list.find((c) => c.x === 2 && c.y === 1)?.tileId).toBe(sand.asset.id); + for (let y = -1; y <= 4; y++) + for (let x = -1; x <= 5; x++) + for (const tileId of [f.tileId, sand.asset.id]) + expect(neighboursIn(cells, x, y, tileId)).toEqual(neighboursOf(asset.props, layer, x, y, tileId)); + }); + + it('reads the cells again once they are said to have changed, or the grid is resized', () => { + const f = makeTilemap(); + f.paint([{ x: 1, y: 1 }]); + const asset = f.assetOf(); + const layer = asset.layers[0]!; + const first = layerCells(asset.props, layer); + expect(layerCells(asset.props, layer)).toBe(first); + layer.cells['2,1'] = f.tileId; + cellsChanged(layer.cells); + const painted = layerCells(asset.props, layer); + expect(painted.list).toHaveLength(2); + expect(neighboursIn(painted, 1, 1, f.tileId).e).toBe(true); + const narrow = layerCells({ ...asset.props, [TILEMAP_COLUMNS_KEY]: 2 }, layer); + expect(narrow.list.map((c) => `${c.x},${c.y}`)).toEqual(['1,1']); + }); +}); + describe('the layers of a tilemap', () => { it('adds, renames, reorders, hides and removes, each with an undo', () => { const f = makeTilemap(); diff --git a/tests/editor/frame-preview.test.ts b/tests/editor/frame-preview.test.ts index 3ddc07ec..d42b00a6 100644 --- a/tests/editor/frame-preview.test.ts +++ b/tests/editor/frame-preview.test.ts @@ -11,6 +11,8 @@ import type { GraphAsset, PaletteAsset, PixelArtAsset, + SceneAsset, + TilemapAsset, VectorArtAsset, VectorGradient, } from '../../src/core/model/Asset'; @@ -26,7 +28,14 @@ import { buildShapeSection, currentGradientBar } from '../../src/editor/inspecto 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 { EditorState, objectSelection } from '../../src/editor/state'; +import { chooseTool, pickTile } from '../../src/editor/panels/TilemapPanel'; +import type { SceneViewHost } from '../../src/editor/viewport/sceneViewHost'; +import { TilemapPainter } from '../../src/editor/viewport/tilemapPainting'; +import { PlaceAssetCommand } from '../../src/core/commands/sceneObjectCommands'; +import { IDENTITY } from '../../src/core/model/matrix'; +import { cellOrigin, tilemapTileSize } from '../../src/core/model/tilemap'; +import type { DrawItem } from '../../src/viewport/items'; import { VStack } from '../../src/ui'; import { initRenderLoop, resetRenderLoop } from '../../src/ui/render'; import { PaintCache } from '../../src/viewport/vectorArtPicture'; @@ -341,6 +350,76 @@ describe('the side panel of the pixel art tab (#274)', () => { }); }); +describe('painting a placed tilemap (#347)', () => { + beforeEach(() => resetRenderLoop()); + + function rig() { + const state = new EditorState(); + state.loadProject(createEmptyProject('p', 'P'), 1); + const scene = buildAsset('scene', 'Scene') as SceneAsset; + const map = buildAsset('tilemap', 'Ground') as TilemapAsset; + const tile = buildAsset('tile', 'Grass'); + state.project.assets.push(scene, map, tile); + state.project.activeSceneId = scene.id; + const placed = new PlaceAssetCommand(scene.id, map.id, { x: 0, y: 0 }); + state.execute(placed); + state.select(objectSelection(scene.id, [placed.objectId])); + pickTile(state, tile.id); + const item = { id: placed.objectId, type: 'tilemap', m: IDENTITY, props: map.props } as DrawItem; + const redraw = vi.fn(); + const host: SceneViewHost = { + state, + holder: () => scene, + items: () => [item], + worldAt: (global) => global, + local: (global) => global, + view: () => ({ x: 0, y: 0, scale: 1, rect: { x: 0, y: 0, w: 0, h: 0 } }), + scale: () => 1, + redraw, + }; + const painter = new TilemapPainter(host); + const size = tilemapTileSize(map.props); + const centre = (x: number) => { + const at = cellOrigin(map.props, x, 0); + return { x: at.x + size / 2, y: at.y + size / 2 }; + }; + const cells = () => Object.keys(map.layers[0]!.cells).sort(); + const press = () => painter.press(painter.targetOf(placed.objectId)!, centre(0)); + return { state, painter, redraw, centre, cells, press, preview: vi.spyOn(state, 'preview') }; + } + + it('writes every cell the brush crosses at once and draws the scene once a frame', () => { + const tick = frames(); + const { painter, redraw, centre, cells, press, preview } = rig(); + expect(press()).toBe(true); + redraw.mockClear(); + for (let x = 1; x <= 4; x++) painter.move(centre(x)); + expect(cells()).toEqual(['0,0', '1,0', '2,0', '3,0', '4,0']); + expect(redraw).not.toHaveBeenCalled(); + tick(); + expect(preview).toHaveBeenCalledTimes(1); + expect(redraw).not.toHaveBeenCalled(); + }); + + it('draws the box a rectangle drag would fill once a frame, and nothing after the release', () => { + const tick = frames(); + const { state, painter, redraw, centre, cells, press } = rig(); + chooseTool(state, 'rect'); + press(); + redraw.mockClear(); + for (let x = 1; x <= 4; x++) painter.move(centre(x)); + expect(redraw).not.toHaveBeenCalled(); + tick(); + expect(redraw).toHaveBeenCalledTimes(1); + painter.move(centre(5)); + painter.release(); + redraw.mockClear(); + tick(); + expect(redraw).not.toHaveBeenCalled(); + expect(cells()).toHaveLength(6); + }); +}); + describe('the place fields of the curve tab (#274)', () => { beforeAll(() => { CanvasTextMetrics.measureText = (() => ({ width: 10, height: 10, lines: [], lineWidths: [] })) as never; diff --git a/tests/ui/list-ping.test.ts b/tests/ui/list-ping.test.ts index ef78a557..f63b3e30 100644 --- a/tests/ui/list-ping.test.ts +++ b/tests/ui/list-ping.test.ts @@ -4,7 +4,7 @@ */ import { describe, expect, it } from 'vitest'; import { EditorState } from '../../src/editor/state'; -import { PING_MS, pingAlpha } from '../../src/ui/widgets/ListView'; +import { type ListItem, PING_MS, pingAlpha, sameListItem } from '../../src/ui/widgets/ListView'; describe('pingAlpha', () => { it('holds full for the first fifth, then fades to nothing at PING_MS', () => { @@ -18,6 +18,30 @@ describe('pingAlpha', () => { }); }); +describe('sameListItem (#347)', () => { + it('keeps a row only while everything it draws is the same', () => { + const row: ListItem = { id: 'a', label: 'Crate', hint: '2', depth: 1, icon: '', marks: [''] }; + expect(sameListItem(row, { ...row, marks: [''] })).toBe(true); + expect( + sameListItem({ id: 'a', label: 'Crate' }, { id: 'a', label: 'Crate', depth: 0, overridden: false }), + ).toBe(true); + const changes: Partial[] = [ + { id: 'b' }, + { label: 'Box' }, + { hint: '3' }, + { depth: 2 }, + { expanded: true }, + { overridden: true }, + { icon: '' }, + { iconAtEnd: true }, + { heading: true }, + { marks: [] }, + { marks: [''] }, + ]; + for (const change of changes) expect(sameListItem(row, { ...row, ...change })).toBe(false); + }); +}); + describe('EditorState.pingAsset', () => { it('names the asset and tells the listeners, but leaves the selection alone', () => { const state = new EditorState(); diff --git a/tests/viewport/symbol-layout.test.ts b/tests/viewport/symbol-layout.test.ts index ab27a493..eb200c6b 100644 --- a/tests/viewport/symbol-layout.test.ts +++ b/tests/viewport/symbol-layout.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { layoutSymbols, ringRadius } from '../../src/viewport/symbolLayout'; +import { layoutSymbols, ringRadius, SymbolLayoutMemo } from '../../src/viewport/symbolLayout'; import { haloSvg } from '../../src/ui/svg'; const SPACING = 28; @@ -89,6 +89,36 @@ describe('layoutSymbols', () => { }); }); +describe('SymbolLayoutMemo (#347)', () => { + it('answers with the same spots while the anchors stay put, and works them out again once one moves', () => { + const memo = new SymbolLayoutMemo(); + const anchors = () => [ + { id: 'a', x: 0, y: 0 }, + { id: 'b', x: 0, y: 0 }, + ]; + const first = memo.layout(anchors(), SPACING); + expect(memo.layout(anchors(), SPACING)).toBe(first); + const moved = memo.layout( + [ + { id: 'a', x: 0, y: 0 }, + { id: 'b', x: 100, y: 0 }, + ], + SPACING, + ); + expect(moved).not.toBe(first); + expect(moved.get('b')).toEqual({ x: 100, y: 0, angle: null }); + const wider = memo.layout( + [ + { id: 'a', x: 0, y: 0 }, + { id: 'b', x: 100, y: 0 }, + ], + 2 * SPACING, + ); + expect(wider).not.toBe(moved); + expect([...memo.layout(anchors(), SPACING)]).toEqual([...layoutSymbols(anchors(), SPACING)]); + }); +}); + describe('haloSvg', () => { it('widens every shape by its outline in currentColor, keeping hollow shapes hollow', () => { const svg = [ diff --git a/tools/e2e/40-tilemap.mjs b/tools/e2e/40-tilemap.mjs index f8ea027f..df92eb9e 100644 --- a/tools/e2e/40-tilemap.mjs +++ b/tools/e2e/40-tilemap.mjs @@ -354,6 +354,45 @@ export async function tilemap(t) { JSON.stringify({ drawnBefore, drawnHeld, drawnReleased, left: await painted() }), ); + // 40c-iii. However fast the pointer crosses cells, a stroke draws the scene at most once a frame, + // and the panels keep their rows instead of drawing every label again (#347). + const along = []; + for (let x = 0; x < COLUMNS; x++) + along.push(await json(`__gm.tilemap.cellAt(${JSON.stringify(MAP_OBJECT)}, ${x}, 2)`)); + await evaluate( + 'window.__texts = []; window.__fillText = CanvasRenderingContext2D.prototype.fillText;' + + ' CanvasRenderingContext2D.prototype.fillText = function (text, ...rest) {' + + ' window.__texts.push(String(text)); return window.__fillText.call(this, text, ...rest); }', + ); + await t.mouse('mouseMoved', along[0].x, along[0].y); + await t.mouse('mousePressed', along[0].x, along[0].y); + await settle(); + const counts = () => json('({ redraws: __gm.sceneRedraws(), drawn: __gm.renderStats().drawn })'); + const countsBefore = await counts(); + await evaluate('window.__texts = []'); + for (let pass = 0; pass < 4; pass++) + for (let i = 0; i < COLUMNS; i++) { + const p = along[pass % 2 ? COLUMNS - 1 - i : i]; + await t.mouse('mouseMoved', p.x, p.y, 'left', { buttons: 1 }); + } + await settle(); + const countsAfter = await counts(); + // The fps readout is the one text that changes on its own. + const redrawnTexts = (await json('window.__texts')).filter((text) => !/fps$/.test(text)); + await evaluate('CanvasRenderingContext2D.prototype.fillText = window.__fillText'); + await t.mouse('mouseReleased', along[0].x, along[0].y); + await settle(); + const strokeCells = Object.keys(await painted()).length; + await undo(); + await settle(); + const redraws = countsAfter.redraws - countsBefore.redraws; + const frames = countsAfter.drawn - countsBefore.drawn; + check( + 'a fast stroke draws the scene at most once a frame and no list label again', + strokeCells === COLUMNS && redraws <= frames && redrawnTexts.length === 0, + JSON.stringify({ strokeCells, redraws, frames, redrawnTexts: redrawnTexts.slice(0, 12) }), + ); + // 40d. The rectangle tool fills the box a drag spans. await clickInPanel(t, '__gm.tilemap.toolAt("rect")'); await settle();