Changeset 277371 in webkit


Ignore:
Timestamp:
May 12, 2021 8:59:48 AM (14 months ago)
Author:
svillar@igalia.com
Message:

[css-flexbox] Do not use margins when computing aspect ratio cross sizes
​https://bugs.webkit.org/show_bug.cgi?id=221210
LayoutTests/imported/w3c:

<rdar://problem/74097534>

Reviewed by Javier Fernandez.

  • web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt: Replaced FAIL by PASS expectations + new expectations.
  • web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html: Imported

latest changes from upstream WPT.

Source/WebCore:

Reviewed by Javier Fernandez.

In r270578 we implemented section 9.8.1 of the flexbox specs which allowed us to compute child's
indefinite cross sizes as definite when the flexbox container had a definite cross size and a couple
of other conditions. However we did not take into account that the child might have some margins in
in the cross direction. Aspect ratio computations must use the content box and thus, we need to
substract the margin extent before trying to compute a cross size based on an aspect ratio.

Note that when computeMainSizeFromAspectRatioUsing() is called the child might not have been ever
laid out. This means that the margins wouldn't have been computed and thus marginXXX() would always
return 0. That's why crossAxisMarginExtentForChild() was also modified so that it actually computes
the margins in case the child needs to be laid out.

  • rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::crossAxisMarginExtentForChild const):
(WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):

LayoutTests:

