[webkit-reviews] review granted: [Bug 181720] [Cairo] Use one-time ShadowBlur objects when performing shadowing : [Attachment 331471] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 181720: [Cairo] Use one-time ShadowBlur objects when performing shadowing
https://bugs.webkit.org/show_bug.cgi?id=181720

Attachment 331471: Patch

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




--- Comment #2 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 331471
  --> https://bugs.webkit.org/attachment.cgi?id=331471
Patch

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

> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:188
> +    ShadowBlur shadow(FloatSize { shadowState.blur, shadowState.blur },

Do you really need to use FloatSize explicitly?

> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:472
> +bool ShadowState::shadowVisible() const

shadow is redundant in method name, I would use isVisible

> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:477
> +bool ShadowState::shadowRequired(PlatformContextCairo& platformContext)
const

Ditto.

> Source/WebCore/platform/graphics/cairo/CairoOperations.h:116
> +    float blur;

Why did you remove the initialization here?