[webkit-reviews] review granted: [Bug 214221] Simplify and improve Gradient, some other small color-related removals : [Attachment 404073] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Darin Adler <darin at apple.com>'s request
for review:
Bug 214221: Simplify and improve Gradient, some other small color-related
removals
https://bugs.webkit.org/show_bug.cgi?id=214221

Attachment 404073: Patch

https://bugs.webkit.org/attachment.cgi?id=404073&action=review




--- Comment #5 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 404073
  --> https://bugs.webkit.org/attachment.cgi?id=404073
Patch

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

> Source/WebCore/platform/graphics/Gradient.h:107
>      WEBCORE_EXPORT static Ref<Gradient> create(LinearData&&);
>      WEBCORE_EXPORT static Ref<Gradient> create(RadialData&&);
> -    WEBCORE_EXPORT static Ref<Gradient> create(ConicData&&);
> +    WEBCORE_EXPORT static Ref<Gradient> create(Data&&);

The inconsistency here bugs me. Why do we still need the create functions
taking LinearData or RadialData?

> Source/WebCore/platform/graphics/Gradient.h:166
>  
>      mutable unsigned m_cachedHash { 0 };

Might be able save a byte or two if we re-order things so m_stopsSorted,
m_spreadMethod and m_cachedHash pack next to each other.

> Source/WebCore/rendering/svg/RenderSVGResourceGradient.cpp:238
>  void RenderSVGResourceGradient::addStops(GradientData* gradientData, const
Vector<Gradient::ColorStop>& stops, const RenderStyle& style) const
>  {
> +    ASSERT(gradientData);
>      ASSERT(gradientData->gradient);

Not sure how far you want to go, but it looks like all the callers can prove
GradientData to be non-null so this could be switched to pass by reference.