Title: [289194] trunk
Revision
289194
Author
wei...@apple.com
Date
2022-02-06 19:44:49 -0800 (Sun, 06 Feb 2022)

Log Message

Update serialization of rgb() functions with none components to latest spec
https://bugs.webkit.org/show_bug.cgi?id=236210

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Update tests for new serialization behavior.

* web-platform-tests/css/css-color/parsing/color-computed.html:
* web-platform-tests/css/css-color/parsing/color-valid.html:
* web-platform-tests/css/css-color/parsing/relative-color-computed.html:
* web-platform-tests/css/css-color/parsing/relative-color-valid.html:

Source/WebCore:

The CSS Color spec now states that rgb() with none components should serialize none
compoonents as 0. This means we can remove a bunch of special serialization code
and use the old code path once again.

* platform/graphics/ColorSerialization.cpp:
(WebCore::serializationForCSS):
(WebCore::decimalDigit):
(WebCore::fractionDigitsForFractionalAlphaValue):
(WebCore::legacyRGBComponent): Deleted.
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::StringTypeAdapter): Deleted.
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::length const): Deleted.
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::is8Bit const): Deleted.
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::writeTo const): Deleted.
(WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::buffer const): Deleted.

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (289193 => 289194)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2022-02-07 03:43:08 UTC (rev 289193)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2022-02-07 03:44:49 UTC (rev 289194)
@@ -1,3 +1,17 @@
+2022-02-06  Sam Weinig  <wei...@apple.com>
+
+        Update serialization of rgb() functions with none components to latest spec
+        https://bugs.webkit.org/show_bug.cgi?id=236210
+
+        Reviewed by Darin Adler.
+
+        Update tests for new serialization behavior.
+
+        * web-platform-tests/css/css-color/parsing/color-computed.html:
+        * web-platform-tests/css/css-color/parsing/color-valid.html:
+        * web-platform-tests/css/css-color/parsing/relative-color-computed.html:
+        * web-platform-tests/css/css-color/parsing/relative-color-valid.html:
+
 2022-02-06  Antoine Quint  <grao...@webkit.org>
 
         [css-logical] Animations should convert logical properties to their physical equivalents

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/color-computed.html (289193 => 289194)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/color-computed.html	2022-02-07 03:43:08 UTC (rev 289193)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/color-computed.html	2022-02-07 03:44:49 UTC (rev 289194)
@@ -38,23 +38,22 @@
 test_computed_value("color", "rgb(20, 10, 0, -10)", "rgba(20, 10, 0, 0)");
 test_computed_value("color", "rgb(100%, 200%, 300%)", "rgb(255, 255, 255)");
 
-// NOTE: Serialization of rgb() with 'none' components is still under discussion - https://github.com/w3c/csswg-drafts/issues/6959
-test_computed_value("color", "rgb(none none none)", "rgb(none none none)");
-test_computed_value("color", "rgb(none none none / none)", "rgba(none none none / none)");
-test_computed_value("color", "rgb(128 none none)", "rgb(128 none none)");
-test_computed_value("color", "rgb(128 none none / none)", "rgba(128 none none / none)");
-test_computed_value("color", "rgb(none none none / .5)", "rgba(none none none / 0.5)");
-test_computed_value("color", "rgb(20% none none)", "rgb(51 none none)");
-test_computed_value("color", "rgb(20% none none / none)", "rgba(51 none none / none)");
-test_computed_value("color", "rgb(none none none / 50%)", "rgba(none none none / 0.5)");
-test_computed_value("color", "rgba(none none none)", "rgb(none none none)");
-test_computed_value("color", "rgba(none none none / none)", "rgba(none none none / none)");
-test_computed_value("color", "rgba(128 none none)", "rgb(128 none none)");
-test_computed_value("color", "rgba(128 none none / none)", "rgba(128 none none / none)");
-test_computed_value("color", "rgba(none none none / .5)", "rgba(none none none / 0.5)");
-test_computed_value("color", "rgba(20% none none)", "rgb(51 none none)");
-test_computed_value("color", "rgba(20% none none / none)", "rgba(51 none none / none)");
-test_computed_value("color", "rgba(none none none / 50%)", "rgba(none none none / 0.5)");
+test_computed_value("color", "rgb(none none none)", "rgb(0, 0, 0)");
+test_computed_value("color", "rgb(none none none / none)", "rgba(0, 0, 0, 0)");
+test_computed_value("color", "rgb(128 none none)", "rgb(128, 0, 0)");
+test_computed_value("color", "rgb(128 none none / none)", "rgba(128, 0, 0, 0)");
+test_computed_value("color", "rgb(none none none / .5)", "rgba(0, 0, 0, 0.5)");
+test_computed_value("color", "rgb(20% none none)", "rgb(51, 0, 0)");
+test_computed_value("color", "rgb(20% none none / none)", "rgba(51, 0, 0, 0)");
+test_computed_value("color", "rgb(none none none / 50%)", "rgba(0, 0, 0, 0.5)");
+test_computed_value("color", "rgba(none none none)", "rgb(0, 0, 0)");
+test_computed_value("color", "rgba(none none none / none)", "rgba(0, 0, 0, 0)");
+test_computed_value("color", "rgba(128 none none)", "rgb(128, 0, 0)");
+test_computed_value("color", "rgba(128 none none / none)", "rgba(128, 0, 0, 0)");
+test_computed_value("color", "rgba(none none none / .5)", "rgba(0, 0, 0, 0.5)");
+test_computed_value("color", "rgba(20% none none)", "rgb(51, 0, 0)");
+test_computed_value("color", "rgba(20% none none / none)", "rgba(51, 0, 0, 0)");
+test_computed_value("color", "rgba(none none none / 50%)", "rgba(0, 0, 0, 0.5)");
 
 test_computed_value("color", "hsl(120 30% 50%)", "rgb(89, 166, 89)");
 test_computed_value("color", "hsl(120 30% 50% / 0.5)", "rgba(89, 166, 89, 0.5)");

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/color-valid.html (289193 => 289194)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/color-valid.html	2022-02-07 03:43:08 UTC (rev 289193)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/color-valid.html	2022-02-07 03:44:49 UTC (rev 289194)
@@ -29,23 +29,22 @@
 test_valid_value("color", "rgb(20, 10, 0, -10)", "rgba(20, 10, 0, 0)");
 test_valid_value("color", "rgb(100%, 200%, 300%)", "rgb(255, 255, 255)");
 
