d2sequence: account for self and sibling message height in note placement

d2layouts/d2sequence/layout_test.go

@@ -471,3 +471,126 @@ func TestSequenceToDescendant(t *testing.T) {
 		t.Fatal("expected route to end at `a` lifeline")
 	}
 }
+
+func TestGroupStartsWithNoteOverlap(t *testing.T) {
+	input := `
+shape: sequence_diagram
+
+job
+
+group1: {
+  job.t1 -> job.t2: one
+  job.t1 -> job.t3: two
+}
+group2: {
+  job.t1."overlap with group1"
+}
+`
+	ctx := log.With(context.Background(), testlog.New(t))
+	g, _, err := d2compiler.Compile("", strings.NewReader(input), nil)
+	assert.Nil(t, err)
+
+	for _, obj := range g.Objects {
+		if obj.Box == nil {
+			obj.Box = geo.NewBox(nil, 100, 100)
+		}
+	}
+
+	err = d2sequence.Layout(ctx, g, func(ctx context.Context, g *d2graph.Graph) error {
+		return nil
+	})
+	assert.Nil(t, err)
+
+	group1, has := g.Root.HasChild([]string{"group1"})
+	assert.True(t, has)
+	group2, has := g.Root.HasChild([]string{"group2"})
+	assert.True(t, has)
+
+	assert.GreaterOrEqual(t, group2.TopLeft.Y, group1.TopLeft.Y+group1.Height,
+		"group2 (top %v) must not overlap group1 (bottom %v)", group2.TopLeft.Y, group1.TopLeft.Y+group1.Height)
+}
+
+func TestGroupStartsWithNoteFollowedByMessages(t *testing.T) {
+	input := `
+shape: sequence_diagram
+
+a
+b
+
+group1: {
+  a."step one note"
+  a -> b: step one
+}
+group2: {
+  b."step two note"
+  b -> a: step two
+}
+`
+	ctx := log.With(context.Background(), testlog.New(t))
+	g, _, err := d2compiler.Compile("", strings.NewReader(input), nil)
+	assert.Nil(t, err)
+
+	for _, obj := range g.Objects {
+		if obj.Box == nil {
+			obj.Box = geo.NewBox(nil, 100, 100)
+		}
+	}
+
+	err = d2sequence.Layout(ctx, g, func(ctx context.Context, g *d2graph.Graph) error {
+		return nil
+	})
+	assert.Nil(t, err)
+
+	group1, has := g.Root.HasChild([]string{"group1"})
+	assert.True(t, has)
+	group2, has := g.Root.HasChild([]string{"group2"})
+	assert.True(t, has)
+
+	assert.GreaterOrEqual(t, group2.TopLeft.Y, group1.TopLeft.Y+group1.Height,
+		"group2 (top %v) must not overlap group1 (bottom %v)", group2.TopLeft.Y, group1.TopLeft.Y+group1.Height)
+}
+
+func TestMultipleGroupsStartingWithNotes(t *testing.T) {
+	input := `
+shape: sequence_diagram
+
+srv
+
+g1: {
+  srv."init config"
+}
+g2: {
+  srv."start worker"
+}
+g3: {
+  srv."listen on port"
+}
+`
+	ctx := log.With(context.Background(), testlog.New(t))
+	g, _, err := d2compiler.Compile("", strings.NewReader(input), nil)
+	assert.Nil(t, err)
+
+	for _, obj := range g.Objects {
+		if obj.Box == nil {
+			obj.Box = geo.NewBox(nil, 100, 100)
+		}
+	}
+
+	err = d2sequence.Layout(ctx, g, func(ctx context.Context, g *d2graph.Graph) error {
+		return nil
+	})
+	assert.Nil(t, err)
+
+	g1, has := g.Root.HasChild([]string{"g1"})
+	assert.True(t, has)
+	g2, has := g.Root.HasChild([]string{"g2"})
+	assert.True(t, has)
+	g3, has := g.Root.HasChild([]string{"g3"})
+	assert.True(t, has)
+
+	assert.GreaterOrEqual(t, g2.TopLeft.Y, g1.TopLeft.Y+g1.Height,
+		"g2 (top %v) must not overlap g1 (bottom %v)", g2.TopLeft.Y, g1.TopLeft.Y+g1.Height)
+	assert.GreaterOrEqual(t, g3.TopLeft.Y, g2.TopLeft.Y+g2.Height,
+		"g3 (top %v) must not overlap g2 (bottom %v)", g3.TopLeft.Y, g2.TopLeft.Y+g2.Height)
+}
+

d2layouts/d2sequence/sequence_diagram.go

@@ -478,6 +478,27 @@ func IsLifelineEnd(obj *d2graph.Object) bool {
 	return obj.ID == LifelineEndID(obj.ID[:markerIndex])
 }
 
+func isSelfOrSibling(message *d2graph.Edge) bool {
+	if message.Src == message.Dst {
+		return true
+	}
+	if strings.HasPrefix(message.Dst.AbsID(), message.Src.AbsID()+".") {
+		return true
+	}
+	if strings.HasPrefix(message.Src.AbsID(), message.Dst.AbsID()+".") {
+		return true
+	}
+	currSrc := message.Src
+	for !currSrc.Parent.IsSequenceDiagram() {
+		currSrc = currSrc.Parent
+	}
+	currDst := message.Dst
+	for !currDst.Parent.IsSequenceDiagram() {
+		currDst = currDst.Parent
+	}
+	return currSrc == currDst
+}
+
 func (sd *sequenceDiagram) placeNotes() {
 	rankToX := make(map[int]float64)
 	for _, actor := range sd.actors {
@@ -490,8 +511,8 @@ func (sd *sequenceDiagram) placeNotes() {
 
 		for _, msg := range sd.messages {
 			if sd.verticalIndices[msg.AbsID()] < verticalIndex {
-				if msg.Src == msg.Dst {
-					// For self-messages, account for the full vertical space they occupy
+				if isSelfOrSibling(msg) {
+					// For self/sibling messages, account for the full vertical space they occupy
 					y += sd.yStep + math.Max(float64(msg.LabelDimensions.Height), MIN_MESSAGE_DISTANCE)*1.5
 				} else {
 					y += sd.yStep + float64(msg.LabelDimensions.Height)
@@ -609,21 +630,7 @@ func (sd *sequenceDiagram) routeMessages() error {
 		} else {
 			return fmt.Errorf("could not find center of %s. Is it declared as an actor?", message.Dst.ID)
 		}
-		isToDescendant := strings.HasPrefix(message.Dst.AbsID(), message.Src.AbsID()+".")
-		isFromDescendant := strings.HasPrefix(message.Src.AbsID(), message.Dst.AbsID()+".")
-		isSelfMessage := message.Src == message.Dst
-
-		currSrc := message.Src
-		for !currSrc.Parent.IsSequenceDiagram() {
-			currSrc = currSrc.Parent
-		}
-		currDst := message.Dst
-		for !currDst.Parent.IsSequenceDiagram() {
-			currDst = currDst.Parent
-		}
-		isToSibling := currSrc == currDst
-
-		if isSelfMessage || isToDescendant || isFromDescendant || isToSibling {
+		if isSelfOrSibling(message) {
 			midX := startX + math.Max(SELF_MESSAGE_HORIZONTAL_TRAVEL, float64(message.LabelDimensions.Width)/2.+label.PADDING*2)
 			startY := messageOffset + noteOffset
 			endY := startY + math.Max(float64(message.LabelDimensions.Height), MIN_MESSAGE_DISTANCE)*1.5