Attachment 372015 Details for Bug 198746 – Patch

[patch] Patch

bug-198746-20190613110800.patch (text/plain), 9.29 KB, created by Fujii Hironori on 2019-06-12 19:08:01 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Fujii Hironori

Created: 2019-06-12 19:08:01 PDT

Size: 9.29 KB

patch

obsolete

>Subversion Revision: 246095
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0b2b44f9e73b3785358cb759c5dd07914ccb5341..3694a654b03a4dabc2341ed2b45676762af17774 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -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  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [cairo][SVG] Putting multiple path elements in clippath causes rendering artifacts
>diff --git a/Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp b/Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp
>index 7f242fdbecb39526c0e3c1c5a602046a786560b4..62e517431df3424b59738f8b2ac1a6f88e907139 100644
>--- a/Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp
>+++ b/Source/WebCore/platform/graphics/cairo/PlatformContextCairo.cpp
>@@ -30,41 +30,20 @@
> 
> #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());
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 00b5d7fb1e7b7aea17b6c5538e88526d4697b343..5006ec5568ad23695cde915b2c356699701386ce 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -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  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [GTK] Some reftest fail with only one or two pixel differences in diff image
>diff --git a/LayoutTests/svg/clip-path/clip-hidpi-expected.svg b/LayoutTests/svg/clip-path/clip-hidpi-expected.svg
>new file mode 100644
>index 0000000000000000000000000000000000000000..d9eb369e94e4a586b724dedcce6cf8d71b26d88c
>--- /dev/null
>+++ b/LayoutTests/svg/clip-path/clip-hidpi-expected.svg
>@@ -0,0 +1,18 @@
>+<svg xmlns="http://www.w3.org/2000/svg">
>+
>+<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
>index 0000000000000000000000000000000000000000..4b525153f82cb5f028203e58e803101ad37011a5
>--- /dev/null
>+++ b/LayoutTests/svg/clip-path/clip-hidpi.svg
>@@ -0,0 +1,22 @@
>+<svg xmlns="http://www.w3.org/2000/svg">
>+
>+<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
>index 0000000000000000000000000000000000000000..9828b5963d9cbea510ef343692785304ebc8f180
>--- /dev/null
>+++ b/LayoutTests/svg/clip-path/clip-opacity-translate-expected.svg
>@@ -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
>index 0000000000000000000000000000000000000000..eab0307fad438778f436e9a34c9d5d1c5d0ec22b
>--- /dev/null
>+++ b/LayoutTests/svg/clip-path/clip-opacity-translate.svg
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 198746: 371826 | 371938 | 371942 | 371944 | 371946 | 372014 | 372015