Merge r246309 - [cairo][SVG] Putting multiple path elements in clippa…

…th causes rendering artifacts https://bugs.webkit.org/show_bug.cgi?id=198701 Source/WebCore: PlatformContextCairo::pushImageMask blits wrong position of the surface to the background of masking objects. And, I don't know the reason why this blitting is needed. Removed the blitting. Reviewed by Carlos Garcia Campos. Tests: svg/clip-path/clip-opacity.html svg/clip-path/svg-in-html.html * platform/graphics/cairo/PlatformContextCairo.cpp: (WebCore::PlatformContextCairo::pushImageMask): Don't blit the surface to the background. LayoutTests: Reviewed by Carlos Garcia Campos. * platform/gtk/TestExpectations: * platform/wpe/TestExpectations: Unskipped svg/gradients/spreadMethodDiagonal3.svg and svg/gradients/spreadMethodDiagonal4.svg. * svg/clip-path/clip-opacity-expected.html: Added. * svg/clip-path/clip-opacity.html: Added. * svg/clip-path/svg-in-html-expected.html: Added. * svg/clip-path/svg-in-html.html: Added.
WebKit · Jul 1, 2019 · bd03030 · bd03030
1 parent ef6bab6
commit bd03030
Show file tree

Hide file tree

Showing 9 changed files with 191 additions and 21 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2019-06-11  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        [cairo][SVG] Putting multiple path elements in clippath causes rendering artifacts
+        https://bugs.webkit.org/show_bug.cgi?id=198701
+
+        Reviewed by Carlos Garcia Campos.
+
+        * platform/gtk/TestExpectations:
+        * platform/wpe/TestExpectations:
+        Unskipped svg/gradients/spreadMethodDiagonal3.svg and svg/gradients/spreadMethodDiagonal4.svg.
+
+        * svg/clip-path/clip-opacity-expected.html: Added.
+        * svg/clip-path/clip-opacity.html: Added.
+        * svg/clip-path/svg-in-html-expected.html: Added.
+        * svg/clip-path/svg-in-html.html: Added.
+
 2019-06-18  Zan Dobersek  <zdobersek@igalia.com>
 
         [WebGL] Extensions3DOpenGLES::bindVertexArrayOES() should allow zero array object

