diff options
author | 2016-06-06 11:26:17 -0700 | |
---|---|---|
committer | 2016-06-06 11:26:17 -0700 | |
commit | d8d6855345b486d2ec03047497da2036ccea3eb7 (patch) | |
tree | e5db9c1a0d63a207d9e3ae686136c51663353eed /src/codec/SkCodec.cpp | |
parent | a4fa4f6eb727e8b97f6d8a58d9fda4aea56bbc75 (diff) |
Revert of Make SkPngCodec decode progressively. (patchset #26 id:520001 of https://codereview.chromium.org/1997703003/ )
Reason for revert:
Still causing problems in Google3, e.g.
https://test.corp.google.com/ui#cl=124138817&flags=CAMQBQ==&id=OCL:124138817:BASE:124139560:1465227435491:219ffbdb&t=//third_party/skia/HEAD:dm
Original issue's description:
> Make SkPngCodec decode progressively.
>
> This is a step towards using SkCodec in Chromium, where progressive
> decoding is necessary.
>
> Switch from using png_read_row (which expects all the data to be
> available) to png_process_data, which uses callbacks when rows are
> available.
>
> Create a new API for SkCodec, which supports progressive decoding and
> scanline decoding. Future changes will switch the other clients off of
> startScanlineDecode and get/skip-Scanlines to the new API.
>
> Remove SkCodec::kNone_ScanlineOrder, which was only used for interlaced
> PNG images. In the new API, interlaced PNG fits kTopDown. Also remove
> updateCurrScanline(), which was only used by the old implementation for
> interlaced PNG.
>
> DMSrcSink:
> - In CodecSrc::kScanline_Mode, use the new method for scanline decoding
> for the supported formats (just PNG and PNG-in-ICO for now).
>
> fuzz.cpp:
> - Remove reference to kNone_ScanlineOrder
>
> SkCodec:
> - Add new APIs:
> - startIncrementalDecode
> - incrementalDecode
> - Remove kNone_SkScanlineOrder and updateCurrScanline()
>
> SkPngCodec:
> - Implement new APIs
> - Switch from sk_read_fn/png_read_row etc to png_process_data
> - Expand AutoCleanPng's role to decode the header and create the
> SkPngCodec
> - Make the interlaced PNG decoder report how many lines were
> initialized during an incomplete decode
> - Make initializeSwizzler return a bool instead of an SkCodec::Result
> (It only returned kSuccess or kInvalidInput anyway)
>
> SkIcoCodec:
> - Implement the new APIs; supported for PNG in ICO
>
> SkSampledCodec:
> - Call the new method for decoding scanlines, and fall back to the old
> method if the new version is unimplemented
> - Remove references to kNone_SkScanlineOrder
>
> tests/CodecPartial:
> - Add a test which decodes part of an image, then finishes the decode,
> and compares it to the straightforward method
>
> tests/CodecTest:
> - Add a test which decodes all scanlines using the new method
> - Repurpose the Codec_stripes test to decode using the new method in
> sections rather than all at once
> - In the method check(), add a parameter for whether the image supports
> the new method of scanline decoding, and be explicit about whether an
> image supports incomplete
> - Test incomplete PNG decodes. We should have been doing it anyway for
> non-interlaced (except for an image that is too small - one row), but
> the new method supports interlaced incomplete as well
> - Make test_invalid_parameters test the new method
> - Add a test to ensure that it's safe to fall back to scanline decoding without
> rewinding
>
> BUG=skia:4211
>
> The new version was generally faster than the old version (but not significantly so).
>
> Some raw performance differences can be found at https://docs.google.com/a/google.com/spreadsheets/d/1Gis3aRCEa72qBNDRMgGDg3jD-pMgO-FXldlNF9ejo4o/
>
> Design doc can be found at https://docs.google.com/a/google.com/document/d/11Mn8-ePDKwVEMCjs3nWwSjxcSpJ_Cu8DF57KNtUmgLM/
>
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1997703003
>
> Committed: https://skia.googlesource.com/skia/+/a4b09a117d4d1ba5dda372e6a2323e653766539e
>
> Committed: https://skia.googlesource.com/skia/+/30e78c9737ff4861dc4e3fa1e4cd010680ed6965
>
> Committed: https://skia.googlesource.com/skia/+/6fb2391b2cc83ee2160b4e994faa8128975acc1f
TBR=reed@google.com,msarett@google.com,scroggo@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=skia:4211
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2044573002
Review-Url: https://codereview.chromium.org/2044573002
Diffstat (limited to 'src/codec/SkCodec.cpp')
-rw-r--r-- | src/codec/SkCodec.cpp | 125 |
1 files changed, 33 insertions, 92 deletions
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp index 3c485c8beb..3da7f5fb07 100644 --- a/src/codec/SkCodec.cpp +++ b/src/codec/SkCodec.cpp @@ -154,20 +154,6 @@ bool SkCodec::rewindIfNeeded() { return this->onRewind(); } -#define CHECK_COLOR_TABLE \ - if (kIndex_8_SkColorType == info.colorType()) { \ - if (nullptr == ctable || nullptr == ctableCount) { \ - return SkCodec::kInvalidParameters; \ - } \ - } else { \ - if (ctableCount) { \ - *ctableCount = 0; \ - } \ - ctableCount = nullptr; \ - ctable = nullptr; \ - } - - SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, const Options* options, SkPMColor ctable[], int* ctableCount) { if (kUnknown_SkColorType == info.colorType()) { @@ -180,7 +166,17 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t return kInvalidParameters; } - CHECK_COLOR_TABLE; + if (kIndex_8_SkColorType == info.colorType()) { + if (nullptr == ctable || nullptr == ctableCount) { + return kInvalidParameters; + } + } else { + if (ctableCount) { + *ctableCount = 0; + } + ctableCount = nullptr; + ctable = nullptr; + } if (!this->rewindIfNeeded()) { return kCouldNotRewind; @@ -232,77 +228,22 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t return this->getPixels(info, pixels, rowBytes, nullptr, nullptr, nullptr); } -SkCodec::Result SkCodec::startIncrementalDecode(const SkImageInfo& info, void* pixels, - size_t rowBytes, const SkCodec::Options* options, SkPMColor* ctable, int* ctableCount) { - fStartedIncrementalDecode = false; - - if (kUnknown_SkColorType == info.colorType()) { - return kInvalidConversion; - } - if (nullptr == pixels) { - return kInvalidParameters; - } - - // Ensure that valid color ptrs are passed in for kIndex8 color type - CHECK_COLOR_TABLE; - - // FIXME: If the rows come after the rows of a previous incremental decode, - // we might be able to skip the rewind, but only the implementation knows - // that. (e.g. PNG will always need to rewind, since we called longjmp, but - // a bottom-up BMP could skip rewinding if the new rows are above the old - // rows.) - if (!this->rewindIfNeeded()) { - return kCouldNotRewind; - } - - // Set options. - Options optsStorage; - if (nullptr == options) { - options = &optsStorage; - } else if (options->fSubset) { - SkIRect size = SkIRect::MakeSize(info.dimensions()); - if (!size.contains(*options->fSubset)) { - return kInvalidParameters; - } - - const int top = options->fSubset->top(); - const int bottom = options->fSubset->bottom(); - if (top < 0 || top >= info.height() || top >= bottom || bottom > info.height()) { - return kInvalidParameters; - } - } - - if (!this->dimensionsSupported(info.dimensions())) { - return kInvalidScale; - } - - fDstInfo = info; - fOptions = *options; - - const Result result = this->onStartIncrementalDecode(info, pixels, rowBytes, - fOptions, ctable, ctableCount); - if (kSuccess == result) { - fStartedIncrementalDecode = true; - } else if (kUnimplemented == result) { - // FIXME: This is temporarily necessary, until we transition SkCodec - // implementations from scanline decoding to incremental decoding. - // SkAndroidCodec will first attempt to use incremental decoding, but - // will fall back to scanline decoding if incremental returns - // kUnimplemented. rewindIfNeeded(), above, set fNeedsRewind to true - // (after potentially rewinding), but we do not want the next call to - // startScanlineDecode() to do a rewind. - fNeedsRewind = false; - } - return result; -} - - -SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& info, +SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& dstInfo, const SkCodec::Options* options, SkPMColor ctable[], int* ctableCount) { // Reset fCurrScanline in case of failure. fCurrScanline = -1; // Ensure that valid color ptrs are passed in for kIndex8 color type - CHECK_COLOR_TABLE; + if (kIndex_8_SkColorType == dstInfo.colorType()) { + if (nullptr == ctable || nullptr == ctableCount) { + return SkCodec::kInvalidParameters; + } + } else { + if (ctableCount) { + *ctableCount = 0; + } + ctableCount = nullptr; + ctable = nullptr; + } if (!this->rewindIfNeeded()) { return kCouldNotRewind; @@ -313,38 +254,36 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& info, if (nullptr == options) { options = &optsStorage; } else if (options->fSubset) { - SkIRect size = SkIRect::MakeSize(info.dimensions()); + SkIRect size = SkIRect::MakeSize(dstInfo.dimensions()); if (!size.contains(*options->fSubset)) { return kInvalidInput; } // We only support subsetting in the x-dimension for scanline decoder. // Subsetting in the y-dimension can be accomplished using skipScanlines(). - if (options->fSubset->top() != 0 || options->fSubset->height() != info.height()) { + if (options->fSubset->top() != 0 || options->fSubset->height() != dstInfo.height()) { return kInvalidInput; } } // FIXME: Support subsets somehow? - if (!this->dimensionsSupported(info.dimensions())) { + if (!this->dimensionsSupported(dstInfo.dimensions())) { return kInvalidScale; } - const Result result = this->onStartScanlineDecode(info, *options, ctable, ctableCount); + const Result result = this->onStartScanlineDecode(dstInfo, *options, ctable, ctableCount); if (result != SkCodec::kSuccess) { return result; } fCurrScanline = 0; - fDstInfo = info; + fDstInfo = dstInfo; fOptions = *options; return kSuccess; } -#undef CHECK_COLOR_TABLE - -SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& info) { - return this->startScanlineDecode(info, nullptr, nullptr, nullptr); +SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& dstInfo) { + return this->startScanlineDecode(dstInfo, nullptr, nullptr, nullptr); } int SkCodec::getScanlines(void* dst, int countLines, size_t rowBytes) { @@ -392,6 +331,7 @@ int SkCodec::outputScanline(int inputScanline) const { int SkCodec::onOutputScanline(int inputScanline) const { switch (this->getScanlineOrder()) { case kTopDown_SkScanlineOrder: + case kNone_SkScanlineOrder: return inputScanline; case kBottomUp_SkScanlineOrder: return this->getInfo().height() - inputScanline - 1; @@ -425,7 +365,8 @@ void SkCodec::fillIncompleteImage(const SkImageInfo& info, void* dst, size_t row } switch (this->getScanlineOrder()) { - case kTopDown_SkScanlineOrder: { + case kTopDown_SkScanlineOrder: + case kNone_SkScanlineOrder: { const SkImageInfo fillInfo = info.makeWH(fillWidth, linesRemaining); fillDst = SkTAddOffset<void>(dst, linesDecoded * rowBytes); fill_proc(fillInfo, fillDst, rowBytes, fillValue, zeroInit, sampler); |