diff --git a/d2layouts/d2sequence/layout.go b/d2layouts/d2sequence/layout.go index 85a9dfcc5..25f299ce2 100644 --- a/d2layouts/d2sequence/layout.go +++ b/d2layouts/d2sequence/layout.go @@ -17,34 +17,29 @@ import ( // Once all nodes were processed, it continues to run the layout engine without the sequence diagram nodes and edges. // Then it restores all objects with their proper layout engine and sequence diagram positions func Layout(ctx context.Context, g *d2graph.Graph, layout func(ctx context.Context, g *d2graph.Graph) error) error { - // keeps the current graph state - oldObjects := g.Objects - oldEdges := g.Edges - // flag objects to keep to avoid having to flag all descendants of sequence diagram to be removed objectsToKeep := make(map[*d2graph.Object]struct{}) // edges flagged to be removed (these are internal edges of the sequence diagrams) edgesToRemove := make(map[*d2graph.Edge]struct{}) // store the sequence diagram related to a given node - sequenceDiagrams := make(map[*d2graph.Object]*sequenceDiagram) + sequenceDiagrams := make(map[string]*sequenceDiagram) // keeps the reference of the children of a given node - objChildrenArray := make(map[*d2graph.Object][]*d2graph.Object) + childrenReplacement := make(map[string][]*d2graph.Object) // goes from root and travers all descendants - queue := make([]*d2graph.Object, 1, len(oldObjects)) + queue := make([]*d2graph.Object, 1, len(g.Objects)) queue[0] = g.Root for len(queue) > 0 { obj := queue[0] queue = queue[1:] - // root is not part of g.Objects, so we can't add it here objectsToKeep[obj] = struct{}{} if obj.Attributes.Shape.Value == d2target.ShapeSequenceDiagram { // TODO: should update obj.References too? // clean current children and keep a backup to restore them later obj.Children = make(map[string]*d2graph.Object) - objChildrenArray[obj] = obj.ChildrenArray + children := obj.ChildrenArray obj.ChildrenArray = nil // creates a mock rectangle so that layout considers this size sdMock := obj.EnsureChild([]string{"sequence_diagram"}) @@ -62,36 +57,47 @@ func Layout(ctx context.Context, g *d2graph.Graph, layout func(ctx context.Conte } } - sd := newSequenceDiagram(objChildrenArray[obj], edges) + sd := newSequenceDiagram(children, edges) sd.layout() sdMock.Box = geo.NewBox(nil, sd.getWidth(), sd.getHeight()) - sequenceDiagrams[obj] = sd + sequenceDiagrams[sdMock.AbsID()] = sd + childrenReplacement[sdMock.AbsID()] = children } else { // only move to children if the parent is not a sequence diagram queue = append(queue, obj.ChildrenArray...) } } + if len(sequenceDiagrams) == 0 { + return layout(ctx, g) + } + // removes the edges - newEdges := make([]*d2graph.Edge, 0, len(g.Edges)-len(edgesToRemove)) + layoutEdges := make([]*d2graph.Edge, 0, len(g.Edges)-len(edgesToRemove)) + sequenceDiagramEdges := make([]*d2graph.Edge, 0, len(edgesToRemove)) for _, edge := range g.Edges { if _, exists := edgesToRemove[edge]; !exists { - newEdges = append(newEdges, edge) + layoutEdges = append(layoutEdges, edge) + } else { + sequenceDiagramEdges = append(sequenceDiagramEdges, edge) } } + g.Edges = layoutEdges // done this way (by flagging objects) instead of appending to `queue` // because appending in that order would change the order of g.Objects which // could lead to layout changes (as the order of the objects might be important for the underlying engine) - newObjects := make([]*d2graph.Object, 0, len(objectsToKeep)) + layoutObjects := make([]*d2graph.Object, 0, len(objectsToKeep)) + sequenceDiagramObjects := make([]*d2graph.Object, 0, len(g.Objects)-len(objectsToKeep)) for _, obj := range g.Objects { if _, exists := objectsToKeep[obj]; exists { - newObjects = append(newObjects, obj) + layoutObjects = append(layoutObjects, obj) + } else { + sequenceDiagramObjects = append(sequenceDiagramObjects, obj) } } + g.Objects = layoutObjects - g.Objects = newObjects - g.Edges = newEdges if g.Root.Attributes.Shape.Value == d2target.ShapeSequenceDiagram { // don't need to run the layout engine if the root is a sequence diagram g.Root.ChildrenArray[0].TopLeft = geo.NewPoint(0, 0) @@ -99,25 +105,36 @@ func Layout(ctx context.Context, g *d2graph.Graph, layout func(ctx context.Conte return err } - // restores objects & edges - g.Edges = oldEdges - g.Objects = oldObjects + g.Edges = append(g.Edges, sequenceDiagramEdges...) + + // restores objects + // it must be this way because the objects pointer reference is lost when calling layout engines compiled in binaries + allObjects := sequenceDiagramObjects + for _, obj := range g.Objects { + if _, exists := sequenceDiagrams[obj.AbsID()]; !exists { + allObjects = append(allObjects, obj) + continue + } + // we don't want `sdMock` in `g.Objects` because they shouldn't be rendered + + // just to make it easier to read + sdMock := obj + parent := obj.Parent - for obj, children := range objChildrenArray { // shift the sequence diagrams as they are always placed at (0, 0) - sdMock := obj.ChildrenArray[0] - sequenceDiagrams[obj].shift(sdMock.TopLeft) + sequenceDiagrams[sdMock.AbsID()].shift(sdMock.TopLeft) // restore children - obj.Children = make(map[string]*d2graph.Object) - for _, child := range children { - obj.Children[child.ID] = child + parent.Children = make(map[string]*d2graph.Object) + for _, child := range childrenReplacement[sdMock.AbsID()] { + parent.Children[child.ID] = child } - obj.ChildrenArray = children + parent.ChildrenArray = childrenReplacement[sdMock.AbsID()] // add lifeline edges - g.Edges = append(g.Edges, sequenceDiagrams[obj].lifelines...) + g.Edges = append(g.Edges, sequenceDiagrams[sdMock.AbsID()].lifelines...) } + g.Objects = allObjects return nil } diff --git a/e2etests/stable_test.go b/e2etests/stable_test.go index 87e6cfd39..0d191b284 100644 --- a/e2etests/stable_test.go +++ b/e2etests/stable_test.go @@ -1046,7 +1046,6 @@ size XXXL -> custom 64: custom 48 { }, { name: "sequence_diagram_simple", script: `shape: sequence_diagram - alice: "Alice\nline\nbreaker" { shape: person style.stroke: red diff --git a/e2etests/testdata/stable/sequence_diagram_nested_span/elk/board.exp.json b/e2etests/testdata/stable/sequence_diagram_nested_span/elk/board.exp.json index ea7c579c9..61003c046 100644 --- a/e2etests/testdata/stable/sequence_diagram_nested_span/elk/board.exp.json +++ b/e2etests/testdata/stable/sequence_diagram_nested_span/elk/board.exp.json @@ -5,8 +5,8 @@ "id": "scorer", "type": "", "pos": { - "x": 12, - "y": 62 + "x": 0, + "y": 50 }, "width": 179, "height": 141, @@ -44,8 +44,8 @@ "id": "scorer.abc", "type": "rectangle", "pos": { - "x": 91, - "y": 1248 + "x": 79, + "y": 1236 }, "width": 20, "height": 50, @@ -82,8 +82,8 @@ "id": "itemResponse", "type": "", "pos": { - "x": 391, - "y": 62 + "x": 379, + "y": 50 }, "width": 270, "height": 141, @@ -121,8 +121,8 @@ "id": "itemResponse.a", "type": "rectangle", "pos": { - "x": 516, - "y": 348 + "x": 504, + "y": 336 }, "width": 20, "height": 160, @@ -159,8 +159,8 @@ "id": "item", "type": "", "pos": { - "x": 861, - "y": 62 + "x": 849, + "y": 50 }, "width": 157, "height": 141, @@ -198,8 +198,8 @@ "id": "item.a", "type": "rectangle", "pos": { - "x": 929, - "y": 488 + "x": 917, + "y": 476 }, "width": 20, "height": 770, @@ -236,8 +236,8 @@ "id": "item.a.b", "type": "rectangle", "pos": { - "x": 924, - "y": 498 + "x": 912, + "y": 486 }, "width": 30, "height": 160, @@ -274,8 +274,8 @@ "id": "essayRubric", "type": "", "pos": { - "x": 1218, - "y": 62 + "x": 1206, + "y": 50 }, "width": 245, "height": 141, @@ -313,8 +313,8 @@ "id": "essayRubric.a", "type": "rectangle", "pos": { - "x": 1330, - "y": 628 + "x": 1318, + "y": 616 }, "width": 20, "height": 340, @@ -351,8 +351,8 @@ "id": "essayRubric.a.b", "type": "rectangle", "pos": { - "x": 1325, - "y": 638 + "x": 1313, + "y": 626 }, "width": 30, "height": 320, @@ -389,8 +389,8 @@ "id": "essayRubric.a.b.c", "type": "rectangle", "pos": { - "x": 1320, - "y": 648 + "x": 1308, + "y": 636 }, "width": 40, "height": 160, @@ -427,8 +427,8 @@ "id": "concept", "type": "", "pos": { - "x": 1663, - "y": 62 + "x": 1651, + "y": 50 }, "width": 200, "height": 141, @@ -466,8 +466,8 @@ "id": "concept.a", "type": "rectangle", "pos": { - "x": 1753, - "y": 768 + "x": 1741, + "y": 756 }, "width": 20, "height": 370, @@ -504,8 +504,8 @@ "id": "concept.a.b", "type": "rectangle", "pos": { - "x": 1748, - "y": 778 + "x": 1736, + "y": 766 }, "width": 30, "height": 350, @@ -542,8 +542,8 @@ "id": "concept.a.b.c", "type": "rectangle", "pos": { - "x": 1743, - "y": 788 + "x": 1731, + "y": 776 }, "width": 40, "height": 330, @@ -580,8 +580,8 @@ "id": "concept.a.b.c.d", "type": "rectangle", "pos": { - "x": 1738, - "y": 798 + "x": 1726, + "y": 786 }, "width": 50, "height": 310, @@ -618,8 +618,8 @@ "id": "itemOutcome", "type": "", "pos": { - "x": 2063, - "y": 62 + "x": 2051, + "y": 50 }, "width": 265, "height": 141, @@ -657,8 +657,8 @@ "id": "itemOutcome.a", "type": "rectangle", "pos": { - "x": 2185, - "y": 1058 + "x": 2173, + "y": 1046 }, "width": 20, "height": 390, @@ -695,8 +695,8 @@ "id": "itemOutcome.a.b", "type": "rectangle", "pos": { - "x": 2180, - "y": 1068 + "x": 2168, + "y": 1056 }, "width": 30, "height": 370, @@ -733,8 +733,8 @@ "id": "itemOutcome.a.b.c", "type": "rectangle", "pos": { - "x": 2175, - "y": 1078 + "x": 2163, + "y": 1066 }, "width": 40, "height": 350, @@ -771,8 +771,8 @@ "id": "itemOutcome.a.b.c.d", "type": "rectangle", "pos": { - "x": 2170, - "y": 1088 + "x": 2158, + "y": 1076 }, "width": 50, "height": 330, @@ -809,8 +809,8 @@ "id": "itemOutcome.a.b.c.d.e", "type": "rectangle", "pos": { - "x": 2165, - "y": 1098 + "x": 2153, + "y": 1086 }, "width": 60, "height": 310, @@ -847,8 +847,8 @@ "id": "itemResponse.c", "type": "rectangle", "pos": { - "x": 516, - "y": 1548 + "x": 504, + "y": 1536 }, "width": 20, "height": 50, @@ -909,12 +909,12 @@ "labelPercentage": 0, "route": [ { - "x": 106.5, - "y": 353 + "x": 94.5, + "y": 341 }, { - "x": 511, - "y": 353 + "x": 499, + "y": 341 } ], "animated": false, @@ -948,12 +948,12 @@ "labelPercentage": 0, "route": [ { - "x": 541, - "y": 503 + "x": 529, + "y": 491 }, { - "x": 919.5, - "y": 503 + "x": 907.5, + "y": 491 } ], "animated": false, @@ -987,12 +987,12 @@ "labelPercentage": 0, "route": [ { - "x": 959.5, - "y": 653 + "x": 947.5, + "y": 641 }, { - "x": 1315.5, - "y": 653 + "x": 1303.5, + "y": 641 } ], "animated": false, @@ -1026,12 +1026,12 @@ "labelPercentage": 0, "route": [ { - "x": 1365.5, - "y": 803 + "x": 1353.5, + "y": 791 }, { - "x": 1733, - "y": 803 + "x": 1721, + "y": 791 } ], "animated": false, @@ -1065,12 +1065,12 @@ "labelPercentage": 0, "route": [ { - "x": 954.5, - "y": 953 + "x": 942.5, + "y": 941 }, { - "x": 1320.5, - "y": 953 + "x": 1308.5, + "y": 941 } ], "animated": false, @@ -1104,12 +1104,12 @@ "labelPercentage": 0, "route": [ { - "x": 1793, - "y": 1103 + "x": 1781, + "y": 1091 }, { - "x": 2160.5, - "y": 1103 + "x": 2148.5, + "y": 1091 } ], "animated": false, @@ -1143,12 +1143,12 @@ "labelPercentage": 0, "route": [ { - "x": 116.5, - "y": 1253 + "x": 104.5, + "y": 1241 }, { - "x": 924.5, - "y": 1253 + "x": 912.5, + "y": 1241 } ], "animated": false, @@ -1182,12 +1182,12 @@ "labelPercentage": 0, "route": [ { - "x": 2160.5, - "y": 1403 + "x": 2148.5, + "y": 1391 }, { - "x": 106.5, - "y": 1403 + "x": 94.5, + "y": 1391 } ], "animated": false, @@ -1221,12 +1221,12 @@ "labelPercentage": 0, "route": [ { - "x": 106.5, - "y": 1553 + "x": 94.5, + "y": 1541 }, { - "x": 511, - "y": 1553 + "x": 499, + "y": 1541 } ], "animated": false, @@ -1260,12 +1260,12 @@ "labelPercentage": 0, "route": [ { - "x": 101.5, - "y": 203 + "x": 89.5, + "y": 191 }, { - "x": 101.5, - "y": 1703 + "x": 89.5, + "y": 1691 } ], "animated": false, @@ -1299,12 +1299,12 @@ "labelPercentage": 0, "route": [ { - "x": 526, - "y": 203 + "x": 514, + "y": 191 }, { - "x": 526, - "y": 1703 + "x": 514, + "y": 1691 } ], "animated": false, @@ -1338,12 +1338,12 @@ "labelPercentage": 0, "route": [ { - "x": 939.5, - "y": 203 + "x": 927.5, + "y": 191 }, { - "x": 939.5, - "y": 1703 + "x": 927.5, + "y": 1691 } ], "animated": false, @@ -1377,12 +1377,12 @@ "labelPercentage": 0, "route": [ { - "x": 1340.5, - "y": 203 + "x": 1328.5, + "y": 191 }, { - "x": 1340.5, - "y": 1703 + "x": 1328.5, + "y": 1691 } ], "animated": false, @@ -1416,12 +1416,12 @@ "labelPercentage": 0, "route": [ { - "x": 1763, - "y": 203 + "x": 1751, + "y": 191 }, { - "x": 1763, - "y": 1703 + "x": 1751, + "y": 1691 } ], "animated": false, @@ -1455,12 +1455,12 @@ "labelPercentage": 0, "route": [ { - "x": 2195.5, - "y": 203 + "x": 2183.5, + "y": 191 }, { - "x": 2195.5, - "y": 1703 + "x": 2183.5, + "y": 1691 } ], "animated": false, diff --git a/e2etests/testdata/stable/sequence_diagram_nested_span/elk/sketch.exp.svg b/e2etests/testdata/stable/sequence_diagram_nested_span/elk/sketch.exp.svg index 585e5a593..6c954c961 100644 --- a/e2etests/testdata/stable/sequence_diagram_nested_span/elk/sketch.exp.svg +++ b/e2etests/testdata/stable/sequence_diagram_nested_span/elk/sketch.exp.svg @@ -2,7 +2,7 @@