Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-27 23:02:20 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1509408 - Fix animated image distortion regressions caused by frame recycling. r=tnikkel

Browse Source 
When bug 1508393 landed, it not only enabled producing of full frames
on the decoder threads, it also enabled recycling of old animated image
frame buffers it would have otherwise discarded. It reuses the contents
of the buffer where possible given we know what pixels changed between
the old frame and the frame we want to produce. However where this
calculation was done was incorrect. We originally calculated it when we
advanced the frame, but at that point there is no guarantee that we have
all of the necessary information; we may have fallen behind on decoding.
As such, we move the calculation to where we actually perform the
recycling. At this point we are guaranteed to have all the necessary
frames between the recycling and display queues.

Differential Revision: https://phabricator.services.mozilla.com/D12903
This commit is contained in:
Andrew Osmond 2018-11-26 07:30:50 -05:00
parent 70de4bde06
commit a24f72635e
3 changed files with 178 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
image/AnimationFrameBuffer.cpp
View File

@ -324,6 +324,7 @@ AnimationFrameDiscardingQueue::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf
AnimationFrameRecyclingQueue::AnimationFrameRecyclingQueue(AnimationFrameRetainedBuffer&& aQueue)
: AnimationFrameDiscardingQueue(std::move(aQueue))
, mForceUseFirstFrameRefreshArea(false)
{
// In an ideal world, we would always save the already displayed frames for
// recycling but none of the frames were marked as recyclable. We will incur
@ -361,31 +362,20 @@ AnimationFrameRecyclingQueue::AdvanceInternal()
MOZ_ASSERT(!mDisplay.empty());
MOZ_ASSERT(mDisplay.front());
// We have advanced past the first frame. That means the next frame we are
// putting in the queue to recycling is the first frame in the animation,
// and we no longer need to worry about having looped around.
if (mGetIndex == 1) {
mForceUseFirstFrameRefreshArea = false;
}
RefPtr<imgFrame>& front = mDisplay.front();
// The first frame should always have a dirty rect that matches the frame
// rect. As such, we should use mFirstFrameRefreshArea instead for recycle
// rect calculations.
MOZ_ASSERT_IF(mGetIndex == 1,
front->GetRect().IsEqualEdges(front->GetDirtyRect()));
RecycleEntry newEntry(mGetIndex == 1 ? mFirstFrameRefreshArea
: front->GetDirtyRect());
RecycleEntry newEntry(mForceUseFirstFrameRefreshArea ? mFirstFrameRefreshArea
: front->GetDirtyRect());
// If we are allowed to recycle the frame, then we should save it before the
// base class's AdvanceInternal discards it.
if (front->ShouldRecycle()) {
// Calculate the recycle rect for the recycled frame. This is the cumulative
// dirty rect of all of the frames ahead of us to be displayed, and to be
// used for recycling. Or in other words, the dirty rect between the
// recycled frame and the decoded frame which reuses the buffer.
for (const RefPtr<imgFrame>& frame : mDisplay) {
newEntry.mRecycleRect = newEntry.mRecycleRect.Union(frame->GetDirtyRect());
}
for (const RecycleEntry& entry : mRecycle) {
newEntry.mRecycleRect = newEntry.mRecycleRect.Union(entry.mDirtyRect);
}
newEntry.mFrame = std::move(front);
}
@ -428,20 +418,62 @@ AnimationFrameRecyclingQueue::ResetInternal()
RawAccessFrameRef
AnimationFrameRecyclingQueue::RecycleFrame(gfx::IntRect& aRecycleRect)
{
if (mInsertIndex == 0) {
// If we are recreating the first frame, then we actually have already
// precomputed aggregate of the dirty rects as the first frame refresh
// area. We know that all of the frames still in the recycling queue
// need to take into account the same dirty rect because they are also
// frames which cross the boundary.
MOZ_ASSERT(mSizeKnown);
MOZ_ASSERT(!mFirstFrameRefreshArea.IsEmpty());
for (RecycleEntry& entry : mRecycle) {
MOZ_ASSERT(mFirstFrameRefreshArea.Contains(entry.mDirtyRect));
entry.mDirtyRect = mFirstFrameRefreshArea;
}
// Until we advance to the first frame again, any subsequent recycled
// frames should also use the first frame refresh area.
mForceUseFirstFrameRefreshArea = true;
}
if (mRecycle.empty()) {
return RawAccessFrameRef();
}
RawAccessFrameRef frame;
RawAccessFrameRef recycledFrame;
if (mRecycle.front().mFrame) {
frame = mRecycle.front().mFrame->RawAccessRef();
if (frame) {
aRecycleRect = mRecycle.front().mRecycleRect;
recycledFrame = mRecycle.front().mFrame->RawAccessRef();
MOZ_ASSERT(recycledFrame);
mRecycle.pop_front();
if (mForceUseFirstFrameRefreshArea) {
// We are still crossing the loop boundary and cannot rely upon the dirty
// rects of entries in mDisplay to be representative. E.g. The first frame
// is probably has a full frame dirty rect.
aRecycleRect = mFirstFrameRefreshArea;
} else {
// Calculate the recycle rect for the recycled frame. This is the
// cumulative dirty rect of all of the frames ahead of us to be displayed,
// and to be used for recycling. Or in other words, the dirty rect between
// the recycled frame and the decoded frame which reuses the buffer.
//
// We know at this point that mRecycle contains either frames from the end
// of the animation with the first frame refresh area as the dirty rect
// (plus the first frame likewise) and frames with their actual dirty rect
// from the start. mDisplay should also only contain frames from the start
// of the animation onwards.
aRecycleRect.SetRect(0, 0, 0, 0);
for (const RefPtr<imgFrame>& frame : mDisplay) {
aRecycleRect = aRecycleRect.Union(frame->GetDirtyRect());
}
for (const RecycleEntry& entry : mRecycle) {
aRecycleRect = aRecycleRect.Union(entry.mDirtyRect);
}
}
} else {
mRecycle.pop_front();
}
mRecycle.pop_front();
return frame;
return recycledFrame;
}
bool

12
image/AnimationFrameBuffer.h
View File

@ -449,15 +449,12 @@ public:
RecycleEntry(RecycleEntry&& aOther)
: mFrame(std::move(aOther.mFrame))
, mDirtyRect(aOther.mDirtyRect)
, mRecycleRect(aOther.mRecycleRect)
{
}
{ }
RecycleEntry& operator=(RecycleEntry&& aOther)
{
mFrame = std::move(aOther.mFrame);
mDirtyRect = aOther.mDirtyRect;
mRecycleRect = aOther.mRecycleRect;
return *this;
}
@ -466,8 +463,6 @@ public:
RefPtr<imgFrame> mFrame; // The frame containing the buffer to recycle.
gfx::IntRect mDirtyRect; // The dirty rect of the frame itself.
gfx::IntRect mRecycleRect; // The dirty rect between the recycled frame and
// the future frame that will be written to it.
};
const std::deque<RecycleEntry>& Recycle() const { return mRecycle; }
@ -488,6 +483,11 @@ protected:
/// The first frame refresh area. This is used instead of the dirty rect for
/// the last frame when transitioning back to the first frame.
gfx::IntRect mFirstFrameRefreshArea;
/// Force recycled frames to use the first frame refresh area as their dirty
/// rect. This is used when we are recycling frames from the end of an
/// animation to produce frames at the beginning of an animation.
bool mForceUseFirstFrameRefreshArea;
};
} // namespace image

