Skip to content

Commit

Permalink
Bug 1465619 - Part 4. Move the first frame refresh area calculation t…
Browse files Browse the repository at this point in the history
…o frame commit. r=tnikkel

If we discard a frame during decoding, e.g. due to an error, then we
don't want to take that frame into account for the first frame refresh
area. We should also be handling partial frames here (the dirty rect
needs to encompass the rows that did not get written with actual pixel
data). The only place that can have the necessary information is at the
end rather than at the beginning.

Differential Revision: https://phabricator.services.mozilla.com/D7509
  • Loading branch information
aosmond committed Oct 22, 2018
1 parent bc81571 commit 01505bd
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 20 deletions.
48 changes: 28 additions & 20 deletions image/Decoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -361,26 +361,11 @@ Decoder::AllocateFrameInternal(const gfx::IntSize& aOutputSize,
if (frameNum == 1) {
MOZ_ASSERT(aPreviousFrame, "Must provide a previous frame when animated");
aPreviousFrame->SetRawAccessOnly();

// If we dispose of the first frame by clearing it, then the first frame's
// refresh area is all of itself.
// RESTORE_PREVIOUS is invalid (assumed to be DISPOSE_CLEAR).
DisposalMethod prevDisposal = aPreviousFrame->GetDisposalMethod();
if (prevDisposal == DisposalMethod::CLEAR ||
prevDisposal == DisposalMethod::CLEAR_ALL ||
prevDisposal == DisposalMethod::RESTORE_PREVIOUS) {
mFirstFrameRefreshArea = aPreviousFrame->GetRect();
}
}

if (frameNum > 0) {
ref->SetRawAccessOnly();

// Some GIFs are huge but only have a small area that they animate. We only
// need to refresh that small area when frame 0 comes around again.
mFirstFrameRefreshArea.UnionRect(mFirstFrameRefreshArea,
ref->GetBoundedBlendRect());

if (ShouldBlendAnimation()) {
if (aPreviousFrame->GetDisposalMethod() !=
DisposalMethod::RESTORE_PREVIOUS) {
Expand Down Expand Up @@ -495,11 +480,34 @@ Decoder::PostFrameStop(Opacity aFrameOpacity)

mLoopLength += mCurrentFrame->GetTimeout();

// If we're not sending partial invalidations, then we send an invalidation
// here when the first frame is complete.
if (!ShouldSendPartialInvalidations() && mFrameCount == 1) {
mInvalidRect.UnionRect(mInvalidRect,
IntRect(IntPoint(), Size()));
if (mFrameCount == 1) {
// If we're not sending partial invalidations, then we send an invalidation
// here when the first frame is complete.
if (!ShouldSendPartialInvalidations()) {
mInvalidRect.UnionRect(mInvalidRect,
IntRect(IntPoint(), Size()));
}

// If we dispose of the first frame by clearing it, then the first frame's
// refresh area is all of itself. RESTORE_PREVIOUS is invalid (assumed to
// be DISPOSE_CLEAR).
switch (mCurrentFrame->GetDisposalMethod()) {
default:
MOZ_FALLTHROUGH_ASSERT("Unexpected DisposalMethod");
case DisposalMethod::CLEAR:
case DisposalMethod::CLEAR_ALL:
case DisposalMethod::RESTORE_PREVIOUS:
mFirstFrameRefreshArea = IntRect(IntPoint(), Size());
break;
case DisposalMethod::KEEP:
case DisposalMethod::NOT_SPECIFIED:
break;
}
} else {
// Some GIFs are huge but only have a small area that they animate. We only
// need to refresh that small area when frame 0 comes around again.
mFirstFrameRefreshArea.UnionRect(mFirstFrameRefreshArea,
mCurrentFrame->GetBoundedBlendRect());
}
}

Expand Down
5 changes: 5 additions & 0 deletions image/Decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,11 @@ class Decoder
return mRecycleRect;
}

const gfx::IntRect& GetFirstFrameRefreshArea() const
{
return mFirstFrameRefreshArea;
}

bool HasFrameToTake() const { return mHasFrameToTake; }
void ClearHasFrameToTake() {
MOZ_ASSERT(mHasFrameToTake);
Expand Down

0 comments on commit 01505bd

Please sign in to comment.