[webkit-reviews] review granted: [Bug 45902] [Cairo] Port drawTiledShadow to the new ContextShadow : [Attachment 69650] Proposed patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Alejandro G. Castro
<alex at igalia.com>'s request for review:
Bug 45902: [Cairo] Port drawTiledShadow to the new ContextShadow
https://bugs.webkit.org/show_bug.cgi?id=45902

Attachment 69650: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=69650&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=69650&action=review

Great! Please fix the small issues before landing.

> WebCore/platform/graphics/cairo/ContextShadowCairo.cpp:139
> +    cairo_set_source_rgba(m_layerContext, 0.f, 0.f, 0.f, alpha);

I think this can just be cairo_set_source_rgba(m_layerContext, 0, 0, 0, alpha);


> WebCore/platform/graphics/cairo/ContextShadowCairo.cpp:144
> +
> +    return;

No need for this.

> WebCore/platform/graphics/cairo/ContextShadowCairo.cpp:199
> +    // Calculate filter values to create appropriate shadow.

Might want to update this comment, as we are no longer using SVG filters.

> WebCore/platform/graphics/cairo/ContextShadowCairo.cpp:201
> +    shadowBufferSize = IntSize(rect.width() + m_blurRadius * 2,
rect.height() + m_blurRadius * 2);

I think this can be:
IntSize shadowBufferSize = IntSize(rect.width() + m_blurRadius * 2,
rect.height() + m_blurRadius * 2);

> WebCore/platform/graphics/cairo/ContextShadowCairo.cpp:248
> +    cairo_set_source_rgba(m_layerContext, 0.f, 0.f, 0.f,
context->getAlpha());

These can also be cairo_set_source_rgba(m_layerContext, 0, 0, 0,
context->getAlpha());