Bug 221868 – Reduce requirements for color types to only conversion to their reference color

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Sam Weinig]

We currently require every new color type to define a conversion to/from xyz and then perhaps some more conversions to reference colors to avoid unnecessary trips through XYZ. For example, if every conversion from HSL to SRGB went through XYZ it would be very silly and inefficient, and the fact that we require the boiler plate for HSL -> XYZ, even though it's always going to convert to SRGB first seems unfortunate. We can do better.

At first I thought we could model color conversion as DAG, with XYZ at the root, and each color type declaring a "reference color" that would get them closer to XYZ. So, HSL would have a "reference color" of sRGB, sRGB would have a "reference color" of linearSRGB, and linearSRGB would have a reference color XYZ. Then, as long as there was a defined conversion to/from the "reference color", we could do a least common ancestor graph search between the two color types being converted between to find the optimal set of conversions needed.

That almost works, but the relationships between the four color types that make up each RGB type (bounded-gamma-encoded, bounded-linear-encoded, extended-gamma-encoded, extended-linear-encoded) make it not quite ideal, since a conversion from something like sRGB to extendedSRGB would wind up going through XYZ (since the reference color for sRGB is linearSRGB and the reference color for extendedSRGB is linearExtendedSRGB, and both either reference colors are XYZ). 

Ultimately, the depth of the type graph is not all that large (though the ascii art I made is quite big, though I did remove A98RGB and Rec2020 as they are same as DisplayP3 here):

                ┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┼ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┐
                 Matrix Conversions    ┌───────────┐│┌───────────┐
                │                      │ XYZ (D50) │││ XYZ (D65) │                                                      │
                                       └─────▲─────┘│└─────▲─────┘
                │                            │      │      │                                                            │
       ┌─────────────────────────┬───────────┘      │      └───────────┬───────────────────────────────┐
       │        │                │                  │                  │                               │                │
       │                         │                  │                  │                               │               
       │        │                │                  │                  │                               │                │
       │          ProPhotoRGB───────────────────┐   │   SRGB──────────────────────────┐ DisplayP3─────────────────────┐
       │        │ │┌────────┐ ┌────────────────┐│   │   │┌────────┐ ┌────────────────┐│ │┌────────┐ ┌────────────────┐│ │
       │          ││ Linear │ │ LinearExtended ││   │   ││ Linear │ │ LinearExtended ││ ││ Linear │ │ LinearExtended ││
       │        │ │└────────┘ └────────────────┘│   │   │└────────┘ └────────────────┘│ │└────────┘ └────────────────┘│ │
       │         ─│─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─│─ ─│─ ─│─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─│─│─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─│─
 ┌───────────┐    │┌────────┐ ┌────────────────┐│   │   │┌────────┐ ┌────────────────┐│ │┌────────┐ ┌────────────────┐│
 │    Lab    │    ││ Gamma  │ │ GammaExtended  ││   │   ││ Gamma  │ │ GammaExtended  ││ ││ Gamma  │ │ GammaExtended  ││
 └─────▲─────┘    │└────────┘ └────────────────┘│   │   │└────▲───┘ └────────────────┘│ │└────────┘ └────────────────┘│
       │          └─────────────────────────────┘   │   └─────┼───────────────────────┘ └─────────────────────────────┘
       │                                            │      ┌──┴──────────┬─────────────┐
       │                                            │      │             │             │
 ┌───────────┐                                      │┌───────────┐ ┌───────────┐┌─────────────┐
 │    LCH    │                                      ││    HSL    │ │    HWB    ││SRGB<uint8_t>│
 └───────────┘                                      │└───────────┘ └───────────┘└─────────────┘

From this, it turns out you can handle all the cases in about 5 steps, with most peeling off a conversion from the front or back or the path and using recursion to continue on. It ultimately leaves a final step where only matrix conversions remain, which can potentially be further optimized in another patch to concatenate the matrices at compile time.

1. Handle the special case SRGBA<uint8_t> for Input and Output, and recursively call conversion.

2. Handle all color types that are not IsRGBType<T> or IsXYZA<T> for Input and Output. For all
   these other color types, we can uncondtionally convert them to their "reference" color, as
   either they have already been handled by a ColorConversion specialization or this will
   get us closer to the final conversion, and recursively call conversion.

3. Handle conversions within a RGBFamily (e.g. all have the same descriptor). This will conclude
   the conversion.

4. Handle any gamma conversions for the Input and Output, recursively call conversion

5. At this point, Input and Output are each either Linear-RGB types (of different familes) or XYZA
   and therefore all additional conversion can happen via matrix transformation. This will conclude
   the conversion.


With these steps, we group the color types into 3 distinct groups.

1. RGB types.

These inherit from the RGBType struct and are defined in terms of their xyzToLinear / linearToXYZ matrices, their transfer function, whether they are bounded or extended and their white point. New RGB color types can be added without any new conversion code being required, as long as they specify those properties.

2. The XYZ type.

This is the root type, and only varies on its white point. If other white points beside D50 and D65 are ever needed, we will need to extend the ChromaticAdapation specializations to also have conversion matrices between those.

3. All other types.

All other types (e.g. Lab, LCHA, HSLA, HWBA are the set currently) have their conversions defined in terms of single reference color and require explicit specialization of the ColorConversion struct to work.

[Sam Weinig]

Created attachment 420222 [details]
Patch

[Sam Weinig]

Created attachment 420223 [details]
Patch

[Darin Adler]

Comment on attachment 420223 [details]
Patch

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

> Source/WebCore/ChangeLog:30
> +                        â â â â â â â â â â â â â â â â â â â¼ â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â

"ASCII art" that uses UTF-8, eh?

> Source/WebCore/platform/graphics/ColorConversion.h:269
> +        // results (due to floating point effects) so if this optimation is considered we should ensure we

Typo in "optimization" here.

> Source/WebCore/platform/graphics/ColorMatrix.h:56
> +template<size_t Columns, size_t Rows>

I prefer not to name counts as if they were objects. "Columns" sounds like it is a collection of columns. Maybe ColumnCount?

This comes up a lot in argument names, not just template arguments.

[Sam Weinig]

(In reply to Darin Adler from comment #3)
> Comment on attachment 420223 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=420223&action=review
> 
> > Source/WebCore/ChangeLog:30
> > +                        â â â â â â â â â â â â â â â â â â â¼ â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â â
> 
> "ASCII art" that uses UTF-8, eh?

Term of art ;).

> 
> > Source/WebCore/platform/graphics/ColorConversion.h:269
> > +        // results (due to floating point effects) so if this optimation is considered we should ensure we
> 
> Typo in "optimization" here.

Fixed.

> 
> > Source/WebCore/platform/graphics/ColorMatrix.h:56
> > +template<size_t Columns, size_t Rows>
> 
> I prefer not to name counts as if they were objects. "Columns" sounds like
> it is a collection of columns. Maybe ColumnCount?
> 
> This comes up a lot in argument names, not just template arguments.

I'll go with ColumnCount/RowCount.

Thanks!

[Sam Weinig]

Created attachment 420226 [details]
Patch

[EWS]

Patch 420226 does not build

[Sam Weinig]

Created attachment 420227 [details]
Patch

[EWS]

Committed r272837: <https://commits.webkit.org/r272837>

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

[Radar WebKit Bug Importer]

<rdar://problem/74315634>