Bug 234653 – On systems without CG support for alpha premultiplied gradients, the CGGradientRef path should still be used for the subset of gradients that can transformed

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

[Sam Weinig]

The current behavior of the CoreGraphics gradient drawing code path is to always use the CGShadingRef strategy for alpha premultiplied gradients if the system does not natively support alpha premultiplied gradients in the CGGradientRef code path.

For performance, we really would like to use the CGGradientRef code path as much as possible, as the backend has a better opportunity to optimize over the CGShadingRef code path.

Fortunately, there are a class of common gradients that can still use the CGGradientRef code path either totally unmodified or with a transformation of the color stops.

Any gradient that uses a uniform alpha value in all its color stops (e.g. linear-gradient(rgba(255, 0, 0, 0.5), rgba(0, 0, 255, 0.5)) can be used directly with no modification, as the alpha premultiplied and alpha non-premultiplied renderings are identical. 

Additionally, gradients that conform to the rule that "any two consecutive color stops must either have one that is fully transparent or have the same alpha value for both" can be transformed to be used by the alpha non-premultiplied backend via a transformation of the color stop list.

[Sam Weinig]

Created attachment 447916 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 447916 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:57
> +static AlphaType classifyAlphaType(float alpha)

This could be called alphaType().

> Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:137
> +    // The following is the set of transforms that can be preformed on a color stop list to transform it from one used with a premuliplied alpha gradient

preformed -> performed

> Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:153
> +    // ... Opaque  -> Transparent -> Transparent -> Transparent -> Opaque  ...    ==>    ... Opaque  -> TRANSFORM{Transparent(PreviousChannels)} -> Transparent -> TRANSFORM{Transparent(NextChannels)} -> Opaque  ...
> +    // ... Partial -> Transparent -> Transparent -> Transparent -> Partial ...    ==>    ... Partial -> TRANSFORM{Transparent(PreviousChannels)} -> Transparent -> TRANSFORM{Transparent(NextChannels)} -> Partial ...
> +    // ... Opaque  -> Transparent -> Transparent -> Transparent -> Partial ...    ==>    ... Opaque  -> TRANSFORM{Transparent(PreviousChannels)} -> Transparent -> TRANSFORM{Transparent(NextChannels)} -> Partial ...
> +    // ... Partial -> Transparent -> Transparent -> Transparent -> Opaque  ...    ==>    ... Partial -> TRANSFORM{Transparent(PreviousChannels)} -> Transparent -> TRANSFORM{Transparent(NextChannels)} -> Opaque  ...

If it simplifies code, the middle Transparent here could be skipped or use either endpoint.

> Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:157
> +    // [ Transparent -> Transparent ...                                           ==>    [ Transparent                          -> Transparent ...

Transparent -> Transparent is also equivalent to TRANSFORM{Transparent(NextChannels)} -> Transparent

> Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:161
> +    // ... Transparent  -> Transparent ]                                          ==>    ... Transparent -> Transparent                              ]

Transparent -> Transparent is equivalent to Transparent -> TRANSFORM{Transparent(PreviousChannels)}

> Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:235
> +    // For three or more stops, we handle the first stop special, then the middle stops one at a time with the next and previous stop as context, then the last stop as special.

"handle the first stop special" isn't very grammatical.

[Sam Weinig]

(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 447916 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=447916&action=review
> 
> > Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:57
> > +static AlphaType classifyAlphaType(float alpha)
> 
> This could be called alphaType().
> 
> > Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:137
> > +    // The following is the set of transforms that can be preformed on a color stop list to transform it from one used with a premuliplied alpha gradient
> 
> preformed -> performed
> 
> > Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:153
> > +    // ... Opaque  -> Transparent -> Transparent -> Transparent -> Opaque  ...    ==>    ... Opaque  -> TRANSFORM{Transparent(PreviousChannels)} -> Transparent -> TRANSFORM{Transparent(NextChannels)} -> Opaque  ...
> > +    // ... Partial -> Transparent -> Transparent -> Transparent -> Partial ...    ==>    ... Partial -> TRANSFORM{Transparent(PreviousChannels)} -> Transparent -> TRANSFORM{Transparent(NextChannels)} -> Partial ...
> > +    // ... Opaque  -> Transparent -> Transparent -> Transparent -> Partial ...    ==>    ... Opaque  -> TRANSFORM{Transparent(PreviousChannels)} -> Transparent -> TRANSFORM{Transparent(NextChannels)} -> Partial ...
> > +    // ... Partial -> Transparent -> Transparent -> Transparent -> Opaque  ...    ==>    ... Partial -> TRANSFORM{Transparent(PreviousChannels)} -> Transparent -> TRANSFORM{Transparent(NextChannels)} -> Opaque  ...
> 
> If it simplifies code, the middle Transparent here could be skipped or use
> either endpoint.
> 
> > Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:157
> > +    // [ Transparent -> Transparent ...                                           ==>    [ Transparent                          -> Transparent ...
> 
> Transparent -> Transparent is also equivalent to
> TRANSFORM{Transparent(NextChannels)} -> Transparent
> 
> > Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:161
> > +    // ... Transparent  -> Transparent ]                                          ==>    ... Transparent -> Transparent                              ]
> 
> Transparent -> Transparent is equivalent to Transparent ->
> TRANSFORM{Transparent(PreviousChannels)}

These basically come down to the choice of either taking an extra branch (what the code currently does) or doing an extra Color copy. I don't think it would make the code much simpler though. For the prefix case it would go from:

    if (firstStopAlphaType == AlphaType::FullyTransparent && secondStopAlphaType != AlphaType::FullyTransparent) {
        // ACTION: Steal next.
        result.append({ firstStop.offset, secondStop.color.colorWithAlpha(0.0f) });
    } else {
        // ACTION: Default.
        result.append(firstStop);
    }

to

    if (firstStopAlphaType == AlphaType::FullyTransparent) {
        // ACTION: Steal next.
        result.append({ firstStop.offset, secondStop.color.colorWithAlpha(0.0f) });
    } else {
        // ACTION: Default.
        result.append(firstStop);
    }


We could similarly remove a branch from the middle and suffix steps, but I would rather not, as the rules are easier for me to follow if we are only talking about transitions from fully transparent to opaque/partially transparent (or vice-versa).

> 
> > Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:235
> > +    // For three or more stops, we handle the first stop special, then the middle stops one at a time with the next and previous stop as context, then the last stop as special.
> 
> "handle the first stop special" isn't very grammatical.

Eek. Indeed.


Thanks for the look.

[Sam Weinig]

Created attachment 447927 [details]
Patch

[EWS]

Committed r287426 (245561@main): <https://commits.webkit.org/245561@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/86894195>

[Darin Adler]

Comment on attachment 447916 [details]
Patch

I would mark classifyAlphaType as constexpr