Bug 214514 – Additional Color related cleanups

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Additional Color related cleanups

[Sam Weinig]

Created attachment 404634 [details]
Patch

[Darin Adler]

Comment on attachment 404634 [details]
Patch

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

> Source/WebCore/platform/graphics/ColorUtilities.h:97
> +    return std::clamp(component, static_cast<ComponentType>(0), static_cast<ComponentType>(255));

Could this be written this way instead?

    return std::clamp<ComponentType>(component, 0, 255);

Are there disadvantages to writing it this way?

> Source/WebCore/platform/graphics/ColorUtilities.h:102
> +    return std::clamp(component, static_cast<ComponentType>(0.0), static_cast<ComponentType>(1.0));

Could this be written this way instead?

    return std::clamp<ComponentType>(component, 0, 1);

Are there disadvantages to writing it this way?

> Source/WebCore/platform/graphics/ColorUtilities.h:153
> +    copy.red = std::invoke(functor, color.red);
> +    copy.green = std::invoke(functor, color.green);
> +    copy.blue = std::invoke(functor, color.blue);

No forwarding, not even on the third call? I wish I understood universal references better.

[Sam Weinig]

(In reply to Darin Adler from comment #2)
> Comment on attachment 404634 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=404634&action=review
> 
> > Source/WebCore/platform/graphics/ColorUtilities.h:97
> > +    return std::clamp(component, static_cast<ComponentType>(0), static_cast<ComponentType>(255));
> 
> Could this be written this way instead?
> 
>     return std::clamp<ComponentType>(component, 0, 255);
> 
> Are there disadvantages to writing it this way?
> 
> > Source/WebCore/platform/graphics/ColorUtilities.h:102
> > +    return std::clamp(component, static_cast<ComponentType>(0.0), static_cast<ComponentType>(1.0));
> 
> Could this be written this way instead?
> 
>     return std::clamp<ComponentType>(component, 0, 1);
> 
> Are there disadvantages to writing it this way?

You know, I am not sure. It just didn't occur to me. Seems like your proposal is better, and the literals should convert cleanly.

> 
> > Source/WebCore/platform/graphics/ColorUtilities.h:153
> > +    copy.red = std::invoke(functor, color.red);
> > +    copy.green = std::invoke(functor, color.green);
> > +    copy.blue = std::invoke(functor, color.blue);
> 
> No forwarding, not even on the third call? I wish I understood universal
> references better.

Seems like I could forward it to the last one. Will change.

Thanks for all the reviews.

[Sam Weinig]

Created attachment 404674 [details]
Patch

[EWS]

Committed r264577: <https://trac.webkit.org/changeset/264577>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404674 [details].

[Radar WebKit Bug Importer]

<rdar://problem/65792738>