tunnel down texture-size-constraint to imagefilters

tunnel down texture-size-constraint to imagefilters

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/c9b5f8b1522e72449d704d30ed6aee4fc6211ee8

[reed1, 2015-10-21 14:41:17]

reed@google.com changed reviewers:
+ bsalomon@google.com, robertphillips@google.com, senorblanco@google.com

[reed1, 2015-10-21 14:41:18]

This should only affect a few filters that create their texture in response to
applyFilter(), which must have an exact size match. Filters called during a draw
should be unaffected.

[reed1, 2015-10-21 14:41:21]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 14:41:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/1

[commit-bot: I haz the power, 2015-10-21 14:42:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-21 14:42:20]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[reed1, 2015-10-21 14:55:41]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 14:55:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/20001

[commit-bot: I haz the power, 2015-10-21 14:56:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-21 14:56:55]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)

[bsalomon, 2015-10-21 15:01:35]

lgtm

[reed1, 2015-10-21 15:11:42]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 15:11:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/40001

[commit-bot: I haz the power, 2015-10-21 15:12:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-21 15:12:41]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)

[robertphillips, 2015-10-21 15:29:41]

https://codereview.chromium.org/1421493003/diff/40001/include/core/SkMaskFilt...
File include/core/SkMaskFilter.h (right):

https://codereview.chromium.org/1421493003/diff/40001/include/core/SkMaskFilt...
include/core/SkMaskFilter.h:149: bool canOverwriteSrc,
How is this going to compile w/o GPU?

https://codereview.chromium.org/1421493003/diff/40001/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.chromium.org/1421493003/diff/40001/src/image/SkImage_Gpu.c...
src/image/SkImage_Gpu.cpp:250: // constructor.
kRequireExactTexSize ?

[reed1, 2015-10-21 17:02:11]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 17:02:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/60001

[reed1, 2015-10-21 17:05:51]

https://codereview.chromium.org/1421493003/diff/40001/include/core/SkMaskFilt...
File include/core/SkMaskFilter.h (right):

https://codereview.chromium.org/1421493003/diff/40001/include/core/SkMaskFilt...
include/core/SkMaskFilter.h:149: bool canOverwriteSrc,
On 2015/10/21 15:29:41, robertphillips wrote:
> How is this going to compile w/o GPU?

It is declared inside SK_SUPPORT_GPU

https://codereview.chromium.org/1421493003/diff/40001/src/image/SkImage_Gpu.cpp
File src/image/SkImage_Gpu.cpp (right):

https://codereview.chromium.org/1421493003/diff/40001/src/image/SkImage_Gpu.c...
src/image/SkImage_Gpu.cpp:250: // constructor.
On 2015/10/21 15:29:41, robertphillips wrote:
> kRequireExactTexSize ?

Done.

[reed1, 2015-10-21 17:09:09]

The CQ bit was unchecked by reed@google.com

[reed1, 2015-10-21 17:37:21]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 17:37:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/80001

[reed1, 2015-10-21 17:38:38]

Stephen, does this look OK to you?

[commit-bot: I haz the power, 2015-10-21 17:47:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-21 17:47:27]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-10-21 17:48:08]

lgtm

[Stephen White, 2015-10-21 17:57:43]

senorblanco@chromium.org changed reviewers:
+ senorblanco@chromium.org

[Stephen White, 2015-10-21 17:57:44]

I'd strongly request that we not do this. The problem with this approach is
that:

1) Allocating textures is slow, and
2) the cache can quickly fill up with differently-sized exact textures.

Consider a small primitive in Chrome with a filter attached. If the primitive
has an animated size or, e.g., is a CSS-transformed element with a rotation
including Z, so its 2D size constantly changes, the cache will have to
constantly allocate textures of a new size.

Note that we used to have this functionality, and I did a fair bit work to
remove it. I'd really prefer we not go back there.

[Stephen White, 2015-10-21 21:06:53]

https://codereview.chromium.org/1421493003/diff/80001/include/core/SkImageFil...
File include/core/SkImageFilter.h (right):

