Changeset 279286 in webkit

Timestamp:

Jun 25, 2021 8:42:07 AM (13 months ago)

Author:

svillar@igalia.com

Message:

[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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderFlexibleBox.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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.
+
 2021-06-17  Sergio Villar Senin  <svillar@igalia.com>
 

trunk/LayoutTests/TestExpectations

@@ -3953 +3953 @@
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-column-015.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ ImageOnlyFailure ]
-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-001.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-002.html [ ImageOnlyFailure ]
-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-003.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-004.html [ ImageOnlyFailure ]
 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-005.html [ ImageOnlyFailure ]
-webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-006.html [ ImageOnlyFailure ]
 
 webkit.org/b/145176 imported/w3c/web-platform-tests/css/css-flexbox/flexbox_align-items-stretch-3.html [ ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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:
+
 2021-06-25  Philippe Normand  <pnormand@igalia.com>
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -213 +213 @@
     RenderBox& child = downcast<RenderBox>(childObject);
 
+    // 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;
+    }
+
     OverridingSizesScope cleanOverridingSizesScope(child, OverridingSizesScope::Axis::Both);
     RenderBlock::computeChildIntrinsicLogicalWidths(childObject, minPreferredLogicalWidth, maxPreferredLogicalWidth);
@@ -870 +880 @@
     if (crossSizeLength.isFixed())
         crossSize = adjustForBoxSizing(child, crossSizeLength);
-    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 {
+    else if (crossSizeLength.isAuto())
+        crossSize = computeCrossSizeForChildUsingContainerCrossSize(child);
+    else {
         ASSERT(crossSizeLength.isPercentOrCalculated());
         crossSize = mainAxisIsChildInlineAxis(child) ? child.computePercentageLogicalHeight(crossSizeLength) : adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth()), crossSizeLength.type());
@@ -949 +955 @@
     // stretched flex items is the flex container's inner cross size (clamped to the flex item's min and max cross size)
     // and is considered definite.
-    if (!isMultiline() && alignmentForChild(child) == ItemPosition::Stretch && !hasAutoMarginsInCrossAxis(child)) {
+    if (!isMultiline() && alignmentForChild(child) == ItemPosition::Stretch && !hasAutoMarginsInCrossAxis(child) && crossSizeLengthForChild(MainOrPreferredSize, child).isAuto()) {
         // This must be kept in sync with computeMainSizeFromAspectRatioUsing().
         // FIXME: so far we're only considered fixed sizes but we should extend it to other definite sizes.
@@ -1668 +1674 @@
 }
 
+// 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));
+}
+
 void RenderFlexibleBox::prepareChildForPositionedLayout(RenderBox& child)
 {

trunk/Source/WebCore/rendering/RenderFlexibleBox.h

@@ -146 +146 @@
     bool childHasComputableAspectRatioAndCrossSizeIsConsideredDefinite(const RenderBox&);
     bool childCrossSizeShouldUseContainerCrossSize(const RenderBox& child) const;
+    LayoutUnit computeCrossSizeForChildUsingContainerCrossSize(const RenderBox& child) const;
     void computeChildIntrinsicLogicalWidths(RenderObject&, LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const override;
     LayoutUnit computeMainSizeFromAspectRatioUsing(const RenderBox& child, Length crossSizeLength) const;