From 267a8589d32dcf25afd23219422e90af4f4b9f9e Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Fri, 13 Nov 2020 10:24:40 -0500 Subject: [PATCH] Revert "SkAndroidCodec: Support decoding all frames" This reverts commit fc4fdc5b25f448dd9c2cd4e445561a840ce8514b. Reason for revert: Google3 and ASAN failures Change-Id: I890cd76109c0375391637f879550837d01e650f9 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334840 Reviewed-by: Leon Scroggins Commit-Queue: Leon Scroggins --- BUILD.gn | 2 +- dm/DM.cpp | 4 - dm/DMSrcSink.cpp | 65 +++++--------- include/codec/SkAndroidCodec.h | 24 ++++- include/codec/SkCodec.h | 14 +-- src/codec/SkAndroidCodec.cpp | 34 +++---- src/codec/SkAndroidCodecAdapter.cpp | 5 +- src/codec/SkCodec.cpp | 133 +++++++++++++--------------- src/codec/SkSampledCodec.cpp | 41 +++++---- tests/AndroidCodecTest.cpp | 58 ------------ tests/CodecAnimTest.cpp | 15 +--- tests/CodecTest.cpp | 5 +- 12 files changed, 148 insertions(+), 252 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 6377dfb8c2..3985f614c6 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1236,7 +1236,7 @@ component("skia") { "src/sfnt/SkOTUtils.cpp", ] - defines = [ "SK_HAS_ANDROID_CODEC" ] + defines = [] libs = [] if (is_win) { diff --git a/dm/DM.cpp b/dm/DM.cpp index 2187ee1dd8..3de6d5fe94 100644 --- a/dm/DM.cpp +++ b/dm/DM.cpp @@ -764,10 +764,6 @@ static void push_codec_srcs(Path path) { push_codec_src(path, CodecSrc::kAnimated_Mode, dstCT, at, 1.0f); } } - for (float scale : { .5f, .33f }) { - push_codec_src(path, CodecSrc::kAnimated_Mode, CodecSrc::kGetFromCanvas_DstColorType, - kPremul_SkAlphaType, scale); - } } } diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index c609d687c3..3911bf4e5c 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -436,13 +436,6 @@ Result CodecSrc::draw(GrDirectContext*, SkCanvas* canvas) const { // Try to scale the image if it is desired SkISize size = codec->getScaledDimensions(fScale); - - std::unique_ptr androidCodec; - if (1.0f != fScale && fMode == kAnimated_Mode) { - androidCodec = SkAndroidCodec::MakeFromData(encoded); - size = androidCodec->getSampledDimensions(1 / fScale); - } - if (size == decodeInfo.dimensions() && 1.0f != fScale) { return Result::Skip("Test without scaling is uninteresting."); } @@ -474,16 +467,7 @@ Result CodecSrc::draw(GrDirectContext*, SkCanvas* canvas) const { switch (fMode) { case kAnimated_Mode: { - SkAndroidCodec::AndroidOptions androidOptions; - if (fScale != 1.0f) { - SkASSERT(androidCodec); - androidOptions.fSampleSize = 1 / fScale; - auto dims = androidCodec->getSampledDimensions(androidOptions.fSampleSize); - decodeInfo = decodeInfo.makeDimensions(dims); - } - - std::vector frameInfos = androidCodec - ? androidCodec->codec()->getFrameInfo() : codec->getFrameInfo(); + std::vector frameInfos = codec->getFrameInfo(); if (frameInfos.size() <= 1) { return Result::Fatal("%s is not an animated image.", fPath.c_str()); } @@ -498,21 +482,19 @@ Result CodecSrc::draw(GrDirectContext*, SkCanvas* canvas) const { SkAutoMalloc priorFramePixels; int cachedFrame = SkCodec::kNoFrame; for (int i = 0; static_cast(i) < frameInfos.size(); i++) { - androidOptions.fFrameIndex = i; + options.fFrameIndex = i; // Check for a prior frame const int reqFrame = frameInfos[i].fRequiredFrame; if (reqFrame != SkCodec::kNoFrame && reqFrame == cachedFrame && priorFramePixels.get()) { // Copy into pixels memcpy(pixels.get(), priorFramePixels.get(), safeSize); - androidOptions.fPriorFrame = reqFrame; + options.fPriorFrame = reqFrame; } else { - androidOptions.fPriorFrame = SkCodec::kNoFrame; + options.fPriorFrame = SkCodec::kNoFrame; } - SkCodec::Result result = androidCodec - ? androidCodec->getAndroidPixels(decodeInfo, pixels.get(), rowBytes, - &androidOptions) - : codec->getPixels(decodeInfo, pixels.get(), rowBytes, &androidOptions); + SkCodec::Result result = codec->getPixels(decodeInfo, pixels.get(), + rowBytes, &options); if (SkCodec::kInvalidInput == result && i > 0) { // Some of our test images have truncated later frames. Treat that // the same as incomplete. @@ -777,33 +759,30 @@ SkISize CodecSrc::size() const { return {0, 0}; } - if (fMode != kAnimated_Mode) { - return codec->getScaledDimensions(fScale); + auto imageSize = codec->getScaledDimensions(fScale); + if (fMode == kAnimated_Mode) { + // We'll draw one of each frame, so make it big enough to hold them all + // in a grid. The grid will be roughly square, with "factor" frames per + // row and up to "factor" rows. + const size_t count = codec->getFrameInfo().size(); + const float root = sqrt((float) count); + const int factor = sk_float_ceil2int(root); + imageSize.fWidth = imageSize.fWidth * factor; + imageSize.fHeight = imageSize.fHeight * sk_float_ceil2int((float) count / (float) factor); } - - // We'll draw one of each frame, so make it big enough to hold them all - // in a grid. The grid will be roughly square, with "factor" frames per - // row and up to "factor" rows. - const size_t count = codec->getFrameInfo().size(); - const float root = sqrt((float) count); - const int factor = sk_float_ceil2int(root); - - auto androidCodec = SkAndroidCodec::MakeFromCodec(std::move(codec)); - auto imageSize = androidCodec->getSampledDimensions(1 / fScale); - imageSize.fWidth = imageSize.fWidth * factor; - imageSize.fHeight = imageSize.fHeight * sk_float_ceil2int((float) count / (float) factor); return imageSize; } Name CodecSrc::name() const { - Name name = SkOSPath::Basename(fPath.c_str()); - if (fMode == kAnimated_Mode) { - name.append("_animated"); - } if (1.0f == fScale) { + Name name = SkOSPath::Basename(fPath.c_str()); + if (fMode == kAnimated_Mode) { + name.append("_animated"); + } return name; } - return get_scaled_name(name.c_str(), fScale); + SkASSERT(fMode != kAnimated_Mode); + return get_scaled_name(fPath, fScale); } /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/ diff --git a/include/codec/SkAndroidCodec.h b/include/codec/SkAndroidCodec.h index c361f8ac50..88eb370f37 100644 --- a/include/codec/SkAndroidCodec.h +++ b/include/codec/SkAndroidCodec.h @@ -195,12 +195,32 @@ public: // called SkAndroidCodec. On the other hand, it's may be a bit confusing to call // these Options when SkCodec has a slightly different set of Options. Maybe these // should be DecodeOptions or SamplingOptions? - struct AndroidOptions : public SkCodec::Options { + struct AndroidOptions { AndroidOptions() - : SkCodec::Options() + : fZeroInitialized(SkCodec::kNo_ZeroInitialized) + , fSubset(nullptr) , fSampleSize(1) {} + /** + * Indicates is destination pixel memory is zero initialized. + * + * The default is SkCodec::kNo_ZeroInitialized. + */ + SkCodec::ZeroInitialized fZeroInitialized; + + /** + * If not NULL, represents a subset of the original image to decode. + * + * Must be within the bounds returned by getInfo(). + * + * If the EncodedFormat is SkEncodedImageFormat::kWEBP, the top and left + * values must be even. + * + * The default is NULL, meaning a decode of the entire image. + */ + SkIRect* fSubset; + /** * The client may provide an integer downscale factor for the decode. * The codec may implement this downscaling by sampling or another diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index 88fd07b5ce..8b1fc3bdcf 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -24,7 +24,6 @@ #include -class SkAndroidCodec; class SkColorSpace; class SkData; class SkFrameHolder; @@ -855,10 +854,6 @@ private: bool fStartedIncrementalDecode; - // Allows SkAndroidCodec to call handleFrameIndex (potentially decoding a prior frame and - // clearing to transparent) without SkCodec calling it, too. - bool fAndroidCodecHandlesFrameIndex; - bool initializeColorXform(const SkImageInfo& dstInfo, SkEncodedInfo::Alpha, bool srcIsOpaque); /** @@ -883,15 +878,8 @@ private: /** * Check for a valid Options.fFrameIndex, and decode prior frames if necessary. - * - * If androidCodec is not null, that means this SkCodec is owned by an SkAndroidCodec. In that - * case, the Options will be treated as an AndroidOptions, and SkAndroidCodec will be used to - * decode a prior frame, if a prior frame is needed. When such an owned SkCodec calls - * handleFrameIndex, it will immediately return kSuccess, since SkAndroidCodec already handled - * it. */ - Result handleFrameIndex(const SkImageInfo&, void* pixels, size_t rowBytes, const Options&, - SkAndroidCodec* androidCodec = nullptr); + Result handleFrameIndex(const SkImageInfo&, void* pixels, size_t rowBytes, const Options&); // Methods for scanline decoding. virtual Result onStartScanlineDecode(const SkImageInfo& /*dstInfo*/, diff --git a/src/codec/SkAndroidCodec.cpp b/src/codec/SkAndroidCodec.cpp index f995c2e8ab..b7878df6d6 100644 --- a/src/codec/SkAndroidCodec.cpp +++ b/src/codec/SkAndroidCodec.cpp @@ -372,33 +372,19 @@ SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo, AndroidOptions defaultOptions; if (!options) { options = &defaultOptions; - } else { - if (options->fSubset) { - if (!is_valid_subset(*options->fSubset, adjustedInfo.dimensions())) { - return SkCodec::kInvalidParameters; - } - - if (SkIRect::MakeSize(adjustedInfo.dimensions()) == *options->fSubset) { - // The caller wants the whole thing, rather than a subset. Modify - // the AndroidOptions passed to onGetAndroidPixels to not specify - // a subset. - defaultOptions = *options; - defaultOptions.fSubset = nullptr; - options = &defaultOptions; - } - } - - // To simplify frame compositing, force the client to use kIgnore and - // handle orientation themselves. - if (options->fFrameIndex != 0 && fOrientationBehavior == ExifOrientationBehavior::kRespect - && fCodec->getOrigin() != kDefault_SkEncodedOrigin) { + } else if (options->fSubset) { + if (!is_valid_subset(*options->fSubset, adjustedInfo.dimensions())) { return SkCodec::kInvalidParameters; } - } - if (auto result = fCodec->handleFrameIndex(requestInfo, requestPixels, requestRowBytes, - *options, this); result != SkCodec::kSuccess) { - return result; + if (SkIRect::MakeSize(adjustedInfo.dimensions()) == *options->fSubset) { + // The caller wants the whole thing, rather than a subset. Modify + // the AndroidOptions passed to onGetAndroidPixels to not specify + // a subset. + defaultOptions = *options; + defaultOptions.fSubset = nullptr; + options = &defaultOptions; + } } if (ExifOrientationBehavior::kIgnore == fOrientationBehavior) { diff --git a/src/codec/SkAndroidCodecAdapter.cpp b/src/codec/SkAndroidCodecAdapter.cpp index 2b8148bb27..5551136fa6 100644 --- a/src/codec/SkAndroidCodecAdapter.cpp +++ b/src/codec/SkAndroidCodecAdapter.cpp @@ -23,5 +23,8 @@ bool SkAndroidCodecAdapter::onGetSupportedSubset(SkIRect* desiredSubset) const { SkCodec::Result SkAndroidCodecAdapter::onGetAndroidPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, const AndroidOptions& options) { - return this->codec()->getPixels(info, pixels, rowBytes, &options); + SkCodec::Options codecOptions; + codecOptions.fZeroInitialized = options.fZeroInitialized; + codecOptions.fSubset = options.fSubset; + return this->codec()->getPixels(info, pixels, rowBytes, &codecOptions); } diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp index 533cef5309..f51efe46b6 100644 --- a/src/codec/SkCodec.cpp +++ b/src/codec/SkCodec.cpp @@ -5,7 +5,6 @@ * found in the LICENSE file. */ -#include "include/codec/SkAndroidCodec.h" #include "include/codec/SkCodec.h" #include "include/core/SkColorSpace.h" #include "include/core/SkData.h" @@ -166,7 +165,6 @@ SkCodec::SkCodec(SkEncodedInfo&& info, XformFormat srcFormat, std::unique_ptrrewindIfNeeded()) { - return kCouldNotRewind; - } - + const Options& options) { const int index = options.fFrameIndex; if (0 == index) { return this->initializeColorXform(info, fEncodedInfo.alpha(), fEncodedInfo.opaque()) @@ -314,55 +304,46 @@ SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, const int requiredFrame = frame->getRequiredFrame(); if (requiredFrame != kNoFrame) { - const SkFrame* preppedFrame = nullptr; - if (options.fPriorFrame == kNoFrame) { - Result result = kInternalError; - if (androidCodec) { -#ifdef SK_HAS_ANDROID_CODEC - SkAndroidCodec::AndroidOptions prevFrameOptions( - reinterpret_cast(options)); - prevFrameOptions.fFrameIndex = requiredFrame; - result = androidCodec->getAndroidPixels(info, pixels, rowBytes, &prevFrameOptions); -#endif - } else { - Options prevFrameOptions(options); - prevFrameOptions.fFrameIndex = requiredFrame; - result = this->getPixels(info, pixels, rowBytes, &prevFrameOptions); - } - if (result != kSuccess) { - return result; - } - preppedFrame = frameHolder->getFrame(requiredFrame); - } else { + if (options.fPriorFrame != kNoFrame) { // 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; } - preppedFrame = frameHolder->getFrame(options.fPriorFrame); - } - - SkASSERT(preppedFrame); - switch (preppedFrame->getDisposalMethod()) { - case SkCodecAnimation::DisposalMethod::kRestorePrevious: - SkASSERT(options.fPriorFrame != kNoFrame); - 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. - // FIXME: If the required frame is kRestoreBGColor, we don't actually need to decode - // it, since we'll just clear it to transparent. Instead, we could decode *its* - // required frame and then clear. - if (preppedFrame->frameId() == requiredFrame) { - SkIRect preppedRect = preppedFrame->frameRect(); - if (!zero_rect(info, pixels, rowBytes, this->dimensions(), preppedRect)) { - return kInternalError; + 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) { + SkIRect prevRect = prevFrame->frameRect(); + if (!zero_rect(info, pixels, rowBytes, this->dimensions(), prevRect)) { + return kInternalError; + } } + break; + default: + break; + } + } else { + Options prevFrameOptions(options); + prevFrameOptions.fFrameIndex = requiredFrame; + prevFrameOptions.fZeroInitialized = kNo_ZeroInitialized; + const Result result = this->getPixels(info, pixels, rowBytes, &prevFrameOptions); + if (result != kSuccess) { + return result; + } + const auto* prevFrame = frameHolder->getFrame(requiredFrame); + const auto disposalMethod = prevFrame->getDisposalMethod(); + if (disposalMethod == SkCodecAnimation::DisposalMethod::kRestoreBGColor) { + auto prevRect = prevFrame->frameRect(); + if (!zero_rect(info, pixels, rowBytes, this->dimensions(), prevRect)) { + return kInternalError; } - break; - default: - break; + } } } @@ -382,6 +363,10 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t return kInvalidParameters; } + if (!this->rewindIfNeeded()) { + return kCouldNotRewind; + } + // Default options. Options optsStorage; if (nullptr == options) { @@ -447,6 +432,15 @@ SkCodec::Result SkCodec::startIncrementalDecode(const SkImageInfo& info, void* p return kInvalidParameters; } + // 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) { @@ -501,6 +495,10 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& info, // Reset fCurrScanline in case of failure. fCurrScanline = -1; + if (!this->rewindIfNeeded()) { + return kCouldNotRewind; + } + // Set options. Options optsStorage; if (nullptr == options) { @@ -540,15 +538,6 @@ SkCodec::Result SkCodec::startScanlineDecode(const SkImageInfo& info, return result; } - // FIXME: See startIncrementalDecode. That method set fNeedsRewind to false - // so that when onStartScanlineDecode calls rewindIfNeeded it would not - // rewind. But it also relies on that call to rewindIfNeeded to set - // fNeedsRewind to true for future decodes. When - // fAndroidCodecHandlesFrameIndex is true, that call to rewindIfNeeded is - // skipped, so this method sets it back to true. - SkASSERT(fAndroidCodecHandlesFrameIndex || fNeedsRewind); - fNeedsRewind = true; - fCurrScanline = 0; fDstInfo = info; fOptions = *options; diff --git a/src/codec/SkSampledCodec.cpp b/src/codec/SkSampledCodec.cpp index 4371e69379..b7ac0ebac1 100644 --- a/src/codec/SkSampledCodec.cpp +++ b/src/codec/SkSampledCodec.cpp @@ -73,10 +73,14 @@ SkISize SkSampledCodec::onGetSampledDimensions(int sampleSize) const { SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, const AndroidOptions& options) { - const SkIRect* subset = options.fSubset; + // Create an Options struct for the codec. + SkCodec::Options codecOptions; + codecOptions.fZeroInitialized = options.fZeroInitialized; + + SkIRect* subset = options.fSubset; if (!subset || subset->size() == this->codec()->dimensions()) { if (this->codec()->dimensionsSupported(info.dimensions())) { - return this->codec()->getPixels(info, pixels, rowBytes, &options); + return this->codec()->getPixels(info, pixels, rowBytes, &codecOptions); } // If the native codec does not support the requested scale, scale by sampling. @@ -99,17 +103,15 @@ SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void const SkImageInfo scaledInfo = info.makeDimensions(scaledSize); - // Copy so we can use a different fSubset. - AndroidOptions subsetOptions = options; { // Although startScanlineDecode expects the bottom and top to match the // SkImageInfo, startIncrementalDecode uses them to determine which rows to // decode. SkIRect incrementalSubset = SkIRect::MakeXYWH(scaledSubsetX, scaledSubsetY, scaledSubsetWidth, scaledSubsetHeight); - subsetOptions.fSubset = &incrementalSubset; + codecOptions.fSubset = &incrementalSubset; const SkCodec::Result startResult = this->codec()->startIncrementalDecode( - scaledInfo, pixels, rowBytes, &subsetOptions); + scaledInfo, pixels, rowBytes, &codecOptions); if (SkCodec::kSuccess == startResult) { int rowsDecoded = 0; const SkCodec::Result incResult = this->codec()->incrementalDecode(&rowsDecoded); @@ -126,17 +128,17 @@ SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void return startResult; } // Otherwise fall down to use the old scanline decoder. - // subsetOptions.fSubset will be reset below, so it will not continue to + // codecOptions.fSubset will be reset below, so it will not continue to // point to the object that is no longer on the stack. } // Start the scanline decode. SkIRect scanlineSubset = SkIRect::MakeXYWH(scaledSubsetX, 0, scaledSubsetWidth, scaledSize.height()); - subsetOptions.fSubset = &scanlineSubset; + codecOptions.fSubset = &scanlineSubset; SkCodec::Result result = this->codec()->startScanlineDecode(scaledInfo, - &subsetOptions); + &codecOptions); if (SkCodec::kSuccess != result) { return result; } @@ -165,6 +167,10 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix // We should only call this function when sampling. SkASSERT(options.fSampleSize > 1); + // Create options struct for the codec. + SkCodec::Options sampledOptions; + sampledOptions.fZeroInitialized = options.fZeroInitialized; + // FIXME: This was already called by onGetAndroidPixels. Can we reduce that? int sampleSize = options.fSampleSize; int nativeSampleSize; @@ -192,6 +198,7 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix // The scanline decoder only needs to be aware of subsetting in the x-dimension. subset.setXYWH(subsetX, 0, subsetWidth, nativeSize.height()); + sampledOptions.fSubset = ⊂ } // Since we guarantee that output dimensions are always at least one (even if the sampleSize @@ -210,13 +217,13 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix // Although startScanlineDecode expects the bottom and top to match the // SkImageInfo, startIncrementalDecode uses them to determine which rows to // decode. - AndroidOptions incrementalOptions = options; + SkCodec::Options incrementalOptions = sampledOptions; SkIRect incrementalSubset; - if (options.fSubset) { - incrementalSubset.fTop = subsetY; - incrementalSubset.fBottom = subsetY + subsetHeight; - incrementalSubset.fLeft = subset.fLeft; - incrementalSubset.fRight = subset.fRight; + if (sampledOptions.fSubset) { + incrementalSubset.fTop = subsetY; + incrementalSubset.fBottom = subsetY + subsetHeight; + incrementalSubset.fLeft = sampledOptions.fSubset->fLeft; + incrementalSubset.fRight = sampledOptions.fSubset->fRight; incrementalOptions.fSubset = &incrementalSubset; } const SkCodec::Result startResult = this->codec()->startIncrementalDecode(nativeInfo, @@ -256,10 +263,6 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix } // Start the scanline decode. - AndroidOptions sampledOptions = options; - if (options.fSubset) { - sampledOptions.fSubset = ⊂ - } SkCodec::Result result = this->codec()->startScanlineDecode(nativeInfo, &sampledOptions); if (SkCodec::kIncompleteInput == result || SkCodec::kErrorInInput == result) { diff --git a/tests/AndroidCodecTest.cpp b/tests/AndroidCodecTest.cpp index 454ea077e8..7193a2a18e 100644 --- a/tests/AndroidCodecTest.cpp +++ b/tests/AndroidCodecTest.cpp @@ -300,61 +300,3 @@ DEF_TEST(AndroidCodec_sampledOrientation, r) { ERRORF(r, "got result \"%s\"\n", SkCodec::ResultToString(result)); } } - -DEF_TEST(AndroidCodec_animatedOrientation, r) { - if (GetResourcePath().isEmpty()) { - return; - } - - static const struct { - const char* path; - SkEncodedOrigin origin; - } gRec[] = { - { "images/stoplight.webp", kDefault_SkEncodedOrigin }, - { "images/stoplight_h.webp", kLeftBottom_SkEncodedOrigin } // Rotated 90 CCW - }; - for (auto rec : gRec) { - auto data = GetResourceAsData(rec.path); - if (!data) { - ERRORF(r, "Failed to get resource %s", rec.path); - return; - } - - // SkAndroidCodec now allows decoding frames beyond the first, but combining this with - // kRespect-ing a non-kDefault_SkEncodedOrigin is not supported. If a frame depends on - // a prior frame, we allow a client to provide that prior frame. But in order to respect - // the origin, we would need to transform the prior frame back to the original orientation - // in order to blend (and potentially erase, for a kRestoreBG frame) just to transform it - // back. Instead, force the client to handle the orientation after the fact. - for (auto behavior : { SkAndroidCodec::ExifOrientationBehavior::kRespect, - SkAndroidCodec::ExifOrientationBehavior::kIgnore }) { - auto androidCodec = SkAndroidCodec::MakeFromCodec(SkCodec::MakeFromData(data), - behavior); - REPORTER_ASSERT(r, androidCodec->codec()->getOrigin() == rec.origin); - - auto info = androidCodec->getInfo(); - SkBitmap bm; - bm.allocPixels(info); - auto result = androidCodec->getAndroidPixels(info, bm.getPixels(), bm.rowBytes()); - REPORTER_ASSERT(r, result == SkCodec::kSuccess); - - SkAndroidCodec::AndroidOptions options; - options.fFrameIndex = 1; - options.fPriorFrame = 0; - result = androidCodec->getAndroidPixels(info, bm.getPixels(), bm.rowBytes(), &options); - switch (behavior) { - case SkAndroidCodec::ExifOrientationBehavior::kRespect: - if (rec.origin != kDefault_SkEncodedOrigin) { - REPORTER_ASSERT(r, result == SkCodec::kInvalidParameters, - "Should not be able to decode frame 1 with exif orientation" - " directly! Result: %s", SkCodec::ResultToString(result)); - break; - } - [[fallthrough]]; - case SkAndroidCodec::ExifOrientationBehavior::kIgnore: - REPORTER_ASSERT(r, result == SkCodec::kSuccess); - break; - } - } - } -} diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp index b0ccb99d62..2d872a107f 100644 --- a/tests/CodecAnimTest.cpp +++ b/tests/CodecAnimTest.cpp @@ -389,15 +389,16 @@ DEF_TEST(Codec_frames, r) { } } -// Verify that an image can be animated scaled down. These images have a -// kRestoreBG frame, so they are interesting to test. After decoding that +// Verify that a webp image can be animated scaled down. This image has a +// kRestoreBG frame, so it is an interesting image to test. After decoding that // frame, we have to erase its rectangle. The rectangle has to be adjusted // based on the scaled size. -static void test_animated_AndroidCodec(skiatest::Reporter* r, const char* file) { +DEF_TEST(AndroidCodec_animated, r) { if (GetResourcePath().isEmpty()) { return; } + const char* file = "images/required.webp"; sk_sp data(GetResourceAsData(file)); if (!data) { ERRORF(r, "Missing %s", file); @@ -455,14 +456,6 @@ static void test_animated_AndroidCodec(skiatest::Reporter* r, const char* file) } } -DEF_TEST(AndroidCodec_animated, r) { - test_animated_AndroidCodec(r, "images/required.webp"); -} - -DEF_TEST(AndroidCodec_animated_gif, r) { - test_animated_AndroidCodec(r, "images/required.gif"); -} - DEF_TEST(EncodedOriginToMatrixTest, r) { // SkAnimCodecPlayer relies on the fact that these matrices are invertible. for (auto origin : { kTopLeft_SkEncodedOrigin , diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp index 08f19f30b4..acfda47727 100644 --- a/tests/CodecTest.cpp +++ b/tests/CodecTest.cpp @@ -610,10 +610,7 @@ static void test_dimensions(skiatest::Reporter* r, const char path[]) { options.fSampleSize = sampleSize; SkCodec::Result result = codec->getAndroidPixels(scaledInfo, pixels.get(), rowBytes, &options); - if (result != SkCodec::kSuccess) { - ERRORF(r, "Failed to decode %s with sample size %i; error: %s", path, sampleSize, - SkCodec::ResultToString(result)); - } + REPORTER_ASSERT(r, SkCodec::kSuccess == result); } }