Add alphaType() to SkImage

Add alphaType() to SkImage

Keep isOpaque as a convenience method -- many places really only need to
know that for optimization purposes (SrcOver -> Src, etc...).

In all the places where we pull data back out or convert to another
object and need to supply an SkImageInfo, we can avoid losing information
about premulness.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250663002

Committed: https://skia.googlesource.com/skia/+/69c166d2ce3f4d2099f35f8e88f1aa56ceeaf960

[Brian Osman, 2016-08-16 15:28:22]

Description was changed from

==========
Add alphaType() to SkImage

Keep isOpaque as a convenience method -- many places really only need to
know that for optimization purposes (SrcOver -> Src, etc...).

In all the places where we pull data back out or convert to another
object and need to supply an SkImageInfo, we can avoid losing information
about premulness.

BUG=skia:
==========

to

==========
Add alphaType() to SkImage

Keep isOpaque as a convenience method -- many places really only need to
know that for optimization purposes (SrcOver -> Src, etc...).

In all the places where we pull data back out or convert to another
object and need to supply an SkImageInfo, we can avoid losing information
about premulness.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250663002
==========

[Brian Osman, 2016-08-16 15:28:48]

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

[commit-bot: I haz the power, 2016-08-16 15:28:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Brian Osman, 2016-08-16 15:35:29]

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

[Brian Osman, 2016-08-16 15:35:29]

Do we want to do this? I don't think it's necessary, but Brian's comments about
supporting non-premul images in the future led me down this path.

[commit-bot: I haz the power, 2016-08-16 15:45:07]

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

[commit-bot: I haz the power, 2016-08-16 15:45:07]

Dry run: This issue passed the CQ dry run.

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

https://codereview.chromium.org/2250663002/diff/1/tests/SurfaceTest.cpp
File tests/SurfaceTest.cpp (right):

https://codereview.chromium.org/2250663002/diff/1/tests/SurfaceTest.cpp#newco...
tests/SurfaceTest.cpp:142: for (auto& surface_func : { &create_surface,
&create_direct_surface }) {
add kUnpremul_SkAlphaType ?

https://codereview.chromium.org/2250663002/diff/1/tests/SurfaceTest.cpp#newco...
tests/SurfaceTest.cpp:151: for (auto& surface_func : { &create_gpu_surface,
&create_gpu_scratch_surface }) {
Here too ?

[Brian Osman, 2016-08-16 17:24:23]

https://codereview.chromium.org/2250663002/diff/1/tests/SurfaceTest.cpp
File tests/SurfaceTest.cpp (right):

https://codereview.chromium.org/2250663002/diff/1/tests/SurfaceTest.cpp#newco...
tests/SurfaceTest.cpp:142: for (auto& surface_func : { &create_surface,
&create_direct_surface }) {
On 2016/08/16 16:23:20, robertphillips wrote:
> add kUnpremul_SkAlphaType ?

Done.

https://codereview.chromium.org/2250663002/diff/1/tests/SurfaceTest.cpp#newco...
tests/SurfaceTest.cpp:151: for (auto& surface_func : { &create_gpu_surface,
&create_gpu_scratch_surface }) {
On 2016/08/16 16:23:20, robertphillips wrote:
> Here too ?

I originally wrote this testing all types in both tests, but got a bunch of test
failures because we can't create unpremul surfaces for GPU. I've modified this
to test all three types for raster, and added a comment to the GPU test
explaining the discrepancy.

[reed1, 2016-08-16 19:22:11]

reed@google.com changed reviewers:
+ fmalita@chromium.org

[reed1, 2016-08-16 19:22:12]

Still a little nervous describing this publicly, tho I don't have very clear
reasons.

What does it return for a PNG image (MakeFromEncoded)? Do we have a test? What
*should* it return? What should we return for Picture-backed generator?

https://codereview.chromium.org/2250663002/diff/20001/include/core/SkImage.h
File include/core/SkImage.h (right):

https://codereview.chromium.org/2250663002/diff/20001/include/core/SkImage.h#...
include/core/SkImage.h:169: virtual SkAlphaType alphaType() const = 0;
*if* we're gonna make changes here, no need for any of these to be explicitly
virtual (can be done in Image_Base)

[Brian Osman, 2016-08-16 19:52:47]

On 2016/08/16 19:22:12, reed1 wrote:
> Still a little nervous describing this publicly, tho I don't have very clear
> reasons.
> 
> What does it return for a PNG image (MakeFromEncoded)? Do we have a test? What
> *should* it return? What should we return for Picture-backed generator?

In all of these cases, it will return premul. SkCodecImageGenerator computes and
stores an info at construction time. It's based on the codec's info, but with
unpremul switched to premul. But because that decision is made and stored by the
codec generator, we could modify that in the future, and let different codecs
choose the most appropriate default format.

For picture-backed images, we explicitly set the info to N32Premul in the
constructor. In this case, I think that's the correct (conservative) answer.
If we're going to use the result of playback for raster/gpu, then we need
to play it back, and the mechanics of doing so (rendering) strongly suggests
premul.

[Brian Osman, 2016-08-17 18:32:17]

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

[commit-bot: I haz the power, 2016-08-17 18:32:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Brian Osman, 2016-08-17 18:33:23]

PTAL - No more virtuals in SkImage, and I shuffled things around to keep
onImageInfo and onAlphaType adjacent in all derived classes.

[reed1, 2016-08-17 19:06:09]

lgtm

[commit-bot: I haz the power, 2016-08-17 19:28:09]

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

[commit-bot: I haz the power, 2016-08-17 19:28:10]

Dry run: This issue passed the CQ dry run.

[Brian Osman, 2016-08-17 20:54:17]

The CQ bit was checked by brianosman@google.com

[commit-bot: I haz the power, 2016-08-17 20:54:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 21:01:09]

Description was changed from

==========
Add alphaType() to SkImage

Keep isOpaque as a convenience method -- many places really only need to
know that for optimization purposes (SrcOver -> Src, etc...).

In all the places where we pull data back out or convert to another
object and need to supply an SkImageInfo, we can avoid losing information
about premulness.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250663002
==========

to

==========
Add alphaType() to SkImage

Keep isOpaque as a convenience method -- many places really only need to
know that for optimization purposes (SrcOver -> Src, etc...).

In all the places where we pull data back out or convert to another
object and need to supply an SkImageInfo, we can avoid losing information
about premulness.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250663002

Committed:
https://skia.googlesource.com/skia/+/69c166d2ce3f4d2099f35f8e88f1aa56ceeaf960
==========

[commit-bot: I haz the power, 2016-08-17 21:01:10]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/69c166d2ce3f4d2099f35f8e88f1aa56ceeaf960