122
image/test/gtest/TestAnimationFrameBuffer.cpp
View File

@ -27,6 +27,9 @@ CreateEmptyFrame(const IntSize& aSize = IntSize(1, 1),
EXPECT_TRUE(NS_SUCCEEDED(rv));
RawAccessFrameRef frameRef = frame->RawAccessRef();
frame->SetRawAccessOnly();
// Normally the blend animation filter would set the dirty rect, but since
// we aren't producing an actual animation here, we need to fake it.
frame->SetDirtyRect(aFrameRect);
frame->Finish();
return frame.forget();
}
@ -105,16 +108,13 @@ VerifyAdvance(AnimationFrameBuffer& aQueue,
if (aQueue.IsRecycling()) {
const AnimationFrameRecyclingQueue& queue =
*static_cast<AnimationFrameRecyclingQueue*>(&aQueue);
EXPECT_FALSE(queue.Recycle().back().mDirtyRect.IsEmpty());
EXPECT_TRUE(queue.Recycle().back().mDirtyRect.Contains(oldFrame->GetDirtyRect()));
EXPECT_EQ(totalRecycled + 1, queue.Recycle().size());
if (oldFrame->ShouldRecycle()) {
EXPECT_EQ(oldFrame.get(), queue.Recycle().back().mFrame.get());
EXPECT_FALSE(queue.Recycle().back().mDirtyRect.IsEmpty());
EXPECT_FALSE(queue.Recycle().back().mRecycleRect.IsEmpty());
EXPECT_EQ(totalRecycled + 1, queue.Recycle().size());
} else {
EXPECT_EQ(totalRecycled, queue.Recycle().size());
if (!queue.Recycle().empty()) {
EXPECT_NE(oldFrame.get(), queue.Recycle().back().mFrame.get());
}
EXPECT_EQ(nullptr, queue.Recycle().back().mFrame.get());
}
}
}
@ -584,7 +584,7 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingLoop)
// All the frames we inserted should have been recycleable.
ASSERT_FALSE(buffer.Recycle().empty());
while (!buffer.Recycle().empty()) {
IntRect expectedRect = buffer.Recycle().front().mRecycleRect;
IntRect expectedRect(0, 0, 1, 1);
RefPtr<imgFrame> expectedFrame = buffer.Recycle().front().mFrame;
EXPECT_FALSE(expectedRect.IsEmpty());
EXPECT_TRUE(expectedFrame.get() != nullptr);
@ -646,3 +646,109 @@ TEST_F(ImageAnimationFrameBuffer, RecyclingReset)
AnimationFrameRecyclingQueue buffer(std::move(retained));
TestDiscardingQueueReset(buffer, firstFrame, kThreshold, kBatch, kStartFrame);
}
TEST_F(ImageAnimationFrameBuffer, RecyclingRect)
{
const size_t kThreshold = 5;
const size_t kBatch = 2;
const size_t kStartFrame = 0;
const IntSize kImageSize(100, 100);
AnimationFrameRetainedBuffer retained(kThreshold, kBatch, kStartFrame);
// Let's get to the recycling state while marking all of the frames as not
// recyclable, just like AnimationFrameBuffer / the decoders would do.
RefPtr<imgFrame> frame;
frame = CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
AnimationFrameBuffer::InsertStatus status = retained.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::CONTINUE, status);
frame = CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
status = retained.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::CONTINUE, status);
frame = CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
status = retained.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::CONTINUE, status);
frame = CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
status = retained.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::YIELD, status);
VerifyAdvance(retained, 1, false);
VerifyAdvance(retained, 2, true);
VerifyAdvance(retained, 3, false);
frame = CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
status = retained.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::DISCARD_CONTINUE, status);
AnimationFrameRecyclingQueue buffer(std::move(retained));
// The first frame is now the candidate for recycling. Since it was marked as
// not recyclable, we should get nothing.
VerifyAdvance(buffer, 4, false);
IntRect recycleRect;
EXPECT_FALSE(buffer.Recycle().empty());
RawAccessFrameRef frameRef = buffer.RecycleFrame(recycleRect);
EXPECT_FALSE(frameRef);
EXPECT_TRUE(recycleRect.IsEmpty());
EXPECT_TRUE(buffer.Recycle().empty());
// Insert a recyclable partial frame. Its dirty rect shouldn't matter since
// the previous frame was not recyclable.
frame = CreateEmptyFrame(kImageSize, IntRect(0, 0, 25, 25));
status = buffer.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::YIELD, status);
VerifyAdvance(buffer, 5, true);
EXPECT_FALSE(buffer.Recycle().empty());
frameRef = buffer.RecycleFrame(recycleRect);
EXPECT_FALSE(frameRef);
EXPECT_TRUE(recycleRect.IsEmpty());
EXPECT_TRUE(buffer.Recycle().empty());
// Insert a recyclable partial frame. Its dirty rect should match the recycle
// rect since it is the only frame in the buffer.
frame = CreateEmptyFrame(kImageSize, IntRect(25, 0, 50, 50));
status = buffer.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::YIELD, status);
VerifyAdvance(buffer, 6, true);
EXPECT_FALSE(buffer.Recycle().empty());
frameRef = buffer.RecycleFrame(recycleRect);
EXPECT_TRUE(frameRef);
EXPECT_EQ(IntRect(25, 0, 50, 50), recycleRect);
EXPECT_TRUE(buffer.Recycle().empty());
// Insert the last frame and mark us as complete. The next recycled frame is
// producing the first frame again, so we should use the first frame refresh
// area instead of its dirty rect.
frame = CreateEmptyFrame(kImageSize, IntRect(10, 10, 60, 10));
status = buffer.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::YIELD, status);
bool continueDecoding = buffer.MarkComplete(IntRect(0, 0, 75, 50));
EXPECT_FALSE(continueDecoding);
VerifyAdvance(buffer, 7, true);
EXPECT_FALSE(buffer.Recycle().empty());
frameRef = buffer.RecycleFrame(recycleRect);
EXPECT_TRUE(frameRef);
EXPECT_EQ(IntRect(0, 0, 75, 50), recycleRect);
EXPECT_TRUE(buffer.Recycle().empty());
// Now let's reinsert the first frame. The recycle rect should still be the
// first frame refresh area instead of the dirty rect of the first frame (e.g.
// the full frame).
frame = CreateEmptyFrame(kImageSize, IntRect(IntPoint(0, 0), kImageSize), false);
status = buffer.Insert(std::move(frame));
EXPECT_EQ(AnimationFrameBuffer::InsertStatus::YIELD, status);
VerifyAdvance(buffer, 0, true);
EXPECT_FALSE(buffer.Recycle().empty());
frameRef = buffer.RecycleFrame(recycleRect);
EXPECT_TRUE(frameRef);
EXPECT_EQ(IntRect(0, 0, 75, 50), recycleRect);
EXPECT_TRUE(buffer.Recycle().empty());
}