Attachment 358444 Details for Bug 193172 – Patch

[patch] Patch

bug-193172-20190105162606.patch (text/plain), 8.53 KB, created by zalan on 2019-01-05 16:26:13 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-01-05 16:26:13 PST

Size: 8.53 KB

patch

obsolete

>Subversion Revision: 239659
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 55b71dc9deb21074a32e10417dfd9dbcea258561..ef6e164c53d75fe31e81ba889bac47559c57c3d7 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-01-05  Zalan Bujtas  <zalan@apple.com>
>+
>+        Incorrect clipping across compositing boundary.
>+        https://bugs.webkit.org/show_bug.cgi?id=193172
>+        <rdar://problem/44693008>
>+
>+        Reviewed by Simon Fraser.
>+
>+        Use temporary cliprect type when crossing compositing boundary to prevent cliprect caching.
>+
>+        The issue here is that RenderLayer::backgroundClipRect() could cross compositing boundary by calling parentClipRects() which triggers
>+        clip rect update using the wrong painting root. This happens when the layer hierarchy and the compositing context do not match.
>+        For clip rect computation, we need to climb up on the layer hierarchy (calling parent layer's cliprect functions)
>+        but we also need to make sure that the computed cliprects on any given layer are cached only when the painting root is correct.
>+        It ensures that when we paint a layer (with the painting root as entry point), the cached cliprects are always based on its onw painting root. 
>+
>+        Test: compositing/clipping/cached-cliprect-with-compositing-boundary.html
>+
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::calculateClipRects const):
>+
> 2019-01-05  Youenn Fablet  <youenn@apple.com>
> 
>         Service Worker fetch should obey its referrer policy
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 0e2e0f16b4490034f3b9af7b4c3dbfd51034bc01..8ad503e465da8d2273a3a3699e407ec81ce38d4c 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -5478,16 +5478,18 @@ static inline ClipRect backgroundClipRectForPosition(const ClipRects& parentRect
> ClipRect RenderLayer::backgroundClipRect(const ClipRectsContext& clipRectsContext) const
> {
>     ASSERT(parent());
>-    auto computeParentRects = [this, &clipRectsContext] () {
>-        // If we cross into a different pagination context, then we can't rely on the cache.
>-        // Just switch over to using TemporaryClipRects.
>-        if (clipRectsContext.clipRectsType != TemporaryClipRects
>-            && parent()->enclosingPaginationLayer(IncludeCompositedPaginatedLayers) != enclosingPaginationLayer(IncludeCompositedPaginatedLayers)) {
>-            ClipRectsContext tempContext(clipRectsContext);
>-            tempContext.clipRectsType = TemporaryClipRects;
>-            return parentClipRects(tempContext);
>-        }
>-        return parentClipRects(clipRectsContext);
>+    auto computeParentRects = [&] {
>+        if (clipRectsContext.clipRectsType == TemporaryClipRects)
>+            return parentClipRects(clipRectsContext);
>+        // If we cross into a different composition/pagination context, then we can't rely on the cache since the root layer differs.
>+        bool crossesPaginationBoundary = parent()->enclosingPaginationLayer(IncludeCompositedPaginatedLayers) != enclosingPaginationLayer(IncludeCompositedPaginatedLayers);
>+        bool crossesCompositingBoundary = parent()->enclosingCompositingLayerForRepaint() != enclosingCompositingLayerForRepaint();
>+        if (!crossesPaginationBoundary && !crossesCompositingBoundary)
>+            return parentClipRects(clipRectsContext);
>+
>+        ClipRectsContext tempContext(clipRectsContext);
>+        tempContext.clipRectsType = TemporaryClipRects;
>+        return parentClipRects(tempContext);
>     };
>     
>     auto parentRects = computeParentRects();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 1ce9b3abfe75aca76d0c1be9a9caec47c2092082..d6da6ca0dd1b8b762c512fb48eb3432eb6e42f53 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-01-05  Zalan Bujtas  <zalan@apple.com>
>+
>+        Incorrect clipping across compositing boundary.
>+        https://bugs.webkit.org/show_bug.cgi?id=193172
>+        <rdar://problem/44693008>
>+
>+        Reviewed by Simon Fraser.
>+
>+        * compositing/clipping/cached-cliprect-with-compositing-boundary-expected.html: Added.
>+        * compositing/clipping/cached-cliprect-with-compositing-boundary.html: Added.
>+
> 2019-01-05  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r239607.
>diff --git a/LayoutTests/compositing/clipping/cached-cliprect-with-compositing-boundary-expected.html b/LayoutTests/compositing/clipping/cached-cliprect-with-compositing-boundary-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5bdb0d21a16f6f6c0f04b51a96c01019d1f58bec
>--- /dev/null
>+++ b/LayoutTests/compositing/clipping/cached-cliprect-with-compositing-boundary-expected.html
>@@ -0,0 +1,56 @@
>+<iframe style="width: 400px; height: 400px" src="data:text/html,
>+<style>
>+.overlap {
>+	position: absolute;
>+	top: 10px;
>+	left: 10px;
>+    width: 100px;
>+    height: 100px;
>+	background-color: silver;
>+}
>+
>+.outer {
>+	border: 5px solid green;
>+	position: relative;
>+	top: 50px;
>+    left: 50px;
>+	height: 250px;
>+	width: 250px;
>+}
>+
>+.middle {
>+    overflow-y: scroll;
>+    border: 5px solid orange;
>+}
>+
>+.overflow-scroll {
>+    height: 100%;
>+    overflow-y: scroll;
>+}
>+
>+.item {
>+    height: 200px;
>+    border: 5px solid blue;
>+    background-color: yellow;
>+}
>+
>+.layery {
>+	position: relative;
>+    padding: 10px;
>+}
>+</style>
>+
>+<div class=overlap></div>
>+<div class=outer>
>+  <div class=middle>
>+	<div class=overflow-scroll>
>+		<div class=item>Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		<div class=layery>Thing with RenderLayer</div>
>+    </div>
>+</div>
>+"></iframe>
>diff --git a/LayoutTests/compositing/clipping/cached-cliprect-with-compositing-boundary.html b/LayoutTests/compositing/clipping/cached-cliprect-with-compositing-boundary.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..41af6f0932351425b6e560f3d547ad6bf7874a42
>--- /dev/null
>+++ b/LayoutTests/compositing/clipping/cached-cliprect-with-compositing-boundary.html
>@@ -0,0 +1,56 @@
>+<iframe style="width: 400px; height: 400px" src="data:text/html,
>+<style>
>+.overlap {
>+	position: fixed;
>+	top: 10px;
>+	left: 10px;
>+    width: 100px;
>+    height: 100px;
>+	background-color: silver;
>+}
>+
>+.outer {
>+	border: 5px solid green;
>+	position: relative;
>+	top: 50px;
>+    left: 50px;
>+	height: 250px;
>+	width: 250px;
>+}
>+
>+.middle {
>+    overflow-y: scroll;
>+    border: 5px solid orange;
>+}
>+
>+.overflow-scroll {
>+    height: 100%;
>+    overflow-y: scroll;
>+}
>+
>+.item {
>+    height: 200px;
>+    border: 5px solid blue;
>+    background-color: yellow;
>+}
>+
>+.layery {
>+	position: relative;
>+    padding: 10px;
>+}
>+</style>
>+
>+<div class=overlap></div>
>+<div class=outer>
>+  <div class=middle>
>+	<div class=overflow-scroll>
>+		<div class=item>Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible. Pass if this text is visible.
>+		<div class=layery>Thing with RenderLayer</div>
>+    </div>
>+</div>
>+"></iframe>

Actions: View | Formatted Diff | Diff

Attachments on bug 193172: 358443 | 358444 | 358448 | 358449