[cairo][SVG] If clipPath has multiple elements, clip-path doesn't work with transform
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Jun 2019 04:10:33 +0000 (04:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Jun 2019 04:10:33 +0000 (04:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198746
Source/WebCore:

Patch by Carlos Garcia Campos <cgarcia@igalia.com> on 2019-06-12
Reviewed by Don Olmstead.

We need to save the current transformation matrix at the moment the image mask is set and set it again on
restore right before applying the mask. This patch also creates a pattern for the image mask surface and set its
transformation matrix according to the mask position, so that we don't need to save the mask rectangle too.

Tests: svg/clip-path/clip-hidpi-expected.svg
       svg/clip-path/clip-hidpi.svg
       svg/clip-path/clip-opacity-translate-expected.svg
       svg/clip-path/clip-opacity-translate.svg

* platform/graphics/cairo/PlatformContextCairo.cpp:
(WebCore::PlatformContextCairo::restore):
(WebCore::PlatformContextCairo::pushImageMask):

LayoutTests:

<rdar://problem/51620347>

Patch by Carlos Garcia Campos <cgarcia@igalia.com> on 2019-06-12
Reviewed by Don Olmstead.

* svg/clip-path/clip-hidpi-expected.svg: Added.
* svg/clip-path/clip-hidpi.svg: Added.
* svg/clip-path/clip-opacity-translate-expected.svg: Added.
* svg/clip-path/clip-opacity-translate.svg: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@246391 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/clip-path/clip-hidpi-expected.svg [new file with mode: 0644]
LayoutTests/svg/clip-path/clip-hidpi.svg [new file with mode: 0644]
LayoutTests/svg/clip-path/clip-opacity-translate-expected.svg [new file with mode: 0644]
LayoutTests/svg/clip-path/clip-opacity-translate.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp

index 21ad40a..00c8196 100644 (file)
@@ -1,3 +1,16 @@
+2019-06-12  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [cairo][SVG] If clipPath has multiple elements, clip-path doesn't work with transform
+        https://bugs.webkit.org/show_bug.cgi?id=198746
+        <rdar://problem/51620347>
+
+        Reviewed by Don Olmstead.
+
+        * svg/clip-path/clip-hidpi-expected.svg: Added.
+        * svg/clip-path/clip-hidpi.svg: Added.
+        * svg/clip-path/clip-opacity-translate-expected.svg: Added.
+        * svg/clip-path/clip-opacity-translate.svg: Added.
+
 2019-06-12  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [WHLSL] Educate the property resolver about IndexExpressions
diff --git a/LayoutTests/svg/clip-path/clip-hidpi-expected.svg b/LayoutTests/svg/clip-path/clip-hidpi-expected.svg
new file mode 100644 (file)
index 0000000..d9eb369
--- /dev/null
@@ -0,0 +1,18 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+<!-- The FO should be clipped with only the green half visible. -->
+<defs>
+<clipPath id="clip">
+    <rect width="200" height="50"/>
+    <rect width="200" height="50"/>
+</clipPath>
+</defs>
+<foreignObject width="200" height="100" clip-path="url(#clip)" opacity=".5">
+    <html xmlns="http://www.w3.org/1999/xhtml">
+    <body>
+        <div style="background: green; height: 50px;"></div>
+        <div style="background: red; height: 50px;"></div>
+    </body>
+    </html>
+</foreignObject>
+</svg>
+
diff --git a/LayoutTests/svg/clip-path/clip-hidpi.svg b/LayoutTests/svg/clip-path/clip-hidpi.svg
new file mode 100644 (file)
index 0000000..4b52515
--- /dev/null
@@ -0,0 +1,22 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+<!-- The FO should be clipped with only the green half visible. -->
+<defs>
+<clipPath id="clip">
+    <rect width="200" height="50"/>
+    <rect width="200" height="50"/>
+</clipPath>
+</defs>
+<foreignObject width="200" height="100" clip-path="url(#clip)" opacity=".5">
+    <html xmlns="http://www.w3.org/1999/xhtml">
+    <body>
+        <div style="background: green; height: 50px;"></div>
+        <div style="background: red; height: 50px;"></div>
+    </body>
+    </html>
+</foreignObject>
+<script>
+  testRunner.setBackingScaleFactor(2, () -> { testRunner.notifyDone() });
+  testRunner.waitUntilDone();
+</script>
+</svg>
+
diff --git a/LayoutTests/svg/clip-path/clip-opacity-translate-expected.svg b/LayoutTests/svg/clip-path/clip-opacity-translate-expected.svg
new file mode 100644 (file)
index 0000000..9828b59
--- /dev/null
@@ -0,0 +1,18 @@
+<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 opacity=".5">
+    <circle cx="50" cy="100" r="50" fill="green"/>
+    <circle cx="100" cy="100" r="50" fill="red" />
+  </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 opacity=".5">
+    <circle cx="50" cy="100" r="50" fill="green"/>
+    <circle cx="100" cy="100" r="50" fill="red" />
+  </g>
+</g>
+</svg>
diff --git a/LayoutTests/svg/clip-path/clip-opacity-translate.svg b/LayoutTests/svg/clip-path/clip-opacity-translate.svg
new file mode 100644 (file)
index 0000000..eab0307
--- /dev/null
@@ -0,0 +1,27 @@
+<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)" opacity=".5">
+    <circle cx="50" cy="100" r="50" fill="green"/>
+    <circle cx="100" cy="100" r="50" fill="red" />
+  </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)" opacity=".5">
+    <circle cx="50" cy="100" r="50" fill="green"/>
+    <circle cx="100" cy="100" r="50" fill="red" />
+  </g>
+</g>
+</svg>
index 7672799..50bbaa7 100644 (file)
@@ -1,3 +1,23 @@
+2019-06-12  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [cairo][SVG] If clipPath has multiple elements, clip-path doesn't work with transform
+        https://bugs.webkit.org/show_bug.cgi?id=198746
+
+        Reviewed by Don Olmstead.
+
+        We need to save the current transformation matrix at the moment the image mask is set and set it again on
+        restore right before applying the mask. This patch also creates a pattern for the image mask surface and set its
+        transformation matrix according to the mask position, so that we don't need to save the mask rectangle too.
+
+        Tests: svg/clip-path/clip-hidpi-expected.svg
+               svg/clip-path/clip-hidpi.svg
+               svg/clip-path/clip-opacity-translate-expected.svg
+               svg/clip-path/clip-opacity-translate.svg
+
+        * platform/graphics/cairo/PlatformContextCairo.cpp:
+        (WebCore::PlatformContextCairo::restore):
+        (WebCore::PlatformContextCairo::pushImageMask):
+
 2019-06-12  Simon Fraser  <simon.fraser@apple.com>
 
         paddingBoxRect() is wrong with RTL scrollbars on the left
