improve code docs and use width for margin calculation

intergalacticspacehighway · intergalacticspacehighway · commit 7a51319a6085 · 2025-11-19T14:19:50.000+05:30
diff --git a/yoga/algorithm/grid/AutoPlacement.h b/yoga/algorithm/grid/AutoPlacement.h
@@ -232,7 +232,7 @@ struct AutoPlacement {
             rowEnd,
             child
           };
-    
+          // TODO: Optimise overlap check with a hash based alternative if necessary
           for (const auto& placedItem: gridItemAreas) {
             if (gridItemArea.overlaps(placedItem)) {
               columnStart++;
diff --git a/yoga/algorithm/grid/GridLayout.cpp b/yoga/algorithm/grid/GridLayout.cpp
@@ -117,7 +117,7 @@ void calculateGridLayoutInternal(Node* node,
     node,
     columnTracks, 
     rowTracks, 
-    containerInnerWidth, 
+    containerInnerWidth,
     containerInnerHeight, 
     itemAreas,
     widthSizingMode,
@@ -154,7 +154,8 @@ void calculateGridLayoutInternal(Node* node,
     Dimension::Height);
 
   
-  // Final position
+  // Layout grid items
+  // Step 4: Lay out the grid items into their respective containing blocks. Each grid area’s width and height are considered definite for this purpose.
   if (performLayout) {
     auto gridWidth = trackSizing.getTotalBaseSize(Dimension::Width);
     auto gridHeight = trackSizing.getTotalBaseSize(Dimension::Height);
@@ -169,32 +170,32 @@ void calculateGridLayoutInternal(Node* node,
 
     float gridInlineOffset = 0.0f;
     float betweenInlineOffset = 0.0f;
-    if (gridWidth < containerInnerWidth) {
-      float freeSpace = containerInnerWidth - gridWidth;
+    float freeSpaceInlineAxis = containerInnerWidth - gridWidth;
+    if (freeSpaceInlineAxis > 0.0f && !yoga::inexactEquals(freeSpaceInlineAxis, 0.0f)) {
       auto justifyContent = node->style().justifyContent();
       size_t numColumnTracks = columnTracks.size();
   
       switch (justifyContent) {
         case Justify::Center:
-          gridInlineOffset = freeSpace / 2.0f;
+          gridInlineOffset = freeSpaceInlineAxis / 2.0f;
           break;
         case Justify::End:
-          gridInlineOffset = freeSpace;
+          gridInlineOffset = freeSpaceInlineAxis;
           break;
         case Justify::SpaceBetween:
           if (numColumnTracks > 1) {
-            betweenInlineOffset = freeSpace / (numColumnTracks - 1);
+            betweenInlineOffset = freeSpaceInlineAxis / (numColumnTracks - 1);
           }
           break;
         case Justify::SpaceAround:
           if (numColumnTracks > 0) {
-            betweenInlineOffset = freeSpace / numColumnTracks;
+            betweenInlineOffset = freeSpaceInlineAxis / numColumnTracks;
             gridInlineOffset = betweenInlineOffset / 2.0f;
           }
           break;
         case Justify::SpaceEvenly:
           if (numColumnTracks > 0) {
-            betweenInlineOffset = freeSpace / (numColumnTracks + 1);
+            betweenInlineOffset = freeSpaceInlineAxis / (numColumnTracks + 1);
             gridInlineOffset = betweenInlineOffset;
           }
           break;
@@ -210,32 +211,32 @@ void calculateGridLayoutInternal(Node* node,
   
     float gridBlockOffset = 0.0f;
     float betweenBlockOffset = 0.0f;
-    if (gridHeight < containerInnerHeight) {
-      float freeSpace = containerInnerHeight - gridHeight;
+    float freeSpaceBlockAxis = containerInnerHeight - gridHeight;
+    if (freeSpaceBlockAxis > 0.0f && !yoga::inexactEquals(freeSpaceBlockAxis, 0.0f)) {
       auto alignContent = node->style().alignContent();
       size_t numRowTracks = rowTracks.size();
   
       switch (alignContent) {
         case Align::Center:
-          gridBlockOffset = freeSpace / 2.0f;
+          gridBlockOffset = freeSpaceBlockAxis / 2.0f;
           break;
         case Align::End:
-          gridBlockOffset = freeSpace;
+          gridBlockOffset = freeSpaceBlockAxis;
           break;
         case Align::SpaceBetween:
           if (numRowTracks > 1) {
-            betweenBlockOffset = freeSpace / (numRowTracks - 1);
+            betweenBlockOffset = freeSpaceBlockAxis / (numRowTracks - 1);
           }
           break;
         case Align::SpaceAround:
           if (numRowTracks > 0) {
-            betweenBlockOffset = freeSpace / numRowTracks;
+            betweenBlockOffset = freeSpaceBlockAxis / numRowTracks;
             gridBlockOffset = betweenBlockOffset / 2.0f;
           }
           break;
         case Align::SpaceEvenly:
           if (numRowTracks > 0) {
-            betweenBlockOffset = freeSpace / (numRowTracks + 1);
+            betweenBlockOffset = freeSpaceBlockAxis / (numRowTracks + 1);
             gridBlockOffset = betweenBlockOffset;
           }
           break;
@@ -270,10 +271,10 @@ void calculateGridLayoutInternal(Node* node,
         }
       }
 
-      auto marginInlineStart = item.node->style().computeInlineStartMargin(FlexDirection::Row, direction, containingBlockWidth);
-      auto marginInlineEnd = item.node->style().computeInlineEndMargin(FlexDirection::Row, direction, containingBlockWidth);
-      auto marginBlockStart = item.node->style().computeInlineStartMargin(FlexDirection::Column, direction, containingBlockHeight);
-      auto marginBlockEnd = item.node->style().computeInlineEndMargin(FlexDirection::Column, direction, containingBlockHeight);
+      const auto marginInlineStart = item.node->style().computeInlineStartMargin(FlexDirection::Row, direction, containingBlockWidth);
+      const auto marginInlineEnd = item.node->style().computeInlineEndMargin(FlexDirection::Row, direction, containingBlockWidth);
+      const auto marginBlockStart = item.node->style().computeInlineStartMargin(FlexDirection::Column, direction, containingBlockWidth);
+      const auto marginBlockEnd = item.node->style().computeInlineEndMargin(FlexDirection::Column, direction, containingBlockWidth);
 
       auto justifySelf = item.node->style().justifySelf();
       if (justifySelf == Justify::Auto) {
@@ -290,14 +291,17 @@ void calculateGridLayoutInternal(Node* node,
       float childWidth = containingBlockWidth;
       float childHeight = containingBlockHeight;
 
+      // https://www.w3.org/TR/css-grid-1/#alignment
       // if justify-self or align-self compute to a value other than stretch or margins are auto, 
       // grid items will auto-size to fit their content.
-      bool hasMarginInlineAuto = item.node->style().flexStartMarginIsAuto(FlexDirection::Row, direction) || item.node->style().flexEndMarginIsAuto(FlexDirection::Row, direction);
+      bool hasMarginInlineAuto = item.node->style().flexStartMarginIsAuto(FlexDirection::Row, direction) 
+        || item.node->style().flexEndMarginIsAuto(FlexDirection::Row, direction);
       if (justifySelf != Justify::Stretch || hasMarginInlineAuto) {
         childWidthSizingMode = SizingMode::FitContent;
       }
 
-      bool hasMarginBlockAuto = item.node->style().flexStartMarginIsAuto(FlexDirection::Column, direction) || item.node->style().flexEndMarginIsAuto(FlexDirection::Column, direction);
+      bool hasMarginBlockAuto = item.node->style().flexStartMarginIsAuto(FlexDirection::Column, direction) 
+        || item.node->style().flexEndMarginIsAuto(FlexDirection::Column, direction);
       if (alignSelf != Align::Stretch || hasMarginBlockAuto) {
         childHeightSizingMode = SizingMode::FitContent;
       }
@@ -307,8 +311,7 @@ void calculateGridLayoutInternal(Node* node,
             direction,
             dimension(FlexDirection::Row),
             containingBlockWidth,
-            containingBlockWidth
-        ).unwrap() + item.node->style().computeMarginForAxis(FlexDirection::Row, containingBlockWidth);
+            containingBlockWidth).unwrap() + marginInlineStart + marginInlineEnd;
         childWidthSizingMode = SizingMode::StretchFit;
       }
 
@@ -317,8 +320,7 @@ void calculateGridLayoutInternal(Node* node,
             direction,
             dimension(FlexDirection::Column),
             containingBlockHeight,
-            containingBlockWidth
-        ).unwrap() + item.node->style().computeMarginForAxis(FlexDirection::Column, containingBlockWidth);
+            containingBlockWidth).unwrap() + marginBlockStart + marginBlockEnd;
         childHeightSizingMode = SizingMode::StretchFit;
       }
       
@@ -358,11 +360,13 @@ void calculateGridLayoutInternal(Node* node,
 
       // since we know the item width and grid width, we can do the alignment here.
       // alignment of grid items happen in the grid area
-      // TODO: confirm that measured dimension includes padding and border
+      // measured dimension includes padding and border
       float actualItemWidth = item.node->getLayout().measuredDimension(Dimension::Width);
       auto freeSpaceInlineAxis = containingBlockWidth - actualItemWidth - marginInlineStart - marginInlineEnd;
       float leftAutoMarginOffset = 0.0f;
       float rightAutoMarginOffset = 0.0f;
+      // https://www.w3.org/TR/css-grid-1/#auto-margins
+      // auto margins in either axis absorb positive free space prior to alignment via the box alignment properties, thereby disabling the effects of any self-alignment properties in that axis.
       if (item.node->style().flexStartMarginIsAuto(FlexDirection::Row, direction) 
           && item.node->style().flexEndMarginIsAuto(FlexDirection::Row, direction)) {
         leftAutoMarginOffset = freeSpaceInlineAxis / 2;
@@ -376,7 +380,6 @@ void calculateGridLayoutInternal(Node* node,
         freeSpaceInlineAxis = 0.0f;
       }
       
-      // Justify self alignment
       float justifySelfOffset = 0.0f;
       if (justifySelf == Justify::End) {
         justifySelfOffset = freeSpaceInlineAxis;
diff --git a/yoga/algorithm/grid/TrackSizing.h b/yoga/algorithm/grid/TrackSizing.h
@@ -874,7 +874,7 @@ struct TrackSizing {
 
     auto marginForAxis = item.node->style().computeMarginForAxis(
         dimension == Dimension::Width ? FlexDirection::Row : FlexDirection::Column,
-        containingBlockSize);
+        containingBlockWidth);
 
     return minContentSize + marginForAxis;
   };
@@ -1150,13 +1150,13 @@ struct TrackSizing {
         auto newMinimumSize = autoMinimumSize(item, dimension);
         auto marginForAxis = item.node->style().computeMarginForAxis(
           dimension == Dimension::Width ? FlexDirection::Row : FlexDirection::Column,
-          containingBlockSize);
+          containingBlockWidth);
         return newMinimumSize + marginForAxis;
       } else {
         auto resolvedMinSize = minSize.resolve(containingBlockSize);
         auto marginForAxis = item.node->style().computeMarginForAxis(
           dimension == Dimension::Width ? FlexDirection::Row : FlexDirection::Column,
-          containingBlockSize);
+          containingBlockWidth);
         return resolvedMinSize.unwrap() + marginForAxis;
       }
     } else {
