Begin removing deprecated (and now, unused) ImageFilter code paths

Begin removing deprecated (and now, unused) ImageFilter code paths

This CL focuses on removing the Proxy objects but takes some other stuff with it.

BUG=skia:4965
TBR=reed@google.com

GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1896383003

Committed: https://skia.googlesource.com/skia/+/3e302275b324172c845627cbd00cee8a06571baf

[robertphillips, 2016-04-20 15:52:45]

Description was changed from

==========
Begin removing deprecated (and now, unused) ImageFilter code paths
==========

to

==========
Begin removing deprecated (and now, unused) ImageFilter code paths
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[robertphillips, 2016-04-20 15:57:12]

Description was changed from

==========
Begin removing deprecated (and now, unused) ImageFilter code paths
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Begin removing deprecated (and now, unused) ImageFilter code paths

BUG=skia:4965

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[robertphillips, 2016-04-20 16:08:16]

Description was changed from

==========
Begin removing deprecated (and now, unused) ImageFilter code paths

BUG=skia:4965

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Begin removing deprecated (and now, unused) ImageFilter code paths

This CL focuses on removing the Proxy objects but takes some other stuff with
it.

BUG=skia:4965

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[robertphillips, 2016-04-20 16:08:16]

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

[robertphillips, 2016-04-20 16:08:33]

Description was changed from

==========
Begin removing deprecated (and now, unused) ImageFilter code paths

This CL focuses on removing the Proxy objects but takes some other stuff with
it.

BUG=skia:4965

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Begin removing deprecated (and now, unused) ImageFilter code paths

This CL focuses on removing the Proxy objects but takes some other stuff with
it.

BUG=skia:4965
TBR=reed@google.com

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[robertphillips, 2016-04-20 16:24:11]

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

[commit-bot: I haz the power, 2016-04-20 16:24:21]

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

[reed1, 2016-04-20 16:25:16]

lgtm

[commit-bot: I haz the power, 2016-04-20 16:25:51]

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

[commit-bot: I haz the power, 2016-04-20 16:25:52]

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

[Stephen White, 2016-04-20 16:53:14]

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

https://codereview.chromium.org/1896383003/diff/20001/include/core/SkImageFil...
include/core/SkImageFilter.h:295: *  If the result image cannot be created, this
should return nullptr, in which
We should probably add something to the effect that nullptr can also mean "fill
to the clip with transparent black".

https://codereview.chromium.org/1896383003/diff/20001/tests/ImageFilterTest.cpp
File tests/ImageFilterTest.cpp (right):

https://codereview.chromium.org/1896383003/diff/20001/tests/ImageFilterTest.c...
tests/ImageFilterTest.cpp:621: const SkSurfaceProps
props(SkSurfaceProps::kLegacyFontHost_InitType);
Remove this?

https://codereview.chromium.org/1896383003/diff/20001/tests/ImageFilterTest.c...
tests/ImageFilterTest.cpp:623: const SkImageInfo info =
SkImageInfo::MakeN32Premul(widthHeight, widthHeight);
and this?

https://codereview.chromium.org/1896383003/diff/20001/tests/ImageFilterTest.c...
tests/ImageFilterTest.cpp:633: const SkSurfaceProps
props(SkSurfaceProps::kLegacyFontHost_InitType);
and this?

[robertphillips, 2016-04-20 17:50:20]

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

https://codereview.chromium.org/1896383003/diff/20001/include/core/SkImageFil...
include/core/SkImageFilter.h:295: *  If the result image cannot be created, this
should return nullptr, in which
On 2016/04/20 16:53:13, Stephen White wrote:
> We should probably add something to the effect that nullptr can also mean
"fill
> to the clip with transparent black".

I've taken a stab at an informative comment.

https://codereview.chromium.org/1896383003/diff/20001/tests/ImageFilterTest.cpp
File tests/ImageFilterTest.cpp (right):

https://codereview.chromium.org/1896383003/diff/20001/tests/ImageFilterTest.c...
tests/ImageFilterTest.cpp:621: const SkSurfaceProps
props(SkSurfaceProps::kLegacyFontHost_InitType);
On 2016/04/20 16:53:13, Stephen White wrote:
> Remove this?

Done.

https://codereview.chromium.org/1896383003/diff/20001/tests/ImageFilterTest.c...
tests/ImageFilterTest.cpp:623: const SkImageInfo info =
SkImageInfo::MakeN32Premul(widthHeight, widthHeight);
On 2016/04/20 16:53:13, Stephen White wrote:
> and this?

Done.

https://codereview.chromium.org/1896383003/diff/20001/tests/ImageFilterTest.c...
tests/ImageFilterTest.cpp:633: const SkSurfaceProps
props(SkSurfaceProps::kLegacyFontHost_InitType);
On 2016/04/20 16:53:13, Stephen White wrote:
> and this?

Done.

[robertphillips, 2016-04-20 17:50:25]

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

[commit-bot: I haz the power, 2016-04-20 17:50:31]

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

[Stephen White, 2016-04-20 17:59:39]

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

