Bug 234653

Summary: On systems without CG support for alpha premultiplied gradients, the CGGradientRef path should still be used for the subset of gradients that can transformed
Product: WebKit Reporter: Sam Weinig <sam>
Component: PlatformAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, mitz, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 150940    
Attachments:
Description Flags
Patch
none
Patch none

Sam Weinig
Reported 2021-12-23 15:21:40 PST
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.
Attachments
Patch (34.73 KB, patch)
2021-12-23 15:47 PST, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Patch (35.93 KB, patch)
2021-12-23 19:12 PST, Sam Weinig 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2021-12-23 15:47:26 PST
Created attachment 447916 [details] Patch
Simon Fraser (smfr)
Comment 2 2021-12-23 16:12:21 PST
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
Comment 3 2021-12-23 16:22:22 PST
(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
Comment 4 2021-12-23 19:12:26 PST
Created attachment 447927 [details] Patch
EWS
Comment 5 2021-12-24 08:48:12 PST
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
Comment 6 2021-12-24 08:49:20 PST
<rdar://problem/86894195>
Darin Adler
Comment 7 2021-12-31 21:15:12 PST
Comment on attachment 447916 [details] Patch I would mark classifyAlphaType as constexpr
Note You need to log in before you can comment on or make changes to this bug.