Refactor SkBitmapRegionDecoderInterface for Android

The result SkBitmap, the pixel allocator, and the alpha
preference need to be communicated from the client to
the region decoder.

BUG=skia:

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

10 files changed, 105 insertions, 135 deletions

diff --git a/bench/BitmapRegionDecoderBench.cpp b/bench/BitmapRegionDecoderBench.cpp
index 092693619b..70e3e760e0 100644
--- a/bench/BitmapRegionDecoderBench.cpp
+++ b/bench/BitmapRegionDecoderBench.cpp
@@ -61,10 +61,8 @@ void BitmapRegionDecoderBench::onDelayedSetup() {
 }
 
 void BitmapRegionDecoderBench::onDraw(int n, SkCanvas* canvas) {
-    SkAutoTDelete<SkBitmap> bitmap;
     for (int i = 0; i < n; i++) {
-        bitmap.reset(fBRD->decodeRegion(fSubset.left(), fSubset.top(), fSubset.width(),
-                fSubset.height(), fSampleSize, fColorType));
-        SkASSERT(nullptr != bitmap.get());
+        SkBitmap bm;
+        SkAssertResult(fBRD->decodeRegion(&bm, nullptr, fSubset, fSampleSize, fColorType, false));
     }
 }
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index 054fbb27b1..de4545f2fa 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -603,9 +603,12 @@ static bool valid_brd_bench(SkData* encoded, SkBitmapRegionDecoderInterface::Str
         return false;
     }
 
