Upgrade SkSpecialImage to have getTextureRef & getROPixels entry points

Upgrade SkSpecialImage to have getTextureRef & getROPixels entry points

This more closely aligns the SkSpecialImage API with the SkImage API. In doing so it allows the image filters to handle SkImages that can sneak through while remaining encoded (e.g., if an input filter just returns a wrapped version of the source SkImage)

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

Committed: https://skia.googlesource.com/skia/+/646125114b42b24e5ada3c9f8fac53a85f9ad2a0

[robertphillips, 2016-04-05 16:52:11]

Description was changed from

==========
Upgrade SkSpecialImage to have getTextureRef & getROPixels entry points
==========

to

==========
Upgrade SkSpecialImage to have getTextureRef & getROPixels entry points
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[robertphillips, 2016-04-05 17:03:02]

Description was changed from

==========
Upgrade SkSpecialImage to have getTextureRef & getROPixels entry points
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Upgrade SkSpecialImage to have getTextureRef & getROPixels entry points

This more closely aligns the SkSpecialImage API with the SkImage API. In doing
so it allows the image filters to handle SkImages that can sneak through while
remaining encoded (e.g., if an input filter just returns a wrapped version of
the source SkImage)

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

[robertphillips, 2016-04-05 17:03:02]

robertphillips@google.com changed reviewers:
+ reed@google.com

[robertphillips, 2016-04-05 22:15:39]

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

[commit-bot: I haz the power, 2016-04-05 22:15:45]

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

[commit-bot: I haz the power, 2016-04-05 22:20:34]

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

[commit-bot: I haz the power, 2016-04-05 22:20:35]

Dry run: Try jobs failed on following builders:
  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...)

[robertphillips, 2016-04-06 12:07:32]

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

[commit-bot: I haz the power, 2016-04-06 12:07:35]

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

[commit-bot: I haz the power, 2016-04-06 12:09:04]

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

[commit-bot: I haz the power, 2016-04-06 12:09:05]

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

[robertphillips, 2016-04-06 14:28:18]

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

[commit-bot: I haz the power, 2016-04-06 14:28:23]

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

[commit-bot: I haz the power, 2016-04-06 14:43:52]

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

[commit-bot: I haz the power, 2016-04-06 14:43:53]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Release-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU...)

[Stephen White, 2016-04-06 16:21:23]

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

[Stephen White, 2016-04-06 16:21:24]

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

