Bug 220079 – Move the space transform outside the Gradient class

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

The goal is get the suitable abstraction for the Gradient class so we can share it among different clients easier.

[Said Abou-Hallawa]

Created attachment 416650 [details]
Patch

[Said Abou-Hallawa]

Created attachment 416651 [details]
Patch

[Said Abou-Hallawa]

Created attachment 416653 [details]
Patch

[Simon Fraser (smfr)]

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 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)]

(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]

<rdar://problem/72715158>

[Said Abou-Hallawa]

Created attachment 417509 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 417509 [details]
Patch

Nice!

[EWS]

Committed r271472: <https://trac.webkit.org/changeset/271472>

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