[css-grid] Implementation of Baseline Self-Alignment

third_party/WebKit/Source/core/layout/LayoutGrid.cpp

Index: third_party/WebKit/Source/core/layout/LayoutGrid.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
index 9cef256ddbad31b5c92ea199f6aa4c92f00ff4b2..51f75a1906db8e8c2e7051b033d3757c8c87c5f2 100644
--- a/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
@@ -25,6 +25,8 @@
 
 #include "core/layout/LayoutGrid.h"
 
+#include <algorithm>
+#include <memory>
 #include "core/frame/UseCounter.h"
 #include "core/layout/LayoutState.h"
 #include "core/layout/TextAutosizer.h"
@@ -33,9 +35,8 @@
 #include "core/style/ComputedStyle.h"
 #include "core/style/GridArea.h"
 #include "platform/LengthFunctions.h"
+#include "platform/text/WritingMode.h"
 #include "wtf/PtrUtil.h"
-#include <algorithm>
-#include <memory>
 
 namespace blink {
 
@@ -167,19 +168,48 @@ void LayoutGrid::computeTrackSizesForDefiniteSize(
 
 void LayoutGrid::repeatTracksSizingIfNeeded(LayoutUnit availableSpaceForColumns,
                                             LayoutUnit availableSpaceForRows) {
+  // Baseline alignment may change item's intrinsic size, hence changing its
+  // min-content contribution.
+  // https://drafts.csswg.org/css-align-3/#baseline-align-content
+  // https://drafts.csswg.org/css-align-3/#baseline-align-self
+  bool baselineAffectIntrinsicWidth = baselineMayAffectIntrinsicWidth();
+  bool baselineAffectIntrinsicHeight = baselineMayAffectIntrinsicHeight();
+
   // In orthogonal flow cases column track's size is determined by using the
   // computed row track's size, which it was estimated during the first cycle of
   // the sizing algorithm.
-  // Hence we need to repeat computeUsedBreadthOfGridTracks for both, columns
-  // and rows, to determine the final values.
-  // TODO (lajava): orthogonal flows is just one of the cases which may require
+  bool hasAnyOrthogonal =
+      m_trackSizingAlgorithm.grid().hasAnyOrthogonalGridItem();

[svillar, 2017/03/24 10:52:05]

Nit: no need for a variable here, we just use this once.

[jfernandez, 2017/03/24 14:15:18]

Well, I admit it's not strictly necessary but IMHO it helps to
place the comment and to define the multiple conditional clause 
below.

+
+  bool intrinsicWidthMayHaveChanged =
+      baselineAffectIntrinsicWidth || hasAnyOrthogonal;
+  if (!intrinsicWidthMayHaveChanged && !baselineAffectIntrinsicHeight)
+    return;
+
+  // TODO (lajava): these are just some of the cases which may require
   // a new cycle of the sizing algorithm; there may be more. In addition, not
   // all the cases with orthogonal flows require this extra cycle; we need a
   // more specific condition to detect whether child's min-content contribution
   // has changed or not.
-  if (m_grid.hasAnyOrthogonalGridItem()) {
-    computeTrackSizesForDefiniteSize(ForColumns, availableSpaceForColumns);
-    computeTrackSizesForDefiniteSize(ForRows, availableSpaceForRows);
+  if (!intrinsicWidthMayHaveChanged && !baselineAffectIntrinsicHeight)
+    return;
+
+  // TODO (lajava): Whenever the min-content contribution of a grid item changes
+  // we may need to update the grid container's intrinsic width. The new
+  // intrinsic width may also affect the extra Track Sizing algorithm cycles we
+  // are about to execute.
+  // https://crbug.com/704713
+  // https://github.com/w3c/csswg-drafts/issues/1039
+
+  // Hence we need to repeat computeUsedBreadthOfGridTracks for both, columns
+  // and rows, to determine the final values.
+  computeTrackSizesForDefiniteSize(ForColumns, availableSpaceForColumns);
+  computeTrackSizesForDefiniteSize(ForRows, availableSpaceForRows);
+
+  if (baselineAffectIntrinsicHeight) {
+    setLogicalHeight(computeTrackBasedLogicalHeight() +
+                     borderAndPaddingLogicalHeight() +
+                     scrollbarLogicalHeight());
   }
 }
 
@@ -193,6 +223,9 @@ void LayoutGrid::layoutBlock(bool relayoutChildren) {
       simplifiedLayout())
     return;
 
+  m_rowAxisAlignmentContext.clear();
+  m_colAxisAlignmentContext.clear();
+
   SubtreeLayoutScope layoutScope(*this);
 
   {
@@ -259,6 +292,15 @@ void LayoutGrid::layoutBlock(bool relayoutChildren) {
           logicalHeight() - trackBasedLogicalHeight;
     }
 
+    // TODO (lajava): We need to compute baselines after step 2 so
+    // items with a relative size (percentages) can resolve it before
+    // determining its baseline. However, we only set item's grid area
+    // (via override sizes) as part of the content-sized tracks sizing
+    // logic. Hence, items located at fixed or flexible tracks can't
+    // resolve correctly their size at this stage, which may lead to
+    // an incorrect computation of their shared context's baseline.
+    computeBaselineAlignmentContext();

[svillar, 2017/03/24 10:52:05]

I have just realized that we do this before applying the alignment stuff. What
happens in those cases? Can't alignment modify the grid in such a way that this
computation becomes wrong?

[jfernandez, 2017/03/24 14:15:18]

Yes, indeed. It's a really good point and I admit I haven't analyzed
it carefully. However, my impression when I cooked the patch was that Content
Distribution does not affect to the Baseline-Self alignment. The reason is that
Content Distribution affects to the
whole track, either its position or its size, and Baseline-Self
alignment operates on the items sharing an alignment context, either
a row or column. Hence, the final baseline offsets will be relative
to the position and size of the tracks that conforms the shared
alignment context.

I played with some tests and I haven't see any wrong behavior, so
far. I don't think we need an exhaustive set of test cases now, since there may
be other combinations with baseline that might be
tested as well. I think we can progressively improve the testing
coverage for Baseline-Self alignment and fixing the bugs that may appear, if
any.

+
     // 3- If the min-content contribution of any grid items have changed based
     // on the row sizes calculated in step 2, steps 1 and 2 are repeated with
     // the new min-content contribution (once only).
@@ -1580,8 +1622,6 @@ static int synthesizedBaselineFromBorderBox(const LayoutBox& box,
       .toInt();
 }
 
-// TODO(lajava): This logic is shared by LayoutFlexibleBox, so it might be
-// refactored somehow.
 int LayoutGrid::baselinePosition(FontBaseline,
                                  bool,
                                  LineDirectionMode direction,
@@ -1595,11 +1635,6 @@ int LayoutGrid::baselinePosition(FontBaseline,
   return baseline + beforeMarginInLineDirection(direction);
 }
 
-bool LayoutGrid::isInlineBaselineAlignedChild(const LayoutBox* child) const {
-  return alignSelfForChild(*child).position() == ItemPositionBaseline &&
-         !isOrthogonalChild(*child) && !hasAutoMarginsInColumnAxis(*child);
-}
-
 int LayoutGrid::firstLineBoxBaseline() const {
   if (isWritingModeRoot() || !m_grid.hasGridItems())
     return -1;
@@ -1612,8 +1647,8 @@ int LayoutGrid::firstLineBoxBaseline() const {
     for (size_t index = 0; index < m_grid.cell(0, column).size(); index++) {
       const LayoutBox* child = m_grid.cell(0, column)[index];
       DCHECK(!child->isOutOfFlowPositioned());
-      // If an item participates in baseline alignmen, we select such item.
-      if (isInlineBaselineAlignedChild(child)) {
+      // If an item participates in baseline alignment, we select such item.
+      if (isBaselineAlignmentForChild(*child)) {
         // TODO (lajava): self-baseline and content-baseline alignment
         // still not implemented.
         baselineChild = child;
@@ -1663,6 +1698,184 @@ int LayoutGrid::inlineBlockBaseline(LineDirectionMode direction) const {
   return synthesizedBaselineFromContentBox(*this, direction) + marginHeight;
 }
 
+bool LayoutGrid::isHorizontalGridAxis(GridAxis axis) const {
+  return axis == GridRowAxis ? isHorizontalWritingMode()
+                             : !isHorizontalWritingMode();
+}
+
+bool LayoutGrid::isParallelToBlockAxisForChild(const LayoutBox& child,
+                                               GridAxis axis) const {
+  return axis == GridColumnAxis ? !isOrthogonalChild(child)
+                                : isOrthogonalChild(child);
+}
+
+bool LayoutGrid::isDescentBaselineForChild(const LayoutBox& child,
+                                           GridAxis baselineAxis) const {
+  return isHorizontalGridAxis(baselineAxis) &&
+         !styleRef().isFlippedBlocksWritingMode();
+}
+
+bool LayoutGrid::isBaselineAlignmentForChild(const LayoutBox& child,
+                                             GridAxis baselineAxis) const {
+  bool isColumnAxisBaseline = baselineAxis == GridColumnAxis;
+  ItemPosition align = isColumnAxisBaseline
+                           ? alignSelfForChild(child).position()
+                           : justifySelfForChild(child).position();
+  bool hasAutoMargins = isColumnAxisBaseline ? hasAutoMarginsInColumnAxis(child)
+                                             : hasAutoMarginsInRowAxis(child);
+  return isBaselinePosition(align) && !hasAutoMargins;
+}

[svillar, 2017/03/24 10:52:05]

I wonder if these four utility methods could be part of the baseline class added
in other file instead of being here. Haven't examined all the dependencies in
detail so feel free to ignore this if it does not make much sense.

[jfernandez, 2017/03/24 14:15:18]

Yeah, I wondered the same and actually tried to move them to the
Baseline class. However, some of these methods have dependencies
with the LayoutGrid object, so while not impossible, it's not a trivial
refactoring.  

I can think of it in a follow up patch.

+
+const BaselineGroup& LayoutGrid::getBaselineGroupForChild(
+    const LayoutBox& child,
+    GridAxis baselineAxis) const {
+  DCHECK(isBaselineAlignmentForChild(child, baselineAxis));
+  auto& grid = m_trackSizingAlgorithm.grid();
+  bool isColumnAxisBaseline = baselineAxis == GridColumnAxis;
+  bool isRowAxisContext = isColumnAxisBaseline;
+  const auto& span = isRowAxisContext ? grid.gridItemSpan(child, ForRows)
+                                      : grid.gridItemSpan(child, ForColumns);
+  auto& contextsMap =
+      isRowAxisContext ? m_rowAxisAlignmentContext : m_colAxisAlignmentContext;
+  auto* context = contextsMap.at(span.startLine());
+  DCHECK(context);
+  ItemPosition align = isColumnAxisBaseline
+                           ? alignSelfForChild(child).position()
+                           : justifySelfForChild(child).position();
+  return context->getSharedGroup(child, align);
+}
+
+LayoutUnit LayoutGrid::marginOverForChild(const LayoutBox& child,
+                                          GridAxis axis) const {
+  return isHorizontalGridAxis(axis) ? child.marginRight() : child.marginTop();
+}
+
+LayoutUnit LayoutGrid::logicalAscentForChild(const LayoutBox& child,
+                                             GridAxis baselineAxis) const {
+  LayoutUnit ascent = ascentForChild(child, baselineAxis);
+  return isDescentBaselineForChild(child, baselineAxis)
+             ? descentForChild(child, ascent, baselineAxis)
+             : ascent;
+}
+
+LayoutUnit LayoutGrid::ascentForChild(const LayoutBox& child,
+                                      GridAxis baselineAxis) const {
+  int baseline = isParallelToBlockAxisForChild(child, baselineAxis)
+                     ? child.firstLineBoxBaseline()
+                     : -1;
+  // We take border-box's bottom if no valid baseline.
+  if (baseline == -1) {
+    baseline = isHorizontalGridAxis(baselineAxis)
+                   ? child.size().width().toInt()
+                   : child.size().height().toInt();
+  }
+  return LayoutUnit(baseline) + marginOverForChild(child, baselineAxis);
+}
+
+LayoutUnit LayoutGrid::descentForChild(const LayoutBox& child,
+                                       LayoutUnit ascent,
+                                       GridAxis baselineAxis) const {
+  if (isParallelToBlockAxisForChild(child, baselineAxis))
+    return child.marginLogicalHeight() + child.logicalHeight() - ascent;
+  return child.marginLogicalWidth() + child.logicalWidth() - ascent;
+}
+
+bool LayoutGrid::isBaselineContextComputed(GridAxis baselineAxis) const {
+  return baselineAxis == GridColumnAxis ? !m_rowAxisAlignmentContext.isEmpty()
+                                        : !m_colAxisAlignmentContext.isEmpty();
+}
+

[svillar, 2017/03/24 10:52:05]

Same question here for the above 5 methods. Do you think it might make sense to
generalize it so they could be part of the baseline class? I know we have
GridAxis there, but I wonder if that could be generalized to inline/block axis
or perhaps they're different, dunno.

[jfernandez, 2017/03/24 14:15:18]

Already explained before, but I'd definitively like to do it, in a 
different patch, to avoid complicate this one even more.

+bool LayoutGrid::baselineMayAffectIntrinsicWidth() const {
+  if (!styleRef().logicalWidth().isIntrinsicOrAuto())
+    return false;
+  for (const auto& context : m_colAxisAlignmentContext) {
+    for (const auto& group : context.value->sharedGroups()) {
+      if (group.size() > 1)
+        return true;
+    }
+  }
+  return false;
+}
+
+bool LayoutGrid::baselineMayAffectIntrinsicHeight() const {
+  if (!styleRef().logicalHeight().isIntrinsicOrAuto())
+    return false;
+  for (const auto& context : m_rowAxisAlignmentContext) {
+    for (const auto& group : context.value->sharedGroups()) {
+      if (group.size() > 1)
+        return true;
+    }
+  }
+  return false;
+}
+
+void LayoutGrid::computeBaselineAlignmentContext() {
+  for (auto* child = firstInFlowChildBox(); child;
+       child = child->nextInFlowSiblingBox()) {
+    updateBaselineAlignmentContextIfNeeded(*child, GridRowAxis);
+    updateBaselineAlignmentContextIfNeeded(*child, GridColumnAxis);
+  }
+}
+
+void LayoutGrid::updateBaselineAlignmentContextIfNeeded(LayoutBox& child,
+                                                        GridAxis baselineAxis) {
+  // DCHECK(m_trackSizingAlgorithm.isTrackSizingOperation());

[svillar, 2017/03/24 10:52:05]

Remove this check if it does not apply.

[jfernandez, 2017/03/24 14:15:18]

Well, I'd like to have this check. However, unfortunately the new tracks sizing
algorithm initialize the sizing phase during the 'setup' method. This is called
as art of the sizing logic, later on during the layout logic. 

It's a PITA we can't figure out before whether we are doing an intrinsic size
computation or a regular layout. The method in question shouldn't be called as
part of an intrinsic size computation, because it performs a layout.

My recommendation would be to remove the check but add a TODO to
revisit the topic in a future patch.

+  if (!isBaselineAlignmentForChild(child, baselineAxis))
+    return;
+
+  child.layoutIfNeeded();
+
+  // Determine Ascent and Descent values of this child with respect to
+  // its grid container.
+  LayoutUnit ascent = ascentForChild(child, baselineAxis);
+  LayoutUnit descent = descentForChild(child, ascent, baselineAxis);
+  if (isDescentBaselineForChild(child, baselineAxis))
+    std::swap(ascent, descent);
+
+  // Looking up for a shared alignment context perpendicular to the
+  // baseline axis.
+  auto& grid = m_trackSizingAlgorithm.grid();
+  bool isColumnAxisBaseline = baselineAxis == GridColumnAxis;
+  bool isRowAxisContext = isColumnAxisBaseline;
+  const auto& span = isRowAxisContext ? grid.gridItemSpan(child, ForRows)
+                                      : grid.gridItemSpan(child, ForColumns);
+  auto& contextsMap =
+      isRowAxisContext ? m_rowAxisAlignmentContext : m_colAxisAlignmentContext;
+  auto addResult = contextsMap.insert(span.startLine(), nullptr);
+
+  // Looking for a compatible baseline-sharing group.
+  ItemPosition align = isColumnAxisBaseline
+                           ? alignSelfForChild(child).position()
+                           : justifySelfForChild(child).position();
+  if (addResult.isNewEntry) {
+    addResult.storedValue->value =
+        WTF::makeUnique<BaselineContext>(child, align, ascent, descent);
+  } else {
+    auto* context = addResult.storedValue->value.get();
+    context->updateSharedGroup(child, align, ascent, descent);
+  }
+}
+
+LayoutUnit LayoutGrid::columnAxisBaselineOffsetForChild(
+    const LayoutBox& child) const {
+  if (!isBaselineAlignmentForChild(child, GridColumnAxis))
+    return LayoutUnit();
+  auto& group = getBaselineGroupForChild(child, GridColumnAxis);
+  if (group.size() > 1)
+    return group.maxAscent() - logicalAscentForChild(child, GridColumnAxis);
+  return LayoutUnit();
+}
+
+LayoutUnit LayoutGrid::rowAxisBaselineOffsetForChild(
+    const LayoutBox& child) const {
+  if (!isBaselineAlignmentForChild(child, GridRowAxis))
+    return LayoutUnit();
+  auto& group = getBaselineGroupForChild(child, GridRowAxis);
+  if (group.size() > 1)
+    return group.maxAscent() - logicalAscentForChild(child, GridRowAxis);
+  return LayoutUnit();
+}
+
 GridAxisPosition LayoutGrid::columnAxisPositionForChild(
     const LayoutBox& child) const {
   bool hasSameWritingMode =
@@ -1732,8 +1945,6 @@ GridAxisPosition LayoutGrid::columnAxisPositionForChild(
       return GridAxisStart;
     case ItemPositionBaseline:
     case ItemPositionLastBaseline:
-      // FIXME: These two require implementing Baseline Alignment. For now, we
-      // always 'start' align the child. crbug.com/234191
       return GridAxisStart;
     case ItemPositionAuto:
     case ItemPositionNormal:
@@ -1811,8 +2022,6 @@ GridAxisPosition LayoutGrid::rowAxisPositionForChild(
       return GridAxisStart;
     case ItemPositionBaseline:
     case ItemPositionLastBaseline:
-      // FIXME: These two require implementing Baseline Alignment. For now, we
-      // always 'start' align the child. crbug.com/234191
       return GridAxisStart;
     case ItemPositionAuto:
     case ItemPositionNormal:
@@ -1834,7 +2043,7 @@ LayoutUnit LayoutGrid::columnAxisOffsetForChild(const LayoutBox& child) const {
   GridAxisPosition axisPosition = columnAxisPositionForChild(child);
   switch (axisPosition) {
     case GridAxisStart:
-      return startPosition;
+      return startPosition + columnAxisBaselineOffsetForChild(child);
     case GridAxisEnd:
     case GridAxisCenter: {
       size_t childEndLine = rowsSpan.endLine();
@@ -1876,7 +2085,7 @@ LayoutUnit LayoutGrid::rowAxisOffsetForChild(const LayoutBox& child) const {
   GridAxisPosition axisPosition = rowAxisPositionForChild(child);
   switch (axisPosition) {
     case GridAxisStart:
-      return startPosition;
+      return startPosition + rowAxisBaselineOffsetForChild(child);
     case GridAxisEnd:
     case GridAxisCenter: {
       size_t childEndLine = columnsSpan.endLine();