From 28b1afa7af7980094c8c39fed72d5ffb69fa992d Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Tue, 22 Sep 2026 15:38:05 +0200 Subject: [PATCH] Panning the scene view no longer redraws the whole scene A middle-mouse pan used to run the full redraw on every frame. It collected every object from the project, cleared and redrew every shape, re-sorted all children and made Pixi re-upload every buffer, although only the camera had moved. The user saw the frame rate drop while panning. - WorldViewport gets moveCamera(). When zoom and view size are unchanged it only moves the world and redraws the overlays drawn in screen pixels. collectFrame, the shape redraw and the re-sort drop out of a pan frame. - Every full redraw remembers the box each object is culled by. A pan that moves an object across the edge of the view still takes the full redraw, so objects appear and vanish as before. - Grid, selection outlines, rays and the container origin mark move into a shared drawOnScreen hook. It runs after every full redraw and after every pan. The parent links stay in afterItems because they are drawn in world units. - Wheel zoom keeps the full redraw, because line widths follow the zoom. A cache for collectFrame was left out: it would save about 1 ms and risk stale data. - debugInfo() reports redraws and cameraMoves, and __gm gets a hook that forces a full redraw, so a check can compare against it. Findings: - Chrome already coalesces mouse moves to one per frame (500 moves gave 183 redraws in 189 frames). A throttle was tried and removed again, because its counter-check did not fail. - A check in module 03 compared the whole gameInfo() object and broke on the new counters. It now compares camera, objects and rendering. - The stall without ?vsync most likely comes from the desynchronized context in src/main.ts. It cannot be reproduced headless. Tests: - Measured on Jump and Run, main-thread time per frame, 3 runs of 240 moves: pan 5.7-7.4 ms before, 3.8-4.8 ms after. Plain mouse moves stay at 2.1-2.4 ms. 208 of 240 moves took the fast path. - ./check.sh: 141 files, 2080 tests, exit 0. - GM_STEPS=1-16,29,42,43: 535 checks, 1 failed (the module 03 test above). After the fix GM_STEPS=1-5: all passed, 130 checks. - 3 new e2e checks in 5d: 12 pan steps give 12 camera moves and 0 full redraws, pixel rows after the pan match a fresh redraw, a box panned out of view and back is drawn again. - Counter-checks: overlays not redrawn on pan gives "3 of 3 rows differ". Edge test disabled keeps the box visible with 0 redraws. - Check total 1285 (1282 confirmed by CI task 229, plus 3). Co-Authored-By: Claude Opus 5 --- README.md | 2 +- src/editor/container/ContainerCanvas.ts | 10 ++-- src/editor/debugHook.ts | 2 + src/editor/viewport/FreeViewport.ts | 2 +- src/editor/viewport/GameViewport.ts | 2 +- src/editor/viewport/SceneViewport.ts | 12 +++-- src/editor/viewport/WorldViewport.ts | 71 ++++++++++++++++++++++--- tools/e2e/03-play.mjs | 7 ++- tools/e2e/05-drawing.mjs | 53 +++++++++++++++++- 9 files changed, 136 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index ccde23a2..f92b2175 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,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, 1282 checks (GM_STEPS=20 for one step) +node tools/seed-e2e.mjs && node tools/e2e.mjs # the editor, 1285 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/editor/container/ContainerCanvas.ts b/src/editor/container/ContainerCanvas.ts index 5c700d29..e9894a86 100644 --- a/src/editor/container/ContainerCanvas.ts +++ b/src/editor/container/ContainerCanvas.ts @@ -42,19 +42,15 @@ export class ContainerCanvas extends SceneViewport { return false; } - protected override beforeItems(): void { - super.beforeItems(); - this.originMark.clear(); - } - /** * The origin, marked because it carries meaning here: it is the point the container is held by * when placed, so where the content sits relative to it is the thing being authored. */ - protected override afterItems(items: DrawItem[], view: View): void { - super.afterItems(items, view); + protected override drawOnScreen(items: DrawItem[], view: View): void { + super.drawOnScreen(items, view); const at = worldToScreen(view, 0, 0); this.originMark + .clear() .moveTo(at.x - ORIGIN_ARM, at.y) .lineTo(at.x + ORIGIN_ARM, at.y) .moveTo(at.x, at.y - ORIGIN_ARM) diff --git a/src/editor/debugHook.ts b/src/editor/debugHook.ts index 59475595..45d927f7 100644 --- a/src/editor/debugHook.ts +++ b/src/editor/debugHook.ts @@ -276,6 +276,8 @@ export function installDebugHook(app: Application, state: EditorState, editor: E return null; }, viewportInfo: () => viewport.debugInfo(), + /** A full redraw of the Scene tab – what a pan, which only moves the camera, is compared with. */ + viewportRedraw: () => viewport.redraw(), /** Where a dragged asset would land in the Scene tab, and why not when it may not. */ dropPreview: () => viewport.dropInfo(), /** The panel-to-view drag in flight, if any – a drop that never started looks like a refused one. */ diff --git a/src/editor/viewport/FreeViewport.ts b/src/editor/viewport/FreeViewport.ts index 082e0fad..88b49742 100644 --- a/src/editor/viewport/FreeViewport.ts +++ b/src/editor/viewport/FreeViewport.ts @@ -104,7 +104,7 @@ export abstract class FreeViewport extends WorldViewport { const { zoom } = this.view; this.view.x = this.panning.viewX - (e.global.x - this.panning.startX) / zoom; this.view.y = this.panning.viewY - (e.global.y - this.panning.startY) / zoom; - this.redraw(); + this.moveCamera(); return true; } diff --git a/src/editor/viewport/GameViewport.ts b/src/editor/viewport/GameViewport.ts index 8737c35b..bf33dc7c 100644 --- a/src/editor/viewport/GameViewport.ts +++ b/src/editor/viewport/GameViewport.ts @@ -84,7 +84,7 @@ export class GameViewport extends WorldViewport { } /** `camera`: object id of the camera the view is through, empty when none is switched on. */ - override debugInfo(): { rendering: boolean; visibleItems: number; camera: string } { + override debugInfo(): ReturnType & { camera: string } { return { ...super.debugInfo(), camera: this.activeCamera()?.id ?? '' }; } diff --git a/src/editor/viewport/SceneViewport.ts b/src/editor/viewport/SceneViewport.ts index 6e1d6a5b..afb28c2f 100644 --- a/src/editor/viewport/SceneViewport.ts +++ b/src/editor/viewport/SceneViewport.ts @@ -731,18 +731,20 @@ export class SceneViewport extends FreeViewport { this.state.select(objectSelection(holder.id, ids)); } - protected override beforeItems(): void { - this.syncGrid(); - this.gizmos.clear(); - } - /** The selection outlines currently drawn (test hook). */ gizmoInfo(): GizmoInfo[] { return this.lastGizmos; } + /** The links are in world units, so a pan leaves them where they are. */ protected override afterItems(items: DrawItem[], view: View): void { this.drawParentLinks(items, view.scale); + } + + /** Grid, outlines and rays are drawn in screen pixels: these follow every pan. */ + protected override drawOnScreen(items: DrawItem[], view: View): void { + this.syncGrid(); + this.gizmos.clear(); this.drawGizmos(items, view); this.drawRays(view); } diff --git a/src/editor/viewport/WorldViewport.ts b/src/editor/viewport/WorldViewport.ts index 2e0fb170..01bc1906 100644 --- a/src/editor/viewport/WorldViewport.ts +++ b/src/editor/viewport/WorldViewport.ts @@ -61,6 +61,20 @@ export abstract class WorldViewport extends Widget { private visibleCount = 0; private animated = false; protected lastItems: DrawItem[] = []; + /** + * What the last full redraw was drawn for: its zoom and view size, and per object node the box + * it is culled by (`null`: never culled, a particle source). What `moveCamera` needs to tell + * whether a pan changes anything but where the world sits. + */ + private drawn: { + scale: number; + w: number; + h: number; + culled: { node: Graphics; extent: Extent | null }[]; + } | null = null; + /** Full redraws and camera-only moves since start (test hook). */ + private redraws = 0; + private cameraMoves = 0; private readonly stopFiles: (() => void)[]; protected constructor(protected readonly state: EditorState) { @@ -123,8 +137,49 @@ export abstract class WorldViewport extends Widget { protected onItemPointerDown(_objectId: string, _e: FederatedPointerEvent): void {} - debugInfo(): { rendering: boolean; visibleItems: number } { - return { rendering: this.currentView() !== null, visibleItems: this.visibleCount }; + /** Screen-space drawing that follows the camera, redrawn after a full redraw and after a pan. */ + protected drawOnScreen(_items: DrawItem[], _view: View): void {} + + debugInfo(): { rendering: boolean; visibleItems: number; redraws: number; cameraMoves: number } { + return { + rendering: this.currentView() !== null, + visibleItems: this.visibleCount, + redraws: this.redraws, + cameraMoves: this.cameraMoves, + }; + } + + /** + * The camera moved and nothing else did – a pan. Zoom and size are the same, so every node is + * drawn as it was; only the world's position and the screen-space overlays follow. Anything + * else, or an object crossing the edge of the view, takes the full redraw: a pan frame used to + * collect, redraw and re-upload every shape for nothing. + */ + protected moveCamera(): void { + const view = this.currentView(); + const drawn = this.drawn; + if (!view || !drawn || view.scale !== drawn.scale || view.rect.w !== drawn.w || view.rect.h !== drawn.h) { + this.redraw(); + return; + } + const bounds = worldBounds(view); + for (const { node, extent } of drawn.culled) + if ((extent === null || rectsIntersect(extent, bounds)) !== node.visible) { + this.redraw(); + return; + } + this.cameraMoves++; + this.placeWorld(view); + this.drawOnScreen(this.lastItems, view); + } + + private placeWorld(view: View): void { + const { rect } = view; + this.world.position.set( + rect.x + rect.w / 2 - view.x * view.scale, + rect.y + rect.h / 2 - view.y * view.scale, + ); + this.world.scale.set(view.scale); } /** @@ -219,21 +274,20 @@ export abstract class WorldViewport extends Widget { this.content.visible = view !== null; this.visibleCount = 0; this.animated = false; + this.drawn = null; if (!view) return; + this.redraws++; const { items, background } = this.collectFrame(); this.lastItems = items; const { rect } = view; if (this.paintsBackground()) this.bg.rect(rect.x, rect.y, rect.w, rect.h).fill(background); this.contentMask.rect(rect.x, rect.y, rect.w, rect.h).fill(0xffffff); - this.world.position.set( - rect.x + rect.w / 2 - view.x * view.scale, - rect.y + rect.h / 2 - view.y * view.scale, - ); - this.world.scale.set(view.scale); + this.placeWorld(view); const bounds = worldBounds(view); const shaders = shaderLibrary.source(this.state.project); + const culled: { node: Graphics; extent: Extent | null }[] = []; const seen = new Set(); for (const item of items) { @@ -254,6 +308,7 @@ export abstract class WorldViewport extends Widget { (isSymbolType(item.type) ? this.overlay : this.objects).addChild(node); } node.visible = onScreen; + if (item.enabled) culled.push({ node, extent: item.type === 'particles' ? null : extent }); node.eventMode = this.itemsInteractive && onScreen ? 'static' : 'none'; this.syncDecorations(item, onScreen, view.scale, extent); this.syncText(item, onScreen, view.scale); @@ -265,6 +320,8 @@ export abstract class WorldViewport extends Widget { drawItem(node, item, size, view.scale * scaleOf(placementOf(item.type, item.m))); } this.afterItems(items, view); + this.drawOnScreen(items, view); + this.drawn = { scale: view.scale, w: rect.w, h: rect.h, culled }; for (const [id, node] of this.nodes) { if (seen.has(id)) continue; this.texts.get(id)?.destroy(); diff --git a/tools/e2e/03-play.mjs b/tools/e2e/03-play.mjs index 17761bd7..13ec1996 100644 --- a/tools/e2e/03-play.mjs +++ b/tools/e2e/03-play.mjs @@ -262,10 +262,13 @@ export async function play(t) { await play(); info = await gameInfo(); const playingPixel = await crateSpot(); + // What is drawn, not how often: the redraw counters differ between any two moments. + const drawnAs = ({ rendering, visibleItems, camera }) => + JSON.stringify({ rendering, visibleItems, camera }); check( 'and it is the picture Play produces: same camera, same items, same pixel on the crate', - JSON.stringify(info) === JSON.stringify(preview) && playingPixel === previewPixel, - `${JSON.stringify(preview)} vs ${JSON.stringify(info)}, ${previewPixel} vs ${playingPixel}`, + drawnAs(info) === drawnAs(preview) && playingPixel === previewPixel, + `${drawnAs(preview)} vs ${drawnAs(info)}, ${previewPixel} vs ${playingPixel}`, ); await stop(); diff --git a/tools/e2e/05-drawing.mjs b/tools/e2e/05-drawing.mjs index 113acf06..7eca23e0 100644 --- a/tools/e2e/05-drawing.mjs +++ b/tools/e2e/05-drawing.mjs @@ -2,7 +2,7 @@ import { LANES, MAKE_OBJECT } from './lib/scaffold.mjs'; /** Draw order: symbols always above sprites, `z` above hierarchy order. */ export async function drawing(t) { - const { check, json, shot, click, locate, typeText, press, undo, mutate, select, object } = t; + const { check, json, shot, click, drag, locate, typeText, press, undo, mutate, select, object } = t; const orderOfCrates = () => json(`__gm.drawOrder().filter(id => id === 'o1' || id === 'o2')`); // 5a. Symbols stay on top: put the camera first in the hierarchy (drawn first) and a crate @@ -103,4 +103,55 @@ export async function drawing(t) { scene.objects = scene.objects.filter((o) => !${JSON.stringify(ids)}.includes(o.id)); p.assets = p.assets.filter((a) => !['s-box-a', 's-rb-a', 's-pt-a'].includes(a.id)); __gm.lookAt(0, 0);`); + + // 5d. A pan only moves the camera: no collect and no redraw of every shape per move. What it + // leaves on screen – the crate, its outline, the grid – must be what a full redraw draws, and + // an object panned back into view must be drawn again. + await select({ kind: 'object', holderId: 'scene1', objectId: 'o1' }); + const vr = await json('__gm.viewportRect()'); + const cx = Math.round(vr.x + vr.w / 2); + const cy = Math.round(vr.y + vr.h / 2); + const info = () => json('__gm.viewportInfo()'); + const rows = () => + json(`(() => { + const r = __gm.objectRect('o1'); + const x = Math.round(r.x - r.w / 2 - 20); + return [r.y, r.y - r.h / 2 - 2, r.y + r.h / 2 + 2].map((y) => __gm.pixelRow(x, Math.round(y), Math.round(r.w + 40)).join()); + })()`); + const sameAsRedraw = async () => { + const panned = await rows(); + await json('(__gm.viewportRedraw(), true)'); + const fresh = await rows(); + return panned.filter((row, i) => row !== fresh[i]).length; + }; + const before = await info(); + await drag(cx, cy, cx + 36, cy + 24, { steps: 12, button: 'middle' }); + const after = await info(); + const moves = after.cameraMoves - before.cameraMoves; + const redraws = after.redraws - before.redraws; + check( + 'a pan moves the camera without redrawing the scene', + moves >= 8 && redraws <= 4, + `${moves} camera moves, ${redraws} redraws in 12 steps`, + ); + const offAfterPan = await sameAsRedraw(); + check('a panned view draws what a full redraw draws', offAfterPan === 0, `${offAfterPan} of 3 rows differ`); + // Far enough that the crate's right edge passes the view's left one. + const crate = await json(`__gm.objectRect('o1')`); + const away = Math.round(crate.x + crate.w / 2 - vr.x + 20); + await drag(cx, cy, cx - away, cy, { steps: 12, button: 'middle' }); + const gone = !(await json('__gm.drawOrder()')).includes('o1'); + const back0 = await info(); + // Pressed inside the view again: a press out over the panel would pan nothing. + await drag(cx, cy, cx + away, cy, { steps: 12, button: 'middle' }); + const back = await info(); + const shown = (await json('__gm.drawOrder()')).includes('o1'); + const offAfterReturn = await sameAsRedraw(); + check( + 'an object panned out of view and back is drawn again', + gone && shown && back.redraws > back0.redraws && offAfterReturn === 0, + `gone=${gone} shown=${shown} redraws=${back.redraws - back0.redraws} rowsOff=${offAfterReturn}`, + ); + await shot('15b-panned'); + await mutate(`__gm.state.select(null); __gm.lookAt(0, 0)`); }