Title: [206188] trunk
Revision
206188
Author
za...@apple.com
Date
2016-09-20 17:25:07 -0700 (Tue, 20 Sep 2016)

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

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (206187 => 206188)


--- trunk/LayoutTests/ChangeLog	2016-09-21 00:21:02 UTC (rev 206187)
+++ trunk/LayoutTests/ChangeLog	2016-09-21 00:25:07 UTC (rev 206188)
@@ -1,3 +1,19 @@
+2016-09-20  Zalan Bujtas  <za...@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.no...@apple.com>
 
         [media-source] Fix imported/w3c/web-platform-tests/media-source/mediasource-appendwindow.html

Added: trunk/LayoutTests/fast/layers/blank-content-when-child-layer-is-at-negative-big-number-expected.html (0 => 206188)


--- trunk/LayoutTests/fast/layers/blank-content-when-child-layer-is-at-negative-big-number-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/layers/blank-content-when-child-layer-is-at-negative-big-number-expected.html	2016-09-21 00:25:07 UTC (rev 206188)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that we paint the parent layer properly even when the child layer is positioned far off.</title>
+<style>
+div {
+    height: 100px;
+    width: 100px;
+    background-color: green;
+}
+</style>
+</head>
+<body>
+<div></div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/layers/blank-content-when-child-layer-is-at-negative-big-number.html (0 => 206188)


--- trunk/LayoutTests/fast/layers/blank-content-when-child-layer-is-at-negative-big-number.html	                        (rev 0)
+++ trunk/LayoutTests/fast/layers/blank-content-when-child-layer-is-at-negative-big-number.html	2016-09-21 00:25:07 UTC (rev 206188)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that we paint the parent layer properly even when the child layer is positioned far off.</title>
+<style>
+.container {
+    transform: translateZ(0);
+    height: 100px;
+    width: 100px;
+    background-color: green;
+}
+
+.offscreen {
+    position: absolute;
+    top: -99999999px;
+}
+</style>
+</head>
+<body>
+<div class="container"><div class="offscreen">foobar</div></div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (206187 => 206188)


--- trunk/Source/WebCore/ChangeLog	2016-09-21 00:21:02 UTC (rev 206187)
+++ trunk/Source/WebCore/ChangeLog	2016-09-21 00:25:07 UTC (rev 206188)
@@ -1,3 +1,20 @@
+2016-09-20  Zalan Bujtas  <za...@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.no...@apple.com>
 
         [media-source] Fix imported/w3c/web-platform-tests/media-source/mediasource-appendwindow.html

Modified: trunk/Source/WebCore/platform/graphics/LayoutRect.cpp (206187 => 206188)


--- trunk/Source/WebCore/platform/graphics/LayoutRect.cpp	2016-09-21 00:21:02 UTC (rev 206187)
+++ trunk/Source/WebCore/platform/graphics/LayoutRect.cpp	2016-09-21 00:25:07 UTC (rev 206188)
@@ -88,6 +88,27 @@
     m_size = newMaxPoint - newLocation;
 }
 
+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)
 {
     // Handle empty special cases first.

Modified: trunk/Source/WebCore/platform/graphics/LayoutRect.h (206187 => 206188)


--- trunk/Source/WebCore/platform/graphics/LayoutRect.h	2016-09-21 00:21:02 UTC (rev 206187)
+++ trunk/Source/WebCore/platform/graphics/LayoutRect.h	2016-09-21 00:25:07 UTC (rev 206188)
@@ -126,6 +126,13 @@
     LayoutPoint maxXMinYCorner() const { return LayoutPoint(m_location.x() + m_size.width(), m_location.y()); } // typically topRight
     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;
     WEBCORE_EXPORT bool contains(const LayoutRect&) const;
@@ -139,6 +146,7 @@
     void intersect(const LayoutRect&);
     WEBCORE_EXPORT void unite(const LayoutRect&);
     void uniteIfNonZero(const LayoutRect&);
+    bool checkedUnite(const LayoutRect&);
 
     void inflateX(LayoutUnit dx)
     {

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (206187 => 206188)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2016-09-21 00:21:02 UTC (rev 206187)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2016-09-21 00:25:07 UTC (rev 206188)
@@ -6003,34 +6003,35 @@
     LayerListMutationDetector mutationChecker(const_cast<RenderLayer*>(this));
 #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);
         }
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to