SkBitmapRegionDecoder clean-up
Use SkCodecPrintf instead of SkDebugf.
Check if the conversion is possible rather than starting many decodes
that will certainly fail.
Small refactor to code that deals with subsets that fall outside
of the image.
BUG=skia:
Review URL: https://codereview.chromium.org/1395383002
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 88371a8..c7b389d 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -114,6 +114,10 @@
return Error::Nonfatal(SkStringPrintf("Could not create brd for %s.", fPath.c_str()));
}
+ if (!brd->conversionSupported(colorType)) {
+ return Error::Nonfatal("Cannot convert to color type.\n");
+ }
+
const uint32_t width = brd->width();
const uint32_t height = brd->height();
// Visually inspecting very small output images is not necessary.
diff --git a/tools/SkBitmapRegionCanvas.cpp b/tools/SkBitmapRegionCanvas.cpp
index 086ac19..7226e04 100644
--- a/tools/SkBitmapRegionCanvas.cpp
+++ b/tools/SkBitmapRegionCanvas.cpp
@@ -7,6 +7,8 @@
#include "SkBitmapRegionCanvas.h"
#include "SkCanvas.h"
+#include "SkCodecPriv.h"
+#include "SkCodecTools.h"
SkBitmapRegionCanvas::SkBitmapRegionCanvas(SkCodec* decoder)
: INHERITED(decoder->getInfo().width(), decoder->getInfo().height())
@@ -15,8 +17,11 @@
/*
* Chooses the correct image subset offsets and dimensions for the partial decode.
+ *
+ * @return true if the subset is completely contained within the image
+ * false otherwise
*/
-static inline void set_subset_region(int inputOffset, int inputDimension,
+static bool set_subset_region(int inputOffset, int inputDimension,
int imageOriginalDimension, int* imageSubsetOffset, int* outOffset,
int* imageSubsetDimension) {
@@ -30,17 +35,8 @@
// Use outOffset to make sure we don't decode pixels past the edge of the region.
*imageSubsetDimension = SkTMin(imageOriginalDimension - *imageSubsetOffset,
inputDimension - *outOffset);
-}
-/*
- * Returns a scaled dimension based on the original dimension and the sample size.
- * TODO: Share this implementation with SkScaledCodec.
- */
-static int get_scaled_dimension(int srcDimension, int sampleSize) {
- if (sampleSize > srcDimension) {
- return 1;
- }
- return srcDimension / sampleSize;
+ return (*outOffset == 0) && (*imageSubsetDimension == inputDimension);
}
/*
@@ -56,7 +52,7 @@
SkColorType dstColorType) {
// Reject color types not supported by this method
if (kIndex_8_SkColorType == dstColorType || kGray_8_SkColorType == dstColorType) {
- SkDebugf("Error: Color type not supported.\n");
+ SkCodecPrintf("Error: Color type not supported.\n");
return nullptr;
}
@@ -79,7 +75,8 @@
// bitmap. If the region is not fully contained within the image, this
// will not be the same as inputWidth.
int imageSubsetWidth;
- set_subset_region(inputX, inputWidth, this->width(), &imageSubsetX, &outX, &imageSubsetWidth);
+ bool imageContainsEntireSubset = set_subset_region(inputX, inputWidth, this->width(),
+ &imageSubsetX, &outX, &imageSubsetWidth);
// The top offset of the portion of the image we want, where zero
// indicates the top edge of the image.
@@ -96,11 +93,11 @@
// bitmap. If the region is not fully contained within the image, this
// will not be the same as inputHeight.
int imageSubsetHeight;
- set_subset_region(inputY, inputHeight, this->height(), &imageSubsetY, &outY,
- &imageSubsetHeight);
+ imageContainsEntireSubset &= set_subset_region(inputY, inputHeight, this->height(),
+ &imageSubsetY, &outY, &imageSubsetHeight);
if (imageSubsetWidth <= 0 || imageSubsetHeight <= 0) {
- SkDebugf("Error: Region must intersect part of the image.\n");
+ SkCodecPrintf("Error: Region must intersect part of the image.\n");
return nullptr;
}
@@ -115,7 +112,7 @@
// Start the scanline decoder
SkCodec::Result r = fDecoder->startScanlineDecode(decodeInfo);
if (SkCodec::kSuccess != r) {
- SkDebugf("Error: Could not start scanline decoder.\n");
+ SkCodecPrintf("Error: Could not start scanline decoder.\n");
return nullptr;
}
@@ -123,13 +120,13 @@
SkBitmap tmp;
SkImageInfo tmpInfo = decodeInfo.makeWH(this->width(), imageSubsetHeight);
if (!tmp.tryAllocPixels(tmpInfo)) {
- SkDebugf("Error: Could not allocate pixels.\n");
+ SkCodecPrintf("Error: Could not allocate pixels.\n");
return nullptr;
}
// Skip the unneeded rows
if (!fDecoder->skipScanlines(imageSubsetY)) {
- SkDebugf("Error: Failed to skip scanlines.\n");
+ SkCodecPrintf("Error: Failed to skip scanlines.\n");
return nullptr;
}
@@ -144,7 +141,7 @@
SkAutoTDelete<SkBitmap> bitmap(new SkBitmap());
SkImageInfo dstInfo = decodeInfo.makeWH(outWidth, outHeight);
if (!bitmap->tryAllocPixels(dstInfo)) {
- SkDebugf("Error: Could not allocate pixels.\n");
+ SkCodecPrintf("Error: Could not allocate pixels.\n");
return nullptr;
}
@@ -155,9 +152,7 @@
// TODO (msarett): This could be skipped if memory is zero initialized.
// This would matter if this code is moved to Android and
// uses Android bitmaps.
- if (0 != outX || 0 != outY ||
- inputX + inputWidth > this->width() ||
- inputY + inputHeight > this->height()) {
+ if (imageContainsEntireSubset) {
bitmap->eraseColor(0);
}
@@ -177,3 +172,15 @@
return bitmap.detach();
}
+
+bool SkBitmapRegionCanvas::conversionSupported(SkColorType colorType) {
+ // SkCanvas does not draw to these color types.
+ if (kIndex_8_SkColorType == colorType || kGray_8_SkColorType == colorType) {
+ return false;
+ }
+
+ // FIXME: Call virtual function when it lands.
+ SkImageInfo info = SkImageInfo::Make(0, 0, colorType, fDecoder->getInfo().alphaType(),
+ fDecoder->getInfo().profileType());
+ return conversion_possible(info, fDecoder->getInfo());
+}
diff --git a/tools/SkBitmapRegionCanvas.h b/tools/SkBitmapRegionCanvas.h
index 05eca8b..f82e9fa 100644
--- a/tools/SkBitmapRegionCanvas.h
+++ b/tools/SkBitmapRegionCanvas.h
@@ -34,6 +34,8 @@
SkBitmap* decodeRegion(int start_x, int start_y, int width, int height,
int sampleSize, SkColorType prefColorType) override;
+ bool conversionSupported(SkColorType colorType) override;
+
private:
SkAutoTDelete<SkCodec> fDecoder;
diff --git a/tools/SkBitmapRegionDecoderInterface.cpp b/tools/SkBitmapRegionDecoderInterface.cpp
index eea2441..835ed9a 100644
--- a/tools/SkBitmapRegionDecoderInterface.cpp
+++ b/tools/SkBitmapRegionDecoderInterface.cpp
@@ -9,6 +9,7 @@
#include "SkBitmapRegionDecoderInterface.h"
#include "SkBitmapRegionSampler.h"
#include "SkCodec.h"
+#include "SkCodecPriv.h"
#include "SkImageDecoder.h"
SkBitmapRegionDecoderInterface* SkBitmapRegionDecoderInterface::CreateBitmapRegionDecoder(
@@ -19,11 +20,11 @@
SkImageDecoder* decoder = SkImageDecoder::Factory(stream);
int width, height;
if (nullptr == decoder) {
- SkDebugf("Error: Could not create image decoder.\n");
+ SkCodecPrintf("Error: Could not create image decoder.\n");
return nullptr;
}
if (!decoder->buildTileIndex(streamDeleter.detach(), &width, &height)) {
- SkDebugf("Error: Could not build tile index.\n");
+ SkCodecPrintf("Error: Could not build tile index.\n");
delete decoder;
return nullptr;
}
@@ -32,7 +33,7 @@
case kCanvas_Strategy: {
SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(streamDeleter.detach()));
if (nullptr == codec) {
- SkDebugf("Error: Failed to create decoder.\n");
+ SkCodecPrintf("Error: Failed to create decoder.\n");
return nullptr;
}
switch (codec->getScanlineOrder()) {
@@ -40,7 +41,7 @@
case SkCodec::kNone_SkScanlineOrder:
break;
default:
- SkDebugf("Error: Scanline ordering not supported.\n");
+ SkCodecPrintf("Error: Scanline ordering not supported.\n");
return nullptr;
}
return new SkBitmapRegionCanvas(codec.detach());
diff --git a/tools/SkBitmapRegionDecoderInterface.h b/tools/SkBitmapRegionDecoderInterface.h
index bc28c2b..047f023 100644
--- a/tools/SkBitmapRegionDecoderInterface.h
+++ b/tools/SkBitmapRegionDecoderInterface.h
@@ -56,6 +56,11 @@
virtual SkBitmap* decodeRegion(int start_x, int start_y, int width,
int height, int sampleSize,
SkColorType colorType) = 0;
+ /*
+ * @param Requested destination color type
+ * @return true if we support the requested color type and false otherwise
+ */
+ virtual bool conversionSupported(SkColorType colorType) = 0;
int width() const { return fWidth; }
int height() const { return fHeight; }
diff --git a/tools/SkBitmapRegionSampler.cpp b/tools/SkBitmapRegionSampler.cpp
index 98c183d..514af10 100644
--- a/tools/SkBitmapRegionSampler.cpp
+++ b/tools/SkBitmapRegionSampler.cpp
@@ -6,6 +6,7 @@
*/
#include "SkBitmapRegionSampler.h"
+#include "SkCodecPriv.h"
SkBitmapRegionSampler::SkBitmapRegionSampler(SkImageDecoder* decoder, int width,
int height)
@@ -43,7 +44,7 @@
SkAutoTDelete<SkBitmap> bitmap(new SkBitmap());
if (!fDecoder->decodeSubset(bitmap.get(), region, prefColorType)) {
- SkDebugf("Error: decodeRegion failed.\n");
+ SkCodecPrintf("Error: decodeRegion failed.\n");
return nullptr;
}
return bitmap.detach();
diff --git a/tools/SkBitmapRegionSampler.h b/tools/SkBitmapRegionSampler.h
index d2f738d..8ed95e2 100644
--- a/tools/SkBitmapRegionSampler.h
+++ b/tools/SkBitmapRegionSampler.h
@@ -32,6 +32,14 @@
SkBitmap* decodeRegion(int start_x, int start_y, int width, int height,
int sampleSize, SkColorType prefColorType) override;
+ bool conversionSupported(SkColorType colorType) override {
+ // SkBitmapRegionSampler does not allow the client to check if the conversion
+ // is supported. We will return true as a default. If the conversion is in
+ // fact not supported, decodeRegion() will ignore the prefColorType and choose
+ // its own color type. We catch this and fail non-fatally in our test code.
+ return true;
+ }
+
private:
SkAutoTDelete<SkImageDecoder> fDecoder;