Changeset 278865 in webkit

Timestamp:

Jun 15, 2021 2:19:23 AM (13 months ago)

Author:

svillar@igalia.com

Message:

[css-flexbox] Do not compute the min-max sizes of flex items twice
​https://bugs.webkit.org/show_bug.cgi?id=226463

Reviewed by Simon Fraser.

When determining the flex base size, the item’s min and max main sizes are ignored (no clamping occurs).
Those limits are used to compute the item's hypothetical main size and also later when the flexible
lengths are resolved. The thing is that we were running the code that clamps the flex item size twice instead
of computing those limits once and apply them twice.

From now one, we just compute them once and store the limits in a std::pair in the FlexItem class. This means
that the FlexItem is able to compute the hypothetical main size on its own and does not need it to be passed
as an argument.

No new tests as this is already being tested by dozens of tests.

• rendering/FlexibleBoxAlgorithm.cpp:

(WebCore::FlexItem::FlexItem):
(WebCore::FlexItem::constrainSizeByMinMax const): Clamp the passed in size by the stored min & max sizes.

• rendering/FlexibleBoxAlgorithm.h:
• rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::computeFlexItemMinMaxSizes): Renamed from adjustChildSizeForMinAndMax and
without the childSize argument which is no longer needed.
(WebCore::RenderFlexibleBox::constructFlexItem): Use constrainSizeByMinMax.
(WebCore::RenderFlexibleBox::resolveFlexibleLengths): Ditto.
(WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax): Deleted.

• rendering/RenderFlexibleBox.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/FlexibleBoxAlgorithm.cpp (modified) (2 diffs)
• rendering/FlexibleBoxAlgorithm.h (modified) (2 diffs)
• rendering/RenderFlexibleBox.cpp (modified) (5 diffs)
• rendering/RenderFlexibleBox.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-05-31  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] Do not compute the min-max sizes of flex items twice
+        https://bugs.webkit.org/show_bug.cgi?id=226463
+
+        Reviewed by Simon Fraser.
+
+        When determining the flex base size, the item’s min and max main sizes are ignored (no clamping occurs).
+        Those limits are used to compute the item's hypothetical main size and also later when the flexible
+        lengths are resolved. The thing is that we were running the code that clamps the flex item size twice instead
+        of computing those limits once and apply them twice.
+
+        From now one, we just compute them once and store the limits in a std::pair in the FlexItem class. This means
+        that the FlexItem is able to compute the hypothetical main size on its own and does not need it to be passed
+        as an argument.
+
+        No new tests as this is already being tested by dozens of tests.
+
+        * rendering/FlexibleBoxAlgorithm.cpp:
+        (WebCore::FlexItem::FlexItem):
+        (WebCore::FlexItem::constrainSizeByMinMax const): Clamp the passed in size by the stored min & max sizes.
+        * rendering/FlexibleBoxAlgorithm.h:
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::computeFlexItemMinMaxSizes): Renamed from adjustChildSizeForMinAndMax and
+        without the childSize argument which is no longer needed.
+        (WebCore::RenderFlexibleBox::constructFlexItem): Use constrainSizeByMinMax.
+        (WebCore::RenderFlexibleBox::resolveFlexibleLengths): Ditto.
+        (WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax): Deleted.
+        * rendering/RenderFlexibleBox.h:
+
 2021-06-11  Sergio Villar Senin  <svillar@igalia.com>
 

trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.cpp

@@ -36 +36 @@
 namespace WebCore {
 
-FlexItem::FlexItem(RenderBox& box, LayoutUnit flexBaseContentSize, LayoutUnit hypotheticalMainContentSize, LayoutUnit mainAxisBorderAndPadding, LayoutUnit mainAxisMargin, bool everHadLayout)
+FlexItem::FlexItem(RenderBox& box, LayoutUnit flexBaseContentSize, LayoutUnit mainAxisBorderAndPadding, LayoutUnit mainAxisMargin, std::pair<LayoutUnit, LayoutUnit> minMaxSizes, bool everHadLayout)
     : box(box)
     , flexBaseContentSize(flexBaseContentSize)
-    , hypotheticalMainContentSize(hypotheticalMainContentSize)
     , mainAxisBorderAndPadding(mainAxisBorderAndPadding)
     , mainAxisMargin(mainAxisMargin)
+    , minMaxSizes(minMaxSizes)
+    , hypotheticalMainContentSize(constrainSizeByMinMax(flexBaseContentSize))
+    , frozen(false)
     , everHadLayout(everHadLayout)
 {
@@ -87 +89 @@
 }
 
+LayoutUnit FlexItem::constrainSizeByMinMax(const LayoutUnit size) const
+{
+    return std::max(minMaxSizes.first, std::min(size, minMaxSizes.second));
+}
+
 } // namespace WebCore

trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.h

@@ -42 +42 @@
 class FlexItem {
 public:
-    FlexItem(RenderBox&, LayoutUnit flexBaseContentSize, LayoutUnit hypotheticalMainContentSize, LayoutUnit mainAxisBorderAndPadding, LayoutUnit mainAxisMargin, bool everHadLayout);
+    FlexItem(RenderBox&, LayoutUnit flexBaseContentSize, LayoutUnit mainAxisBorderAndPadding, LayoutUnit mainAxisMargin, std::pair<LayoutUnit, LayoutUnit> minMaxSizes, bool everHadLayout);
 
     LayoutUnit hypotheticalMainAxisMarginBoxSize() const
@@ -59 +59 @@
     }
 
