Attachment 418553 Details for Bug 219679 – Patch

[patch] Patch

bug-219679-20210127181807.patch (text/plain), 12.02 KB, created by Rob Buis on 2021-01-27 09:18:09 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Rob Buis

Created: 2021-01-27 09:18:09 PST

Size: 12.02 KB

patch

obsolete

>Subversion Revision: 271954
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 1ddc68adddcde2315ba84b7639f50868d3cb6a7d..7364e2e86b5b2edbad22d8bf5fb24945f0a710bb 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2021-01-27  Rob Buis  <rbuis@igalia.com>
>+
>+        Support aspect-ratio on flexbox children
>+        https://bugs.webkit.org/show_bug.cgi?id=219679
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WIP.
>+
>+        * rendering/RenderBox.cpp:
>+        (WebCore::RenderBox::cacheIntrinsicContentLogicalHeightForFlexItem const):
>+        (WebCore::RenderBox::shouldComputeLogicalWidthFromAspectRatio const):
>+        * rendering/RenderFlexibleBox.cpp:
>+        (WebCore::childHasAspectRatio):
>+        (WebCore::RenderFlexibleBox::computeMainAxisExtentForChild):
>+        (WebCore::RenderFlexibleBox::useChildAspectRatio const):
>+        (WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
>+        (WebCore::RenderFlexibleBox::childCrossSizeIsDefinite const):
>+        (WebCore::RenderFlexibleBox::computeInnerFlexBaseSizeForChild):
>+        (WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax):
>+        (WebCore::RenderFlexibleBox::needToStretchChildLogicalHeight const):
>+
> 2021-01-27  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Expand "empty line" fast path support for multiline inline boxes
>diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
>index 98d2c0a2995eeb435cd17265f1e73ff04edf5112..ae29425d1dd5a662ee1b041e617112966e71edb3 100644
>--- a/Source/WebCore/rendering/RenderBox.cpp
>+++ b/Source/WebCore/rendering/RenderBox.cpp
>@@ -2861,7 +2861,7 @@ void RenderBox::cacheIntrinsicContentLogicalHeightForFlexItem(LayoutUnit height)
>     // FIXME: it should be enough with checking hasOverridingLogicalHeight() as this logic could be shared
>     // by any layout system using overrides like grid or flex. However this causes a never ending sequence of calls
>     // between layoutBlock() <-> relayoutToAvoidWidows().
>-    if (isFloatingOrOutOfFlowPositioned() || !parent() || !parent()->isFlexibleBox() || hasOverridingLogicalHeight())
>+    if (isFloatingOrOutOfFlowPositioned() || !parent() || !parent()->isFlexibleBox() || hasOverridingLogicalHeight() || shouldComputeLogicalHeightFromAspectRatio())
>         return;
>     downcast<RenderFlexibleBox>(parent())->setCachedChildIntrinsicContentLogicalHeight(*this, height);
> }
>@@ -5086,7 +5086,7 @@ bool RenderBox::shouldComputeLogicalWidthFromAspectRatio() const
>     if (!style().hasAspectRatio())
>         return false;
> 
>-    if (!shouldComputeLogicalWidthFromAspectRatioAndInsets() && !style().logicalHeight().isFixed() && !style().logicalHeight().isPercentOrCalculated())
>+    if (!hasOverridingLogicalHeight() && !shouldComputeLogicalWidthFromAspectRatioAndInsets() && !style().logicalHeight().isFixed() && !style().logicalHeight().isPercentOrCalculated())
>         return false;
> 
>     return true;
>diff --git a/Source/WebCore/rendering/RenderFlexibleBox.cpp b/Source/WebCore/rendering/RenderFlexibleBox.cpp
>index a736c45a8176cc6d9a966834856f7171db0a81a2..d6635c9dda5f61c01d40074b499bd533c5fdfe7c 100644
>--- a/Source/WebCore/rendering/RenderFlexibleBox.cpp
>+++ b/Source/WebCore/rendering/RenderFlexibleBox.cpp
>@@ -543,6 +543,11 @@ LayoutUnit RenderFlexibleBox::mainAxisContentExtent(LayoutUnit contentLogicalHei
>     return std::max(0_lu, computedValues.m_extent - borderPaddingAndScrollbar);
> }
> 
>+static bool childHasAspectRatio(const RenderBox& child)
>+{
>+    return child.hasAspectRatio() || child.style().hasAspectRatio();
>+}
>+
> Optional<LayoutUnit> RenderFlexibleBox::computeMainAxisExtentForChild(const RenderBox& child, SizeType sizeType, const Length& size)
> {
>     // If we have a horizontal flow, that means the main size is the width.
>@@ -569,7 +574,7 @@ Optional<LayoutUnit> RenderFlexibleBox::computeMainAxisExtentForChild(const Rend
>     // our logical width is auto, we can just use our cached value. So let's do
>     // that here. (Compare code in LayoutBlock::computePreferredLogicalWidths)
>     LayoutUnit borderAndPadding = child.borderAndPaddingLogicalWidth();
>-    if (child.style().logicalWidth().isAuto() && !child.hasAspectRatio()) {
>+    if (child.style().logicalWidth().isAuto() && !childHasAspectRatio(child)) {
>         if (size.type() == MinContent)
>             return child.minPreferredLogicalWidth() - borderAndPadding;
>         if (size.type() == MaxContent)
>@@ -739,9 +744,9 @@ LayoutPoint RenderFlexibleBox::flowAwareLocationForChild(const RenderBox& child)
> 
> bool RenderFlexibleBox::useChildAspectRatio(const RenderBox& child) const
> {
>-    if (!child.hasAspectRatio())
>+    if (!childHasAspectRatio(child))
>         return false;
>-    if (!child.intrinsicSize().height()) {
>+    if (!child.intrinsicSize().height() && !child.style().hasAspectRatio()) {
>         // We can't compute a ratio in this case.
>         return false;
>     }
>@@ -756,8 +761,7 @@ bool RenderFlexibleBox::useChildAspectRatio(const RenderBox& child) const
>     
> LayoutUnit RenderFlexibleBox::computeMainSizeFromAspectRatioUsing(const RenderBox& child, Length crossSizeLength) const
> {
>-    ASSERT(child.hasAspectRatio());
>-    ASSERT(child.intrinsicSize().height());
>+    ASSERT(childHasAspectRatio(child));
> 
>     auto adjustForBoxSizing = [this] (const RenderBox& box, Length length) -> LayoutUnit {
>         ASSERT(length.isFixed());
>@@ -786,7 +790,13 @@ LayoutUnit RenderFlexibleBox::computeMainSizeFromAspectRatioUsing(const RenderBo
>     }
> 
>     const LayoutSize& childIntrinsicSize = child.intrinsicSize();
>-    double ratio = childIntrinsicSize.width().toFloat() / childIntrinsicSize.height().toFloat();
>+    double ratio;
>+    if (child.style().aspectRatioType() == AspectRatioType::Ratio || (child.style().aspectRatioType() == AspectRatioType::AutoAndRatio && childIntrinsicSize.isEmpty()))
>+        ratio = child.style().aspectRatioWidth() / child.style().aspectRatioHeight();
>+    else {
>+        ASSERT(childIntrinsicSize.height());
>+        ratio = childIntrinsicSize.width().toFloat() / childIntrinsicSize.height().toFloat();
>+    }
>     if (isHorizontalFlow())
>         return LayoutUnit(crossSize.value() * ratio);
>     return LayoutUnit(crossSize.value() / ratio);
>@@ -833,7 +843,7 @@ bool RenderFlexibleBox::childCrossSizeIsDefinite(const RenderBox& child, const L
>         // 1. If a single-line flex container has a definite cross size, the automatic preferred outer cross size of any
>         // 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) {
>+        if (!isMultiline() && alignmentForChild(child) == ItemPosition::Stretch && !hasAutoMarginsInCrossAxis(child)) {
>             // 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.
>             if (auto& crossSize = isHorizontalFlow() ? style().height() : style().width(); crossSize.isFixed())
>@@ -896,7 +906,7 @@ LayoutUnit RenderFlexibleBox::computeInnerFlexBaseSizeForChild(RenderBox& child,
>     // width of the child. For the logical width axis we just use the preferred
>     // width; for the height we need to lay out the child.
>     LayoutUnit mainAxisExtent;
>-    if (!mainAxisIsChildInlineAxis(child)) {
>+    if (!mainAxisIsChildInlineAxis(child) || child.style().hasAspectRatio()) {
>         updateBlockChildDirtyBitsBeforeLayout(relayoutChildren, child);
>         if (child.needsLayout() || relayoutChildren || !m_intrinsicSizeAlongMainAxis.contains(&child)) {
>             if (!child.needsLayout())
>@@ -1167,7 +1177,7 @@ LayoutUnit RenderFlexibleBox::adjustChildSizeForMinAndMax(const RenderBox& child
>         // ensure it's valid through the virtual calls of computeIntrinsicLogicalContentHeightUsing.
>         LayoutUnit contentSize = computeMainAxisExtentForChild(child, MinSize, Length(MinContent)).valueOr(0);
>         ASSERT(contentSize >= 0);
>-        if (child.hasAspectRatio() && child.intrinsicSize().height() > 0)
>+        if (childHasAspectRatio(child) && child.intrinsicSize().height() > 0)
>             contentSize = adjustChildSizeForAspectRatioCrossAxisMinAndMax(child, contentSize);
>         contentSize = std::min(contentSize, maxExtent.valueOr(contentSize));
>         
>@@ -1582,7 +1592,7 @@ bool RenderFlexibleBox::needToStretchChildLogicalHeight(const RenderBox& child)
>         return false;
> 
>     // Aspect ratio is properly handled by RenderReplaced during layout.
>-    if (child.isRenderReplaced() && child.hasAspectRatio())
>+    if (child.isRenderReplaced() && childHasAspectRatio(child))
>         return false;
> 
>     return child.style().logicalHeight().isAuto();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index fb8e54c52d16052d3ef7d9ef20ac8b2e49951bbb..42fa899e20e4858f7fdf9d4b3975d0950b9e4801 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2021-01-27  Rob Buis  <rbuis@igalia.com>
>+
>+        Support aspect-ratio on flexbox children
>+        https://bugs.webkit.org/show_bug.cgi?id=219679
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Unskip some tests that pass now.
>+
>+        * TestExpectations:
>+
> 2021-01-27  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, reverting r271940.
>diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
>index eee16e627366df77e3ced05b59437d3edbb958c4..46bd2c829f1b5ae43217047a74036ca619c1cc12 100644
>--- a/LayoutTests/TestExpectations
>+++ b/LayoutTests/TestExpectations
>@@ -4423,19 +4423,6 @@ webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/block-aspect-ratio-024.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/block-aspect-ratio-028.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/block-aspect-ratio-030.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-001.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-002.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-004.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-005.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-009.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-011.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-012.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-014.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-015.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-016.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-017.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-018.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-019.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-021.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-022.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-025.html [ ImageOnlyFailure ]

Actions: View | Formatted Diff | Diff

Attachments on bug 219679: 415748 | 415761 | 417080 | 417261 | 417841 | 417966 | 417971 | 418553 | 418734 | 420005 | 420456 | 420624 | 420628 | 420814 | 420937 | 420958 | 420979