Only decode opaque to premul in DM image

Only decode opaque to premul in DM image

Previously, we would test decoding an opaque SkCodec to all three:
kUnpremul,
kPremul,
kOpaque

The image should look the same in all three cases. We already test for
that in CodecTest [1], where we require that the result matches exactly.
CodecTest runs on a smaller set of images, but it covers a variety of
opaque images. No need to test on all opaque images.

Running locally on my Mac laptop, the following command:

    dm --src image --images resources/

dropped from:
5932 srcs
11.2s

to:

3544 srcs
6.69s

for a 40% speedup.

BUG=skia:5307
[1] https://skia.googlesource.com/skia/+/master/tests/CodecTest.cpp#119
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1999593003

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

[scroggo_chromium, 2016-05-20 14:44:45]

Description was changed from

==========
Stop testing decoding to opaque in DM image

This is already tested in CodecTest [1], where we require that the
result matches exactly. CodecTest runs on a smaller set of images, but
it covers a variety of opaque images. No need to test on all opaque
images.

Running locally on my Mac laptop, the following command:

    dm --src image --images resources/

dropped from:
11063 srcs
13.2s

to:

9084 srcs
9.26s

for a 30% speedup.

BUG=skia:5307

[1] https://skia.googlesource.com/skia/+/master/tests/CodecTest.cpp#119
==========

to

==========
Stop testing decoding to opaque in DM image

This is already tested in CodecTest [1], where we require that the
result matches exactly. CodecTest runs on a smaller set of images, but
it covers a variety of opaque images. No need to test on all opaque
images.

Running locally on my Mac laptop, the following command:

    dm --src image --images resources/

dropped from:
11063 srcs
13.2s

to:

9084 srcs
9.26s

for a 30% speedup.

BUG=skia:5307

[1] https://skia.googlesource.com/skia/+/master/tests/CodecTest.cpp#119
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[scroggo_chromium, 2016-05-20 14:52:40]

scroggo@chromium.org changed reviewers:
+ msarett@google.com

[scroggo_chromium, 2016-05-20 14:52:40]

Maybe a more exciting difference here is that we'll reduce the time decoding RAW
images. I just tried running

  dm --src image --images HTC.dng