https://codereview.chromium.org/1896383003/diff/40001/include/core/SkImageFil...
include/core/SkImageFilter.h:295: *  If the result image cannot be created
(either bc of error or if, say, the result
Nit: bc -> due to

We should also re-add something about ignoring offset in the nullptr case.

https://codereview.chromium.org/1896383003/diff/40001/tests/ImageFilterTest.cpp
File tests/ImageFilterTest.cpp (right):

https://codereview.chromium.org/1896383003/diff/40001/tests/ImageFilterTest.c...
tests/ImageFilterTest.cpp:1074: DEF_TEST(ImageFilterMatrixConvolutionBigKernel,
reporter) {
Not in this patch, but maybe we should have another meta-macro that calls both
the CPU and GPU backends (if defined). Something like:

#if SK_SUPPORT_GPU
#define DEF_CPUGPU_TEST(name, func, reporter) \
DEF_TEST(name, reporter) { \
  func(reporter, nullptr); \
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(name##_GPU, reporter, ctxInfo) { \
  func(reporter, ctxInfo.fGrContext); \
}
#else
#define DEF_CPU_AND_GPU_TEST(name, func, reporter) \
DEF_TEST(name, reporter) { \
  func(reporter, nullptr); \
}
#endif

[Stephen White, 2016-04-20 18:00:44]

On 2016/04/20 17:59:39, Stephen White wrote:
> Not in this patch, but maybe we should have another meta-macro that calls both
> the CPU and GPU backends (if defined). Something like:
> 
> #if SK_SUPPORT_GPU
> #define DEF_CPUGPU_TEST(name, func, reporter) \
> DEF_TEST(name, reporter) { \
>   func(reporter, nullptr); \
> }
> DEF_GPUTEST_FOR_RENDERING_CONTEXTS(name##_GPU, reporter, ctxInfo) { \
>   func(reporter, ctxInfo.fGrContext); \
> }
> #else
> #define DEF_CPU_AND_GPU_TEST(name, func, reporter) \
> DEF_TEST(name, reporter) { \
>   func(reporter, nullptr); \
> }
> #endif

er, more like the below, but you get the idea.

#if SK_SUPPORT_GPU
#define DEF_CPUGPU_TEST(name, func, reporter) \
DEF_TEST(name, reporter) { \
  func(reporter, nullptr); \
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(name##_GPU, reporter, ctxInfo) { \
  func(reporter, ctxInfo.fGrContext); \
}
#else
#define DEF_CPUGPU_TEST(name, func, reporter) \
DEF_TEST(name, reporter) { \
  func(reporter, nullptr); \
}
#endif

[commit-bot: I haz the power, 2016-04-20 18:03:11]

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

[commit-bot: I haz the power, 2016-04-20 18:03:13]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2016-04-20 18:27:58]

On 2016/04/20 18:00:44, Stephen White wrote:
> On 2016/04/20 17:59:39, Stephen White wrote:
> > Not in this patch, but maybe we should have another meta-macro that calls
both
> > the CPU and GPU backends (if defined). Something like:
> > 
> > #if SK_SUPPORT_GPU
> > #define DEF_CPUGPU_TEST(name, func, reporter) \
> > DEF_TEST(name, reporter) { \
> >   func(reporter, nullptr); \
> > }
> > DEF_GPUTEST_FOR_RENDERING_CONTEXTS(name##_GPU, reporter, ctxInfo) { \
> >   func(reporter, ctxInfo.fGrContext); \
> > }
> > #else
> > #define DEF_CPU_AND_GPU_TEST(name, func, reporter) \
> > DEF_TEST(name, reporter) { \
> >   func(reporter, nullptr); \
> > }
> > #endif
> 
> er, more like the below, but you get the idea.
> 
> #if SK_SUPPORT_GPU
> #define DEF_CPUGPU_TEST(name, func, reporter) \
> DEF_TEST(name, reporter) { \
>   func(reporter, nullptr); \
> }
> DEF_GPUTEST_FOR_RENDERING_CONTEXTS(name##_GPU, reporter, ctxInfo) { \
>   func(reporter, ctxInfo.fGrContext); \
> }
> #else
> #define DEF_CPUGPU_TEST(name, func, reporter) \
> DEF_TEST(name, reporter) { \
>   func(reporter, nullptr); \
> }
> #endif

Yep - that would be nice.

[robertphillips, 2016-04-20 18:28:52]

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

https://codereview.chromium.org/1896383003/diff/40001/include/core/SkImageFil...
include/core/SkImageFilter.h:295: *  If the result image cannot be created
(either bc of error or if, say, the result
On 2016/04/20 17:59:39, Stephen White wrote:
> Nit: bc -> due to
> 
> We should also re-add something about ignoring offset in the nullptr case.

Done.

[Stephen White, 2016-04-20 18:31:28]

LGTM.

Awesome. \o/

[robertphillips, 2016-04-20 18:32:29]

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

[commit-bot: I haz the power, 2016-04-20 18:32:33]

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

[commit-bot: I haz the power, 2016-04-20 18:46:11]

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

[commit-bot: I haz the power, 2016-04-20 18:46:12]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2016-04-20 18:47:34]

The CQ bit was checked by robertphillips@google.com

[robertphillips, 2016-04-20 18:47:34]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1896383003/#ps60001
(title: "Fix comment")

[commit-bot: I haz the power, 2016-04-20 18:47:41]

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

[commit-bot: I haz the power, 2016-04-20 18:48:39]

Description was changed from

==========
Begin removing deprecated (and now, unused) ImageFilter code paths

This CL focuses on removing the Proxy objects but takes some other stuff with
it.

BUG=skia:4965
TBR=reed@google.com

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Begin removing deprecated (and now, unused) ImageFilter code paths

This CL focuses on removing the Proxy objects but takes some other stuff with
it.

BUG=skia:4965
TBR=reed@google.com

GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/3e302275b324172c845627cbd00cee8a06571baf
==========

[commit-bot: I haz the power, 2016-04-20 18:48:40]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/3e302275b324172c845627cbd00cee8a06571baf