Merge pull request #1734 from gavin-ts/shaped-grid-positioning

fix shaped grid positioning and circle innerBox/sizing

ci/release/changelogs/next.md

@@ -29,3 +29,4 @@
 - Fixes crash when using `--watch` and navigating to an invalid board path [#1693](https://github.com/terrastruct/d2/pull/1693)
 - Fixes edge case where nested edge globs were creating excess shapes [#1713](https://github.com/terrastruct/d2/pull/1713)
 - Fixes a panic with a connection to a grid cell that is a container in TALA [#1729](https://github.com/terrastruct/d2/pull/1729)
+- Fixes incorrect grid cell positioning when the grid has a shape set and fixes content sometimes escaping circle shapes. [#1734](https://github.com/terrastruct/d2/pull/1734)

d2layouts/d2grid/layout.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"fmt"
 	"math"
-	"strconv"
 
 	"oss.terrastruct.com/d2/d2graph"
 	"oss.terrastruct.com/d2/d2target"
@@ -69,7 +68,6 @@ func Layout(ctx context.Context, g *d2graph.Graph) error {
 			labelHeight = float64(obj.LabelDimensions.Height) + 2*label.PADDING
 		}
 
-		var dx, dy float64
 		if labelWidth > 0 {
 			switch labelPosition {
 			case label.OutsideTopLeft, label.OutsideTopCenter, label.OutsideTopRight,
@@ -112,67 +110,34 @@ func Layout(ctx context.Context, g *d2graph.Graph) error {
 			}
 		}
 
-		overflowTop := padding.Top - float64(verticalPadding)
-		if overflowTop > 0 {
-			contentHeight += overflowTop
-			dy += overflowTop
-		}
-		overflowBottom := padding.Bottom - float64(verticalPadding)
-		if overflowBottom > 0 {
-			contentHeight += overflowBottom
-		}
-		overflowLeft := padding.Left - float64(horizontalPadding)
-		if overflowLeft > 0 {
-			contentWidth += overflowLeft
-			dx += overflowLeft
-		}
-		overflowRight := padding.Right - float64(horizontalPadding)
-		if overflowRight > 0 {
-			contentWidth += overflowRight
-		}
+		padding.Top = math.Max(padding.Top, float64(verticalPadding))
+		padding.Bottom = math.Max(padding.Bottom, float64(verticalPadding))
+		padding.Left = math.Max(padding.Left, float64(horizontalPadding))
+		padding.Right = math.Max(padding.Right, float64(horizontalPadding))
 
-		// manually handle desiredWidth/Height so we can center the grid
-		var desiredWidth, desiredHeight int
-		var originalWidthAttr, originalHeightAttr *d2graph.Scalar
-		if obj.WidthAttr != nil {
-			desiredWidth, _ = strconv.Atoi(obj.WidthAttr.Value)
-			// SizeToContent without desired width
-			originalWidthAttr = obj.WidthAttr
-			obj.WidthAttr = nil
-		}
-		if obj.HeightAttr != nil {
-			desiredHeight, _ = strconv.Atoi(obj.HeightAttr.Value)
-			originalHeightAttr = obj.HeightAttr
-			obj.HeightAttr = nil
-		}
-		// size shape according to grid
-		obj.SizeToContent(contentWidth, contentHeight, float64(2*horizontalPadding), float64(2*verticalPadding))
-		if originalWidthAttr != nil {
-			obj.WidthAttr = originalWidthAttr
-		}
-		if originalHeightAttr != nil {
-			obj.HeightAttr = originalHeightAttr
-		}
+		totalWidth := padding.Left + contentWidth + padding.Right
+		totalHeight := padding.Top + contentHeight + padding.Bottom
+		obj.SizeToContent(totalWidth, totalHeight, 0, 0)
 
-		if desiredWidth > 0 {
-			ddx := float64(desiredWidth) - obj.Width
-			if ddx > 0 {
-				dx += ddx / 2
-				obj.Width = float64(desiredWidth)
-			}
+		// compute where the grid should be placed inside shape
+		s := obj.ToShape()
+		innerTL := s.GetInsidePlacement(totalWidth, totalHeight, 0, 0)
+		// depending on the shape innerBox may be larger than totalWidth, totalHeight
+		// if this is the case, we want to center the cells within the larger innerBox
+		innerBox := s.GetInnerBox()
+
+		var resizeDx, resizeDy float64
+		if innerBox.Width > totalWidth {
+			resizeDx = (innerBox.Width - totalWidth) / 2
 		}
-		if desiredHeight > 0 {
-			ddy := float64(desiredHeight) - obj.Height
-			if ddy > 0 {
-				dy += ddy / 2
-				obj.Height = float64(desiredHeight)
-			}
+		if innerBox.Height > totalHeight {
+			resizeDy = (innerBox.Height - totalHeight) / 2
 		}
 
-		// compute where the grid should be placed inside shape
-		innerBox := obj.ToShape().GetInnerBox()
-		dx = innerBox.TopLeft.X + dx
-		dy = innerBox.TopLeft.Y + dy
+		// move from horizontalPadding,verticalPadding to innerTL.X+padding.Left, innerTL.Y+padding.Top
+		// and if innerBox is larger than content dimensions, adjust to center within innerBox
+		dx := -float64(horizontalPadding) + innerTL.X + padding.Left + resizeDx
+		dy := -float64(verticalPadding) + innerTL.Y + padding.Top + resizeDy
 		if dx != 0 || dy != 0 {
 			gd.shift(dx, dy)
 		}

d2renderers/d2svg/d2svg.go

@@ -1192,10 +1192,15 @@ func drawShape(writer, appendixWriter io.Writer, diagramHash string, targetShape
 		}
 	}
 
-	// to examine GetInsidePlacement
-	// padX, padY := s.GetDefaultPadding()
-	// innerTL := s.GetInsidePlacement(s.GetInnerBox().Width, s.GetInnerBox().Height, padX, padY)
-	// fmt.Fprint(writer, renderOval(&innerTL, 5, 5, "fill:red;"))
+	// // to examine shape's innerBox
+	// innerBox := s.GetInnerBox()
+	// el := d2themes.NewThemableElement("rect")
+	// el.X = float64(innerBox.TopLeft.X)
+	// el.Y = float64(innerBox.TopLeft.Y)
+	// el.Width = float64(innerBox.Width)
+	// el.Height = float64(innerBox.Height)
+	// el.Style = "fill:rgba(255,0,0,0.5);"
+	// fmt.Fprint(writer, el.Render())
 
 	// Closes the class=shape
 	fmt.Fprint(writer, `</g>`)

e2etests/regression_test.go

@@ -1050,6 +1050,7 @@ cf many required: {
 		loadFromFile(t, "grid_rows_gap_bug"),
 		loadFromFile(t, "grid_image_label_position"),
 		loadFromFile(t, "glob_dimensions"),
+		loadFromFile(t, "shaped_grid_positioning"),
 	}
 
 	runa(t, tcs)

e2etests/testdata/files/shaped_grid_positioning.d2

@@ -0,0 +1,39 @@
+vars: {
+  grid-container: {
+    label: aaaaaaaaaaaaaaa aaaaaaaaaaaaaaa
+    grid-rows: 2
+    grid-columns: 2
+    a
+    b
+    c
+    d
+  }
+}
+circle: {
+  shape: circle
+  ...${grid-container}
+}
+
+oval: {
+  shape: oval
+  ...${grid-container}
+}
+
+diamond: {
+  shape: diamond
+  ...${grid-container}
+}
+
+square: {
+  shape: square
+  ...${grid-container}
+}
+
+rect: {
+  ...${grid-container}
+}
+
+cloud: {
+  shape: cloud
+  ...${grid-container}
+}