and the time dropped from 47s to 27.8, for a 40% speedup. Not only was this a
bigger percentage (which makes sense - some of the images in resources have
transparency, so they wouldn't have decoded to opaque anyway), but RAW images
take a long time period, so this will be a good place to shave off time.

My only concern here is BMP. I think you said we had a BMP that used to
incorrectly decode differently depending on the alpha type. I'd feel better if
we had an image in resources run through CodecTest that checks for that
regression. (Do we? Or just in the images used for DM?)

[msarett, 2016-05-20 15:19:47]

On 2016/05/20 14:52:40, scroggo_chromium wrote:
> Maybe a more exciting difference here is that we'll reduce the time decoding
RAW
> images. I just tried running
> 
>   dm --src image --images HTC.dng
> 
> and the time dropped from 47s to 27.8, for a 40% speedup. Not only was this a
> bigger percentage (which makes sense - some of the images in resources have
> transparency, so they wouldn't have decoded to opaque anyway), but RAW images
> take a long time period, so this will be a good place to shave off time.
> 
> My only concern here is BMP. I think you said we had a BMP that used to
> incorrectly decode differently depending on the alpha type. I'd feel better if
> we had an image in resources run through CodecTest that checks for that
> regression. (Do we? Or just in the images used for DM?)

I feel good about this.  We had issues when decoding "opaque" bmps to
premul/unpremul - because looking at the dstInfo made us think it wasn't opaque.
 Removing the opaque to opaque tests (which are non-confusing) and keeping the
more interesting cases SGTM.

We don't have an interesting image in resources...  I'll see if we can get one,
but I don't think it's critical.

LGTM

[scroggo, 2016-05-20 15:23:44]

The CQ bit was checked by scroggo@google.com

[commit-bot: I haz the power, 2016-05-20 15:23:50]

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

[commit-bot: I haz the power, 2016-05-20 15:41:51]

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

[commit-bot: I haz the power, 2016-05-20 15:41:52]

Failed to apply patch for dm/DM.cpp:
While running git apply --index -3 -p1;
  error: patch failed: dm/DM.cpp:521
  Falling back to three-way merge...
  Applied patch to 'dm/DM.cpp' with conflicts.
  U dm/DM.cpp

Patch:       dm/DM.cpp
Index: dm/DM.cpp
diff --git a/dm/DM.cpp b/dm/DM.cpp
index
8685ec55670a669ddf545ab7bb5b3bca41ce7101..ad96d9a834878df5b2d125ce96ddde8a6c012d77
100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -373,9 +373,6 @@ static void push_codec_src(Path path, CodecSrc::Mode mode,
CodecSrc::DstColorTyp
     }
 
     switch (dstAlphaType) {
-        case kOpaque_SkAlphaType:
-            folder.append("_opaque");
-            break;
         case kPremul_SkAlphaType:
             folder.append("_premul");
             break;
@@ -414,9 +411,6 @@ static void push_android_codec_src(Path path,
CodecSrc::DstColorType dstColorTyp
     }
 
     switch (dstAlphaType) {
-        case kOpaque_SkAlphaType:
-            folder.append("_opaque");
-            break;
         case kPremul_SkAlphaType:
             folder.append("_premul");
             break;
@@ -521,21 +515,16 @@ static void push_codec_srcs(Path path) {
             break;
     }
 
-    SkTArray<SkAlphaType> alphaModes;
-    alphaModes.push_back(kPremul_SkAlphaType);
-    alphaModes.push_back(kUnpremul_SkAlphaType);
-    if (codec->getInfo().alphaType() == kOpaque_SkAlphaType) {
-        alphaModes.push_back(kOpaque_SkAlphaType);
-    }
+    const auto alphaModes = { kPremul_SkAlphaType, kUnpremul_SkAlphaType };
 
     for (CodecSrc::Mode mode : nativeModes) {
         for (float scale : nativeScales) {
             for (CodecSrc::DstColorType colorType : colorTypes) {
                 for (SkAlphaType alphaType : alphaModes) {
-                    // Only test kCroppedScanline_Mode when the alpha type is
opaque.  The test is
+                    // Only test kCroppedScanline_Mode when the alpha type is
unpremul.  The test is
                     // slow and won't be interestingly different with different
alpha types.
                     if (CodecSrc::kCroppedScanline_Mode == mode &&
-                            kOpaque_SkAlphaType != alphaType) {
+                            kUnpremul_SkAlphaType != alphaType) {
                         continue;
                     }

[commit-bot: I haz the power, 2016-05-20 15:41:53]

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

[scroggo, 2016-05-20 17:31:40]

The CQ bit was checked by scroggo@google.com

[scroggo, 2016-05-20 17:31:40]

The patchset sent to the CQ was uploaded after l-g-t-m from msarett@google.com
Link to the patchset: https://codereview.chromium.org/1999593003/#ps20001
(title: "Rebase")

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

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

[scroggo, 2016-05-20 17:32:13]

The CQ bit was unchecked by scroggo@google.com

[scroggo_chromium, 2016-05-20 17:43:18]

Description was changed from

==========
Stop testing decoding to opaque in DM image

This is already tested in CodecTest [1], where we require that the
result matches exactly. CodecTest runs on a smaller set of images, but
it covers a variety of opaque images. No need to test on all opaque
images.

Running locally on my Mac laptop, the following command:

    dm --src image --images resources/

dropped from:
11063 srcs
13.2s

to:

9084 srcs
9.26s

for a 30% speedup.

BUG=skia:5307

[1] https://skia.googlesource.com/skia/+/master/tests/CodecTest.cpp#119
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Only decode opaque to premul in DM image

Previously, we would test decoding an opaque SkCodec to all three:
kUnpremul,
kPremul,
kOpaque

The image should look the same in all three cases. We already test for
that in CodecTest [1], where we require that the result matches exactly.
CodecTest runs on a smaller set of images, but it covers a variety of
opaque images. No need to test on all opaque images.

Running locally on my Mac laptop, the following command:

    dm --src image --images resources/

dropped from:
5932 srcs
11.2s

to:

3544 srcs
6.69s

for a 40% speedup.

BUG=skia:5307

[1] https://skia.googlesource.com/skia/+/master/tests/CodecTest.cpp#119
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[scroggo_chromium, 2016-05-20 17:44:47]

FYI: I updated this to stop decoding kOpaque to both kPremul and kUnpremul, for
an extra speedup. I've also updated the description, which now starts from the
other CL as a baseline (and still has a greater speedup because of the extra
work skipped).

[scroggo_chromium, 2016-05-20 17:50:14]

On 2016/05/20 17:44:47, scroggo_chromium wrote:
> FYI: I updated this to stop decoding kOpaque to both kPremul and kUnpremul,
for
> an extra speedup. I've also updated the description, which now starts from the
> other CL as a baseline (and still has a greater speedup because of the extra
> work skipped).

Further, it reduces decoding HTC.dng from:

45 srcs
37.8s

to

15 srcs
10.8s

for a 70% speedup (over master; not comparing to the speedup of patch set 1),
only decoding that image. Given that we decode 15 raw images on the bots (which
will vary in time, but are all large, which is at least part of why they are so
slow), this should make a big difference.

[msarett, 2016-05-20 17:51:16]

On 2016/05/20 17:50:14, scroggo_chromium wrote:
> On 2016/05/20 17:44:47, scroggo_chromium wrote:
> > FYI: I updated this to stop decoding kOpaque to both kPremul and kUnpremul,
> for
> > an extra speedup. I've also updated the description, which now starts from
the
> > other CL as a baseline (and still has a greater speedup because of the extra
> > work skipped).
> 
> Further, it reduces decoding HTC.dng from:
> 
> 45 srcs
> 37.8s
> 
> to
> 
> 15 srcs
> 10.8s
> 
> for a 70% speedup (over master; not comparing to the speedup of patch set 1),
> only decoding that image. Given that we decode 15 raw images on the bots
(which
> will vary in time, but are all large, which is at least part of why they are
so
> slow), this should make a big difference.

Great!

[scroggo, 2016-05-20 17:54:23]

The CQ bit was checked by scroggo@google.com

[scroggo, 2016-05-20 17:54:23]

The patchset sent to the CQ was uploaded after l-g-t-m from msarett@google.com
Link to the patchset: https://codereview.chromium.org/1999593003/#ps40001
(title: "Don't decode opaque to unpremul, either")

[commit-bot: I haz the power, 2016-05-20 17:54:26]

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

[commit-bot: I haz the power, 2016-05-20 18:08:30]

Description was changed from

==========
Only decode opaque to premul in DM image

Previously, we would test decoding an opaque SkCodec to all three:
kUnpremul,
kPremul,
kOpaque

The image should look the same in all three cases. We already test for
that in CodecTest [1], where we require that the result matches exactly.
CodecTest runs on a smaller set of images, but it covers a variety of
opaque images. No need to test on all opaque images.

Running locally on my Mac laptop, the following command:

    dm --src image --images resources/

dropped from:
5932 srcs
11.2s

to:

3544 srcs
6.69s

for a 40% speedup.

BUG=skia:5307

[1] https://skia.googlesource.com/skia/+/master/tests/CodecTest.cpp#119
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Only decode opaque to premul in DM image

Previously, we would test decoding an opaque SkCodec to all three:
kUnpremul,
kPremul,
kOpaque

The image should look the same in all three cases. We already test for
that in CodecTest [1], where we require that the result matches exactly.
CodecTest runs on a smaller set of images, but it covers a variety of
opaque images. No need to test on all opaque images.

Running locally on my Mac laptop, the following command:

    dm --src image --images resources/

dropped from:
5932 srcs
11.2s

to:

3544 srcs
6.69s

for a 40% speedup.

BUG=skia:5307

[1] https://skia.googlesource.com/skia/+/master/tests/CodecTest.cpp#119
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/f2c96a2a6fa22c674083db701d15702636f9d5fc
==========

[commit-bot: I haz the power, 2016-05-20 18:08:31]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/f2c96a2a6fa22c674083db701d15702636f9d5fc