Reviewed by Javier Fernandez.

  • TestExpectations: Unskipped flex-aspect-ratio-img-row-013.html which is now passing.
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r277368 r277371  
     12021-05-12  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-flexbox] Do not use margins when computing aspect ratio cross sizes
     4        https://bugs.webkit.org/show_bug.cgi?id=221210
     5
     6        Reviewed by Javier Fernandez.
     7
     8        * TestExpectations: Unskipped flex-aspect-ratio-img-row-013.html which is now passing.
     9
    1102021-05-12  Diego Pino Garcia  <dpino@igalia.com>
    211

  • trunk/LayoutTests/TestExpectations

    r277321 r277371  
    38703870webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-007.html [ ImageOnlyFailure ]
    38713871webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ ImageOnlyFailure ]
    3872 webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html [ ImageOnlyFailure ]
    38733872webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-001.html [ ImageOnlyFailure ]
    38743873webkit.org/b/219343 imported/w3c/web-platform-tests/css/css-flexbox/aspect-ratio-intrinsic-size-002.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r277321 r277371  
     12021-05-12  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-flexbox] Do not use margins when computing aspect ratio cross sizes
     4        https://bugs.webkit.org/show_bug.cgi?id=221210
     5        <rdar://problem/74097534>
     6
     7        Reviewed by Javier Fernandez.
     8
     9        * web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt:
     10         Replaced FAIL by PASS expectations + new expectations.
     11        * web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html: Imported
     12        latest changes from upstream WPT.
     13
    1142021-05-11  Cathie Chen  <cathiechen@igalia.com>
    215

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013-expected.txt

    r273072 r277371  
    1 Pass condition is 3 green 100x100 squares.
     1Pass condition is 4 green 100x100 squares and 1 0x0 square.
    22
    33Firefox 84a1 passes. Chrome 87 fails them all by making the green rectangles be 200x100.
    … …  
    99
    1010
     11Have to subtract a margin larger than the stretched height to get 0px transferred size suggestion:
     12
     13
     14Have to subtract the margin from the stretched height (ignoring the presence of a border) to get the transferred size suggestion:
     15
     16
    1117Stretched transferred size suggestion has to obey min-height:
    1218
    … …  
    1420
    1521PASS img 1
    16 FAIL img 2 assert_equals:
    17 <img src="support/200x200-green.png" style="margin-bottom: 20px" data-expected-height="100" data-expected-width="100">
    18 width expected 100 but got 120
     22PASS img 2
    1923PASS img 3
     24PASS img 4
     25PASS img 5
    2026

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flex-aspect-ratio-img-row-013.html

    r270237 r277371  
    1111<body onload="checkLayout('img')">
    1212<p>
    13 Pass condition is 3 green 100x100 squares.
     13Pass condition is 4 green 100x100 squares and 1 0x0 square.
    1414</p>
    1515
    … …  
    2828</div>
    2929
     30<p>Have to subtract a margin larger than the stretched height to get 0px transferred size suggestion:</p>
     31<div style="display: flex; width: 0; height: 120px;">
     32  <img src="support/200x200-green.png" style="margin-bottom: 160px" data-expected-height=0 data-expected-width=0>
     33</div>
     34
     35<p>Have to subtract the margin from the stretched height (ignoring the presence of a border) to get the transferred size suggestion:</p>
     36<div style="display: flex; width: 0; height: 120px;">
     37  <img src="support/200x200-green.png" style="border-right: 10px solid black; margin-bottom: 20px" data-expected-height=100 data-expected-width=110>
     38</div>
     39
    3040<p>Stretched transferred size suggestion has to obey min-height:</p>
    3141<div style="display: flex; width: 0; height: 50px;">

  • trunk/Source/WebCore/ChangeLog

    r277369 r277371  
     12021-05-12  Sergio Villar Senin  <svillar@igalia.com>
     2
     3        [css-flexbox] Do not use margins when computing aspect ratio cross sizes
     4        https://bugs.webkit.org/show_bug.cgi?id=221210
     5
     6        Reviewed by Javier Fernandez.
     7
     8        In r270578 we implemented section 9.8.1 of the flexbox specs which allowed us to compute child's
     9        indefinite cross sizes as definite when the flexbox container had a definite cross size and a couple
     10        of other conditions. However we did not take into account that the child might have some margins in
     11        in the cross direction. Aspect ratio computations must use the content box and thus, we need to
     12        substract the margin extent before trying to compute a cross size based on an aspect ratio.
     13
     14        Note that when computeMainSizeFromAspectRatioUsing() is called the child might not have been ever
     15        laid out. This means that the margins wouldn't have been computed and thus marginXXX() would always
     16        return 0. That's why crossAxisMarginExtentForChild() was also modified so that it actually computes
     17        the margins in case the child needs to be laid out.
     18
     19        * rendering/RenderFlexibleBox.cpp:
     20        (WebCore::RenderFlexibleBox::crossAxisMarginExtentForChild const):
     21        (WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
     22
    1232021-05-12  Sam Weinig  <weinig@apple.com>
    224

  • trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

    r277321 r277371  
    742742LayoutUnit RenderFlexibleBox::crossAxisMarginExtentForChild(const RenderBox& child) const
    743743{
    744     return isHorizontalFlow() ? child.verticalMarginExtent() : child.horizontalMarginExtent();
     744    if (!child.needsLayout())
     745        return isHorizontalFlow() ? child.verticalMarginExtent() : child.horizontalMarginExtent();
     746
     747    LayoutUnit marginStart;
     748    LayoutUnit marginEnd;
     749    if (isHorizontalFlow())
     750        child.computeBlockDirectionMargins(*this, marginStart, marginEnd);
     751    else
     752        child.computeInlineDirectionMargins(*this, child.containingBlockLogicalWidthForContentInFragment(nullptr), child.logicalWidth(), marginStart, marginEnd);
     753    return marginStart + marginEnd;
    745754}
    746755
    … …  
    819828        // Keep this sync'ed with childCrossSizeShouldUseContainerCrossSize().
    820829        ASSERT(containerCrossSizeLength.isFixed());
    821         crossSize = valueForLength(containerCrossSizeLength, -1_lu);
     830        crossSize = std::max(0_lu, valueForLength(containerCrossSizeLength, -1_lu) - crossAxisMarginExtentForChild(child));
    822831    } else {
    823832        ASSERT(crossSizeLength.isPercentOrCalculated());
Note: See TracChangeset for help on using the changeset viewer.