Apply the CTM to filter parameters for 4 pixel-moving filters.

Apply the CTM to filter parameters for SkBlurImageFilter, SkDropShadowImageFilter, SkDisplacementMapEffect and SkMorphologyImageFilter. I had resisted this (and may later put in an assert that the CTM contains no rotation, skew or perspective), but it just makes the filters play better with Skia generally.

This revealed that the displacement map was not handling clipping or upstream cropping at all well (the color would "swim" through the displacement at the edge of the clip rect). Fixed by passing through the correct offsets to the bitmap accesses in both raster and GPU paths. Same for morphology.

R=sugoi@google.com

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

[Stephen White, 2014-01-17 21:37:55]

PTAL. Thanks!

[sugoi, 2014-01-17 21:56:27]

https://codereview.chromium.org/137053003/diff/120001/gm/imagefiltersscaled.cpp
File gm/imagefiltersscaled.cpp (right):

https://codereview.chromium.org/137053003/diff/120001/gm/imagefiltersscaled.c...
gm/imagefiltersscaled.cpp:98: SkVector::Make(SkIntToScalar(1),
SkIntToScalar(1)),
How about having at least 1 test case for downscaling (scale < 1) ?

https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
File src/effects/SkDisplacementMapEffect.cpp (right):

https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
src/effects/SkDisplacementMapEffect.cpp:217: bounds.offset(colorOffset);
We offset the source here and then we subtract a constant offset (srcOffset) to
every pixel within the loop ? It seems inefficient, unless I'm missing
something.

https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
src/effects/SkDisplacementMapEffect.cpp:395:
matrix.setTranslate(-SkIntToScalar(bounds.left()),
-SkIntToScalar(bounds.top()));
Same comment about offsetting the bounds and translating back.

[Stephen White, 2014-01-18 01:23:19]

https://codereview.chromium.org/137053003/diff/120001/gm/imagefiltersscaled.cpp
File gm/imagefiltersscaled.cpp (right):

https://codereview.chromium.org/137053003/diff/120001/gm/imagefiltersscaled.c...
gm/imagefiltersscaled.cpp:98: SkVector::Make(SkIntToScalar(1),
SkIntToScalar(1)),
On 2014/01/17 21:56:27, sugoi wrote:
> How about having at least 1 test case for downscaling (scale < 1) ?

Done.

https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
File src/effects/SkDisplacementMapEffect.cpp (right):

https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
src/effects/SkDisplacementMapEffect.cpp:217: bounds.offset(colorOffset);
On 2014/01/17 21:56:27, sugoi wrote:
> We offset the source here and then we subtract a constant offset (srcOffset)
to
> every pixel within the loop ? It seems inefficient, unless I'm missing
> something.

We have to offset it to intersect it below. But we can untranslate it
afterwards.

Fixed.

https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
src/effects/SkDisplacementMapEffect.cpp:395:
matrix.setTranslate(-SkIntToScalar(bounds.left()),
-SkIntToScalar(bounds.top()));
On 2014/01/17 21:56:27, sugoi wrote:
> Same comment about offsetting the bounds and translating back.

Fixed (although I don't think it'll make it any faster in the GPU case.)

[sugoi, 2014-01-20 12:44:23]

On 2014/01/18 01:23:19, Stephen White wrote:
>
https://codereview.chromium.org/137053003/diff/120001/gm/imagefiltersscaled.cpp
> File gm/imagefiltersscaled.cpp (right):
> 
>
https://codereview.chromium.org/137053003/diff/120001/gm/imagefiltersscaled.c...
> gm/imagefiltersscaled.cpp:98: SkVector::Make(SkIntToScalar(1),
> SkIntToScalar(1)),
> On 2014/01/17 21:56:27, sugoi wrote:
> > How about having at least 1 test case for downscaling (scale < 1) ?
> 
> Done.
> 
>
https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
> File src/effects/SkDisplacementMapEffect.cpp (right):
> 
>
https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
> src/effects/SkDisplacementMapEffect.cpp:217: bounds.offset(colorOffset);
> On 2014/01/17 21:56:27, sugoi wrote:
> > We offset the source here and then we subtract a constant offset (srcOffset)
> to
> > every pixel within the loop ? It seems inefficient, unless I'm missing
> > something.
> 
> We have to offset it to intersect it below. But we can untranslate it
> afterwards.
> 
> Fixed.
> 
>
https://codereview.chromium.org/137053003/diff/120001/src/effects/SkDisplacem...
> src/effects/SkDisplacementMapEffect.cpp:395:
> matrix.setTranslate(-SkIntToScalar(bounds.left()),
> -SkIntToScalar(bounds.top()));
> On 2014/01/17 21:56:27, sugoi wrote:
> > Same comment about offsetting the bounds and translating back.
> 
> Fixed (although I don't think it'll make it any faster in the GPU case.)

