From f0445b74d17106509b681f7c521c00a24a081e19 Mon Sep 17 00:00:00 2001 From: Knut Sveidqvist Date: Thu, 12 Jun 2025 13:33:05 +0200 Subject: [PATCH] #6646 Tidy-tree layout in place for Mindmaps --- cypress/platform/knsv2.html | 79 ++++---- .../layout-algorithms/cose-bilkent/render.ts | 6 +- .../tidy-tree/layout.test.ts | 106 +++++++++- .../layout-algorithms/tidy-tree/layout.ts | 183 ++++++++++++------ .../layout-algorithms/tidy-tree/render.ts | 2 +- 5 files changed, 278 insertions(+), 98 deletions(-) diff --git a/cypress/platform/knsv2.html b/cypress/platform/knsv2.html index 689f20ed2..38ac97b45 100644 --- a/cypress/platform/knsv2.html +++ b/cypress/platform/knsv2.html @@ -105,7 +105,7 @@ - 
+    
       ---
       config:
         layout: tidy-tree
@@ -135,31 +135,25 @@
         layout: tidy-tree
       ---
       mindmap
-      root((mindmap))
+      root((mindmap is a long thing))
         A
-
         B
-          a
-          b
-          c
-          d
-          a1
-          a2
         C
-          e
-          f
-          g
-          h
-          i
         D
-         q1
-         q2
-         I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on
-         q3
-         q4
-
-    

-    
+      

+    
+      ---
+      config:
+        layout: tidy-tree
+      ---
+      mindmap
+      root((mindmap))
+        A
+        B
+      

+    
       ---
       config:
         layout: tidy-tree
@@ -167,22 +161,37 @@
       mindmap
       root((mindmap))
         A
-        Origins
-        Tools
-          e
-        Third
-          q
-          w
-          e
-          r
-          t
-          y
-        Forth
           a