-// NOTE: Serialization of rgb() with 'none' components is still under discussion - https://github.com/w3c/csswg-drafts/issues/6959
-test_valid_value("color", "rgb(none none none)", "rgb(none none none)");
-test_valid_value("color", "rgb(none none none / none)", "rgba(none none none / none)");
-test_valid_value("color", "rgb(128 none none)", "rgb(128 none none)");
-test_valid_value("color", "rgb(128 none none / none)", "rgba(128 none none / none)");
-test_valid_value("color", "rgb(none none none / .5)", "rgba(none none none / 0.5)");
-test_valid_value("color", "rgb(20% none none)", "rgb(51 none none)");
-test_valid_value("color", "rgb(20% none none / none)", "rgba(51 none none / none)");
-test_valid_value("color", "rgb(none none none / 50%)", "rgba(none none none / 0.5)");
-test_valid_value("color", "rgba(none none none)", "rgb(none none none)");
-test_valid_value("color", "rgba(none none none / none)", "rgba(none none none / none)");
-test_valid_value("color", "rgba(128 none none)", "rgb(128 none none)");
-test_valid_value("color", "rgba(128 none none / none)", "rgba(128 none none / none)");
-test_valid_value("color", "rgba(none none none / .5)", "rgba(none none none / 0.5)");
-test_valid_value("color", "rgba(20% none none)", "rgb(51 none none)");
-test_valid_value("color", "rgba(20% none none / none)", "rgba(51 none none / none)");
-test_valid_value("color", "rgba(none none none / 50%)", "rgba(none none none / 0.5)");
+test_valid_value("color", "rgb(none none none)", "rgb(0, 0, 0)");
+test_valid_value("color", "rgb(none none none / none)", "rgba(0, 0, 0, 0)");
+test_valid_value("color", "rgb(128 none none)", "rgb(128, 0, 0)");
+test_valid_value("color", "rgb(128 none none / none)", "rgba(128, 0, 0, 0)");
+test_valid_value("color", "rgb(none none none / .5)", "rgba(0, 0, 0, 0.5)");
+test_valid_value("color", "rgb(20% none none)", "rgb(51, 0, 0)");
+test_valid_value("color", "rgb(20% none none / none)", "rgba(51, 0, 0, 0)");
+test_valid_value("color", "rgb(none none none / 50%)", "rgba(0, 0, 0, 0.5)");
+test_valid_value("color", "rgba(none none none)", "rgb(0, 0, 0)");
+test_valid_value("color", "rgba(none none none / none)", "rgba(0, 0, 0, 0)");
+test_valid_value("color", "rgba(128 none none)", "rgb(128, 0, 0)");
+test_valid_value("color", "rgba(128 none none / none)", "rgba(128, 0, 0, 0)");
+test_valid_value("color", "rgba(none none none / .5)", "rgba(0, 0, 0, 0.5)");
+test_valid_value("color", "rgba(20% none none)", "rgb(51, 0, 0)");
+test_valid_value("color", "rgba(20% none none / none)", "rgba(51, 0, 0, 0)");
+test_valid_value("color", "rgba(none none none / 50%)", "rgba(0, 0, 0, 0.5)");
 
 test_valid_value("color", "hsl(120 30% 50%)", "rgb(89, 166, 89)");
 test_valid_value("color", "hsl(120 30% 50% / 0.5)", "rgba(89, 166, 89, 0.5)");

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/relative-color-computed.html (289193 => 289194)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/relative-color-computed.html	2022-02-07 03:43:08 UTC (rev 289193)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/relative-color-computed.html	2022-02-07 03:44:49 UTC (rev 289194)
@@ -107,13 +107,13 @@
 
     // Testing with 'none'.
     // NOTE: Serialization of rgb() with 'none' components is still under discussion - https://github.com/w3c/csswg-drafts/issues/6959
