From 900e4bab83a7a42afdbd6bc4112375ea67f057fe Mon Sep 17 00:00:00 2001
From: Alexander Wang <alex@terrastruct.com>
Date: Sun, 12 Feb 2023 09:59:44 -0800
Subject: [PATCH] fix note placement bug

---
 d2layouts/d2sequence/constants.go        | 23 +++++----
 d2layouts/d2sequence/sequence_diagram.go | 65 ++++++++++++++++++++++--
 2 files changed, 73 insertions(+), 15 deletions(-)

diff --git a/d2layouts/d2sequence/constants.go b/d2layouts/d2sequence/constants.go
index 95de80681..3a92ab1a4 100644
--- a/d2layouts/d2sequence/constants.go
+++ b/d2layouts/d2sequence/constants.go
@@ -1,21 +1,24 @@
 package d2sequence
 
-// leaves at least 25 units of space on the left/right when computing the space required between actors
-const HORIZONTAL_PAD = 50.
+// units of space on the left/right when computing the space required between actors
+const HORIZONTAL_PAD = 40.
 
-// leaves at least 25 units of space on the top/bottom when computing the space required between messages
-const VERTICAL_PAD = 50.
+// units of space on the top/bottom when computing the space required between messages
+// TODO lower
+const VERTICAL_PAD = 40.
 
-const MIN_ACTOR_DISTANCE = 250.
+const MIN_ACTOR_DISTANCE = 150.
 
-const MIN_ACTOR_WIDTH = 150.
+const MIN_ACTOR_WIDTH = 100.
 
-const SELF_MESSAGE_HORIZONTAL_TRAVEL = 100.
+const SELF_MESSAGE_HORIZONTAL_TRAVEL = 80.
 
-const GROUP_CONTAINER_PADDING = 24.
+const GROUP_CONTAINER_PADDING = 12.
+
+const EDGE_GROUP_LABEL_PADDING = 20.
 
 // min vertical distance between messages
-const MIN_MESSAGE_DISTANCE = 80.
+const MIN_MESSAGE_DISTANCE = 30.
 
 // default size
 const SPAN_BASE_WIDTH = 12.
@@ -24,7 +27,7 @@ const SPAN_BASE_WIDTH = 12.
 const SPAN_DEPTH_GROWTH_FACTOR = 8.
 
 // when a span has a single messages
-const MIN_SPAN_HEIGHT = 80.
+const MIN_SPAN_HEIGHT = 30.
 
 const SPAN_MESSAGE_PAD = 16.
 
diff --git a/d2layouts/d2sequence/sequence_diagram.go b/d2layouts/d2sequence/sequence_diagram.go
index c1dd1de2a..53cdc8e86 100644
--- a/d2layouts/d2sequence/sequence_diagram.go
+++ b/d2layouts/d2sequence/sequence_diagram.go
@@ -156,6 +156,7 @@ func newSequenceDiagram(objects []*d2graph.Object, messages []*d2graph.Edge) *se
 
 	for _, message := range sd.messages {
 		sd.verticalIndices[message.AbsID()] = getEdgeEarliestLineNum(message)
+		// TODO this should not be global yStep, only affect the neighbors
 		sd.yStep = math.Max(sd.yStep, float64(message.LabelDimensions.Height))
 
 		// ensures that long labels, spanning over multiple actors, don't make for large gaps between actors
@@ -176,7 +177,6 @@ func newSequenceDiagram(objects []*d2graph.Object, messages []*d2graph.Edge) *se
 		if _, exists := sd.firstMessage[message.Dst]; !exists {
 			sd.firstMessage[message.Dst] = message
 		}
-
 	}
 
 	sd.yStep += VERTICAL_PAD
@@ -209,6 +209,9 @@ func (sd *sequenceDiagram) placeGroups() {
 		group.ZIndex = GROUP_Z_INDEX
 		sd.placeGroup(group)
 	}
+	for _, group := range sd.groups {
+		sd.adjustGroupLabel(group)
+	}
 }
 
 func (sd *sequenceDiagram) placeGroup(group *d2graph.Object) {
@@ -273,6 +276,56 @@ func (sd *sequenceDiagram) placeGroup(group *d2graph.Object) {
 	)
 }
 
+func (sd *sequenceDiagram) adjustGroupLabel(group *d2graph.Object) {
+	if group.LabelHeight == nil {
+		return
+	}
+
+	heightAdd := (*group.LabelHeight + EDGE_GROUP_LABEL_PADDING) - GROUP_CONTAINER_PADDING
+	if heightAdd < 0 {
+		return
+	}
+
+	group.Height += float64(heightAdd)
+
+	// Extend stuff within this group
+	for _, g := range sd.groups {
+		if g.TopLeft.Y < group.TopLeft.Y && g.TopLeft.Y+g.Height > group.TopLeft.Y {
+			g.Height += float64(heightAdd)
+		}
+	}
+	for _, s := range sd.spans {
+		if s.TopLeft.Y < group.TopLeft.Y && s.TopLeft.Y+s.Height > group.TopLeft.Y {
+			s.Height += float64(heightAdd)
+		}
+	}
+
+	// Move stuff down
+	for _, m := range sd.messages {
+		if go2.Min(m.Route[0].Y, m.Route[len(m.Route)-1].Y) > group.TopLeft.Y {
+			for _, p := range m.Route {
+				p.Y += float64(heightAdd)
+			}
+		}
+	}
+	for _, s := range sd.spans {
+		if s.TopLeft.Y > group.TopLeft.Y {
+			s.TopLeft.Y += float64(heightAdd)
+		}
+	}
+	for _, g := range sd.groups {
+		if g.TopLeft.Y > group.TopLeft.Y {
+			g.TopLeft.Y += float64(heightAdd)
+		}
+	}
+	for _, n := range sd.notes {
+		if n.TopLeft.Y > group.TopLeft.Y {
+			n.TopLeft.Y += float64(heightAdd)
+		}
+	}
+
+}
+
 // placeActors places actors bottom aligned, side by side with centers spaced by sd.actorXStep
 func (sd *sequenceDiagram) placeActors() {
 	centerX := sd.actors[0].Width / 2.
@@ -354,7 +407,7 @@ func (sd *sequenceDiagram) placeNotes() {
 		rankToX[sd.objectRank[actor]] = actor.Center().X
 	}
 
-	for i, note := range sd.notes {
+	for _, note := range sd.notes {
 		verticalIndex := sd.verticalIndices[note.AbsID()]
 		y := sd.maxActorHeight + sd.yStep
 
@@ -363,8 +416,10 @@ func (sd *sequenceDiagram) placeNotes() {
 				y += sd.yStep
 			}
 		}
-		for _, otherNote := range sd.notes[:i] {
-			y += otherNote.Height + sd.yStep
+		for _, otherNote := range sd.notes {
+			if sd.verticalIndices[otherNote.AbsID()] < verticalIndex {
+				y += otherNote.Height + sd.yStep
+			}
 		}
 
 		x := rankToX[sd.objectRank[note]] - (note.Width / 2.)
@@ -499,7 +554,7 @@ func (sd *sequenceDiagram) routeMessages() error {
 
 		if isSelfMessage || isToDescendant || isFromDescendant || isToSibling {
 			midX := startX + SELF_MESSAGE_HORIZONTAL_TRAVEL
-			endY := startY + MIN_MESSAGE_DISTANCE
+			endY := startY + MIN_MESSAGE_DISTANCE*1.5
 			message.Route = []*geo.Point{
 				geo.NewPoint(startX, startY),
 				geo.NewPoint(midX, startY),