[webkit-reviews] review denied: [Bug 120346] [cairo] ShadowBlur is extremely slow when canvas's scale is very small. : [Attachment 211396] Offset issue is fixed.

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam at webkit.org> has denied Yonggeol Jung
<yg48.jung at samsung.com>'s request for review:
Bug 120346: [cairo] ShadowBlur is extremely slow when canvas's scale is very
small.
https://bugs.webkit.org/show_bug.cgi?id=120346

Attachment 211396: Offset issue is fixed.
https://bugs.webkit.org/attachment.cgi?id=211396&action=review

------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=211396&action=review


This is a good catch, but the patch is hard to understand and needs to be a lot
clearer.

> Source/WebCore/ChangeLog:11
> +	   ShadowBlur drawing is too slow if scale of canvas is set to very
small value such as 0.01.
> +	   The main reason is blurRadius gets huge value if scale is small.
> +	   If shadow value is 100 with 0.01 scale value, blurRadius will be
10000.
> +	   Almost time is consumed to make blurred-layer.

This ChangeLog is written with poor English. Please improve.
Also though you're describing the problem, you're not at all describing the
solution.

> Source/WebCore/platform/graphics/ShadowBlur.cpp:402
> +    const AffineTransform transform = context->getCTM();

No need for const

> Source/WebCore/platform/graphics/ShadowBlur.cpp:403
> +    if (m_shadowsIgnoreTransforms && !transform.isIdentity() &&
transform.xScale() < 1 && transform.yScale() < 1)

We should exit early on m_shadowsIgnoreTransforms before we get the CTM from
the context.

> Source/WebCore/platform/graphics/ShadowBlur.cpp:420
> +	   if (scaledBlurLayer)
> +	       transform.scale(1 / transform.xScale(), 1 / transform.yScale());


What does this do? maybe a comment is in place? at least the changelog should
describe this.