https://codereview.chromium.org/1421493003/diff/80001/include/core/SkImageFil...
include/core/SkImageFilter.h:102: Proxy(bool needExactTextureSize = false);
As discussed, let's try adding this to the Context rather than the Proxy.

https://codereview.chromium.org/1421493003/diff/80001/src/effects/SkBlurMaskF...
File src/effects/SkBlurMaskFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/80001/src/effects/SkBlurMaskF...
src/effects/SkBlurMaskFilter.cpp:71: GrTextureProvider::SizeConstraint) const
override;
It seems a little strange that we need to modify mask filters to satisfy image
filters. Could we just always request an approx-match in the GaussianBlur call
for mask filters, and avoid plumbing this through?

[reed1, 2015-10-21 21:08:02]

Very good suggestion about moving flag to context -- I *think* its easy to know
where to cancel-out the exact request -- in filterInput(). Thanks.

[reed1, 2015-10-21 22:04:21]

patch set #7 passes cc_unittests w/ the chrome CL (just tested locally)

I know I have some line-ending nits, and perhaps could make the last arg to
Context's constructor have a default (approx) value. Other than those, this is
ready for review.

[reed1, 2015-10-21 22:05:48]

Potentially I could land the infrastructure change in GrTextureProvider
separately. It would reduce the size of this CL

[reed1, 2015-10-21 22:06:08]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-21 22:06:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/120001

[Stephen White, 2015-10-21 22:07:14]

On 2015/10/21 21:08:02, reed1 wrote:
> Very good suggestion about moving flag to context -- I *think* its easy to
know
> where to cancel-out the exact request -- in filterInput(). Thanks.

I think you'll need to do it in filterInputGPU() as well, since GPU
implementations
short-circuit filterInput() in that case.

(There's also a call to canFilterImageGPU() / filterImageGPU() in that method
which should probably go away, but that's orthogonal to your change. Just make
sure you pass the modified Context there as well.)

[commit-bot: I haz the power, 2015-10-21 22:16:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-21 22:16:03]

Dry run: This issue passed the CQ dry run.

[Stephen White, 2015-10-21 22:25:09]

https://codereview.chromium.org/1421493003/diff/120001/include/core/SkImageFi...
File include/core/SkImageFilter.h (right):

https://codereview.chromium.org/1421493003/diff/120001/include/core/SkImageFi...
include/core/SkImageFilter.h:84: GrTextureProvider::SizeConstraint
grSizeConstraint() const;
Could we make this a static or unnamed-namespace utility function in the .cpp,
so we don't need an #ifdef/#include in the header?

https://codereview.chromium.org/1421493003/diff/120001/include/core/SkImageFi...
include/core/SkImageFilter.h:380: SkBitmap* result, SkIPoint* offset, bool
relaxSizeConstraint = true) const;
This does seem a little fragile: seems like we'll be constantly tracking down
places where we have to prevent the relaxing of the size constraint.

But I suppose the assert in SkImage that tex size == image size should catch
them as they occur.

https://codereview.chromium.org/1421493003/diff/120001/src/core/SkImageFilter...
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/120001/src/core/SkImageFilter...
src/core/SkImageFilter.cpp:275: context.grSizeConstraint() ==
GrTextureProvider::kExact_SizeConstraint)
Could we compare against sizeConstraint(), so we can remove the #if around this?

[reed1, 2015-10-22 14:24:52]

https://codereview.chromium.org/1421493003/diff/120001/include/core/SkImageFi...
File include/core/SkImageFilter.h (right):

https://codereview.chromium.org/1421493003/diff/120001/include/core/SkImageFi...
include/core/SkImageFilter.h:84: GrTextureProvider::SizeConstraint
grSizeConstraint() const;
On 2015/10/21 22:25:09, Stephen White wrote:
> Could we make this a static or unnamed-namespace utility function in the .cpp,
> so we don't need an #ifdef/#include in the header?

Done.

