Bug 221352 - Add support for hwb() colors defined in CSS Color 4
Summary: Add support for hwb() colors defined in CSS Color 4
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-03 12:43 PST by Sam Weinig
Modified: 2021-03-18 11:04 PDT (History)
11 users (show)

See Also:


Attachments
Patch (589.23 KB, patch)
2021-02-03 12:50 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (589.26 KB, patch)
2021-02-03 12:53 PST, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2021-02-03 12:43:53 PST
We should add support for hwb() colors defined in CSS Color 4 - https://drafts.csswg.org/css-color/#the-hwb-notation.
Comment 1 Sam Weinig 2021-02-03 12:50:36 PST Comment hidden (obsolete)
Comment 2 EWS Watchlist 2021-02-03 12:51:44 PST
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 3 Sam Weinig 2021-02-03 12:53:58 PST
Created attachment 419175 [details]
Patch
Comment 4 Sam Weinig 2021-02-03 14:44:12 PST
Don't be put off by the big patch size, most is the output of a wpt case.
Comment 5 Darin Adler 2021-02-03 15:13:09 PST
Comment on attachment 419175 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=419175&action=review

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:835
> +    CSSParserTokenRange args = consumeFunction(range);

auto?

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:888
> +    return convertTo<SRGBA<uint8_t>>(toSRGBA(HWBA<float> { static_cast<float>(normalizedHue), static_cast<float>(nomalizedWhiteness), static_cast<float>(nomalizedBlackness), static_cast<float>(*alpha) }));

I’m a little surprised a this:

    convertTo<SRGBA<uint8_t>>(toSRGBA(x))

I understand that we have to specify "uint8_t"; my surprise is that convertTo does not implicitly include the toSRGBA work.
Comment 6 EWS 2021-02-03 15:37:21 PST
Committed r272344: <https://trac.webkit.org/changeset/272344>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419175 [details].
Comment 7 Radar WebKit Bug Importer 2021-02-03 15:38:14 PST
<rdar://problem/73952934>
Comment 8 Sam Weinig 2021-02-03 16:21:47 PST
(In reply to Darin Adler from comment #5)
> Comment on attachment 419175 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=419175&action=review
> 
> 
> > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:888
> > +    return convertTo<SRGBA<uint8_t>>(toSRGBA(HWBA<float> { static_cast<float>(normalizedHue), static_cast<float>(nomalizedWhiteness), static_cast<float>(nomalizedBlackness), static_cast<float>(*alpha) }));
> 
> I’m a little surprised a this:
> 
>     convertTo<SRGBA<uint8_t>>(toSRGBA(x))
> 
> I understand that we have to specify "uint8_t"; my surprise is that
> convertTo does not implicitly include the toSRGBA work.

This is due to color space conversion (the toSRGBA()) and component type conversion (the convertTo<SRGBA<uint8_t>>()) being separate currently.

My plan is to make them into one thing, tentatively using the convertTo<> naming.
Comment 9 Sam Weinig 2021-02-04 20:37:31 PST
(In reply to Sam Weinig from comment #8)
> (In reply to Darin Adler from comment #5)
> > Comment on attachment 419175 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=419175&action=review
> > 
> > 
> > > Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:888
> > > +    return convertTo<SRGBA<uint8_t>>(toSRGBA(HWBA<float> { static_cast<float>(normalizedHue), static_cast<float>(nomalizedWhiteness), static_cast<float>(nomalizedBlackness), static_cast<float>(*alpha) }));
> > 
> > I’m a little surprised a this:
> > 
> >     convertTo<SRGBA<uint8_t>>(toSRGBA(x))
> > 
> > I understand that we have to specify "uint8_t"; my surprise is that
> > convertTo does not implicitly include the toSRGBA work.
> 
> This is due to color space conversion (the toSRGBA()) and component type
> conversion (the convertTo<SRGBA<uint8_t>>()) being separate currently.
> 
> My plan is to make them into one thing, tentatively using the convertTo<>
> naming.

This is resolved in https://bugs.webkit.org/show_bug.cgi?id=221443.