GIF decoding to Index8, unit tests and misusing unit test as benchmark

third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp

Index: third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp b/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
index 39a11f9ea619244d84ebd18df4dbbdba22cfbd6d..1c331e81e5715c84810e07eaa14d310d2331d99b 100644
--- a/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
+++ b/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
@@ -108,6 +108,7 @@ ImageFrameGenerator::ImageFrameGenerator(const SkISize& fullSize, PassRefPtr<Sha
     , m_decodeCount(0)
     , m_frameCount(0)
     , m_encodedData(nullptr)
+    , m_decoderCanDecodeToIndex8(false)
 {
     setData(data.get(), allDataReceived);
 }
@@ -175,7 +176,30 @@ SkData* ImageFrameGenerator::refEncodedData()
     return m_encodedData;
 }
 
-bool ImageFrameGenerator::decodeAndScale(const SkImageInfo& info, size_t index, void* pixels, size_t rowBytes)
+static bool copyIndex8PixelsTo(const SkBitmap& bitmap, const SkImageInfo& info, void* pixels, size_t rowBytes)
+{
+    // This method is called for GIF animations, only for frames that, in first animation loop, get
+    // decoded to N32, but then are decoded to Index8 in later animation loop. It is needed to return
+    // N32 pixels but internally GIF decoder produces and caches Index8 bitmap.
+    if (bitmap.colorType() != kIndex_8_SkColorType)
+        return false;
+    // GIF decoder always produce colortable with 256 entries.
+    ASSERT(bitmap.getColorTable() && bitmap.getColorTable()->count() == 256);
+
+    const SkPMColor* colors = bitmap.getColorTable()->readColors();
+    uint8_t* destination8 = (uint8_t*)pixels;
+    for (int j = 0; j < bitmap.height(); ++j) {
+        uint32_t* destination32 = (uint32_t*)destination8;
+        const uint8_t* source = bitmap.getAddr8(0, j);
+        const uint8_t* sourceEnd = source + bitmap.width();
+        while (source != sourceEnd)
+            *destination32++ = colors[*source++];
+        destination8 += rowBytes;
+    }
+    return true;
+}
+
+bool ImageFrameGenerator::decodeAndScale(const SkImageInfo& info, size_t index, void* pixels, size_t rowBytes, SkPMColor ctable[], int* ctableCount)
 {
     // This method is called to populate a discardable memory owned by Skia.
 
@@ -193,7 +217,7 @@ bool ImageFrameGenerator::decodeAndScale(const SkImageInfo& info, size_t index,
 
     m_externalAllocator = adoptPtr(new ExternalMemoryAllocator(info, pixels, rowBytes));
 
-    SkBitmap bitmap = tryToResumeDecode(scaledSize, index);
+    SkBitmap bitmap = tryToResumeDecode(scaledSize, index, info.colorType());
     if (bitmap.isNull())
         return false;
 
@@ -204,12 +228,31 @@ bool ImageFrameGenerator::decodeAndScale(const SkImageInfo& info, size_t index,
     ASSERT(bitmap.width() == scaledSize.width());
     ASSERT(bitmap.height() == scaledSize.height());
 
+    // If decoder cannot produce requested color type, return false.
+    if (bitmap.colorType() != info.colorType() && bitmap.colorType() != kIndex_8_SkColorType)
+        return false;
+
     bool result = true;
     SkAutoLockPixels bitmapLock(bitmap);
     // Check to see if decoder has written directly to the memory provided
     // by Skia. If not make a copy.
-    if (bitmap.getPixels() != pixels)
-        result = bitmap.copyPixelsTo(pixels, rowBytes * info.height(), rowBytes);
+    if (bitmap.getPixels() != pixels) {
+        if (bitmap.colorType() == info.colorType()) {
+            result = bitmap.copyPixelsTo(pixels, rowBytes * info.height(), rowBytes);
+        } else {
+            ASSERT(bitmap.colorType() == kIndex_8_SkColorType && info.colorType() == kN32_SkColorType);
+            result = copyIndex8PixelsTo(bitmap, info, pixels, rowBytes);
+        }
+    }
+    if (info.colorType() == kIndex_8_SkColorType && ctable && bitmap.getColorTable()) {
+        memcpy(ctable, bitmap.getColorTable()->readColors(), bitmap.getColorTable()->count() * sizeof(SkPMColor));
+    }
+    if (ctableCount) {
+        // Based on SkImageGenerator API spec, ctableCount needs to be set to 0 for other types.
+        *ctableCount = (info.colorType() == kIndex_8_SkColorType && bitmap.getColorTable())
+            ? bitmap.getColorTable()->count()
+            : 0;
+    }
     return result;
 }
 
@@ -255,7 +298,7 @@ bool ImageFrameGenerator::decodeToYUV(SkISize componentSizes[3], void* planes[3]
     return yuvDecoded;
 }
 
-SkBitmap ImageFrameGenerator::tryToResumeDecode(const SkISize& scaledSize, size_t index)
+SkBitmap ImageFrameGenerator::tryToResumeDecode(const SkISize& scaledSize, size_t index, SkColorType outputColor)

[aleksandar.stojiljkovic, 2016/01/18 13:58:49]

 SkColorType outputColor is not necessary. Removed.

 {
     TRACE_EVENT1("blink", "ImageFrameGenerator::tryToResumeDecodeAndScale", "index", static_cast<int>(index));
 
@@ -264,7 +307,7 @@ SkBitmap ImageFrameGenerator::tryToResumeDecode(const SkISize& scaledSize, size_
     ASSERT(!resumeDecoding || decoder);
 
     SkBitmap fullSizeImage;
-    bool complete = decode(index, &decoder, &fullSizeImage);
+    bool complete = decode(index, &decoder, &fullSizeImage, outputColor);
 
     if (!decoder)
         return SkBitmap();
@@ -330,7 +373,7 @@ void ImageFrameGenerator::setHasAlpha(size_t index, bool hasAlpha)
     m_hasAlpha[index] = hasAlpha;
 }
 
-bool ImageFrameGenerator::decode(size_t index, ImageDecoder** decoder, SkBitmap* bitmap)
+bool ImageFrameGenerator::decode(size_t index, ImageDecoder** decoder, SkBitmap* bitmap, SkColorType outputColor)
 {
     TRACE_EVENT2("blink", "ImageFrameGenerator::decode", "width", m_fullSize.width(), "height", m_fullSize.height());
 
@@ -424,4 +467,37 @@ bool ImageFrameGenerator::getYUVComponentSizes(SkISize componentSizes[3])
     return updateYUVComponentSizes(decoder.get(), componentSizes, ImageDecoder::SizeForMemoryAllocation);
 }
 
+bool ImageFrameGenerator::canDecodeTo(size_t index, SkColorType outputType)
+{
+    if (m_decoderCanDecodeToIndex8) {
+        // Prevents concurrent decode or scale operations on the same image data.
+        MutexLocker lock(m_decodeMutex);
+        ImageDecoder* decoder = 0;
+        const bool decoderExist = ImageDecodingStore::instance().lockDecoder(this, m_fullSize, &decoder);
+        ASSERT(!decoderExist || decoder);
+        SharedBuffer* data = 0;
+        bool allDataReceived = false;
+        m_data->data(&data, &allDataReceived);
+        if (!decoderExist) {
+            if (m_imageDecoderFactory)
+                decoder = m_imageDecoderFactory->create().leakPtr();
+            if (!decoder)
+                decoder = ImageDecoder::create(*data, ImageDecoder::AlphaPremultiplied, ImageDecoder::GammaAndColorProfileApplied).leakPtr();
+            if (!decoder)
+                return (outputType == kIndex_8_SkColorType);
+        }
+        decoder->setData(data, allDataReceived);
+        bool canDecodeToIndex8 = decoder->canDecodeTo(index, static_cast<ImageFrame::ColorType>(outputType));
+        decoder->setData(0, false); // Unref SharedBuffer from ImageDecoder.
+        if (decoderExist) {
+            ImageDecodingStore::instance().unlockDecoder(this, decoder);
+        } else {
+            ImageDecodingStore::instance().insertDecoder(this, PassOwnPtr<blink::ImageDecoder>(adoptPtr(decoder)));
+        }
+        return (canDecodeToIndex8 && (outputType == kIndex_8_SkColorType));
+    }
+    return (outputType == kN32_SkColorType);

[scroggo_chromium, 2016/01/06 21:50:40]

Would it make more sense to put this at the beginning of the method?

[aleksandar.stojiljkovic, 2016/01/18 13:58:49]

Done.

+}
+
+
 } // namespace blink