Reland "Properly set alpha type in webp decode."

Also use the newer setConfig function.

Add a test to confirm that we set the alpha type properly.

Add some images with alpha for testing. (These images are also
beneficial for the compare_unpremul test, which was previously
not meaningful on 100% opaque images.)

All of the added images are in the public domain. They were
taken from https://developers.google.com/speed/webp/gallery2:

yellow_rose:
"Free Stock Photo in High Resolution - Yellow Rose 3 - Flowers"
Image Author: Jon Sullivan
This file is in the public domain.
http://www.public-domain-photos.com/free-stock-photos-4/flowers/yellow-rose-3.jpg

baby_tux:
"baby tux for my user page"
Image Author: Fizyplankton
This file is in the public domain.
http://www.minecraftwiki.net/images/8/85/Fizyplankton.png

NOTRY=true

TBR=halcanary@google.com

BUG=skia:2388

Author: scroggo@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@14360 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/resources/baby_tux.png b/resources/baby_tux.png
new file mode 100644 (file)
index 0000000..dd082c4
Binary files /dev/null and b/resources/baby_tux.png differ

diff --git a/resources/baby_tux.webp b/resources/baby_tux.webp
new file mode 100644 (file)
index 0000000..8764f06
Binary files /dev/null and b/resources/baby_tux.webp differ

diff --git a/resources/half-transparent-white-pixel.png b/resources/half-transparent-white-pixel.png
new file mode 100644 (file)
index 0000000..03565d3
Binary files /dev/null and b/resources/half-transparent-white-pixel.png differ

diff --git a/resources/half-transparent-white-pixel.webp b/resources/half-transparent-white-pixel.webp
new file mode 100644 (file)
index 0000000..b3a3307
Binary files /dev/null and b/resources/half-transparent-white-pixel.webp differ

diff --git a/resources/yellow_rose.png b/resources/yellow_rose.png
new file mode 100644 (file)
index 0000000..140c0b9
Binary files /dev/null and b/resources/yellow_rose.png differ

diff --git a/resources/yellow_rose.webp b/resources/yellow_rose.webp
new file mode 100644 (file)
index 0000000..f7dc208
Binary files /dev/null and b/resources/yellow_rose.webp differ

diff --git a/src/images/SkImageDecoder_libwebp.cpp b/src/images/SkImageDecoder_libwebp.cpp
index 5174806..4e23e50 100644 (file)
--- a/src/images/SkImageDecoder_libwebp.cpp
+++ b/src/images/SkImageDecoder_libwebp.cpp
@@ -298,8 +298,20 @@ bool SkWEBPImageDecoder::setDecodeConfig(SkBitmap* decodedBitmap,
         return false;
     }
 
-    return decodedBitmap->setConfig(config, width, height, 0,
-                                    fHasAlpha ? kPremul_SkAlphaType : kOpaque_SkAlphaType);
+    SkImageInfo info;
+    info.fWidth = width;
+    info.fHeight = height;
+    info.fColorType = SkBitmapConfigToColorType(config);
+    if (SkToBool(fHasAlpha)) {
+        if (this->getRequireUnpremultipliedColors()) {
+            info.fAlphaType = kUnpremul_SkAlphaType;
+        } else {
+            info.fAlphaType = kPremul_SkAlphaType;
+        }
+    } else {
+        info.fAlphaType = kOpaque_SkAlphaType;
+    }
+    return decodedBitmap->setConfig(info);
 }
 
 bool SkWEBPImageDecoder::onBuildTileIndex(SkStreamRewindable* stream,

diff --git a/tests/ImageDecodingTest.cpp b/tests/ImageDecodingTest.cpp
index 5d52f90..e9348fe 100644 (file)
--- a/tests/ImageDecodingTest.cpp
+++ b/tests/ImageDecodingTest.cpp
@@ -163,6 +163,162 @@ static void test_unpremul(skiatest::Reporter* reporter) {
     }
 }
 
