<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[278865] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/278865">278865</a></dd>
<dt>Author</dt> <dd>svillar@igalia.com</dd>
<dt>Date</dt> <dd>2021-06-15 02:19:23 -0700 (Tue, 15 Jun 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>[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:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingFlexibleBoxAlgorithmcpp">trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingFlexibleBoxAlgorithmh">trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderFlexibleBoxcpp">trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderFlexibleBoxh">trunk/Source/WebCore/rendering/RenderFlexibleBox.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (278864 => 278865)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2021-06-15 08:32:33 UTC (rev 278864)
+++ trunk/Source/WebCore/ChangeLog      2021-06-15 09:19:23 UTC (rev 278865)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+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:
+
</ins><span class="cx"> 2021-06-11  Sergio Villar Senin  <svillar@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         logged in GitHub issue pages have bad layout for "Notifications Customize" link
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingFlexibleBoxAlgorithmcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.cpp (278864 => 278865)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.cpp  2021-06-15 08:32:33 UTC (rev 278864)
+++ trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.cpp     2021-06-15 09:19:23 UTC (rev 278865)
</span><span class="lines">@@ -35,12 +35,14 @@
</span><span class="cx"> 
</span><span class="cx"> namespace WebCore {
</span><span class="cx"> 
</span><del>-FlexItem::FlexItem(RenderBox& box, LayoutUnit flexBaseContentSize, LayoutUnit hypotheticalMainContentSize, LayoutUnit mainAxisBorderAndPadding, LayoutUnit mainAxisMargin, bool everHadLayout)
</del><ins>+FlexItem::FlexItem(RenderBox& box, LayoutUnit flexBaseContentSize, LayoutUnit mainAxisBorderAndPadding, LayoutUnit mainAxisMargin, std::pair<LayoutUnit, LayoutUnit> minMaxSizes, bool everHadLayout)
</ins><span class="cx">     : box(box)
</span><span class="cx">     , flexBaseContentSize(flexBaseContentSize)
</span><del>-    , hypotheticalMainContentSize(hypotheticalMainContentSize)
</del><span class="cx">     , mainAxisBorderAndPadding(mainAxisBorderAndPadding)
</span><span class="cx">     , mainAxisMargin(mainAxisMargin)
</span><ins>+    , minMaxSizes(minMaxSizes)
+    , hypotheticalMainContentSize(constrainSizeByMinMax(flexBaseContentSize))
+    , frozen(false)
</ins><span class="cx">     , everHadLayout(everHadLayout)
</span><span class="cx"> {
</span><span class="cx">     ASSERT(!box.isOutOfFlowPositioned());
</span><span class="lines">@@ -86,4 +88,9 @@
</span><span class="cx">     return lineItems.size() > 0;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+LayoutUnit FlexItem::constrainSizeByMinMax(const LayoutUnit size) const
+{
+    return std::max(minMaxSizes.first, std::min(size, minMaxSizes.second));
+}
+
</ins><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingFlexibleBoxAlgorithmh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.h (278864 => 278865)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.h    2021-06-15 08:32:33 UTC (rev 278864)
+++ trunk/Source/WebCore/rendering/FlexibleBoxAlgorithm.h       2021-06-15 09:19:23 UTC (rev 278865)
</span><span class="lines">@@ -41,7 +41,7 @@
</span><span class="cx"> 
</span><span class="cx"> class FlexItem {
</span><span class="cx"> public:
</span><del>-    FlexItem(RenderBox&, LayoutUnit flexBaseContentSize, LayoutUnit hypotheticalMainContentSize, LayoutUnit mainAxisBorderAndPadding, LayoutUnit mainAxisMargin, bool everHadLayout);
</del><ins>+    FlexItem(RenderBox&, LayoutUnit flexBaseContentSize, LayoutUnit mainAxisBorderAndPadding, LayoutUnit mainAxisMargin, std::pair<LayoutUnit, LayoutUnit> minMaxSizes, bool everHadLayout);
</ins><span class="cx"> 
</span><span class="cx">     LayoutUnit hypotheticalMainAxisMarginBoxSize() const
</span><span class="cx">     {
</span><span class="lines">@@ -58,11 +58,14 @@
</span><span class="cx">         return flexedContentSize + mainAxisBorderAndPadding + mainAxisMargin;
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    LayoutUnit constrainSizeByMinMax(const LayoutUnit) const;
+
</ins><span class="cx">     RenderBox& box;
</span><span class="cx">     const LayoutUnit flexBaseContentSize;
</span><del>-    const LayoutUnit hypotheticalMainContentSize;
</del><span class="cx">     const LayoutUnit mainAxisBorderAndPadding;
</span><span class="cx">     const LayoutUnit mainAxisMargin;
</span><ins>+    const std::pair<LayoutUnit, LayoutUnit> minMaxSizes;
+    const LayoutUnit hypotheticalMainContentSize;
</ins><span class="cx">     LayoutUnit flexedContentSize;
</span><span class="cx">     bool frozen { false };
</span><span class="cx">     bool everHadLayout { false };
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderFlexibleBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (278864 => 278865)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp     2021-06-15 08:32:33 UTC (rev 278864)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp        2021-06-15 09:19:23 UTC (rev 278865)
</span><span class="lines">@@ -1201,19 +1201,17 @@
</span><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-LayoutUnit RenderFlexibleBox::adjustChildSizeForMinAndMax(RenderBox& child, LayoutUnit childSize)
</del><ins>+std::pair<LayoutUnit, LayoutUnit> RenderFlexibleBox::computeFlexItemMinMaxSizes(RenderBox& child)
</ins><span class="cx"> {
</span><span class="cx">     Length max = mainSizeLengthForChild(MaxSize, child);
</span><span class="cx">     std::optional<LayoutUnit> maxExtent = std::nullopt;
</span><del>-    if (max.isSpecifiedOrIntrinsic()) {
</del><ins>+    if (max.isSpecifiedOrIntrinsic())
</ins><span class="cx">         maxExtent = computeMainAxisExtentForChild(child, MaxSize, max);
</span><del>-        childSize = std::min(childSize, maxExtent.value_or(childSize));
-    }
</del><span class="cx"> 
</span><span class="cx">     Length min = mainSizeLengthForChild(MinSize, child);
</span><span class="cx">     // Intrinsic sizes in child's block axis are handled by the min-size:auto code path.
</span><span class="cx">     if (min.isSpecified() || (min.isIntrinsic() && mainAxisIsChildInlineAxis(child)))
</span><del>-        return std::max(childSize, std::max(0_lu, computeMainAxisExtentForChild(child, MinSize, min).value_or(childSize)));
</del><ins>+        return { computeMainAxisExtentForChild(child, MinSize, min).value_or(0_lu), maxExtent.value_or(LayoutUnit::max()) };
</ins><span class="cx">     
</span><span class="cx">     if (shouldApplyMinSizeAutoForChild(child)) {
</span><span class="cx">         // FIXME: If the min value is expected to be valid here, we need to come up with a non optional version of computeMainAxisExtentForChild and
</span><span class="lines">@@ -1234,19 +1232,19 @@
</span><span class="cx">             LayoutUnit resolvedMainSize = computeMainAxisExtentForChild(child, MainOrPreferredSize, mainSize).value_or(0);
</span><span class="cx">             ASSERT(resolvedMainSize >= 0);
</span><span class="cx">             LayoutUnit specifiedSize = std::min(resolvedMainSize, maxExtent.value_or(resolvedMainSize));
</span><del>-            return std::max(childSize, std::min(specifiedSize, contentSize));
</del><ins>+            return { std::min(specifiedSize, contentSize), maxExtent.value_or(LayoutUnit::max()) };
</ins><span class="cx">         }
</span><span class="cx"> 
</span><span class="cx">         if (child.isRenderReplaced() && childHasComputableAspectRatioAndCrossSizeIsConsideredDefinite(child)) {
</span><span class="cx">             LayoutUnit transferredSize = computeMainSizeFromAspectRatioUsing(child, childCrossSizeLength);
</span><span class="cx">             transferredSize = adjustChildSizeForAspectRatioCrossAxisMinAndMax(child, transferredSize);
</span><del>-            return std::max(childSize, std::min(transferredSize, contentSize));
</del><ins>+            return { std::min(transferredSize, contentSize), maxExtent.value_or(LayoutUnit::max()) };
</ins><span class="cx">         }
</span><span class="cx"> 
</span><del>-        return std::max(childSize, contentSize);
</del><ins>+        return { contentSize, maxExtent.value_or(LayoutUnit::max()) };
</ins><span class="cx">     }
</span><span class="cx"> 
</span><del>-    return std::max(0_lu, childSize);
</del><ins>+    return { 0_lu, maxExtent.value_or(LayoutUnit::max()) };
</ins><span class="cx"> }
</span><span class="cx">     
</span><span class="cx"> bool RenderFlexibleBox::useChildOverridingCrossSizeForPercentageResolution(const RenderBox& child)
</span><span class="lines">@@ -1334,9 +1332,8 @@
</span><span class="cx">     
</span><span class="cx">     LayoutUnit borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
</span><span class="cx">     LayoutUnit childInnerFlexBaseSize = computeInnerFlexBaseSizeForChild(child, borderAndPadding);
</span><del>-    LayoutUnit childMinMaxAppliedMainAxisExtent = adjustChildSizeForMinAndMax(child, childInnerFlexBaseSize);
</del><span class="cx">     LayoutUnit margin = isHorizontalFlow() ? child.horizontalMarginExtent() : child.verticalMarginExtent();
</span><del>-    return FlexItem(child, childInnerFlexBaseSize, childMinMaxAppliedMainAxisExtent, borderAndPadding, margin, childHadLayout);
</del><ins>+    return FlexItem(child, childInnerFlexBaseSize, borderAndPadding, margin, computeFlexItemMinMaxSizes(child), childHadLayout);
</ins><span class="cx"> }
</span><span class="cx">     
</span><span class="cx"> void RenderFlexibleBox::freezeViolations(Vector<FlexItem*>& violations, LayoutUnit& availableFreeSpace, double& totalFlexGrow, double& totalFlexShrink, double& totalWeightedFlexShrink)
</span><span class="lines">@@ -1410,8 +1407,8 @@
</span><span class="cx">             extraSpace = remainingFreeSpace * child.style().flexShrink() * flexItem.flexBaseContentSize / totalWeightedFlexShrink;
</span><span class="cx">         if (std::isfinite(extraSpace))
</span><span class="cx">             childSize += LayoutUnit::fromFloatRound(extraSpace);
</span><del>-        
-        LayoutUnit adjustedChildSize = adjustChildSizeForMinAndMax(child, childSize);
</del><ins>+
+        LayoutUnit adjustedChildSize = flexItem.constrainSizeByMinMax(childSize);
</ins><span class="cx">         ASSERT(adjustedChildSize >= 0);
</span><span class="cx">         flexItem.flexedContentSize = adjustedChildSize;
</span><span class="cx">         usedFreeSpace += adjustedChildSize - flexItem.flexBaseContentSize;
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderFlexibleBoxh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.h (278864 => 278865)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderFlexibleBox.h       2021-06-15 08:32:33 UTC (rev 278864)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.h  2021-06-15 09:19:23 UTC (rev 278865)
</span><span class="lines">@@ -174,7 +174,7 @@
</span><span class="cx">     
</span><span class="cx">     LayoutUnit computeChildMarginValue(Length margin);
</span><span class="cx">     void prepareOrderIteratorAndMargins();
</span><del>-    LayoutUnit adjustChildSizeForMinAndMax(RenderBox& child, LayoutUnit childSize);
</del><ins>+    std::pair<LayoutUnit, LayoutUnit> computeFlexItemMinMaxSizes(RenderBox& child);
</ins><span class="cx">     LayoutUnit adjustChildSizeForAspectRatioCrossAxisMinAndMax(const RenderBox& child, LayoutUnit childSize);
</span><span class="cx">     FlexItem constructFlexItem(RenderBox&, bool relayoutChildren);
</span><span class="cx">     
</span></span></pre>
</div>
</div>

</body>
</html>