Add SkAndroidCodec::getPixels

This is a synonym for the version of getAndroidPixels that accepts
only three parameters (i.e. no AndroidOptions). It is very similar
to SkCodec::getPixels, so I think the motivation for naming the
version with options differently does not apply here.

Add comments to the header describing defaults.

Update the test to use a template, and delete a lot of redundant code.

Rename a variable to stop shadowing another variable.

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

Review URL: https://codereview.chromium.org/1647153002

diff --git a/include/codec/SkAndroidCodec.h b/include/codec/SkAndroidCodec.h
index c5578d3..7fee5be 100644 (file)
--- a/include/codec/SkAndroidCodec.h
+++ b/include/codec/SkAndroidCodec.h
@@ -143,6 +143,8 @@ public:
 
         /**
          *  Indicates is destination pixel memory is zero initialized.
+         *
+         *  The default is SkCodec::kNo_ZeroInitialized.
          */
         SkCodec::ZeroInitialized fZeroInitialized;
 
@@ -153,6 +155,8 @@ public:
          *
          *  If the EncodedFormat is kWEBP_SkEncodedFormat, the top and left
          *  values must be even.
+         *
+         *  The default is NULL, meaning a decode of the entire image.
          */
         SkIRect* fSubset;
 
@@ -166,6 +170,8 @@ public:
          *  If the client does not request kIndex8_SkColorType, then the last
          *  two parameters may be NULL. If fColorCount is not null, it will be
          *  set to 0.
+         *
+         *  The default is NULL for both pointers.
          */
         SkPMColor* fColorPtr;
         int*       fColorCount;
@@ -174,6 +180,8 @@ public:
          *  The client may provide an integer downscale factor for the decode.
          *  The codec may implement this downscaling by sampling or another
          *  method if it is more efficient.
+         *
+         *  The default is 1, representing no downscaling.
          */
         int fSampleSize;
     };
@@ -207,7 +215,8 @@ public:
      *  be nullptr.
      *
      *  The AndroidOptions object is also used to specify any requested scaling or subsetting
-     *  using options->fSampleSize and options->fSubset.
+     *  using options->fSampleSize and options->fSubset. If NULL, the defaults (as specified above
+     *  for AndroidOptions) are used.
      *
      *  @return Result kSuccess, or another value explaining the type of failure.
      */
@@ -219,13 +228,18 @@ public:
             const AndroidOptions* options);
 
     /**
-     *  Simplified version of getAndroidPixels() where we supply the default AndroidOptions.
+     *  Simplified version of getAndroidPixels() where we supply the default AndroidOptions as
+     *  specified above for AndroidOptions.
      *
      *  This will return an error if the info is kIndex_8_SkColorType and also will not perform
      *  any scaling or subsetting.
      */
     SkCodec::Result getAndroidPixels(const SkImageInfo& info, void* pixels, size_t rowBytes);
 
+    SkCodec::Result getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes) {
+        return this->getAndroidPixels(info, pixels, rowBytes);
+    }
+
 protected:
 
     SkAndroidCodec(SkCodec*);

diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp
index 1f6b7ac..747d064 100644 (file)
--- a/tests/CodexTest.cpp
+++ b/tests/CodexTest.cpp
@@ -56,7 +56,8 @@ static void compare_to_good_digest(skiatest::Reporter* r, const SkMD5::Digest& g
  *  Calling getPixels(info) should return expectedResult, and if goodDigest is non nullptr,
  *  the resulting decode should match.
  */
-static void test_info(skiatest::Reporter* r, SkCodec* codec, const SkImageInfo& info,
+template<typename Codec>
+static void test_info(skiatest::Reporter* r, Codec* codec, const SkImageInfo& info,
                       SkCodec::Result expectedResult, const SkMD5::Digest* goodDigest) {
     SkBitmap bm;
     bm.allocPixels(info);
@@ -70,20 +71,6 @@ static void test_info(skiatest::Reporter* r, SkCodec* codec, const SkImageInfo&
     }
 }
 
-static void test_android_info(skiatest::Reporter* r, SkAndroidCodec* codec, const SkImageInfo& info,
-                              SkCodec::Result expectedResult, const SkMD5::Digest* goodDigest) {
-    SkBitmap bm;
-    bm.allocPixels(info);
-    SkAutoLockPixels autoLockPixels(bm);
-
-    SkCodec::Result result = codec->getAndroidPixels(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 {
@@ -96,7 +83,8 @@ SkIRect generate_random_subset(SkRandom* rand, int w, int h) {
     return rect;
 }
 
-static void test_codec(skiatest::Reporter* r, SkCodec* codec, SkBitmap& bm, const SkImageInfo& info,
+template<typename Codec>
+static void test_codec(skiatest::Reporter* r, Codec* codec, SkBitmap& bm, const SkImageInfo& info,
         const SkISize& size, SkCodec::Result expectedResult, SkMD5::Digest* digest,
         const SkMD5::Digest* goodDigest) {
 
@@ -149,59 +137,6 @@ static void test_codec(skiatest::Reporter* r, SkCodec* codec, SkBitmap& bm, cons
     }
 }
 
-static void test_android_codec(skiatest::Reporter* r, SkAndroidCodec* codec, SkBitmap& bm,
-        const SkImageInfo& info, const SkISize& size, SkCodec::Result expectedResult,
-        SkMD5::Digest* digest, const SkMD5::Digest* goodDigest) {
-
-    REPORTER_ASSERT(r, info.dimensions() == size);
-    bm.allocPixels(info);
-    SkAutoLockPixels autoLockPixels(bm);
-
-    SkCodec::Result result = codec->getAndroidPixels(info, bm.getPixels(), bm.rowBytes());
-    REPORTER_ASSERT(r, result == expectedResult);
-
-    md5(bm, digest);
-    if (goodDigest) {
-        REPORTER_ASSERT(r, *digest == *goodDigest);
-    }
-
-    {
-        // Test decoding to 565
-        SkImageInfo info565 = info.makeColorType(kRGB_565_SkColorType);
-        SkCodec::Result expected565 = info.alphaType() == kOpaque_SkAlphaType ?
-                expectedResult : SkCodec::kInvalidConversion;
-        test_android_info(r, codec, info565, expected565, nullptr);
-    }
-
-    // Verify that re-decoding gives the same result.  It is interesting to check this after
-    // a decode to 565, since choosing to decode to 565 may result in some of the decode
-    // options being modified.  These options should return to their defaults on another
-    // decode to kN32, so the new digest should match the old digest.
-    test_android_info(r, codec, info, expectedResult, digest);
-
-    {
-        // Check alpha type conversions
-        if (info.alphaType() == kOpaque_SkAlphaType) {
-            test_android_info(r, codec, info.makeAlphaType(kUnpremul_SkAlphaType),
-                    expectedResult, digest);
-            test_android_info(r, codec, info.makeAlphaType(kPremul_SkAlphaType),
-                    expectedResult, digest);
-        } else {
-            // Decoding to opaque should fail
-            test_android_info(r, codec, info.makeAlphaType(kOpaque_SkAlphaType),
-                    SkCodec::kInvalidConversion, nullptr);
-            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_android_info(r, codec, info.makeAlphaType(otherAt), expectedResult, nullptr);
-        }
-    }
-}
-
 // FIXME: SkScaledCodec is currently only supported for types used by BRD
 // https://bug.skia.org/4428
 static bool supports_scaled_codec(const char path[]) {
@@ -250,7 +185,7 @@ static void check(skiatest::Reporter* r,
     SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
     SkBitmap bm;
     SkCodec::Result expectedResult = isIncomplete ? SkCodec::kIncompleteInput : SkCodec::kSuccess;
-    test_codec(r, codec, bm, info, size, expectedResult, &codecDigest, nullptr);
+    test_codec(r, codec.get(), bm, info, size, expectedResult, &codecDigest, nullptr);
 
     // Scanline decoding follows.
     // Need to call startScanlineDecode() first.
@@ -361,23 +296,23 @@ static void check(skiatest::Reporter* r,
             return;
         }
 
-        SkAutoTDelete<SkAndroidCodec> codec(nullptr);
+        SkAutoTDelete<SkAndroidCodec> androidCodec(nullptr);
         if (isIncomplete) {
             size_t size = stream->getLength();
             SkAutoTUnref<SkData> data((SkData::NewFromStream(stream, 2 * size / 3)));
-            codec.reset(SkAndroidCodec::NewFromData(data));
+            androidCodec.reset(SkAndroidCodec::NewFromData(data));
         } else {
-            codec.reset(SkAndroidCodec::NewFromStream(stream.detach()));
+            androidCodec.reset(SkAndroidCodec::NewFromStream(stream.detach()));
         }
-        if (!codec) {
+        if (!androidCodec) {
             ERRORF(r, "Unable to decode '%s'", path);
             return;
         }
 
         SkBitmap bm;
         SkMD5::Digest scaledCodecDigest;
-        test_android_codec(r, codec, bm, info, size, expectedResult,
-                &scaledCodecDigest, &codecDigest);
+        test_codec(r, androidCodec.get(), bm, info, size, expectedResult, &scaledCodecDigest,
+                   &codecDigest);
     }
 
     // Test SkCodecImageGenerator
@@ -909,13 +844,13 @@ DEF_TEST(Codec_webp_peek, r) {
     SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(new LimitedPeekingMemStream(data, 25)));
     REPORTER_ASSERT(r, codec);
 
-    test_info(r, codec, codec->getInfo(), SkCodec::kSuccess, nullptr);
+    test_info(r, codec.get(), codec->getInfo(), SkCodec::kSuccess, nullptr);
 
     // Similarly, a stream which does not peek should still succeed.
     codec.reset(SkCodec::NewFromStream(new LimitedPeekingMemStream(data, 0)));
     REPORTER_ASSERT(r, codec);
 
-    test_info(r, codec, codec->getInfo(), SkCodec::kSuccess, nullptr);
+    test_info(r, codec.get(), codec->getInfo(), SkCodec::kSuccess, nullptr);
 }
 
 // SkCodec's wbmp decoder was initially more restrictive than SkImageDecoder.
@@ -945,7 +880,7 @@ DEF_TEST(Codec_wbmp, r) {
     if (!codec) {
         return;
     }
-    test_info(r, codec, codec->getInfo(), SkCodec::kSuccess, nullptr);
+    test_info(r, codec.get(), codec->getInfo(), SkCodec::kSuccess, nullptr);
 }
 
 // wbmp images have a header that can be arbitrarily large, depending on the