[webkit-reviews] review granted: [Bug 214204] Reduce surface area of the ExtendedColor class to a bare minimum : [Attachment 404072] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 214204: Reduce surface area of the ExtendedColor class to a bare minimum
https://bugs.webkit.org/show_bug.cgi?id=214204

Attachment 404072: Patch

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




--- Comment #14 from Darin Adler <darin at apple.com> ---
Comment on attachment 404072
  --> https://bugs.webkit.org/attachment.cgi?id=404072
Patch

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

> Source/WebCore/platform/graphics/Color.h:340
> +    return functor(asInline());

Wonder what determines if we should call the function like this or use
std::invoke/std::forward. I am guessing std::invoke works on a slightly larger
variety of functors?

> Source/WebCore/platform/graphics/ColorSerialization.cpp:167
> +    return color.callOnUnderlyingType([] (auto c) {

Would have preferred a word rather than the letter "c".

> Source/WebCore/platform/graphics/ColorSerialization.cpp:174
> +    return color.callOnUnderlyingType([] (auto c) {

Ditto.

> Source/WebCore/platform/graphics/ColorSerialization.cpp:181
> +    return color.callOnUnderlyingType([] (auto c) {

Ditto.

> Source/WebCore/platform/graphics/ColorTypes.h:33
> +template<typename T> struct ComponentTraits;

No need for the "T" here.

> Source/WebCore/platform/graphics/ColorTypes.h:53
> +    static constexpr ColorSpace colorSpace = ColorSpace::SRGB;

I would do "constexpr auto" instead of "constexpr ColorSpace".