-    test_computed_value(`color`, `rgb(from rebeccapurple none none none)`,                         `rgb(none none none)`);
-    test_computed_value(`color`, `rgb(from rebeccapurple none none none / none)`,                  `rgba(none none none / none)`);
-    test_computed_value(`color`, `rgb(from rebeccapurple r g none)`,                               `rgb(102 51 none)`);
-    test_computed_value(`color`, `rgb(from rebeccapurple r g none / alpha)`,                       `rgb(102 51 none)`);
-    test_computed_value(`color`, `rgb(from rebeccapurple r g b / none)`,                           `rgba(102 51 153 / none)`);
-    test_computed_value(`color`, `rgb(from rgb(20% 40% 60% / 80%) r g none / alpha)`,              `rgba(51 102 none / 0.8)`);
-    test_computed_value(`color`, `rgb(from rgb(20% 40% 60% / 80%) r g b / none)`,                  `rgba(51 102 153 / none)`);
+    test_computed_value(`color`, `rgb(from rebeccapurple none none none)`,                         `rgb(0, 0, 0)`);
+    test_computed_value(`color`, `rgb(from rebeccapurple none none none / none)`,                  `rgba(0, 0, 0, 0)`);
+    test_computed_value(`color`, `rgb(from rebeccapurple r g none)`,                               `rgb(102, 51, 0)`);
+    test_computed_value(`color`, `rgb(from rebeccapurple r g none / alpha)`,                       `rgb(102, 51, 0)`);
+    test_computed_value(`color`, `rgb(from rebeccapurple r g b / none)`,                           `rgba(102, 51, 153, 0)`);
+    test_computed_value(`color`, `rgb(from rgb(20% 40% 60% / 80%) r g none / alpha)`,              `rgba(51, 102, 0, 0.8)`);
+    test_computed_value(`color`, `rgb(from rgb(20% 40% 60% / 80%) r g b / none)`,                  `rgba(51, 102, 153, 0)`);
     // FIXME: Clarify with spec editors if 'none' should pass through to the constants.
     test_computed_value(`color`, `rgb(from rgb(none none none) r g b)`,                            `rgb(0, 0, 0)`);
     test_computed_value(`color`, `rgb(from rgb(none none none / none) r g b / alpha)`,             `rgba(0, 0, 0, 0)`);

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/relative-color-valid.html (289193 => 289194)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/relative-color-valid.html	2022-02-07 03:43:08 UTC (rev 289193)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-color/parsing/relative-color-valid.html	2022-02-07 03:44:49 UTC (rev 289194)
@@ -105,14 +105,13 @@
     test_valid_value(`color`, `rgb(from rgb(20%, 40%, 60%, 80%) calc(r) calc(g) calc(b) / calc(alpha))`, `rgba(51, 102, 153, 0.8)`);
 
     // Testing with 'none'.
