Title: [207487] trunk/Source/WebCore
Revision
207487
Author
d...@apple.com
Date
2016-10-18 13:55:49 -0700 (Tue, 18 Oct 2016)

Log Message

Add preliminary support for extended colors to WebCore::Color
https://bugs.webkit.org/show_bug.cgi?id=162878
<rdar://problem/28596413>

Follow-up review comments from Darin Adler.

* html/canvas/CanvasGradient.cpp:
(WebCore::CanvasGradient::addColorStop): Use nullptr.
* platform/graphics/Color.cpp:
(WebCore::Color::Color): Explicitly zero before assigning the pointer.
* platform/graphics/Color.h: Add some comments about the failings of operator== and hash.
(WebCore::Color::Color): Add some static_asserts to the constructors. Move the empty and deleted values
to static constants.
(WebCore::Color::isHashTableDeletedValue):
(WebCore::Color::hash): Replacement for asUint64, which was only being used for a hash.
(WebCore::Color::asUint64): Deleted.
* platform/graphics/ColorHash.h: Use new hash functions. Use "using" instead of typedef.
(WTF::ColorHash::hash):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (207486 => 207487)


--- trunk/Source/WebCore/ChangeLog	2016-10-18 20:40:03 UTC (rev 207486)
+++ trunk/Source/WebCore/ChangeLog	2016-10-18 20:55:49 UTC (rev 207487)
@@ -1,3 +1,24 @@
+2016-10-18  Dean Jackson  <d...@apple.com>
+
+        Add preliminary support for extended colors to WebCore::Color
+        https://bugs.webkit.org/show_bug.cgi?id=162878
+        <rdar://problem/28596413>
+
+        Follow-up review comments from Darin Adler.
+
+        * html/canvas/CanvasGradient.cpp:
+        (WebCore::CanvasGradient::addColorStop): Use nullptr.
+        * platform/graphics/Color.cpp:
+        (WebCore::Color::Color): Explicitly zero before assigning the pointer.
+        * platform/graphics/Color.h: Add some comments about the failings of operator== and hash.
+        (WebCore::Color::Color): Add some static_asserts to the constructors. Move the empty and deleted values
+        to static constants.
+        (WebCore::Color::isHashTableDeletedValue):
+        (WebCore::Color::hash): Replacement for asUint64, which was only being used for a hash.
+        (WebCore::Color::asUint64): Deleted.
+        * platform/graphics/ColorHash.h: Use new hash functions. Use "using" instead of typedef.
+        (WTF::ColorHash::hash):
+
 2016-10-18  Ryosuke Niwa  <rn...@webkit.org>
 
         REGRESSION (r201471): Keyboard remains visible when swiping back on twitter.com

Modified: trunk/Source/WebCore/html/canvas/CanvasGradient.cpp (207486 => 207487)


--- trunk/Source/WebCore/html/canvas/CanvasGradient.cpp	2016-10-18 20:40:03 UTC (rev 207486)
+++ trunk/Source/WebCore/html/canvas/CanvasGradient.cpp	2016-10-18 20:55:49 UTC (rev 207487)
@@ -57,7 +57,7 @@
         return;
     }
 
