Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple edges across nested diagrams #1631

Merged
merged 21 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/release/changelogs/next.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#### Improvements 🧹

- Grid cells can now contain nested edges [#1629](https://github.com/terrastruct/d2/pull/1629)
- Edges can now go across constant nears, sequence diagrams, and grids including nested ones. [#1631](https://github.com/terrastruct/d2/pull/1631)

#### Bugfixes ⛑️

Expand Down
74 changes: 21 additions & 53 deletions d2compiler/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -1068,20 +1068,13 @@ func (c *compiler) validateNear(g *d2graph.Graph) {
}

for _, edge := range g.Edges {
srcNearContainer := edge.Src.OuterNearContainer()
dstNearContainer := edge.Dst.OuterNearContainer()

var isSrcNearConst, isDstNearConst bool

if srcNearContainer != nil {
_, isSrcNearConst = d2graph.NearConstants[d2graph.Key(srcNearContainer.NearKey)[0]]
}
if dstNearContainer != nil {
_, isDstNearConst = d2graph.NearConstants[d2graph.Key(dstNearContainer.NearKey)[0]]
if edge.Src.IsConstantNear() && edge.Dst.IsDescendantOf(edge.Src) {
c.errorf(edge.GetAstEdge(), "edge from constant near %#v cannot enter itself", edge.Src.AbsID())
continue
}

if (isSrcNearConst || isDstNearConst) && srcNearContainer != dstNearContainer {
c.errorf(edge.References[0].Edge, "cannot connect objects from within a container, that has near constant set, to objects outside that container")
if edge.Dst.IsConstantNear() && edge.Src.IsDescendantOf(edge.Dst) {
c.errorf(edge.GetAstEdge(), "edge from constant near %#v cannot enter itself", edge.Dst.AbsID())
continue
}
}

Expand All @@ -1101,47 +1094,22 @@ func (c *compiler) validateEdges(g *d2graph.Graph) {
c.errorf(edge.GetAstEdge(), "edge from grid diagram %#v cannot enter itself", edge.Dst.AbsID())
continue
}

srcGrid := edge.Src.Parent.ClosestGridDiagram()
dstGrid := edge.Dst.Parent.ClosestGridDiagram()
if srcGrid != nil || dstGrid != nil {
if srcGrid != dstGrid {
// valid: a -> grid
// invalid: a -> grid.child
if dstGrid != nil && !(srcGrid != nil && srcGrid.IsDescendantOf(dstGrid)) {
c.errorf(edge.GetAstEdge(), "edge cannot enter grid diagram %#v", dstGrid.AbsID())
} else {
c.errorf(edge.GetAstEdge(), "edge cannot exit grid diagram %#v", srcGrid.AbsID())
}
continue
}

srcCell := edge.Src.ClosestGridCell()
dstCell := edge.Dst.ClosestGridCell()
// edges within a grid cell are ok now
// grid.cell.a -> grid.cell.b : ok
// grid.cell.a.c -> grid.cell.b.d : ok
// edges between grid cells themselves are ok
// grid.cell -> grid.cell2 : ok
// grid.cell -> grid.cell.inside : not ok
// grid.cell -> grid.cell2.inside : not ok
srcIsGridCell := edge.Src == srcCell
dstIsGridCell := edge.Dst == dstCell
if srcIsGridCell != dstIsGridCell {
if srcIsGridCell {
c.errorf(edge.GetAstEdge(), "grid cell %#v can only connect to another grid cell", edge.Src.AbsID())
} else {
c.errorf(edge.GetAstEdge(), "grid cell %#v can only connect to another grid cell", edge.Dst.AbsID())
}
continue
}

if srcCell != dstCell && (!srcIsGridCell || !dstIsGridCell) {
c.errorf(edge.GetAstEdge(), "edge cannot exit grid cell %#v", srcCell.AbsID())
continue
}
if edge.Src.Parent.IsGridDiagram() && edge.Dst.IsDescendantOf(edge.Src) {
c.errorf(edge.GetAstEdge(), "edge from grid cell %#v cannot enter itself", edge.Src.AbsID())
continue
}
if edge.Dst.Parent.IsGridDiagram() && edge.Src.IsDescendantOf(edge.Dst) {
c.errorf(edge.GetAstEdge(), "edge from grid cell %#v cannot enter itself", edge.Dst.AbsID())
continue
}
if edge.Src.IsSequenceDiagram() && edge.Dst.IsDescendantOf(edge.Src) {
c.errorf(edge.GetAstEdge(), "edge from sequence diagram %#v cannot enter itself", edge.Src.AbsID())
continue
}
if edge.Dst.IsSequenceDiagram() && edge.Src.IsDescendantOf(edge.Dst) {
c.errorf(edge.GetAstEdge(), "edge from sequence diagram %#v cannot enter itself", edge.Dst.AbsID())
continue
}

}
}

Expand Down
56 changes: 41 additions & 15 deletions d2compiler/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1616,7 +1616,7 @@ d2/testdata/d2compiler/TestCompile/near-invalid.d2:14:9: near keys cannot be set
}
x -> y
`,
expErr: `d2/testdata/d2compiler/TestCompile/near_bad_connected.d2:5:5: cannot connect objects from within a container, that has near constant set, to objects outside that container`,
expErr: ``,
},
{
name: "near_descendant_connect_to_outside",
Expand All @@ -1627,7 +1627,7 @@ d2/testdata/d2compiler/TestCompile/near-invalid.d2:14:9: near keys cannot be set
}
x.y -> z
`,
expErr: "d2/testdata/d2compiler/TestCompile/near_descendant_connect_to_outside.d2:6:5: cannot connect objects from within a container, that has near constant set, to objects outside that container",
expErr: "",
},
{
name: "nested_near_constant",
Expand Down Expand Up @@ -2040,7 +2040,7 @@ b
}
b -> x.a
`,
expErr: `d2/testdata/d2compiler/TestCompile/leaky_sequence.d2:5:1: connections within sequence diagrams can connect only to other objects within the same sequence diagram`,
expErr: ``,
},
{
name: "sequence_scoping",
Expand Down Expand Up @@ -2484,9 +2484,7 @@ hey -> hey.a

hey -> c: ok
`,
expErr: `d2/testdata/d2compiler/TestCompile/grid_edge.d2:5:1: edge cannot enter grid diagram "hey"
d2/testdata/d2compiler/TestCompile/grid_edge.d2:6:1: edge cannot exit grid diagram "hey"
d2/testdata/d2compiler/TestCompile/grid_edge.d2:7:1: edge from grid diagram "hey" cannot enter itself`,
expErr: `d2/testdata/d2compiler/TestCompile/grid_edge.d2:7:1: edge from grid diagram "hey" cannot enter itself`,
},
{
name: "grid_deeper_edge",
Expand All @@ -2506,19 +2504,47 @@ d2/testdata/d2compiler/TestCompile/grid_edge.d2:7:1: edge from grid diagram "hey
g -> h: ok
g -> h.h: ok
}
e -> f.i: not ok
e.g -> f.i: not ok
e -> f.i: ok now
e.g -> f.i: ok now
}
a -> b.c: not yet
a.e -> b.c: also not yet
a -> b.c: ok now
a.e -> b.c: ok now
a -> a.e: not ok
}
`,
expErr: `d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:17:3: grid cell "hey.a.e" can only connect to another grid cell
d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:18:3: edge cannot exit grid cell "hey.a.e"
d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:20:2: grid cell "hey.a" can only connect to another grid cell
d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:21:2: edge cannot exit grid diagram "hey.a"
d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:22:2: edge from grid diagram "hey.a" cannot enter itself`,
expErr: `d2/testdata/d2compiler/TestCompile/grid_deeper_edge.d2:22:2: edge from grid diagram "hey.a" cannot enter itself`,
},
{
name: "parent_graph_edge_to_descendant",
text: `tl: {
near: top-left
a.b
}
grid: {
grid-rows: 1
cell.c.d
}
seq: {
shape: sequence_diagram
e.f
}
tl -> tl.a: no
tl -> tl.a.b: no
grid-> grid.cell: no
grid-> grid.cell.c: no
grid.cell -> grid.cell.c: no
grid.cell -> grid.cell.c.d: no
seq -> seq.e: no
seq -> seq.e.f: no
`,
expErr: `d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:13:1: edge from constant near "tl" cannot enter itself
d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:14:1: edge from constant near "tl" cannot enter itself
d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:17:1: edge from grid cell "grid.cell" cannot enter itself
d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:18:1: edge from grid cell "grid.cell" cannot enter itself
d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:15:1: edge from grid diagram "grid" cannot enter itself
d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:16:1: edge from grid diagram "grid" cannot enter itself
d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:19:1: edge from sequence diagram "seq" cannot enter itself
d2/testdata/d2compiler/TestCompile/parent_graph_edge_to_descendant.d2:20:1: edge from sequence diagram "seq" cannot enter itself`,
},
{
name: "grid_nested",
Expand Down
4 changes: 0 additions & 4 deletions d2graph/d2graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -1225,10 +1225,6 @@ func (obj *Object) Connect(srcID, dstID []string, srcArrow, dstArrow bool, label
src := obj.ensureChildEdge(srcID)
dst := obj.ensureChildEdge(dstID)

if src.OuterSequenceDiagram() != dst.OuterSequenceDiagram() {
return nil, errors.New("connections within sequence diagrams can connect only to other objects within the same sequence diagram")
}

e := &Edge{
Attributes: Attributes{
Label: Scalar{
Expand Down
76 changes: 61 additions & 15 deletions d2layouts/d2layouts.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (
"oss.terrastruct.com/d2/d2layouts/d2near"
"oss.terrastruct.com/d2/d2layouts/d2sequence"
"oss.terrastruct.com/d2/lib/geo"
"oss.terrastruct.com/d2/lib/label"
"oss.terrastruct.com/d2/lib/log"
"oss.terrastruct.com/util-go/go2"
)

type DiagramType string
Expand Down Expand Up @@ -80,6 +82,7 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
// Before we can layout these nodes, we need to handle all nested diagrams first.
extracted := make(map[string]*d2graph.Graph)
var extractedOrder []string
var extractedEdges []*d2graph.Edge

var constantNears []*d2graph.Graph
restoreOrder := SaveOrder(g)
Expand All @@ -100,14 +103,15 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
if isGridCellContainer && gi.isDefault() {
// if we are in a grid diagram, and our children have descendants
// we need to run layout on them first, even if they are not special diagram types
nestedGraph := ExtractSubgraph(curr, true)
nestedGraph, externalEdges := ExtractSubgraph(curr, true)
id := curr.AbsID()
err := LayoutNested(ctx, nestedGraph, GraphInfo{}, coreLayout)
if err != nil {
return err
}

InjectNested(g.Root, nestedGraph, false)
g.Edges = append(g.Edges, externalEdges...)
restoreOrder()

// need to update curr *Object incase layout changed it
Expand Down Expand Up @@ -138,7 +142,8 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
}

// now we keep the descendants out until after grid layout
nestedGraph = ExtractSubgraph(curr, false)
nestedGraph, externalEdges = ExtractSubgraph(curr, false)
extractedEdges = append(extractedEdges, externalEdges...)

extracted[id] = nestedGraph
extractedOrder = append(extractedOrder, id)
Expand All @@ -152,7 +157,8 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
}

// There is a nested diagram here, so extract its contents and process in the same way
nestedGraph := ExtractSubgraph(curr, gi.IsConstantNear)
nestedGraph, externalEdges := ExtractSubgraph(curr, gi.IsConstantNear)
extractedEdges = append(extractedEdges, externalEdges...)

log.Info(ctx, "layout nested", slog.F("level", curr.Level()), slog.F("child", curr.AbsID()), slog.F("gi", gi))
nestedInfo := gi
Expand Down Expand Up @@ -228,24 +234,51 @@ func LayoutNested(ctx context.Context, g *d2graph.Graph, graphInfo GraphInfo, co
}
}

idToObj := make(map[string]*d2graph.Object)
for _, o := range g.Objects {
idToObj[o.AbsID()] = o
}

// With the layout set, inject all the extracted graphs
for _, id := range extractedOrder {
nestedGraph := extracted[id]
// we have to find the object by ID because coreLayout can replace the Objects in graph
var obj *d2graph.Object
for _, o := range g.Objects {
if o.AbsID() == id {
obj = o
break
}
}
if obj == nil {
obj, exists := idToObj[id]
if !exists {
return fmt.Errorf("could not find object %#v after layout", id)
}
InjectNested(obj, nestedGraph, true)
PositionNested(obj, nestedGraph)
}

// update map with injected objects
for _, o := range g.Objects {
idToObj[o.AbsID()] = o
}

// Restore cross-graph edges and route them
g.Edges = append(g.Edges, extractedEdges...)
for _, e := range extractedEdges {
// update object references
src, exists := idToObj[e.Src.AbsID()]
if !exists {
return fmt.Errorf("could not find object %#v after layout", e.Src.AbsID())
}
e.Src = src
dst, exists := idToObj[e.Dst.AbsID()]
if !exists {
return fmt.Errorf("could not find object %#v after layout", e.Dst.AbsID())
}
e.Dst = dst

// simple straight line edge routing when going across graphs
e.Route = []*geo.Point{e.Src.Center(), e.Dst.Center()}
e.TraceToShape(e.Route, 0, 1)
if e.Label.Value != "" {
e.LabelPosition = go2.Pointer(string(label.InsideMiddleCenter))
}
}

log.Debug(ctx, "done", slog.F("rootlevel", g.RootLevel), slog.F("shapes", g.PrintString()))
return err
}
Expand All @@ -262,10 +295,10 @@ func NestedGraphInfo(obj *d2graph.Object) (gi GraphInfo) {
return gi
}

func ExtractSubgraph(container *d2graph.Object, includeSelf bool) *d2graph.Graph {
func ExtractSubgraph(container *d2graph.Object, includeSelf bool) (nestedGraph *d2graph.Graph, externalEdges []*d2graph.Edge) {
// includeSelf: when we have a constant near or a grid cell that is a container,
// we want to include itself in the nested graph, not just its descendants,
nestedGraph := d2graph.NewGraph()
nestedGraph = d2graph.NewGraph()
nestedGraph.RootLevel = int(container.Level())
if includeSelf {
nestedGraph.RootLevel--
Expand All @@ -284,8 +317,21 @@ func ExtractSubgraph(container *d2graph.Object, includeSelf bool) *d2graph.Graph
g := container.Graph
remainingEdges := make([]*d2graph.Edge, 0, len(g.Edges))
for _, edge := range g.Edges {
if isNestedObject(edge.Src) && isNestedObject(edge.Dst) {
srcIsNested := isNestedObject(edge.Src)
if d2sequence.IsLifelineEnd(edge.Dst) {
// special handling for lifelines since their edge.Dst is a special Object
if srcIsNested {
nestedGraph.Edges = append(nestedGraph.Edges, edge)
} else {
remainingEdges = append(remainingEdges, edge)
}
continue
}
dstIsNested := isNestedObject(edge.Dst)
if srcIsNested && dstIsNested {
nestedGraph.Edges = append(nestedGraph.Edges, edge)
} else if srcIsNested || dstIsNested {
externalEdges = append(externalEdges, edge)
} else {
remainingEdges = append(remainingEdges, edge)
}
Expand Down Expand Up @@ -333,7 +379,7 @@ func ExtractSubgraph(container *d2graph.Object, includeSelf bool) *d2graph.Graph
container.ChildrenArray = nil
}

return nestedGraph
return nestedGraph, externalEdges
}

func InjectNested(container *d2graph.Object, nestedGraph *d2graph.Graph, isRoot bool) {
Expand Down
20 changes: 20 additions & 0 deletions d2layouts/d2sequence/sequence_diagram.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"math"
"sort"
"strconv"
"strings"

"oss.terrastruct.com/util-go/go2"
Expand Down Expand Up @@ -411,6 +412,25 @@ func (sd *sequenceDiagram) addLifelineEdges() {
}
}

func IsLifelineEnd(obj *d2graph.Object) bool {
// lifeline ends only have ID and no graph parent or box set
if obj.Graph != nil || obj.Parent != nil || obj.Box != nil {
return false
}
if !strings.Contains(obj.ID, "-lifeline-end-") {
return false
}
parts := strings.Split(obj.ID, "-lifeline-end-")
if len(parts) > 1 {
hash := parts[len(parts)-1]
actorID := strings.Join(parts[:len(parts)-1], "-lifeline-end-")
if strconv.Itoa(go2.StringToIntHash(actorID+"-lifeline-end")) == hash {
return true
}
}
return false
}

func (sd *sequenceDiagram) placeNotes() {
rankToX := make(map[int]float64)
for _, actor := range sd.actors {
Expand Down
Loading