[3604] Improves overlap management performance during arrange all

Bug: #3604 Signed-off-by: Pierre-Charles David <[email protected]> Signed-off-by: Florian ROUËNÉ <[email protected]>
eclipse-sirius · Jun 19, 2024 · 9371720 · 9371720
1 parent da99868
commit 9371720
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 60 deletions.
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -99,6 +99,7 @@ image:doc/screenshots/insideLabelPositions.png[Inside label positions, 70%]
 - https://github.com/eclipse-sirius/sirius-web/issues/3627[#3627] [form] Remove unused mutation in form
 - https://github.com/eclipse-sirius/sirius-web/issues/3606[#3606] [test] Improve error handling in ExecuteEditingContextFunctionRunner and ExecuteEditingContextFunctionEventHandler
 - https://github.com/eclipse-sirius/sirius-web/issues/3561[#3561] [diagram] Add support for background and border on diagram labels
+- https://github.com/eclipse-sirius/sirius-web/issues/3604[#3604] [diagram] Make node overlap resolution faster during "Arrange All"
 
 == v2024.5.0
 

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/edge/EdgeLayout.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/edge/EdgeLayout.ts
@@ -189,8 +189,8 @@ export const getNodeCenter: GetNodeCenter = (node, visiblesNodes) => {
     };
     while (parentNode) {
       position = {
-        x: position.x + parentNode.position?.x ?? 0,
-        y: position.y + parentNode.position?.y ?? 0,
+        x: position.x + (parentNode.position?.x ?? 0),
+        y: position.y + (parentNode.position?.y ?? 0),
       };
       let parentNodeId = parentNode.parentNode ?? '';
       parentNode = visiblesNodes.find((nodeParent) => nodeParent.id === parentNodeId);

diff --git a/...diagrams/frontend/sirius-components-diagrams/src/renderer/helper-lines/useHelperLines.tsx b/...diagrams/frontend/sirius-components-diagrams/src/renderer/helper-lines/useHelperLines.tsx
@@ -283,7 +283,7 @@ export const useHelperLines = (): UseHelperLinesValue => {
             while (parentNode) {
               snapOffsetX -= parentNode.position.x;
               snapOffsetY -= parentNode.position.y;
-              parentNode = getNodes().find((node) => node.id === parentNode?.parentNode ?? '');
+              parentNode = getNodes().find((node) => node.id === (parentNode?.parentNode ?? ''));
             }
             if (helperLines.snapX && change.position) {
               change.position.x = helperLines.snapX + snapOffsetX;
@@ -324,7 +324,7 @@ export const useHelperLines = (): UseHelperLinesValue => {
             while (parentNode) {
               snapOffsetX -= parentNode.position.x;
               snapOffsetY -= parentNode.position.y;
-              parentNode = getNodes().find((node) => node.id === parentNode?.parentNode ?? '');
+              parentNode = getNodes().find((node) => node.id === (parentNode?.parentNode ?? ''));
             }
             if (
               helperLines.snapX &&

diff --git a/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/overlap/useOverlap.ts b/packages/diagrams/frontend/sirius-components-diagrams/src/renderer/overlap/useOverlap.ts
@@ -11,80 +11,62 @@
  *     Obeo - initial API and implementation
  *******************************************************************************/
 import { useCallback, useState } from 'react';
-import { Node, Dimensions, XYPosition } from 'reactflow';
+import { Node } from 'reactflow';
 import { UseOverlapValue } from './useOverlap.types';
 
-type Rect = Dimensions & XYPosition;
-
-const getOverlappingArea = (rectA: Rect, rectB: Rect): number => {
-  const xOverlap = Math.max(0, Math.min(rectA.x + rectA.width, rectB.x + rectB.width) - Math.max(rectA.x, rectB.x));
-  const yOverlap = Math.max(0, Math.min(rectA.y + rectA.height, rectB.y + rectB.height) - Math.max(rectA.y, rectB.y));
-
-  return Math.ceil(xOverlap * yOverlap);
-};
-
-const nodeToRect = (node: Node): Rect => {
-  const { position } = node;
-  return {
-    ...position,
-    width: node.width ?? 0,
-    height: node.height ?? 0,
-  };
+const nodesOverlap = (nodeA: Node, nodeB: Node): boolean => {
+  const topLeftX = Math.max(nodeA.position.x, nodeB.position.x);
+  const topLeftY = Math.max(nodeA.position.y, nodeB.position.y);
+  const bottomRightX = Math.min(nodeA.position.x + (nodeA.width ?? 0), nodeB.position.x + (nodeB.width ?? 0));
+  const bottomRightY = Math.min(nodeA.position.y + (nodeA.height ?? 0), nodeB.position.y + (nodeB.height ?? 0));
+  return topLeftX < bottomRightX && topLeftY < bottomRightY;
 };
 
 const getIntersectingNodes = (node: Node, nodes: Node[]): Node[] => {
-  const nodeRect = nodeToRect(node);
-  if (!nodeRect) {
-    return [];
-  }
   return nodes.filter((n) => {
-    if (n.id === node.id) {
+    if (n.parentNode !== node.parentNode || n.id === node.id) {
       return false;
     }
-    const currNodeRect = nodeToRect(n);
-    const overlappingArea = getOverlappingArea(currNodeRect, nodeRect);
-    return overlappingArea > 0;
+    return nodesOverlap(node, n);
   });
 };
 
 export const useOverlap = (): UseOverlapValue => {
   const [enabled, setEnabled] = useState<boolean>(true);
 
   const applyOverlap = (movingNode: Node, nodes: Node[], direction: 'horizontal' | 'vertical' = 'horizontal'): void => {
-    getIntersectingNodes(movingNode, nodes)
-      .filter((n) => n.parentNode === movingNode.parentNode)
-      .forEach((node) => {
-        const overlapNode = nodes.find((n) => n.id === node.id);
-        if (overlapNode) {
-          if (overlapNode.data.pinned) {
-            if (direction === 'horizontal') {
-              movingNode.position = {
-                ...movingNode.position,
-                x: overlapNode.position.x + (overlapNode.width ?? 0) + 40,
-              };
-            } else {
-              movingNode.position = {
-                ...movingNode.position,
-                y: overlapNode.position.y + (overlapNode.height ?? 0) + 40,
-              };
-            }
-            applyOverlap(movingNode, nodes, direction);
+    getIntersectingNodes(movingNode, nodes).forEach((node) => {
+      const overlapNode = nodes.find((n) => n.id === node.id);
+      if (overlapNode) {
+        if (overlapNode.data.pinned) {
+          if (direction === 'horizontal') {
+            movingNode.position = {
+              ...movingNode.position,
+              x: overlapNode.position.x + (overlapNode.width ?? 0) + 40,
+            };
+          } else {
+            movingNode.position = {
+              ...movingNode.position,
+              y: overlapNode.position.y + (overlapNode.height ?? 0) + 40,
+            };
+          }
+          applyOverlap(movingNode, nodes, direction);
+        } else {
+          if (direction === 'horizontal') {
+            overlapNode.position = {
+              ...overlapNode.position,
+              x: movingNode.position.x + (movingNode.width ?? 0) + 40,
+            };
           } else {
-            if (direction === 'horizontal') {
-              overlapNode.position = {
-                ...overlapNode.position,
-                x: movingNode.position.x + (movingNode.width ?? 0) + 40,
-              };
-            } else {
-              overlapNode.position = {
-                ...overlapNode.position,
-                y: movingNode.position.y + (movingNode.height ?? 0) + 40,
-              };
-            }
-            applyOverlap(overlapNode, nodes, direction);
+            overlapNode.position = {
+              ...overlapNode.position,
+              y: movingNode.position.y + (movingNode.height ?? 0) + 40,
+            };
           }
+          applyOverlap(overlapNode, nodes, direction);
         }
-      });
+      }
+    });
   };
 
   const resolveNodeOverlap = useCallback(