Update CodexTest to test valid_alpha.

tests/CodexTest.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "Resources.h"
 #include "SkBitmap.h"
 #include "SkCodec.h"
@@ skipping 24 matching lines @@
  *  @param goodDigest The known good digest to compare to.
  *  @param bm The bitmap to test.
  */
 static void compare_to_good_digest(skiatest::Reporter* r, const SkMD5::Digest& goodDigest,
                            const SkBitmap& bm) {
     SkMD5::Digest digest;
     md5(bm, &digest);
     REPORTER_ASSERT(r, digest == goodDigest);
 }
 
+/**
+ *  Test decoding an SkCodec to a particular SkImageInfo.
+ *
+ *  Calling getPixels(info) should return expectedResult, and if goodDigest is non NULL,
+ *  the resulting decode should match.
+ */
+static void test_info(skiatest::Reporter* r, SkCodec* codec, const SkImageInfo& info,
+                      SkCodec::Result expectedResult, const SkMD5::Digest* goodDigest) {
+    SkBitmap bm;
+    bm.allocPixels(info);
+    SkAutoLockPixels autoLockPixels(bm);
+
+    SkCodec::Result result = codec->getPixels(info, bm.getPixels(), bm.rowBytes());
+    REPORTER_ASSERT(r, result == expectedResult);
+
+    if (goodDigest) {
+        compare_to_good_digest(r, *goodDigest, bm);
+    }
+}
+
 SkIRect generate_random_subset(SkRandom* rand, int w, int h) {
     SkIRect rect;
     do {
         rect.fLeft = rand->nextRangeU(0, w);
         rect.fTop = rand->nextRangeU(0, h);
         rect.fRight = rand->nextRangeU(0, w);
         rect.fBottom = rand->nextRangeU(0, h);
         rect.sort();
     } while (rect.isEmpty());
     return rect;
@@ skipping 24 matching lines @@
     SkBitmap bm;
     bm.allocPixels(info);
     SkAutoLockPixels autoLockPixels(bm);
     SkCodec::Result result =
         codec->getPixels(info, bm.getPixels(), bm.rowBytes(), NULL, NULL, NULL);
     REPORTER_ASSERT(r, result == SkCodec::kSuccess);
 
     SkMD5::Digest digest;
     md5(bm, &digest);
 
-    bm.eraseColor(SK_ColorYELLOW);
+    // verify that re-decoding gives the same result.
+    test_info(r, codec, info, SkCodec::kSuccess, &digest);
 
-    result =
-        codec->getPixels(info, bm.getPixels(), bm.rowBytes(), NULL, NULL, NULL);
+    {
+        // Check alpha type conversions
+        if (info.alphaType() == kOpaque_SkAlphaType) {

[scroggo_chromium, 2015/08/07 23:16:31]

When I started writing this CL, I expected that an opaque image (e.g. JPEG)
could be decoded to any alpha type, since it is a valid premul/unpremul image.
So the content of this block was to verify that we succeeded, and the digest
matched the original.

I realized that we do not behave that way. Instead, we require the client to
request opaque if the image is opaque. Is that the right way to behave? I can
see the argument that it lets them catch their mistake.

If we stick with the current behavior, we need to add the following to this
block:

test_info(r, codec, info.makeAlphaType(<>), SkCodec::kInvalidConversion,
          NULL);

(Twice, once with kUnpremul and once with kPremul)

If we decide to support decoding opaque to non-opaque, we'll need to apply
https://codereview.chromium.org/1282073004 (DIFFERENT_ISSUE).

(Maybe we already discussed this and came to a decision, but I just forgot...)

[msarett, 2015/08/10 13:51:12]

I would guess that most clients would just use the alpha type that we recommend,
but some may always ask for kPremul/kUnpremul because that is what is convenient
for them:
1)I don't see a real problem with supporting kPremul, other than the fact that
it is a bit misleading about what the image actually is.
2) I'm a little more concerned about kUnpremul because I wouldn't want a client
that does it's own premultiplying to be premultiplying opaque images.

I think it makes the most sense to me to disable conversions to
kPremul/kUnpremul.  That way we force clients to at least be aware that the
image is opaque.  Then, if they still want to treat the output as
kPremul/kUnpremul, they can still do that.

Having said that, I don't really feel strongly about this.  I could be swayed
easily.

[scroggo_chromium, 2015/08/12 14:55:31]

Sticking with the existing decision sounds good to me. Added the tests I left
out before. This also required updating wbmp to check valid_alpha.

+        } else {
+            // Decoding to opaque should fail
+            test_info(r, codec, info.makeAlphaType(kOpaque_SkAlphaType),
+                      SkCodec::kInvalidConversion, NULL);
+            SkAlphaType otherAt = info.alphaType();
+            if (kPremul_SkAlphaType == otherAt) {
+                otherAt = kUnpremul_SkAlphaType;
+            } else {
+                otherAt = kPremul_SkAlphaType;
+            }
+            // The other non-opaque alpha type should always succeed, but not match.
+            test_info(r, codec, info.makeAlphaType(otherAt), SkCodec::kSuccess, NULL);
+        }
+    }
 
-    REPORTER_ASSERT(r, result == SkCodec::kSuccess);
-    // verify that re-decoding gives the same result.
-    compare_to_good_digest(r, digest, bm);
+    // Scanline decoding follows.
 
     stream.reset(resource(path));
     SkAutoTDelete<SkScanlineDecoder> scanlineDecoder(
             SkScanlineDecoder::NewFromStream(stream.detach()));
     if (supportsScanlineDecoding) {
         bm.eraseColor(SK_ColorYELLOW);
         REPORTER_ASSERT(r, scanlineDecoder);
 
         REPORTER_ASSERT(r, scanlineDecoder->start(info) == SkCodec::kSuccess);
 
@@ skipping 210 matching lines @@
     REPORTER_ASSERT(r, SkCodec::kInvalidParameters == result);
     result = decoder->start(
         decoder->getInfo().makeColorType(kIndex_8_SkColorType));
     REPORTER_ASSERT(r, SkCodec::kInvalidParameters == result);
 }
 
 DEF_TEST(Codec_Params, r) {
     test_invalid_parameters(r, "index8.png");
     test_invalid_parameters(r, "mandrill.wbmp");
 }