-    // NOTE: Serialization of rgb() with 'none' components is still under discussion - https://github.com/w3c/csswg-drafts/issues/6959
-    test_valid_value(`color`, `rgb(from rebeccapurple none none none)`,                         `rgb(none none none)`);
-    test_valid_value(`color`, `rgb(from rebeccapurple none none none / none)`,                  `rgba(none none none / none)`);
-    test_valid_value(`color`, `rgb(from rebeccapurple r g none)`,                               `rgb(102 51 none)`);
-    test_valid_value(`color`, `rgb(from rebeccapurple r g none / alpha)`,                       `rgb(102 51 none)`);
-    test_valid_value(`color`, `rgb(from rebeccapurple r g b / none)`,                           `rgba(102 51 153 / none)`);
-    test_valid_value(`color`, `rgb(from rgb(20% 40% 60% / 80%) r g none / alpha)`,              `rgba(51 102 none / 0.8)`);
-    test_valid_value(`color`, `rgb(from rgb(20% 40% 60% / 80%) r g b / none)`,                  `rgba(51 102 153 / none)`);
+    test_valid_value(`color`, `rgb(from rebeccapurple none none none)`,                         `rgb(0, 0, 0)`);
+    test_valid_value(`color`, `rgb(from rebeccapurple none none none / none)`,                  `rgba(0, 0, 0, 0)`);
+    test_valid_value(`color`, `rgb(from rebeccapurple r g none)`,                               `rgb(102, 51, 0)`);
+    test_valid_value(`color`, `rgb(from rebeccapurple r g none / alpha)`,                       `rgb(102, 51, 0)`);
+    test_valid_value(`color`, `rgb(from rebeccapurple r g b / none)`,                           `rgba(102, 51, 153, 0)`);
+    test_valid_value(`color`, `rgb(from rgb(20% 40% 60% / 80%) r g none / alpha)`,              `rgba(51, 102, 0, 0.8)`);
+    test_valid_value(`color`, `rgb(from rgb(20% 40% 60% / 80%) r g b / none)`,                  `rgba(51, 102, 153, 0)`);
     // FIXME: Clarify with spec editors if 'none' should pass through to the constants.
     test_valid_value(`color`, `rgb(from rgb(none none none) r g b)`,                            `rgb(0, 0, 0)`);
     test_valid_value(`color`, `rgb(from rgb(none none none / none) r g b / alpha)`,             `rgba(0, 0, 0, 0)`);

Modified: trunk/Source/WebCore/ChangeLog (289193 => 289194)


--- trunk/Source/WebCore/ChangeLog	2022-02-07 03:43:08 UTC (rev 289193)
+++ trunk/Source/WebCore/ChangeLog	2022-02-07 03:44:49 UTC (rev 289194)
@@ -1,3 +1,25 @@
+2022-02-06  Sam Weinig  <wei...@apple.com>
+
+        Update serialization of rgb() functions with none components to latest spec
+        https://bugs.webkit.org/show_bug.cgi?id=236210
+
+        Reviewed by Darin Adler.
+
+        The CSS Color spec now states that rgb() with none components should serialize none
+        compoonents as 0. This means we can remove a bunch of special serialization code
+        and use the old code path once again.
+
+        * platform/graphics/ColorSerialization.cpp:
+        (WebCore::serializationForCSS):
+        (WebCore::decimalDigit):
+        (WebCore::fractionDigitsForFractionalAlphaValue):
+        (WebCore::legacyRGBComponent): Deleted.
+        (WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::StringTypeAdapter): Deleted.
+        (WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::length const): Deleted.
+        (WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::is8Bit const): Deleted.
+        (WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::writeTo const): Deleted.
+        (WTF::StringTypeAdapter<WebCore::LegacyRGBComponent>::buffer const): Deleted.
+
 2022-02-06  Alan Bujtas  <za...@apple.com>
 
         [LFC][IFC] LineBox should hold on to its logical rect

Modified: trunk/Source/WebCore/platform/graphics/ColorSerialization.cpp (289193 => 289194)


