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 "SkData.h"
 #include "SkDecodingImageGenerator.h"
-#include "SkData.h"
 #include "SkImageDecoder.h"
+#include "SkImageInfo.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)
         : 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)
+            || (fConfig != bm->config())
+            || (fWidth != bm->width())
+            || (fHeight != bm->height())
+            || (ct != NULL)) {
+            // Call default allcoator.

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

allocator*

[hal.canary, 2013/12/16 15:10:27]

Done.

+            return bm->allocPixels(NULL, ct);
         }
-        bm->setConfig(bm->config(), bm->width(), bm->height(),
-                      fRowBytes, bm->alphaType());
-        bm->setPixels(fTarget, ct);
+        // make sure fRowBytes is correct.
+        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;  // never alloc smae pixels twice!

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

same*

[hal.canary, 2013/12/16 15:10:27]

Done.

         return true;
     }
 
 private:
-    void* fTarget;
-    size_t fRowBytes;
-    SkImageInfo fInfo;
+    void* fTarget;  // Pointer to block of memory.

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

While these comments are true, I don't think they're particularly helpful for
someone trying to read and understand this class. How about something like:

fTarget; // Block of memory to be supplied as pixel memory in allocPixelRef.
Must be large enough to hold a bitmap described by fWidth, fHeight, and
fRowBytes.

fRowBytes; // rowbytes for the destination bitmap
fWidth; // Along with fHeight and fConfig, the information about the bitmap
whose pixels this allocator is expected to allocate. If they do not match the
bitmap passed to allocPixelRef, it is assumed that the bitmap will be copied to
a bitmap with the correct info using this allocator, so the default allocator
will be used instead of fTarget.
fHeight;
fConfig;

[hal.canary, 2013/12/16 15:10:27]

Done.

+    size_t fRowBytes;  // Will make sure target bitmap knows this.
+    int fWidth;  // Verify that the target bitmap
+    int fHeight;  //  has correct dimensions.
+    SkBitmap::Config fConfig;  // And correct config.
     typedef SkBitmap::Allocator INHERITED;
 };
-}  // namespace
-////////////////////////////////////////////////////////////////////////////////
 
-SkDecodingImageGenerator::SkDecodingImageGenerator(SkData* data)
-    : fData(data)
-    , fHasInfo(false)
-    , fDoCopyTo(false) {
-    SkASSERT(fData != NULL);
-    fStream = SkNEW_ARGS(SkMemoryStream, (fData));
-    SkASSERT(fStream != NULL);
-    SkASSERT(fStream->unique());
-    fData->ref();
-}
+/**
+ *  An implementation of SkImageGenerator that calls into
+ *  SkImageDecoder.  Since the consumers of this class only rely on
+ *  virtual functions, we hide all implementation details inside this
+ *  source file.  This implementaion of getInfo() always returns true.
+ */
+class DecodingImageGenerator : public SkImageGenerator {
+public:
+    DecodingImageGenerator(SkData* data,
+                           SkStreamRewindable* stream,
+                           const SkImageInfo& info,
+                           int sampleSize,
+                           bool ditherImage,
+                           SkBitmap::Config requestedConfig)
+        : fData(data)
+        , fStream(stream)
+        , fInfo(info)
+        , fSampleSize(sampleSize)
+        , fDitherImage(ditherImage)
+        , fRequestedConfig(requestedConfig) {
+        SkASSERT(stream != NULL);
+        SkSafeRef(fData);  // may be NULL.
+    }
+    virtual ~DecodingImageGenerator() {
+        SkSafeUnref(fData);
+        fStream->unref();
+    }
 
-SkDecodingImageGenerator::SkDecodingImageGenerator(SkStreamRewindable* stream)
-    : fData(NULL)
-    , fStream(stream)
-    , fHasInfo(false)
-    , fDoCopyTo(false) {
-    SkASSERT(fStream != NULL);
-    SkASSERT(fStream->unique());
-}
+    virtual SkData* refEncodedData() SK_OVERRIDE;
 
-SkDecodingImageGenerator::~SkDecodingImageGenerator() {
-    SkSafeUnref(fData);
-    fStream->unref();
-}
+    virtual bool getInfo(SkImageInfo* info) SK_OVERRIDE {
+        if (info != NULL) {
+            *info = fInfo;
+        }
+        return true;
+    }
 
-// TODO(halcanary): Give this macro a better name and move it into SkTypes.h
+    virtual bool getPixels(const SkImageInfo& info,
+                           void* pixels,
+                           size_t rowBytes) SK_OVERRIDE;
+
+private:
+    SkData*                fData;
+    SkStreamRewindable*    fStream;
+    const SkImageInfo      fInfo;
+    const int              fSampleSize;
+    const bool             fDitherImage;
+    const SkBitmap::Config fRequestedConfig;
+    typedef SkImageGenerator INHERITED;
+};
+
 #ifdef SK_DEBUG
     #define SkCheckResult(expr, value)  SkASSERT((value) == (expr))
 #else
     #define SkCheckResult(expr, value)  (void)(expr)
 #endif
 
-SkData* SkDecodingImageGenerator::refEncodedData() {
+
+SkData* DecodingImageGenerator::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);
 }
 
