Implement a speedup for Y-only blurs by transposing.

Implement a speedup for Y-only blurs by transposing.

R=mtklein@google.com, mtklein, reed@google.com
BUG=

Committed: https://code.google.com/p/skia/source/detail?r=12227

[Stephen White, 2013-11-11 17:06:25]

Mike &| Mike: PTAL. Thanks!

[reed1, 2013-11-11 17:09:24]

What is the histogram usage of blurs that suggest making this change? Can we
justify the (modest) extra complexity?

[mtklein, 2013-11-11 17:43:23]

My hunch is that we probably wouldn't even have thought to ask about the
benefit/complexity trade off if the y-only transpose had come along with the XY
transpose in the same CL.  This looks to me just the logical completion of that
first pass.

https://codereview.chromium.org/66413007/diff/20001/src/effects/SkBlurImageFi...
File src/effects/SkBlurImageFilter.cpp (right):

https://codereview.chromium.org/66413007/diff/20001/src/effects/SkBlurImageFi...
src/effects/SkBlurImageFilter.cpp:205: #ifndef
SK_DISABLE_BLUR_DIVISION_OPTIMIZATION
Either the new y-only transposed version needs to be guarded by this same
#ifndef, or we don't really need to guard any of the transpose code?

[Stephen White, 2013-11-11 18:06:07]

On 2013/11/11 17:09:24, reed1 wrote:
> What is the histogram usage of blurs that suggest making this change? Can we
> justify the (modest) extra complexity?

I don't have histogram data for image blurs. Sadly, the Chrome CSS usage tracker
only gathers at the top-level identifier level, not which filter it is or what
the parameters are.

The performance impact is ~1.85X on 64-bit Linux:

before:
running bench [640 480]  blur_image_filter_large_0.00_1.00    8888: cmsecs =  
8821.33
running bench [640 480] blur_image_filter_large_0.00_10.00    8888: cmsecs =  
8925.64

after:
running bench [640 480]  blur_image_filter_large_0.00_1.00    8888: cmsecs =  
4874.39
running bench [640 480] blur_image_filter_large_0.00_10.00    8888: cmsecs =  
4757.25

and about 1.5X on 32-bit Windows.

[Stephen White, 2013-11-11 18:12:50]

On 2013/11/11 17:43:23, mtklein wrote:
> My hunch is that we probably wouldn't even have thought to ask about the
> benefit/complexity trade off if the y-only transpose had come along with the
XY
> transpose in the same CL.  This looks to me just the logical completion of
that
> first pass.
> 
>
https://codereview.chromium.org/66413007/diff/20001/src/effects/SkBlurImageFi...
> File src/effects/SkBlurImageFilter.cpp (right):
> 
>
https://codereview.chromium.org/66413007/diff/20001/src/effects/SkBlurImageFi...
> src/effects/SkBlurImageFilter.cpp:205: #ifndef
> SK_DISABLE_BLUR_DIVISION_OPTIMIZATION
> Either the new y-only transposed version needs to be guarded by this same
> #ifndef, or we don't really need to guard any of the transpose code?

The transpose in this case shouldn't affect pixel results, since we're not
changing the order of the passes, just how they're laid out in memory in the
temporary buffers. In the X-and-Y case, the order of the passes was changed from
XYXYXY to XXXYYY, so the rounding comes out differently.

(BTW, I flipped the flag in Blink on Friday, so we should be ok to remove the
#ifdef. I just wanted to give it a few days to make sure it sticks.)

[reed1, 2013-11-11 18:14:56]

If you two perceive this extra code as no burden, then I'm fine with it.

In general, I think we as a team need to begin taking code
organization/maintenance into account as we move forward : we are growing more
and more on the edges (new backends, special cases, etc.). This is true
everywhere (fonts, assembly, path sniffing, picture recording peepholes, bitmap
formats, etc.).

I will look at beefing up the Lua code, to see if we can get some insight into
how lots of our effects are getting used (e.g. imagefilters, shaders, blurs,
etc.)

[mtklein, 2013-11-11 18:27:10]

On 2013/11/11 18:14:56, reed1 wrote:
> If you two perceive this extra code as no burden, then I'm fine with it.
> 
> In general, I think we as a team need to begin taking code
> organization/maintenance into account as we move forward : we are growing more
> and more on the edges (new backends, special cases, etc.). This is true
> everywhere (fonts, assembly, path sniffing, picture recording peepholes,
bitmap
> formats, etc.).
> 
> I will look at beefing up the Lua code, to see if we can get some insight into
> how lots of our effects are getting used (e.g. imagefilters, shaders, blurs,
> etc.)

lgtm

Agreed Mike.  I think this code is no extra burden.  Rather, I think this is a
good example of code that's organized to make adding a new special case no extra
burden.  It would be amazing if every new optimization we upstream is a <30 line
change.

I also agree that it's difficult to make a call based solely on microbenchmarks
and GMs.  I'd feel considerably more confident this is useful if we had a
compelling .skp or two that triggered this code path and drew faster as the code
evolves.

[Stephen White, 2013-11-11 20:12:40]

Committed patchset #2 manually as r12227 (presubmit successful).