Refine and simplify some color-related code
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Sep 2015 01:25:35 +0000 (01:25 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 19 Sep 2015 01:25:35 +0000 (01:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148961

Reviewed by Anders Carlsson.

Refactoring code that seems to be covered by existing tests.

* css/CSSParser.cpp:
(WebCore::CSSParser::parseColor): Handle the empty string efficiently so that
callers don't need to do that.

* platform/graphics/Color.h: Started adding comments about deprecation.
Added RGBA class for future use whenever we need an RGBA quadruplet rather than
a color with a color space. Added FIXME about future evoluation of the classes here.
Added OptionalColor so we can start removing the "invalid color" feature from Color.
Added roundAndClampColorChannel function.

* svg/ColorDistance.cpp: Removed.
* svg/ColorDistance.h: Removed.

* CMakeLists.txt: Removed ColorDistance.
* WebCore.vcxproj/WebCore.vcxproj: Ditto.
* WebCore.vcxproj/WebCore.vcxproj.filters: Ditto.
* WebCore.xcodeproj/project.pbxproj: Ditto.
* svg/SVGAllInOne.cpp: Ditto.

* svg/SVGAnimatedColor.cpp:
(WebCore::SVGAnimatedColorAnimator::SVGAnimatedColorAnimator): Changed to take
a reference instead of a pointer.
(WebCore::SVGAnimatedColorAnimator::constructFromString): Simplified since the
SVGColor::colorFromRGBColorString will handle the empty string.
(WebCore::SVGAnimatedColorAnimator::addAnimatedTypes): Moved the code to add the
RGB channels of two colors here from ColorDistance::addColors since this is the
only place it was used.
(WebCore::currentColor): Refactored adjustForCurrentColor function into this.
Helper for the code below.
(WebCore::SVGAnimatedColorAnimator::calculateAnimatedValue): Refactored to use
the new currentColor function and replaced the use of the ColorDistance::clampColor
function here with a bit of code here in the one place it was used.
(WebCore::SVGAnimatedColorAnimator::calculateDistance): Moved the distance algorithm
here from ColorDistance::distance.

* svg/SVGAnimatedColor.h: Removed unneeded forward declaration, changed constructor
to take references instead of pointers, and made all class member functions private.

* svg/SVGAnimatedType.cpp:
(WebCore::SVGAnimatedType::setValueAsString): Removed special case for empty string,
since SVGColor::colorFromRGBColorString does the same thing.

* svg/SVGAnimatorFactory.h:
(WebCore::SVGAnimatorFactory::create): Pass references rather tha pointers to the
SVGAnimatedColorAnimator constructor.

* svg/SVGColor.cpp:
(WebCore::SVGColor::colorFromRGBColorString): Added more FIXMEs about the future of
this function.
(WebCore::SVGColor::setRGBColor): Fixed confusing verb tense.
(WebCore::SVGColor::customCSSText): Use ASCII literal for an ASCII literal.

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

15 files changed:
Source/WebCore/CMakeLists.txt
Source/WebCore/ChangeLog
Source/WebCore/WebCore.vcxproj/WebCore.vcxproj
Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/css/CSSParser.cpp
Source/WebCore/platform/graphics/Color.h
Source/WebCore/svg/ColorDistance.cpp [deleted file]
Source/WebCore/svg/ColorDistance.h [deleted file]
Source/WebCore/svg/SVGAllInOne.cpp
Source/WebCore/svg/SVGAnimatedColor.cpp
Source/WebCore/svg/SVGAnimatedColor.h
Source/WebCore/svg/SVGAnimatedType.cpp
Source/WebCore/svg/SVGAnimatorFactory.h
Source/WebCore/svg/SVGColor.cpp

index 3c147ef73714ed3705ec86a0e58ba3cb21f36197..ca1de4d156948bf7e10d0eeb466ccfc5e366b11a 100644 (file)
@@ -2593,7 +2593,6 @@ set(WebCore_SOURCES
     style/StyleResolveForDocument.cpp
     style/StyleResolveTree.cpp
 
-    svg/ColorDistance.cpp
     svg/SVGAElement.cpp
     svg/SVGAltGlyphDefElement.cpp
     svg/SVGAltGlyphElement.cpp
index 7996d7df299a2454b66b947409d7e7051526360f..51d4a357d891957d959b65b3ba866356e281b849 100644 (file)
@@ -1,3 +1,64 @@
+2015-09-08  Darin Adler  <darin@apple.com>
+
+        Refine and simplify some color-related code
+        https://bugs.webkit.org/show_bug.cgi?id=148961
+
+        Reviewed by Anders Carlsson.
+
+        Refactoring code that seems to be covered by existing tests.
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseColor): Handle the empty string efficiently so that
+        callers don't need to do that.
+
+        * platform/graphics/Color.h: Started adding comments about deprecation.
+        Added RGBA class for future use whenever we need an RGBA quadruplet rather than
+        a color with a color space. Added FIXME about future evoluation of the classes here.
+        Added OptionalColor so we can start removing the "invalid color" feature from Color.
+        Added roundAndClampColorChannel function.
+
+        * svg/ColorDistance.cpp: Removed.
+        * svg/ColorDistance.h: Removed.
+
+        * CMakeLists.txt: Removed ColorDistance.
+        * WebCore.vcxproj/WebCore.vcxproj: Ditto.
+        * WebCore.vcxproj/WebCore.vcxproj.filters: Ditto.
+        * WebCore.xcodeproj/project.pbxproj: Ditto.
+        * svg/SVGAllInOne.cpp: Ditto.
+
+        * svg/SVGAnimatedColor.cpp:
+        (WebCore::SVGAnimatedColorAnimator::SVGAnimatedColorAnimator): Changed to take
+        a reference instead of a pointer.
+        (WebCore::SVGAnimatedColorAnimator::constructFromString): Simplified since the
+        SVGColor::colorFromRGBColorString will handle the empty string.
+        (WebCore::SVGAnimatedColorAnimator::addAnimatedTypes): Moved the code to add the
+        RGB channels of two colors here from ColorDistance::addColors since this is the
+        only place it was used.
+        (WebCore::currentColor): Refactored adjustForCurrentColor function into this.
+        Helper for the code below.
+        (WebCore::SVGAnimatedColorAnimator::calculateAnimatedValue): Refactored to use
+        the new currentColor function and replaced the use of the ColorDistance::clampColor
+        function here with a bit of code here in the one place it was used.
+        (WebCore::SVGAnimatedColorAnimator::calculateDistance): Moved the distance algorithm
+        here from ColorDistance::distance.
+
+        * svg/SVGAnimatedColor.h: Removed unneeded forward declaration, changed constructor
+        to take references instead of pointers, and made all class member functions private.
+
+        * svg/SVGAnimatedType.cpp:
+        (WebCore::SVGAnimatedType::setValueAsString): Removed special case for empty string,
+        since SVGColor::colorFromRGBColorString does the same thing.
+
+        * svg/SVGAnimatorFactory.h:
+        (WebCore::SVGAnimatorFactory::create): Pass references rather tha pointers to the
+        SVGAnimatedColorAnimator constructor.
+
+        * svg/SVGColor.cpp:
+        (WebCore::SVGColor::colorFromRGBColorString): Added more FIXMEs about the future of
+        this function.
+        (WebCore::SVGColor::setRGBColor): Fixed confusing verb tense.
+        (WebCore::SVGColor::customCSSText): Use ASCII literal for an ASCII literal.
+
 2015-09-18  Eric Carlson  <eric.carlson@apple.com>
 
         UserMediaClientMock leaks every test run
index 08c449668aa28d60e553f95e2622ff02051f6b50..7f392458bbcfd52bc4d5b671d8a38e4933dc3d8e 100644 (file)
     <ClInclude Include="..\plugins\npfunctions.h" />
     <ClInclude Include="..\plugins\npruntime.h" />
     <ClInclude Include="..\plugins\PluginData.h" />
-    <ClInclude Include="..\svg\ColorDistance.h" />
     <ClInclude Include="..\svg\SVGAElement.h" />
     <ClInclude Include="..\svg\SVGAltGlyphDefElement.h" />
     <ClInclude Include="..\svg\SVGAltGlyphElement.h" />
index 9787bc805b3b191c0491932ce47f657dd9009b37..6e8c2840f23dea9f44f2e84d0f8936e73a5eba55 100644 (file)
     <ClInclude Include="..\platform\graphics\opentype\OpenTypeVerticalData.h">
       <Filter>platform\graphics\opentype</Filter>
     </ClInclude>
-    <ClInclude Include="..\svg\ColorDistance.h">
-      <Filter>svg</Filter>
-    </ClInclude>
     <ClInclude Include="..\svg\SVGAElement.h">
       <Filter>svg</Filter>
     </ClInclude>
index 784b03223a6282c431f41452c9583497ef2a9a3f..588a7fc88630cd885561ec5d05152f0d64ac05f8 100644 (file)
                B1D5ECB5134B58DA0087C78F /* CallbackFunction.h in Headers */ = {isa = PBXBuildFile; fileRef = B1D5ECB4134B58DA0087C78F /* CallbackFunction.h */; };
                B20111070AB7740500DB0E68 /* JSSVGAElement.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B20111050AB7740500DB0E68 /* JSSVGAElement.cpp */; };
                B20111080AB7740500DB0E68 /* JSSVGAElement.h in Headers */ = {isa = PBXBuildFile; fileRef = B20111060AB7740500DB0E68 /* JSSVGAElement.h */; };
-               B22279620D00BF220071B782 /* ColorDistance.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B22277CB0D00BF1F0071B782 /* ColorDistance.cpp */; };
-               B22279630D00BF220071B782 /* ColorDistance.h in Headers */ = {isa = PBXBuildFile; fileRef = B22277CC0D00BF1F0071B782 /* ColorDistance.h */; };
                B22279640D00BF220071B782 /* GradientAttributes.h in Headers */ = {isa = PBXBuildFile; fileRef = B22277CD0D00BF1F0071B782 /* GradientAttributes.h */; };
                B22279650D00BF220071B782 /* LinearGradientAttributes.h in Headers */ = {isa = PBXBuildFile; fileRef = B22277CE0D00BF1F0071B782 /* LinearGradientAttributes.h */; };
                B22279710D00BF220071B782 /* PatternAttributes.h in Headers */ = {isa = PBXBuildFile; fileRef = B22277DB0D00BF1F0071B782 /* PatternAttributes.h */; };
                B1D5ECB4134B58DA0087C78F /* CallbackFunction.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CallbackFunction.h; sourceTree = "<group>"; };
                B20111050AB7740500DB0E68 /* JSSVGAElement.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = JSSVGAElement.cpp; sourceTree = "<group>"; };
                B20111060AB7740500DB0E68 /* JSSVGAElement.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = JSSVGAElement.h; sourceTree = "<group>"; };
-               B22277CB0D00BF1F0071B782 /* ColorDistance.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = ColorDistance.cpp; sourceTree = "<group>"; };
-               B22277CC0D00BF1F0071B782 /* ColorDistance.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = ColorDistance.h; sourceTree = "<group>"; };
                B22277CD0D00BF1F0071B782 /* GradientAttributes.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = GradientAttributes.h; sourceTree = "<group>"; };
                B22277CE0D00BF1F0071B782 /* LinearGradientAttributes.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = LinearGradientAttributes.h; sourceTree = "<group>"; };
                B22277DB0D00BF1F0071B782 /* PatternAttributes.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = PatternAttributes.h; sourceTree = "<group>"; };
                                E46E97860DAAD61B0071E894 /* animation */,
                                B25598860D00D8B800BB825C /* graphics */,
                                081CDFBD126ECFE800D215CA /* properties */,
