git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: ae439e6)
feFlood with alpha color doesn't work correctly
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Nov 2018 17:58:56 +0000 (17:58 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Nov 2018 17:58:56 +0000 (17:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163666

Reviewed by Zalan Bujtas.
Source/WebCore:

FEFlood::platformApplySoftware() erroneously used colorWithOverrideAlpha()
rather than multiplying the flood color with the flood opacity as other browsers do.

Test: svg/filters/feFlood-with-alpha-color.html

* platform/graphics/Color.cpp:
(WebCore::Color::colorWithAlpha const): I tried using colorWithAlphaMultipliedBy() elsewhere,
and it triggered a behavior change, so add a comment.
* platform/graphics/filters/FEFlood.cpp:
(WebCore::FEFlood::platformApplySoftware):
* svg/SVGStopElement.cpp:
(WebCore::SVGStopElement::stopColorIncludingOpacity const):

LayoutTests:

* svg/filters/feFlood-with-alpha-color-expected.html: Added.
* svg/filters/feFlood-with-alpha-color.html: Added.

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

LayoutTests/ChangeLog patch | blob | history
LayoutTests/svg/filters/feFlood-with-alpha-color-expected.html [new file with mode: 0644] patch | blob
LayoutTests/svg/filters/feFlood-with-alpha-color.html [new file with mode: 0644] patch | blob
Source/WebCore/ChangeLog patch | blob | history
Source/WebCore/platform/graphics/Color.cpp patch | blob | history
Source/WebCore/platform/graphics/filters/FEFlood.cpp patch | blob | history
Source/WebCore/svg/SVGStopElement.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 298fa83..cf70365 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2018-11-12  Simon Fraser  <simon.fraser@apple.com>
+
+        feFlood with alpha color doesn't work correctly
+        https://bugs.webkit.org/show_bug.cgi?id=163666
+
+        Reviewed by Zalan Bujtas.
+
+        * svg/filters/feFlood-with-alpha-color-expected.html: Added.
+        * svg/filters/feFlood-with-alpha-color.html: Added.
+
 2018-11-12  Eric Carlson  <eric.carlson@apple.com>
 
         Require <iframe allow="display"> for an iframe to use getDisplayMedia
 2018-11-12  Eric Carlson  <eric.carlson@apple.com>
 
         Require <iframe allow="display"> for an iframe to use getDisplayMedia
diff --git a/LayoutTests/svg/filters/feFlood-with-alpha-color-expected.html b/LayoutTests/svg/filters/feFlood-with-alpha-color-expected.html
new file mode 100644 (file)
index 0000000..c64bef0
--- /dev/null
+++ b/LayoutTests/svg/filters/feFlood-with-alpha-color-expected.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+    <body>
+        <p>You should see two identical gray squares below</p>
+        <svg width="600px" height="500px" xmlns="http://www.w3.org/2000/svg">
+            <rect x="0" y="0" width="200" height="200" fill="rgba(0, 0, 0, 0.4)"/>
+            <rect x="0" y="250" width="200" height="200" fill="rgba(0, 0, 0, 0.4)"/>
+        </svg>
+    </body>
+</html>
diff --git a/LayoutTests/svg/filters/feFlood-with-alpha-color.html b/LayoutTests/svg/filters/feFlood-with-alpha-color.html
new file mode 100644 (file)
index 0000000..2c46708
--- /dev/null
+++ b/LayoutTests/svg/filters/feFlood-with-alpha-color.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+    <body>
+        <p>You should see two identical gray squares below</p>
+        <svg width="600px" height="500px" xmlns="http://www.w3.org/2000/svg">
+            <defs>
+                <filter id="flood" filterUnits="objectBoundingBox" x="0" y="0" width="1" height="1">
+                    <feFlood flood-color="rgba(0, 0, 0, 0.4)"/>
+                </filter>
+
+                <filter id="flood-with-alpha" filterUnits="objectBoundingBox" x="0" y="0" width="1" height="1">
+                    <feFlood flood-color="rgba(0, 0, 0, 0.8)" flood-opacity="0.5"/>
+                </filter>
+            </defs>
+            <rect x="0" y="0" width="200" height="200" filter="url(#flood)"/>
+            <rect x="0" y="250" width="200" height="200" filter="url(#flood-with-alpha)"/>
+        </svg>
+    </body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index b63e170..a7839fb 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2018-11-12  Simon Fraser  <simon.fraser@apple.com>
+
+        feFlood with alpha color doesn't work correctly
+        https://bugs.webkit.org/show_bug.cgi?id=163666
+
+        Reviewed by Zalan Bujtas.
+        
+        FEFlood::platformApplySoftware() erroneously used colorWithOverrideAlpha()
+        rather than multiplying the flood color with the flood opacity as other browsers do.
+
+        Test: svg/filters/feFlood-with-alpha-color.html
+
+        * platform/graphics/Color.cpp:
+        (WebCore::Color::colorWithAlpha const): I tried using colorWithAlphaMultipliedBy() elsewhere,
+        and it triggered a behavior change, so add a comment.
+        * platform/graphics/filters/FEFlood.cpp:
+        (WebCore::FEFlood::platformApplySoftware):
+        * svg/SVGStopElement.cpp:
+        (WebCore::SVGStopElement::stopColorIncludingOpacity const):
+
 2018-11-12  Eric Carlson  <eric.carlson@apple.com>
 
         Require <iframe allow="display"> for an iframe to use getDisplayMedia
 2018-11-12  Eric Carlson  <eric.carlson@apple.com>
 
         Require <iframe allow="display"> for an iframe to use getDisplayMedia
diff --git a/Source/WebCore/platform/graphics/Color.cpp b/Source/WebCore/platform/graphics/Color.cpp
index 5ef11b0..8dbd36c 100644 (file)
--- a/Source/WebCore/platform/graphics/Color.cpp
+++ b/Source/WebCore/platform/graphics/Color.cpp
@@ -517,7 +517,7 @@ Color Color::colorWithAlpha(float alpha) const
     if (isExtended())
         return Color { m_colorData.extendedColor->red(), m_colorData.extendedColor->green(), m_colorData.extendedColor->blue(), alpha, m_colorData.extendedColor->colorSpace() };
 
     if (isExtended())
         return Color { m_colorData.extendedColor->red(), m_colorData.extendedColor->green(), m_colorData.extendedColor->blue(), alpha, m_colorData.extendedColor->colorSpace() };
 
-    int newAlpha = alpha * 255;
+    int newAlpha = alpha * 255; // Why doesn't this use colorFloatToRGBAByte() like colorWithOverrideAlpha()?
 
     Color result = { red(), green(), blue(), newAlpha };
     if (isSemantic())
 
     Color result = { red(), green(), blue(), newAlpha };
     if (isSemantic())
diff --git a/Source/WebCore/platform/graphics/filters/FEFlood.cpp b/Source/WebCore/platform/graphics/filters/FEFlood.cpp
index 7840aae..60476eb 100644 (file)
--- a/Source/WebCore/platform/graphics/filters/FEFlood.cpp
+++ b/Source/WebCore/platform/graphics/filters/FEFlood.cpp
@@ -63,7 +63,9 @@ void FEFlood::platformApplySoftware()
     if (!resultImage)
         return;
 
     if (!resultImage)
         return;
 
-    const Color& color = colorWithOverrideAlpha(floodColor().rgb(), floodOpacity());
+    // FIXME: This should use colorWithAlphaMultipliedBy() but that has different rounding of the alpha component that breaks some tests.
+    float colorAlpha = floodColor().alpha() / 255.0;
+    auto color = colorWithOverrideAlpha(floodColor().rgb(), colorAlpha * floodOpacity());
     resultImage->context().fillRect(FloatRect(FloatPoint(), absolutePaintRect().size()), color);
 }
 
     resultImage->context().fillRect(FloatRect(FloatPoint(), absolutePaintRect().size()), color);
 }
 
