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

improve skewed container-container dagre layouts #1384

Merged
merged 3 commits into from
Jun 9, 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 @@ -18,6 +18,7 @@
- Common invalid array separator `,` usage in class arrays returns a helpful error message [#1376](https://github.com/terrastruct/d2/pull/1376)
- Invalid `constraint` usage is met with an error message, preventing a common mistake of omitting `shape: sql_table` [#1379](https://github.com/terrastruct/d2/pull/1379)
- Connections now stop at all outside labels. [#1381](https://github.com/terrastruct/d2/pull/1381)
- Container connections in `dagre` are more balanced [#1384](https://github.com/terrastruct/d2/pull/1384)

#### Bugfixes ⛑️

Expand Down
98 changes: 77 additions & 21 deletions d2layouts/d2dagrelayout/layout.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,22 +544,7 @@ func getEdgeEndpoints(g *d2graph.Graph, edge *d2graph.Edge) (*d2graph.Object, *d
}
dst := edge.Dst
for len(dst.Children) > 0 && dst.Class == nil && dst.SQLTable == nil {
dst = dst.ChildrenArray[0]

// We want to get the top node of destinations
for _, child := range dst.ChildrenArray {
isHead := true
for _, e := range g.Edges {
if inContainer(e.Src, child) != nil && inContainer(e.Dst, dst) != nil {
isHead = false
break
}
}
if isHead {
dst = child
break
}
}
dst = getLongestEdgeChainHead(g, dst)
}
if edge.SrcArrow && !edge.DstArrow {
// for `b <- a`, edge.Edge is `a -> b` and we expect this routing result
Expand Down Expand Up @@ -607,8 +592,73 @@ func generateAddEdgeLine(fromID, toID, edgeID string, width, height int) string
return fmt.Sprintf("g.setEdge({v:`%s`, w:`%s`, name:`%s`}, { width:%d, height:%d, labelpos: `c` });\n", escapeID(fromID), escapeID(toID), escapeID(edgeID), width, height)
}

// getLongestEdgeChainHead finds the longest chain in a container and gets its head
// If there are multiple chains of the same length, get the head closest to the center
func getLongestEdgeChainHead(g *d2graph.Graph, container *d2graph.Object) *d2graph.Object {
rank := make(map[*d2graph.Object]int)
chainLength := make(map[*d2graph.Object]int)

for _, obj := range container.ChildrenArray {
isHead := true
for _, e := range g.Edges {
if inContainer(e.Src, container) != nil && inContainer(e.Dst, obj) != nil {
isHead = false
break
}
}
if !isHead {
continue
}
rank[obj] = 1
chainLength[obj] = 1
// BFS
queue := []*d2graph.Object{obj}
visited := make(map[*d2graph.Object]struct{})
for len(queue) > 0 {
curr := queue[0]
queue = queue[1:]
if _, ok := visited[curr]; ok {
continue
}
visited[curr] = struct{}{}
for _, e := range g.Edges {
child := inContainer(e.Dst, container)
if child == curr {
continue
}
if child != nil && inContainer(e.Src, curr) != nil {
if rank[curr]+1 > rank[child] {
rank[child] = rank[curr] + 1
chainLength[obj] = go2.Max(chainLength[obj], rank[child])
}
queue = append(queue, child)
}
}
}
}
max := int(math.MinInt32)
for _, obj := range container.ChildrenArray {
if chainLength[obj] > max {
max = chainLength[obj]
}
}

var heads []*d2graph.Object
for i, obj := range container.ChildrenArray {
if rank[obj] == 1 && chainLength[obj] == max {
heads = append(heads, container.ChildrenArray[i])
}
}

if len(heads) > 0 {
return heads[int(math.Floor(float64(len(heads))/2.0))]
}
return container.ChildrenArray[0]
}

// getLongestEdgeChainTail gets the node at the end of the longest edge chain, because that will be the end of the container
// and is what external connections should connect with
// and is what external connections should connect with.
// If there are multiple of same length, get the one closest to the middle
func getLongestEdgeChainTail(g *d2graph.Graph, container *d2graph.Object) *d2graph.Object {
rank := make(map[*d2graph.Object]int)

Expand Down Expand Up @@ -647,14 +697,20 @@ func getLongestEdgeChainTail(g *d2graph.Graph, container *d2graph.Object) *d2gra
}
}
max := int(math.MinInt32)
var tail *d2graph.Object
for _, obj := range container.ChildrenArray {
if rank[obj] >= max {
if rank[obj] > max {
max = rank[obj]
tail = obj
}
}
return tail

var tails []*d2graph.Object
for i, obj := range container.ChildrenArray {
if rank[obj] == max {
tails = append(tails, container.ChildrenArray[i])
}
}

return tails[int(math.Floor(float64(len(tails))/2.0))]
}

func inContainer(obj, container *d2graph.Object) *d2graph.Object {
Expand Down
17 changes: 17 additions & 0 deletions e2etests/stable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2316,6 +2316,23 @@ Listen <-> Talk: {
target-arrowhead.shape: diamond
label: hear
}
`,
},
{
name: "dagre-container",
script: `a: {
a
b
c
}

b: {
a
b
c
}

a -> b
`,
},
{
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading