Attachment 371946 Details for Bug 198746 – Patch

[patch] Patch

wk-clip-to-image.diff (text/plain), 8.91 KB, created by Carlos Garcia Campos on 2019-06-12 06:04:50 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Carlos Garcia Campos

Created: 2019-06-12 06:04:50 PDT

Size: 8.91 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index dfb5839c37f..1b4e48faa11 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-06-12  Carlos Garcia Campos  <cgarcia@igalia.com>
>+
>+        [cairo][SVG] Putting multiple path elements in clippath causes rendering artifacts
>+        https://bugs.webkit.org/show_bug.cgi?id=198701
>+        <rdar://problem/51620347>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * 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-11  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [cairo][SVG] Putting multiple path elements in clippath causes rendering artifacts
>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 00000000000..d9eb369e94e
>--- /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 00000000000..a2873e987be
>--- /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, function() { 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 00000000000..9828b5963d9
>--- /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 00000000000..eab0307fad4
>--- /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>
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index db1b48905e8..8c594c42cb3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-06-12  Carlos Garcia Campos  <cgarcia@igalia.com>
>+
>+        [cairo][SVG] Putting multiple path elements in clippath causes rendering artifacts
>+        https://bugs.webkit.org/show_bug.cgi?id=198701
>+        <rdar://problem/51620347>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-11  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 7f242fdbecb..62e517431df 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());
> }
>

Actions: View | Formatted Diff | Diff

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