From 5b3d8d3410bc42d11559976ebfdddc81969ae81f Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 23 Jan 2025 16:19:48 -0800 Subject: [PATCH] More spec compliant rgb function parsing (#48839) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/48839 In the last diff I mixed and matched `` and `` a bit to keep compatiblity with `normalze-color`. Spec noncompliant values have only been allowed since https://github.com/facebook/react-native/pull/34600 with the main issue being that legacy syntax rgb functions are allowed to use the `/` based alpha syntax, and commas can be mixed with whitespace. This seems like an exceedingly rare real-world scenario (there are currently zero usages of slash syntax in RKJSModules validated by `rgb\([^\)]*/`), so I'm going to instead just follow the spec for more sanity. Another bit that I missed was that modern RGB functions allow individual components to be `` or `` compared to legacy functions which only allow the full function to accept one or the other (`normalize-color` doesn't support `` at all), so I fixed that as well. I started sharing a little bit more of the logic here, to make things more readable when adding more functions. Changelog: [Internal] Reviewed By: javache Differential Revision: D68468275 fbshipit-source-id: f1dfab51b91a3f64436c2559daa3d1e8891db889 --- .../react/renderer/css/CSSColorFunction.h | 200 +++++++++++++----- .../react/renderer/css/CSSSyntaxParser.h | 9 +- .../react/renderer/css/tests/CSSColorTest.cpp | 35 ++- .../css/tests/CSSSyntaxParserTest.cpp | 15 +- 4 files changed, 178 insertions(+), 81 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSColorFunction.h b/packages/react-native/ReactCommon/react/renderer/css/CSSColorFunction.h index 4a6e2c75c5c..82af245a6a9 100644 --- a/packages/react-native/ReactCommon/react/renderer/css/CSSColorFunction.h +++ b/packages/react-native/ReactCommon/react/renderer/css/CSSColorFunction.h @@ -17,6 +17,7 @@ #include #include #include +#include #include namespace facebook::react { @@ -33,71 +34,156 @@ constexpr uint8_t clamp255Component(float f) { return static_cast(std::clamp(ceiled, 0, 255)); } +constexpr std::optional normalizeNumberComponent( + const std::variant& component) { + if (std::holds_alternative(component)) { + return std::get(component).value; + } + + return {}; +} + +template + requires( + (std::is_same_v || + std::is_same_v) && + ...) +constexpr std::optional normalizeComponent( + const std::variant& component, + float baseValue) { + if constexpr (traits::containsType()) { + if (std::holds_alternative(component)) { + return std::get(component).value / 100.0f * baseValue; + } + } + + if constexpr (traits::containsType()) { + if (std::holds_alternative(component)) { + return std::get(component).value; + } + } + + return {}; +} + +template +constexpr bool isLegacyColorFunction(CSSSyntaxParser& parser) { + auto lookahead = parser; + auto next = parseNextCSSValue(lookahead); + if (std::holds_alternative(next)) { + return false; + } + + return lookahead.consumeComponentValue( + CSSDelimiter::OptionalWhitespace, [](CSSPreservedToken token) { + return token.type() == CSSTokenType::Comma; + }); +} + +/** + * Parses a legacy syntax rgb() or rgba() function and returns a CSSColor if it + * is valid. + * https://www.w3.org/TR/css-color-4/#typedef-legacy-rgb-syntax + */ +template +constexpr std::optional parseLegacyRgbFunction( + CSSSyntaxParser& parser) { + auto rawRed = parseNextCSSValue(parser); + bool usesNumber = std::holds_alternative(rawRed); + + auto red = normalizeComponent(rawRed, 255.0f); + if (!red.has_value()) { + return {}; + } + + auto green = usesNumber + ? normalizeNumberComponent( + parseNextCSSValue(parser, CSSDelimiter::Comma)) + : normalizeComponent( + parseNextCSSValue(parser, CSSDelimiter::Comma), + 255.0f); + if (!green.has_value()) { + return {}; + } + + auto blue = usesNumber + ? normalizeNumberComponent( + parseNextCSSValue(parser, CSSDelimiter::Comma)) + : normalizeComponent( + parseNextCSSValue(parser, CSSDelimiter::Comma), + 255.0f); + if (!blue.has_value()) { + return {}; + } + + auto alpha = normalizeComponent( + parseNextCSSValue(parser, CSSDelimiter::Comma), + 1.0f); + + return CSSColor{ + .r = clamp255Component(*red), + .g = clamp255Component(*green), + .b = clamp255Component(*blue), + .a = alpha.has_value() ? clamp255Component(*alpha * 255.0f) + : static_cast(255u), + }; +} + +/** + * Parses a modern syntax rgb() or rgba() function and returns a CSSColor if it + * is valid. + * https://www.w3.org/TR/css-color-4/#typedef-modern-rgb-syntax + */ +template +constexpr std::optional parseModernRgbFunction( + CSSSyntaxParser& parser) { + auto red = normalizeComponent( + parseNextCSSValue(parser), 255.0f); + if (!red.has_value()) { + return {}; + } + + auto green = normalizeComponent( + parseNextCSSValue( + parser, CSSDelimiter::Whitespace), + 255.0f); + if (!green.has_value()) { + return {}; + } + + auto blue = normalizeComponent( + parseNextCSSValue( + parser, CSSDelimiter::Whitespace), + 255.0f); + if (!blue.has_value()) { + return {}; + } + + auto alpha = normalizeComponent( + parseNextCSSValue( + parser, CSSDelimiter::SolidusOrWhitespace), + 1.0f); + + return CSSColor{ + .r = clamp255Component(*red), + .g = clamp255Component(*green), + .b = clamp255Component(*blue), + .a = alpha.has_value() ? clamp255Component(*alpha * 255.0f) + : static_cast(255u), + }; +} + /** * Parses an rgb() or rgba() function and returns a CSSColor if it is valid. - * Some invalid syntax (like mixing commas and whitespace) are allowed for - * backwards compatibility with normalize-color. * https://www.w3.org/TR/css-color-4/#funcdef-rgb */ template constexpr std::optional parseRgbFunction(CSSSyntaxParser& parser) { - auto firstValue = parseNextCSSValue(parser); - if (std::holds_alternative(firstValue)) { - return {}; - } - - float redNumber = 0; - float greenNumber = 0; - float blueNumber = 0; - - if (std::holds_alternative(firstValue)) { - redNumber = std::get(firstValue).value; - - auto green = - parseNextCSSValue(parser, CSSDelimiter::CommaOrWhitespace); - if (!std::holds_alternative(green)) { - return {}; - } - greenNumber = std::get(green).value; - - auto blue = - parseNextCSSValue(parser, CSSDelimiter::CommaOrWhitespace); - if (!std::holds_alternative(blue)) { - return {}; - } - blueNumber = std::get(blue).value; + if (isLegacyColorFunction(parser)) { + return parseLegacyRgbFunction(parser); } else { - redNumber = std::get(firstValue).value * 2.55f; - - auto green = parseNextCSSValue( - parser, CSSDelimiter::CommaOrWhitespace); - if (!std::holds_alternative(green)) { - return {}; - } - greenNumber = std::get(green).value * 2.55f; - - auto blue = parseNextCSSValue( - parser, CSSDelimiter::CommaOrWhitespace); - if (!std::holds_alternative(blue)) { - return {}; - } - blueNumber = std::get(blue).value * 2.55f; + return parseModernRgbFunction(parser); } - - auto alphaValue = parseNextCSSValue( - parser, CSSDelimiter::CommaOrWhitespaceOrSolidus); - - float alphaNumber = std::holds_alternative(alphaValue) ? 1.0f - : std::holds_alternative(alphaValue) - ? std::get(alphaValue).value - : std::get(alphaValue).value / 100.0f; - - return CSSColor{ - .r = clamp255Component(redNumber), - .g = clamp255Component(greenNumber), - .b = clamp255Component(blueNumber), - .a = clamp255Component(alphaNumber * 255.0f), - }; } } // namespace detail diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h b/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h index 3f4160a0c84..5eea737d7fd 100644 --- a/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h +++ b/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h @@ -93,9 +93,9 @@ enum class CSSDelimiter { Whitespace, OptionalWhitespace, Solidus, + SolidusOrWhitespace, Comma, CommaOrWhitespace, - CommaOrWhitespaceOrSolidus, None, }; @@ -313,10 +313,9 @@ struct CSSComponentValueVisitorDispatcher { return true; } return false; - case CSSDelimiter::CommaOrWhitespaceOrSolidus: - if (parser.peek().type() == CSSTokenType::Comma || - (parser.peek().type() == CSSTokenType::Delim && - parser.peek().stringValue() == "/")) { + case CSSDelimiter::SolidusOrWhitespace: + if (parser.peek().type() == CSSTokenType::Delim && + parser.peek().stringValue() == "/") { parser.consumeToken(); parser.consumeWhitespace(); return true; diff --git a/packages/react-native/ReactCommon/react/renderer/css/tests/CSSColorTest.cpp b/packages/react-native/ReactCommon/react/renderer/css/tests/CSSColorTest.cpp index 3935b400669..ce928caadc0 100644 --- a/packages/react-native/ReactCommon/react/renderer/css/tests/CSSColorTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/css/tests/CSSColorTest.cpp @@ -122,13 +122,9 @@ TEST(CSSColor, rgb_rgba_values) { EXPECT_EQ(std::get(modernSyntaxValue).a, 255); auto mixedDelimeterValue = parseCSSProperty("rgb(255,255 255)"); - EXPECT_TRUE(std::holds_alternative(mixedDelimeterValue)); - EXPECT_EQ(std::get(mixedDelimeterValue).r, 255); - EXPECT_EQ(std::get(mixedDelimeterValue).g, 255); - EXPECT_EQ(std::get(mixedDelimeterValue).b, 255); - EXPECT_EQ(std::get(mixedDelimeterValue).a, 255); + EXPECT_TRUE(std::holds_alternative(mixedDelimeterValue)); - auto mixedSpacingValue = parseCSSProperty("rgb( 5 4,3)"); + auto mixedSpacingValue = parseCSSProperty("rgb( 5 4 3)"); EXPECT_TRUE(std::holds_alternative(mixedSpacingValue)); EXPECT_EQ(std::get(mixedSpacingValue).r, 5); EXPECT_EQ(std::get(mixedSpacingValue).g, 4); @@ -155,10 +151,19 @@ TEST(CSSColor, rgb_rgba_values) { EXPECT_EQ(std::get(percentageValue).g, 128); EXPECT_EQ(std::get(percentageValue).b, 128); - auto mixedNumberPercentageValue = + auto mixedLegacyNumberPercentageValue = parseCSSProperty("rgb(50%, 0.5, 50%)"); EXPECT_TRUE( - std::holds_alternative(mixedNumberPercentageValue)); + std::holds_alternative(mixedLegacyNumberPercentageValue)); + + auto mixedModernNumberPercentageValue = + parseCSSProperty("rgb(50% 0.5 50%)"); + EXPECT_TRUE( + std::holds_alternative(mixedModernNumberPercentageValue)); + EXPECT_EQ(std::get(mixedModernNumberPercentageValue).r, 128); + EXPECT_EQ(std::get(mixedModernNumberPercentageValue).g, 1); + EXPECT_EQ(std::get(mixedModernNumberPercentageValue).b, 128); + EXPECT_EQ(std::get(mixedModernNumberPercentageValue).a, 255); auto rgbWithNumberAlphaValue = parseCSSProperty("rgb(255 255 255 0.5)"); @@ -169,7 +174,7 @@ TEST(CSSColor, rgb_rgba_values) { EXPECT_EQ(std::get(rgbWithNumberAlphaValue).a, 128); auto rgbWithPercentageAlphaValue = - parseCSSProperty("rgb(255 255 255, 50%)"); + parseCSSProperty("rgb(255 255 255 50%)"); EXPECT_TRUE(std::holds_alternative(rgbWithPercentageAlphaValue)); EXPECT_EQ(std::get(rgbWithPercentageAlphaValue).r, 255); EXPECT_EQ(std::get(rgbWithPercentageAlphaValue).g, 255); @@ -184,6 +189,11 @@ TEST(CSSColor, rgb_rgba_values) { EXPECT_EQ(std::get(rgbWithSolidusAlphaValue).b, 255); EXPECT_EQ(std::get(rgbWithSolidusAlphaValue).a, 128); + auto rgbLegacySyntaxWithSolidusAlphaValue = + parseCSSProperty("rgb(1, 4, 5 /0.5)"); + EXPECT_TRUE(std::holds_alternative( + rgbLegacySyntaxWithSolidusAlphaValue)); + auto rgbaWithSolidusAlphaValue = parseCSSProperty("rgba(255 255 255 / 0.5)"); EXPECT_TRUE(std::holds_alternative(rgbaWithSolidusAlphaValue)); @@ -236,7 +246,12 @@ TEST(CSSColor, rgb_rgba_values) { } TEST(CSSColor, constexpr_values) { - [[maybe_unused]] constexpr auto simpleValue = + [[maybe_unused]] constexpr auto emptyValue = parseCSSProperty(""); + + [[maybe_unused]] constexpr auto hexColorValue = + parseCSSProperty("#fff"); + + [[maybe_unused]] constexpr auto rgbFunctionValue = parseCSSProperty("rgb(255, 255, 255)"); } diff --git a/packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp b/packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp index dddccd1f09d..9dc8aa6721a 100644 --- a/packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp @@ -533,8 +533,8 @@ TEST(CSSSyntaxParser, required_whitespace_not_present) { EXPECT_EQ(delimValue2, "/"); } -TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) { - CSSSyntaxParser parser{"foo, bar / baz potato%"}; +TEST(CSSSyntaxParser, solidus_or_whitespace) { + CSSSyntaxParser parser{"foo bar / baz potato, papaya"}; auto identValue1 = parser.consumeComponentValue( CSSDelimiter::OptionalWhitespace, [](const CSSPreservedToken& token) { @@ -546,8 +546,7 @@ TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) { EXPECT_EQ(identValue1, "foo"); auto identValue2 = parser.consumeComponentValue( - CSSDelimiter::CommaOrWhitespaceOrSolidus, - [](const CSSPreservedToken& token) { + CSSDelimiter::SolidusOrWhitespace, [](const CSSPreservedToken& token) { EXPECT_EQ(token.type(), CSSTokenType::Ident); EXPECT_EQ(token.stringValue(), "bar"); return token.stringValue(); @@ -556,8 +555,7 @@ TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) { EXPECT_EQ(identValue2, "bar"); auto identValue3 = parser.consumeComponentValue( - CSSDelimiter::CommaOrWhitespaceOrSolidus, - [](const CSSPreservedToken& token) { + CSSDelimiter::SolidusOrWhitespace, [](const CSSPreservedToken& token) { EXPECT_EQ(token.type(), CSSTokenType::Ident); EXPECT_EQ(token.stringValue(), "baz"); return token.stringValue(); @@ -566,8 +564,7 @@ TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) { EXPECT_EQ(identValue3, "baz"); auto identValue4 = parser.consumeComponentValue( - CSSDelimiter::CommaOrWhitespaceOrSolidus, - [](const CSSPreservedToken& token) { + CSSDelimiter::SolidusOrWhitespace, [](const CSSPreservedToken& token) { EXPECT_EQ(token.type(), CSSTokenType::Ident); EXPECT_EQ(token.stringValue(), "potato"); return token.stringValue(); @@ -576,7 +573,7 @@ TEST(CSSSyntaxParser, comma_or_whitespace_or_solidus) { EXPECT_EQ(identValue4, "potato"); auto delimValue1 = parser.consumeComponentValue( - CSSDelimiter::CommaOrWhitespaceOrSolidus, + CSSDelimiter::SolidusOrWhitespace, [](const CSSPreservedToken& token) { return true; }); EXPECT_FALSE(delimValue1);