-bool SkDecodingImageGenerator::getInfo(SkImageInfo* info) {
-    // info can be NULL.  If so, will update fInfo, fDoCopyTo, and fHasInfo.
-    if (fHasInfo) {
-        if (info != NULL) {
-            *info = fInfo;
+// 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;
+}
+
+// A contructor-type function that returns NULL on failure.  This
+// prevents the returned SkImageGenerator from ever being in a bad
+// state.
+static SkImageGenerator* create_decoding_generator(SkData* data,
+                                                   SkStreamRewindable* stream,
+                                                   const SkDecoderOptions& opts) {
+    SkASSERT(stream);
+    SkAutoTUnref<SkStreamRewindable> autoStream(stream);  // always unref this.
+    if (SkBitmap::kIndex8_Config == opts.fRequestedConfig) {
+        // We do not support indexed color with SkImageGenerators,
+        return NULL;
+    }
+    SkAssertResult(autoStream->rewind());
+    SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(autoStream));
+    if (NULL == decoder.get()) {
+        return NULL;
+    }
+    SkBitmap bitmap;
+    decoder->setSampleSize(opts.fSampleSize);
+    if (!decoder->decode(stream, &bitmap,
+                         SkImageDecoder::kDecodeBounds_Mode)) {
+        return NULL;
+    }
+    if (bitmap.config() == SkBitmap::kNo_Config) {
+        return NULL;
+    }
+
+    SkImageInfo info;
+    SkBitmap::Config config = opts.fRequestedConfig;
+    if (opts.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.
+            config = SkBitmap::kARGB_8888_Config;
+            info.fWidth = bitmap.width();
+            info.fHeight = bitmap.height();
+            info.fColorType = kPMColor_SkColorType;
+            info.fAlphaType = bitmap.alphaType();
+        } else {
+            config = bitmap.config();  // Save for later!
+            if (!bitmap.asImageInfo(&info)) {
+                SkDEBUGFAIL("Getting SkImageInfo from bitmap failed.");
+                return NULL;
+            }
         }
-        return true;
+    } else {
+        if (!bitmap.canCopyTo(opts.fRequestedConfig)) {
+            SkASSERT(SkBitmap::kARGB_4444_Config == opts.fRequestedConfig);

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

I don't think this assert belongs here. What if canCopyTo changes in the future?

[hal.canary, 2013/12/16 15:10:27]

Done.

+            SkASSERT(bitmap.config() != opts.fRequestedConfig);
+            return NULL;  // Can not translate to needed config.
+        }
+        info.fWidth = bitmap.width();
+        info.fHeight = bitmap.height();
+        if (!config_to_colorType(opts.fRequestedConfig,
+                                 &(info.fColorType))) {
+            SkDEBUGFAIL("SkBitmapConfigToColorType fails.");
+            return NULL;
+        }
+        info.fAlphaType = bitmap.alphaType();
     }
+    return SkNEW_ARGS(DecodingImageGenerator,
+                      (data, autoStream.detach(), info,
+                       opts.fSampleSize, opts.fDitherImage, config));
+}
+
+inline bool check_alpha(SkAlphaType reported, SkAlphaType actual) {
+    return ((reported == actual)
+            || ((reported == kPremul_SkAlphaType)
+                && (actual == kOpaque_SkAlphaType)));
+}
+
+bool DecodingImageGenerator::getPixels(const SkImageInfo& info,
+                                       void* pixels,
+                                       size_t rowBytes) {
+    if (NULL == pixels) {
+        return false;
+    }
+    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(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;
     }
+    decoder->setDitherImage(fDitherImage);
+    decoder->setSampleSize(fSampleSize);
+
     SkBitmap bitmap;
-    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)");
-            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;
-}
-
-bool SkDecodingImageGenerator::getPixels(const SkImageInfo& info,
-                                         void* pixels,
-                                         size_t rowBytes) {
-    if (NULL == pixels) {
-        return false;
-    }
-    if (!this->getInfo(NULL)) {
-        return false;
-    }
-    if (SkImageInfoToBitmapConfig(info) == SkBitmap::kNo_Config) {
-        return false;  // Unsupported SkColorType.
-    }
-    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;
-    }
-    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, fRequestedConfig);
+    decoder->setAllocator(&allocator);
+    bool success = decoder->decode(fStream, &bitmap, 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(fRequestedConfig));
+        bitmap.copyTo(&bm, fRequestedConfig, &allocator);

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

Check the return value of copyTo?

[hal.canary, 2013/12/16 15:10:27]

Done.

+        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,
-                                       SkDiscardableMemory::Factory* factory) {
+}  // namespace
+////////////////////////////////////////////////////////////////////////////////
+
+SkImageGenerator* SkNewDecodingImageGenerator(SkData* data,
+                                              const SkDecoderOptions& opts) {
     SkASSERT(data != NULL);
-    SkASSERT(dst != NULL);
-    SkImageGenerator* gen(SkNEW_ARGS(SkDecodingImageGenerator, (data)));
-    return SkInstallDiscardablePixelRef(gen, dst, factory);
+    if (NULL == data) {
+        return NULL;
+    }
+    SkStreamRewindable* stream = SkNEW_ARGS(SkMemoryStream, (data));
+    SkASSERT(stream != NULL);
+    SkASSERT(stream->unique());
+    return create_decoding_generator(data, stream, opts);
 }
 
-bool SkDecodingImageGenerator::Install(SkStreamRewindable* stream,
-                                       SkBitmap* dst,
-                                       SkDiscardableMemory::Factory* factory) {
+SkImageGenerator* SkNewDecodingImageGenerator(SkStreamRewindable* stream,
+                                              const SkDecoderOptions& opts) {
     SkASSERT(stream != NULL);
-    SkASSERT(dst != NULL);
+    SkASSERT(stream->unique());
     if ((stream == NULL) || !stream->unique()) {
         SkSafeUnref(stream);
         return false;
     }
-    SkImageGenerator* gen(SkNEW_ARGS(SkDecodingImageGenerator, (stream)));
-    return SkInstallDiscardablePixelRef(gen, dst, factory);
+    return create_decoding_generator(NULL, stream, opts);
 }
+