Reland "SkAndroidCodec: Support decoding all frames"
This is a reland of fc4fdc5b25f448dd9c2cd4e445561a840ce8514b
Original change's description:
> SkAndroidCodec: Support decoding all frames
>
> Bug: b/160984428
> Bug: b/163595585
>
> Add support to SkAndroidCodec for decoding all frames with an
> fSampleSize, so that an entire animation can be decoded to a smaller
> size.
>
> dm/:
> - Test scaled + animated decodes
>
> SkAndroidCodec:
> - Make AndroidOptions inherit from SkCodec::Options. This allows
> SkAndroidCodec to use fFrameIndex. (It also combines the two versions
> of fSubset, which is now const for both.)
> - Respect fFrameIndex, and call SkCodec::handleFrameIndex to decode
> the required frame.
> - Disallow decoding with kRespect + fFrameIndex > 0 if there is a
> non-default orientation. As currently written (except without
> disabling this combination), SkPixmapPriv::Orient would draw the new
> portion of the frame on top of uninitialized pixels, instead of the
> prior frame. This could be fixed by
> - If SkAndroidCodec needs to decode the required frame, it could do so
> without applying the orientation, then decode fFrameIndex, and then
> apply the orientation.
> - If the client provided the required frame, SkAndroidCodec would need
> to un-apply the orientation to get the proper starting state, then
> decode and apply.
> I think it is simpler to force the client to handle the orientation
> externally.
>
> SkCodec:
> - Allow SkAndroidCodec to call its private method handleFrameIndex. This
> method handles decoding a required frame, if necessary. When called by
> SkAndroidCodec, it now uses the SkAndroidCodec to check for/decode the
> required frame, so that it will scale properly.
> - Call rewindIfNeeded inside handleFrameIndex. handleFrameIndex calls a
> virtual method which may set some state (e.g. in SkJpegCodec). Without
> this change, that state would be reset by rewindIfNeeded.
> - Simplify handling a kRestoreBGColor frame. Whether provided or not,
> take the same path to calling zero_rect.
> - Updates to zero_rect:
> - Intersect after scaling, which will also check for empty.
> - Round out instead of in - this ensures we don't under-erase
> - Use kFill_ScaleToFit, which better matches the intent.
>
> Change-Id: Ibe1951980a0dca8f5b7b1f20192432d395681683
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333225
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
Bug: b/160984428
Bug: b/163595585
Change-Id: I7c1e79e0f92c75b4840eef65c8fc2b8497189e81
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334842
Auto-Submit: Leon Scroggins <scroggo@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index f51efe4..533cef5 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -5,6 +5,7 @@
* 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"
@@ -165,6 +166,7 @@
, fOptions()
, fCurrScanline(-1)
, fStartedIncrementalDecode(false)
+ , fAndroidCodecHandlesFrameIndex(false)
{}
SkCodec::~SkCodec() {}
@@ -245,25 +247,21 @@
bool zero_rect(const SkImageInfo& dstInfo, void* pixels, size_t rowBytes,
SkISize srcDimensions, SkIRect prevRect) {
- prevRect = frame_rect_on_screen(prevRect, SkIRect::MakeSize(srcDimensions));
- if (prevRect.isEmpty()) {
- return true;
- }
const auto dimensions = dstInfo.dimensions();
if (dimensions != srcDimensions) {
SkRect src = SkRect::Make(srcDimensions);
SkRect dst = SkRect::Make(dimensions);
- SkMatrix map = SkMatrix::MakeRectToRect(src, dst, SkMatrix::kCenter_ScaleToFit);
+ SkMatrix map = SkMatrix::MakeRectToRect(src, dst, SkMatrix::kFill_ScaleToFit);
SkRect asRect = SkRect::Make(prevRect);
if (!map.mapRect(&asRect)) {
return false;
}
- asRect.roundIn(&prevRect);
- if (prevRect.isEmpty()) {
- // Down-scaling shrank the empty portion to nothing,
- // so nothing to zero.
- return true;
- }
+ asRect.roundOut(&prevRect);
+ }
+
+ if (!prevRect.intersect(SkIRect::MakeSize(dimensions))) {
+ // Nothing to zero, due to scaling or bad frame rect.
+ return true;
}
const SkImageInfo info = dstInfo.makeDimensions(prevRect.size());
@@ -275,7 +273,19 @@
}
SkCodec::Result SkCodec::handleFrameIndex(const SkImageInfo& info, void* pixels, size_t rowBytes,
- const Options& options) {
+ const Options& options, SkAndroidCodec* androidCodec) {
+ if (androidCodec) {
+ // This is never set back to false. If SkAndroidCodec is calling this method, its fCodec
+ // should never call it directly.
+ fAndroidCodecHandlesFrameIndex = true;
+ } else if (fAndroidCodecHandlesFrameIndex) {
+ return kSuccess;
+ }
+
+ if (!this->rewindIfNeeded()) {
+ return kCouldNotRewind;
+ }
+
const int index = options.fFrameIndex;
if (0 == index) {
return this->initializeColorXform(info, fEncodedInfo.alpha(), fEncodedInfo.opaque())
@@ -304,46 +314,55 @@
const int requiredFrame = frame->getRequiredFrame();
if (requiredFrame != kNoFrame) {
- if (options.fPriorFrame != kNoFrame) {
+ const SkFrame* preppedFrame = nullptr;
+ if (options.fPriorFrame == kNoFrame) {
+ Result result = kInternalError;
+ if (androidCodec) {
+#ifdef SK_HAS_ANDROID_CODEC
+ SkAndroidCodec::AndroidOptions prevFrameOptions(
+ reinterpret_cast<const SkAndroidCodec::AndroidOptions&>(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 {
// 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) {
- SkIRect prevRect = prevFrame->frameRect();
- if (!zero_rect(info, pixels, rowBytes, this->dimensions(), prevRect)) {
- return kInternalError;
- }
+ 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;
}
- 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;
}
}
@@ -363,10 +382,6 @@
return kInvalidParameters;
}
- if (!this->rewindIfNeeded()) {
- return kCouldNotRewind;
- }
-
// Default options.
Options optsStorage;
if (nullptr == options) {
@@ -432,15 +447,6 @@
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) {
@@ -495,10 +501,6 @@
// Reset fCurrScanline in case of failure.
fCurrScanline = -1;
- if (!this->rewindIfNeeded()) {
- return kCouldNotRewind;
- }
-
// Set options.
Options optsStorage;
if (nullptr == options) {
@@ -538,6 +540,15 @@
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;