https://codereview.chromium.org/1421493003/diff/120001/include/core/SkImageFi...
include/core/SkImageFilter.h:380: SkBitmap* result, SkIPoint* offset, bool
relaxSizeConstraint = true) const;
On 2015/10/21 22:25:09, Stephen White wrote:
> This does seem a little fragile: seems like we'll be constantly tracking down
> places where we have to prevent the relaxing of the size constraint.
> 
> But I suppose the assert in SkImage that tex size == image size should catch
> them as they occur.

Agree on both counts. This is a case where we have an idea of the "optimal"
usage of exactly-sized textures, but its not obvious how to completely catch
every case. I suggest this is like most optimization tasks: we do what is
obvious up front, and then time/measure from then on, catching new
cases/opportunities as we discover them.

https://codereview.chromium.org/1421493003/diff/120001/src/core/SkImageFilter...
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/120001/src/core/SkImageFilter...
src/core/SkImageFilter.cpp:275: context.grSizeConstraint() ==
GrTextureProvider::kExact_SizeConstraint)
On 2015/10/21 22:25:09, Stephen White wrote:
> Could we compare against sizeConstraint(), so we can remove the #if around
this?

Done.

[reed1, 2015-10-22 14:28:39]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-22 14:28:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/140001

[reed1, 2015-10-22 14:30:33]

Have plumbed in the relax option to filterImageGPU (good suggestion).

Inspecting all call-site (so far), they all would pass true, so I made that the
default arg value, and didn't bother to update the sites explicitly.
Fortunately, this is the "fast" case we want, so if I missed one where I should
have passed "false" (i.e. slow), it should get caught by the final ASSERT in the
image constructor.

[commit-bot: I haz the power, 2015-10-22 14:42:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-22 14:42:45]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2015-10-22 14:43:36]

https://codereview.chromium.org/1421493003/diff/140001/gyp/core.gyp
File gyp/core.gyp (right):

https://codereview.chromium.org/1421493003/diff/140001/gyp/core.gyp#newcode109
gyp/core.gyp:109: '../include/core',
Do we still need this ?

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkMaskFil...
File include/core/SkMaskFilter.h (right):

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkMaskFil...
include/core/SkMaskFilter.h:149: bool canOverwriteSrc,
Can this be the ImageFilter enum ?

[Stephen White, 2015-10-22 15:25:04]

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkImageFi...
File include/core/SkImageFilter.h (right):

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkImageFi...
include/core/SkImageFilter.h:61: Context(const Context& src, SizeConstraint
constraint)
This is a convenience for two call sites. I'd prefer that it was a helper
function in the .cpp rather than polluting the header.

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkImageFi...
include/core/SkImageFilter.h:67: Context(const Context& src, const SkMatrix&
ctm)
Same here.

https://codereview.chromium.org/1421493003/diff/140001/src/core/SkImageFilter...
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/140001/src/core/SkImageFilter...
src/core/SkImageFilter.cpp:273: #if SK_SUPPORT_GPU
Now that we're no longer using GPU-specific enums, can we remove this #if?

https://codereview.chromium.org/1421493003/diff/140001/src/effects/SkBlurMask...
File src/effects/SkBlurMaskFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/140001/src/effects/SkBlurMask...
src/effects/SkBlurMaskFilter.cpp:1240: constraint);
Just hardcode this to kApproxMatch here, and avoid all the plumbing?

https://codereview.chromium.org/1421493003/diff/140001/src/gpu/GrBlurUtils.cpp
File src/gpu/GrBlurUtils.cpp (right):