diff --git a/LayoutTests/platform/gtk/TestExpectations b/LayoutTests/platform/gtk/TestExpectations
@@ -3572,8 +3572,6 @@ webkit.org/b/189737 editing/pasteboard/4930986-2-paste-as-quotation.html [ Failu
 webkit.org/b/189737 editing/pasteboard/4930986-3-paste-as-quotation.html [ Failure ]
 
 webkit.org/b/189739 svg/gradients/spreadMethodClose2.svg [ ImageOnlyFailure ]
-webkit.org/b/189739 svg/gradients/spreadMethodDiagonal3.svg [ ImageOnlyFailure ]
-webkit.org/b/189739 svg/gradients/spreadMethodDiagonal4.svg [ ImageOnlyFailure ]
 
 webkit.org/b/189994 fast/files/xhr-response-blob.html [ Failure ]
 

diff --git a/LayoutTests/platform/wpe/TestExpectations b/LayoutTests/platform/wpe/TestExpectations
@@ -1319,8 +1319,6 @@ webkit.org/b/160137 svg/custom/non-scaling-stroke.svg [ Failure Pass ]
 webkit.org/b/160137 svg/custom/non-scaling-stroke-update.svg [ ImageOnlyFailure Pass ]
 webkit.org/b/112228 svg/custom/resources-css-scaled.html [ ImageOnlyFailure ]
 webkit.org/b/189739 svg/gradients/spreadMethodClose2.svg [ ImageOnlyFailure ]
-webkit.org/b/189739 svg/gradients/spreadMethodDiagonal3.svg [ ImageOnlyFailure ]
-webkit.org/b/189739 svg/gradients/spreadMethodDiagonal4.svg [ ImageOnlyFailure ]
 webkit.org/b/115440 svg/stroke/animated-non-scaling-stroke.html [ ImageOnlyFailure Pass ]
 webkit.org/b/88230 svg/stroke/non-scaling-stroke-pattern.svg [ ImageOnlyFailure Pass ]
 webkit.org/b/137096 svg/text/alt-glyph-for-surrogate-pair.svg [ ImageOnlyFailure ]

diff --git a/LayoutTests/svg/clip-path/clip-opacity-expected.html b/LayoutTests/svg/clip-path/clip-opacity-expected.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<body>
+<svg xmlns="http://www.w3.org/2000/svg" width="350">
+<g>
+  <circle cx="50" cy="50" r="50" fill="green"/>
+  <circle cx="100" cy="50" r="50" fill="red" />
+  <g>
+    <circle cx="50" cy="100" r="50" fill="green" opacity=".5" />
+    <circle cx="100" cy="100" r="50" fill="red"  opacity=".5" />
+  </g>
+</g>
+<g transform="translate(200,0)">
+  <circle cx="50" cy="50" r="50" fill="green"/>
+  <circle cx="100" cy="50" r="50" fill="red" />
+  <g>
+    <circle cx="50" cy="100" r="50" fill="green" opacity=".5" />
+    <circle cx="100" cy="100" r="50" fill="red"  opacity=".5" />
+  </g>
+</g>
+</svg>
+</body>
+</html>
diff --git a/LayoutTests/svg/clip-path/clip-opacity.html b/LayoutTests/svg/clip-path/clip-opacity.html
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<body>
+<svg xmlns="http://www.w3.org/2000/svg" width="350">
+<defs>
+<clipPath id="clip1">
+    <rect width="200" height="200"/>
+</clipPath>
+<clipPath id="clip2">
+    <rect width="200" height="200"/>
+    <rect width="200" height="200"/>
+</clipPath>
+</defs>
+<g>
+  <circle cx="50" cy="50" r="50" fill="green"/>
+  <circle cx="100" cy="50" r="50" fill="red" />
+  <g clip-path="url(#clip1)">
+    <circle cx="50" cy="100" r="50" fill="green" opacity=".5" />
+    <circle cx="100" cy="100" r="50" fill="red"  opacity=".5" />
+  </g>
+</g>
+<g transform="translate(200,0)">
+  <circle cx="50" cy="50" r="50" fill="green"/>
+  <circle cx="100" cy="50" r="50" fill="red" />
+  <g clip-path="url(#clip2)">
+    <circle cx="50" cy="100" r="50" fill="green" opacity=".5" />
+    <circle cx="100" cy="100" r="50" fill="red"  opacity=".5" />
+  </g>
+</g>
+</svg>
+</body>
+</html>
diff --git a/LayoutTests/svg/clip-path/svg-in-html-expected.html b/LayoutTests/svg/clip-path/svg-in-html-expected.html
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+    border: 2em blue solid;
+}
+svg {
+    background: pink;
+}
+</style>
+</head>
+<body>
+<svg width="200px" height="200px" viewBox="0 0 8 8" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <g>
+        <circle cx="1" cy="1" r="1" fill="green"></circle>
+        <circle cx="7" cy="1" r="1" fill="green"></circle>
+    </g>
+</svg>
+<svg width="200px" height="200px" viewBox="0 0 8 8" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <g>
+        <circle cx="1" cy="7" r="1" fill="green"></circle>
+        <circle cx="7" cy="7" r="1" fill="green"></circle>
+    </g>
+</svg>
+<br>
+<svg width="200px" height="200px" viewBox="0 0 8 8" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <g>
+        <circle cx="1" cy="1" r="1" fill="green"></circle>
+        <circle cx="7" cy="1" r="1" fill="green"></circle>
+    </g>
+</svg>
+<svg width="200px" height="200px" viewBox="0 0 8 8" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <g>
+        <circle cx="1" cy="7" r="1" fill="green"></circle>
+        <circle cx="7" cy="7" r="1" fill="green"></circle>
+    </g>
+</svg>
+</body>
+</html>
diff --git a/LayoutTests/svg/clip-path/svg-in-html.html b/LayoutTests/svg/clip-path/svg-in-html.html
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+    border: 2em blue solid;
+}
+svg {
+    background: pink;
+}
+</style>
+</head>
+<body>
+<svg width="200px" height="200px" viewBox="0 0 8 8" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <clippath id="clip1">
+        <rect width="8" height="4"></rect>
+    </clippath>
+    <g clip-path="url(#clip1)">
+        <circle cx="1" cy="1" r="1" fill="green"></circle>
+        <circle cx="7" cy="1" r="1" fill="green"></circle>
+        <circle cx="1" cy="7" r="1" fill="green"></circle>
+        <circle cx="7" cy="7" r="1" fill="green"></circle>
+    </g>
+</svg>
+<svg width="200px" height="200px" viewBox="0 0 8 8" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <clippath id="clip2">
+        <rect y="4" width="8" height="4"></rect>
+    </clippath>
+    <g clip-path="url(#clip2)">
+        <circle cx="1" cy="1" r="1" fill="green"></circle>
+        <circle cx="7" cy="1" r="1" fill="green"></circle>
+        <circle cx="1" cy="7" r="1" fill="green"></circle>
+        <circle cx="7" cy="7" r="1" fill="green"></circle>
+    </g>
+</svg>
+<br>
+<svg width="200px" height="200px" viewBox="0 0 8 8" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <clippath id="clip3">
+        <rect width="8" height="2"></rect>
+        <rect y="4" width="8" height="2"></rect>
+    </clippath>
+    <g clip-path="url(#clip3)">
+        <circle cx="1" cy="1" r="1" fill="green"></circle>
+        <circle cx="7" cy="1" r="1" fill="green"></circle>
+        <circle cx="1" cy="7" r="1" fill="green"></circle>
+        <circle cx="7" cy="7" r="1" fill="green"></circle>
+    </g>
+</svg>
+<svg width="200px" height="200px" viewBox="0 0 8 8" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <clippath id="clip4">
+        <rect y="2" width="8" height="2"></rect>
+        <rect y="6" width="8" height="2"></rect>
+    </clippath>
+    <g clip-path="url(#clip4)">
+        <circle cx="1" cy="1" r="1" fill="green"></circle>
+        <circle cx="7" cy="1" r="1" fill="green"></circle>
+        <circle cx="1" cy="7" r="1" fill="green"></circle>
+        <circle cx="7" cy="7" r="1" fill="green"></circle>
+    </g>
+</svg>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,21 @@
+2019-06-11  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        [cairo][SVG] Putting multiple path elements in clippath causes rendering artifacts
+        https://bugs.webkit.org/show_bug.cgi?id=198701
+
+        PlatformContextCairo::pushImageMask blits wrong position of the
+        surface to the background of masking objects. And, I don't know
+        the reason why this blitting is needed. Removed the blitting.
+
+        Reviewed by Carlos Garcia Campos.
+
+        Tests: svg/clip-path/clip-opacity.html
+               svg/clip-path/svg-in-html.html
+
+        * platform/graphics/cairo/PlatformContextCairo.cpp:
+        (WebCore::PlatformContextCairo::pushImageMask): Don't blit the
+        surface to the background.
+
 2019-06-18  Zan Dobersek  <zdobersek@igalia.com>
 
         [WebGL] Extensions3DOpenGLES::bindVertexArrayOES() should allow zero array object

