From 7b5e5536a18123f568afca99dc8f89019eab2c77 Mon Sep 17 00:00:00 2001 From: scroggo Date: Thu, 4 Feb 2016 06:14:24 -0800 Subject: [PATCH] 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 --- include/codec/SkAndroidCodec.h | 18 ++++++- tests/CodexTest.cpp | 93 +++++----------------------------- 2 files changed, 30 insertions(+), 81 deletions(-) diff --git a/include/codec/SkAndroidCodec.h b/include/codec/SkAndroidCodec.h index c5578d3964..7fee5be255 100644 --- 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 1f6b7ac4c1..747d064481 100644 --- 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 +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 +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 codec(nullptr); + SkAutoTDelete androidCodec(nullptr); if (isIncomplete) { size_t size = stream->getLength(); SkAutoTUnref 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 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 -- 2.34.1