https://codereview.chromium.org/1421493003/diff/140001/src/gpu/GrBlurUtils.cp...
src/gpu/GrBlurUtils.cpp:159: GrTextureProvider::SizeConstraint constraint) {
Could we remove this, and just change the parameter to GaussianBlur()?

[reed1, 2015-10-22 16:24:55]

https://codereview.chromium.org/1421493003/diff/140001/gyp/core.gyp
File gyp/core.gyp (right):

https://codereview.chromium.org/1421493003/diff/140001/gyp/core.gyp#newcode109
gyp/core.gyp:109: '../include/core',
On 2015/10/22 14:43:36, robertphillips wrote:
> Do we still need this ?

Not sure, will test.

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkImageFi...
File include/core/SkImageFilter.h (right):

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkImageFi...
include/core/SkImageFilter.h:61: Context(const Context& src, SizeConstraint
constraint)
On 2015/10/22 15:25:04, Stephen White wrote:
> This is a convenience for two call sites. I'd prefer that it was a helper
> function in the .cpp rather than polluting the header.

Done.

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkImageFi...
include/core/SkImageFilter.h:67: Context(const Context& src, const SkMatrix&
ctm)
On 2015/10/22 15:25:04, Stephen White wrote:
> Same here.

Done.

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkMaskFil...
File include/core/SkMaskFilter.h (right):

https://codereview.chromium.org/1421493003/diff/140001/include/core/SkMaskFil...
include/core/SkMaskFilter.h:149: bool canOverwriteSrc,
On 2015/10/22 14:43:36, robertphillips wrote:
> Can this be the ImageFilter enum ?

Removed.

https://codereview.chromium.org/1421493003/diff/140001/src/core/SkImageFilter...
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/140001/src/core/SkImageFilter...
src/core/SkImageFilter.cpp:273: #if SK_SUPPORT_GPU
On 2015/10/22 15:25:04, Stephen White wrote:
> Now that we're no longer using GPU-specific enums, can we remove this #if?

Done. Good catch.

https://codereview.chromium.org/1421493003/diff/140001/src/effects/SkBlurMask...
File src/effects/SkBlurMaskFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/140001/src/effects/SkBlurMask...
src/effects/SkBlurMaskFilter.cpp:1240: constraint);
On 2015/10/22 15:25:04, Stephen White wrote:
> Just hardcode this to kApproxMatch here, and avoid all the plumbing?

Doh, great catch. Didn't realize the output of this method never "escaped" to
become a real boy. Done.

https://codereview.chromium.org/1421493003/diff/140001/src/gpu/GrBlurUtils.cpp
File src/gpu/GrBlurUtils.cpp (right):