lgtm

[Stephen White, 2014-01-20 15:33:11]

New patch: add morphology to the party (which is even more in need of a
refactoring now).

[sugoi, 2014-01-20 19:36:56]

https://codereview.chromium.org/137053003/diff/670001/gm/imagefiltersscaled.cpp
File gm/imagefiltersscaled.cpp (right):

https://codereview.chromium.org/137053003/diff/670001/gm/imagefiltersscaled.c...
gm/imagefiltersscaled.cpp:70: SkAutoTUnref<SkShader>
shader(SkGradientShader::CreateRadial(SkPoint::Make(x, y), radius, colors, NULL,
2, SkShader::kClamp_TileMode));
Exceeds 100 chars.

https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
File src/effects/SkMorphologyImageFilter.cpp (right):

https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
src/effects/SkMorphologyImageFilter.cpp:611: if (!apply_morphology(input,
srcBounds, GrMorphologyEffect::kDilate_MorphologyType, SkISize::Make(width,
height), result)) {
Exceeds 100 chars.

https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
src/effects/SkMorphologyImageFilter.cpp:652: if (!apply_morphology(input,
srcBounds, GrMorphologyEffect::kErode_MorphologyType, SkISize::Make(width,
height), result)) {
Exceeds 100 chars.

[Stephen White, 2014-01-20 19:46:01]

https://codereview.chromium.org/137053003/diff/670001/gm/imagefiltersscaled.cpp
File gm/imagefiltersscaled.cpp (right):

https://codereview.chromium.org/137053003/diff/670001/gm/imagefiltersscaled.c...
gm/imagefiltersscaled.cpp:70: SkAutoTUnref<SkShader>
shader(SkGradientShader::CreateRadial(SkPoint::Make(x, y), radius, colors, NULL,
2, SkShader::kClamp_TileMode));
On 2014/01/20 19:36:57, sugoi wrote:
> Exceeds 100 chars.

Fixed.

https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
File src/effects/SkMorphologyImageFilter.cpp (right):

https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
src/effects/SkMorphologyImageFilter.cpp:611: if (!apply_morphology(input,
srcBounds, GrMorphologyEffect::kDilate_MorphologyType, SkISize::Make(width,
height), result)) {
On 2014/01/20 19:36:57, sugoi wrote:
> Exceeds 100 chars.

Fixed.

https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
src/effects/SkMorphologyImageFilter.cpp:652: if (!apply_morphology(input,
srcBounds, GrMorphologyEffect::kErode_MorphologyType, SkISize::Make(width,
height), result)) {
On 2014/01/20 19:36:57, sugoi wrote:
> Exceeds 100 chars.

Fixed.

[sugoi, 2014-01-20 19:57:13]

On 2014/01/20 19:46:01, Stephen White wrote:
>
https://codereview.chromium.org/137053003/diff/670001/gm/imagefiltersscaled.cpp
> File gm/imagefiltersscaled.cpp (right):
> 
>
https://codereview.chromium.org/137053003/diff/670001/gm/imagefiltersscaled.c...
> gm/imagefiltersscaled.cpp:70: SkAutoTUnref<SkShader>
> shader(SkGradientShader::CreateRadial(SkPoint::Make(x, y), radius, colors,
NULL,
> 2, SkShader::kClamp_TileMode));
> On 2014/01/20 19:36:57, sugoi wrote:
> > Exceeds 100 chars.
> 
> Fixed.
> 
>
https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
> File src/effects/SkMorphologyImageFilter.cpp (right):
> 
>
https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
> src/effects/SkMorphologyImageFilter.cpp:611: if (!apply_morphology(input,
> srcBounds, GrMorphologyEffect::kDilate_MorphologyType, SkISize::Make(width,
> height), result)) {
> On 2014/01/20 19:36:57, sugoi wrote:
> > Exceeds 100 chars.
> 
> Fixed.
> 
>
https://codereview.chromium.org/137053003/diff/670001/src/effects/SkMorpholog...
> src/effects/SkMorphologyImageFilter.cpp:652: if (!apply_morphology(input,
> srcBounds, GrMorphologyEffect::kErode_MorphologyType, SkISize::Make(width,
> height), result)) {
> On 2014/01/20 19:36:57, sugoi wrote:
> > Exceeds 100 chars.
> 
> Fixed.

lgtm

[Stephen White, 2014-01-20 19:58:33]

Committed patchset #16 manually as r13127 (presubmit successful).