Fix auto layout (#8345)

So it turns out auto-layout was broken if there were existing, already positioned nodes

# Important Notes
None

app/gui2/src/components/ComponentBrowser.vue

@@ -45,9 +45,7 @@ const emit = defineEmits<{
 function getInitialContent(): string {
   if (props.sourceNode == null) return props.initialContent
   const sourceNode = props.sourceNode
-  console.log(sourceNode)
   const sourceNodeName = graphStore.db.getNodeMainOutputPortIdentifier(sourceNode)
-  console.log(sourceNodeName)
   const sourceNodeNameWithDot = sourceNodeName ? sourceNodeName + '.' : ''
   return sourceNodeNameWithDot + props.initialContent
 }

app/gui2/src/components/GraphEditor.vue

@@ -288,37 +288,37 @@ async function handleFileDrop(event: DragEvent) {
   }
 }
 
-function onComponentBrowserCommit(content: string) {
-  if (content != null && graphStore.editedNodeInfo != null) {
-    // We finish editing a node.
-    graphStore.setNodeContent(graphStore.editedNodeInfo.id, content)
-  } else if (content != null) {
-    // We finish creating a new node.
-    const nodePosition = componentBrowserPosition.value
-    graphStore.createNode(nodePosition.sub(COMPONENT_BROWSER_TO_NODE_OFFSET), content)
-  }
+function resetComponentBrowserState() {
   componentBrowserVisible.value = false
   graphStore.editedNodeInfo = undefined
   interaction.setCurrent(undefined)
 }
 
-function onComponentBrowserCancel() {
-  componentBrowserVisible.value = false
-  graphStore.editedNodeInfo = undefined
-  interaction.setCurrent(undefined)
+function onComponentBrowserCommit(content: string) {
+  if (content != null) {
+    if (graphStore.editedNodeInfo) {
+      // We finish editing a node.
+      graphStore.setNodeContent(graphStore.editedNodeInfo.id, content)
+    } else {
+      // We finish creating a new node.
+      const nodePosition = componentBrowserPosition.value
+      graphStore.createNode(nodePosition.sub(COMPONENT_BROWSER_TO_NODE_OFFSET), content)
+    }
+  }
+  resetComponentBrowserState()
 }
 
+const onComponentBrowserCancel = resetComponentBrowserState
+
 function getNodeContent(id: ExprId): string {
-  const node = graphStore.db.nodeIdToNode.get(id)
-  if (node == null) return ''
-  return node.rootSpan.repr()
+  return graphStore.db.nodeIdToNode.get(id)?.rootSpan.repr() ?? ''
 }
 
 // Watch the `editedNode` in the graph store
 watch(
   () => graphStore.editedNodeInfo,
   (editedInfo) => {
-    if (editedInfo != null) {
+    if (editedInfo) {
       componentBrowserPosition.value = targetComponentBrowserPosition()
       componentBrowserInputContent.value = getNodeContent(editedInfo.id)
       componentBrowserVisible.value = true
@@ -328,16 +328,16 @@ watch(
   },
 )
 
-const breadcrumbs = computed(() => {
-  return projectStore.executionContext.desiredStack.map((frame) => {
+const breadcrumbs = computed(() =>
+  projectStore.executionContext.desiredStack.map((frame) => {
     switch (frame.type) {
       case 'ExplicitCall':
         return frame.methodPointer.name
       case 'LocalCall':
         return frame.expressionId
     }
-  })
-})
+  }),
+)
 
 // === Clipboard ===
 
@@ -358,7 +358,7 @@ interface CopiedNode {
 function copyNodeContent() {
   const id = nodeSelection.selected.values().next().value
   const node = graphStore.db.nodeIdToNode.get(id)
-  if (node == null) return
+  if (!node) return
   const content = node.rootSpan.repr()
   const metadata = projectStore.module?.getNodeMetadata(id) ?? undefined
   const copiedNode: CopiedNode = { expression: content, metadata }
@@ -392,24 +392,23 @@ async function retrieveDataFromClipboard(): Promise<ClipboardData | undefined> {
 /// Read the clipboard and if it contains valid data, create a node from the content.
 async function readNodeFromClipboard() {
   let clipboardData = await retrieveDataFromClipboard()
-  if (clipboardData == undefined) {
+  if (!clipboardData) {
     console.warn('No valid data in clipboard.')
     return
   }
   const copiedNode = clipboardData.nodes[0]
-  if (copiedNode == undefined) {
+  if (!copiedNode) {
     console.warn('No valid node in clipboard.')
     return
   }
-  if (copiedNode.expression != null) {
-    graphStore.createNode(
-      graphNavigator.sceneMousePos ?? Vec2.Zero,
-      copiedNode.expression,
-      copiedNode.metadata,
-    )
-  } else {
+  if (copiedNode.expression == null) {
     console.warn('No valid expression in clipboard.')
   }
+  graphStore.createNode(
+    graphNavigator.sceneMousePos ?? Vec2.Zero,
+    copiedNode.expression,
+    copiedNode.metadata,
+  )
 }
 
 function handleNodeOutputPortDoubleClick(id: ExprId) {
@@ -428,15 +427,14 @@ function handleNodeOutputPortDoubleClick(id: ExprId) {
 </script>
 
 <template>
-  <!-- eslint-disable vue/attributes-order -->
   <div
     ref="viewportNode"
     class="GraphEditor"
     :class="{ draggingEdge: graphStore.unconnectedEdge != null }"
     :style="groupColors"
-    @click="graphBindingsHandler"
     v-on.="graphNavigator.events"
     v-on..="nodeSelection.events"
+    @click="graphBindingsHandler"
     @dragover.prevent
     @drop.prevent="handleFileDrop($event)"
   >
@@ -451,12 +449,12 @@ function handleNodeOutputPortDoubleClick(id: ExprId) {
       ref="componentBrowser"
       :navigator="graphNavigator"
       :position="componentBrowserPosition"
-      @accepted="onComponentBrowserCommit"
-      @closed="onComponentBrowserCancel"
-      @canceled="onComponentBrowserCancel"
       :initialContent="componentBrowserInputContent"
       :initialCaretPosition="graphStore.editedNodeInfo?.range ?? [0, 0]"
       :sourceNode="componentBrowserSourceNode"
+      @accepted="onComponentBrowserCommit"
+      @closed="onComponentBrowserCancel"
+      @canceled="onComponentBrowserCancel"
     />
     <TopBar
       v-model:mode="projectStore.executionMode"

app/gui2/src/stores/graph/__tests__/graphDatabase.test.ts

@@ -38,13 +38,7 @@ test('Reading graph from definition', () => {
     else return undefined
   })
   assert(func?.isTree(Ast.Tree.Type.Function))
-  db.readFunctionAst(
-    func,
-    (_) => {
-      return { x: 0.0, y: 0.0, vis: null }
-    },
-    (_) => 100.0,
-  )
+  db.readFunctionAst(func, (_) => ({ x: 0.0, y: 0.0, vis: null }))
 
   expect(Array.from(db.nodeIdToNode.keys())).toEqual([id04, id08])
   expect(db.getExpressionNodeId(id04)).toBe(id04)

app/gui2/src/stores/graph/graphDatabase.ts

@@ -1,4 +1,3 @@
-import { nonDictatedPlacement } from '@/components/ComponentBrowser/placement'
 import { SuggestionDb, groupColorStyle, type Group } from '@/stores/suggestionDatabase'
 import type { SuggestionEntry } from '@/stores/suggestionDatabase/entry'
 import { byteArraysEqual, tryGetIndex } from '@/util/array'
@@ -8,10 +7,7 @@ import { colorFromString } from '@/util/colors'
 import { ComputedValueRegistry, type ExpressionInfo } from '@/util/computedValueRegistry'
 import { MappedKeyMap, MappedSet } from '@/util/containers'
 import { ReactiveDb, ReactiveIndex, ReactiveMapping } from '@/util/database/reactiveDb'
-import { getTextWidth } from '@/util/measurement'
 import type { Opt } from '@/util/opt'
-import { Rect } from '@/util/rect'
-import theme from '@/util/theme.json'
 import { Vec2 } from '@/util/vec2'
 import * as set from 'lib0/set'
 import type { MethodCall } from 'shared/languageServerTypes'
@@ -243,15 +239,8 @@ export class GraphDb {
   readFunctionAst(
     functionAst: AstExtended<Ast.Tree.Function>,
     getMeta: (id: ExprId) => NodeMetadata | undefined,
-    getWidth: (node: Node) => number = (node: Node) =>
-      // FIXME [ao]: This should take into account the width of all widgets. Probably
-      //   the better solution is to layout nodes once rendered (and all sizes are known).
-      getTextWidth(node.rootSpan.repr(), '11.5px', '"M PLUS 1", sans-serif') * 1.2,
   ) {
     const currentNodeIds = new Set<ExprId>()
-    const nodeRectMap = new Map<ExprId, Rect>()
-    let numberOfUnpositionedNodes = 0
-    let maxUnpositionedNodeWidth = 0
     if (functionAst) {
       for (const nodeAst of functionAst.visit(getFunctionNodeExpressions)) {
         const newNode = nodeFromAst(nodeAst)
@@ -272,20 +261,8 @@ export class GraphDb {
             node.rootSpan = newNode.rootSpan
           }
         }
-        if (!nodeMeta) {
-          numberOfUnpositionedNodes += 1
-          maxUnpositionedNodeWidth = Math.max(maxUnpositionedNodeWidth, getWidth(node ?? newNode))
-        } else {
+        if (nodeMeta) {
           this.assignUpdatedMetadata(node ?? newNode, nodeMeta)
-          nodeRectMap.set(
-            nodeId,
-            Rect.FromBounds(
-              nodeMeta.x,
-              nodeMeta.y,
-              nodeMeta.x + getWidth(node ?? newNode),
-              nodeMeta.y + theme.node.height,
-            ),
-          )
         }
       }
     }
@@ -295,37 +272,8 @@ export class GraphDb {
         this.nodeIdToNode.delete(nodeId)
       }
     }
-    this.bindings.readFunctionAst(functionAst)
 
-    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.nodeIdToNode.keys()) {
-      const meta = getMeta(nodeId)
-      if (meta) continue
-      const node = this.nodeIdToNode.get(nodeId)!
-      const size = new Vec2(getWidth(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
-    }
+    this.bindings.readFunctionAst(functionAst)
   }
 
   assignUpdatedMetadata(node: Node, meta: NodeMetadata) {