Changeset 206188 in webkit

Timestamp:

Sep 20, 2016 5:25:07 PM (8 years ago)

Author:

Alan Bujtas

Message:

REGRESSION (r204552): Athlete search on Strava gives bad rendering.
​https://bugs.webkit.org/show_bug.cgi?id=162250

Reviewed by Simon Fraser.

Source/WebCore:

Test: fast/layers/blank-content-when-child-layer-is-at-negative-big-number.html

• platform/graphics/LayoutRect.cpp:

(WebCore::LayoutRect::checkedUnite):

• platform/graphics/LayoutRect.h:

(WebCore::LayoutRect::isMaxXMaxYRepresentable):
(WebCore::LayoutRect::maxXMaxYCorner): Deleted.

• rendering/RenderLayer.cpp:

(WebCore::RenderLayer::calculateClipRects):

LayoutTests:

While computing the size of a particular layer, we unite the content size and the descendant layers' size.
If a descendant layer is positioned far off, the computed rectangle might not fully cover the original rectangles.
This happens when the 2 rectangles' distance is close to the maximum LayoutUnit value.
It's fairly common technic to put some content offscreen (top: -99999999px;). In order to keep the main content
visible, we need to ensure that the parent layer never gets cut off, while uniting it with the descendant layers.

• fast/layers/blank-content-when-child-layer-is-at-negative-big-number-expected.html: Added.
• fast/layers/blank-content-when-child-layer-is-at-negative-big-number.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/layers/blank-content-when-child-layer-is-at-negative-big-number-expected.html (added)
• LayoutTests/fast/layers/blank-content-when-child-layer-is-at-negative-big-number.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/LayoutRect.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/LayoutRect.h (modified) (2 diffs)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-09-20  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION (r204552): Athlete search on Strava gives bad rendering.
+        https://bugs.webkit.org/show_bug.cgi?id=162250
+
+        Reviewed by Simon Fraser.
+
+        While computing the size of a particular layer, we unite the content size and the descendant layers' size.
+        If a descendant layer is positioned far off, the computed rectangle might not fully cover the original rectangles.
+        This happens when the 2 rectangles' distance is close to the maximum LayoutUnit value.
+        It's fairly common technic to put some content offscreen (top: -99999999px;). In order to keep the main content
+        visible, we need to ensure that the parent layer never gets cut off, while uniting it with the descendant layers.
+
+        * fast/layers/blank-content-when-child-layer-is-at-negative-big-number-expected.html: Added.
+        * fast/layers/blank-content-when-child-layer-is-at-negative-big-number.html: Added.
+
 2016-09-20  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-09-20  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION (r204552): Athlete search on Strava gives bad rendering.
+        https://bugs.webkit.org/show_bug.cgi?id=162250
+
+        Reviewed by Simon Fraser.
+
+        Test: fast/layers/blank-content-when-child-layer-is-at-negative-big-number.html
+
+        * platform/graphics/LayoutRect.cpp:
+        (WebCore::LayoutRect::checkedUnite):
+        * platform/graphics/LayoutRect.h:
+        (WebCore::LayoutRect::isMaxXMaxYRepresentable):
+        (WebCore::LayoutRect::maxXMaxYCorner): Deleted.
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::calculateClipRects):
+
 2016-09-20  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/platform/graphics/LayoutRect.cpp

@@ -89 +89 @@
 }
 