https://codereview.chromium.org/1421493003/diff/140001/src/gpu/GrBlurUtils.cp...
src/gpu/GrBlurUtils.cpp:159: GrTextureProvider::SizeConstraint constraint) {
On 2015/10/22 15:25:04, Stephen White wrote:
> Could we remove this, and just change the parameter to GaussianBlur()?

Removed

[reed1, 2015-10-22 16:39:08]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-22 16:39:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/180001

[reed1, 2015-10-22 16:40:01]

The CQ bit was unchecked by reed@google.com

[reed1, 2015-10-22 18:27:39]

ptal

[Stephen White, 2015-10-22 18:44:16]

https://codereview.chromium.org/1421493003/diff/180001/gyp/core.gyp
File gyp/core.gyp (right):

https://codereview.chromium.org/1421493003/diff/180001/gyp/core.gyp#newcode110
gyp/core.gyp:110: '../include/gpu',
Is this still necessary, now that it's GrTexture.h depending on SkImageFilter.h
(which is much better, BTW), and not the reverse?

https://codereview.chromium.org/1421493003/diff/180001/src/core/SkImageFilter...
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/180001/src/core/SkImageFilter...
src/core/SkImageFilter.cpp:273: if (relaxSizeConstraint &&
(kExact_SizeConstraint == context.sizeConstraint())) {
Nit: could you rewrite this one to look more like filterInputGPU()?

https://codereview.chromium.org/1421493003/diff/180001/src/gpu/GrBlurUtils.h
File src/gpu/GrBlurUtils.h (right):

https://codereview.chromium.org/1421493003/diff/180001/src/gpu/GrBlurUtils.h#...
src/gpu/GrBlurUtils.h:11: #include "GrTextureProvider.h"
Remove this?

[reed1, 2015-10-22 19:36:32]

https://codereview.chromium.org/1421493003/diff/180001/gyp/core.gyp
File gyp/core.gyp (right):

https://codereview.chromium.org/1421493003/diff/180001/gyp/core.gyp#newcode110
gyp/core.gyp:110: '../include/gpu',
On 2015/10/22 18:44:16, Stephen White wrote:
> Is this still necessary, now that it's GrTexture.h depending on
SkImageFilter.h
> (which is much better, BTW), and not the reverse?

Done.

https://codereview.chromium.org/1421493003/diff/180001/src/core/SkImageFilter...
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/180001/src/core/SkImageFilter...
src/core/SkImageFilter.cpp:273: if (relaxSizeConstraint &&
(kExact_SizeConstraint == context.sizeConstraint())) {
On 2015/10/22 18:44:16, Stephen White wrote:
> Nit: could you rewrite this one to look more like filterInputGPU()?

Done.

https://codereview.chromium.org/1421493003/diff/180001/src/gpu/GrBlurUtils.h
File src/gpu/GrBlurUtils.h (right):

https://codereview.chromium.org/1421493003/diff/180001/src/gpu/GrBlurUtils.h#...
src/gpu/GrBlurUtils.h:11: #include "GrTextureProvider.h"
On 2015/10/22 18:44:16, Stephen White wrote:
> Remove this?

Thanks, missed this on the prev cleanup.

[reed1, 2015-10-22 19:36:43]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-22 19:36:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/200001

[Stephen White, 2015-10-22 19:53:14]

It would be great to have some benches for this, but given the, er, "in
progress" state of our GPU benchmarking, we can skip it for now.

LGTM (comments are nits)

https://codereview.chromium.org/1421493003/diff/200001/src/effects/SkBlurImag...
File src/effects/SkBlurImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/200001/src/effects/SkBlurImag...
src/effects/SkBlurImageFilter.cpp:222:
GrTextureProvider::FromImageFilter(ctx.sizeConstraint())));
Nit: weird formatting here.

https://codereview.chromium.org/1421493003/diff/200001/src/effects/SkMorpholo...
File src/effects/SkMorphologyImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/200001/src/effects/SkMorpholo...
src/effects/SkMorphologyImageFilter.cpp:571: // Optimization: we will fall
through and allocate the "real" texture after this one
For bonus points in a followup patch, do a similar optimization in the blur
image filter (for the easy and fairly common case where we have to do a bilinear
upres after blurring: scaleFactorX > 1 || scaleFactorY > 1).

[reed1, 2015-10-22 20:08:26]

https://codereview.chromium.org/1421493003/diff/200001/src/effects/SkBlurImag...
File src/effects/SkBlurImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/200001/src/effects/SkBlurImag...
src/effects/SkBlurImageFilter.cpp:222:
GrTextureProvider::FromImageFilter(ctx.sizeConstraint())));
On 2015/10/22 19:53:14, Stephen White wrote:
> Nit: weird formatting here.

Done.

https://codereview.chromium.org/1421493003/diff/200001/src/effects/SkMorpholo...
File src/effects/SkMorphologyImageFilter.cpp (right):

https://codereview.chromium.org/1421493003/diff/200001/src/effects/SkMorpholo...
src/effects/SkMorphologyImageFilter.cpp:571: // Optimization: we will fall
through and allocate the "real" texture after this one
On 2015/10/22 19:53:14, Stephen White wrote:
> For bonus points in a followup patch, do a similar optimization in the blur
> image filter (for the easy and fairly common case where we have to do a
bilinear
> upres after blurring: scaleFactorX > 1 || scaleFactorY > 1).

Indeed. I looked at the blur code, but couldn't see a clear (finite) way to
preflight the predicate to know when which would be scratch. I agree that should
be done.

[reed1, 2015-10-22 20:08:34]

The CQ bit was checked by reed@google.com

[reed1, 2015-10-22 20:08:35]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com,
robertphillips@google.com, senorblanco@chromium.org
Link to the patchset: https://codereview.chromium.org/1421493003/#ps220001
(title: "fix formating")

[commit-bot: I haz the power, 2015-10-22 20:08:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1421493003/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1421493003/220001

[commit-bot: I haz the power, 2015-10-22 20:20:23]

Committed patchset #12 (id:220001) as
https://skia.googlesource.com/skia/+/c9b5f8b1522e72449d704d30ed6aee4fc6211ee8