[webkit-reviews] review granted: [Bug 221443] Generalize color conversion code to reduce number of overloads required : [Attachment 419355] 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 221443: Generalize color conversion code to reduce number of overloads
required
https://bugs.webkit.org/show_bug.cgi?id=221443

Attachment 419355: Patch

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




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

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

> Source/WebCore/platform/graphics/ColorConversion.h:342
> +// FIXME: Reduce total number of matrix tranforms by contatentaing the
matrices

Typo in "concatenating"

> Source/WebCore/platform/graphics/ColorConversion.h:355
> +// Steps 2, 3, and 4 can be combined into one single matrix if we linearly
> +// concatented the three matrices. To do this, we will have to tag which
conversions
> +// are matrix based, expose the matrices, add support for constexpr
concatenting
> +// of ColorMatrix and find a way to merge the conversions.

Will we get slightly numerically different results when we concatenate matrices
instead of doing multiple transformations? (I love floating point.)

> Source/WebCore/rendering/RenderTheme.cpp:1422
> +    // FIXME: Consider using LCHA<float> rather than HSLA<float> for better
perceptual results.

Sure seems like a great idea.

> Source/WebCore/rendering/RenderTheme.cpp:1429
> +    // FIXME: Consider converting back to the initial underlying color type
rather than hardcoding SRGBA<float> here.

This one is not quite as obvious a choice. Clearly SRGBA might have
insufficient gamut to cover the color. Not perfectly clear what the benefits
are of preserving color space further through the pipeline. Better rendering?
Better performance? Avoiding unnecessarily limited gamut or loss of precision?