Change SkDecodingImageGenerator API

src/images/SkDecodingImageGenerator.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkDecodingImageGenerator.h"
 #include "SkData.h"
 #include "SkImageDecoder.h"
 #include "SkImageGenerator.h"
 #include "SkImagePriv.h"
 #include "SkStream.h"
-
+#include "SkUtils.h"
 
 namespace {
 /**
  *  Special allocator used by getPixels(). Uses preallocated memory
  *  provided.
  */
 class TargetAllocator : public SkBitmap::Allocator {
 public:
-    TargetAllocator(void* target, size_t rowBytes, const SkImageInfo& info)
+    TargetAllocator(void* target,
+                    size_t rowBytes,
+                    int width,
+                    int height,
+                    SkBitmap::Config config)

[scroggo, 2013/12/12 22:33:46]

Comments would be nice here, explaining the parameters.

[hal.canary, 2013/12/13 15:48:48]

Done.

         : fTarget(target)
         , fRowBytes(rowBytes)
-        , fInfo(info) { }
+        , fWidth(width)
+        , fHeight(height)
+        , fConfig(config) { }
+
+    bool isReady() { return (fTarget != NULL); }
 
     virtual bool allocPixelRef(SkBitmap* bm, SkColorTable* ct) SK_OVERRIDE {
-        if ((SkImageInfoToBitmapConfig(fInfo) != bm->config())
-            || (bm->width() != fInfo.fWidth)
-            || (bm->height() != fInfo.fHeight)) {
-            return false;
+        if ((NULL == fTarget)

[scroggo, 2013/12/12 22:33:46]

When will fTarget be NULL and called?

[hal.canary, 2013/12/13 15:48:48]

probably shouldn't happen.  but a weird decoder could try and use the allocator
multiple times.

[scroggo, 2013/12/13 17:23:03]

What should we do in that situation? I think the client will want the final
bitmap to be allocated via the allocator, but in this case, the first
(presumably thrown away) bitmap was allocated via the allocator.

+            || (fConfig != bm->config())
+            || (fWidth != bm->width())

[scroggo, 2013/12/12 22:33:46]

It seems you're mixing uses of this if statement. If the configs don't match,
then we should use the default allocator so we can later use fTarget in copyTo.
If the width or height don't match, isn't that an error? Likewise, I suspect the
ct will never be non NULL unless the configs don't match (since you're
explicitly disallowing Index8).

[hal.canary, 2013/12/13 15:48:48]

I just want to assert that everyone agrees on what the pixels are before we
write to them.

[scroggo, 2013/12/13 17:23:03]

You don't assert anything - you allocate new pixels not using fTarget, which may
or may not be what the caller expected.

+            || (fHeight != bm->height())
+            || (ct != NULL)) {
+            return bm->allocPixels(NULL, ct);
         }
-        bm->setConfig(bm->config(), bm->width(), bm->height(),
-                      fRowBytes, bm->alphaType());
-        bm->setPixels(fTarget, ct);
+        bm->setConfig(fConfig, fWidth, fHeight, fRowBytes, bm->alphaType());
+        // TODO(halcanary): verify that all callers of this function
+        // will respect new RowBytes.  Will be moot once rowbytes belongs
+        // to PixelRef.
+        bm->setPixels(fTarget, NULL);
+        fTarget = NULL;
         return true;
     }
 
 private:
     void* fTarget;
     size_t fRowBytes;
-    SkImageInfo fInfo;
+    int fWidth;
+    int fHeight;
+    SkBitmap::Config fConfig;
     typedef SkBitmap::Allocator INHERITED;
 };
 }  // namespace
 ////////////////////////////////////////////////////////////////////////////////
 
-SkDecodingImageGenerator::SkDecodingImageGenerator(SkData* data)
+SkDecodingImageGenerator::SkDecodingImageGenerator(SkData* data,
+                                                   const Options* opts)
     : fData(data)
     , fHasInfo(false)