diff --git a/Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp b/Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp
@@ -110,24 +110,7 @@ void PlatformContextCairo::pushImageMask(cairo_surface_t* surface, const FloatRe
     // Cairo doesn't support the notion of an image clip, so we push a group here
     // and then paint it to the surface with an image mask (which is an immediate
     // operation) during restorePlatformState.
-
-    // We want to allow the clipped elements to composite with the surface as it
-    // is now, but they are isolated in another group. To make this work, we're
-    // going to blit the current surface contents onto the new group once we push it.
-    cairo_surface_t* currentTarget = cairo_get_target(m_cr.get());
-    cairo_surface_flush(currentTarget);
-
-    // Pushing a new group ensures that only things painted after this point are clipped.
     cairo_push_group(m_cr.get());
-    cairo_set_operator(m_cr.get(), CAIRO_OPERATOR_SOURCE);
-
-    // To avoid the limit of Pixman backend, we need to reduce the size of pattern matrix
-    // See https://bugs.webkit.org/show_bug.cgi?id=154283
-    cairo_set_source_surface(m_cr.get(), currentTarget, rect.x(), rect.y());
-    cairo_translate(m_cr.get(), rect.x(), rect.y());
-    cairo_rectangle(m_cr.get(), 0, 0, rect.width(), rect.height());
-    cairo_fill(m_cr.get());
-    cairo_translate(m_cr.get(), -rect.x(), -rect.y());
 }
 
 } // namespace WebCore