+    LayoutUnit constrainSizeByMinMax(const LayoutUnit) const;
+
     RenderBox& box;
     const LayoutUnit flexBaseContentSize;
-    const LayoutUnit hypotheticalMainContentSize;
     const LayoutUnit mainAxisBorderAndPadding;
     const LayoutUnit mainAxisMargin;
+    const std::pair<LayoutUnit, LayoutUnit> minMaxSizes;
+    const LayoutUnit hypotheticalMainContentSize;
     LayoutUnit flexedContentSize;
     bool frozen { false };

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -1202 +1202 @@
 }
 
-LayoutUnit RenderFlexibleBox::adjustChildSizeForMinAndMax(RenderBox& child, LayoutUnit childSize)
+std::pair<LayoutUnit, LayoutUnit> RenderFlexibleBox::computeFlexItemMinMaxSizes(RenderBox& child)
 {
     Length max = mainSizeLengthForChild(MaxSize, child);
     std::optional<LayoutUnit> maxExtent = std::nullopt;
-    if (max.isSpecifiedOrIntrinsic()) {
+    if (max.isSpecifiedOrIntrinsic())
         maxExtent = computeMainAxisExtentForChild(child, MaxSize, max);
-        childSize = std::min(childSize, maxExtent.value_or(childSize));
-    }
 
     Length min = mainSizeLengthForChild(MinSize, child);
     // Intrinsic sizes in child's block axis are handled by the min-size:auto code path.
     if (min.isSpecified() || (min.isIntrinsic() && mainAxisIsChildInlineAxis(child)))
-        return std::max(childSize, std::max(0_lu, computeMainAxisExtentForChild(child, MinSize, min).value_or(childSize)));
+        return { computeMainAxisExtentForChild(child, MinSize, min).value_or(0_lu), maxExtent.value_or(LayoutUnit::max()) };
     
     if (shouldApplyMinSizeAutoForChild(child)) {
@@ -1235 +1233 @@
             ASSERT(resolvedMainSize >= 0);
             LayoutUnit specifiedSize = std::min(resolvedMainSize, maxExtent.value_or(resolvedMainSize));
-            return std::max(childSize, std::min(specifiedSize, contentSize));
+            return { std::min(specifiedSize, contentSize), maxExtent.value_or(LayoutUnit::max()) };
         }
 
@@ -1241 +1239 @@
             LayoutUnit transferredSize = computeMainSizeFromAspectRatioUsing(child, childCrossSizeLength);
             transferredSize = adjustChildSizeForAspectRatioCrossAxisMinAndMax(child, transferredSize);
-            return std::max(childSize, std::min(transferredSize, contentSize));
-        }
-
-        return std::max(childSize, contentSize);
-    }
-
-    return std::max(0_lu, childSize);
+            return { std::min(transferredSize, contentSize), maxExtent.value_or(LayoutUnit::max()) };
+        }
+
+        return { contentSize, maxExtent.value_or(LayoutUnit::max()) };
+    }
+
+    return { 0_lu, maxExtent.value_or(LayoutUnit::max()) };
 }
     
@@ -1335 +1333 @@
     LayoutUnit borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
     LayoutUnit childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding);
-    LayoutUnit childMinMaxAppliedMainAxisExtent = adjustChildSizeForMinAndMax(child, childInnerFlexBaseSize);
     LayoutUnit margin = isHorizontalFlow() ? child.horizontalMarginExtent() : child.verticalMarginExtent();
-    return FlexItem(child, childInnerFlexBaseSize, childMinMaxAppliedMainAxisExtent, borderAndPadding, margin, childHadLayout);
+    return FlexItem(child, childInnerFlexBaseSize, borderAndPadding, margin, computeFlexItemMinMaxSizes(child), childHadLayout);
 }
     
@@ -1411 +1408 @@
         if (std::isfinite(extraSpace))
             childSize += LayoutUnit::fromFloatRound(extraSpace);
-        
-        LayoutUnit adjustedChildSize = adjustChildSizeForMinAndMax(child, childSize);
+
+        LayoutUnit adjustedChildSize = flexItem.constrainSizeByMinMax(childSize);
         ASSERT(adjustedChildSize >= 0);
         flexItem.flexedContentSize = adjustedChildSize;

trunk/Source/WebCore/rendering/RenderFlexibleBox.h

@@ -175 +175 @@
     LayoutUnit computeChildMarginValue(Length margin);
     void prepareOrderIteratorAndMargins();
-    LayoutUnit adjustChildSizeForMinAndMax(RenderBox& child, LayoutUnit childSize);
+    std::pair<LayoutUnit, LayoutUnit> computeFlexItemMinMaxSizes(RenderBox& child);
     LayoutUnit adjustChildSizeForAspectRatioCrossAxisMinAndMax(const RenderBox& child, LayoutUnit childSize);
     FlexItem constructFlexItem(RenderBox&, bool relayoutChildren);