Consolidate SkCodec functions for handling rewind
Previously, many of our codec implementations followed the same
pattern (often in a function named handleRewind):
switch (this->rewindIfNeeded()) {
case CouldNotRewind:
return CouldNotRewind;
case NoRewindNecessary:
// keep going
break;
case Rewound:
<re-read header etc>
break;
}
In this CL, remove the enum, and put the piece that happens in the
Rewound case into a virtual function, onRewind. rewindIfNeeded now
contains the common pieces from various functions named handleRewind.
In SkBmpCodec, add a function that returns whether the BMP is in ICO,
so it can have a common implementation for onRewind.
BUG=skia:3257
Review URL: https://codereview.chromium.org/1288483002
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index ce58163..a0bd18a 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -233,26 +233,27 @@
virtual bool onReallyHasAlpha() const { return false; }
- enum RewindState {
- kRewound_RewindState,
- kNoRewindNecessary_RewindState,
- kCouldNotRewind_RewindState
- };
/**
* If the stream was previously read, attempt to rewind.
- * @returns:
- * kRewound if the stream needed to be rewound, and the
- * rewind succeeded.
- * kNoRewindNecessary if the stream did not need to be
- * rewound.
- * kCouldNotRewind if the stream needed to be rewound, and
- * rewind failed.
+ *
+ * If the stream needed to be rewound, call onRewind.
+ * @returns true if the codec is at the right position and can be used.
+ * false if there was a failure to rewind.
*
* Subclasses MUST call this function before reading the stream (e.g. in
* onGetPixels). If it returns false, onGetPixels should return
* kCouldNotRewind.
*/
- RewindState SK_WARN_UNUSED_RESULT rewindIfNeeded();
+ bool SK_WARN_UNUSED_RESULT rewindIfNeeded();
+
+ /**
+ * Called by rewindIfNeeded, if the stream needed to be rewound.
+ *
+ * Subclasses should do any set up needed after a rewind.
+ */
+ virtual bool onRewind() {
+ return true;
+ }
/**
* Get method for the input stream
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 4383382..5eb5354 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -533,19 +533,8 @@
, fRowOrder(rowOrder)
{}
-/*
- * Rewinds the image stream if necessary
- */
-bool SkBmpCodec::handleRewind(bool inIco) {
- SkCodec::RewindState rewindState = this->rewindIfNeeded();
- if (rewindState == kCouldNotRewind_RewindState) {
- return false;
- } else if (rewindState == kRewound_RewindState) {
- if (!SkBmpCodec::ReadHeader(this->stream(), inIco, NULL)) {
- return false;
- }
- }
- return true;
+bool SkBmpCodec::onRewind() {
+ return SkBmpCodec::ReadHeader(this->stream(), this->inIco(), NULL);
}
/*
diff --git a/src/codec/SkBmpCodec.h b/src/codec/SkBmpCodec.h
index 51d0300..71006f7 100644
--- a/src/codec/SkBmpCodec.h
+++ b/src/codec/SkBmpCodec.h
@@ -63,10 +63,18 @@
*/
static bool ReadHeader(SkStream*, bool inIco, SkCodec** codecOut);
+ bool onRewind() override;
+
/*
- * Rewinds the image stream if necessary
+ * Returns whether this BMP is part of an ICO image.
*/
- bool handleRewind(bool inIco);
+ bool inIco() const {
+ return this->onInIco();
+ }
+
+ virtual bool onInIco() const {
+ return false;
+ }
/*
* Get the destination row to start filling from
diff --git a/src/codec/SkBmpMaskCodec.cpp b/src/codec/SkBmpMaskCodec.cpp
index f30266b..7d1706e 100644
--- a/src/codec/SkBmpMaskCodec.cpp
+++ b/src/codec/SkBmpMaskCodec.cpp
@@ -56,7 +56,7 @@
const Options& opts,
SkPMColor* inputColorPtr,
int* inputColorCount) {
- if (!this->handleRewind(false)) {
+ if (!this->rewindIfNeeded()) {
return kCouldNotRewind;
}
if (opts.fSubset) {
diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index 828871c..6be7449 100644
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -66,7 +66,7 @@
const Options& opts,
SkPMColor* inputColorPtr,
int* inputColorCount) {
- if (!this->handleRewind(false)) {
+ if (!this->rewindIfNeeded()) {
return kCouldNotRewind;
}
if (opts.fSubset) {
diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 70535a7..08146fc 100644
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -66,7 +66,7 @@
const Options& opts,
SkPMColor* inputColorPtr,
int* inputColorCount) {
- if (!this->handleRewind(fInIco)) {
+ if (!this->rewindIfNeeded()) {
return kCouldNotRewind;
}
if (opts.fSubset) {
diff --git a/src/codec/SkBmpStandardCodec.h b/src/codec/SkBmpStandardCodec.h
index ff91dcf..45450e6 100644
--- a/src/codec/SkBmpStandardCodec.h
+++ b/src/codec/SkBmpStandardCodec.h
@@ -45,6 +45,9 @@
size_t dstRowBytes, const Options&, SkPMColor*,
int*) override;
+ bool onInIco() const override {
+ return fInIco;
+ }
private:
/*
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index ffe90af..3e5ebe1 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -83,16 +83,20 @@
SkCodec::~SkCodec() {}
-SkCodec::RewindState SkCodec::rewindIfNeeded() {
+bool SkCodec::rewindIfNeeded() {
// Store the value of fNeedsRewind so we can update it. Next read will
// require a rewind.
const bool needsRewind = fNeedsRewind;
fNeedsRewind = true;
if (!needsRewind) {
- return kNoRewindNecessary_RewindState;
+ return true;
}
- return fStream->rewind() ? kRewound_RewindState
- : kCouldNotRewind_RewindState;
+
+ if (!fStream->rewind()) {
+ return false;
+ }
+
+ return this->onRewind();
}
SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
diff --git a/src/codec/SkCodec_libgif.cpp b/src/codec/SkCodec_libgif.cpp
index cf93577..2e348cc 100644
--- a/src/codec/SkCodec_libgif.cpp
+++ b/src/codec/SkCodec_libgif.cpp
@@ -234,6 +234,17 @@
}
}
+bool SkGifCodec::onRewind() {
+ GifFileType* gifOut = NULL;
+ if (!ReadHeader(this->stream(), NULL, &gifOut)) {
+ return false;
+ }
+
+ SkASSERT(NULL != gifOut);
+ fGif.reset(gifOut);
+ return true;
+}
+
/*
* Initiates the gif decode
*/
@@ -243,17 +254,8 @@
SkPMColor* inputColorPtr,
int* inputColorCount) {
// Rewind if necessary
- SkCodec::RewindState rewindState = this->rewindIfNeeded();
- if (rewindState == kCouldNotRewind_RewindState) {
+ if (!this->rewindIfNeeded()) {
return kCouldNotRewind;
- } else if (rewindState == kRewound_RewindState) {
- GifFileType* gifOut = NULL;
- if (!ReadHeader(this->stream(), NULL, &gifOut)) {
- return kCouldNotRewind;
- } else {
- SkASSERT(NULL != gifOut);
- fGif.reset(gifOut);
- }
}
// Check for valid input parameters
diff --git a/src/codec/SkCodec_libgif.h b/src/codec/SkCodec_libgif.h
index d805acc..109020b 100644
--- a/src/codec/SkCodec_libgif.h
+++ b/src/codec/SkCodec_libgif.h
@@ -64,6 +64,8 @@
return kGIF_SkEncodedFormat;
}
+ bool onRewind() override;
+
private:
/*
diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index f239900..c94f371 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -471,32 +471,23 @@
}
-bool SkPngCodec::handleRewind() {
- switch (this->rewindIfNeeded()) {
- case kNoRewindNecessary_RewindState:
- return true;
- case kCouldNotRewind_RewindState:
- return false;
- case kRewound_RewindState: {
- // This sets fPng_ptr and fInfo_ptr to NULL. If read_header
- // succeeds, they will be repopulated, and if it fails, they will
- // remain NULL. Any future accesses to fPng_ptr and fInfo_ptr will
- // come through this function which will rewind and again attempt
- // to reinitialize them.
- this->destroyReadStruct();
- png_structp png_ptr;
- png_infop info_ptr;
- if (read_header(this->stream(), &png_ptr, &info_ptr, NULL, NULL)) {
- fPng_ptr = png_ptr;
- fInfo_ptr = info_ptr;
- return true;
- }
- return false;
- }
- default:
- SkASSERT(false);
- return false;
+bool SkPngCodec::onRewind() {
+ // This sets fPng_ptr and fInfo_ptr to NULL. If read_header
+ // succeeds, they will be repopulated, and if it fails, they will
+ // remain NULL. Any future accesses to fPng_ptr and fInfo_ptr will
+ // come through this function which will rewind and again attempt
+ // to reinitialize them.
+ this->destroyReadStruct();
+
+ png_structp png_ptr;
+ png_infop info_ptr;
+ if (!read_header(this->stream(), &png_ptr, &info_ptr, NULL, NULL)) {
+ return false;
}
+
+ fPng_ptr = png_ptr;
+ fInfo_ptr = info_ptr;
+ return true;
}
SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
@@ -512,7 +503,7 @@
if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
return kInvalidScale;
}
- if (!this->handleRewind()) {
+ if (!this->rewindIfNeeded()) {
return kCouldNotRewind;
}
@@ -593,7 +584,7 @@
const SkCodec::Options& options,
SkPMColor ctable[], int* ctableCount) override
{
- if (!fCodec->handleRewind()) {
+ if (!fCodec->rewindIfNeeded()) {
return SkCodec::kCouldNotRewind;
}
@@ -677,7 +668,7 @@
const SkCodec::Options& options,
SkPMColor ctable[], int* ctableCount) override
{
- if (!fCodec->handleRewind()) {
+ if (!fCodec->rewindIfNeeded()) {
return SkCodec::kCouldNotRewind;
}
@@ -714,7 +705,7 @@
// We already rewound in onStart, so there is no reason to rewind.
// Next time onGetScanlines is called, we will need to rewind.
fCanSkipRewind = false;
- } else if (!fCodec->handleRewind()) {
+ } else if (!fCodec->rewindIfNeeded()) {
return SkCodec::kCouldNotRewind;
}
diff --git a/src/codec/SkCodec_libpng.h b/src/codec/SkCodec_libpng.h
index 21bbdad..8904022 100644
--- a/src/codec/SkCodec_libpng.h
+++ b/src/codec/SkCodec_libpng.h
@@ -35,6 +35,7 @@
Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, SkPMColor*, int*)
override;
SkEncodedFormat onGetEncodedFormat() const override { return kPNG_SkEncodedFormat; }
+ bool onRewind() override;
bool onReallyHasAlpha() const override { return fReallyHasAlpha; }
private:
png_structp fPng_ptr;
@@ -56,8 +57,6 @@
Result initializeSwizzler(const SkImageInfo& requestedInfo, const Options&,
SkPMColor*, int* ctableCount);
- // Calls rewindIfNeeded and returns true if the decoder can continue.
- bool handleRewind();
bool decodePalette(bool premultiply, int* ctableCount);
void destroyReadStruct();
diff --git a/src/codec/SkCodec_wbmp.cpp b/src/codec/SkCodec_wbmp.cpp
index 86dce5c..52535e1 100644
--- a/src/codec/SkCodec_wbmp.cpp
+++ b/src/codec/SkCodec_wbmp.cpp
@@ -66,16 +66,8 @@
return true;
}
-bool SkWbmpCodec::handleRewind() {
- SkCodec::RewindState rewindState = this->rewindIfNeeded();
- if (rewindState == kCouldNotRewind_RewindState) {
- return false;
- } else if (rewindState == kRewound_RewindState) {
- if (!read_header(this->stream(), NULL)) {
- return false;
- }
- }
- return true;
+bool SkWbmpCodec::onRewind() {
+ return read_header(this->stream(), NULL);
}
SkSwizzler* SkWbmpCodec::initializeSwizzler(const SkImageInfo& info,
@@ -117,7 +109,7 @@
const Options& options,
SkPMColor ctable[],
int* ctableCount) {
- if (!this->handleRewind()) {
+ if (!this->rewindIfNeeded()) {
return kCouldNotRewind;
}
if (options.fSubset) {
@@ -197,7 +189,7 @@
SkCodec::Result onStart(const SkImageInfo& dstInfo,
const SkCodec::Options& options, SkPMColor inputColorTable[],
int* inputColorCount) {
- if (!fCodec->handleRewind()) {
+ if (!fCodec->rewindIfNeeded()) {
return SkCodec::kCouldNotRewind;
}
if (options.fSubset) {
diff --git a/src/codec/SkCodec_wbmp.h b/src/codec/SkCodec_wbmp.h
index cec398d..8c54661 100644
--- a/src/codec/SkCodec_wbmp.h
+++ b/src/codec/SkCodec_wbmp.h
@@ -33,13 +33,8 @@
SkEncodedFormat onGetEncodedFormat() const override;
Result onGetPixels(const SkImageInfo&, void*, size_t,
const Options&, SkPMColor[], int*) override;
+ bool onRewind() override;
private:
-
- /*
- * Calls rewindIfNeeded and returns true if the decoder can continue
- */
- bool handleRewind();
-
/*
* Returns a swizzler on success, NULL on failure
*/
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 0dc2795..83656da 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -189,28 +189,14 @@
return SkISize::Make(dinfo.output_width, dinfo.output_height);
}
-/*
- * Handles rewinding the input stream if it is necessary
- */
-bool SkJpegCodec::handleRewind() {
- switch(this->rewindIfNeeded()) {
- case kCouldNotRewind_RewindState:
- return fDecoderMgr->returnFalse("could not rewind");
- case kRewound_RewindState: {
- JpegDecoderMgr* decoderMgr = NULL;
- if (!ReadHeader(this->stream(), NULL, &decoderMgr)) {
- return fDecoderMgr->returnFalse("could not rewind");
- }
- SkASSERT(NULL != decoderMgr);
- fDecoderMgr.reset(decoderMgr);
- return true;
- }
- case kNoRewindNecessary_RewindState:
- return true;
- default:
- SkASSERT(false);
- return false;
+bool SkJpegCodec::onRewind() {
+ JpegDecoderMgr* decoderMgr = NULL;
+ if (!ReadHeader(this->stream(), NULL, &decoderMgr)) {
+ return fDecoderMgr->returnFalse("could not rewind");
}
+ SkASSERT(NULL != decoderMgr);
+ fDecoderMgr.reset(decoderMgr);
+ return true;
}
/*
@@ -304,7 +290,7 @@
void* dst, size_t dstRowBytes,
const Options& options, SkPMColor*, int*) {
// Rewind the stream if needed
- if (!this->handleRewind()) {
+ if (!this->rewindIfNeeded()) {
return fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind);
}
@@ -399,7 +385,7 @@
SkPMColor ctable[], int* ctableCount) override {
// Rewind the stream if needed
- if (!fCodec->handleRewind()) {
+ if (!fCodec->rewindIfNeeded()) {
return SkCodec::kCouldNotRewind;
}
diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h
index 5e2135c..62a5bbc 100644
--- a/src/codec/SkJpegCodec.h
+++ b/src/codec/SkJpegCodec.h
@@ -65,6 +65,8 @@
return kJPEG_SkEncodedFormat;
}
+ bool onRewind() override;
+
private:
/*
@@ -101,11 +103,6 @@
SkJpegCodec(const SkImageInfo& srcInfo, SkStream* stream, JpegDecoderMgr* decoderMgr);
/*
- * Handles rewinding the input stream if it is necessary
- */
- bool handleRewind();
-
- /*
* Checks if the conversion between the input image and the requested output
* image has been implemented
* Sets the output color space
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 041bfb6..624ff74 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -153,16 +153,8 @@
SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes,
const Options& options, SkPMColor*, int*) {
- switch (this->rewindIfNeeded()) {
- case kCouldNotRewind_RewindState:
- return kCouldNotRewind;
- case kRewound_RewindState:
- // Rewound to the beginning. Since creation only does a peek, the stream is at the
- // correct position.
- break;
- case kNoRewindNecessary_RewindState:
- // Already at the right spot for decoding.
- break;
+ if (!this->rewindIfNeeded()) {
+ return kCouldNotRewind;
}
if (!conversion_possible(dstInfo, this->getInfo())) {