-    Color color = parseColorOrCurrentColor(colorString, 0 /*canvas*/);
+    Color color = parseColorOrCurrentColor(colorString, nullptr /*canvas*/);
     if (!color.isValid()) {
 #if ENABLE(DASHBOARD_SUPPORT)
         if (!m_dashboardCompatibilityMode)

Modified: trunk/Source/WebCore/platform/graphics/Color.cpp (207486 => 207487)


--- trunk/Source/WebCore/platform/graphics/Color.cpp	2016-10-18 20:40:03 UTC (rev 207486)
+++ trunk/Source/WebCore/platform/graphics/Color.cpp	2016-10-18 20:55:49 UTC (rev 207487)
@@ -289,6 +289,9 @@
 
 Color::Color(float r, float g, float b, float a, ColorSpace colorSpace)
 {
+    // Zero the union, just in case a 32-bit system only assigns the
+    // top 32 bits when copying the extendedColor pointer below.
+    m_colorData.rgbaAndFlags = 0;
     auto extendedColorRef = ExtendedColor::create(r, g, b, a, colorSpace);
     m_colorData.extendedColor = &extendedColorRef.leakRef();
     ASSERT(isExtended());

Modified: trunk/Source/WebCore/platform/graphics/Color.h (207486 => 207487)


--- trunk/Source/WebCore/platform/graphics/Color.h	2016-10-18 20:40:03 UTC (rev 207486)
+++ trunk/Source/WebCore/platform/graphics/Color.h	2016-10-18 20:55:49 UTC (rev 207487)
@@ -32,6 +32,7 @@
 #include <cmath>
 #include <unicode/uchar.h>
 #include <wtf/Forward.h>
+#include <wtf/HashFunctions.h>
 #include <wtf/Optional.h>
 #include <wtf/text/LChar.h>
 
@@ -143,18 +144,23 @@
 
     explicit Color(WTF::HashTableDeletedValueType)
     {
-        m_colorData.rgbaAndFlags = 0xfffffffffffffffd;
+        static_assert(deletedHashValue & invalidRGBAColor, "Color's deleted hash value must not look like an ExtendedColor");
+        static_assert(!(deletedHashValue & validRGBAColorBit), "Color's deleted hash value must not look like a valid RGBA32 Color");
+        static_assert(deletedHashValue & (1 << 4), "Color's deleted hash value must have some bits set that an RGBA32 Color wouldn't have");
+        m_colorData.rgbaAndFlags = deletedHashValue;
         ASSERT(!isExtended());
     }
 
     bool isHashTableDeletedValue() const
     {
-        return m_colorData.rgbaAndFlags == 0xfffffffffffffffd;
+        return m_colorData.rgbaAndFlags == deletedHashValue;
     }
 
     explicit Color(WTF::HashTableEmptyValueType)
     {
-        m_colorData.rgbaAndFlags = 0xffffffffffffffb;
+        static_assert(emptyHashValue & invalidRGBAColor, "Color's empty hash value must not look like an ExtendedColor");
+        static_assert(emptyHashValue & (1 << 4), "Color's deleted hash value must have some bits set that an RGBA32 Color wouldn't have");
+        m_colorData.rgbaAndFlags = emptyHashValue;
         ASSERT(!isExtended());
     }
 
@@ -200,8 +206,11 @@
     int alpha() const { return alphaChannel(rgb()); }
     
     RGBA32 rgb() const { ASSERT(!isExtended()); return static_cast<RGBA32>(m_colorData.rgbaAndFlags >> 32); }
-    uint64_t asUint64() const { return m_colorData.rgbaAndFlags; }
 
+    // FIXME: Like operator==, this will give different values for ExtendedColors that
+    // should be identical, since the respective pointer will be different.
+    unsigned hash() const { return WTF::intHash(m_colorData.rgbaAndFlags); }
+
     WEBCORE_EXPORT void getRGBA(float& r, float& g, float& b, float& a) const;
     WEBCORE_EXPORT void getRGBA(double& r, double& g, double& b, double& a) const;
     WEBCORE_EXPORT void getHSL(double& h, double& s, double& l) const;
@@ -274,6 +283,9 @@
     static const uint64_t validRGBAColorBit = 0x2;
     static const uint64_t validRGBAColor = 0x3;
 
+    static const uint64_t deletedHashValue = 0xFFFFFFFFFFFFFFFD;
+    static const uint64_t emptyHashValue = 0xFFFFFFFFFFFFFFFB;
+
     WEBCORE_EXPORT void tagAsValid();
 
     union {
@@ -282,6 +294,8 @@
     } m_colorData;
 };
 
+// FIXME: These do not work for ExtendedColor because
+// they become just pointer comparison.
 bool operator==(const Color&, const Color&);
 bool operator!=(const Color&, const Color&);
 

Modified: trunk/Source/WebCore/platform/graphics/ColorHash.h (207486 => 207487)


--- trunk/Source/WebCore/platform/graphics/ColorHash.h	2016-10-18 20:40:03 UTC (rev 207486)
+++ trunk/Source/WebCore/platform/graphics/ColorHash.h	2016-10-18 20:55:49 UTC (rev 207487)
@@ -31,13 +31,13 @@
 namespace WTF {
 
 struct ColorHash {
-    static unsigned hash(const WebCore::Color& key) { return intHash(key.asUint64()); }
+    static unsigned hash(const WebCore::Color& key) { return key.hash(); }
     static bool equal(const WebCore::Color& a, const WebCore::Color& b) { return a == b; }
     static const bool safeToCompareToEmptyOrDeleted = true;
 };
 
 template<> struct DefaultHash<WebCore::Color> {
-    typedef ColorHash Hash;
+    using Hash = ColorHash;
 };
 
 template<> struct HashTraits<WebCore::Color> : GenericHashTraits<WebCore::Color> {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to