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

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

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 87 matching lines @@
     return true;
 }
 
 ImageFrameGenerator::ImageFrameGenerator(const SkISize& fullSize, PassRefPtr<SharedBuffer> data, bool allDataReceived, bool isMultiFrame)
     : m_fullSize(fullSize)
     , m_data(adoptRef(new ThreadSafeDataTransport()))
     , m_isMultiFrame(isMultiFrame)
     , m_decodeFailed(false)
     , m_frameCount(0)
     , m_encodedData(nullptr)
+    , m_decoderCanDecodeToIndex8(false)
 {
     setData(data.get(), allDataReceived);
 }
 
 ImageFrameGenerator::~ImageFrameGenerator()
 {
     if (m_encodedData)
         m_encodedData->unref();
     ImageDecodingStore::instance().removeCacheIndexedByGenerator(this);
 }
@@ skipping 47 matching lines @@
         // While m_encodedData is referenced, prevent disposing m_data and its content.
         // it is dereferenced in sharedSkDataReleaseCallback, called when m_encodedData gets dereferenced.
         m_data->ref();
     }
     // Increase the reference, caller must decrease it. One reference is always kept by ImageFrameGenerator and released
     // in destructor.
     m_encodedData->ref();
     return m_encodedData;
 }
 
-bool ImageFrameGenerator::decodeAndScale(size_t index, const SkImageInfo& info, void* pixels, size_t rowBytes)
+static bool copyIndex8PixelsTo(const SkBitmap& bitmap, const SkImageInfo& info, void* pixels, size_t rowBytes)

[scroggo_chromium, 2016/04/29 19:48:14]

Various thoughts:

- I think it would be helpful to name these how they are used, src versus dst.
e.g.

(const SkBitmap& src, const SkImageInfo& dstInfo, void* dstPixels, size_t
dstRowBytes)

You could also wrap the dst into an SkPixmap, so it would be

copyIndex8PixelsTo(const SkBitmap&src, SkPixmap& dst)

- You actually never use info. It might be useful for error checking (must be
N32, must have the same width and height as the bitmap). My preference would be
to drop it and use clear comments/names to indicate how the function is to be
called, but I'm okay with using it in asserts, too. (I assume this method is
intended to mimic SkBitmap::copyPixelsTo, which does not take an SkImageInfo
argument, FWIW.)

- This isn't really a copy. I suggest calling this convertIndex8ToN32. (Another
confusing part about the current name - it sounds like pixels are the source,
but they are the destination.)

+{
+    // 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)

[scroggo_chromium, 2016/04/29 19:48:14]

I think this should be an assert statement, rather than an if, since it will
only be called with index8.

Then this method can return void.

+        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(size_t index, const SkImageInfo& info, void* pixels, size_t rowBytes, SkPMColor ctable[], int* ctableCount)
 {
     // Prevent concurrent decode or scale operations on the same image data.
     MutexLocker lock(m_decodeMutex);
 
     if (m_decodeFailed)
         return false;
 
     TRACE_EVENT1("blink", "ImageFrameGenerator::decodeAndScale", "frame index", static_cast<int>(index));
 
     m_externalAllocator = adoptPtr(new ExternalMemoryAllocator(info, pixels, rowBytes));
 
     // This implementation does not support scaling so check the requested size.
     SkISize scaledSize = SkISize::Make(info.width(), info.height());
     ASSERT(m_fullSize == scaledSize);
 
     SkBitmap bitmap = tryToResumeDecode(index, scaledSize);
     if (bitmap.isNull())
         return false;
 
     // Don't keep the allocator because it contains a pointer to memory
     // that we do not own.
     m_externalAllocator.clear();
 
     // Check to see if the decoder has written directly to the pixel memory
     // provided. If not, make a copy.
     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;

[scroggo_chromium, 2016/04/29 19:48:14]

I don't think this variable is necessary. When you would have set it to false
(only when returning from bitmap.copyPixelsTo; as I suggest above,
copyIndex8PixelsTo should never return false) you can just return false (and
skip checking whether we need to memcpy/set the ctableCount).

     SkAutoLockPixels bitmapLock(bitmap);
-    if (bitmap.getPixels() != pixels)
-        return bitmap.copyPixelsTo(pixels, rowBytes * info.height(), rowBytes);
-    return true;
+    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;
 }
 
 bool ImageFrameGenerator::decodeToYUV(size_t index, SkISize componentSizes[3], void* planes[3], size_t rowBytes[3])
 {
     // Prevent concurrent decode or scale operations on the same image data.
     MutexLocker lock(m_decodeMutex);
 
     if (m_decodeFailed)
         return false;
 
@@ skipping 195 matching lines @@
 
     // Setting a dummy ImagePlanes object signals to the decoder that we want to do YUV decoding.
     decoder->setData(data, allDataReceived);
     OwnPtr<ImagePlanes> dummyImagePlanes = adoptPtr(new ImagePlanes);
     decoder->setImagePlanes(dummyImagePlanes.release());
 
     ASSERT(componentSizes);
     return updateYUVComponentSizes(decoder.get(), componentSizes, ImageDecoder::SizeForMemoryAllocation);
 }
 
+bool ImageFrameGenerator::canDecodeTo(size_t index, SkColorType outputType)
+{
+    if (!m_decoderCanDecodeToIndex8)
+        return (outputType == kN32_SkColorType);
+
+    // 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));
+}
+
+
 } // namespace blink