Bug 220079

Summary: Move the space transform outside the Gradient class
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: Layout and RenderingAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, kondapallykalyan, pdr, schenney, sergio, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 219468    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Said Abou-Hallawa
Reported 2020-12-21 20:41:49 PST
The goal is get the suitable abstraction for the Gradient class so we can share it among different clients easier.
Attachments
Patch (39.56 KB, patch)
2020-12-21 20:53 PST, Said Abou-Hallawa 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (44.98 KB, patch)
2020-12-21 21:18 PST, Said Abou-Hallawa 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (45.27 KB, patch)
2020-12-21 21:45 PST, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Patch (31.74 KB, patch)
2021-01-12 23:01 PST, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Said Abou-Hallawa
Comment 1 2020-12-21 20:53:24 PST
Created attachment 416650 [details] Patch
Said Abou-Hallawa
Comment 2 2020-12-21 21:18:09 PST
Created attachment 416651 [details] Patch
Said Abou-Hallawa
Comment 3 2020-12-21 21:45:51 PST
Created attachment 416653 [details] Patch
Simon Fraser (smfr)
Comment 4 2020-12-22 08:48:48 PST
Comment on attachment 416653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416653&action=review > Source/WebCore/platform/graphics/Gradient.h:77 > + GradientSpreadMethod spreadMethod { GradientSpreadMethod::Pad }; I think you can keep GradientSpreadMethod as part of Gradient. All existing code sets it at the same time as the color stops. It's only gradientSpaceTransform that's change per-paint (see RenderSVGResourceGradient::applyResource()).
Said Abou-Hallawa
Comment 5 2020-12-22 10:39:38 PST
Comment on attachment 416653 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416653&action=review >> Source/WebCore/platform/graphics/Gradient.h:77 >> + GradientSpreadMethod spreadMethod { GradientSpreadMethod::Pad }; > > I think you can keep GradientSpreadMethod as part of Gradient. All existing code sets it at the same time as the color stops. It's only gradientSpaceTransform that's change per-paint (see RenderSVGResourceGradient::applyResource()). I looked at CanvasGradient https://developer.mozilla.org/en-US/docs/Web/API/CanvasGradient and tried to make the Gradient class store what it stores: gradient data and color stops.
Simon Fraser (smfr)
Comment 6 2020-12-22 11:00:46 PST
(In reply to Said Abou-Hallawa from comment #5) > Comment on attachment 416653 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=416653&action=review > > >> Source/WebCore/platform/graphics/Gradient.h:77 > >> + GradientSpreadMethod spreadMethod { GradientSpreadMethod::Pad }; > > > > I think you can keep GradientSpreadMethod as part of Gradient. All existing code sets it at the same time as the color stops. It's only gradientSpaceTransform that's change per-paint (see RenderSVGResourceGradient::applyResource()). > > I looked at CanvasGradient > https://developer.mozilla.org/en-US/docs/Web/API/CanvasGradient and tried to > make the Gradient class store what it stores: gradient data and color stops. Right but the spread method isn't exposed for canvas gradients.
Radar WebKit Bug Importer
Comment 7 2020-12-28 20:42:13 PST
<rdar://problem/72715158>
Said Abou-Hallawa
Comment 8 2021-01-12 23:01:29 PST
Created attachment 417509 [details] Patch
Simon Fraser (smfr)
Comment 9 2021-01-13 09:37:53 PST
Comment on attachment 417509 [details] Patch Nice!
EWS
Comment 10 2021-01-13 16:07:30 PST
Committed r271472: <https://trac.webkit.org/changeset/271472> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417509 [details].
Note You need to log in before you can comment on or make changes to this bug.