Merge pull request #926 from gavin-ts/fix-dagre-unconnected

Fix dagre edge shifting with direction right

d2layouts/d2dagrelayout/layout.go

@@ -300,26 +300,9 @@ func Layout(ctx context.Context, g *d2graph.Graph, opts *ConfigurableOpts) (err
 		// If the edge is connected to two descendants that are about to be downshifted, their whole route gets downshifted
 		movedEdges := make(map[*d2graph.Edge]struct{})
 		for _, e := range g.Edges {
-			currSrc := e.Src
+			isSrcDesc := e.Src.IsDescendantOf(obj)
-			currDst := e.Dst
+			isDstDesc := e.Dst.IsDescendantOf(obj)
 
-			isSrcDesc := false
-			isDstDesc := false
-
-			for currSrc != nil {
-				if currSrc == obj {
-					isSrcDesc = true
-					break
-				}
-				currSrc = currSrc.Parent
-			}
-			for currDst != nil {
-				if currDst == obj {
-					isDstDesc = true
-					break
-				}
-				currDst = currDst.Parent
-			}
 			if isSrcDesc && isDstDesc {
 				stepSize := subtract
 				if e.Src != obj || e.Dst != obj {
@@ -332,45 +315,74 @@ func Layout(ctx context.Context, g *d2graph.Graph, opts *ConfigurableOpts) (err
 			}
 		}
 
-		// Downshift descendents and edges that have one endpoint connected to a descendant
 		q := []*d2graph.Object{obj}
+		// Downshift descendants and edges that have one endpoint connected to a descendant
 		for len(q) > 0 {
 			curr := q[0]
 			q = q[1:]
 
 			stepSize := subtract
 			// The object itself needs to move down the height it was just subtracted
-			// all descendents move half, to maintain vertical padding
+			// all descendants move half, to maintain vertical padding
 			if curr != obj {
 				stepSize /= 2.
 			}
 			curr.TopLeft.Y += stepSize
+			almostEqual := func(a, b float64) bool {
+				return b-1 <= a && a <= b+1
+			}
 			shouldMove := func(p *geo.Point) bool {
 				if curr != obj {
 					return true
 				}
+				if isHorizontal {
+					// Only move horizontal edges if they are connected to the top side of the shrinking container
+					return almostEqual(p.Y, obj.TopLeft.Y-stepSize)
+				} else {
 					// Edge should only move if it's not connected to the bottom side of the shrinking container
-				// Give some margin for error
+					return !almostEqual(p.Y, obj.TopLeft.Y+obj.Height)
-				return !(obj.TopLeft.Y+obj.Height-1 <= p.Y && obj.TopLeft.Y+obj.Height+1 >= p.Y && p.X != obj.TopLeft.X && p.X != (obj.TopLeft.X+obj.Width))
+				}
 			}
 			for _, e := range g.Edges {
 				if _, ok := movedEdges[e]; ok {
 					continue
 				}
+				moveWholeEdge := false
 				if e.Src == curr {
+					// Don't move src points on side of container
+					if almostEqual(e.Route[0].X, obj.TopLeft.X) || almostEqual(e.Route[0].X, obj.TopLeft.X+obj.Width) {
+						// Unless the dst is also on a container
+						if e.Dst.LabelHeight == nil || len(e.Dst.ChildrenArray) <= 0 {
+							continue
+						}
+					}
 					if shouldMove(e.Route[0]) {
+						if isHorizontal && e.Src.Parent != g.Root && e.Dst.Parent != g.Root {
+							moveWholeEdge = true
+						} else {
 							e.Route[0].Y += stepSize
 						}
 					}
-				if e.Dst == curr {
+				}
+				if !moveWholeEdge && e.Dst == curr {
 					if shouldMove(e.Route[len(e.Route)-1]) {
+						if isHorizontal && e.Dst.Parent != g.Root && e.Src.Parent != g.Root {
+							moveWholeEdge = true
+						} else {
 							e.Route[len(e.Route)-1].Y += stepSize
 						}
 					}
 				}
-			for _, c := range curr.ChildrenArray {
+
-				q = append(q, c)
+				if moveWholeEdge {
+					for _, p := range e.Route {
+						p.Y += stepSize / 2.
 					}
+					movedEdges[e] = struct{}{}
+				}
+
+			}
+			q = append(q, curr.ChildrenArray...)
 		}
 	}
 

e2etests/regression_test.go

@@ -582,6 +582,66 @@ b
 `,
 		},
 		loadFromFile(t, "unconnected"),
+		{
+			name: "straight_hierarchy_container_direction_right",
+			script: `
+direction: right
+a
+c
+b
+
+l1: {
+	b
+	a
+	c
+}
+
+b -> l1.b
+a -> l1.a
+c -> l1.c
+
+l2c1: {
+	a
+}
+l1.a -> l2c1.a
+
+l2c3: {
+	c
+}
+l1.c -> l2c3.c
+
+l2c2: {
+	b
+}
+l1.b -> l2c2.b
+
+l3c1: {
+	a
+	b
+}
+l2c1.a -> l3c1.a
+l2c2.b -> l3c1.b
+
+l3c2: {
+	c
+}
+l2c3.c -> l3c2.c
+
+l4: {
+	c1: {
+		a
+	}
+	c2: {
+		b
+	}
+	c3: {
+		c
+	}
+}
+l3c1.a -> l4.c1.a
+l3c1.b -> l4.c2.b
+l3c2.c -> l4.c3.c`,
+		},
 	}
 
 	runa(t, tcs)

e2etests/testdata/regression/unconnected/dagre/board.exp.json

@@ -692,11 +692,11 @@
         },
         {
           "x": 599,
-          "y": 300.2
+          "y": 292
         },
         {
           "x": 639,
-          "y": 333
+          "y": 292
         }
       ],
       "isCurve": true,
@@ -1088,11 +1088,11 @@
         },
         {
           "x": 599,
-          "y": 373.2
+          "y": 365
         },
         {
           "x": 639,
-          "y": 406
+          "y": 365
         }
       ],
       "isCurve": true,
@@ -1136,11 +1136,11 @@
         },
         {
           "x": 599,
-          "y": 499.2
+          "y": 491
         },
         {
           "x": 639,
-          "y": 532
+          "y": 491
         }
       ],
       "isCurve": true,

e2etests/testdata/regression/unconnected/dagre/sketch.exp.svg

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?><svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" preserveAspectRatio="xMidYMid meet" viewBox="0 0 1513 605"><svg id="d2-svg" width="1513" height="605" viewBox="-1 -72 1513 605"><rect x="-1.000000" y="-72.000000" width="1513.000000" height="605.000000" rx="0.000000" class=" fill-N7" stroke-width="0" /><style type="text/css"><![CDATA[
+<?xml version="1.0" encoding="utf-8"?><svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" preserveAspectRatio="xMidYMid meet" viewBox="0 0 1513 597"><svg id="d2-svg" width="1513" height="597" viewBox="-1 -72 1513 597"><rect x="-1.000000" y="-72.000000" width="1513.000000" height="597.000000" rx="0.000000" class=" fill-N7" stroke-width="0" /><style type="text/css"><![CDATA[
 .text {
 	font-family: "font-regular";
 }
@@ -772,7 +772,7 @@
 <li>Staging</li>
 <li>Dispatch to Site</li>
 </ul>
-</div></foreignObject></g></g><g id="Customer Site.Installation"><g class="shape" ><rect x="55.000000" y="60.000000" width="126.000000" height="66.000000" class=" stroke-B1 fill-B5" style="stroke-width:2;" /></g><text x="118.000000" y="98.500000" class="text-bold fill-N1" style="text-anchor:middle;font-size:16px">Installation</text></g><g id="Customer Site.Support"><g class="shape" ><rect x="66.000000" y="186.000000" width="103.000000" height="66.000000" class=" stroke-B1 fill-B5" style="stroke-width:2;" /></g><text x="117.500000" y="224.500000" class="text-bold fill-N1" style="text-anchor:middle;font-size:16px">Support</text></g><g id="(OEM Factory -&gt; OEM Warehouse)[0]"><marker id="mk-3488378134" markerWidth="10.000000" markerHeight="12.000000" refX="7.000000" refY="6.000000" viewBox="0.000000 0.000000 10.000000 12.000000" orient="auto" markerUnits="userSpaceOnUse"> <polygon points="0.000000,0.000000 10.000000,6.000000 0.000000,12.000000" class="connection fill-B1" stroke-width="2" /> </marker><path d="M 186.992796 360.914281 C 225.000000 364.148936 299.500000 365.000000 353.500000 365.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="(OEM Factory -&gt; Distributor Warehouse)[0]"><path d="M 147.318559 389.513396 C 217.202206 470.400000 295.000000 491.000000 331.000000 491.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="(OEM Factory -&gt; Gos Warehouse)[0]"><path d="M 180.810244 321.054933 C 223.809524 298.000000 245.000000 292.000000 260.000000 292.000000 C 275.000000 292.000000 315.400000 292.000000 361.000000 292.000000 C 406.600000 292.000000 599.000000 300.200000 635.906929 330.463682" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="Gos Warehouse.(Master -&gt; Regional-1)[0]"><path d="M 782.450980 384.197777 C 822.569863 414.200000 843.000000 422.000000 879.000000 422.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="Gos Warehouse.(Master -&gt; Regional-2)[0]"><path d="M 780.846867 314.786926 C 822.251351 283.200000 855.000000 275.000000 888.000000 275.000000 C 921.000000 275.000000 1063.000000 277.400000 1099.110451 286.066508" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="Gos Warehouse.(Master -&gt; Regional-N)[0]"><path d="M 763.818430 314.442054 C 818.912863 246.000000 855.000000 228.500000 888.000000 228.500000 C 921.000000 228.500000 965.000000 228.500000 998.000000 228.500000 C 1031.000000 228.500000 1075.000000 228.500000 1108.000000 228.500000 C 1141.000000 228.500000 1284.600000 234.500000 1327.447129 256.662308" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="Gos Warehouse.(Regional-1 -&gt; Regional-2)[0]"><path d="M 1004.836125 395.298037 C 1043.000000 378.818182 1065.000000 366.500000 1109.671799 336.718801" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="Gos Warehouse.(Regional-2 -&gt; Regional-N)[0]"><path d="M 1225.000000 301.500000 C 1263.000000 301.500000 1284.600000 300.500000 1327.014781 296.843553" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="Gos Warehouse.(Regional-N -&gt; Regional-1)[0]"><path d="M 1365.942336 325.097570 C 1291.829114 423.500000 1251.000000 448.500000 1218.000000 448.500000 C 1185.000000 448.500000 1043.000000 446.100000 1006.889549 437.433492" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="(OEM Warehouse -&gt; Gos Warehouse)[0]"><path d="M 519.500000 365.000000 C 574.700000 365.000000 599.000000 373.200000 635.906929 403.463682" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><g id="(Distributor Warehouse -&gt; Gos Warehouse)[0]"><path d="M 541.000000 491.000000 C 579.000000 491.000000 599.000000 499.200000 635.906929 529.463682" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#745835176)" /></g><mask id="745835176" maskUnits="userSpaceOnUse" x="-1" y="-72" width="1513" height="605">
+</div></foreignObject></g></g><g id="Customer Site.Installation"><g class="shape" ><rect x="55.000000" y="60.000000" width="126.000000" height="66.000000" class=" stroke-B1 fill-B5" style="stroke-width:2;" /></g><text x="118.000000" y="98.500000" class="text-bold fill-N1" style="text-anchor:middle;font-size:16px">Installation</text></g><g id="Customer Site.Support"><g class="shape" ><rect x="66.000000" y="186.000000" width="103.000000" height="66.000000" class=" stroke-B1 fill-B5" style="stroke-width:2;" /></g><text x="117.500000" y="224.500000" class="text-bold fill-N1" style="text-anchor:middle;font-size:16px">Support</text></g><g id="(OEM Factory -&gt; OEM Warehouse)[0]"><marker id="mk-3488378134" markerWidth="10.000000" markerHeight="12.000000" refX="7.000000" refY="6.000000" viewBox="0.000000 0.000000 10.000000 12.000000" orient="auto" markerUnits="userSpaceOnUse"> <polygon points="0.000000,0.000000 10.000000,6.000000 0.000000,12.000000" class="connection fill-B1" stroke-width="2" /> </marker><path d="M 186.992796 360.914281 C 225.000000 364.148936 299.500000 365.000000 353.500000 365.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="(OEM Factory -&gt; Distributor Warehouse)[0]"><path d="M 147.318559 389.513396 C 217.202206 470.400000 295.000000 491.000000 331.000000 491.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="(OEM Factory -&gt; Gos Warehouse)[0]"><path d="M 180.810244 321.054933 C 223.809524 298.000000 245.000000 292.000000 260.000000 292.000000 C 275.000000 292.000000 315.400000 292.000000 361.000000 292.000000 C 406.600000 292.000000 599.000000 292.000000 635.000000 292.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="Gos Warehouse.(Master -&gt; Regional-1)[0]"><path d="M 782.450980 384.197777 C 822.569863 414.200000 843.000000 422.000000 879.000000 422.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="Gos Warehouse.(Master -&gt; Regional-2)[0]"><path d="M 780.846867 314.786926 C 822.251351 283.200000 855.000000 275.000000 888.000000 275.000000 C 921.000000 275.000000 1063.000000 277.400000 1099.110451 286.066508" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="Gos Warehouse.(Master -&gt; Regional-N)[0]"><path d="M 763.818430 314.442054 C 818.912863 246.000000 855.000000 228.500000 888.000000 228.500000 C 921.000000 228.500000 965.000000 228.500000 998.000000 228.500000 C 1031.000000 228.500000 1075.000000 228.500000 1108.000000 228.500000 C 1141.000000 228.500000 1284.600000 234.500000 1327.447129 256.662308" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="Gos Warehouse.(Regional-1 -&gt; Regional-2)[0]"><path d="M 1004.836125 395.298037 C 1043.000000 378.818182 1065.000000 366.500000 1109.671799 336.718801" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="Gos Warehouse.(Regional-2 -&gt; Regional-N)[0]"><path d="M 1225.000000 301.500000 C 1263.000000 301.500000 1284.600000 300.500000 1327.014781 296.843553" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="Gos Warehouse.(Regional-N -&gt; Regional-1)[0]"><path d="M 1365.942336 325.097570 C 1291.829114 423.500000 1251.000000 448.500000 1218.000000 448.500000 C 1185.000000 448.500000 1043.000000 446.100000 1006.889549 437.433492" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="(OEM Warehouse -&gt; Gos Warehouse)[0]"><path d="M 519.500000 365.000000 C 574.700000 365.000000 599.000000 365.000000 635.000000 365.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><g id="(Distributor Warehouse -&gt; Gos Warehouse)[0]"><path d="M 541.000000 491.000000 C 579.000000 491.000000 599.000000 491.000000 635.000000 491.000000" fill="none" class="connection stroke-B1" style="stroke-width:2;" marker-end="url(#mk-3488378134)" mask="url(#319647009)" /></g><mask id="319647009" maskUnits="userSpaceOnUse" x="-1" y="-72" width="1513" height="597">
-<rect x="-1" y="-72" width="1513" height="605" fill="white"></rect>
+<rect x="-1" y="-72" width="1513" height="597" fill="white"></rect>
 
 </mask></svg></svg>

e2etests/testdata/stable/different_subgraphs/dagre/board.exp.json

@@ -1414,7 +1414,7 @@
       "route": [
         {
           "x": 1112.75,
-          "y": 385
+          "y": 344
         },
         {
           "x": 1302.75,
@@ -1549,7 +1549,7 @@
       "route": [
         {
           "x": 1112.75,
-          "y": 407
+          "y": 366
         },
         {
           "x": 1160.75,

e2etests/testdata/stable/direction/dagre/board.exp.json

@@ -695,7 +695,7 @@
       "route": [
         {
           "x": 297.75,
-          "y": 464
+          "y": 423
         },
         {
           "x": 337.75,
@@ -878,7 +878,7 @@
       "route": [
         {
           "x": 163.5,
-          "y": 637.5
+          "y": 601.5
         },
         {
           "x": 204.5,