[webkit-reviews] review granted: [Bug 234798] Add resolved/unresolved color type accessors to require users to be explicit about whether they will handle missing/none color components : [Attachment 448202] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Sam Weinig <sam at webkit.org>'s
request for review:
Bug 234798: Add resolved/unresolved color type accessors to require users to be
explicit about whether they will handle missing/none color components
https://bugs.webkit.org/show_bug.cgi?id=234798

Attachment 448202: Patch

https://bugs.webkit.org/attachment.cgi?id=448202&action=review




--- Comment #10 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 448202
  --> https://bugs.webkit.org/attachment.cgi?id=448202
Patch

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

> Source/WebCore/ChangeLog:33
> +	   opertunity to replace all its uses with the generic
toColorTypeLossy<SRGBA<>>.

spelling "opertunity"

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:95
> +	   auto [r, g, b, a] =
bgColor.toColorTypeLossy<SRGBA<uint8_t>>().resolved();

There is a lot of this. Could we have a template-free shortcut for this
apparently common case with a sensible name that hints the proper use?

One should be able to write WebKit code without becoming a color space expert
or mindlessly copy-pasting.

> Source/WebCore/accessibility/atspi/AccessibilityObjectTextAtspi.cpp:827
> -		   auto [r, g, b, a] = bgColor.toSRGBALossy<uint8_t>();
> +		   auto [r, g, b, a] =
bgColor..toColorTypeLossy<SRGBA<uint8_t>>().resolved();

what's up with those double ..?

> Source/WebCore/accessibility/atspi/AccessibilityObjectTextAtspi.cpp:833
> +		   auto [r, g, b, a] =
fgColor..toColorTypeLossy<SRGBA<uint8_t>>().resolved();

here too

> Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:1534
> +    auto originColorAsSRGB =
originColor.toColorTypeLossy<SRGBA<float>>().resolved();

Similarly maybe this could have a shortcut?