index 7f242fd..62e5174 100644 (file)
 
 #if USE(CAIRO)
 
-#include "CairoUtilities.h"
-#include "Gradient.h"
-#include "GraphicsContext.h"
-#include "Pattern.h"
 #include <cairo.h>
 
 namespace WebCore {
 
-// In Cairo image masking is immediate, so to emulate image clipping we must save masking
-// details as part of the context state and apply them during platform restore.
-class ImageMaskInformation {
-public:
-    void update(cairo_surface_t* maskSurface, const FloatRect& maskRect)
-    {
-        m_maskSurface = maskSurface;
-        m_maskRect = maskRect;
-    }
-
-    bool isValid() const { return m_maskSurface; }
-    cairo_surface_t* maskSurface() const { return m_maskSurface.get(); }
-    const FloatRect& maskRect() const { return m_maskRect; }
-
-private:
-    RefPtr<cairo_surface_t> m_maskSurface;
-    FloatRect m_maskRect;
-};
-
-
 // Encapsulates the additional painting state information we store for each
 // pushed graphics state.
 class PlatformContextCairo::State {
 public:
     State() = default;
 
-    ImageMaskInformation m_imageMaskInformation;
+    struct {
+        RefPtr<cairo_pattern_t> pattern;
+        cairo_matrix_t matrix;
+    } m_mask;
 };
 
 PlatformContextCairo::PlatformContextCairo(cairo_t* cr)
@@ -76,11 +55,14 @@ PlatformContextCairo::PlatformContextCairo(cairo_t* cr)
 
 void PlatformContextCairo::restore()
 {
-    const ImageMaskInformation& maskInformation = m_state->m_imageMaskInformation;
-    if (maskInformation.isValid()) {
-        const FloatRect& maskRect = maskInformation.maskRect();
+    if (m_state->m_mask.pattern) {
         cairo_pop_group_to_source(m_cr.get());
-        cairo_mask_surface(m_cr.get(), maskInformation.maskSurface(), maskRect.x(), maskRect.y());
+
+        cairo_matrix_t matrix;
+        cairo_get_matrix(m_cr.get(), &matrix);
+        cairo_set_matrix(m_cr.get(), &m_state->m_mask.matrix);
+        cairo_mask(m_cr.get(), m_state->m_mask.pattern.get());
+        cairo_set_matrix(m_cr.get(), &matrix);
     }
 
     m_stateStack.removeLast();
@@ -105,11 +87,13 @@ void PlatformContextCairo::pushImageMask(cairo_surface_t* surface, const FloatRe
     // We must call savePlatformState at least once before we can use image masking,
     // since we actually apply the mask in restorePlatformState.
     ASSERT(!m_stateStack.isEmpty());
-    m_state->m_imageMaskInformation.update(surface, rect);
+    m_state->m_mask.pattern = adoptRef(cairo_pattern_create_for_surface(surface));
+    cairo_get_matrix(m_cr.get(), &m_state->m_mask.matrix);
+
+    cairo_matrix_t matrix;
+    cairo_matrix_init_translate(&matrix, -rect.x(), -rect.y());
+    cairo_pattern_set_matrix(m_state->m_mask.pattern.get(), &matrix);
 
-    // 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.
     cairo_push_group(m_cr.get());
 }