--- trunk/Source/WebCore/platform/graphics/ColorSerialization.cpp	2022-02-07 03:43:08 UTC (rev 289193)
+++ trunk/Source/WebCore/platform/graphics/ColorSerialization.cpp	2022-02-07 03:44:49 UTC (rev 289194)
@@ -35,15 +35,9 @@
 
 namespace WebCore {
 
-struct LegacyRGBComponent { float value; };
 struct NumericComponent { float value; };
 struct PercentageComponent { float value; };
 
-static LegacyRGBComponent legacyRGBComponent(float value)
-{
-    return { value };
-}
-
 static NumericComponent numericComponent(float value)
 {
     return { value };
@@ -59,31 +53,6 @@
 
 namespace WTF {
 
-template<> class StringTypeAdapter<WebCore::LegacyRGBComponent> {
-public:
-    StringTypeAdapter(WebCore::LegacyRGBComponent number)
-    {
-        if (std::isnan(number.value)) {
-            m_buffer = { 'n', 'o', 'n', 'e' };
-            m_length = 4;
-        } else {
-            auto valueAsByte = std::clamp(std::lround(number.value * 255.0f), 0l, 255l);
-            m_length = lengthOfIntegerAsString(valueAsByte);
-            writeIntegerToBuffer(valueAsByte, m_buffer.data());
-        }
-    }
-
-    unsigned length() const { return m_length; }
-    bool is8Bit() const { return true; }
-    template<typename CharacterType> void writeTo(CharacterType* destination) const { StringImpl::copyCharacters(destination, buffer(), m_length); }
-
-private:
-    const LChar* buffer() const { return reinterpret_cast<const LChar*>(&m_buffer[0]); }
-
-    std::array<char, 4> m_buffer;
-    unsigned m_length;
-};
-
 template<> class StringTypeAdapter<WebCore::NumericComponent> {
 public:
     StringTypeAdapter(WebCore::NumericComponent number)
@@ -606,42 +575,12 @@
 
 // MARK: SRGBA<float> overloads
 
-static char decimalDigit(unsigned number)
-{
-    ASSERT(number < 10);
-    return '0' + number;
-}
-
-static std::array<char, 4> fractionDigitsForFractionalAlphaValue(uint8_t alpha)
-{
-    ASSERT(alpha > 0);
-    ASSERT(alpha < 0xFF);
-    if (((alpha * 100 + 0x7F) / 0xFF * 0xFF + 50) / 100 != alpha)
-        return { { decimalDigit(alpha * 10 / 0xFF % 10), decimalDigit(alpha * 100 / 0xFF % 10), decimalDigit((alpha * 1000 + 0x7F) / 0xFF % 10), '\0' } };
-    if (int thirdDigit = (alpha * 100 + 0x7F) / 0xFF % 10)
-        return { { decimalDigit(alpha * 10 / 0xFF), decimalDigit(thirdDigit), '\0', '\0' } };
-    return { { decimalDigit((alpha * 10 + 0x7F) / 0xFF), '\0', '\0', '\0' } };
-}
-
 String serializationForCSS(const SRGBA<float>& color, bool useColorFunctionSerialization)
 {
     if (useColorFunctionSerialization)
         return serializationUsingColorFunction(color);
 
-    auto [red, green, blue, alpha] = color.unresolved();
-
-    if (std::isnan(alpha))
-        return makeString("rgba(", legacyRGBComponent(red), ' ', legacyRGBComponent(green), ' ', legacyRGBComponent(blue), " / none)");
-
-    auto normalizedAlpha = convertFloatAlphaTo<uint8_t>(alpha);
-    switch (normalizedAlpha) {
-    case 0:
-        return makeString("rgba(", legacyRGBComponent(red), ' ', legacyRGBComponent(green), ' ', legacyRGBComponent(blue), " / 0)");
-    case 0xFF:
-        return makeString("rgb(", legacyRGBComponent(red), ' ', legacyRGBComponent(green), ' ', legacyRGBComponent(blue), ')');
-    default:
-        return makeString("rgba(", legacyRGBComponent(red), ' ', legacyRGBComponent(green), ' ', legacyRGBComponent(blue), " / 0.", fractionDigitsForFractionalAlphaValue(normalizedAlpha).data(), ')');
-    }
+    return serializationForCSS(convertColor<SRGBA<uint8_t>>(color), useColorFunctionSerialization);
 }
 
 String serializationForHTML(const SRGBA<float>& color, bool useColorFunctionSerialization)
@@ -656,6 +595,23 @@
 
 // MARK: SRGBA<uint8_t> overloads
 
+static char decimalDigit(unsigned number)
+{
+    ASSERT(number < 10);
+    return '0' + number;
+}
+
+static std::array<char, 4> fractionDigitsForFractionalAlphaValue(uint8_t alpha)
+{
+    ASSERT(alpha > 0);
+    ASSERT(alpha < 0xFF);
+    if (((alpha * 100 + 0x7F) / 0xFF * 0xFF + 50) / 100 != alpha)
+        return { { decimalDigit(alpha * 10 / 0xFF % 10), decimalDigit(alpha * 100 / 0xFF % 10), decimalDigit((alpha * 1000 + 0x7F) / 0xFF % 10), '\0' } };
+    if (int thirdDigit = (alpha * 100 + 0x7F) / 0xFF % 10)
+        return { { decimalDigit(alpha * 10 / 0xFF), decimalDigit(thirdDigit), '\0', '\0' } };
+    return { { decimalDigit((alpha * 10 + 0x7F) / 0xFF), '\0', '\0', '\0' } };
+}
+
 String serializationForCSS(SRGBA<uint8_t> color, bool useColorFunctionSerialization)
 {
     if (useColorFunctionSerialization)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to