<!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>[279286] trunk</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/279286">279286</a></dd>
<dt>Author</dt> <dd>svillar@igalia.com</dd>
<dt>Date</dt> <dd>2021-06-25 08:42:07 -0700 (Fri, 25 Jun 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>[css-flexbox] Improve computation of intrinsic sizes of flex items with aspect ratio
https://bugs.webkit.org/show_bug.cgi?id=227395

Reviewed by Rob Buis.

Source/WebCore:

Before computing the intrinsic sizes of flex items we first clear the overriding sizes of the item
that might have been set before in order to get the proper intrinsic size. However there is one
situation in which having an overriding size is desirable and it's when the cross size should
be set to the flex container's definite cross size. That's one of the exceptions for definite/indefinite
sizes mentioned in the specs (see https://drafts.csswg.org/css-flexbox/#definite-sizes).

In the aforementioned case we should temporarily set that overriding size in the cross axis so that
the intrinsic size could be computed with that constrain.

* rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::computeChildIntrinsicLogicalWidths const): Set the overriding size
in the cross axis to compute the intrinsic size.
(WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
(WebCore::RenderFlexibleBox::childCrossSizeShouldUseContainerCrossSize const): Added a missing check,
the cross size of the child must be auto, otherwise the rule does not apply.
(WebCore::RenderFlexibleBox::computeCrossSizeForChildUsingContainerCrossSize const): Refactored from
computeMainSizeFromAspectRatioUsing() as it's now used in two different places.
* rendering/RenderFlexibleBox.h:

LayoutTests:

* TestExpectations: Unskipped 3 tests that are now passing.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsTestExpectations">trunk/LayoutTests/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</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="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (279285 => 279286)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2021-06-25 15:39:12 UTC (rev 279285)
+++ trunk/LayoutTests/ChangeLog 2021-06-25 15:42:07 UTC (rev 279286)
</span><span class="lines">@@ -1,3 +1,12 @@
</span><ins>+2021-06-25  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] Improve computation of intrinsic sizes of flex items with aspect ratio
+        https://bugs.webkit.org/show_bug.cgi?id=227395
+
+        Reviewed by Rob Buis.
+
+        * TestExpectations: Unskipped 3 tests that are now passing.
+
</ins><span class="cx"> 2021-06-17  Sergio Villar Senin  <svillar@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         Nullptr crash in StyledMarkupAccumulator::traverseNodesForSerialization
</span></span></pre></div>
<a id="trunkLayoutTestsTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/TestExpectations (279285 => 279286)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/TestExpectations       2021-06-25 15:39:12 UTC (rev 279285)
+++ trunk/LayoutTests/TestExpectations  2021-06-25 15:42:07 UTC (rev 279286)
</span><span class="lines">@@ -3952,12 +3952,9 @@
</span><span class="cx"> 
</span><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-015.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ ImageOnlyFailure ]
</span><del>-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-001.html [ ImageOnlyFailure ]
</del><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-002.html [ ImageOnlyFailure ]
</span><del>-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-003.html [ ImageOnlyFailure ]
</del><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-004.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-005.html [ ImageOnlyFailure ]
</span><del>-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-006.html [ ImageOnlyFailure ]
</del><span class="cx"> 
</span><span class="cx"> webkit.org/b/145176 imported/w3c/web-platform-tests/css/css-flexbox/flexbox_align-items-stretch-3.html [ ImageOnlyFailure ]
</span><span class="cx"> webkit.org/b/210093 imported/w3c/web-platform-tests/css/css-flexbox/select-element-zero-height-001.html [ ImageOnlyFailure ]
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (279285 => 279286)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2021-06-25 15:39:12 UTC (rev 279285)
+++ trunk/Source/WebCore/ChangeLog      2021-06-25 15:42:07 UTC (rev 279286)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2021-06-25  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] Improve computation of intrinsic sizes of flex items with aspect ratio
+        https://bugs.webkit.org/show_bug.cgi?id=227395
+
+        Reviewed by Rob Buis.
+
+        Before computing the intrinsic sizes of flex items we first clear the overriding sizes of the item
+        that might have been set before in order to get the proper intrinsic size. However there is one
+        situation in which having an overriding size is desirable and it's when the cross size should
+        be set to the flex container's definite cross size. That's one of the exceptions for definite/indefinite
+        sizes mentioned in the specs (see https://drafts.csswg.org/css-flexbox/#definite-sizes).
+
+        In the aforementioned case we should temporarily set that overriding size in the cross axis so that
+        the intrinsic size could be computed with that constrain.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::computeChildIntrinsicLogicalWidths const): Set the overriding size
+        in the cross axis to compute the intrinsic size.
+        (WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
+        (WebCore::RenderFlexibleBox::childCrossSizeShouldUseContainerCrossSize const): Added a missing check,
+        the cross size of the child must be auto, otherwise the rule does not apply.
+        (WebCore::RenderFlexibleBox::computeCrossSizeForChildUsingContainerCrossSize const): Refactored from
+        computeMainSizeFromAspectRatioUsing() as it's now used in two different places.
+        * rendering/RenderFlexibleBox.h:
+
</ins><span class="cx"> 2021-06-25  Philippe Normand  <pnormand@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         [GStreamer] TextCombiner has unlinked internal encoders
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderFlexibleBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (279285 => 279286)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp     2021-06-25 15:39:12 UTC (rev 279285)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp        2021-06-25 15:42:07 UTC (rev 279286)
</span><span class="lines">@@ -212,6 +212,16 @@
</span><span class="cx">     ASSERT(childObject.isBox());
</span><span class="cx">     RenderBox& child = downcast<RenderBox>(childObject);
</span><span class="cx"> 
</span><ins>+    // If the item cross size should use the definite container cross size then set the overriding size now so
+    // the intrinsic sizes are properly computed in the presence of aspect ratios. The only exception is when
+    // we are both a flex item&container, because our parent might have already set our overriding size.
+    if (childCrossSizeShouldUseContainerCrossSize(child) && !isFlexItem()) {
+        auto axis = mainAxisIsChildInlineAxis(child) ? OverridingSizesScope::Axis::Block : OverridingSizesScope::Axis::Inline;
+        OverridingSizesScope overridingSizeScope(child, axis, computeCrossSizeForChildUsingContainerCrossSize(child));
+        RenderBlock::computeChildIntrinsicLogicalWidths(childObject, minPreferredLogicalWidth, maxPreferredLogicalWidth);
+        return;
+    }
+
</ins><span class="cx">     OverridingSizesScope cleanOverridingSizesScope(child, OverridingSizesScope::Axis::Both);
</span><span class="cx">     RenderBlock::computeChildIntrinsicLogicalWidths(childObject, minPreferredLogicalWidth, maxPreferredLogicalWidth);
</span><span class="cx"> }
</span><span class="lines">@@ -869,13 +879,9 @@
</span><span class="cx">     std::optional<LayoutUnit> crossSize;
</span><span class="cx">     if (crossSizeLength.isFixed())
</span><span class="cx">         crossSize = adjustForBoxSizing(child, crossSizeLength);
</span><del>-    else if (crossSizeLength.isAuto()) {
-        ASSERT(childCrossSizeShouldUseContainerCrossSize(child));
-        auto containerCrossSizeLength = isHorizontalFlow() ? style().height() : style().width();
-        // Keep this sync'ed with childCrossSizeShouldUseContainerCrossSize().
-        ASSERT(containerCrossSizeLength.isFixed());
-        crossSize = std::max(0_lu, valueForLength(containerCrossSizeLength, -1_lu) - crossAxisMarginExtentForChild(child));
-    } else {
</del><ins>+    else if (crossSizeLength.isAuto())
+        crossSize = computeCrossSizeForChildUsingContainerCrossSize(child);
+    else {
</ins><span class="cx">         ASSERT(crossSizeLength.isPercentOrCalculated());
</span><span class="cx">         crossSize = mainAxisIsChildInlineAxis(child) ? child.computePercentageLogicalHeight(crossSizeLength) : adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth()), crossSizeLength.type());
</span><span class="cx">         if (!crossSize)
</span><span class="lines">@@ -948,7 +954,7 @@
</span><span class="cx">     // 1. If a single-line flex container has a definite cross size, the automatic preferred outer cross size of any
</span><span class="cx">     // stretched flex items is the flex container's inner cross size (clamped to the flex item's min and max cross size)
</span><span class="cx">     // and is considered definite.
</span><del>-    if (!isMultiline() && alignmentForChild(child) == ItemPosition::Stretch && !hasAutoMarginsInCrossAxis(child)) {
</del><ins>+    if (!isMultiline() && alignmentForChild(child) == ItemPosition::Stretch && !hasAutoMarginsInCrossAxis(child) && crossSizeLengthForChild(MainOrPreferredSize, child).isAuto()) {
</ins><span class="cx">         // This must be kept in sync with computeMainSizeFromAspectRatioUsing().
</span><span class="cx">         // FIXME: so far we're only considered fixed sizes but we should extend it to other definite sizes.
</span><span class="cx">         auto& crossSize = isHorizontalFlow() ? style().height() : style().width();
</span><span class="lines">@@ -1667,6 +1673,15 @@
</span><span class="cx">     return positionChanged;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+// This refers to https://drafts.csswg.org/css-flexbox-1/#definite-sizes, section 1).
+LayoutUnit RenderFlexibleBox::computeCrossSizeForChildUsingContainerCrossSize(const RenderBox& child) const
+{
+    auto containerCrossSizeLength = isHorizontalFlow() ? style().height() : style().width();
+    // Keep this sync'ed with childCrossSizeShouldUseContainerCrossSize().
+    ASSERT(containerCrossSizeLength.isFixed());
+    return std::max(0_lu, valueForLength(containerCrossSizeLength, -1_lu) - crossAxisMarginExtentForChild(child));
+}
+
</ins><span class="cx"> void RenderFlexibleBox::prepareChildForPositionedLayout(RenderBox& child)
</span><span class="cx"> {
</span><span class="cx">     ASSERT(child.isOutOfFlowPositioned());
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderFlexibleBoxh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.h (279285 => 279286)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderFlexibleBox.h       2021-06-25 15:39:12 UTC (rev 279285)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.h  2021-06-25 15:42:07 UTC (rev 279286)
</span><span class="lines">@@ -145,6 +145,7 @@
</span><span class="cx">     bool childHasComputableAspectRatio(const RenderBox&) const;
</span><span class="cx">     bool childHasComputableAspectRatioAndCrossSizeIsConsideredDefinite(const RenderBox&);
</span><span class="cx">     bool childCrossSizeShouldUseContainerCrossSize(const RenderBox& child) const;
</span><ins>+    LayoutUnit computeCrossSizeForChildUsingContainerCrossSize(const RenderBox& child) const;
</ins><span class="cx">     void computeChildIntrinsicLogicalWidths(RenderObject&, LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
</span><span class="cx">     LayoutUnit computeMainSizeFromAspectRatioUsing(const RenderBox& child, Length crossSizeLength) const;
</span><span class="cx">     void setFlowAwareLocationForChild(RenderBox& child, const LayoutPoint&);
</span></span></pre>
</div>
</div>

</body>
</html>