-    SkAutoTDelete<SkBitmap> bitmap(brd->decodeRegion(0, 0, brd->width(), brd->height(), 1,
-            colorType));
-    if (nullptr == bitmap.get() || colorType != bitmap->colorType()) {
+    SkBitmap bitmap;
+    if (!brd->decodeRegion(&bitmap, nullptr, SkIRect::MakeXYWH(0, 0, brd->width(), brd->height()),
+            1, colorType, false)) {
+        return false;
+    }
+    if (colorType != bitmap.colorType()) {
         // This indicates that conversion to the requested color type is not supported for the
         // particular image.
         return false;
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index e0ec5e139c..b14c573679 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -125,12 +125,18 @@ Error BRDSrc::draw(SkCanvas* canvas) const {
     }
     switch (fMode) {
         case kFullImage_Mode: {
-            SkAutoTDelete<SkBitmap> bitmap(brd->decodeRegion(0, 0, width, height, fSampleSize,
-                    colorType));
-            if (nullptr == bitmap.get() || colorType != bitmap->colorType()) {
+            SkBitmap bitmap;
+            if (!brd->decodeRegion(&bitmap, nullptr, SkIRect::MakeXYWH(0, 0, width, height),
+                    fSampleSize, colorType, false)) {
+                // FIXME: Make this a fatal error.  We need to disable webps for kCanvas_Strategy
+                // because we have not implemented kCanvas_Strategy for webp.  We may also need to
+                // deal with color conversion errors for kOriginal_Strategy.
+                return Error::Nonfatal("Cannot decode region.\n");
+            }
+            if (colorType != bitmap.colorType()) {
                 return Error::Nonfatal("Cannot convert to color type.\n");
             }
-            canvas->drawBitmap(*bitmap, 0, 0);
+            canvas->drawBitmap(bitmap, 0, 0);
             return "";
         }
         case kDivisor_Mode: {
@@ -178,13 +184,20 @@ Error BRDSrc::draw(SkCanvas* canvas) const {
                     const int decodeTop = top - unscaledBorder;
                     const uint32_t decodeWidth = subsetWidth + unscaledBorder * 2;
                     const uint32_t decodeHeight = subsetHeight + unscaledBorder * 2;
-                    SkAutoTDelete<SkBitmap> bitmap(brd->decodeRegion(decodeLeft,
-                            decodeTop, decodeWidth, decodeHeight, fSampleSize, colorType));
-                    if (nullptr == bitmap.get() || colorType != bitmap->colorType()) {
+                    SkBitmap bitmap;
+                    if (!brd->decodeRegion(&bitmap, nullptr, SkIRect::MakeXYWH(decodeLeft,
+                            decodeTop, decodeWidth, decodeHeight), fSampleSize, colorType, false)) {
+                        // FIXME: Make this a fatal error.  We need to disable webps for
+                        // kCanvas_Strategy because we have not implemented kCanvas_Strategy for
+                        // webp.  We may also need to deal with color conversion errors for
+                        // kOriginal_Strategy.
+                        return Error::Nonfatal("Cannot not decode region.\n");
+                    }
+                    if (colorType != bitmap.colorType()) {
                         return Error::Nonfatal("Cannot convert to color type.\n");
                     }
 
-                    canvas->drawBitmapRect(*bitmap,
+                    canvas->drawBitmapRect(bitmap,
                             SkRect::MakeXYWH((SkScalar) scaledBorder, (SkScalar) scaledBorder,
                                     (SkScalar) (subsetWidth / fSampleSize),
                                     (SkScalar) (subsetHeight / fSampleSize)),
diff --git a/tools/SkBitmapRegionCanvas.cpp b/tools/SkBitmapRegionCanvas.cpp
index f9c9573c44..9129245b11 100644
--- a/tools/SkBitmapRegionCanvas.cpp
+++ b/tools/SkBitmapRegionCanvas.cpp
@@ -15,23 +15,27 @@ SkBitmapRegionCanvas::SkBitmapRegionCanvas(SkCodec* decoder)
     , fDecoder(decoder)
 {}
 
-/*
- * Three differences from the Android version:
- *     Returns a Skia bitmap instead of an Android bitmap.
- *     Android version attempts to reuse a recycled bitmap.
- *     Removed the options object and used parameters for color type and
- *     sample size.
- */
-SkBitmap* SkBitmapRegionCanvas::decodeRegion(int inputX, int inputY,
-                                             int inputWidth, int inputHeight,
-                                             int sampleSize,
-                                             SkColorType dstColorType) {
+bool SkBitmapRegionCanvas::decodeRegion(SkBitmap* bitmap, SkBitmap::Allocator* allocator,
+        const SkIRect& desiredSubset, int sampleSize, SkColorType dstColorType,
+        bool requireUnpremul) {
     // Reject color types not supported by this method
     if (kIndex_8_SkColorType == dstColorType || kGray_8_SkColorType == dstColorType) {
         SkCodecPrintf("Error: Color type not supported.\n");
-        return nullptr;
+        return false;
+    }
+
+    // Reject requests for unpremultiplied alpha
+    if (requireUnpremul) {
+        SkCodecPrintf("Error: Alpha type not supported.\n");
+        return false;
+    }
+    SkAlphaType dstAlphaType = fDecoder->getInfo().alphaType();
+    if (kUnpremul_SkAlphaType == dstAlphaType) {
+        dstAlphaType = kPremul_SkAlphaType;
     }
 
+    // FIXME: Can we add checks and support kIndex8 or unpremultiplied alpha in special cases?
+
     // Fix the input sampleSize if necessary.
     if (sampleSize < 1) {
         sampleSize = 1;
@@ -48,17 +52,13 @@ SkBitmap* SkBitmapRegionCanvas::decodeRegion(int inputX, int inputY,
     // If outY is non-zero, subsetY must be zero.
     int outX;
     int outY;
-    SkIRect subset = SkIRect::MakeXYWH(inputX, inputY, inputWidth, inputHeight);
+    SkIRect subset = desiredSubset;
     SubsetType type = adjust_subset_rect(fDecoder->getInfo().dimensions(), &subset, &outX, &outY);
     if (SubsetType::kOutside_SubsetType == type) {
-        return nullptr;
+        return false;
     }
 
     // Create the image info for the decode
-    SkAlphaType dstAlphaType = fDecoder->getInfo().alphaType();
-    if (kUnpremul_SkAlphaType == dstAlphaType) {
-        dstAlphaType = kPremul_SkAlphaType;
-    }
     SkImageInfo decodeInfo = SkImageInfo::Make(this->width(), this->height(),
             dstColorType, dstAlphaType);
 
@@ -66,7 +66,7 @@ SkBitmap* SkBitmapRegionCanvas::decodeRegion(int inputX, int inputY,
     SkCodec::Result r = fDecoder->startScanlineDecode(decodeInfo);
     if (SkCodec::kSuccess != r) {
         SkCodecPrintf("Error: Could not start scanline decoder.\n");
-        return nullptr;
+        return false;
     }
 
     // Allocate a bitmap for the unscaled decode
@@ -74,28 +74,28 @@ SkBitmap* SkBitmapRegionCanvas::decodeRegion(int inputX, int inputY,
     SkImageInfo tmpInfo = decodeInfo.makeWH(this->width(), subset.height());
     if (!tmp.tryAllocPixels(tmpInfo)) {
         SkCodecPrintf("Error: Could not allocate pixels.\n");
-        return nullptr;
+        return false;
     }
 
     // Skip the unneeded rows
     if (!fDecoder->skipScanlines(subset.y())) {
         SkCodecPrintf("Error: Failed to skip scanlines.\n");
-        return nullptr;
+        return false;
     }
 
     // Decode the necessary rows
     fDecoder->getScanlines(tmp.getAddr(0, 0), subset.height(), tmp.rowBytes());
 
     // Calculate the size of the output
-    const int outWidth = get_scaled_dimension(inputWidth, sampleSize);
-    const int outHeight = get_scaled_dimension(inputHeight, sampleSize);
+    const int outWidth = get_scaled_dimension(desiredSubset.width(), sampleSize);
+    const int outHeight = get_scaled_dimension(desiredSubset.height(), sampleSize);
 
     // Initialize the destination bitmap
-    SkAutoTDelete<SkBitmap> bitmap(new SkBitmap());
     SkImageInfo dstInfo = decodeInfo.makeWH(outWidth, outHeight);
-    if (!bitmap->tryAllocPixels(dstInfo)) {
+    bitmap->setInfo(dstInfo, dstInfo.minRowBytes());
+    if (!bitmap->tryAllocPixels(allocator, nullptr)) {
         SkCodecPrintf("Error: Could not allocate pixels.\n");
-        return nullptr;
+        return false;
     }
 
     // Zero the bitmap if the region is not completely within the image.
@@ -123,7 +123,7 @@ SkBitmap* SkBitmapRegionCanvas::decodeRegion(int inputX, int inputY,
     // TODO (msarett): Test multiple filter qualities.  kNone is the default.
     canvas.drawBitmapRect(tmp, src, dst, &paint);
 
-    return bitmap.detach();
+    return true;
 }
 
 bool SkBitmapRegionCanvas::conversionSupported(SkColorType colorType) {
diff --git a/tools/SkBitmapRegionCanvas.h b/tools/SkBitmapRegionCanvas.h
index f82e9fa77a..217082ac33 100644
--- a/tools/SkBitmapRegionCanvas.h
+++ b/tools/SkBitmapRegionCanvas.h
@@ -24,15 +24,9 @@ public:
      */
     SkBitmapRegionCanvas(SkCodec* decoder);
 
-    /*
-     * Three differences from the Android version:
-     *     Returns a Skia bitmap instead of an Android bitmap.
-     *     Android version attempts to reuse a recycled bitmap.
-     *     Removed the options object and used parameters for color type and
-     *     sample size.
-     */
-    SkBitmap* decodeRegion(int start_x, int start_y, int width, int height,
-                           int sampleSize, SkColorType prefColorType) override;
+    bool decodeRegion(SkBitmap* bitmap, SkBitmap::Allocator* allocator,
+                      const SkIRect& desiredSubset, int sampleSize,
+                      SkColorType colorType, bool requireUnpremul) override;
 
     bool conversionSupported(SkColorType colorType) override;
 
diff --git a/tools/SkBitmapRegionCodec.cpp b/tools/SkBitmapRegionCodec.cpp
index 3f2cd24b78..cac99452e0 100644
--- a/tools/SkBitmapRegionCodec.cpp
+++ b/tools/SkBitmapRegionCodec.cpp
@@ -15,15 +15,9 @@ SkBitmapRegionCodec::SkBitmapRegionCodec(SkAndroidCodec* codec)
     , fCodec(codec)
 {}
 
-/*
- * Three differences from the Android version:
- *     Returns a skia bitmap instead of an Android bitmap.
- *     Android version attempts to reuse a recycled bitmap.
- *     Removed the options object and used parameters for color type and sample size.
- */
-// FIXME: Should this function should take in SkIRect?
-SkBitmap* SkBitmapRegionCodec::decodeRegion(int inputX, int inputY, int inputWidth, int inputHeight,
-        int sampleSize, SkColorType dstColorType) {
+bool SkBitmapRegionCodec::decodeRegion(SkBitmap* bitmap, SkBitmap::Allocator* allocator,
+        const SkIRect& desiredSubset, int sampleSize, SkColorType dstColorType,
+        bool requireUnpremul) {
 
     // Fix the input sampleSize if necessary.
     if (sampleSize < 1) {
@@ -41,23 +35,23 @@ SkBitmap* SkBitmapRegionCodec::decodeRegion(int inputX, int inputY, int inputWid
     // If outY is non-zero, subsetY must be zero.
     int outX;
     int outY;
-    SkIRect subset = SkIRect::MakeXYWH(inputX, inputY, inputWidth, inputHeight);
+    SkIRect subset = desiredSubset;
     SubsetType type = adjust_subset_rect(fCodec->getInfo().dimensions(), &subset, &outX, &outY);
     if (SubsetType::kOutside_SubsetType == type) {
-        return nullptr;
+        return false;
     }
 
     // Ask the codec for a scaled subset
     if (!fCodec->getSupportedSubset(&subset)) {
         SkCodecPrintf("Error: Could not get subset.\n");
-        return nullptr;
+        return false;
     }
     SkISize scaledSize = fCodec->getSampledSubsetDimensions(sampleSize, subset);
 
     // Create the image info for the decode
     SkAlphaType dstAlphaType = fCodec->getInfo().alphaType();
-    if (kUnpremul_SkAlphaType == dstAlphaType) {
-        dstAlphaType = kPremul_SkAlphaType;
+    if (kOpaque_SkAlphaType != dstAlphaType) {
+        dstAlphaType = requireUnpremul ? kUnpremul_SkAlphaType : kPremul_SkAlphaType;
     }
     SkImageInfo decodeInfo = SkImageInfo::Make(scaledSize.width(), scaledSize.height(),
             dstColorType, dstAlphaType);
@@ -83,7 +77,6 @@ SkBitmap* SkBitmapRegionCodec::decodeRegion(int inputX, int inputY, int inputWid
     }
 
     // Initialize the destination bitmap
-    SkAutoTDelete<SkBitmap> bitmap(new SkBitmap());
     int scaledOutX = 0;
     int scaledOutY = 0;
     int scaledOutWidth = scaledSize.width();
@@ -92,17 +85,18 @@ SkBitmap* SkBitmapRegionCodec::decodeRegion(int inputX, int inputY, int inputWid
         scaledOutX = outX / sampleSize;
         scaledOutY = outY / sampleSize;
         // We need to be safe here because getSupportedSubset() may have modified the subset.
-        const int extraX = SkTMax(0, inputWidth - outX - subset.width());
-        const int extraY = SkTMax(0, inputHeight - outY - subset.height());
+        const int extraX = SkTMax(0, desiredSubset.width() - outX - subset.width());
+        const int extraY = SkTMax(0, desiredSubset.height() - outY - subset.height());
         const int scaledExtraX = extraX / sampleSize;
         const int scaledExtraY = extraY / sampleSize;
         scaledOutWidth += scaledOutX + scaledExtraX;
         scaledOutHeight += scaledOutY + scaledExtraY;
     }
     SkImageInfo outInfo = decodeInfo.makeWH(scaledOutWidth, scaledOutHeight);
-    if (!bitmap->tryAllocPixels(outInfo, nullptr, colorTable.get())) {
+    bitmap->setInfo(outInfo, outInfo.minRowBytes());
+    if (!bitmap->tryAllocPixels(allocator, colorTable.get())) {
         SkCodecPrintf("Error: Could not allocate pixels.\n");
-        return nullptr;
+        return false;
     }
 
     // Zero the bitmap if the region is not completely within the image.
@@ -129,10 +123,10 @@ SkBitmap* SkBitmapRegionCodec::decodeRegion(int inputX, int inputY, int inputWid
     SkCodec::Result result = fCodec->getAndroidPixels(decodeInfo, dst, rowBytes, &options);
     if (SkCodec::kSuccess != result && SkCodec::kIncompleteInput != result) {
         SkCodecPrintf("Error: Could not get pixels.\n");
-        return nullptr;
+        return false;
     }
 
-    return bitmap.detach();
+    return true;
 }
 
 bool SkBitmapRegionCodec::conversionSupported(SkColorType colorType) {
diff --git a/tools/SkBitmapRegionCodec.h b/tools/SkBitmapRegionCodec.h
index 14d024ea45..4dd69ab0fa 100644
--- a/tools/SkBitmapRegionCodec.h
+++ b/tools/SkBitmapRegionCodec.h
@@ -20,15 +20,9 @@ public:
      */
     SkBitmapRegionCodec(SkAndroidCodec* codec);
 
-    /*
-     * Three differences from the Android version:
-     *     Returns a Skia bitmap instead of an Android bitmap.
-     *     Android version attempts to reuse a recycled bitmap.
-     *     Removed the options object and used parameters for color type and
-     *     sample size.
-     */
-    SkBitmap* decodeRegion(int start_x, int start_y, int width, int height,
-                           int sampleSize, SkColorType prefColorType) override;
+    bool decodeRegion(SkBitmap* bitmap, SkBitmap::Allocator* allocator,
+                      const SkIRect& desiredSubset, int sampleSize,
+                      SkColorType colorType, bool requireUnpremul) override;
 
     bool conversionSupported(SkColorType colorType) override;
 
diff --git a/tools/SkBitmapRegionDecoderInterface.h b/tools/SkBitmapRegionDecoderInterface.h
index e1f79bccb6..a6a3111457 100644
--- a/tools/SkBitmapRegionDecoderInterface.h
+++ b/tools/SkBitmapRegionDecoderInterface.h
@@ -35,26 +35,24 @@ public:
     /*
      * Decode a scaled region of the encoded image stream
      *
-     * @param start_x    X-coordinate of upper-left corner of region.
-     *                   This coordinate is unscaled, relative to the original dimensions.
-     * @param start_y    Y-coordinate of upper-left corner of region.
-     *                   This coordinate is unscaled, relative to the original dimensions.
-     * @param width      Width of the region to decode.
-     *                   This distance is unscaled, relative to the original dimensions.
-     * @param height     Height of the region to decode.
-     *                   This distance is unscaled, relative to the original dimensions.
-     * @param sampleSize An integer downscaling factor for the decode.
-     * @param colorType  Preferred output colorType.
-     *                   New implementations should return NULL if they do not support
-     *                   decoding to this color type.
-     *                   The old kOriginal_Strategy will decode to a default color type
-     *                   if this color type is unsupported.
-     * @return           Pointer to a bitmap of the decoded region on success, NULL on
-     *                   failure.
+     * @param bitmap          Container for decoded pixels.  It is assumed that the pixels
+     *                        are initially unallocated and will be allocated by this function.
+     * @param allocator       Allocator for the pixels.  If this is NULL, the default
+     *                        allocator (HeapAllocator) will be used.
+     * @param desiredSubset   Subset of the original image to decode.
+     * @param sampleSize      An integer downscaling factor for the decode.
+     * @param colorType       Preferred output colorType.
+     *                        New implementations should return NULL if they do not support
+     *                        decoding to this color type.
+     *                        The old kOriginal_Strategy will decode to a default color type
+     *                        if this color type is unsupported.
+     * @param requireUnpremul If the image is not opaque, we will use this to determine the
+     *                        alpha type to use.
+     *
      */
-    virtual SkBitmap* decodeRegion(int start_x, int start_y, int width,
-                                   int height, int sampleSize,
-                                   SkColorType colorType) = 0;
+    virtual bool decodeRegion(SkBitmap* bitmap, SkBitmap::Allocator* allocator,
+                              const SkIRect& desiredSubset, int sampleSize,
+                              SkColorType colorType, bool requireUnpremul) = 0;
     /*
      * @param  Requested destination color type
      * @return true if we support the requested color type and false otherwise
diff --git a/tools/SkBitmapRegionSampler.cpp b/tools/SkBitmapRegionSampler.cpp
index 514af10859..0714c26fbf 100644
--- a/tools/SkBitmapRegionSampler.cpp
+++ b/tools/SkBitmapRegionSampler.cpp
@@ -14,38 +14,20 @@ SkBitmapRegionSampler::SkBitmapRegionSampler(SkImageDecoder* decoder, int width,
     , fDecoder(decoder)
 {}
 
-/*
- * Three differences from the Android version:
- *     Returns a Skia bitmap instead of an Android bitmap.
- *     Android version attempts to reuse a recycled bitmap.
- *     Removed the options object and used parameters for color type and
- *     sample size.
- */
-SkBitmap* SkBitmapRegionSampler::decodeRegion(int start_x, int start_y,
-                                              int width, int height,
-                                              int sampleSize,
-                                              SkColorType prefColorType) {
-    // Match Android's default settings
+bool SkBitmapRegionSampler::decodeRegion(SkBitmap* bitmap, SkBitmap::Allocator* allocator,
+        const SkIRect& desiredSubset, int sampleSize, SkColorType colorType, bool requireUnpremul) {
     fDecoder->setDitherImage(true);
     fDecoder->setPreferQualityOverSpeed(false);
     fDecoder->setRequireUnpremultipliedColors(false);
     fDecoder->setSampleSize(sampleSize);
+    fDecoder->setAllocator(allocator);
 
     // kAlpha8 is the legacy representation of kGray8 used by SkImageDecoder
-    if (kGray_8_SkColorType == prefColorType) {
-        prefColorType = kAlpha_8_SkColorType;
+    if (kGray_8_SkColorType == colorType) {
+        colorType = kAlpha_8_SkColorType;
     }
 
-    SkIRect region;
-    region.fLeft = start_x;
-    region.fTop = start_y;
-    region.fRight = start_x + width;
-    region.fBottom = start_y + height;
-
-    SkAutoTDelete<SkBitmap> bitmap(new SkBitmap());
-    if (!fDecoder->decodeSubset(bitmap.get(), region, prefColorType)) {
-        SkCodecPrintf("Error: decodeRegion failed.\n");
-        return nullptr;
-    }
-    return bitmap.detach();
+    bool result = fDecoder->decodeSubset(bitmap, desiredSubset, colorType);
+    fDecoder->setAllocator(nullptr);
+    return result;
 }
diff --git a/tools/SkBitmapRegionSampler.h b/tools/SkBitmapRegionSampler.h
index 8ed95e2524..be2d23961e 100644
--- a/tools/SkBitmapRegionSampler.h
+++ b/tools/SkBitmapRegionSampler.h
@@ -22,15 +22,9 @@ public:
      */
     SkBitmapRegionSampler(SkImageDecoder* decoder, int width, int height);
 
-    /*
-     * Three differences from the Android version:
-     *     Returns a Skia bitmap instead of an Android bitmap.
-     *     Android version attempts to reuse a recycled bitmap.
-     *     Removed the options object and used parameters for color type and
-     *     sample size.
-     */
-    SkBitmap* decodeRegion(int start_x, int start_y, int width, int height,
-                           int sampleSize, SkColorType prefColorType) override;
+    bool decodeRegion(SkBitmap* bitmap, SkBitmap::Allocator* allocator,
+                      const SkIRect& desiredSubset, int sampleSize,
+                      SkColorType colorType, bool requireUnpremul) override;
 
     bool conversionSupported(SkColorType colorType) override {
         // SkBitmapRegionSampler does not allow the client to check if the conversion