+bool LayoutRect::checkedUnite(const LayoutRect& other)
+{
+    if (other.isEmpty())
+        return true;
+    if (isEmpty()) {
+        *this = other;
+        return true;
+    }
+    if (!isMaxXMaxYRepresentable() || !other.isMaxXMaxYRepresentable())
+        return false;
+    FloatPoint topLeft = FloatPoint(std::min<float>(x(), other.x()), std::min<float>(y(), other.y()));
+    FloatPoint bottomRight = FloatPoint(std::max<float>(maxX(), other.maxX()), std::max<float>(maxY(), other.maxY()));
+    FloatSize size = bottomRight - topLeft;
+    
+    if (size.width() >= LayoutUnit::nearlyMax() || size.height() >= LayoutUnit::nearlyMax())
+        return false;
+    m_location = LayoutPoint(topLeft);
+    m_size = LayoutSize(size);
+    return true;
+}
+
 void LayoutRect::uniteIfNonZero(const LayoutRect& other)
 {

trunk/Source/WebCore/platform/graphics/LayoutRect.h

@@ -127 +127 @@
     LayoutPoint minXMaxYCorner() const { return LayoutPoint(m_location.x(), m_location.y() + m_size.height()); } // typically bottomLeft
     LayoutPoint maxXMaxYCorner() const { return LayoutPoint(m_location.x() + m_size.width(), m_location.y() + m_size.height()); } // typically bottomRight
+    bool isMaxXMaxYRepresentable() const
+    {
+        FloatRect rect = *this;
+        float maxX = rect.maxX();
+        float maxY = rect.maxY();
+        return maxX > LayoutUnit::nearlyMin() && maxX < LayoutUnit::nearlyMax() && maxY > LayoutUnit::nearlyMin() && maxY < LayoutUnit::nearlyMax();
+    }
     
     bool intersects(const LayoutRect&) const;
@@ -140 +147 @@
     WEBCORE_EXPORT void unite(const LayoutRect&);
     void uniteIfNonZero(const LayoutRect&);
+    bool checkedUnite(const LayoutRect&);
 
     void inflateX(LayoutUnit dx)

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -6004 +6004 @@
 #endif
 
+    auto computeLayersUnion = [this, &unionBounds, flags, descendantFlags] (const RenderLayer& childLayer) {
+        if (!(flags & IncludeCompositedDescendants) && childLayer.isComposited())
+            return;
+        LayoutRect childBounds = childLayer.calculateLayerBounds(this, childLayer.offsetFromAncestor(this), descendantFlags);
+        // Ignore child layer (and behave as if we had overflow: hidden) when it is positioned off the parent layer so much
+        // that we hit the max LayoutUnit value.
+        unionBounds.checkedUnite(childBounds);
+    };
+
     if (Vector<RenderLayer*>* negZOrderList = this->negZOrderList()) {
-        for (auto* curLayer : *negZOrderList) {
-            if (flags & IncludeCompositedDescendants || !curLayer->isComposited()) {
-                LayoutRect childUnionBounds = curLayer->calculateLayerBounds(this, curLayer->offsetFromAncestor(this), descendantFlags);
-                unionBounds.unite(childUnionBounds);
-            }
-        }
+        for (auto* childLayer : *negZOrderList)
+            computeLayersUnion(*childLayer);
     }
 
     if (Vector<RenderLayer*>* posZOrderList = this->posZOrderList()) {
-        for (auto* curLayer : *posZOrderList) {
+        for (auto* childLayer : *posZOrderList) {
             // The RenderNamedFlowThread is ignored when we calculate the bounds of the RenderView.
-            if ((flags & IncludeCompositedDescendants || !curLayer->isComposited()) && !curLayer->isFlowThreadCollectingGraphicsLayersUnderRegions()) {
-                LayoutRect childUnionBounds = curLayer->calculateLayerBounds(this, curLayer->offsetFromAncestor(this), descendantFlags);
-                unionBounds.unite(childUnionBounds);
-            }
+            if (childLayer->isFlowThreadCollectingGraphicsLayersUnderRegions())
+                continue;
+            computeLayersUnion(*childLayer);
         }
     }
 
     if (Vector<RenderLayer*>* normalFlowList = this->normalFlowList()) {
-        for (auto* curLayer : *normalFlowList) {
+        for (auto* childLayer : *normalFlowList) {
             // RenderView will always return the size of the document, before reaching this point,
             // so there's no way we could hit a RenderNamedFlowThread here.
-            ASSERT(!curLayer->isFlowThreadCollectingGraphicsLayersUnderRegions());
-            if (flags & IncludeCompositedDescendants || !curLayer->isComposited()) {
-                LayoutRect curAbsBounds = curLayer->calculateLayerBounds(this, curLayer->offsetFromAncestor(this), descendantFlags);
-                unionBounds.unite(curAbsBounds);
-            }
+            ASSERT(!childLayer->isFlowThreadCollectingGraphicsLayersUnderRegions());
+            computeLayersUnion(*childLayer);
         }
     }