[Color] Make gradients work with ExtendedColors
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Apr 2017 17:34:50 +0000 (17:34 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Apr 2017 17:34:50 +0000 (17:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171315
<rdar://problems/31830177>

Reviewed by Antoine Quint.

Source/WebCore:

Allow gradients to hold Color objects, and thus
handle ExtendedColor. Implement the backend for
CoreGraphics.

Test: css3/color/gradients.html

* platform/graphics/Gradient.cpp:
(WebCore::Gradient::addColorStop): Just copy the Color now.
(WebCore::compareStops): Handle rename of stop to offset.
(WebCore::Gradient::hasAlpha): Use Color's helper.
* platform/graphics/Gradient.h:
(WebCore::Gradient::ColorStop::ColorStop): Rename stop
to offset, and store a Color rather than four floating
point values.
* platform/graphics/cairo/GradientCairo.cpp:
(WebCore::Gradient::platformGradient):
* platform/graphics/cg/GradientCG.cpp:
(WebCore::Gradient::platformGradient): Use the CG method
that can handle CGColorRefs with ColorSpace values, and
pass the Extended sRGB space which should be no change for
all existing gradients but also handle ColorSpaces like
Display P3.
* platform/graphics/win/GradientDirect2D.cpp:
(WebCore::Gradient::generateGradient):
* svg/SVGGradientElement.cpp:
(WebCore::SVGGradientElement::buildStops):

LayoutTests:

Test for gradients using the color() syntax. Unfortunately,
due to rounding errors, we can't use a ref test for some
transparent colors, so comment them out for the moment.
These can be re-enabled when we can specify a tolerance
value for image comparison in the testing framework.

* css3/color/gradients-expected.html: Added.
* css3/color/gradients.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/color/gradients-expected.html [new file with mode: 0644]
LayoutTests/css3/color/gradients.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Gradient.cpp
Source/WebCore/platform/graphics/Gradient.h
Source/WebCore/platform/graphics/cairo/GradientCairo.cpp
Source/WebCore/platform/graphics/cg/GradientCG.cpp
Source/WebCore/platform/graphics/win/GradientDirect2D.cpp
Source/WebCore/svg/SVGGradientElement.cpp

index da4ad19..631a2f2 100644 (file)
@@ -1,3 +1,20 @@
+2017-04-25  Dean Jackson  <dino@apple.com>
+
+        [Color] Make gradients work with ExtendedColors
+        https://bugs.webkit.org/show_bug.cgi?id=171315
+        <rdar://problems/31830177>
+
+        Reviewed by Antoine Quint.
+
+        Test for gradients using the color() syntax. Unfortunately,
+        due to rounding errors, we can't use a ref test for some
+        transparent colors, so comment them out for the moment.
+        These can be re-enabled when we can specify a tolerance
+        value for image comparison in the testing framework.
+
+        * css3/color/gradients-expected.html: Added.
+        * css3/color/gradients.html: Added.
+
 2017-04-26  Zalan Bujtas  <zalan@apple.com>
 
         Forced page break on :after triggers infinite loop in column balancing
diff --git a/LayoutTests/css3/color/gradients-expected.html b/LayoutTests/css3/color/gradients-expected.html
new file mode 100644 (file)
index 0000000..8eadf43
--- /dev/null
@@ -0,0 +1,40 @@
+<style>
+body {
+    background-image: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10"><g fill="#ccc"><rect width="5" height="5"/><rect x="5" y="5" width="5" height="5"/></g></svg>');
+    background-size: 100px 100px;
+}
+.box {
+    display: inline-block;
+    width: 80px;
+    height: 80px;
+}
+</style>
+<p>These two rows should be identical</p>
+<div class="box" style="background: linear-gradient(red, blue);"></div>
+<!-- <div class="box" style="background: linear-gradient(red, rgba(0, 0, 255, 0.5));"></div> -->
+<div class="box" style="background: linear-gradient(red, blue, green, cyan);"></div>
+<div class="box" style="background: linear-gradient(to bottom right, red, blue);"></div>
+<div class="box" style="background: linear-gradient(to bottom left, rgba(255, 0, 0, 0.6), rgba(0, 0, 255, 0.6));"></div>
+
+<br>
+
+<div class="box" style="background: linear-gradient(red, blue);"></div>
+<!-- <div class="box" style="background: linear-gradient(red, rgba(0, 0, 255, 0.5));"></div> -->
+<div class="box" style="background: linear-gradient(red, blue, green, cyan);"></div>
+<div class="box" style="background: linear-gradient(to bottom right, red, blue);"></div>
+<div class="box" style="background: linear-gradient(to bottom left, rgba(255, 0, 0, 0.6), rgba(0, 0, 255, 0.6));"></div>
+
+<br>
+
+<p>These two rows should be identical</p>
+<div class="box" style="background: radial-gradient(green, yellow);"></div>
+<!-- <div class="box" style="background: radial-gradient(orange, rgba(0, 0, 255, 0.5));"></div> -->
+<div class="box" style="background: radial-gradient(red, blue, green, cyan);"></div>
+<!-- <div class="box" style="background: radial-gradient(circle at top right, orange, purple);"></div> -->
+
+<br>
+
+<div class="box" style="background: radial-gradient(green, yellow);"></div>
+<!-- <div class="box" style="background: radial-gradient(orange, rgba(0, 0, 255, 0.5));"></div> -->
+<div class="box" style="background: radial-gradient(red, blue, green, cyan);"></div>
+<!-- <div class="box" style="background: radial-gradient(circle at top right, orange, purple);"></div> -->
diff --git a/LayoutTests/css3/color/gradients.html b/LayoutTests/css3/color/gradients.html
new file mode 100644 (file)
index 0000000..b053593
--- /dev/null
@@ -0,0 +1,46 @@
+<style>
+body {
+    background-image: url('data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10"><g fill="#ccc"><rect width="5" height="5"/><rect x="5" y="5" width="5" height="5"/></g></svg>');
+    background-size: 100px 100px;
+}
+.box {
+    display: inline-block;
+    width: 80px;
+    height: 80px;
+}
+</style>
+<p>These two rows should be identical</p>
+<div class="box" style="background: linear-gradient(red, blue);"></div>
+<!-- Floating point rounding means we can't use this in a ref test without some tolerance -->
+<!-- <div class="box" style="background: linear-gradient(red, rgba(0, 0, 255, 0.5));"></div> -->
+<div class="box" style="background: linear-gradient(red, blue, green, cyan);"></div>
+<div class="box" style="background: linear-gradient(to bottom right, red, blue);"></div>
+<div class="box" style="background: linear-gradient(to bottom left, rgba(255, 0, 0, 0.6), rgba(0, 0, 255, 0.6));"></div>
+
+<br>
+
+<div class="box" style="background: linear-gradient(color(srgb 1 0 0), blue);"></div>
+<!-- Floating point rounding means we can't use this in a ref test without some tolerance -->
+<!-- <div class="box" style="background: linear-gradient(red, color(srgb 0 0 1 / 0.5));"></div> -->
+<div class="box" style="background: linear-gradient(red, color(srgb 0 0 1), green, color(srgb 0 1 1));"></div>
+<div class="box" style="background: linear-gradient(to bottom right, color(srgb 1 0 0), color(srgb 0 0 1));"></div>
+<div class="box" style="background: linear-gradient(to bottom left, color(srgb 1 0 0 / 0.6), color(srgb 0 0 1 / 0.6));"></div>
+
+<br>
+
+<p>These two rows should be identical</p>
+<div class="box" style="background: radial-gradient(green, yellow);"></div>
+<!-- Floating point rounding means we can't use this in a ref test without some tolerance -->
+<!-- <div class="box" style="background: radial-gradient(orange, rgba(0, 0, 255, 0.5));"></div> -->
+<div class="box" style="background: radial-gradient(red, blue, green, cyan);"></div>
+<!-- Floating point rounding means we can't use this in a ref test without some tolerance -->
+<!-- <div class="box" style="background: radial-gradient(circle at top right, orange, purple);"></div> -->
+
+<br>
+
+<div class="box" style="background: radial-gradient(color(srgb 0 0.5 0), color(srgb 1 1 0));"></div>
+<!-- Floating point rounding means we can't use this in a ref test without some tolerance -->
+<!-- <div class="box" style="background: radial-gradient(orange, color(srgb 0 0 1 / 0.5));"></div> -->
+<div class="box" style="background: radial-gradient(red, color(srgb 0 0 1), green, color(srgb 0 1 1));"></div>
+<!-- Floating point rounding means we can't use this in a ref test without some tolerance -->
+<!-- <div class="box" style="background: radial-gradient(circle at top right, orange, color(srgb 0.5 0 0.5));"></div> -->
index 0882eff..e30d314 100644 (file)
@@ -1,3 +1,38 @@
+2017-04-25  Dean Jackson  <dino@apple.com>
+
+        [Color] Make gradients work with ExtendedColors
+        https://bugs.webkit.org/show_bug.cgi?id=171315
+        <rdar://problems/31830177>
+
+        Reviewed by Antoine Quint.
+
+        Allow gradients to hold Color objects, and thus
+        handle ExtendedColor. Implement the backend for
+        CoreGraphics.
+
+        Test: css3/color/gradients.html
+
+        * platform/graphics/Gradient.cpp:
+        (WebCore::Gradient::addColorStop): Just copy the Color now.
+        (WebCore::compareStops): Handle rename of stop to offset.
+        (WebCore::Gradient::hasAlpha): Use Color's helper.
+        * platform/graphics/Gradient.h:
+        (WebCore::Gradient::ColorStop::ColorStop): Rename stop
+        to offset, and store a Color rather than four floating
+        point values.
+        * platform/graphics/cairo/GradientCairo.cpp:
+        (WebCore::Gradient::platformGradient):
+        * platform/graphics/cg/GradientCG.cpp:
+        (WebCore::Gradient::platformGradient): Use the CG method
+        that can handle CGColorRefs with ColorSpace values, and
+        pass the Extended sRGB space which should be no change for
+        all existing gradients but also handle ColorSpaces like
+        Display P3.
+        * platform/graphics/win/GradientDirect2D.cpp:
+        (WebCore::Gradient::generateGradient):
+        * svg/SVGGradientElement.cpp:
+        (WebCore::SVGGradientElement::buildStops):
+
 2017-04-25  Alex Christensen  <achristensen@webkit.org>
 
         Encoded filename should be decoded for WKContentExtension.identifier
index b0d1f6e..d810c6c 100644 (file)
@@ -94,15 +94,9 @@ void Gradient::adjustParametersForTiledDrawing(FloatSize& size, FloatRect& srcRe
     srcRect.setY(0);
 }
 
-void Gradient::addColorStop(float value, const Color& color)
+void Gradient::addColorStop(float offset, const Color& color)
 {
-    // FIXME: ExtendedColor - update this to support colors with color spaces.
-    float r;
-    float g;
-    float b;
-    float a;
-    color.getRGBA(r, g, b, a);
-    m_stops.append(ColorStop(value, r, g, b, a));
+    m_stops.append(ColorStop(offset, color));
 
     m_stopsSorted = false;
     platformDestroy();
@@ -122,7 +116,7 @@ void Gradient::addColorStop(const Gradient::ColorStop& stop)
 
 static inline bool compareStops(const Gradient::ColorStop& a, const Gradient::ColorStop& b)
 {
-    return a.stop < b.stop;
+    return a.offset < b.offset;
 }
 
 void Gradient::sortStopsIfNecessary()
@@ -142,8 +136,8 @@ void Gradient::sortStopsIfNecessary()
 
 bool Gradient::hasAlpha() const
 {
-    for (size_t i = 0; i < m_stops.size(); i++) {
-        if (m_stops[i].alpha < 1)
+    for (const auto& stop : m_stops) {
+        if (!stop.color.isOpaque())
             return true;
     }
 
index ccb0188..6b91bf5 100644 (file)
@@ -29,6 +29,7 @@
 #define Gradient_h
 
 #include "AffineTransform.h"
+#include "Color.h"
 #include "FloatPoint.h"
 #include "GraphicsTypes.h"
 #include <wtf/RefCounted.h>
@@ -135,14 +136,14 @@ namespace WebCore {
 
         // FIXME: ExtendedColor - A color stop needs a notion of color space.
         struct ColorStop {
-            float stop;
-            float red;
-            float green;
-            float blue;
-            float alpha;
-
-            ColorStop() : stop(0), red(0), green(0), blue(0), alpha(0) { }
-            ColorStop(float s, float r, float g, float b, float a) : stop(s), red(r), green(g), blue(b), alpha(a) { }
+            float offset { 0 };
+            Color color;
+
+            ColorStop() { }
+            ColorStop(float offset, const Color& color)
+                : offset(offset)
+                , color(color)
+                { }
         };
 
         void setStopsSorted(bool s) { m_stopsSorted = s; }
index 724d832..a3e40df 100644 (file)
@@ -61,12 +61,15 @@ cairo_pattern_t* Gradient::platformGradient(float globalAlpha)
     else
         m_gradient = cairo_pattern_create_linear(m_p0.x(), m_p0.y(), m_p1.x(), m_p1.y());
 
-    Vector<ColorStop>::iterator stopIterator = m_stops.begin();
-    while (stopIterator != m_stops.end()) {
-        cairo_pattern_add_color_stop_rgba(m_gradient, stopIterator->stop,
-                                          stopIterator->red, stopIterator->green, stopIterator->blue,
-                                          stopIterator->alpha * globalAlpha);
-        ++stopIterator;
+    // FIXME: Add support for ExtendedColor.
+    for (const auto& stop : m_stops) {
+        float r;
+        float g;
+        float b;
+        float a;
+        stop.color.getRGBA(r, g, b, a);
+        cairo_pattern_add_color_stop_rgba(m_gradient, stop.offset,
+            r, g, b, a * globalAlpha);
     }
 
     switch (m_spreadMethod) {
index a01f147..675f045 100644 (file)
@@ -48,27 +48,18 @@ CGGradientRef Gradient::platformGradient()
 
     sortStopsIfNecessary();
 
-    // FIXME: ExtendedColor - we should generate CGColorRefs here, so that
-    // we can handle color spaces.
+    auto colorsArray = adoptCF(CFArrayCreateMutable(0, m_stops.size(), &kCFTypeArrayCallBacks));
 
-    const int cReservedStops = 3;
-    Vector<CGFloat, 4 * cReservedStops> colorComponents;
-    colorComponents.reserveInitialCapacity(m_stops.size() * 4); // RGBA components per stop
-
-    Vector<CGFloat, cReservedStops> locations;
+    const int reservedStops = 3;
+    Vector<CGFloat, reservedStops> locations;
     locations.reserveInitialCapacity(m_stops.size());
 
-    for (size_t i = 0; i < m_stops.size(); ++i) {
-        colorComponents.uncheckedAppend(m_stops[i].red);
-        colorComponents.uncheckedAppend(m_stops[i].green);
-        colorComponents.uncheckedAppend(m_stops[i].blue);
-        colorComponents.uncheckedAppend(m_stops[i].alpha);
-
-        locations.uncheckedAppend(m_stops[i].stop);
+    for (const auto& stop : m_stops) {
+        CFArrayAppendValue(colorsArray.get(), cachedCGColor(stop.color));
+        locations.uncheckedAppend(stop.offset);
     }
 
-    // FIXME: ExtendedColor - use CGGradientCreateWithColors so that we can have stops in different color spaces.
-    m_gradient = CGGradientCreateWithColorComponents(sRGBColorSpaceRef(), colorComponents.data(), locations.data(), m_stops.size());
+    m_gradient = CGGradientCreateWithColors(extendedSRGBColorSpaceRef(), colorsArray.get(), locations.data());
 
     return m_gradient;
 }
index a0380e6..a5374b3 100644 (file)
@@ -59,8 +59,15 @@ void Gradient::generateGradient(ID2D1RenderTarget* renderTarget)
     sortStopsIfNecessary();
 
     Vector<D2D1_GRADIENT_STOP> gradientStops;
-    for (auto stop : m_stops)
-        gradientStops.append(D2D1::GradientStop(stop.stop, D2D1::ColorF(stop.red, stop.green, stop.blue, stop.alpha)));
+    // FIXME: Add support for ExtendedColor.
+    for (auto stop : m_stops) {
+        float r;
+        float g;
+        float b;
+        float a;
+        stop.color.getRGBA(r, g, b, a);
+        gradientStops.append(D2D1::GradientStop(stop.offset, D2D1::ColorF(r, g, b, a)));
+    }
 
     COMPtr<ID2D1GradientStopCollection> gradientStopCollection;
     HRESULT hr = renderTarget->CreateGradientStopCollection(gradientStops.data(), gradientStops.size(), &gradientStopCollection);
index e0659e6..f211045 100644 (file)
@@ -135,17 +135,12 @@ Vector<Gradient::ColorStop> SVGGradientElement::buildStops()
     for (auto& stop : childrenOfType<SVGStopElement>(*this)) {
         const Color& color = stop.stopColorIncludingOpacity();
 
-        // Figure out right monotonic offset
+        // Figure out right monotonic offset.
         float offset = stop.offset();
         offset = std::min(std::max(previousOffset, offset), 1.0f);
         previousOffset = offset;
 
-        // Extract individual channel values
-        // FIXME: Why doesn't ColorStop take a Color and an offset??
-        float r, g, b, a;
-        color.getRGBA(r, g, b, a);
-
-        stops.append(Gradient::ColorStop(offset, r, g, b, a));
+        stops.append(Gradient::ColorStop(offset, color));
     }
 
     return stops;