diff --git a/Source/WebCore/svg/SVGStopElement.cpp b/Source/WebCore/svg/SVGStopElement.cpp
index ecc20c4..6601121 100644 (file)
--- a/Source/WebCore/svg/SVGStopElement.cpp
+++ b/Source/WebCore/svg/SVGStopElement.cpp
@@ -92,14 +92,13 @@ bool SVGStopElement::rendererIsNeeded(const RenderStyle&)
 Color SVGStopElement::stopColorIncludingOpacity() const
 {
     auto* style = renderer() ? &renderer()->style() : nullptr;
 Color SVGStopElement::stopColorIncludingOpacity() const
 {
     auto* style = renderer() ? &renderer()->style() : nullptr;
-    // FIXME: This check for null style exists to address Bug WK 90814, a rare crash condition in
-    // which the renderer or style is null. This entire class is scheduled for removal (Bug WK 86941)
-    // and we will tolerate this null check until then.
+    // FIXME: This check for null style exists to address Bug WK 90814, a rare crash condition in which the renderer or style is null.
     if (!style)
     if (!style)
-        return Color(Color::transparent, true); // Transparent black.
+        return Color(Color::transparent, true);
 
     const SVGRenderStyle& svgStyle = style->svgStyle();
     float colorAlpha = svgStyle.stopColor().alpha() / 255.0;
 
     const SVGRenderStyle& svgStyle = style->svgStyle();
     float colorAlpha = svgStyle.stopColor().alpha() / 255.0;
+    // FIXME: This should use colorWithAlphaMultipliedBy() but that has different rounding of the alpha component.
     return colorWithOverrideAlpha(svgStyle.stopColor().rgb(), colorAlpha * svgStyle.stopOpacity());
 }
 
     return colorWithOverrideAlpha(svgStyle.stopColor().rgb(), colorAlpha * svgStyle.stopOpacity());
 }
 
Mirror of the WebKit open source project from svn.webkit.org.