Add support for "relative color syntax" for color()
Created attachment 430296 [details] Patch
Created attachment 430312 [details] Patch
Comment on attachment 430312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=430312&action=review > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1566 > + ASSERT(range.peek().functionId() == CSSValueRgb || range.peek().functionId() == CSSValueRgba); Wouldn’t we want the more precise assertion? ASSERT(range.peek().functionId() == (Mode == RGBFunctionMode::RGB ? CSSValueRgb : CSSValueRgba)); Or is my assertion wrong? > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1583 > + return convertColor<SRGBA<uint8_t>>(HSLA<float> { static_cast<float>(normalizedHue), static_cast<float>(normalizedSaturation), static_cast<float>(normalizedLightness), static_cast<float>(normalizedAlpha) }); This explicit conversion to SRGBA really does need a comment. I know it’s not new. The fact that we do not create an extended color, but instead round and clamp to SRGBA intentionally really does not go without saying! > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1659 > + ASSERT(range.peek().functionId() == CSSValueHsl || range.peek().functionId() == CSSValueHsla); Same thought about the assertion. > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1720 > + return convertColor<SRGBA<uint8_t>>(HWBA<float> { static_cast<float>(normalizedHue), static_cast<float>(normalizedWhitness), static_cast<float>(normalizedBlackness), static_cast<float>(*alpha) }); Same thought about the comment. > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1929 > + for (int i = 0; i < 3; ++i) { for (auto& channel : channels) { > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1932 > + if (!value) > + break; All unspecified channels are set to 0? Peculiar syntax!
Created attachment 430356 [details] Patch
Created attachment 430362 [details] Patch
(In reply to Darin Adler from comment #3) > Comment on attachment 430312 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=430312&action=review > > > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1566 > > + ASSERT(range.peek().functionId() == CSSValueRgb || range.peek().functionId() == CSSValueRgba); > > Wouldn’t we want the more precise assertion? > > ASSERT(range.peek().functionId() == (Mode == RGBFunctionMode::RGB ? > CSSValueRgb : CSSValueRgba)); > > Or is my assertion wrong? Nope. Great idea. The template parameter and the assertion were added at different times and I didn't put them together. Done. > > > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1583 > > + return convertColor<SRGBA<uint8_t>>(HSLA<float> { static_cast<float>(normalizedHue), static_cast<float>(normalizedSaturation), static_cast<float>(normalizedLightness), static_cast<float>(normalizedAlpha) }); > > This explicit conversion to SRGBA really does need a comment. I know it’s > not new. The fact that we do not create an extended color, but instead round > and clamp to SRGBA intentionally really does not go without saying! Totally agree. Added. > > > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1659 > > + ASSERT(range.peek().functionId() == CSSValueHsl || range.peek().functionId() == CSSValueHsla); > > Same thought about the assertion. Done. > > > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1720 > > + return convertColor<SRGBA<uint8_t>>(HWBA<float> { static_cast<float>(normalizedHue), static_cast<float>(normalizedWhitness), static_cast<float>(normalizedBlackness), static_cast<float>(*alpha) }); > > Same thought about the comment. Done. > > > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1929 > > + for (int i = 0; i < 3; ++i) { > > for (auto& channel : channels) { Done. > > > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1932 > > + if (!value) > > + break; > > All unspecified channels are set to 0? Peculiar syntax! So odd. It has to do with the fact that a custom profile can have any number of components, so it just defaults everything to 0, but I think they should just special case the predefined ones to require the right number of components, but I think I lost that argument.
Created attachment 430363 [details] Patch
Committed r278364 (238394@main): <https://commits.webkit.org/238394@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430363 [details].
<rdar://problem/78771441>