Qualify the return value of SkImageDecoder::decode
Add a new enum to differentiate between a complete decode and a
partial decode (with the third value being failure). Return this
value from SkImageDecoder::onDecode (in all subclasses, plus
SkImageDecoder_empty) and ::decode.
For convenience, if the enum is treated as a boolean, success and
partial success are both considered true.
Note that the static helper functions (DecodeFile etc) still return
true and false (for one thing, this allows us to continue to use
SkImageDecoder::DecodeMemory as an SkPicture::InstallPixelRefProc in
SkPicture::CreateFromStream).
Also correctly report failure in SkASTCImageDecoder::onDecode when
SkTextureCompressor::DecompressBufferFromFormat fails.
BUG=skia:3037
BUG:b/17419670
Review URL: https://codereview.chromium.org/647023006
diff --git a/src/images/SkImageDecoder_libjpeg.cpp b/src/images/SkImageDecoder_libjpeg.cpp
index 4103eef..082adca 100644
--- a/src/images/SkImageDecoder_libjpeg.cpp
+++ b/src/images/SkImageDecoder_libjpeg.cpp
@@ -238,7 +238,7 @@
virtual bool onBuildTileIndex(SkStreamRewindable *stream, int *width, int *height) SK_OVERRIDE;
virtual bool onDecodeSubset(SkBitmap* bitmap, const SkIRect& rect) SK_OVERRIDE;
#endif
- virtual bool onDecode(SkStream* stream, SkBitmap* bm, Mode) SK_OVERRIDE;
+ virtual Result onDecode(SkStream* stream, SkBitmap* bm, Mode) SK_OVERRIDE;
virtual bool onDecodeYUV8Planes(SkStream* stream, SkISize componentSizes[3],
void* planes[3], size_t rowBytes[3],
SkYUVColorSpace* colorSpace) SK_OVERRIDE;
@@ -325,9 +325,11 @@
}
#endif
+///////////////////////////////////////////////////////////////////////////////
+
// This guy exists just to aid in debugging, as it allows debuggers to just
// set a break-point in one place to see all error exists.
-static bool return_false(const jpeg_decompress_struct& cinfo,
+static void print_jpeg_decoder_errors(const jpeg_decompress_struct& cinfo,
int width, int height, const char caller[]) {
if (!(c_suppressJPEGImageDecoderErrors)) {
char buffer[JMSG_LENGTH_MAX];
@@ -335,14 +337,28 @@
SkDebugf("libjpeg error %d <%s> from %s [%d %d]\n",
cinfo.err->msg_code, buffer, caller, width, height);
}
- return false; // must always return false
+}
+
+static bool return_false(const jpeg_decompress_struct& cinfo,
+ const char caller[]) {
+ print_jpeg_decoder_errors(cinfo, 0, 0, caller);
+ return false;
}
static bool return_false(const jpeg_decompress_struct& cinfo,
const SkBitmap& bm, const char caller[]) {
- return return_false(cinfo, bm.width(), bm.height(), caller);
+ print_jpeg_decoder_errors(cinfo, bm.width(), bm.height(), caller);
+ return false;
}
+static SkImageDecoder::Result return_failure(const jpeg_decompress_struct& cinfo,
+ const SkBitmap& bm, const char caller[]) {
+ print_jpeg_decoder_errors(cinfo, bm.width(), bm.height(), caller);
+ return SkImageDecoder::kFailure;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
// Convert a scanline of CMYK samples to RGBX in place. Note that this
// method moves the "scanline" pointer in its processing
static void convert_CMYK_to_RGB(uint8_t* scanline, unsigned int width) {
@@ -491,7 +507,6 @@
#endif
}
-
/**
Sets all pixels in given bitmap to SK_ColorWHITE for all rows >= y.
Used when decoding fails partway through reading scanlines to fill
@@ -537,7 +552,7 @@
return true;
}
-bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
+SkImageDecoder::Result SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
#ifdef TIME_DECODE
SkAutoTime atm("JPEG Decode");
#endif
@@ -553,7 +568,7 @@
// All objects need to be instantiated before this setjmp call so that
// they will be cleaned up properly if an error occurs.
if (setjmp(errorManager.fJmpBuf)) {
- return return_false(cinfo, *bm, "setjmp");
+ return return_failure(cinfo, *bm, "setjmp");
}
initialize_info(&cinfo, &srcManager);
@@ -561,7 +576,7 @@
int status = jpeg_read_header(&cinfo, true);
if (status != JPEG_HEADER_OK) {
- return return_false(cinfo, *bm, "read_header");
+ return return_failure(cinfo, *bm, "read_header");
}
/* Try to fulfill the requested sampleSize. Since jpeg can do it (when it
@@ -588,8 +603,9 @@
// individual pixel. It is very unlikely to be opaque, since
// an opaque A8 bitmap would not be very interesting.
// Otherwise, a jpeg image is opaque.
- return bm->setInfo(SkImageInfo::Make(cinfo.image_width, cinfo.image_height,
- colorType, alphaType));
+ bool success = bm->setInfo(SkImageInfo::Make(cinfo.image_width, cinfo.image_height,
+ colorType, alphaType));
+ return success ? kSuccess : kFailure;
}
/* image_width and image_height are the original dimensions, available
@@ -613,10 +629,11 @@
// individual pixel. It is very unlikely to be opaque, since
// an opaque A8 bitmap would not be very interesting.
// Otherwise, a jpeg image is opaque.
- return bm->setInfo(SkImageInfo::Make(smpl.scaledWidth(), smpl.scaledHeight(),
- colorType, alphaType));
+ bool success = bm->setInfo(SkImageInfo::Make(smpl.scaledWidth(), smpl.scaledHeight(),
+ colorType, alphaType));
+ return success ? kSuccess : kFailure;
} else {
- return return_false(cinfo, *bm, "start_decompress");
+ return return_failure(cinfo, *bm, "start_decompress");
}
}
sampleSize = recompute_sampleSize(sampleSize, cinfo);
@@ -624,7 +641,7 @@
#ifdef SK_SUPPORT_LEGACY_IMAGEDECODER_CHOOSER
// should we allow the Chooser (if present) to pick a colortype for us???
if (!this->chooseFromOneChoice(colorType, cinfo.output_width, cinfo.output_height)) {
- return return_false(cinfo, *bm, "chooseFromOneChoice");
+ return return_failure(cinfo, *bm, "chooseFromOneChoice");
}
#endif
@@ -636,10 +653,10 @@
bm->setInfo(SkImageInfo::Make(sampler.scaledWidth(), sampler.scaledHeight(),
colorType, alphaType));
if (SkImageDecoder::kDecodeBounds_Mode == mode) {
- return true;
+ return kSuccess;
}
if (!this->allocPixelRef(bm, NULL)) {
- return return_false(cinfo, *bm, "allocPixelRef");
+ return return_failure(cinfo, *bm, "allocPixelRef");
}
SkAutoLockPixels alp(*bm);
@@ -662,15 +679,16 @@
// so return early. We will return a partial image.
fill_below_level(cinfo.output_scanline, bm);
cinfo.output_scanline = cinfo.output_height;
- break; // Skip to jpeg_finish_decompress()
+ jpeg_finish_decompress(&cinfo);
+ return kPartialSuccess;
}
if (this->shouldCancelDecode()) {
- return return_false(cinfo, *bm, "shouldCancelDecode");
+ return return_failure(cinfo, *bm, "shouldCancelDecode");
}
rowptr += bpr;
}
jpeg_finish_decompress(&cinfo);
- return true;
+ return kSuccess;
}
#endif
@@ -679,11 +697,11 @@
int srcBytesPerPixel;
if (!get_src_config(cinfo, &sc, &srcBytesPerPixel)) {
- return return_false(cinfo, *bm, "jpeg colorspace");
+ return return_failure(cinfo, *bm, "jpeg colorspace");
}
if (!sampler.begin(bm, sc, *this)) {
- return return_false(cinfo, *bm, "sampler.begin");
+ return return_failure(cinfo, *bm, "sampler.begin");
}
SkAutoMalloc srcStorage(cinfo.output_width * srcBytesPerPixel);
@@ -691,7 +709,7 @@
// Possibly skip initial rows [sampler.srcY0]
if (!skip_src_rows(&cinfo, srcRow, sampler.srcY0())) {
- return return_false(cinfo, *bm, "skip rows");
+ return return_failure(cinfo, *bm, "skip rows");
}
// now loop through scanlines until y == bm->height() - 1
@@ -703,10 +721,11 @@
// so return early. We will return a partial image.
fill_below_level(y, bm);
cinfo.output_scanline = cinfo.output_height;
- break; // Skip to jpeg_finish_decompress()
+ jpeg_finish_decompress(&cinfo);
+ return kSuccess;
}
if (this->shouldCancelDecode()) {
- return return_false(cinfo, *bm, "shouldCancelDecode");
+ return return_failure(cinfo, *bm, "shouldCancelDecode");
}
if (JCS_CMYK == cinfo.out_color_space) {
@@ -720,20 +739,22 @@
}
if (!skip_src_rows(&cinfo, srcRow, sampler.srcDY() - 1)) {
- return return_false(cinfo, *bm, "skip rows");
+ return return_failure(cinfo, *bm, "skip rows");
}
}
// we formally skip the rest, so we don't get a complaint from libjpeg
if (!skip_src_rows(&cinfo, srcRow,
cinfo.output_height - cinfo.output_scanline)) {
- return return_false(cinfo, *bm, "skip rows");
+ return return_failure(cinfo, *bm, "skip rows");
}
jpeg_finish_decompress(&cinfo);
- return true;
+ return kSuccess;
}
+///////////////////////////////////////////////////////////////////////////////
+
enum SizeType {
kSizeForMemoryAllocation_SizeType,
kActualSize_SizeType
@@ -818,8 +839,9 @@
}
JDIMENSION scanlinesRead = jpeg_read_raw_data(&cinfo, bufferraw, yScanlinesToRead);
- if (scanlinesRead == 0)
+ if (scanlinesRead == 0) {
return false;
+ }
if (hasYLastRow) {
memcpy(&outputY[yMaxH * rowBytesY], yLastRow, yWidth);
@@ -857,7 +879,7 @@
// All objects need to be instantiated before this setjmp call so that
// they will be cleaned up properly if an error occurs.
if (setjmp(errorManager.fJmpBuf)) {
- return return_false(cinfo, 0, 0, "setjmp YUV8");
+ return return_false(cinfo, "setjmp YUV8");
}
initialize_info(&cinfo, &srcManager);
@@ -865,7 +887,7 @@
int status = jpeg_read_header(&cinfo, true);
if (status != JPEG_HEADER_OK) {
- return return_false(cinfo, 0, 0, "read_header YUV8");
+ return return_false(cinfo, "read_header YUV8");
}
if (cinfo.jpeg_color_space != JCS_YCbCr) {
@@ -897,11 +919,11 @@
jpeg_start_decompress(), and then read output_width and output_height.
*/
if (!jpeg_start_decompress(&cinfo)) {
- return return_false(cinfo, 0, 0, "start_decompress YUV8");
+ return return_false(cinfo, "start_decompress YUV8");
}
if (!output_raw_data(cinfo, planes, rowBytes)) {
- return return_false(cinfo, 0, 0, "output_raw_data");
+ return return_false(cinfo, "output_raw_data");
}
update_components_sizes(cinfo, componentSizes, kActualSize_SizeType);
@@ -914,6 +936,8 @@
return true;
}
+///////////////////////////////////////////////////////////////////////////////
+
#ifdef SK_BUILD_FOR_ANDROID
bool SkJPEGImageDecoder::onBuildTileIndex(SkStreamRewindable* stream, int *width, int *height) {