diff options
| -rw-r--r-- | include/codec/SkCodec.h | 14 | ||||
| -rw-r--r-- | src/codec/SkCodec.cpp | 125 | ||||
| -rw-r--r-- | src/codec/SkGifCodec.cpp | 144 | ||||
| -rw-r--r-- | src/codec/SkGifCodec.h | 4 | ||||
| -rw-r--r-- | src/codec/SkWebpCodec.cpp | 61 | ||||
| -rw-r--r-- | src/codec/SkWebpCodec.h | 4 | ||||
| -rw-r--r-- | tests/CodecPartialTest.cpp | 8 | ||||
| -rw-r--r-- | tests/CodecTest.cpp | 24 |
8 files changed, 177 insertions, 207 deletions
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index 4773d20a0c..6b3aa5e020 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -23,8 +23,8 @@ #include <vector> class SkColorSpace; -class SkColorSpaceXform; class SkData; +class SkFrameHolder; class SkPngChunkReader; class SkSampler; @@ -880,6 +880,18 @@ private: return dim == fSrcInfo.dimensions() || this->onDimensionsSupported(dim); } + /** + * For multi-framed images, return the object with information about the frames. + */ + virtual const SkFrameHolder* getFrameHolder() const { + return nullptr; + } + + /** + * Check for a valid Options.fFrameIndex, and decode prior frames if necessary. + */ + Result handleFrameIndex(const SkImageInfo&, void* pixels, size_t rowBytes, const Options&); + // Methods for scanline decoding. virtual Result onStartScanlineDecode(const SkImageInfo& /*dstInfo*/, const Options& /*options*/, SkPMColor* /*ctable*/, int* /*ctableCount*/) { diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp index 2cae6db1df..60d25210c8 100644 --- a/src/codec/SkCodec.cpp +++ b/src/codec/SkCodec.cpp @@ -11,6 +11,7 @@ #include "SkColorSpace.h" #include "SkColorSpaceXform_Base.h" #include "SkData.h" +#include "SkFrameHolder.h" #include "SkGifCodec.h" #include "SkHalf.h" #include "SkIcoCodec.h" @@ -179,6 +180,90 @@ bool SkCodec::rewindIfNeeded() { ctable = nullptr; \ } +static void zero_rect(const SkImageInfo& dstInfo, void* pixels, size_t rowBytes, + SkIRect frameRect) { + if (!frameRect.intersect(dstInfo.bounds())) { + return; + } + const auto info = dstInfo.makeWH(frameRect.width(), frameRect.height()); + const size_t bpp = SkColorTypeBytesPerPixel(dstInfo.colorType()); + const size_t offset = frameRect.x() * bpp + frameRect.y() * rowBytes; + auto* eraseDst = SkTAddOffset<void>(pixels, offset); + SkSampler::Fill(info, eraseDst, rowBytes, 0, SkCodec::kNo_ZeroInitialized); +} + +SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, size_t rowBytes, + const Options& options) { + const int index = options.fFrameIndex; + if (0 == index) { + return kSuccess; + } + + if (options.fSubset || info.dimensions() != fSrcInfo.dimensions()) { + // If we add support for these, we need to update the code that zeroes + // a kRestoreBGColor frame. + return kInvalidParameters; + } + + // index 8 is not supported beyond the first frame. + if (index < 0 || info.colorType() == kIndex_8_SkColorType) { + return kInvalidParameters; + } + + if (index >= this->onGetFrameCount()) { + return kIncompleteInput; + } + + const auto* frameHolder = this->getFrameHolder(); + SkASSERT(frameHolder); + + const auto* frame = frameHolder->getFrame(index); + SkASSERT(frame); + + const int requiredFrame = frame->getRequiredFrame(); + if (requiredFrame == kNone) { + return kSuccess; + } + + if (options.fPriorFrame != kNone) { + // Check for a valid frame as a starting point. Alternatively, we could + // treat an invalid frame as not providing one, but rejecting it will + // make it easier to catch the mistake. + if (options.fPriorFrame < requiredFrame || options.fPriorFrame >= index) { + return kInvalidParameters; + } + const auto* prevFrame = frameHolder->getFrame(options.fPriorFrame); + switch (prevFrame->getDisposalMethod()) { + case SkCodecAnimation::DisposalMethod::kRestorePrevious: + return kInvalidParameters; + case SkCodecAnimation::DisposalMethod::kRestoreBGColor: + // If a frame after the required frame is provided, there is no + // need to clear, since it must be covered by the desired frame. + if (options.fPriorFrame == requiredFrame) { + zero_rect(info, pixels, rowBytes, prevFrame->frameRect()); + } + break; + default: + break; + } + return kSuccess; + } + + Options prevFrameOptions(options); + prevFrameOptions.fFrameIndex = requiredFrame; + prevFrameOptions.fZeroInitialized = kNo_ZeroInitialized; + const Result result = this->getPixels(info, pixels, rowBytes, &prevFrameOptions, + nullptr, nullptr); + if (result == kSuccess) { + const auto* prevFrame = frameHolder->getFrame(requiredFrame); + const auto disposalMethod = prevFrame->getDisposalMethod(); + if (disposalMethod == SkCodecAnimation::DisposalMethod::kRestoreBGColor) { + zero_rect(info, pixels, rowBytes, prevFrame->frameRect()); + } + } + + return result; +} SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, const Options* options, SkPMColor ctable[], int* ctableCount) { @@ -202,12 +287,18 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t Options optsStorage; if (nullptr == options) { options = &optsStorage; - } else if (options->fSubset) { - SkIRect subset(*options->fSubset); - if (!this->onGetValidSubset(&subset) || subset != *options->fSubset) { - // FIXME: How to differentiate between not supporting subset at all - // and not supporting this particular subset? - return kUnimplemented; + } else { + const Result frameIndexResult = this->handleFrameIndex(info, pixels, rowBytes, *options); + if (frameIndexResult != kSuccess) { + return frameIndexResult; + } + if (options->fSubset) { + SkIRect subset(*options->fSubset); + if (!this->onGetValidSubset(&subset) || subset != *options->fSubset) { + // FIXME: How to differentiate between not supporting subset at all + // and not supporting this particular subset? + return kUnimplemented; + } } } @@ -280,16 +371,22 @@ SkCodec::Result SkCodec::startIncrementalDecode(const SkImageInfo& info, void* p Options optsStorage; if (nullptr == options) { options = &optsStorage; - } else if (options->fSubset) { - SkIRect size = SkIRect::MakeSize(info.dimensions()); - if (!size.contains(*options->fSubset)) { - return kInvalidParameters; + } else { + const Result frameIndexResult = this->handleFrameIndex(info, pixels, rowBytes, *options); + if (frameIndexResult != kSuccess) { + return frameIndexResult; } + 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; + 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; + } } } diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp index 6c4273444c..eefe8986b7 100644 --- a/src/codec/SkGifCodec.cpp +++ b/src/codec/SkGifCodec.cpp @@ -196,51 +196,40 @@ SkCodec::Result SkGifCodec::prepareToDecode(const SkImageInfo& dstInfo, SkPMColo } const int frameIndex = opts.fFrameIndex; - if (frameIndex > 0) { - switch (dstInfo.colorType()) { - case kIndex_8_SkColorType: - // FIXME: It is possible that a later frame can be decoded to index8, if it does one - // of the following: - // - Covers the entire previous frame - // - Shares a color table (and transparent index) with any prior frames that are - // showing. - // We must support index8 for the first frame to be backwards compatible on Android, - // but we do not (currently) need to support later frames as index8. - return gif_error("Cannot decode multiframe gif (except frame 0) as index 8.\n", - kInvalidConversion); - case kRGB_565_SkColorType: - // FIXME: In theory, we might be able to support this, but it's not clear that it - // is necessary (Chromium does not decode to 565, and Android does not decode - // frames beyond the first). Disabling it because it is somewhat difficult: - // - If there is a transparent pixel, and this frame draws on top of another frame - // (if the frame is independent with a transparent pixel, we should not decode to - // 565 anyway, since it is not opaque), we need to skip drawing the transparent - // pixels (see writeTransparentPixels in haveDecodedRow). We currently do this by - // first swizzling into temporary memory, then copying into the destination. (We - // let the swizzler handle it first because it may need to sample.) After - // swizzling to 565, we do not know which pixels in our temporary memory - // correspond to the transparent pixel, so we do not know what to skip. We could - // special case the non-sampled case (no need to swizzle), but as this is - // currently unused we can just not support it. - return gif_error("Cannot decode multiframe gif (except frame 0) as 565.\n", - kInvalidConversion); - default: - break; - } - } - - fReader->parse((SkGifImageReader::SkGIFParseQuery) frameIndex); - - if (frameIndex >= fReader->imagesCount()) { - return gif_error("frame index out of range!\n", kIncompleteInput); + if (frameIndex > 0 && kRGB_565_SkColorType == dstInfo.colorType()) { + // FIXME: In theory, we might be able to support this, but it's not clear that it + // is necessary (Chromium does not decode to 565, and Android does not decode + // frames beyond the first). Disabling it because it is somewhat difficult: + // - If there is a transparent pixel, and this frame draws on top of another frame + // (if the frame is independent with a transparent pixel, we should not decode to + // 565 anyway, since it is not opaque), we need to skip drawing the transparent + // pixels (see writeTransparentPixels in haveDecodedRow). We currently do this by + // first swizzling into temporary memory, then copying into the destination. (We + // let the swizzler handle it first because it may need to sample.) After + // swizzling to 565, we do not know which pixels in our temporary memory + // correspond to the transparent pixel, so we do not know what to skip. We could + // special case the non-sampled case (no need to swizzle), but as this is + // currently unused we can just not support it. + return gif_error("Cannot decode multiframe gif (except frame 0) as 565.\n", + kInvalidConversion); } const auto* frame = fReader->frameContext(frameIndex); - if (!frame->reachedStartOfData()) { - // We have parsed enough to know that there is a color map, but cannot - // parse the map itself yet. Exit now, so we do not build an incorrect - // table. - return gif_error("color map not available yet\n", kIncompleteInput); + SkASSERT(frame); + if (0 == frameIndex) { + // SkCodec does not have a way to just parse through frame 0, so we + // have to do so manually, here. + fReader->parse((SkGifImageReader::SkGIFParseQuery) 0); + if (!frame->reachedStartOfData()) { + // We have parsed enough to know that there is a color map, but cannot + // parse the map itself yet. Exit now, so we do not build an incorrect + // table. + return gif_error("color map not available yet\n", kIncompleteInput); + } + } else { + // Parsing happened in SkCodec::getPixels. + SkASSERT(frameIndex < fReader->imagesCount()); + SkASSERT(frame->reachedStartOfData()); } const auto at = frame->hasAlpha() ? kUnpremul_SkAlphaType : kOpaque_SkAlphaType; @@ -398,75 +387,8 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts, filledBackground = true; } } else { - // Not independent - // FIXME: Share this code with WEBP - const int reqFrame = frameContext->getRequiredFrame(); - if (opts.fPriorFrame != kNone) { - if (opts.fPriorFrame < reqFrame || opts.fPriorFrame >= frameIndex) { - // Alternatively, we could correct this to kNone. - return kInvalidParameters; - } - const auto* prevFrame = fReader->frameContext(opts.fPriorFrame); - if (prevFrame->getDisposalMethod() - == SkCodecAnimation::DisposalMethod::kRestorePrevious) { - // Similarly, this could be corrected to kNone. - return kInvalidParameters; - } - } else { - // Decode that frame into pixels. - Options prevFrameOpts(opts); - prevFrameOpts.fFrameIndex = frameContext->getRequiredFrame(); - prevFrameOpts.fPriorFrame = kNone; - // The prior frame may have a different color table, so update it and the - // swizzler. - this->initializeColorTable(dstInfo, prevFrameOpts.fFrameIndex); - this->initializeSwizzler(dstInfo, prevFrameOpts.fFrameIndex); - - const Result prevResult = this->decodeFrame(true, prevFrameOpts, nullptr); - switch (prevResult) { - case kSuccess: - // Prior frame succeeded. Carry on. - break; - case kIncompleteInput: - // Prior frame was incomplete. So this frame cannot be decoded. - return kInvalidInput; - default: - return prevResult; - } - - // Go back to using the correct color table for this frame. - this->initializeColorTable(dstInfo, frameIndex); - this->initializeSwizzler(dstInfo, frameIndex); - } - - // If the required frame is RestoreBG, we need to erase it. If a frame after the - // required frame is provided, there is no need to erase, since it will be covered - // anyway. - if (opts.fPriorFrame == reqFrame || opts.fPriorFrame == kNone) { - const auto* prevFrame = fReader->frameContext(reqFrame); - if (prevFrame->getDisposalMethod() - == SkCodecAnimation::DisposalMethod::kRestoreBGColor) { - SkIRect prevRect = prevFrame->frameRect(); - if (prevRect.intersect(this->getInfo().bounds())) { - // Do the divide ourselves for left and top, since we do not want - // get_scaled_dimension to upgrade 0 to 1. (This is similar to - // SkSampledCodec's sampling of the subset.) - const auto sampleX = fSwizzler->sampleX(); - const auto sampleY = fSwizzler->sampleY(); - auto left = prevRect.fLeft / sampleX; - auto top = prevRect.fTop / sampleY; - void* const eraseDst = SkTAddOffset<void>(fDst, top * fDstRowBytes - + left * SkColorTypeBytesPerPixel(dstInfo.colorType())); - auto width = get_scaled_dimension(prevRect.width(), sampleX); - auto height = get_scaled_dimension(prevRect.height(), sampleY); - // fSwizzler->fill() would fill to the scaled width of the frame, but we - // want to fill to the scaled with of the width of the PRIOR frame, so we - // do all the scaling ourselves and call the static version. - SkSampler::Fill(dstInfo.makeWH(width, height), eraseDst, fDstRowBytes, - this->getFillValue(dstInfo), kNo_ZeroInitialized); - } - } - } + // Not independent. + // SkCodec ensured that the prior frame has been decoded. filledBackground = true; } diff --git a/src/codec/SkGifCodec.h b/src/codec/SkGifCodec.h index 314b50b798..33472d8ac1 100644 --- a/src/codec/SkGifCodec.h +++ b/src/codec/SkGifCodec.h @@ -59,6 +59,10 @@ protected: Result onIncrementalDecode(int*) override; + const SkFrameHolder* getFrameHolder() const override { + return fReader.get(); + } + private: /* diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp index 65250a41c1..68301b93f8 100644 --- a/src/codec/SkWebpCodec.cpp +++ b/src/codec/SkWebpCodec.cpp @@ -381,11 +381,8 @@ static void blend_line(SkColorType dstCT, void* dst, SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, const Options& options, SkPMColor*, int*, int* rowsDecodedPtr) { - // Ensure that we have parsed this far. const int index = options.fFrameIndex; - if (index >= this->onGetFrameCount()) { - return kIncompleteInput; - } + SkASSERT(0 == index || index < fFrameHolder.size()); const auto& srcInfo = this->getInfo(); { @@ -401,13 +398,7 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, } } - if (index > 0 && (options.fSubset || dstInfo.dimensions() != srcInfo.dimensions())) { - // Subsetting and scaling are tricky when asking for frames beyond frame 0. In order to - // support it, we'll need to determine the proper rectangle for a - // WEBP_MUX_DISPOSE_BACKGROUND required frame before erasing it. (Currently the order - // is backwards.) Disable until it becomes clear that supporting it is important. - return kUnimplemented; - } + SkASSERT(0 == index || (!options.fSubset && dstInfo.dimensions() == srcInfo.dimensions())); WebPDecoderConfig config; if (0 == WebPInitDecoderConfig(&config)) { @@ -424,53 +415,15 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, // If this succeeded in onGetFrameCount(), it should succeed again here. SkAssertResult(WebPDemuxGetFrame(fDemux, index + 1, &frame)); - const int requiredFrame = index == 0 ? kNone : fFrameHolder.frame(index)->getRequiredFrame(); + const bool independent = index == 0 ? true : + (fFrameHolder.frame(index)->getRequiredFrame() == kNone); // Get the frameRect. libwebp will have already signaled an error if this is not fully // contained by the canvas. auto frameRect = SkIRect::MakeXYWH(frame.x_offset, frame.y_offset, frame.width, frame.height); SkASSERT(srcInfo.bounds().contains(frameRect)); const bool frameIsSubset = frameRect != srcInfo.bounds(); - if (kNone == requiredFrame) { - if (frameIsSubset) { - SkSampler::Fill(dstInfo, dst, rowBytes, 0, options.fZeroInitialized); - } - } else { - // FIXME: Share with GIF - if (options.fPriorFrame != kNone) { - if (options.fPriorFrame < requiredFrame || options.fPriorFrame >= index) { - return kInvalidParameters; - } - } else { - Options prevFrameOpts(options); - prevFrameOpts.fFrameIndex = requiredFrame; - prevFrameOpts.fPriorFrame = kNone; - const auto result = this->getPixels(dstInfo, dst, rowBytes, &prevFrameOpts, - nullptr, nullptr); - switch (result) { - case kSuccess: - break; - case kIncompleteInput: - return kInvalidInput; - default: - return result; - } - } - - if (options.fPriorFrame == requiredFrame || options.fPriorFrame == kNone) { - // Dispose bg color - const Frame* priorFrame = fFrameHolder.frame(requiredFrame); - if (priorFrame->getDisposalMethod() - == SkCodecAnimation::DisposalMethod::kRestoreBGColor) { - // FIXME: If we add support for scaling/subsets, this rectangle needs to be - // adjusted. - const auto priorRect = priorFrame->frameRect(); - const auto info = dstInfo.makeWH(priorRect.width(), priorRect.height()); - const size_t bpp = SkColorTypeBytesPerPixel(dstInfo.colorType()); - const size_t offset = priorRect.x() * bpp + priorRect.y() * rowBytes; - auto* eraseDst = SkTAddOffset<void>(dst, offset); - SkSampler::Fill(info, eraseDst, rowBytes, 0, kNo_ZeroInitialized); - } - } + if (independent && frameIsSubset) { + SkSampler::Fill(dstInfo, dst, rowBytes, 0, options.fZeroInitialized); } int dstX = frameRect.x(); @@ -540,7 +493,7 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst, config.options.scaled_height = scaledHeight; } - const bool blendWithPrevFrame = requiredFrame != kNone && frame.blend_method == WEBP_MUX_BLEND + const bool blendWithPrevFrame = !independent && frame.blend_method == WEBP_MUX_BLEND && frame.has_alpha; if (blendWithPrevFrame && options.fPremulBehavior == SkTransferFunctionBehavior::kRespect) { // Blending is done with SkRasterPipeline, which requires a color space that is valid for diff --git a/src/codec/SkWebpCodec.h b/src/codec/SkWebpCodec.h index f4c7910ee6..fc9e6838a6 100644 --- a/src/codec/SkWebpCodec.h +++ b/src/codec/SkWebpCodec.h @@ -45,6 +45,10 @@ protected: bool onGetFrameInfo(int, FrameInfo*) const override; int onGetRepetitionCount() override; + const SkFrameHolder* getFrameHolder() const override { + return &fFrameHolder; + } + private: SkWebpCodec(int width, int height, const SkEncodedInfo&, sk_sp<SkColorSpace>, SkStream*, WebPDemuxer*, sk_sp<SkData>); diff --git a/tests/CodecPartialTest.cpp b/tests/CodecPartialTest.cpp index a232290d0d..975ca63ee9 100644 --- a/tests/CodecPartialTest.cpp +++ b/tests/CodecPartialTest.cpp @@ -200,13 +200,9 @@ DEF_TEST(Codec_partialAnim, r) { // frameByteCounts stores the number of bytes to decode a particular frame. // - [0] is the number of bytes for the header // - frames[i] requires frameByteCounts[i+1] bytes to decode - std::vector<size_t> frameByteCounts; + const std::vector<size_t> frameByteCounts = { 455, 69350, 1344, 1346, 1327 }; std::vector<SkBitmap> frames; - size_t lastOffset = 0; for (size_t i = 0; true; i++) { - frameByteCounts.push_back(stream->getPosition() - lastOffset); - lastOffset = stream->getPosition(); - SkBitmap frame; frame.allocPixels(info); @@ -216,8 +212,6 @@ DEF_TEST(Codec_partialAnim, r) { frame.rowBytes(), &opts, nullptr, nullptr); if (result == SkCodec::kIncompleteInput || result == SkCodec::kInvalidInput) { - frameByteCounts.push_back(stream->getPosition() - lastOffset); - // We need to distinguish between a partial frame and no more frames. // getFrameInfo lets us do this, since it tells the number of frames // not considering whether they are complete. diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp index f839aaad80..04c5a088bd 100644 --- a/tests/CodecTest.cpp +++ b/tests/CodecTest.cpp @@ -1254,8 +1254,8 @@ static void decode_frame(skiatest::Reporter* r, SkCodec* codec, size_t frame) { bm.getPixels(), bm.rowBytes(), &opts, nullptr, nullptr)); } -// For an animated image, we should only read enough to decode the requested -// frame if the client never calls getFrameInfo. +// For an animated GIF, we should only read enough to decode frame 0 if the +// client never calls getFrameInfo and only decodes frame 0. DEF_TEST(Codec_skipFullParse, r) { auto path = "test640x479.gif"; SkStream* stream(GetResourceAsStream(path)); @@ -1282,26 +1282,10 @@ DEF_TEST(Codec_skipFullParse, r) { REPORTER_ASSERT(r, positionAfterFirstFrame > sizePosition && positionAfterFirstFrame < stream->getLength()); - // Again, this should read more of the stream. - decode_frame(r, codec.get(), 2); - const size_t positionAfterThirdFrame = stream->getPosition(); - REPORTER_ASSERT(r, positionAfterThirdFrame > positionAfterFirstFrame - && positionAfterThirdFrame < stream->getLength()); - - // This does not need to read any more of the stream, since it has already - // parsed the second frame. - decode_frame(r, codec.get(), 1); - REPORTER_ASSERT(r, stream->getPosition() == positionAfterThirdFrame); - - // This should read the rest of the frames. - decode_frame(r, codec.get(), 3); - const size_t finalPosition = stream->getPosition(); - REPORTER_ASSERT(r, finalPosition > positionAfterThirdFrame); - - // There may be more data in the stream. + // There is more data in the stream. auto frameInfo = codec->getFrameInfo(); REPORTER_ASSERT(r, frameInfo.size() == 4); - REPORTER_ASSERT(r, stream->getPosition() >= finalPosition); + REPORTER_ASSERT(r, stream->getPosition() > positionAfterFirstFrame); } // Only rewinds up to a limit. |