Bug 120346 – [cairo] ShadowBlur is extremely slow when canvas's scale is very small.

• Status: UNCONFIRMED
• Component: WebKitGTK

[Yonggeol Jung]

ShadowBlur drawing is too slow if scale of canvas is set to very small value such as 0.01.

[Yonggeol Jung]

Created attachment 209729 [details]
testcase

This is test file for this problem.

It draws (10000 X 10000) size rectangle with shadowblur(100).
And canvas's scale is set to 0.01.

You can see this problem at 5'th test of "http://www.kevs3d.co.uk/dev/canvasmark/" also.

[Yonggeol Jung]

Created attachment 209730 [details]
Patch

[WebKit Commit Bot]

Attachment 209730 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/ShadowBlur.cpp', u'Source/WebCore/platform/graphics/ShadowBlur.h']" exit_code: 1
Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yonggeol Jung]

Created attachment 209733 [details]
Patch

[WebKit Commit Bot]

Attachment 209733 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/ShadowBlur.cpp', u'Source/WebCore/platform/graphics/ShadowBlur.h']" exit_code: 1
Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yonggeol Jung]

Created attachment 209735 [details]
Patch

[WebKit Commit Bot]

Attachment 209735 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/ShadowBlur.cpp', u'Source/WebCore/platform/graphics/ShadowBlur.h']" exit_code: 1
Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yonggeol Jung]

Created attachment 209736 [details]
Patch

[WebKit Commit Bot]

Attachment 209736 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/ShadowBlur.cpp', u'Source/WebCore/platform/graphics/ShadowBlur.h']" exit_code: 1
Source/WebCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yonggeol Jung]

Created attachment 209738 [details]
Patch

[Yonggeol Jung]

Created attachment 211396 [details]
Offset issue is fixed.

[Noam Rosenthal]

Comment on attachment 211396 [details]
Offset issue is fixed.

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.

[Yonggeol Jung]

Created attachment 214427 [details]
Patch

[Yonggeol Jung]

(In reply to comment #12)
> (From update of attachment 211396 [details])
> 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.

Thanks for review.

> 
> > 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.

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

done.

> > 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.
> 

done.

> > 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.

Before drawing the shadows, the scale factor of context is 1.
So, the positions and sizes of rectangles should be calculated as considering the scale factor is 1. (I added comment.)

[Brady Eidson]

Comment on attachment 214427 [details]
Patch

Assuming that patches for review since 2013 are stale, r-

[Michael Catanzaro]

Reassigning some EFL bugs that are likely shared with GTK/WPE to the GTK component.