Move a large graph instead of drawing it again (#292)
In a graph of many nodes the frame rate dropped while panning, and more so while one node was dragged. Every pointer move rebuilt every box, every text and every curve, and a drag also announced a changed project, so the panels rebuilt along with it. - Panning and zooming move the picture. GraphCanvas.applyView sets position and scale and draws anew only the grid and the rubber band. It used to call redraw(). - A node draws its box and makes its texts only when what they show has changed. NodeView keeps a signature of what redraw reads and one of what rebuildTexts reads, and returns early while they hold. - A drag does not announce a changed project. GraphDrags writes the model on every pointer move and asks the canvas once a frame to follow (GraphCanvasHost.moved). state.preview() waits for the command of the release. - Nodes outside the picture are not drawn. inView in graphView.ts is the rule, cull() applies it after every redraw and every change of the view. - The graph is a render group, and box and pins go into the batch. The mask sits on a container above the group, on the group itself it would be panned and zoomed along. - The rubber band and the drop mark of a dragged behavior are drawn on their own (drawBand), without a redraw of the graph. Measured in the CI image with 388 nodes and 540 links, fitted to the canvas, 240 pointer moves 16 ms apart: - Pan, script per move: 25.2 ms before, 2.0 ms after - Pan: 4.4 frames per second before, 18.7 after - Drag a node: 2.7 frames per second before, 22.4 after - The still picture: 9 frames per second before, 21 after Not solved: the still picture of such a graph stays at 21 frames per second in the test browser. What is left is the drawing itself, and the frame rates are those of that browser, not of a desktop GPU. Tests: - Gate on melektaus-atx: 207 files, 3121 tests (3114 + 7), exit 0 - e2e 4,6,18,19,20,23,24,26,28,31,36,43,51,53 on melektaus-atx: 542 pass, 1 fail. The fail is module 51 after module 43, an order the suite does not have, and it fails the same way without this change. Module 51 alone: 12 checks, all pass. - No full run and no smokes. The sum of 1660 (1656 + 4) is reckoned, and the CI workflow is switched off. - Seven unit tests: six for inView in tests/editor/graph-view.test.ts and one in frame-preview.test.ts - Four e2e checks in block 6z-xii of module 06: pan, drag, zoom in and fit, a changed value - Seven browser counter-probes fail 2 to 3 checks each Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
32a0af6e2c
commit
6d79cf59a3
11 files changed
+393
-23
No files matched your search
@@ -58,6 +58,9 @@ export function graphHooks({ state, editor }: HookContext) {
|
||||
snapping: () => graphEditor.canvas.snapEnabled,
|
||||
snapButton: () => snapButton(graphEditor.snap),
|
||||
zoom: () => graphEditor.canvas.zoomLevel,
|
||||
/** Nodes left in the picture, and how often texts, boxes and links were really made (#292). */
|
||||
nodesDrawn: () => graphEditor.canvas.nodesDrawn(),
|
||||
made: () => graphEditor.canvas.partsMade(),
|
||||
crumbs: () => graphEditor.breadcrumb.crumbsDrawn(),
|
||||
},
|
||||
stateMachine: {
|
||||
|
||||
@@ -82,6 +82,8 @@ export class EdgeLayer extends Container {
|
||||
/** Every link the last draw stroked, bottom first – what a click on a line is tested against. */
|
||||
private paths: EdgePath[] = [];
|
||||
private picked: ReadonlySet<string> = new Set();
|
||||
/** How often the links were drawn, for the checks: a pan must not add to it (#292). */
|
||||
drawCount = 0;
|
||||
|
||||
constructor() {
|
||||
super();
|
||||
@@ -96,6 +98,7 @@ export class EdgeLayer extends Container {
|
||||
beads: (edgeId: string) => readonly number[] = () => [],
|
||||
picked: ReadonlySet<string> = new Set(),
|
||||
): void {
|
||||
this.drawCount += 1;
|
||||
this.paths = [];
|
||||
this.picked = picked;
|
||||
this.curves.clear();
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
endDrag,
|
||||
onDragChange,
|
||||
pressCount,
|
||||
requestRender,
|
||||
type MenuItem,
|
||||
type Size,
|
||||
Theme,
|
||||
@@ -65,7 +66,7 @@ import { NodePalette, type PaletteAction } from './NodePalette';
|
||||
import { splitEntries } from './splitMenu';
|
||||
import { type PaletteEntry, paletteEntries } from './paletteEntries';
|
||||
import { NODE_WIDTH, NodeView, TITLE_HEIGHT } from './NodeView';
|
||||
import { drawGraphGrid, FIT_MARGIN, ZOOM_LIMITS, zoomedAbout } from './graphView';
|
||||
import { drawGraphGrid, FIT_MARGIN, inView, ZOOM_LIMITS, zoomedAbout } from './graphView';
|
||||
import { GraphWiring } from './wiring';
|
||||
|
||||
/** One running instance's answer for a machine: which state it holds, and whose instance it is. */
|
||||
@@ -96,6 +97,7 @@ export class GraphCanvas extends Widget {
|
||||
private readonly bg = new Graphics();
|
||||
private readonly grid = new Graphics();
|
||||
private readonly clip = new Graphics();
|
||||
private readonly clipped = new Container();
|
||||
private readonly canvas = new Container();
|
||||
private readonly annotationLayer = new AnnotationLayer((view, e) => this.onAnnotationPointerDown(view, e));
|
||||
private readonly blockLayer = new Graphics();
|
||||
@@ -159,6 +161,7 @@ export class GraphCanvas extends Widget {
|
||||
entries: () => this.entries(),
|
||||
newNode: (entry, at) => this.newNode(entry, at),
|
||||
redraw: () => this.redraw(),
|
||||
moved: () => this.moved(),
|
||||
};
|
||||
this.wiring = new GraphWiring(host);
|
||||
this.drags = new GraphDrags(host);
|
||||
@@ -169,8 +172,13 @@ export class GraphCanvas extends Widget {
|
||||
this.nodeLayer,
|
||||
this.tempEdge,
|
||||
);
|
||||
this.addChild(this.bg, this.grid, this.canvas, this.band, this.hint, this.clip);
|
||||
this.canvas.mask = this.clip;
|
||||
this.clipped.addChild(this.canvas);
|
||||
this.addChild(this.bg, this.grid, this.clipped, this.band, this.hint, this.clip);
|
||||
this.clipped.mask = this.clip;
|
||||
// A group of its own: panning and zooming move it as one, and a node that enters or leaves the
|
||||
// picture rebuilds the drawing of the graph only, not of the whole editor. The mask sits one
|
||||
// level above it: on the group itself it would be panned and zoomed along with the graph.
|
||||
this.canvas.isRenderGroup = true;
|
||||
this.cursor = 'default';
|
||||
|
||||
this.on('pointerdown', (e: FederatedPointerEvent) => this.onCanvasPointerDown(e));
|
||||
@@ -304,6 +312,18 @@ export class GraphCanvas extends Widget {
|
||||
this.redraw();
|
||||
}
|
||||
|
||||
/**
|
||||
* A drag moved nodes or boxes in the model. Nothing a node shows depends on where it stands, so
|
||||
* the views are put there and the links drawn again – without asking every node what it is.
|
||||
*/
|
||||
private moved(): void {
|
||||
const asset = this.asset();
|
||||
if (!asset) return;
|
||||
this.annotationLayer.sync(asset.graph);
|
||||
for (const node of asset.graph.nodes) this.views.get(node.id)?.position.set(node.x, node.y);
|
||||
this.redraw();
|
||||
}
|
||||
|
||||
private onAnnotationPointerDown(view: AnnotationView, e: FederatedPointerEvent): void {
|
||||
if (e.button === 1 || !this.assetId) return;
|
||||
e.stopPropagation();
|
||||
@@ -430,10 +450,41 @@ export class GraphCanvas extends Widget {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Moves the picture, and draws anew only what is laid out on the screen rather than in the graph:
|
||||
* the grid and the band. Nodes and links keep their shapes – a pan that rebuilt them on every
|
||||
* pointer move cost more than a frame in a graph of a few hundred nodes (#292).
|
||||
*/
|
||||
private applyView(): void {
|
||||
this.canvas.position.set(Math.round(this.pan.x), Math.round(this.pan.y));
|
||||
this.canvas.scale.set(this.zoom);
|
||||
this.redraw();
|
||||
this.syncGrid();
|
||||
this.cull();
|
||||
this.drawBand();
|
||||
requestRender();
|
||||
}
|
||||
|
||||
/** Takes the nodes outside the picture out of the drawing – see `inView`. */
|
||||
private cull(): void {
|
||||
const view = { pan: this.pan, zoom: this.zoom };
|
||||
for (const node of this.views.values()) node.visible = inView(node, view, this.w, this.h);
|
||||
}
|
||||
|
||||
/** How many nodes the last redraw left in the picture. */
|
||||
nodesDrawn(): number {
|
||||
let drawn = 0;
|
||||
for (const node of this.views.values()) if (node.visible) drawn += 1;
|
||||
return drawn;
|
||||
}
|
||||
|
||||
/** How often the texts and boxes of the nodes were really made, and the links drawn – a pan must add nothing. */
|
||||
partsMade(): { texts: number; box: number; links: number } {
|
||||
const made = { texts: 0, box: 0, links: this.edgeLayer.drawCount };
|
||||
for (const node of this.views.values()) {
|
||||
made.texts += node.made.texts;
|
||||
made.box += node.made.box;
|
||||
}
|
||||
return made;
|
||||
}
|
||||
|
||||
private screenToGraph(sx: number, sy: number): { x: number; y: number } {
|
||||
@@ -626,7 +677,7 @@ export class GraphCanvas extends Widget {
|
||||
const d = Math.hypot(g.x - this.marquee.start.x, g.y - this.marquee.start.y) * this.zoom;
|
||||
if (!this.marquee.active && d < DRAG_THRESHOLD) return;
|
||||
this.marquee.active = true;
|
||||
this.redraw();
|
||||
this.drawBand();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -654,7 +705,7 @@ export class GraphCanvas extends Widget {
|
||||
private setIncoming(incoming: GraphCanvas['incoming']): void {
|
||||
if (!incoming && !this.incoming) return;
|
||||
this.incoming = incoming;
|
||||
this.redraw();
|
||||
this.drawBand();
|
||||
}
|
||||
|
||||
private dropIncoming(drop: NonNullable<GraphCanvas['incoming']>): void {
|
||||
@@ -852,6 +903,7 @@ export class GraphCanvas extends Widget {
|
||||
target && target.nodeId === view.node.id ? { port: target.port, isOutput: target.isOutput } : null;
|
||||
view.redraw();
|
||||
}
|
||||
this.cull();
|
||||
|
||||
this.drawnBlocks = blockFrames(asset.graph, (id) => {
|
||||
const view = this.views.get(id);
|
||||
@@ -869,8 +921,14 @@ export class GraphCanvas extends Widget {
|
||||
);
|
||||
|
||||
this.wiring.draw(this.tempEdge, asset.graph);
|
||||
this.drawBand();
|
||||
}
|
||||
|
||||
/** The rubber band and the place a dragged behavior would land, both in screen space. */
|
||||
private drawBand(): void {
|
||||
const c = Theme.colors;
|
||||
this.band.clear();
|
||||
if (!this.asset()) return;
|
||||
const m = this.marquee;
|
||||
if (m?.active) {
|
||||
drawBand(
|
||||
|
||||
@@ -131,6 +131,15 @@ export class NodeView extends Widget {
|
||||
private readonly valueTexts = new Map<string, Text>();
|
||||
/** Where an output's value may be drawn: right-aligned before its label, clear of the input text. */
|
||||
private valueSlots = new Map<string, { left: number; right: number; top: number }>();
|
||||
/**
|
||||
* What the texts and the box were last made from. A graph is redrawn on every model event and on
|
||||
* every frame of a run, and for nearly every node nothing has changed: a text made anew is a
|
||||
* picture uploaded anew, and a box cleared is a shape built anew (#292).
|
||||
*/
|
||||
private textsOf = '';
|
||||
private drawnOf = '';
|
||||
/** How often the texts and the box were really made, for the checks. */
|
||||
readonly made = { texts: 0, box: 0 };
|
||||
|
||||
/**
|
||||
* Not fixed at construction: a nested-behavior node's ports come from another graph, so the
|
||||
@@ -145,6 +154,10 @@ export class NodeView extends Widget {
|
||||
super();
|
||||
this.def = def;
|
||||
this.cursor = 'grab';
|
||||
// Pixi draws a shape of more than 200 vertices on its own, and the round pins are: a few hundred
|
||||
// nodes were a few hundred draw calls. In the batch they share one.
|
||||
this.box.context.batchMode = 'batch';
|
||||
this.ports.context.batchMode = 'batch';
|
||||
this.addChild(this.box, this.ports);
|
||||
this.sync(node, { nodes: [node], edges: [] }, null, def);
|
||||
}
|
||||
@@ -215,10 +228,51 @@ export class NodeView extends Widget {
|
||||
const height = this.collapsed ? TITLE_HEIGHT : this.headerHeight() + this.rowCount() * ROW_HEIGHT + PAD;
|
||||
// Sized before the texts: output labels are placed against the right edge.
|
||||
this.resize(this.collapsed ? this.collapsedWidth() : NODE_WIDTH, height);
|
||||
this.rebuildTexts();
|
||||
const texts = this.textSignature();
|
||||
if (texts !== this.textsOf) {
|
||||
this.textsOf = texts;
|
||||
this.made.texts += 1;
|
||||
this.rebuildTexts();
|
||||
}
|
||||
this.position.set(node.x, node.y);
|
||||
}
|
||||
|
||||
/** Everything `rebuildTexts` reads. */
|
||||
private textSignature(): string {
|
||||
const rows: unknown[] = [];
|
||||
for (let i = 0; i < this.rowCount(); i++) {
|
||||
const input = this.def.inputs[i];
|
||||
const output = this.def.outputs[i];
|
||||
rows.push(
|
||||
input ? [this.inputText(input), rowStyle(input)] : null,
|
||||
output ? [output.label, rowStyle(output), output.type !== 'exec' && !output.ghost, output.key] : null,
|
||||
);
|
||||
}
|
||||
return JSON.stringify([this.w, this.collapsed, this.def.label, this.paramLines, this.problem, rows]);
|
||||
}
|
||||
|
||||
/** Everything `redraw` reads. */
|
||||
private drawSignature(): string {
|
||||
const port = (p: PortDef) => `${p.key}:${p.paintType ?? p.type}:${p.ghost ? 1 : 0}`;
|
||||
return JSON.stringify([
|
||||
this.w,
|
||||
this.h,
|
||||
this.collapsed,
|
||||
this.def.category,
|
||||
this.def.transitionEdges === true,
|
||||
this.isStart,
|
||||
this.hot,
|
||||
this.selected,
|
||||
this.problem !== null,
|
||||
this.stateMark,
|
||||
this.linked,
|
||||
this.targetPort,
|
||||
this.headerHeight(),
|
||||
this.def.inputs.map(port),
|
||||
this.def.outputs.map(port),
|
||||
]);
|
||||
}
|
||||
|
||||
private collapsedWidth(): number {
|
||||
const title = CanvasTextMetrics.measureText(this.def.label, textStyle('accent')).width;
|
||||
return collapsedNodeWidth(title, TITLE_X + COLLAPSED_TITLE_END, NODE_WIDTH);
|
||||
@@ -408,6 +462,10 @@ export class NodeView extends Widget {
|
||||
}
|
||||
|
||||
override redraw(): void {
|
||||
const drawn = this.drawSignature();
|
||||
if (drawn === this.drawnOf) return;
|
||||
this.drawnOf = drawn;
|
||||
this.made.box += 1;
|
||||
const c = Theme.colors;
|
||||
this.box.clear();
|
||||
this.box.roundRect(2, 3, this.w, this.h, Theme.radius).fill({ color: 0x000000, alpha: 0.35 });
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { annotationNoun, normalizeBox } from '../../core/commands/annotationCommands';
|
||||
import { oneStep } from '../../core/commands/CompoundCommand';
|
||||
import { findAnnotation, MIN_ANNOTATION_SIZE } from '../../core/graph';
|
||||
import { cancelFrameWork, onFrame } from '../../ui';
|
||||
import { dragOffset, resizeSize } from '../framing';
|
||||
import {
|
||||
boxOf,
|
||||
@@ -11,7 +12,6 @@ import {
|
||||
travellers,
|
||||
type Travellers,
|
||||
} from './annotationDrag';
|
||||
import { FramePreview } from '../framePreview';
|
||||
import type { AnnotationView } from './AnnotationView';
|
||||
import type { GraphCanvasHost } from './graphHost';
|
||||
import { GRID_STEP } from './graphView';
|
||||
@@ -20,6 +20,8 @@ import type { Point } from './transitions';
|
||||
/** Pointer travel (screen pixels) before a press turns into a drag rather than a click. */
|
||||
export const DRAG_THRESHOLD = 3;
|
||||
|
||||
let instances = 0;
|
||||
|
||||
/** Nodes, groups and comments dragged about the canvas, each gesture one undo step. */
|
||||
export class GraphDrags {
|
||||
/** Nodes being dragged, with the boxes a band caught alongside them – they travel together. */
|
||||
@@ -37,10 +39,17 @@ export class GraphDrags {
|
||||
moved: boolean;
|
||||
} | null = null;
|
||||
|
||||
private readonly perFrame: FramePreview;
|
||||
/**
|
||||
* The gesture writes the model on every pointer move, the canvas follows once a frame (#213) – and
|
||||
* only the canvas: where a node stands concerns no panel, no run and no picture, so the drag does
|
||||
* not announce a changed project until the command of its release does (#292).
|
||||
*/
|
||||
private readonly frameKey = `graph-drag:${(instances += 1)}`;
|
||||
|
||||
constructor(private readonly host: GraphCanvasHost) {
|
||||
this.perFrame = new FramePreview(host.state, 'graph-drag');
|
||||
constructor(private readonly host: GraphCanvasHost) {}
|
||||
|
||||
private follow(): void {
|
||||
onFrame(this.frameKey, () => this.host.moved());
|
||||
}
|
||||
|
||||
/** A press on a node: the selection travels with it. False when there is no graph to move in. */
|
||||
@@ -100,7 +109,7 @@ export class GraphDrags {
|
||||
const step = this.host.snapping() ? GRID_STEP : 0;
|
||||
// Each thing snaps for itself, node or box, so what lands on the grid is what was grabbed.
|
||||
placeTravellers(graph, drag.along, (start) => dragOffset(start, dx, dy, step));
|
||||
this.perFrame.request();
|
||||
this.follow();
|
||||
}
|
||||
|
||||
/** Moves or resizes the dragged box in the model, for the preview; committed on release. */
|
||||
@@ -123,7 +132,7 @@ export class GraphDrags {
|
||||
const offset = dragOffset(drag.grabbed.start, dx, dy, step);
|
||||
placeTravellers(graph, drag.along, () => offset);
|
||||
}
|
||||
this.perFrame.request();
|
||||
this.follow();
|
||||
}
|
||||
|
||||
/** Puts the previewed boxes back and records the whole gesture as one undo step. False when no box was dragged. */
|
||||
@@ -131,7 +140,7 @@ export class GraphDrags {
|
||||
const drag = this.box;
|
||||
if (!drag) return false;
|
||||
this.box = null;
|
||||
this.perFrame.cancel();
|
||||
cancelFrameWork(this.frameKey);
|
||||
const graph = this.host.asset()?.graph;
|
||||
const assetId = this.host.assetId();
|
||||
if (!drag.moved || !graph || !assetId) return true;
|
||||
@@ -146,7 +155,7 @@ export class GraphDrags {
|
||||
releaseNodes(): void {
|
||||
const drag = this.nodes;
|
||||
this.nodes = null;
|
||||
this.perFrame.cancel();
|
||||
cancelFrameWork(this.frameKey);
|
||||
const graph = this.host.asset()?.graph;
|
||||
const assetId = this.host.assetId();
|
||||
if (!drag?.moved || !graph || !assetId) return;
|
||||
|
||||
@@ -28,4 +28,6 @@ export interface GraphCanvasHost {
|
||||
/** The command that adds a node from the palette at a graph point. */
|
||||
newNode(entry: PaletteEntry, at: Point): AddNodeCommand;
|
||||
redraw(): void;
|
||||
/** A drag moved nodes or boxes in the model: their views follow, and the links between them. */
|
||||
moved(): void;
|
||||
}
|
||||
@@ -80,6 +80,28 @@ export function panAndZoom(
|
||||
});
|
||||
}
|
||||
|
||||
/** What a node draws beyond its box: the shadow, the pins on its edges and the ring around a drop target. */
|
||||
export const NODE_OVERHANG = 12;
|
||||
|
||||
/**
|
||||
* Whether any of a box, with what hangs over its edges, is inside a view of `w` by `h` screen pixels.
|
||||
* A graph is mostly looked at in parts, and a node outside the picture costs as much to draw as one
|
||||
* inside it (#292).
|
||||
*/
|
||||
export function inView(
|
||||
box: { x: number; y: number; w: number; h: number },
|
||||
view: GraphView,
|
||||
w: number,
|
||||
h: number,
|
||||
overhang = NODE_OVERHANG,
|
||||
): boolean {
|
||||
const left = (0 - view.pan.x) / view.zoom - overhang;
|
||||
const top = (0 - view.pan.y) / view.zoom - overhang;
|
||||
const right = (w - view.pan.x) / view.zoom + overhang;
|
||||
const bottom = (h - view.pan.y) / view.zoom + overhang;
|
||||
return box.x < right && box.x + box.w > left && box.y < bottom && box.y + box.h > top;
|
||||
}
|
||||
|
||||
export function drawGraphGrid(g: Graphics, view: GraphView, w: number, h: number): void {
|
||||
g.clear();
|
||||
const step = GRID_STEP * view.zoom;
|
||||
|
||||
Reference in new issue
Block a user