https://codereview.chromium.org/1861643003/diff/60001/src/effects/SkMorpholog...
src/effects/SkMorphologyImageFilter.cpp:555: if (!inputTexture) {
I find this API somewhat error-prone, in that it's not clear when you have to
call peekTexture() and when you call asTextureRef() (and the fact that you don't
use the result of the latter call). Could we bake this call into filterInput()
or (better) filterImage(), so the onus is not on each filter to do it?

[robertphillips, 2016-04-06 17:19:53]

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

https://codereview.chromium.org/1861643003/diff/60001/src/effects/SkMorpholog...
src/effects/SkMorphologyImageFilter.cpp:555: if (!inputTexture) {
I have updated the SkSpecialImage API to cleave more closely to SkImage (i.e.,
peekTexture has been replaced with isTextureBacked).

Wrt baking this into filterInput or filterImage, the image filters that have a
GPU-specific special case are going to have to burrow into the SkSpecialImage no
matter what. So we'll need something here. I agree that we should always force
all images in the ImageFilter cache to be GrTexture-backed when targeting an
SkGpuDevice but I think that would be a separate CL.

[robertphillips, 2016-04-06 17:42:33]

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

[commit-bot: I haz the power, 2016-04-06 17:42:36]

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

[commit-bot: I haz the power, 2016-04-06 17:44:06]

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

[commit-bot: I haz the power, 2016-04-06 17:44:07]

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

[robertphillips, 2016-04-06 18:11:26]

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

[commit-bot: I haz the power, 2016-04-06 18:11:32]

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

[commit-bot: I haz the power, 2016-04-06 19:05:47]

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

[commit-bot: I haz the power, 2016-04-06 19:05:48]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-04-08 15:11:06]

fine with new isTextureBacked addition, makes it clearer. lgtm

[Stephen White, 2016-04-08 15:14:19]

LGTM w/nit

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

https://codereview.chromium.org/1861643003/diff/100001/src/effects/SkBlurImag...
src/effects/SkBlurImageFilter.cpp:101: if (!inputTexture) {
Could you change this to an assert? It should be handled by filterInput(), no?

[robertphillips, 2016-04-08 18:07:50]

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

https://codereview.chromium.org/1861643003/diff/100001/src/effects/SkBlurImag...
src/effects/SkBlurImageFilter.cpp:101: if (!inputTexture) {
On 2016/04/08 15:14:19, Stephen White wrote:
> Could you change this to an assert? It should be handled by filterInput(), no?

Done.

https://codereview.chromium.org/1861643003/diff/100001/src/effects/SkBlurImag...
src/effects/SkBlurImageFilter.cpp:101: if (!inputTexture) {
On 2016/04/08 15:14:19, Stephen White wrote:
> Could you change this to an assert? It should be handled by filterInput(), no?

Done.

[robertphillips, 2016-04-08 18:07:57]

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

[commit-bot: I haz the power, 2016-04-08 18:08:08]

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

[Stephen White, 2016-04-08 18:23:20]

some nits I missed.

https://codereview.chromium.org/1861643003/diff/120001/src/core/SkSpecialImag...
File src/core/SkSpecialImage.cpp (right):

https://codereview.chromium.org/1861643003/diff/120001/src/core/SkSpecialImag...
src/core/SkSpecialImage.cpp:116: return nullptr;
Nit: return false?

https://codereview.chromium.org/1861643003/diff/120001/src/core/SkSpecialImag...
src/core/SkSpecialImage.cpp:377: if (!context) {
micronit: for brevity, this could be

#if SK_SUPPORT_GPU
if (context) {
   return GrRef...
}
#endif

return nullptr;

[commit-bot: I haz the power, 2016-04-08 18:26:58]

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

[commit-bot: I haz the power, 2016-04-08 18:26:59]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2016-04-08 18:31:31]

The CQ bit was checked by robertphillips@google.com

[robertphillips, 2016-04-08 18:31:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
senorblanco@chromium.org, reed@google.com
Link to the patchset: https://codereview.chromium.org/1861643003/#ps120001
(title: "Address code review comments")

[commit-bot: I haz the power, 2016-04-08 18:31:41]

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

[robertphillips, 2016-04-08 18:32:16]

The CQ bit was unchecked by robertphillips@google.com

[robertphillips, 2016-04-08 18:57:50]

https://codereview.chromium.org/1861643003/diff/120001/src/core/SkSpecialImag...
File src/core/SkSpecialImage.cpp (right):

https://codereview.chromium.org/1861643003/diff/120001/src/core/SkSpecialImag...
src/core/SkSpecialImage.cpp:116: return nullptr;
On 2016/04/08 18:23:20, Stephen White wrote:
> Nit: return false?

Done.

https://codereview.chromium.org/1861643003/diff/120001/src/core/SkSpecialImag...
src/core/SkSpecialImage.cpp:377: if (!context) {
On 2016/04/08 18:23:20, Stephen White wrote:
> micronit: for brevity, this could be
> 
> #if SK_SUPPORT_GPU
> if (context) {
>    return GrRef...
> }
> #endif
> 
> return nullptr;

Done.

[robertphillips, 2016-04-08 18:57:57]

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

[commit-bot: I haz the power, 2016-04-08 18:58:02]

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

[commit-bot: I haz the power, 2016-04-08 19:08:36]

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

[commit-bot: I haz the power, 2016-04-08 19:08:37]

Dry run: This issue passed the CQ dry run.

[robertphillips, 2016-04-08 19:09:38]

The CQ bit was checked by robertphillips@google.com

[robertphillips, 2016-04-08 19:09:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
senorblanco@chromium.org, reed@google.com
Link to the patchset: https://codereview.chromium.org/1861643003/#ps140001
(title: "Address code review comments")

[commit-bot: I haz the power, 2016-04-08 19:09:47]

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

[commit-bot: I haz the power, 2016-04-08 19:10:46]

Description was changed from

==========
Upgrade SkSpecialImage to have getTextureRef & getROPixels entry points

This more closely aligns the SkSpecialImage API with the SkImage API. In doing
so it allows the image filters to handle SkImages that can sneak through while
remaining encoded (e.g., if an input filter just returns a wrapped version of
the source SkImage)

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

to

==========
Upgrade SkSpecialImage to have getTextureRef & getROPixels entry points

This more closely aligns the SkSpecialImage API with the SkImage API. In doing
so it allows the image filters to handle SkImages that can sneak through while
remaining encoded (e.g., if an input filter just returns a wrapped version of
the source SkImage)

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

Committed:
https://skia.googlesource.com/skia/+/646125114b42b24e5ada3c9f8fac53a85f9ad2a0
==========

[commit-bot: I haz the power, 2016-04-08 19:10:47]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/646125114b42b24e5ada3c9f8fac53a85f9ad2a0