Bug 220079

Summary:

Move the space transform outside the Gradient class

Product:

WebKit

Reporter:

Said Abou-Hallawa <sabouhallawa>

Component:

Layout and Rendering

Assignee:

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:

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

Description Said Abou-Hallawa 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.

Comment 1 Said Abou-Hallawa 2020-12-21 20:53:24 PST

Created attachment 416650 [details]
Patch

Comment 2 Said Abou-Hallawa 2020-12-21 21:18:09 PST

Created attachment 416651 [details]
Patch

Comment 3 Said Abou-Hallawa 2020-12-21 21:45:51 PST

Created attachment 416653 [details]
Patch

Comment 4 Simon Fraser (smfr) 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()).

Comment 5 Said Abou-Hallawa 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.

Comment 6 Simon Fraser (smfr) 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.

Comment 7 Radar WebKit Bug Importer 2020-12-28 20:42:13 PST

<rdar://problem/72715158>

Comment 8 Said Abou-Hallawa 2021-01-12 23:01:29 PST

Created attachment 417509 [details]
Patch

Comment 9 Simon Fraser (smfr) 2021-01-13 09:37:53 PST

Comment on attachment 417509 [details]
Patch

Nice!

Comment 10 EWS 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].