Bug 234492

Summary: Add support for premultiplied alpha interpolated gradients and defaulted off option to use them for CSS Gradients
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, bramus, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, macpherson, menard, mitz, ryuan.choi, sergio, simon.fraser, tmjenox, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 150940    
Attachments:
Description Flags
For the bots
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch none

Description Sam Weinig 2021-12-19 13:14:34 PST 
Add CGShadingRef based implementation of Gradient as stepping stone toward premultiplied interpolation
Comment 1 Sam Weinig 2021-12-19 13:20:22 PST Comment hidden (obsolete)
Comment 2 Sam Weinig 2021-12-19 13:55:40 PST Comment hidden (obsolete)
Comment 3 Sam Weinig 2021-12-19 15:38:43 PST Comment hidden (obsolete)
Comment 4 Sam Weinig 2021-12-19 17:48:16 PST Comment hidden (obsolete)
Comment 5 Sam Weinig 2021-12-19 17:48:42 PST Comment hidden (obsolete)
Comment 6 Sam Weinig 2021-12-19 18:10:46 PST Comment hidden (obsolete)
Comment 7 Sam Weinig 2021-12-19 20:59:10 PST Comment hidden (obsolete)
Comment 8 Sam Weinig 2021-12-19 21:00:06 PST Comment hidden (obsolete)
Comment 9 Sam Weinig 2021-12-19 21:55:27 PST Comment hidden (obsolete)
Comment 10 Sam Weinig 2021-12-20 11:31:20 PST Comment hidden (obsolete)
Comment 11 Sam Weinig 2021-12-20 15:16:10 PST 
Created attachment 447654 [details]
Patch
Comment 12 Sam Weinig 2021-12-20 15:17:09 PST 
Comment on attachment 447654 [details]
Patch

Still need to figure out what to do about the GTK port, since it doesn't look like Cairo has premultiplied alpha support, but this is good to review now.
Comment 13 Simon Fraser (smfr) 2021-12-20 15:32:17 PST 
Comment on attachment 447654 [details]
Patch

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

> Source/WTF/Scripts/Preferences/WebPreferencesExperimental.yaml:287
> +CSSGradientAlphaPremultipliedInterpolationEnabled:

287CSSGradientAlphaPremultipliedInterpolationEnabled -> 287CSSGradientPremultipliedAlphaInterpolationEnabled ?

> Source/WebCore/platform/graphics/cg/GradientCGStrategy.h:82
> +    using Strategy = std::variant<Gradient, Shading>;

I don't like the naming here. To me, a "strategy" sounds like an enum value describing which implementation to use. I was confused that pickStrategy(), makeGradient() and makeShading() all return something called a "Strategy".

std::variant<Gradient, Shading> is more like "GradientRenderer" or something.
Comment 14 Sam Weinig 2021-12-20 17:16:57 PST 
Created attachment 447668 [details]
Patch
Comment 15 Sam Weinig 2021-12-20 21:24:02 PST 
Created attachment 447685 [details]
Patch
Comment 16 Sam Weinig 2021-12-20 21:29:44 PST 
Created attachment 447686 [details]
Patch
Comment 17 EWS 2021-12-21 04:43:34 PST 
Committed r287310 (245462@main): <https://commits.webkit.org/245462@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447686 [details].
Comment 18 Radar WebKit Bug Importer 2021-12-21 04:44:18 PST 
<rdar://problem/86764026>
Comment 19 mitz 2021-12-21 06:53:42 PST 
Comment on attachment 447686 [details]
Patch

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

> Source/WebCore/platform/graphics/cg/GradientRendererCG.cpp:53
> +                // FIXME: Use gradient strategy if none of the stops have alpha.

For performance, it may be useful to do this also whenever the only stops with non-1 alpha have 0 alpha, by adjusting and, if necessary, repeating the transparent stop. There are likely other cases (likely the most common cases) that can be optimized this way.
Comment 20 Fujii Hironori 2021-12-21 13:43:40 PST 
*** Bug 150940 has been marked as a duplicate of this bug. ***