Bug 1465619 - Part 4. Move the first frame refresh area calculation to frame commit. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Sun, 03 Jun 2018 19:21:48 -0400
changeset 490706 edeea0a49a3322a3af796fba2f582ed0f1f6bd5f
parent 490705 fc76b59e51b4f9faf21f40f2f518a94ce87ea100
child 490707 ddaa812c31e4589b269dc622c6f6a1bd049f81ec
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerstnikkel
bugs1465619
milestone65.0a1
Bug 1465619 - Part 4. Move the first frame refresh area calculation to 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
image/Decoder.cpp
image/Decoder.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -356,36 +356,21 @@ Decoder::AllocateFrameInternal(const gfx
   if (!ref) {
     frame->Abort();
     return RawAccessFrameRef();
   }
 
   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) {
         // If the new restore frame is the direct previous frame, then we know
         // the dirty rect is composed only of the current frame's blend rect and
         // the restore frame's clear rect (if applicable) which are handled in
         // filters.
         mRestoreFrame = std::move(aPreviousFrame);
@@ -490,21 +475,44 @@ Decoder::PostFrameStop(Opacity aFrameOpa
   mFinishedNewFrame = true;
 
   mCurrentFrame->Finish(aFrameOpacity, mFinalizeFrames);
 
   mProgress |= FLAG_FRAME_COMPLETE;
 
   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());
   }
 }
 
 void
 Decoder::PostInvalidation(const gfx::IntRect& aRect,
                           const Maybe<gfx::IntRect>& aRectAtOutputSize
                             /* = Nothing() */)
 {
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -446,16 +446,21 @@ public:
   }
 
   const gfx::IntRect& GetRecycleRect() const
   {
     MOZ_ASSERT(ShouldBlendAnimation());
     return mRecycleRect;
   }
 
+  const gfx::IntRect& GetFirstFrameRefreshArea() const
+  {
+    return mFirstFrameRefreshArea;
+  }
+
   bool HasFrameToTake() const { return mHasFrameToTake; }
   void ClearHasFrameToTake() {
     MOZ_ASSERT(mHasFrameToTake);
     mHasFrameToTake = false;
   }
 
 protected:
   friend class AutoRecordDecoderTelemetry;