From 375e610660322439f08fb9b2664ba421db334f85 Mon Sep 17 00:00:00 2001 From: somebody1234 Date: Tue, 21 Nov 2023 00:17:34 +1000 Subject: [PATCH] Layout nodes without position (#8326) - Closes #8071 # Important Notes There is currently no way to predict the width a node, taking into account the width of widgets. This should probably be done in another task. --- .../__tests__/placement.test.ts | 100 ++++++++++-------- .../components/ComponentBrowser/placement.ts | 34 +++--- app/gui2/src/components/GraphEditor.vue | 3 +- .../src/components/GraphEditor/GraphEdge.vue | 11 +- app/gui2/src/stores/graph/graphDatabase.ts | 63 ++++++++++- app/gui2/src/util/theme.json | 8 ++ app/gui2/tsconfig.app.json | 9 +- 7 files changed, 156 insertions(+), 72 deletions(-) create mode 100644 app/gui2/src/util/theme.json diff --git a/app/gui2/src/components/ComponentBrowser/__tests__/placement.test.ts b/app/gui2/src/components/ComponentBrowser/__tests__/placement.test.ts index 31ea9693da..0045eb91a4 100644 --- a/app/gui2/src/components/ComponentBrowser/__tests__/placement.test.ts +++ b/app/gui2/src/components/ComponentBrowser/__tests__/placement.test.ts @@ -21,7 +21,8 @@ const radius = size.y / 2 const getScreenBounds = vi.fn(() => defaultScreenBounds) const getNodeRects = vi.fn(() => iterable.empty()) -const getGap = vi.fn(() => 24) +const getHorizontalGap = vi.fn(() => 24) +const getVerticalGap = vi.fn(() => 24) const getSelectedNodeRects = vi.fn(() => iterable.empty()) const getMousePosition = vi.fn(() => Vec2.Zero) // Center is at (1100, 700) @@ -57,36 +58,36 @@ describe('Non dictated placement', () => { test.each([ // === Miscellaneous tests === - { desc: 'Empty graph', nodes: [], pos: new Vec2(1050, 690) }, + { desc: 'Empty graph', nodes: [], pos: new Vec2(1090, 690) }, // === Single node tests === - { desc: 'Single node', nodes: [rectAt(1050, 690)], pos: new Vec2(1050, 734) }, + { desc: 'Single node', nodes: [rectAt(1050, 690)], pos: new Vec2(1090, 734) }, // { desc: 'Single node (far enough left that it does not overlap)', - nodes: [rectAt(950, 690)], - pos: new Vec2(1050, 690), + nodes: [rectAt(990, 690)], + pos: new Vec2(1090, 690), }, { desc: 'Single node (far enough right that it does not overlap)', - nodes: [rectAt(1150, 690)], - pos: new Vec2(1050, 690), + nodes: [rectAt(1190, 690)], + pos: new Vec2(1090, 690), }, { desc: 'Single node (overlaps on the left by 1px)', - nodes: [rectAt(951, 690)], - pos: new Vec2(1050, 734), + nodes: [rectAt(991, 690)], + pos: new Vec2(1090, 734), }, { desc: 'Single node (overlaps on the right by 1px)', - nodes: [rectAt(1149, 690)], - pos: new Vec2(1050, 734), + nodes: [rectAt(1189, 690)], + pos: new Vec2(1090, 734), }, { desc: 'Single node (BIG gap)', nodes: [rectAt(1050, 690)], gap: 1000, - pos: new Vec2(1050, 1710), + pos: new Vec2(1090, 1710), pan: new Vec2(0, 1020), }, @@ -94,12 +95,12 @@ describe('Non dictated placement', () => { { desc: 'Multiple nodes', nodes: map(range(0, 1001, 20), rectAtX(1050)), - pos: new Vec2(1050, 1044), + pos: new Vec2(1090, 1044), }, { desc: 'Multiple nodes with gap', nodes: map(range(1000, -1, -20), rectAtX(1050)), - pos: new Vec2(1050, 1044), + pos: new Vec2(1090, 1044), }, { desc: 'Multiple nodes with gap 2', @@ -107,17 +108,17 @@ describe('Non dictated placement', () => { map(range(500, 901, 20), rectAtX(1050)), map(range(1000, 1501, 20), rectAtX(1050)), ), - pos: new Vec2(1050, 944), + pos: new Vec2(1090, 944), }, { desc: 'Multiple nodes with gap (just big enough)', nodes: map(range(690, 1500, 88), rectAtX(1050)), - pos: new Vec2(1050, 734), + pos: new Vec2(1090, 734), }, { desc: 'Multiple nodes with gap (slightly too small)', nodes: map(range(500, 849, 87), rectAtX(1050)), - pos: new Vec2(1050, 892), + pos: new Vec2(1090, 892), }, { desc: 'Multiple nodes with smallest gap', @@ -125,7 +126,7 @@ describe('Non dictated placement', () => { map(range(500, 901, 20), rectAtX(1050)), map(range(988, 1489, 20), rectAtX(1050)), ), - pos: new Vec2(1050, 944), + pos: new Vec2(1090, 944), }, { desc: 'Multiple nodes with smallest gap (reverse)', @@ -133,7 +134,7 @@ describe('Non dictated placement', () => { map(range(1488, 987, -20), rectAtX(1050)), map(range(900, 499, -20), rectAtX(1050)), ), - pos: new Vec2(1050, 944), + pos: new Vec2(1090, 944), }, { desc: 'Multiple nodes with gap that is too small', @@ -143,7 +144,7 @@ describe('Non dictated placement', () => { ), // This gap is 1px smaller than the previous test - so, 1px too small. // This position is offscreen (y >= 1000), so we pan so that the new node is centered (1531 - 690). - pos: new Vec2(1050, 1531), + pos: new Vec2(1090, 1531), pan: new Vec2(0, 841), }, { @@ -152,12 +153,15 @@ describe('Non dictated placement', () => { map(range(900, 499, -20), rectAtX(1050)), map(range(1487, 986, -20), rectAtX(1050)), ), - pos: new Vec2(1050, 1531), + pos: new Vec2(1090, 1531), pan: new Vec2(0, 841), }, ])('$desc', ({ nodes, pos, gap, pan }) => { expect( - nonDictatedPlacement(nodeSize, nonDictatedEnvironment(nodes), gap ? { gap } : {}), + nonDictatedPlacement(nodeSize, nonDictatedEnvironment(nodes), { + horizontalGap: gap ?? 24, + verticalGap: gap ?? 24, + }), ).toEqual({ position: pos, pan }) expect(getSelectedNodeRects, 'Should not depend on `selectedNodeRects`').not.toHaveBeenCalled() expect(getMousePosition, 'Should not depend on `mousePosition`').not.toHaveBeenCalled() @@ -213,36 +217,36 @@ describe('Previous node dictated placement', () => { test.each([ // === Single node tests === - { desc: 'Single node', nodes: [], pos: new Vec2(1050, 734) }, + { desc: 'Single node', nodes: [], pos: new Vec2(1090, 734) }, { desc: 'Single node (far enough up that it does not overlap)', - nodes: [rectAt(1150, 714)], - pos: new Vec2(1050, 734), + nodes: [rectAt(1189, 714)], + pos: new Vec2(1090, 734), }, { desc: 'Single node (far enough down that it does not overlap)', - nodes: [rectAt(1150, 754)], - pos: new Vec2(1050, 734), + nodes: [rectAt(1189, 754)], + pos: new Vec2(1090, 734), }, { desc: 'Single node (far enough left that it does not overlap)', - nodes: [rectAt(926, 734)], - pos: new Vec2(1050, 734), + nodes: [rectAt(966, 734)], + pos: new Vec2(1090, 734), }, { desc: 'Single node (overlapping on the left by 1px)', - nodes: [rectAt(927, 734)], - pos: new Vec2(1051, 734), + nodes: [rectAt(967, 734)], + pos: new Vec2(1091, 734), }, { desc: 'Single node (blocking initial position)', - nodes: [rectAt(1050, 734)], - pos: new Vec2(1174, 734), + nodes: [rectAt(1090, 734)], + pos: new Vec2(1214, 734), }, { desc: 'Single node (far enough right that it does not overlap)', nodes: [rectAt(1174, 690)], - pos: new Vec2(1050, 734), + pos: new Vec2(1090, 734), }, { desc: 'Single node (overlapping on the right by 1px)', @@ -263,14 +267,14 @@ describe('Previous node dictated placement', () => { desc: 'Single node (BIG gap)', nodes: [], gap: 1000, - pos: new Vec2(1050, 1710), + pos: new Vec2(1090, 1710), pan: new Vec2(0, 1020), }, { desc: 'Single node (BIG gap, overlapping on the left by 1px)', - nodes: [rectAt(927, 1710)], + nodes: [rectAt(967, 1710)], gap: 1000, - pos: new Vec2(2027, 1710), + pos: new Vec2(2067, 1710), pan: new Vec2(977, 1020), }, @@ -279,13 +283,13 @@ describe('Previous node dictated placement', () => { desc: 'Multiple nodes', nodes: map(range(1000, 2001, 100), rectAtY(734)), pos: new Vec2(2124, 734), - pan: new Vec2(1074, 44), + pan: new Vec2(1034, 44), }, { desc: 'Multiple nodes (reverse)', nodes: map(range(2000, 999, -100), rectAtY(734)), pos: new Vec2(2124, 734), - pan: new Vec2(1074, 44), + pan: new Vec2(1034, 44), }, { desc: 'Multiple nodes with gap', @@ -328,7 +332,7 @@ describe('Previous node dictated placement', () => { map(range(1647, 1948, 100), rectAtY(734)), ), pos: new Vec2(2071, 734), - pan: new Vec2(1021, 44), + pan: new Vec2(981, 44), }, { desc: 'Multiple nodes with gap that is too small (each range reversed)', @@ -337,14 +341,14 @@ describe('Previous node dictated placement', () => { map(range(1947, 1646, -100), rectAtY(734)), ), pos: new Vec2(2071, 734), - pan: new Vec2(1021, 44), + pan: new Vec2(981, 44), }, ])('$desc', ({ nodes, gap, pos, pan }) => { expect( previousNodeDictatedPlacement( nodeSize, - previousNodeDictatedEnvironment([...nodes, rectAt(1050, 690)]), - gap != null ? { gap } : {}, + previousNodeDictatedEnvironment([...nodes, rectAt(1090, 690)]), + { horizontalGap: gap ?? 24, verticalGap: gap ?? 24 }, ), ).toEqual({ position: pos, pan }) expect(getMousePosition, 'Should not depend on `mousePosition`').not.toHaveBeenCalled() @@ -422,8 +426,11 @@ describe('Mouse dictated placement', () => { }, }, { - get gap() { - return getGap() + get horizontalGap() { + return getHorizontalGap() + }, + get verticalGap() { + return getVerticalGap() }, }, ), @@ -435,7 +442,8 @@ describe('Mouse dictated placement', () => { expect(getScreenBounds, 'Should not depend on `screenBounds`').not.toHaveBeenCalled() expect(getNodeRects, 'Should not depend on `nodeRects`').not.toHaveBeenCalled() expect(getSelectedNodeRects, 'Should not depend on `selectedNodeRects`').not.toHaveBeenCalled() - expect(getGap, 'Should not depend on `gap`').not.toHaveBeenCalled() + expect(getHorizontalGap, 'Should not depend on `horizontalGap`').not.toHaveBeenCalled() + expect(getVerticalGap, 'Should not depend on `verticalGap`').not.toHaveBeenCalled() }) }) diff --git a/app/gui2/src/components/ComponentBrowser/placement.ts b/app/gui2/src/components/ComponentBrowser/placement.ts index e8651a8edc..af1b12bb16 100644 --- a/app/gui2/src/components/ComponentBrowser/placement.ts +++ b/app/gui2/src/components/ComponentBrowser/placement.ts @@ -1,5 +1,6 @@ import { bail } from '@/util/assert' import { Rect } from '@/util/rect' +import theme from '@/util/theme.json' import { Vec2 } from '@/util/vec2' export interface Environment { @@ -10,7 +11,8 @@ export interface Environment { } export interface PlacementOptions { - gap?: number + horizontalGap?: number + verticalGap?: number } export interface Placement { @@ -18,9 +20,6 @@ export interface Placement { pan?: Vec2 } -// The default gap is the height of a single node. -const defaultGap = 24 - /** The new node should appear at the center of the screen if there is enough space for the new node. * Otherwise, it should be moved down to the closest free space. * @@ -35,18 +34,18 @@ const defaultGap = 24 export function nonDictatedPlacement( nodeSize: Vec2, { screenBounds, nodeRects }: Environment, - { gap = defaultGap }: PlacementOptions = {}, + { verticalGap = theme.node.vertical_gap }: PlacementOptions = {}, ): Placement { - const initialPosition = screenBounds.center().sub(nodeSize.scale(0.5)) + const initialPosition = screenBounds.center().sub(new Vec2(nodeSize.y / 2, nodeSize.y / 2)) const initialRect = new Rect(initialPosition, nodeSize) let top = initialPosition.y const height = nodeSize.y const bottom = () => top + height const nodeRectsSorted = Array.from(nodeRects).sort((a, b) => a.top - b.top) for (const rect of nodeRectsSorted) { - if (initialRect.intersectsX(rect) && rect.bottom + gap > top) { - if (rect.top - bottom() < gap) { - top = rect.bottom + gap + if (initialRect.intersectsX(rect) && rect.bottom + verticalGap > top) { + if (rect.top - bottom() < verticalGap) { + top = rect.bottom + verticalGap } } } @@ -63,7 +62,7 @@ export function nonDictatedPlacement( * In case the place is offscreen, the camera should be panned accordingly. * * Specifically, this code, in order: - * - uses the left side of the first selected node and as the initial x-position + * - uses the left side of the first selected node as the initial x-position * - uses the lowest (highest y-position) of all selected nodes, plus the specified gap, * as the initial y-position * - searches for all horizontal spans to the right of the initial position, @@ -79,13 +78,16 @@ export function nonDictatedPlacement( export function previousNodeDictatedPlacement( nodeSize: Vec2, { screenBounds, selectedNodeRects, nodeRects }: Environment, - { gap = defaultGap }: PlacementOptions = {}, + { + horizontalGap = theme.node.horizontal_gap, + verticalGap = theme.node.vertical_gap, + }: PlacementOptions = {}, ): Placement { let initialLeft: number | undefined let top = -Infinity for (const rect of selectedNodeRects) { initialLeft ??= rect.left - const newTop = rect.bottom + gap + const newTop = rect.bottom + verticalGap if (newTop > top) top = newTop } if (initialLeft == null) @@ -97,16 +99,16 @@ export function previousNodeDictatedPlacement( const initialRect = new Rect(initialPosition, nodeSize) const sortedNodeRects = Array.from(nodeRects).sort((a, b) => a.left - b.left) for (const rect of sortedNodeRects) { - if (initialRect.intersectsY(rect) && rect.right + gap > left) { - if (rect.left - right() < gap) { - left = rect.right + gap + if (initialRect.intersectsY(rect) && rect.right + horizontalGap > left) { + if (rect.left - right() < horizontalGap) { + left = rect.right + horizontalGap } } } const finalPosition = new Vec2(left, top) if (new Rect(finalPosition, nodeSize).within(screenBounds)) return { position: finalPosition } else { - const screenCenter = screenBounds.center().sub(nodeSize.scale(0.5)) + const screenCenter = screenBounds.center().sub(new Vec2(nodeSize.y / 2, nodeSize.y / 2)) return { position: finalPosition, pan: finalPosition.sub(screenCenter) } } } diff --git a/app/gui2/src/components/GraphEditor.vue b/app/gui2/src/components/GraphEditor.vue index 857cf86b0b..e983f41c6d 100644 --- a/app/gui2/src/components/GraphEditor.vue +++ b/app/gui2/src/components/GraphEditor.vue @@ -67,7 +67,8 @@ function targetComponentBrowserPosition() { } else if (hasNodeSelected) { const gapBetweenNodes = 48.0 return previousNodeDictatedPlacement(nodeSize, placementEnvironment.value, { - gap: gapBetweenNodes, + horizontalGap: gapBetweenNodes, + verticalGap: gapBetweenNodes, }).position } else { return mouseDictatedPlacement(nodeSize, placementEnvironment.value).position diff --git a/app/gui2/src/components/GraphEditor/GraphEdge.vue b/app/gui2/src/components/GraphEditor/GraphEdge.vue index c561258fe8..bd9a5ae401 100644 --- a/app/gui2/src/components/GraphEditor/GraphEdge.vue +++ b/app/gui2/src/components/GraphEditor/GraphEdge.vue @@ -4,6 +4,7 @@ import { injectGraphSelection } from '@/providers/graphSelection.ts' import { useGraphStore, type Edge } from '@/stores/graph' import { assert } from '@/util/assert' import { Rect } from '@/util/rect' +import theme from '@/util/theme.json' import { Vec2 } from '@/util/vec2' import { clamp } from '@vueuse/core' import { computed, ref } from 'vue' @@ -94,8 +95,6 @@ type JunctionPoints = { /** Minimum height above the target the edge must approach it from. */ const MIN_APPROACH_HEIGHT = 32 -const NODE_HEIGHT = 32 // TODO (crate::component::node::HEIGHT) -const NODE_CORNER_RADIUS = 16 // TODO (crate::component::node::CORNER_RADIUS) /** The preferred arc radius. */ const RADIUS_BASE = 20 @@ -173,7 +172,7 @@ function junctionPoints(inputs: Inputs): JunctionPoints | null { let halfSourceSize = inputs.sourceSize?.scale(0.5) ?? Vec2.Zero // The maximum x-distance from the source (our local coordinate origin) for the point where the // edge will begin. - const sourceMaxXOffset = Math.max(halfSourceSize.x - NODE_CORNER_RADIUS, 0) + const sourceMaxXOffset = Math.max(halfSourceSize.x - theme.node.corner_radius, 0) const attachment = inputs.targetPortTopDistanceInNode != null ? { @@ -184,7 +183,7 @@ function junctionPoints(inputs: Inputs): JunctionPoints | null { const targetWellBelowSource = inputs.targetOffset.y - (inputs.targetPortTopDistanceInNode ?? 0) >= MIN_APPROACH_HEIGHT - const targetBelowSource = inputs.targetOffset.y > NODE_HEIGHT / 2.0 + const targetBelowSource = inputs.targetOffset.y > theme.node.height / 2.0 const targetBeyondSource = Math.abs(inputs.targetOffset.x) > sourceMaxXOffset const horizontalRoomFor3Corners = targetBeyondSource && @@ -222,9 +221,9 @@ function junctionPoints(inputs: Inputs): JunctionPoints | null { // at the point that it exits the node. const radius = Math.min(naturalRadius, maxRadius) const arcOriginX = Math.abs(inputs.targetOffset.x) - radius - const sourceArcOrigin = halfSourceSize.x - NODE_CORNER_RADIUS + const sourceArcOrigin = halfSourceSize.x - theme.node.corner_radius const circleOffset = arcOriginX - sourceArcOrigin - const intersection = circleIntersection(circleOffset, NODE_CORNER_RADIUS, radius) + const intersection = circleIntersection(circleOffset, theme.node.corner_radius, radius) sourceDY = -Math.abs(radius - intersection) } else if (halfSourceSize.y != 0) { sourceDY = -SOURCE_NODE_OVERLAP + halfSourceSize.y diff --git a/app/gui2/src/stores/graph/graphDatabase.ts b/app/gui2/src/stores/graph/graphDatabase.ts index 3aaa8604ce..6dce87b171 100644 --- a/app/gui2/src/stores/graph/graphDatabase.ts +++ b/app/gui2/src/stores/graph/graphDatabase.ts @@ -1,11 +1,15 @@ +import { nonDictatedPlacement } from '@/components/ComponentBrowser/placement' import { SuggestionDb, groupColorStyle, type Group } from '@/stores/suggestionDatabase' import { tryGetIndex } from '@/util/array' import { Ast, AstExtended } from '@/util/ast' import { colorFromString } from '@/util/colors' import { ComputedValueRegistry, type ExpressionInfo } from '@/util/computedValueRegistry' import { ReactiveDb, ReactiveIndex, ReactiveMapping } from '@/util/database/reactiveDb' +import { getTextWidth } from '@/util/measurement' import type { Opt } from '@/util/opt' import { qnJoin, tryQualifiedName } from '@/util/qualifiedName' +import { Rect } from '@/util/rect' +import theme from '@/util/theme.json' import { Vec2 } from '@/util/vec2' import * as set from 'lib0/set' import { @@ -103,11 +107,20 @@ export class GraphDb { this.nodes.moveToLast(id) } + getNodeWidth(node: Node) { + // FIXME [sb]: This should take into account the width of all widgets. + // This will require a recursive traversal of the `Node`'s children. + return getTextWidth(node.rootSpan.repr(), '11.5px', '"M PLUS 1", sans-serif') * 1.2 + } + readFunctionAst( functionAst: AstExtended, getMeta: (id: ExprId) => NodeMetadata | undefined, ) { const currentNodeIds = new Set() + const nodeRectMap = new Map() + let numberOfUnpositionedNodes = 0 + let maxUnpositionedNodeWidth = 0 if (functionAst) { for (const nodeAst of functionAst.visit(getFunctionNodeExpressions)) { const newNode = nodeFromAst(nodeAst) @@ -117,7 +130,6 @@ export class GraphDb { currentNodeIds.add(nodeId) if (node == null) { this.nodes.set(nodeId, newNode) - if (nodeMeta) this.assignUpdatedMetadata(newNode, nodeMeta) } else { if (node.binding !== newNode.binding) { node.binding = newNode.binding @@ -128,7 +140,24 @@ export class GraphDb { if (indexedDB.cmp(node.rootSpan.contentHash(), newNode.rootSpan.contentHash()) !== 0) { node.rootSpan = newNode.rootSpan } - if (nodeMeta) this.assignUpdatedMetadata(node, nodeMeta) + } + if (!nodeMeta) { + numberOfUnpositionedNodes += 1 + maxUnpositionedNodeWidth = Math.max( + maxUnpositionedNodeWidth, + this.getNodeWidth(node ?? newNode), + ) + } else { + this.assignUpdatedMetadata(node ?? newNode, nodeMeta) + nodeRectMap.set( + nodeId, + Rect.FromBounds( + nodeMeta.x, + nodeMeta.y, + nodeMeta.x + this.getNodeWidth(node ?? newNode), + nodeMeta.y + theme.node.height, + ), + ) } } } @@ -138,6 +167,36 @@ export class GraphDb { this.nodes.delete(nodeId) } } + + const nodeRects = [...nodeRectMap.values()] + const rectsHeight = + numberOfUnpositionedNodes * (theme.node.height + theme.node.vertical_gap) - + theme.node.vertical_gap + const { position: rectsPosition } = nonDictatedPlacement( + new Vec2(maxUnpositionedNodeWidth, rectsHeight), + { + nodeRects, + // The rest of the properties should not matter. + selectedNodeRects: [], + screenBounds: Rect.Zero, + mousePosition: Vec2.Zero, + }, + ) + let nodeIndex = 0 + for (const nodeId of this.allNodeIds()) { + const meta = getMeta(nodeId) + if (meta) continue + const node = this.nodes.get(nodeId)! + const size = new Vec2(this.getNodeWidth(node), theme.node.height) + const position = new Vec2( + rectsPosition.x, + rectsPosition.y + (theme.node.height + theme.node.vertical_gap) * nodeIndex, + ) + nodeRects.push(new Rect(position, size)) + node.position = new Vec2(position.x, position.y) + + nodeIndex += 1 + } } assignUpdatedMetadata(node: Node, meta: NodeMetadata) { diff --git a/app/gui2/src/util/theme.json b/app/gui2/src/util/theme.json new file mode 100644 index 0000000000..90aecec1ca --- /dev/null +++ b/app/gui2/src/util/theme.json @@ -0,0 +1,8 @@ +{ + "node": { + "height": 32, + "corner_radius": 16, + "vertical_gap": 32, + "horizontal_gap": 32 + } +} diff --git a/app/gui2/tsconfig.app.json b/app/gui2/tsconfig.app.json index 4acc93f3ce..511c7be1ff 100644 --- a/app/gui2/tsconfig.app.json +++ b/app/gui2/tsconfig.app.json @@ -1,6 +1,13 @@ { "extends": "@vue/tsconfig/tsconfig.dom.json", - "include": ["env.d.ts", "src/**/*", "src/**/*.vue", "shared/**/*", "shared/**/*.vue"], + "include": [ + "env.d.ts", + "src/**/*", + "src/**/*.vue", + "shared/**/*", + "shared/**/*.vue", + "src/util/theme.json" + ], "exclude": ["src/**/__tests__/*", "shared/**/__tests__/*", "public/**/__tests__/*"], "compilerOptions": { "lib": ["ES2021", "DOM", "DOM.Iterable"],