-                               B22277CB0D00BF1F0071B782 /* ColorDistance.cpp */,
-                               B22277CC0D00BF1F0071B782 /* ColorDistance.h */,
                                B22277CD0D00BF1F0071B782 /* GradientAttributes.h */,
                                B22277CE0D00BF1F0071B782 /* LinearGradientAttributes.h */,
                                B22277DB0D00BF1F0071B782 /* PatternAttributes.h */,
                                B27535670B053814002CE64F /* Color.h in Headers */,
                                C330A22313EC196B0000B45B /* ColorChooser.h in Headers */,
                                C37CDEBD149EF2030042090D /* ColorChooserClient.h in Headers */,
-                               B22279630D00BF220071B782 /* ColorDistance.h in Headers */,
                                F55B3DB41251F12D003EF269 /* ColorInputType.h in Headers */,
                                EDE3A5000C7A430600956A37 /* ColorMac.h in Headers */,
                                9382DF5810A8D5C900925652 /* ColorSpace.h in Headers */,
                                B27535660B053814002CE64F /* Color.cpp in Sources */,
                                0FCF33240F2B9715004B6795 /* ColorCG.cpp in Sources */,
                                1ABA76C911D20E47004C201C /* ColorData.cpp in Sources */,
-                               B22279620D00BF220071B782 /* ColorDistance.cpp in Sources */,
                                BC29935D17A1DD5800BCE880 /* ColorInputType.cpp in Sources */,
                                B27535770B053814002CE64F /* ColorMac.mm in Sources */,
                                26E944D81AC4B2DD007B85B5 /* CombinedURLFilters.cpp in Sources */,
