From bc69ce982f8374742ca910587485f0d741350c2d Mon Sep 17 00:00:00 2001 From: "scroggo@google.com" Date: Tue, 9 Jul 2013 15:45:14 +0000 Subject: [PATCH] Remove bitmap reuse from SkImageDecoder. Now that Android is using an SkBitmap::Allocator to reuse bitmap memory, remove the unnecessary code to handle bitmap reuse inside the decoders themselves. Leaves in the code for bitmap reuse in decodeSubset, which still may reuse bitmaps, and cropBitmap, which is called by decodeSubset. R=djsollen@google.com Review URL: https://codereview.chromium.org/17620004 git-svn-id: http://skia.googlecode.com/svn/trunk@9931 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkImageDecoder.h | 23 +++++++++++----- src/images/SkImageDecoder.cpp | 9 +------ src/images/SkImageDecoder_libbmp.cpp | 14 +++------- src/images/SkImageDecoder_libgif.cpp | 8 +----- src/images/SkImageDecoder_libico.cpp | 8 ++---- src/images/SkImageDecoder_libjpeg.cpp | 38 ++++++--------------------- src/images/SkImageDecoder_libpng.cpp | 26 +++--------------- src/images/SkImageDecoder_libwebp.cpp | 19 ++++---------- src/images/SkImageDecoder_wbmp.cpp | 15 +++-------- src/ports/SkImageDecoder_empty.cpp | 2 +- 10 files changed, 46 insertions(+), 116 deletions(-) diff --git a/include/core/SkImageDecoder.h b/include/core/SkImageDecoder.h index 360f03acb6..67c8257827 100644 --- a/include/core/SkImageDecoder.h +++ b/include/core/SkImageDecoder.h @@ -117,10 +117,10 @@ public: Peeker* getPeeker() const { return fPeeker; } Peeker* setPeeker(Peeker*); - /** \class Peeker + /** \class Chooser - Base class for optional callbacks to retrieve meta/chunk data out of - an image as it is being decoded. + Base class for optional callbacks to choose an image from a format that + contains multiple images. */ class Chooser : public SkRefCnt { public: @@ -219,11 +219,20 @@ public: * to pref if possible. Whether a conversion is feasible is * tested by Bitmap::canCopyTo(pref). - note: document use of Allocator, Peeker and Chooser + If an SkBitmap::Allocator is installed via setAllocator, it will be + used to allocate the pixel memory. A clever allocator can be used + to allocate the memory from a cache, volatile memory, or even from + an existing bitmap's memory. + + If a Peeker is installed via setPeeker, it may be used to peek into + meta data during the decode. + + If a Chooser is installed via setChooser, it may be used to select + which image to return from a format that contains multiple images. */ - bool decode(SkStream*, SkBitmap* bitmap, SkBitmap::Config pref, Mode, bool reuseBitmap = false); - bool decode(SkStream* stream, SkBitmap* bitmap, Mode mode, bool reuseBitmap = false) { - return this->decode(stream, bitmap, SkBitmap::kNo_Config, mode, reuseBitmap); + bool decode(SkStream*, SkBitmap* bitmap, SkBitmap::Config pref, Mode); + bool decode(SkStream* stream, SkBitmap* bitmap, Mode mode) { + return this->decode(stream, bitmap, SkBitmap::kNo_Config, mode); } /** diff --git a/src/images/SkImageDecoder.cpp b/src/images/SkImageDecoder.cpp index 38aa7fda0d..0aa752e38f 100644 --- a/src/images/SkImageDecoder.cpp +++ b/src/images/SkImageDecoder.cpp @@ -164,19 +164,12 @@ SkBitmap::Config SkImageDecoder::getPrefConfig(SrcDepth srcDepth, } bool SkImageDecoder::decode(SkStream* stream, SkBitmap* bm, - SkBitmap::Config pref, Mode mode, bool reuseBitmap) { + SkBitmap::Config pref, Mode mode) { // we reset this to false before calling onDecode fShouldCancelDecode = false; // assign this, for use by getPrefConfig(), in case fUsePrefTable is false fDefaultPref = pref; - if (reuseBitmap) { - SkAutoLockPixels alp(*bm); - if (NULL != bm->getPixels()) { - return this->onDecode(stream, bm, mode); - } - } - // pass a temporary bitmap, so that if we return false, we are assured of // leaving the caller's bitmap untouched. SkBitmap tmp; diff --git a/src/images/SkImageDecoder_libbmp.cpp b/src/images/SkImageDecoder_libbmp.cpp index 5c2299b77c..14b9090f9b 100644 --- a/src/images/SkImageDecoder_libbmp.cpp +++ b/src/images/SkImageDecoder_libbmp.cpp @@ -130,19 +130,13 @@ bool SkBMPImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) { SkScaledBitmapSampler sampler(width, height, getSampleSize()); - if (justBounds) { - bm->setConfig(config, sampler.scaledWidth(), sampler.scaledHeight()); - bm->setIsOpaque(true); - return true; - } - // No Bitmap reuse supported for this format - if (!bm->isNull()) { - return false; - } - bm->setConfig(config, sampler.scaledWidth(), sampler.scaledHeight()); bm->setIsOpaque(true); + if (justBounds) { + return true; + } + if (!this->allocPixelRef(bm, NULL)) { return false; } diff --git a/src/images/SkImageDecoder_libgif.cpp b/src/images/SkImageDecoder_libgif.cpp index f6c54c2dc0..d368eccd92 100644 --- a/src/images/SkImageDecoder_libgif.cpp +++ b/src/images/SkImageDecoder_libgif.cpp @@ -202,17 +202,11 @@ bool SkGIFImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* bm, Mode mode) { return error_return(gif, *bm, "chooseFromOneChoice"); } + bm->setConfig(SkBitmap::kIndex8_Config, width, height); if (SkImageDecoder::kDecodeBounds_Mode == mode) { - bm->setConfig(SkBitmap::kIndex8_Config, width, height); return true; } - // No Bitmap reuse supported for this format - if (!bm->isNull()) { - return false; - } - - bm->setConfig(SkBitmap::kIndex8_Config, width, height); SavedImage* image = &gif->SavedImages[gif->ImageCount-1]; const GifImageDesc& desc = image->ImageDesc; diff --git a/src/images/SkImageDecoder_libico.cpp b/src/images/SkImageDecoder_libico.cpp index 195b6ff08b..6335136fa0 100644 --- a/src/images/SkImageDecoder_libico.cpp +++ b/src/images/SkImageDecoder_libico.cpp @@ -232,16 +232,12 @@ bool SkICOImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) //if the andbitmap (mask) is all zeroes, then we can easily do an index bitmap //however, with small images with large colortables, maybe it's better to still do argb_8888 + bm->setConfig(SkBitmap::kARGB_8888_Config, w, h, calculateRowBytesFor8888(w, bitCount)); + if (SkImageDecoder::kDecodeBounds_Mode == mode) { - bm->setConfig(SkBitmap::kARGB_8888_Config, w, h, calculateRowBytesFor8888(w, bitCount)); delete[] colors; return true; } - // No Bitmap reuse supported for this format - if (!bm->isNull()) { - return false; - } - bm->setConfig(SkBitmap::kARGB_8888_Config, w, h, calculateRowBytesFor8888(w, bitCount)); if (!this->allocPixelRef(bm, NULL)) { diff --git a/src/images/SkImageDecoder_libjpeg.cpp b/src/images/SkImageDecoder_libjpeg.cpp index 4818743605..bea1e98641 100644 --- a/src/images/SkImageDecoder_libjpeg.cpp +++ b/src/images/SkImageDecoder_libjpeg.cpp @@ -357,29 +357,13 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) { } SkScaledBitmapSampler sampler(cinfo.output_width, cinfo.output_height, sampleSize); - - bm->lockPixels(); - JSAMPLE* rowptr = (JSAMPLE*)bm->getPixels(); - bm->unlockPixels(); - bool reuseBitmap = (rowptr != NULL); - - if (reuseBitmap) { - if (sampler.scaledWidth() != bm->width() || - sampler.scaledHeight() != bm->height()) { - // Dimensions must match - return false; - } else if (SkImageDecoder::kDecodeBounds_Mode == mode) { - return true; - } - } else { - bm->setConfig(config, sampler.scaledWidth(), sampler.scaledHeight()); - bm->setIsOpaque(true); - if (SkImageDecoder::kDecodeBounds_Mode == mode) { - return true; - } - if (!this->allocPixelRef(bm, NULL)) { - return return_false(cinfo, *bm, "allocPixelRef"); - } + bm->setConfig(config, sampler.scaledWidth(), sampler.scaledHeight()); + bm->setIsOpaque(true); + if (SkImageDecoder::kDecodeBounds_Mode == mode) { + return true; + } + if (!this->allocPixelRef(bm, NULL)) { + return return_false(cinfo, *bm, "allocPixelRef"); } SkAutoLockPixels alp(*bm); @@ -394,7 +378,7 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) { (config == SkBitmap::kRGB_565_Config && cinfo.out_color_space == JCS_RGB_565))) { - rowptr = (JSAMPLE*)bm->getPixels(); + JSAMPLE* rowptr = (JSAMPLE*)bm->getPixels(); INT32 const bpr = bm->rowBytes(); while (cinfo.output_scanline < cinfo.output_height) { @@ -409,9 +393,6 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) { } rowptr += bpr; } - if (reuseBitmap) { - bm->notifyPixelsChanged(); - } jpeg_finish_decompress(&cinfo); return true; } @@ -481,9 +462,6 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) { cinfo.output_height - cinfo.output_scanline)) { return return_false(cinfo, *bm, "skip rows"); } - if (reuseBitmap) { - bm->notifyPixelsChanged(); - } jpeg_finish_decompress(&cinfo); return true; diff --git a/src/images/SkImageDecoder_libpng.cpp b/src/images/SkImageDecoder_libpng.cpp index 729f87169a..d59a67bf8e 100644 --- a/src/images/SkImageDecoder_libpng.cpp +++ b/src/images/SkImageDecoder_libpng.cpp @@ -299,21 +299,8 @@ bool SkPNGImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* decodedBitmap, const int sampleSize = this->getSampleSize(); SkScaledBitmapSampler sampler(origWidth, origHeight, sampleSize); + decodedBitmap->setConfig(config, sampler.scaledWidth(), sampler.scaledHeight()); - decodedBitmap->lockPixels(); - void* rowptr = (void*) decodedBitmap->getPixels(); - bool reuseBitmap = (rowptr != NULL); - decodedBitmap->unlockPixels(); - if (reuseBitmap && (sampler.scaledWidth() != decodedBitmap->width() || - sampler.scaledHeight() != decodedBitmap->height())) { - // Dimensions must match - return false; - } - - if (!reuseBitmap) { - decodedBitmap->setConfig(config, sampler.scaledWidth(), - sampler.scaledHeight()); - } if (SkImageDecoder::kDecodeBounds_Mode == mode) { return true; } @@ -332,11 +319,9 @@ bool SkPNGImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* decodedBitmap, SkAutoUnref aur(colorTable); - if (!reuseBitmap) { - if (!this->allocPixelRef(decodedBitmap, - SkBitmap::kIndex8_Config == config ? colorTable : NULL)) { - return false; - } + if (!this->allocPixelRef(decodedBitmap, + SkBitmap::kIndex8_Config == config ? colorTable : NULL)) { + return false; } SkAutoLockPixels alp(*decodedBitmap); @@ -445,9 +430,6 @@ bool SkPNGImageDecoder::onDecode(SkStream* sk_stream, SkBitmap* decodedBitmap, return false; } decodedBitmap->setIsOpaque(!reallyHasAlpha); - if (reuseBitmap) { - decodedBitmap->notifyPixelsChanged(); - } return true; } diff --git a/src/images/SkImageDecoder_libwebp.cpp b/src/images/SkImageDecoder_libwebp.cpp index 9cf84493ad..b0fa7f7053 100644 --- a/src/images/SkImageDecoder_libwebp.cpp +++ b/src/images/SkImageDecoder_libwebp.cpp @@ -412,25 +412,16 @@ bool SkWEBPImageDecoder::onDecode(SkStream* stream, SkBitmap* decodedBitmap, const int sampleSize = this->getSampleSize(); SkScaledBitmapSampler sampler(origWidth, origHeight, sampleSize); - - // If only bounds are requested, done - if (SkImageDecoder::kDecodeBounds_Mode == mode) { - if (!setDecodeConfig(decodedBitmap, sampler.scaledWidth(), - sampler.scaledHeight())) { - return false; - } - return true; - } - - // No Bitmap reuse supported for this format - if (!decodedBitmap->isNull()) { - return false; - } if (!setDecodeConfig(decodedBitmap, sampler.scaledWidth(), sampler.scaledHeight())) { return false; } + // If only bounds are requested, done + if (SkImageDecoder::kDecodeBounds_Mode == mode) { + return true; + } + if (!this->allocPixelRef(decodedBitmap, NULL)) { return return_false(*decodedBitmap, "allocPixelRef"); } diff --git a/src/images/SkImageDecoder_wbmp.cpp b/src/images/SkImageDecoder_wbmp.cpp index db40f595fa..8b1659ba8d 100644 --- a/src/images/SkImageDecoder_wbmp.cpp +++ b/src/images/SkImageDecoder_wbmp.cpp @@ -111,20 +111,13 @@ bool SkWBMPImageDecoder::onDecode(SkStream* stream, SkBitmap* decodedBitmap, int width = head.fWidth; int height = head.fHeight; - if (SkImageDecoder::kDecodeBounds_Mode == mode) { - decodedBitmap->setConfig(SkBitmap::kIndex8_Config, width, height); - decodedBitmap->setIsOpaque(true); - return true; - } - - // No Bitmap reuse supported for this format - if (!decodedBitmap->isNull()) { - return false; - } - decodedBitmap->setConfig(SkBitmap::kIndex8_Config, width, height); decodedBitmap->setIsOpaque(true); + if (SkImageDecoder::kDecodeBounds_Mode == mode) { + return true; + } + const SkPMColor colors[] = { SK_ColorBLACK, SK_ColorWHITE }; SkColorTable* ct = SkNEW_ARGS(SkColorTable, (colors, 2)); SkAutoUnref aur(ct); diff --git a/src/ports/SkImageDecoder_empty.cpp b/src/ports/SkImageDecoder_empty.cpp index ccfeb272a3..94db139656 100644 --- a/src/ports/SkImageDecoder_empty.cpp +++ b/src/ports/SkImageDecoder_empty.cpp @@ -27,7 +27,7 @@ bool SkImageDecoder::DecodeFile(const char[], SkBitmap*, SkBitmap::Config, return false; } -bool SkImageDecoder::decode(SkStream*, SkBitmap*, SkBitmap::Config, Mode, bool) { +bool SkImageDecoder::decode(SkStream*, SkBitmap*, SkBitmap::Config, Mode) { return false; }