+#if defined(SK_BUILD_FOR_ANDROID) || defined(SK_BUILD_FOR_UNIX)
+// Test that the alpha type is what we expect.
+static void test_alphaType(skiatest::Reporter* reporter, const SkString& filename,
+                           bool requireUnpremul) {
+    SkBitmap bm;
+    SkFILEStream stream(filename.c_str());
+
+    SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(&stream));
+    if (NULL == decoder.get()) {
+        return;
+    }
+
+    decoder->setRequireUnpremultipliedColors(requireUnpremul);
+
+    // Decode just the bounds. This should always succeed.
+    bool success = decoder->decode(&stream, &bm, SkBitmap::kARGB_8888_Config,
+                                   SkImageDecoder::kDecodeBounds_Mode);
+    REPORTER_ASSERT(reporter, success);
+    if (!success) {
+        return;
+    }
+
+    // Keep track of the alpha type for testing later. If the full decode
+    // succeeds, the alpha type should be the same, unless the full decode
+    // determined that the alpha type should actually be opaque, which may
+    // not be known when only decoding the bounds.
+    const SkAlphaType boundsAlphaType = bm.alphaType();
+
+    // rewind should always succeed on SkFILEStream.
+    success = stream.rewind();
+    REPORTER_ASSERT(reporter, success);
+    if (!success) {
+        return;
+    }
+
+    success = decoder->decode(&stream, &bm, SkBitmap::kARGB_8888_Config,
+                              SkImageDecoder::kDecodePixels_Mode);
+
+    if (!success) {
+        // When the decoder is set to require unpremul, if it does not support
+        // unpremul it will fail. This is the only reason the decode should
+        // fail (since we know the files we are using to test can be decoded).
+        REPORTER_ASSERT(reporter, requireUnpremul);
+        return;
+    }
+
+    // The bounds decode should return with either the requested
+    // premul/unpremul or opaque, if that value could be determined when only
+    // decoding the bounds.
+    if (requireUnpremul) {
+        REPORTER_ASSERT(reporter, kUnpremul_SkAlphaType == boundsAlphaType
+                                  || kOpaque_SkAlphaType == boundsAlphaType);
+    } else {
+        REPORTER_ASSERT(reporter, kPremul_SkAlphaType == boundsAlphaType
+                                  || kOpaque_SkAlphaType == boundsAlphaType);
+    }
+
+    // When decoding the full image, the alpha type should match the one
+    // returned by the bounds decode, unless the full decode determined that
+    // the alpha type is actually opaque.
+    REPORTER_ASSERT(reporter, bm.alphaType() == boundsAlphaType
+                              || bm.alphaType() == kOpaque_SkAlphaType);
+}
+
+DEF_TEST(ImageDecoding_alphaType, reporter) {
+    SkString resourcePath = skiatest::Test::GetResourcePath();
+    if (resourcePath.isEmpty()) {
+        SkDebugf("Could not run alphaType test because resourcePath not specified.");
+        return;
+    }
+
+    SkOSFile::Iter iter(resourcePath.c_str());
+    SkString basename;
+    if (iter.next(&basename)) {
+        do {
+            SkString filename = SkOSPath::SkPathJoin(resourcePath.c_str(), basename.c_str());
+            for (int truth = 0; truth <= 1; ++truth) {
+                test_alphaType(reporter, filename, SkToBool(truth));
+            }
+        } while (iter.next(&basename));
+    } else {
+        SkDebugf("Failed to find any files :(\n");
+    }
+
+}
+
+// Using known images, test that decoding into unpremul and premul behave as expected.
+DEF_TEST(ImageDecoding_unpremul, reporter) {
+    SkString resourcePath = skiatest::Test::GetResourcePath();
+    if (resourcePath.isEmpty()) {
+        SkDebugf("Could not run unpremul test because resourcePath not specified.");
+        return;
+    }
+    const char* root = "half-transparent-white-pixel";
+    const char* suffixes[] = { ".png", ".webp" };
+
+    for (size_t i = 0; i < SK_ARRAY_COUNT(suffixes); ++i) {
+        SkString basename = SkStringPrintf("%s%s", root, suffixes[i]);
+        SkString fullName = SkOSPath::SkPathJoin(resourcePath.c_str(), basename.c_str());
+
+        SkBitmap bm;
+        SkFILEStream stream(fullName.c_str());
+
+        if (!stream.isValid()) {
+            SkDebugf("file %s missing from resource directoy %s\n",
+                     basename.c_str(), resourcePath.c_str());
+            continue;
+        }
+
+        // This should never fail since we know the images we're decoding.
+        SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(&stream));
+        REPORTER_ASSERT(reporter, NULL != decoder.get());
+        if (NULL == decoder.get()) {
+            continue;
+        }
+
+        // Test unpremultiplied. We know what color this should result in.
+        decoder->setRequireUnpremultipliedColors(true);
+        bool success = decoder->decode(&stream, &bm, SkBitmap::kARGB_8888_Config,
+                                       SkImageDecoder::kDecodePixels_Mode);
+        REPORTER_ASSERT(reporter, success);
+        if (!success) {
+            continue;
+        }
+
+        REPORTER_ASSERT(reporter, bm.width() == 1 && bm.height() == 1);
+        {
+            SkAutoLockPixels alp(bm);
+            REPORTER_ASSERT(reporter, bm.getAddr32(0, 0)[0] == 0x7fffffff);
+        }
+
+        success = stream.rewind();
+        REPORTER_ASSERT(reporter, success);
+        if (!success) {
+            continue;
+        }
+
+        // Test premultiplied. Once again, we know which color this should
+        // result in.
+        decoder->setRequireUnpremultipliedColors(false);
+        success = decoder->decode(&stream, &bm, SkBitmap::kARGB_8888_Config,
+                                  SkImageDecoder::kDecodePixels_Mode);
+        REPORTER_ASSERT(reporter, success);
+        if (!success) {
+            continue;
+        }
+
+        REPORTER_ASSERT(reporter, bm.width() == 1 && bm.height() == 1);
+        {
+            SkAutoLockPixels alp(bm);
+            REPORTER_ASSERT(reporter, bm.getAddr32(0, 0)[0] == 0x7f7f7f7f);
+        }
+    }
+}
+#endif // SK_BUILD_FOR_UNIX/ANDROID skbug.com/2388
+
 #ifdef SK_DEBUG
 // Create a stream containing a bitmap encoded to Type type.
 static SkMemoryStream* create_image_stream(SkImageEncoder::Type type) {