-    , fDoCopyTo(false) {
+    , fDoCopyTo(false)
+    , fOpts() {
     SkASSERT(fData != NULL);
     fStream = SkNEW_ARGS(SkMemoryStream, (fData));
     SkASSERT(fStream != NULL);
     SkASSERT(fStream->unique());
     fData->ref();
+    if (opts != NULL) {
+        fOpts = *opts;
+        SkASSERT(fOpts.fRequestedConfig != SkBitmap::kIndex8_Config);
+    }
 }
 
-SkDecodingImageGenerator::SkDecodingImageGenerator(SkStreamRewindable* stream)
+SkDecodingImageGenerator::SkDecodingImageGenerator(SkStreamRewindable* stream,
+                                                   const Options* opts)
     : fData(NULL)
     , fStream(stream)
     , fHasInfo(false)
-    , fDoCopyTo(false) {
+    , fDoCopyTo(false)
+    , fOpts() {
     SkASSERT(fStream != NULL);
     SkASSERT(fStream->unique());
+    if (opts != NULL) {
+        fOpts = *opts;
+        SkASSERT(fOpts.fRequestedConfig != SkBitmap::kIndex8_Config);
+    }
 }
 
 SkDecodingImageGenerator::~SkDecodingImageGenerator() {
     SkSafeUnref(fData);
     fStream->unref();
 }
 
 // TODO(halcanary): Give this macro a better name and move it into SkTypes.h
 #ifdef SK_DEBUG
     #define SkCheckResult(expr, value)  SkASSERT((value) == (expr))
 #else
     #define SkCheckResult(expr, value)  (void)(expr)
 #endif
 
 SkData* SkDecodingImageGenerator::refEncodedData() {
     // This functionality is used in `gm --serialize`
+    // Does not encode options.
     if (fData != NULL) {
         return SkSafeRef(fData);
     }
     // TODO(halcanary): SkStreamRewindable needs a refData() function
     // which returns a cheap copy of the underlying data.
     if (!fStream->rewind()) {
         return NULL;
     }
     size_t length = fStream->getLength();
     if (0 == length) {
         return NULL;
     }
     void* buffer = sk_malloc_flags(length, 0);
     SkCheckResult(fStream->read(buffer, length), length);
     fData = SkData::NewFromMalloc(buffer, length);
     return SkSafeRef(fData);
 }
 
+// TODO(halcanary) move this function into a standard header.
+static bool config_to_colorType(SkBitmap::Config config, SkColorType* ctOut) {
+    SkColorType ct;
+    switch (config) {
+        case SkBitmap::kA8_Config:
+            ct = kAlpha_8_SkColorType;
+            break;
+        case SkBitmap::kIndex8_Config:
+            ct = kIndex_8_SkColorType;
+            break;
+        case SkBitmap::kRGB_565_Config:
+            ct = kRGB_565_SkColorType;
+            break;
+        case SkBitmap::kARGB_4444_Config:
+            ct = kARGB_4444_SkColorType;
+            break;
+        case SkBitmap::kARGB_8888_Config:
+            ct = kPMColor_SkColorType;
+            break;
+        case SkBitmap::kNo_Config:
+        default:
+            return false;
+    }
+    if (ctOut) {
+        *ctOut = ct;
+    }
+    return true;
+}
+
+
 bool SkDecodingImageGenerator::getInfo(SkImageInfo* info) {
+    if (SkBitmap::kIndex8_Config == fOpts.fRequestedConfig) {
+        // We do not support indexed color with SkImageGenerators,
+        SkDEBUGFAIL("SkBitmap::kIndex8_Config == fOpts.fRequestedConfig");
+        return false;
+    }
+
     // info can be NULL.  If so, will update fInfo, fDoCopyTo, and fHasInfo.
     if (fHasInfo) {
         if (info != NULL) {
             *info = fInfo;
         }
         return true;
     }
     SkAssertResult(fStream->rewind());
     SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(fStream));
     if (NULL == decoder.get()) {
         return false;
     }
+
     SkBitmap bitmap;
+    decoder->setSampleSize(fOpts.fSampleSize);
     if (!decoder->decode(fStream, &bitmap,
                          SkImageDecoder::kDecodeBounds_Mode)) {
         return false;
     }
     if (bitmap.config() == SkBitmap::kNo_Config) {
         return false;
     }
-    if (!bitmap.asImageInfo(&fInfo)) {
-        // We can't use bitmap.config() as is.
-        if (!bitmap.canCopyTo(SkBitmap::kARGB_8888_Config)) {
-            SkDEBUGFAIL("!bitmap->canCopyTo(SkBitmap::kARGB_8888_Config)");
+
+    if (fOpts.fRequestedConfig == SkBitmap::kNo_Config) {
+        // Use default config.
+        if (SkBitmap::kIndex8_Config == bitmap.config()) {
+            // We don't support kIndex8 because we don't support
+            // colortables in this workflow.
+            fOpts.fRequestedConfig = SkBitmap::kARGB_8888_Config;
+            fInfo.fWidth = bitmap.width();
+            fInfo.fHeight = bitmap.height();
+            fInfo.fColorType = kPMColor_SkColorType;
+            fInfo.fAlphaType = bitmap.alphaType();
+        } else {
+            fOpts.fRequestedConfig = bitmap.config();  // Save for later!
+            if (!bitmap.asImageInfo(&fInfo)) {
+                SkDEBUGFAIL("!bitmap.asImageInfo() && !kNoConfig");

[scroggo, 2013/12/12 22:33:46]

Can you make this message more informative?

[hal.canary, 2013/12/13 15:48:48]

Done.

+                return false;
+            }
+        }
+    } else {
+        if (!bitmap.canCopyTo(fOpts.fRequestedConfig)) {
+            SkASSERT(SkBitmap::kARGB_4444_Config == fOpts.fRequestedConfig);
+            SkASSERT(bitmap.config() != fOpts.fRequestedConfig);
+            return false;  // Can not translate to needed config.
+        }
+        fInfo.fWidth = bitmap.width();
+        fInfo.fHeight = bitmap.height();
+        if (!config_to_colorType(fOpts.fRequestedConfig,
+                                 &(fInfo.fColorType))) {
+            SkDEBUGFAIL("SkBitmapConfigToColorType fails.");
             return false;
         }
-        fDoCopyTo = true;
-        fInfo.fWidth = bitmap.width();
-        fInfo.fHeight = bitmap.height();
-        fInfo.fColorType = kPMColor_SkColorType;
         fInfo.fAlphaType = bitmap.alphaType();
     }
     if (info != NULL) {
         *info = fInfo;
     }
     fHasInfo = true;
     return true;
 }
 
+static inline bool check_alpha(SkAlphaType reported, SkAlphaType actual) {
+    return ((reported == actual)
+            || ((reported == kPremul_SkAlphaType)
+                && (actual == kOpaque_SkAlphaType)));
+}
+
 bool SkDecodingImageGenerator::getPixels(const SkImageInfo& info,
                                          void* pixels,
                                          size_t rowBytes) {
     if (NULL == pixels) {
         return false;
     }
-    if (!this->getInfo(NULL)) {
+    if (!this->getInfo(NULL)) {  // Verify that fInfo populated.
         return false;
     }
     if (SkImageInfoToBitmapConfig(info) == SkBitmap::kNo_Config) {
         return false;  // Unsupported SkColorType.
     }
+    if (fInfo != info) {
+        // The caller has specified a different info.  This is an
+        // error for this kind of SkImageGenerator.  Use the Options
+        // to change the settings.
+        return false;
+    }
+    int bpp = SkBitmap::ComputeBytesPerPixel(fOpts.fRequestedConfig);
+    if (static_cast<size_t>(bpp * info.fWidth) > rowBytes) {
+        // The caller has specified a bad rowBytes.
+        return false;
+    }
+
     SkAssertResult(fStream->rewind());
     SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(fStream));
     if (NULL == decoder.get()) {
         return false;
     }
-    if (fInfo != info) {
-        // The caller has specified a different info.  For now, this
-        // is an error.  In the future, we will check to see if we can
-        // convert.
-        return false;
-    }
-    int bpp = SkBitmap::ComputeBytesPerPixel(SkImageInfoToBitmapConfig(info));
-    if (static_cast<size_t>(bpp * info.fWidth) > rowBytes) {
-        return false;
-    }
+    decoder->setDitherImage(fOpts.fDitherImage);
+    decoder->setPreferQualityOverSpeed(fOpts.fPreferQualityOverSpeed);
+    decoder->setSampleSize(fOpts.fSampleSize);
+
     SkBitmap bitmap;
-    if (!bitmap.setConfig(info, rowBytes)) {
-        return false;
-    }
-
-    TargetAllocator allocator(pixels, rowBytes, info);
-    if (!fDoCopyTo) {
-        decoder->setAllocator(&allocator);
-    }
-    bool success = decoder->decode(fStream, &bitmap,
+    TargetAllocator allocator(pixels, rowBytes, info.fWidth,
+                              info.fHeight, fOpts.fRequestedConfig);
+    decoder->setAllocator(&allocator);
+    bool success = decoder->decode(fStream, &bitmap, fOpts.fRequestedConfig,
                                    SkImageDecoder::kDecodePixels_Mode);
     decoder->setAllocator(NULL);
     if (!success) {
         return false;
     }
-    if (fDoCopyTo) {
-        SkBitmap bm8888;
-        bitmap.copyTo(&bm8888, SkBitmap::kARGB_8888_Config, &allocator);
+    if (allocator.isReady()) {  // Did not use pixels!
+        SkBitmap bm;
+        SkASSERT(bitmap.canCopyTo(fOpts.fRequestedConfig));
+        bitmap.copyTo(&bm, fOpts.fRequestedConfig, &allocator);
+        if (allocator.isReady()) {
+            SkDEBUGFAIL("bitmap.copyTo(requestedConfig) failed.");
+            return false;
+        }
+        SkASSERT(check_alpha(fInfo.fAlphaType, bm.alphaType()));
+    } else {
+        SkASSERT(check_alpha(fInfo.fAlphaType, bitmap.alphaType()));
     }
     return true;
 }
+
 bool SkDecodingImageGenerator::Install(SkData* data, SkBitmap* dst,
+                                       const Options* opts,
                                        SkDiscardableMemory::Factory* factory) {
     SkASSERT(data != NULL);
     SkASSERT(dst != NULL);
-    SkImageGenerator* gen(SkNEW_ARGS(SkDecodingImageGenerator, (data)));
+    SkImageGenerator* gen(SkNEW_ARGS(SkDecodingImageGenerator, (data, opts)));
     return SkInstallDiscardablePixelRef(gen, dst, factory);
 }
 
 bool SkDecodingImageGenerator::Install(SkStreamRewindable* stream,
                                        SkBitmap* dst,
+                                       const Options* opts,
                                        SkDiscardableMemory::Factory* factory) {
     SkASSERT(stream != NULL);
     SkASSERT(dst != NULL);
     if ((stream == NULL) || !stream->unique()) {
         SkSafeUnref(stream);
         return false;
     }
-    SkImageGenerator* gen(SkNEW_ARGS(SkDecodingImageGenerator, (stream)));
+    SkImageGenerator* gen(SkNEW_ARGS(SkDecodingImageGenerator, (stream, opts)));
     return SkInstallDiscardablePixelRef(gen, dst, factory);
 }