index 99fd21b499d0c0fde02202ce8e50e720cf7145ef..636d07fda632f191f097d3d80a485a4aafe2fc7f 100644 (file)
@@ -1364,6 +1364,9 @@ CSSParser::ParseResult CSSParser::parseValue(MutableStyleProperties* declaration
 // can set it to a default color and ignore the boolean result.
 bool CSSParser::parseColor(RGBA32& color, const String& string, bool strict)
 {
+    if (string.isEmpty())
+        return false;
+
     // First try creating a color specified by name, rgba(), rgb() or "#" syntax.
     if (fastParseColor(color, string, strict))
         return true;
index c5f09e77c5e288974d4945b6ef14a74063844827..6c79c63bcfa812fac468fe886d7045c898284433 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2003, 2004, 2005, 2006, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2003-2006, 2010, 2015 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 #ifndef Color_h
 #define Color_h
 
+#include "ColorSpace.h"
+#include <algorithm>
+#include <cmath>
 #include <unicode/uchar.h>
-#include <wtf/FastMalloc.h>
 #include <wtf/Forward.h>
 #include <wtf/text/LChar.h>
 
 #if USE(CG)
-#include "ColorSpace.h"
 typedef struct CGColor* CGColorRef;
-#if PLATFORM(IOS)
-typedef struct CGColorSpace* CGColorSpaceRef;
-#endif // PLATFORM(IOS)
 #endif
 
 #if PLATFORM(GTK)
@@ -48,9 +46,7 @@ typedef struct _GdkRGBA GdkRGBA;
 
 namespace WebCore {
 
-class Color;
-
-typedef unsigned RGBA32;        // RGBA quadruplet
+typedef unsigned RGBA32; // Deprecated: Type for an RGBA quadruplet. Use RGBA class instead.
 
 WEBCORE_EXPORT RGBA32 makeRGB(int r, int g, int b);
 WEBCORE_EXPORT RGBA32 makeRGBA(int r, int g, int b, int a);
@@ -60,17 +56,43 @@ WEBCORE_EXPORT RGBA32 makeRGBA32FromFloats(float r, float g, float b, float a);
 RGBA32 makeRGBAFromHSLA(double h, double s, double l, double a);
 RGBA32 makeRGBAFromCMYKA(float c, float m, float y, float k, float a);
 
-int differenceSquared(const Color&, const Color&);
-
 inline int redChannel(RGBA32 color) { return (color >> 16) & 0xFF; }
 inline int greenChannel(RGBA32 color) { return (color >> 8) & 0xFF; }
 inline int blueChannel(RGBA32 color) { return color & 0xFF; }
 inline int alphaChannel(RGBA32 color) { return (color >> 24) & 0xFF; }
 
+uint8_t roundAndClampColorChannel(int);
+uint8_t roundAndClampColorChannel(float);
+
+class RGBA {
+public:
+    RGBA(); // all channels zero, including alpha
+    RGBA(uint8_t red, uint8_t green, uint8_t blue, uint8_t alpha);
+    RGBA(uint8_t red, uint8_t green, uint8_t blue); // opaque, alpha of 1
+
+    uint8_t red() const;
+    uint8_t green() const;
+    uint8_t blue() const;
+    uint8_t alpha() const;
+
+    bool hasAlpha() const;
+
+private:
+    friend class Color;
+
+    unsigned m_integer { 0 };
+};
+
+bool operator==(const RGBA&, const RGBA&);
+bool operator!=(const RGBA&, const RGBA&);
+
 class Color {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     Color() : m_color(0), m_valid(false) { }
+    Color(RGBA, ColorSpace);
+
+    // FIXME: Remove all these constructors and creation functions and replace the ones that are still needed with free functions.
     Color(RGBA32 color, bool valid = true) : m_color(color), m_valid(valid) { ASSERT(!m_color || m_valid); }
     Color(int r, int g, int b) : m_color(makeRGB(r, g, b)), m_valid(true) { }
     Color(int r, int g, int b, int a) : m_color(makeRGBA(r, g, b, a)), m_valid(true) { }
@@ -80,7 +102,6 @@ public:
     Color(float c, float m, float y, float k, float a) : m_color(makeRGBAFromCMYKA(c, m, y, k, a)), m_valid(true) { }
     WEBCORE_EXPORT explicit Color(const String&);
     explicit Color(const char*);
-
     static Color createUnchecked(int r, int g, int b)
     {
         RGBA32 color = 0xFF000000 | r << 16 | g << 8 | b;
@@ -104,6 +125,7 @@ public:
 
     void setNamedColor(const String&);
 
+    // FIXME: Remove this after moving clients to all use OptionalColor instead.
     bool isValid() const { return m_valid; }
 
     bool hasAlpha() const { return alpha() < 255; }
@@ -165,6 +187,82 @@ private:
     bool m_valid;
 };
 
+class OptionalColor : public Color {
+public:
+    OptionalColor();
+    OptionalColor(const Color&);
+
+    explicit operator bool() const;
+
+private:
+    // FIXME: Change to use Optional<Color>?
+    // FIXME: Convert all callers to use Optional<Color>?
+    bool m_isEngaged;
+    Color m_color;
+};
+
+bool operator==(const Color&, const Color&);
+bool operator!=(const Color&, const Color&);
+
+Color colorFromPremultipliedARGB(RGBA32);
+RGBA32 premultipliedARGBFromColor(const Color&);
+
+Color blend(const Color& from, const Color& to, double progress, bool blendPremultiplied = true);
+
+int differenceSquared(const Color&, const Color&);
+
+uint16_t fastDivideBy255(uint16_t);
+
+#if USE(CG)
+WEBCORE_EXPORT CGColorRef cachedCGColor(const Color&, ColorSpace);
+#endif
+
+inline RGBA::RGBA()
+{
+}
+
+inline RGBA::RGBA(uint8_t red, uint8_t green, uint8_t blue, uint8_t alpha)
+    : m_integer(alpha << 24 | red << 16 | green << 8 | blue)
+{
+}
+
+inline RGBA::RGBA(uint8_t red, uint8_t green, uint8_t blue)
+    : m_integer(0xFF000000 | red << 16 | green << 8 | blue)
+{
+}
+
+inline uint8_t RGBA::red() const
+{
+    return m_integer >> 16;
+}
+
+inline uint8_t RGBA::green() const
+{
+    return m_integer >> 8;
+}
+
+inline uint8_t RGBA::blue() const
+{
+    return m_integer;
+}
+
+inline uint8_t RGBA::alpha() const
+{
+    return m_integer >> 24;
+}
+
+inline bool RGBA::hasAlpha() const
+{
+    return (m_integer & 0xFF000000) != 0xFF000000;
+}
+
+inline Color::Color(RGBA color, ColorSpace space)
+    : m_color(color.m_integer)
+    , m_valid(true)
+{
+    ASSERT_UNUSED(space, space == ColorSpaceSRGB);
+}
+
 inline bool operator==(const Color& a, const Color& b)
 {
     return a.rgb() == b.rgb() && a.isValid() == b.isValid();
@@ -175,22 +273,25 @@ inline bool operator!=(const Color& a, const Color& b)
     return !(a == b);
 }
 
-Color colorFromPremultipliedARGB(RGBA32);
-RGBA32 premultipliedARGBFromColor(const Color&);
-Color blend(const Color& from, const Color& to, double progress, bool blendPremultiplied = true);
+inline uint8_t roundAndClampColorChannel(int value)
+{
+    return std::max(0, std::min(255, value));
+}
+
+inline uint8_t roundAndClampColorChannel(float value)
+{
+    return std::max(0.f, std::min(255.f, std::round(value)));
+}
 
 inline uint16_t fastDivideBy255(uint16_t value)
 {
-    // This is an approximate algorithm for division by 255, but it gives accurate results for 16bit values.
+    // While this is an approximate algorithm for division by 255, it gives perfectly accurate results for 16-bit values.
+    // FIXME: Since this gives accurate results for 16-bit values, we should get this optimization into compilers like clang.
     uint16_t approximation = value >> 8;
     uint16_t remainder = value - (approximation * 255) + 1;
     return approximation + (remainder >> 8);
 }
 
-#if USE(CG)
-WEBCORE_EXPORT CGColorRef cachedCGColor(const Color&, ColorSpace);
-#endif
-
 } // namespace WebCore
 
 #endif // Color_h
diff --git a/Source/WebCore/svg/ColorDistance.cpp b/Source/WebCore/svg/ColorDistance.cpp
deleted file mode 100644 (file)
index a2ee930..0000000
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- * Copyright (C) 2007 Eric Seidel <eric@webkit.org>
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Library General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Library General Public License for more details.
- *
- * You should have received a copy of the GNU Library General Public License
- * along with this library; see the file COPYING.LIB.  If not, write to
- * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- * Boston, MA 02110-1301, USA.
- */
-
-#include "config.h"
-#include "ColorDistance.h"
-
-#include "Color.h"
-#include <wtf/MathExtras.h>
-
-namespace WebCore {
-
-ColorDistance::ColorDistance()
-    : m_redDiff(0)
-    , m_greenDiff(0)
-    , m_blueDiff(0)
-{
-}
-
-ColorDistance::ColorDistance(const Color& fromColor, const Color& toColor)
-    : m_redDiff(toColor.red() - fromColor.red())
-    , m_greenDiff(toColor.green() - fromColor.green())
-    , m_blueDiff(toColor.blue() - fromColor.blue())
-{
-}
-
-ColorDistance::ColorDistance(int redDiff, int greenDiff, int blueDiff)
-    : m_redDiff(redDiff)
-    , m_greenDiff(greenDiff)
-    , m_blueDiff(blueDiff)
-{
-}
-
-static inline int clampColorValue(int v)
-{
-    if (v > 255)
-        v = 255;
-    else if (v < 0)
-        v = 0;
-    return v;
-}
-
-ColorDistance ColorDistance::scaledDistance(float scaleFactor) const
-{
-    return ColorDistance(static_cast<int>(scaleFactor * m_redDiff),
-                         static_cast<int>(scaleFactor * m_greenDiff),
-                         static_cast<int>(scaleFactor * m_blueDiff));
-}
-
-Color ColorDistance::clampColor(int red, int green, int blue, int alpha)
-{
-    return Color(clampColorValue(red), clampColorValue(green), clampColorValue(blue), clampColorValue(alpha));
-}
-
-Color ColorDistance::addColors(const Color& first, const Color& second)
-{
-    return Color(first.red() + second.red(), first.green() + second.green(), first.blue() + second.blue());
-}
-
-Color ColorDistance::addToColor(const Color& color) const
-{
-    return Color(color.red() + m_redDiff, color.green() + m_greenDiff, color.blue() + m_blueDiff);
-}
-
-bool ColorDistance::isZero() const
-{
-    return !m_redDiff && !m_blueDiff && !m_greenDiff;
-}
-
-float ColorDistance::distance() const
-{
-    // This is just a simple distance calculation, not respecting color spaces
-    return sqrtf(m_redDiff * m_redDiff + m_blueDiff * m_blueDiff + m_greenDiff * m_greenDiff);
-}
-
-}
diff --git a/Source/WebCore/svg/ColorDistance.h b/Source/WebCore/svg/ColorDistance.h
deleted file mode 100644 (file)
index 57dd2ad..0000000
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Copyright (C) 2007 Eric Seidel <eric@webkit.org>
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Library General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Library General Public License for more details.
- *
- * You should have received a copy of the GNU Library General Public License
- * along with this library; see the file COPYING.LIB.  If not, write to
- * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- * Boston, MA 02110-1301, USA.
- */
-
-#ifndef ColorDistance_h
-#define ColorDistance_h
-
-namespace WebCore {
-    
-class Color;
-    
-class ColorDistance {
-public:
-    ColorDistance();
-    ColorDistance(const Color& fromColor, const Color& toColor);
-    ColorDistance(int redDiff, int blueDiff, int greenDiff);
-
-    ColorDistance scaledDistance(float scaleFactor) const;
-    Color addToColor(const Color&) const;
-        
-    static Color addColors(const Color&, const Color&);
-    static Color clampColor(int red, int green, int blue, int alpha);
-
-    bool isZero() const;
-
-    float distance() const;
-        
-private:
-    int m_redDiff;
-    int m_greenDiff;
-    int m_blueDiff;
-};
-}
-
-#endif // ColorDistance_h
index 214fb084f17d1064bd44bb26569aca2cf4dff451..2f26c363250172979cc2783709ee3d10a83b0674 100644 (file)
@@ -25,7 +25,6 @@
 
 // This all-in-one cpp file cuts down on template bloat to allow us to build our Windows release build.
 
-#include "ColorDistance.cpp"
 #include "SVGAElement.cpp"
 #include "SVGAltGlyphDefElement.cpp"
 #include "SVGAltGlyphElement.cpp"
index ac486458dd250fd892024355b8d64694d5072399..de8a22bad2b5ac7fe9491c178032918172d0a76b 100644 (file)
 #include "config.h"
 #include "SVGAnimatedColor.h"
 
-#include "ColorDistance.h"
 #include "RenderElement.h"
 #include "SVGAnimateElementBase.h"
 #include "SVGColor.h"
 
 namespace WebCore {
 
-SVGAnimatedColorAnimator::SVGAnimatedColorAnimator(SVGAnimationElement* animationElement, SVGElement* contextElement)
-    : SVGAnimatedTypeAnimator(AnimatedColor, animationElement, contextElement)
+SVGAnimatedColorAnimator::SVGAnimatedColorAnimator(SVGAnimationElement& animationElement, SVGElement& contextElement)
+    : SVGAnimatedTypeAnimator(AnimatedColor, &animationElement, &contextElement)
 {
 }
 
 std::unique_ptr<SVGAnimatedType> SVGAnimatedColorAnimator::constructFromString(const String& string)
 {
-    auto animatedType = SVGAnimatedType::createColor(std::make_unique<Color>());
-    animatedType->color() = string.isEmpty() ? Color() : SVGColor::colorFromRGBColorString(string);
-    return animatedType;
+    return SVGAnimatedType::createColor(std::make_unique<Color>(SVGColor::colorFromRGBColorString(string)));
 }
 
 void SVGAnimatedColorAnimator::addAnimatedTypes(SVGAnimatedType* from, SVGAnimatedType* to)
 {
+    ASSERT(from);
+    ASSERT(to);
     ASSERT(from->type() == AnimatedColor);
-    ASSERT(from->type() == to->type());
-    to->color() = ColorDistance::addColors(from->color(), to->color());
+    ASSERT(to->type() == AnimatedColor);
+
+    // Ignores any alpha and sets alpha on result to 100% opaque.
+    auto& fromColor = from->color();
+    auto& toColor = to->color();
+    toColor = { roundAndClampColorChannel(toColor.red() + fromColor.red()),
+        roundAndClampColorChannel(toColor.green() + fromColor.green()),
+        roundAndClampColorChannel(toColor.blue() + fromColor.blue()) };
 }
 
-static inline void adjustForCurrentColor(SVGElement* targetElement, Color& color)
+static inline Color currentColor(SVGElement& targetElement)
 {
-    ASSERT(targetElement);
-
-    if (RenderElement* targetRenderer = targetElement->renderer())
-        color = targetRenderer->style().visitedDependentColor(CSSPropertyColor);
-    else
-        color = Color();
+    RenderElement* targetRenderer = targetElement.renderer();
+    if (!targetRenderer)
+        return { };
+    return targetRenderer->style().visitedDependentColor(CSSPropertyColor);
 }
 
 static Color parseColorFromString(SVGAnimationElement*, const String& string)
@@ -68,8 +71,6 @@ void SVGAnimatedColorAnimator::calculateAnimatedValue(float percentage, unsigned
 
     Color fromColor = m_animationElement->animationMode() == ToAnimation ? animated->color() : from->color();
     Color toColor = to->color();
-    const Color& toAtEndOfDurationColor = toAtEndOfDuration->color();
-    Color& animatedColor = animated->color();
 
     // Apply CSS inheritance rules.
     m_animationElement->adjustForInheritance<Color>(parseColorFromString, m_animationElement->fromPropertyValueType(), fromColor, m_contextElement);
@@ -77,35 +78,40 @@ void SVGAnimatedColorAnimator::calculateAnimatedValue(float percentage, unsigned
 
     // Apply <animateColor> rules.
     if (m_animationElement->fromPropertyValueType() == CurrentColorValue)
-        adjustForCurrentColor(m_contextElement, fromColor);
+        fromColor = currentColor(*m_contextElement);
     if (m_animationElement->toPropertyValueType() == CurrentColorValue)
-        adjustForCurrentColor(m_contextElement, toColor);
+        toColor = currentColor(*m_contextElement);
+
+    auto& toAtEndOfDurationColor = toAtEndOfDuration->color();
+    auto& animatedColor = animated->color();
 
-    float animatedRed = animatedColor.red();
-    m_animationElement->animateAdditiveNumber(percentage, repeatCount, fromColor.red(), toColor.red(), toAtEndOfDurationColor.red(), animatedRed);
+    float red = animatedColor.red();
+    m_animationElement->animateAdditiveNumber(percentage, repeatCount, fromColor.red(), toColor.red(), toAtEndOfDurationColor.red(), red);
 
-    float animatedGreen = animatedColor.green();
-    m_animationElement->animateAdditiveNumber(percentage, repeatCount, fromColor.green(), toColor.green(), toAtEndOfDurationColor.green(), animatedGreen);
+    float green = animatedColor.green();
+    m_animationElement->animateAdditiveNumber(percentage, repeatCount, fromColor.green(), toColor.green(), toAtEndOfDurationColor.green(), green);
 
-    float animatedBlue = animatedColor.blue();
-    m_animationElement->animateAdditiveNumber(percentage, repeatCount, fromColor.blue(), toColor.blue(), toAtEndOfDurationColor.blue(), animatedBlue);
+    float blue = animatedColor.blue();
+    m_animationElement->animateAdditiveNumber(percentage, repeatCount, fromColor.blue(), toColor.blue(), toAtEndOfDurationColor.blue(), blue);
 
-    float animatedAlpha = animatedColor.alpha();
-    m_animationElement->animateAdditiveNumber(percentage, repeatCount, fromColor.alpha(), toColor.alpha(), toAtEndOfDurationColor.alpha(), animatedAlpha);
+    float alpha = animatedColor.alpha();
+    m_animationElement->animateAdditiveNumber(percentage, repeatCount, fromColor.alpha(), toColor.alpha(), toAtEndOfDurationColor.alpha(), alpha);
 
-    animatedColor = ColorDistance::clampColor(static_cast<int>(roundf(animatedRed)), static_cast<int>(roundf(animatedGreen)), static_cast<int>(roundf(animatedBlue)), static_cast<int>(roundf(animatedAlpha)));
+    animatedColor = { { roundAndClampColorChannel(red), roundAndClampColorChannel(green), roundAndClampColorChannel(blue), roundAndClampColorChannel(alpha) }, ColorSpaceSRGB };
 }
 
 float SVGAnimatedColorAnimator::calculateDistance(const String& fromString, const String& toString)
 {
-    ASSERT(m_contextElement);
     Color from = SVGColor::colorFromRGBColorString(fromString);
     if (!from.isValid())
         return -1;
     Color to = SVGColor::colorFromRGBColorString(toString);
     if (!to.isValid())
         return -1;
-    return ColorDistance(from, to).distance();
+    float red = from.red() - to.red();
+    float green = from.green() - to.green();
+    float blue = from.blue() - to.blue();
+    return sqrtf(red * red + green * green + blue * blue);
 }
 
 }
index 28962377299109f30b301cca51e87ec55953457c..f3e4b719783ff06d04c1bcded528dda4c0cd81f3 100644 (file)
 
 namespace WebCore {
 
-class SVGAnimationElement;
-
 class SVGAnimatedColorAnimator final : public SVGAnimatedTypeAnimator {
 public:
-    SVGAnimatedColorAnimator(SVGAnimationElement*, SVGElement*);
-
-    virtual std::unique_ptr<SVGAnimatedType> constructFromString(const String&) override;
-    virtual std::unique_ptr<SVGAnimatedType> startAnimValAnimation(const SVGElementAnimatedPropertyList&) override { return nullptr; }
-    virtual void stopAnimValAnimation(const SVGElementAnimatedPropertyList&) override { }
-    virtual void resetAnimValToBaseVal(const SVGElementAnimatedPropertyList&, SVGAnimatedType*) override { }
-    virtual void animValWillChange(const SVGElementAnimatedPropertyList&) override { }
-    virtual void animValDidChange(const SVGElementAnimatedPropertyList&) override { }
-
-    virtual void addAnimatedTypes(SVGAnimatedType*, SVGAnimatedType*) override;
-    virtual void calculateAnimatedValue(float percentage, unsigned repeatCount, SVGAnimatedType*, SVGAnimatedType*, SVGAnimatedType*, SVGAnimatedType*) override;
-    virtual float calculateDistance(const String& fromString, const String& toString) override;
+    SVGAnimatedColorAnimator(SVGAnimationElement&, SVGElement&);
+
+private:
+    std::unique_ptr<SVGAnimatedType> constructFromString(const String&) override;
+    std::unique_ptr<SVGAnimatedType> startAnimValAnimation(const SVGElementAnimatedPropertyList&) override { return nullptr; }
+    void stopAnimValAnimation(const SVGElementAnimatedPropertyList&) override { }
+    void resetAnimValToBaseVal(const SVGElementAnimatedPropertyList&, SVGAnimatedType*) override { }
+    void animValWillChange(const SVGElementAnimatedPropertyList&) override { }
+    void animValDidChange(const SVGElementAnimatedPropertyList&) override { }
+    void addAnimatedTypes(SVGAnimatedType*, SVGAnimatedType*) override;
+    void calculateAnimatedValue(float percentage, unsigned repeatCount, SVGAnimatedType*, SVGAnimatedType*, SVGAnimatedType*, SVGAnimatedType*) override;
+    float calculateDistance(const String& fromString, const String& toString) override;
 };
+
 } // namespace WebCore
 
 #endif
index ef86f29113923c3e9c729b9105857e36d281f57d..237197567cfcbff99bf59fc99568712620e89c1d 100644 (file)
@@ -275,7 +275,7 @@ bool SVGAnimatedType::setValueAsString(const QualifiedName& attrName, const Stri
     switch (m_type) {
     case AnimatedColor:
         ASSERT(m_data.color);
-        *m_data.color = value.isEmpty() ? Color() : SVGColor::colorFromRGBColorString(value);
+        *m_data.color = SVGColor::colorFromRGBColorString(value);
         break;
     case AnimatedLength: {
         ASSERT(m_data.length);
index b1bc742508d0060ca4e9dc35dc95cbd6345ae72b..ee5507280c666772ca834907dabcf317ae8edb0d 100644 (file)
@@ -55,7 +55,7 @@ public:
         case AnimatedBoolean:
             return std::make_unique<SVGAnimatedBooleanAnimator>(animationElement, contextElement);
         case AnimatedColor:
-            return std::make_unique<SVGAnimatedColorAnimator>(animationElement, contextElement);
+            return std::make_unique<SVGAnimatedColorAnimator>(*animationElement, *contextElement);
         case AnimatedEnumeration:
             return std::make_unique<SVGAnimatedEnumerationAnimator>(animationElement, contextElement);
         case AnimatedInteger:
index e3f768f284158d9bb6ddaf9f67ced1e9bc26a7ad..40dc4ad4bc6671d7a8a8da30fd827b27606c4b23 100644 (file)
@@ -47,7 +47,10 @@ Ref<RGBColor> SVGColor::rgbColor() const
 
 Color SVGColor::colorFromRGBColorString(const String& colorString)
 {
-    // FIXME: Rework css parser so it is more SVG aware.
+    // FIXME: Rename to parseSVGColor? There's already a parseSVGColor in the CSS parser. How is it different?
+    // FIXME: Rework CSS parser so it exactly matches what the SVG specification requires?
+    // FIXME: Move this out of the SVGColor class?
+    // FIXME: Is it really OK to do stripWhitespace here instead of stripLeadingAndTrailingHTMLSpaces?
     RGBA32 color;
     if (CSSParser::parseColor(color, colorString.stripWhiteSpace()))
         return color;
@@ -57,7 +60,7 @@ Color SVGColor::colorFromRGBColorString(const String& colorString)
 void SVGColor::setRGBColor(const String&, ExceptionCode& ec)
 {
     // The whole SVGColor interface is deprecated in SVG 1.1 (2nd edition).
-    // The setters are the most problematic part so we remove the support for those first.
+    // Since the setters are the most problematic part, we removed the support for those first.
     ec = NO_MODIFICATION_ALLOWED_ERR;
 }
 
@@ -83,7 +86,7 @@ String SVGColor::customCSSText() const
     case SVG_COLORTYPE_CURRENTCOLOR:
         if (m_color.isValid())
             return m_color.cssText();
-        return "currentColor";
+        return ASCIILiteral("currentColor");
     }
 
     ASSERT_NOT_REACHED();