+            apa[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
+            apa2[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
           b
+          c
+          d
+        B
+            apa3[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
+        D
+          apa5[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
+          apa4[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
 
     

-    
+
+    
+      ---
+      config:
+        layout: tidy-tree
+      ---
+      mindmap
+      root((mindmap))
+        a
+          apa[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
+          apa2[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
+        b
+          apa3[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
+          apa4[I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on. I am a long long multline string passing several levels of text. Lorum ipsum and so on]
+
+
+    

+    
       ---
       config:
         layout: tidy-tree
@@ -210,7 +219,7 @@
         layout: dagre
       ---
       mindmap
-      root((mindmap))
+      root((mindmap is a long thing))
         Origins
           Long history
           ::icon(fa fa-book)
diff --git a/packages/mermaid/src/rendering-util/layout-algorithms/cose-bilkent/render.ts b/packages/mermaid/src/rendering-util/layout-algorithms/cose-bilkent/render.ts
index e63b66691..b95d92360 100644
--- a/packages/mermaid/src/rendering-util/layout-algorithms/cose-bilkent/render.ts
+++ b/packages/mermaid/src/rendering-util/layout-algorithms/cose-bilkent/render.ts
@@ -29,7 +29,7 @@ export const render = async (
     log,
     positionEdgeLabel,
   }: InternalHelpers,
-  { algorithm }: RenderOptions
+  { algorithm: _algorithm }: RenderOptions
 ) => {
   const nodeDb: Record = {};
   const clusterDb: Record = {};
@@ -121,7 +121,7 @@ export const render = async (
   await Promise.all(
     data4Layout.edges.map(async (edge) => {
       // Insert edge label first
-      const edgeLabel = await insertEdgeLabel(edgeLabels, edge);
+      const _edgeLabel = await insertEdgeLabel(edgeLabels, edge);
 
       // Get start and end nodes
       const startNode = nodeDb[edge.start];
@@ -132,7 +132,7 @@ export const render = async (
         const positionedEdge = layoutResult.edges.find((e) => e.id === edge.id);
 
         if (positionedEdge) {
-          console.debug('APA01 positionedEdge', positionedEdge);
+          log.debug('APA01 positionedEdge', positionedEdge);
           // Create edge path with positioned coordinates
           const edgeWithPath = {
             ...edge,
diff --git a/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/layout.test.ts b/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/layout.test.ts
index ba7bcf5d1..932c83b43 100644
--- a/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/layout.test.ts
+++ b/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/layout.test.ts
@@ -9,7 +9,7 @@ vi.mock('non-layered-tidy-tree-layout', () => ({
       const result = { ...treeData };
 
       // Set positions for the virtual root (if it exists)
-      if (result.id && result.id.toString().startsWith('virtual-root')) {
+      if (result.id?.toString().startsWith('virtual-root')) {
         result.x = 0;
         result.y = 0;
       } else {
@@ -322,5 +322,109 @@ describe('Tidy-Tree Layout Algorithm', () => {
       expect(child2!.x).toBeGreaterThan(100); // Should be significantly right of center
       expect(child4!.x).toBeGreaterThan(100); // Should be significantly right of center
     });
+
+    it('should correctly transpose coordinates to prevent high nodes from covering nodes above them', async () => {
+      // Create a test case with nodes that have different heights to test transposition
+      const testData = {
+        ...mockLayoutData,
+        nodes: [
+          {
+            id: 'root',
+            label: 'Root',
+            isGroup: false,
+            shape: 'rect' as const,
+            width: 100,
+            height: 50,
+            padding: 10,
+            x: 0,
+            y: 0,
+            cssClasses: '',
+            cssStyles: [],
+            look: 'default',
+          },
+          {
+            id: 'tall-child',
+            label: 'Tall Child',
+            isGroup: false,
+            shape: 'rect' as const,
+            width: 80,
+            height: 120, // Tall node
+            padding: 10,
+            x: 0,
+            y: 0,
+            cssClasses: '',
+            cssStyles: [],
+            look: 'default',
+          },
+          {
+            id: 'short-child',
+            label: 'Short Child',
+            isGroup: false,
+            shape: 'rect' as const,
+            width: 80,
+            height: 30, // Short node
+            padding: 10,
+            x: 0,
+            y: 0,
+            cssClasses: '',
+            cssStyles: [],
+            look: 'default',
+          },
+        ],
+        edges: [
+          {
+            id: 'root_tall',
+            start: 'root',
+            end: 'tall-child',
+            type: 'edge',
+            classes: '',
+            style: [],
+            animate: false,
+            arrowTypeEnd: 'arrow_point',
+            arrowTypeStart: 'none',
+          },
+          {
+            id: 'root_short',
+            start: 'root',
+            end: 'short-child',
+            type: 'edge',
+            classes: '',
+            style: [],
+            animate: false,
+            arrowTypeEnd: 'arrow_point',
+            arrowTypeStart: 'none',
+          },
+        ],
+      };
+
+      const result = await executeTidyTreeLayout(testData, mockConfig);
+
+      expect(result).toBeDefined();
+      expect(result.nodes).toHaveLength(3); // root + 2 children
+
+      // Find all nodes
+      const rootNode = result.nodes.find((node) => node.id === 'root');
+      const tallChild = result.nodes.find((node) => node.id === 'tall-child');
+      const shortChild = result.nodes.find((node) => node.id === 'short-child');
+
+      expect(rootNode).toBeDefined();
+      expect(tallChild).toBeDefined();
+      expect(shortChild).toBeDefined();
+
+      // Verify that nodes are positioned correctly with proper transposition
+      // The tall child and short child should be on opposite sides
+      expect(tallChild!.x).not.toBe(shortChild!.x); // Should be on different sides
+
+      // Verify that the original dimensions are preserved (not transposed in final output)
+      expect(tallChild!.width).toBe(80); // Original width preserved
+      expect(tallChild!.height).toBe(120); // Original height preserved
+      expect(shortChild!.width).toBe(80); // Original width preserved
+      expect(shortChild!.height).toBe(30); // Original height preserved
+
+      // Verify that nodes don't overlap vertically (the transposition fix)
+      // Both children should have reasonable Y positions that don't cause overlap
+      const yDifference = Math.abs(tallChild!.y - shortChild!.y);
+      expect(yDifference).toBeGreaterThanOrEqual(0); // Should have proper vertical spacing
+    });
   });
 });
diff --git a/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/layout.ts b/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/layout.ts
index c6d09cebf..1e8a37b8e 100644
--- a/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/layout.ts
+++ b/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/layout.ts
@@ -19,7 +19,7 @@ export function executeTidyTreeLayout(
   data: LayoutData,
   _config: MermaidConfig
 ): Promise {
-  log.debug('Starting tidy-tree layout algorithm');
+  log.debug('APA01 Starting tidy-tree layout algorithm');
 
   return new Promise((resolve, reject) => {
     try {
@@ -33,13 +33,15 @@ export function executeTidyTreeLayout(
       }
 
       // Find the maximum number of children any one node have expect for the root node
-      const maxChildren = Math.max(...data.nodes.map((node) => node.children?.length ?? 0));
+      const _maxChildren = Math.max(...data.nodes.map((node) => node.children?.length ?? 0));
 
       // Convert layout data to dual-tree format (left and right trees)
       const { leftTree, rightTree, rootNode } = convertToDualTreeFormat(data);
 
-      // Configure tidy-tree layout
-      const gap = 20; // Vertical gap between nodes
+      // Configure tidy-tree layout with dynamic gap based on node heights
+      // Since we transpose coordinates, the gap becomes horizontal spacing in final layout
+      // We need to ensure enough space for the tallest nodes
+      const gap = 20; // Math.max(20, maxNodeHeight + 20); // Ensure minimum gap plus node height
       const bottomPadding = 40; // Horizontal gap between levels
       intersectionShift = 30;
 
@@ -58,8 +60,10 @@ export function executeTidyTreeLayout(
       let rightBoundingBox = null;
 
       if (leftTree) {
+        // log.debug('APA01 Left tree before layout', leftTree.children[0]?.children[0]);
         const leftLayoutResult = layout.layout(leftTree);
         leftResult = leftLayoutResult.result;
+        log.debug('APA01 Left tree before layout', JSON.stringify(leftResult, null, 2));
         leftBoundingBox = leftLayoutResult.boundingBox;
       }
 
@@ -128,11 +132,13 @@ function convertToDualTreeFormat(data: LayoutData): {
     const parentId = edge.start;
     const childId = edge.end;
 
-    if (!children.has(parentId)) {
-      children.set(parentId, []);
+    if (parentId && childId) {
+      if (!children.has(parentId)) {
+        children.set(parentId, []);
+      }
+      children.get(parentId)!.push(childId);
+      parents.set(childId, parentId);
     }
-    children.get(parentId)!.push(childId);
-    parents.set(childId, parentId);
   });
 
   // Find root node (node with no parent)
@@ -241,7 +247,7 @@ function combineAndPositionTrees(
 ): PositionedNode[] {
   const positionedNodes: PositionedNode[] = [];
 
-  console.log('combineAndPositionTrees', {
+  log.debug('combineAndPositionTrees', {
     leftResult,
     rightResult,
   });
@@ -251,7 +257,7 @@ function combineAndPositionTrees(
   const rootY = 0;
 
   // Calculate spacing between trees
-  const treeSpacing = 150; // Horizontal spacing from root to tree
+  const treeSpacing = rootNode.width / 2 + 30; // Horizontal spacing from root to tree
 
   // First, collect node positions for each tree separately
   const leftTreeNodes: PositionedNode[] = [];
@@ -287,8 +293,13 @@ function combineAndPositionTrees(
     const firstLevelLeftNodes = leftTreeNodes.filter((node) => node.x === firstLevelLeftX);
 
     if (firstLevelLeftNodes.length > 0) {
-      const leftMinY = Math.min(...firstLevelLeftNodes.map((node) => node.y));
-      const leftMaxY = Math.max(...firstLevelLeftNodes.map((node) => node.y));
+      // Calculate bounding box considering node heights
+      const leftMinY = Math.min(
+        ...firstLevelLeftNodes.map((node) => node.y - (node.height ?? 50) / 2)
+      );
+      const leftMaxY = Math.max(
+        ...firstLevelLeftNodes.map((node) => node.y + (node.height ?? 50) / 2)
+      );
       leftTreeCenterY = (leftMinY + leftMaxY) / 2;
     }
   }
@@ -303,8 +314,13 @@ function combineAndPositionTrees(
     const firstLevelRightNodes = rightTreeNodes.filter((node) => node.x === firstLevelRightX);
 
     if (firstLevelRightNodes.length > 0) {
-      const rightMinY = Math.min(...firstLevelRightNodes.map((node) => node.y));
-      const rightMaxY = Math.max(...firstLevelRightNodes.map((node) => node.y));
+      // Calculate bounding box considering node heights
+      const rightMinY = Math.min(
+        ...firstLevelRightNodes.map((node) => node.y - (node.height ?? 50) / 2)
+      );
+      const rightMaxY = Math.max(
+        ...firstLevelRightNodes.map((node) => node.y + (node.height ?? 50) / 2)
+      );
       rightTreeCenterY = (rightMinY + rightMaxY) / 2;
     }
   }
@@ -317,7 +333,7 @@ function combineAndPositionTrees(
   positionedNodes.push({
     id: String(rootNode.id),
     x: rootX,
-    y: rootY, // Root stays at center
+    y: rootY + 20, // Root stays at center
     section: 'root',
     width: rootNode._originalNode?.width || rootNode.width,
     height: rootNode._originalNode?.height || rootNode.height,
@@ -325,30 +341,31 @@ function combineAndPositionTrees(
   });
 
   // Add left tree nodes with their specific offset
-  leftTreeNodes.forEach((node) => {
-    positionedNodes.push({
-      id: node.id,
-      x: node.x,
-      y: node.y + leftTreeOffset,
-      section: 'left',
-      width: node.width,
-      height: node.height,
-      originalNode: node.originalNode,
-    });
-  });
+  const leftTreeNodesWithOffset = leftTreeNodes.map((node) => ({
+    id: node.id,
+    x: node.x - node.width / 2,
+    y: node.y + leftTreeOffset + node.height / 2,
+    section: 'left' as const,
+    width: node.width,
+    height: node.height,
+    originalNode: node.originalNode,
+  }));
 
   // Add right tree nodes with their specific offset
-  rightTreeNodes.forEach((node) => {
-    positionedNodes.push({
-      id: node.id,
-      x: node.x,
-      y: node.y + rightTreeOffset,
-      section: 'right',
-      width: node.width,
-      height: node.height,
-      originalNode: node.originalNode,
-    });
-  });
+  const rightTreeNodesWithOffset = rightTreeNodes.map((node) => ({
+    id: node.id,
+    x: node.x + node.width / 2,
+    y: node.y + rightTreeOffset + node.height / 2,
+    section: 'right' as const,
+    width: node.width,
+    height: node.height,
+    originalNode: node.originalNode,
+  }));
+
+  // Add all nodes to the final result
+  // The tidy-tree algorithm should handle proper spacing, so no additional collision detection needed
+  positionedNodes.push(...leftTreeNodesWithOffset);
+  positionedNodes.push(...rightTreeNodesWithOffset);
 
   return positionedNodes;
 }
@@ -364,18 +381,25 @@ function positionLeftTreeBidirectional(
   offsetY: number
 ): void {
   nodes.forEach((node) => {
-    // For left tree: transpose the tidy-tree coordinates
-    // Tidy-tree Y becomes our X distance from root (grows left)
-    // Tidy-tree X becomes our Y position (tree levels) - this gives proper spacing
-    const distanceFromRoot = node.y ?? 0; // How far left from root
-    const treeLevel = node.x ?? 0; // Use X coordinate for tree level (proper vertical spacing)
+    // For left tree: transpose the tidy-tree coordinates correctly
+    // Tidy-tree X (tree levels going down) becomes our Y position (vertical spacing)
+    // Tidy-tree Y (sibling spacing) becomes our X distance from root (grows left)
+    const distanceFromRoot = node.y ?? 0; // Horizontal distance from root (grows left)
+    const verticalPosition = node.x ?? 0; // Vertical position (tree levels)
+
+    // Get the original node dimensions directly from the stored original node
+    const originalWidth = node._originalNode?.width ?? 100;
+    const originalHeight = node._originalNode?.height ?? 50;
+
+    // Use the vertical position as calculated by the tidy-tree algorithm
+    const adjustedY = offsetY + verticalPosition;
 
     positionedNodes.push({
       id: String(node.id),
       x: offsetX - distanceFromRoot, // Negative to grow left from root
-      y: offsetY + treeLevel, // Use tidy-tree's Y as Y (tree levels)
-      width: node._originalNode?.width || node.width,
-      height: node._originalNode?.height || node.height,
+      y: adjustedY, // Vertical position based on tree level
+      width: originalWidth,
+      height: originalHeight,
       originalNode: node._originalNode,
     });
 
@@ -396,18 +420,25 @@ function positionRightTreeBidirectional(
   offsetY: number
 ): void {
   nodes.forEach((node) => {
-    // For right tree: transpose the tidy-tree coordinates
-    // Tidy-tree Y becomes our X distance from root (grows right)
-    // Tidy-tree X becomes our Y position (tree levels) - this gives proper spacing
-    const distanceFromRoot = node.y ?? 0; // How far right from root
-    const treeLevel = node.x ?? 0; // Use X coordinate for tree level (proper vertical spacing)
+    // For right tree: transpose the tidy-tree coordinates correctly
+    // Tidy-tree X (tree levels going down) becomes our Y position (vertical spacing)
+    // Tidy-tree Y (sibling spacing) becomes our X distance from root (grows right)
+    const distanceFromRoot = node.y ?? 0; // Horizontal distance from root (grows right)
+    const verticalPosition = node.x ?? 0; // Vertical position (tree levels)
+
+    // Get the original node dimensions directly from the stored original node
+    const originalWidth = node._originalNode?.width ?? 100;
+    const originalHeight = node._originalNode?.height ?? 50;
+
+    // Use the vertical position as calculated by the tidy-tree algorithm
+    const adjustedY = offsetY + verticalPosition;
 
     positionedNodes.push({
       id: String(node.id),
       x: offsetX + distanceFromRoot, // Positive to grow right from root
-      y: offsetY + treeLevel, // Use tidy-tree's Y as Y (tree levels)
-      width: node._originalNode?.width || node.width,
-      height: node._originalNode?.height || node.height,
+      y: adjustedY, // Vertical position based on tree level
+      width: originalWidth,
+      height: originalHeight,
       originalNode: node._originalNode,
     });
 
@@ -434,15 +465,51 @@ function calculateEdgePositions(
   return edges.map((edge) => {
     const sourceNode = nodeInfo.get(edge.start ?? '');
     const targetNode = nodeInfo.get(edge.end ?? '');
-    console.debug('APA01 calculateEdgePositions', edge, sourceNode, 'targetNode', targetNode);
+    log.debug('APA01 calculateEdgePositions', edge, sourceNode, 'targetNode', targetNode);
 
     if (!sourceNode) {
-      console.error('APA01 Source node not found for edge', edge);
-      return;
+      log.error('APA01 Source node not found for edge', edge);
+      // Return a default edge instead of undefined
+      return {
+        id: edge.id,
+        source: edge.start ?? '',
+        target: edge.end ?? '',
+        startX: 0,
+        startY: 0,
+        midX: 0,
+        midY: 0,
+        endX: 0,
+        endY: 0,
+        points: [{ x: 0, y: 0 }],
+        sourceSection: undefined,
+        targetSection: undefined,
+        sourceWidth: undefined,
+        sourceHeight: undefined,
+        targetWidth: undefined,
+        targetHeight: undefined,
+      };
     }
     if (targetNode === undefined) {
-      console.error('APA01 Target node not found for edge', edge);
-      return;
+      log.error('APA01 Target node not found for edge', edge);
+      // Return a default edge instead of undefined
+      return {
+        id: edge.id,
+        source: edge.start ?? '',
+        target: edge.end ?? '',
+        startX: 0,
+        startY: 0,
+        midX: 0,
+        midY: 0,
+        endX: 0,
+        endY: 0,
+        points: [{ x: 0, y: 0 }],
+        sourceSection: undefined,
+        targetSection: undefined,
+        sourceWidth: undefined,
+        sourceHeight: undefined,
+        targetWidth: undefined,
+        targetHeight: undefined,
+      };
     }
 
     // Fallback positions if nodes not found
diff --git a/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/render.ts b/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/render.ts
index c5ca8767c..3a6256ab4 100644
--- a/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/render.ts
+++ b/packages/mermaid/src/rendering-util/layout-algorithms/tidy-tree/render.ts
@@ -152,7 +152,7 @@ export const render = async (
         const positionedEdge = layoutResult.edges.find((e) => e.id === edge.id);
 
         if (positionedEdge) {
-          console.debug('APA01 positionedEdge', positionedEdge);
+          log.debug('APA01 positionedEdge', positionedEdge);
           // Create edge path with positioned coordinates
           const edgeWithPath = {
             ...edge,