Bug 1353299. Make sure to invalidate when composited frame becomes valid. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Thu, 06 Apr 2017 04:00:36 -0500
changeset 399720 8b4e96304c8d9c5b9ce79333741d2da51be3669d
parent 399719 94a566b7fe81021a899fcbadfb0b85cd66e95688
child 399721 1c0d3fedaa67a06344c1b15a38ea25609914b01b
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1353299
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1353299. Make sure to invalidate when composited frame becomes valid. r=aosmond We draw nothing when the composited frame is invalid, so when we mark it valid we should invalidate. Usually the action that causes the composited frame to be valid will invalidate (ie RequestRefresh).
image/FrameAnimator.cpp
image/FrameAnimator.h
image/RasterImage.cpp
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -20,33 +20,34 @@ namespace mozilla {
 using namespace gfx;
 
 namespace image {
 
 ///////////////////////////////////////////////////////////////////////////////
 // AnimationState implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
-void
+const gfx::IntRect
 AnimationState::UpdateState(bool aAnimationFinished,
                             RasterImage *aImage,
                             const gfx::IntSize& aSize)
 {
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(aImage),
                          RasterSurfaceKey(aSize,
                                           DefaultSurfaceFlags(),
                                           PlaybackType::eAnimated));
 
-  UpdateStateInternal(result, aAnimationFinished);
+  return UpdateStateInternal(result, aAnimationFinished, aSize);
 }
 
-void
+const gfx::IntRect
 AnimationState::UpdateStateInternal(LookupResult& aResult,
-                                    bool aAnimationFinished)
+                                    bool aAnimationFinished,
+                                    const gfx::IntSize& aSize)
 {
   // Update mDiscarded and mIsCurrentlyDecoded.
   if (aResult.Type() == MatchType::NOT_FOUND) {
     // no frames, we've either been discarded, or never been decoded before.
     mDiscarded = mHasBeenDecoded;
     mIsCurrentlyDecoded = false;
   } else if (aResult.Type() == MatchType::PENDING) {
     // no frames yet, but a decoder is or will be working on it.
@@ -68,37 +69,45 @@ AnimationState::UpdateStateInternal(Look
           aResult.Surface()->IsFinished()) {
         mIsCurrentlyDecoded = true;
       } else {
         mIsCurrentlyDecoded = false;
       }
     }
   }
 
+  gfx::IntRect ret;
+
   // Update the value of mCompositedFrameInvalid.
   if (mIsCurrentlyDecoded || aAnimationFinished) {
     // Animated images that have finished their animation (ie because it is a
     // finite length animation) don't have RequestRefresh called on them, and so
     // mCompositedFrameInvalid would never get cleared. We clear it here (and
     // also in RasterImage::Decode when we create a decoder for an image that
     // has finished animated so it can display sooner than waiting until the
     // decode completes). We also do it if we are fully decoded. This is safe
     // to do for images that aren't finished animating because before we paint
     // the refresh driver will call into us to advance to the correct frame,
     // and that will succeed because we have all the frames.
+    if (mCompositedFrameInvalid) {
+      // Invalidate if we are marking the composited frame valid.
+      ret.SizeTo(aSize);
+    }
     mCompositedFrameInvalid = false;
   } else if (aResult.Type() == MatchType::NOT_FOUND ||
              aResult.Type() == MatchType::PENDING) {
     if (mHasBeenDecoded) {
       MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
       mCompositedFrameInvalid = true;
     }
   }
   // Otherwise don't change the value of mCompositedFrameInvalid, it will be
   // updated by RequestRefresh.
+
+  return ret;
 }
 
 void
 AnimationState::NotifyDecodeComplete()
 {
   mHasBeenDecoded = true;
 }
 
@@ -367,18 +376,21 @@ FrameAnimator::RequestRefresh(AnimationS
   // must easier to reason about then trying to write code that is safe to
   // having the surface disappear at anytime.
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
                                           PlaybackType::eAnimated));
 
-  aState.UpdateStateInternal(result, aAnimationFinished);
+  ret.mDirtyRect = aState.UpdateStateInternal(result, aAnimationFinished, mSize);
   if (aState.IsDiscarded() || !result) {
+    if (!ret.mDirtyRect.IsEmpty()) {
+      ret.mFrameAdvanced = true;
+    }
     return ret;
   }
 
   // only advance the frame if the current time is greater than or
   // equal to the current frame's end time.
   Maybe<TimeStamp> currentFrameEndTime =
     GetCurrentImgFrameEndTime(aState, result.Surface());
   if (currentFrameEndTime.isNothing()) {
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -37,24 +37,26 @@ public:
     , mIsCurrentlyDecoded(false)
     , mCompositedFrameInvalid(false)
     , mDiscarded(false)
   { }
 
   /**
    * Call this whenever a decode completes, a decode starts, or the image is
    * discarded. It will update the internal state. Specifically mDiscarded,
-   * mCompositedFrameInvalid, and mIsCurrentlyDecoded.
+   * mCompositedFrameInvalid, and mIsCurrentlyDecoded. Returns a rect to
+   * invalidate.
    */
-  void UpdateState(bool aAnimationFinished,
-                   RasterImage *aImage,
-                   const gfx::IntSize& aSize);
+  const gfx::IntRect UpdateState(bool aAnimationFinished,
+                            RasterImage *aImage,
+                            const gfx::IntSize& aSize);
 private:
-  void UpdateStateInternal(LookupResult& aResult,
-                           bool aAnimationFinished);
+  const gfx::IntRect UpdateStateInternal(LookupResult& aResult,
+                                    bool aAnimationFinished,
+                                    const gfx::IntSize& aSize);
 
 public:
   /**
    * Call when a decode of this image has been completed.
    */
   void NotifyDecodeComplete();
 
   /**
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -466,17 +466,19 @@ RasterImage::OnSurfaceDiscarded(const Su
 
 void
 RasterImage::OnSurfaceDiscardedInternal(bool aAnimatedFramesDiscarded)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (aAnimatedFramesDiscarded && mAnimationState) {
     MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
-    mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+    gfx::IntRect rect =
+      mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+    NotifyProgress(NoProgress, rect);
   }
 
   if (mProgressTracker) {
     mProgressTracker->OnDiscard();
   }
 }
 
 //******************************************************************************
@@ -1079,17 +1081,19 @@ RasterImage::Discard()
   MOZ_ASSERT(CanDiscard(), "Asked to discard but can't");
   MOZ_ASSERT(!mAnimationState || gfxPrefs::ImageMemAnimatedDiscardable(),
     "Asked to discard for animated image");
 
   // Delete all the decoded frames.
   SurfaceCache::RemoveImage(ImageKey(this));
 
   if (mAnimationState) {
-    mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+    gfx::IntRect rect =
+      mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+    NotifyProgress(NoProgress, rect);
   }
 
   // Notify that we discarded.
   if (mProgressTracker) {
     mProgressTracker->OnDiscard();
   }
 }
 
@@ -1252,16 +1256,21 @@ RasterImage::Decode(const IntSize& aSize
   }
 
   // Create a decoder.
   RefPtr<IDecodingTask> task;
   if (mAnimationState && aPlaybackType == PlaybackType::eAnimated) {
     task = DecoderFactory::CreateAnimationDecoder(mDecoderType, WrapNotNull(this),
                                                   mSourceBuffer, mSize,
                                                   decoderFlags, surfaceFlags);
+    // We may not be able to send an invalidation right here because of async
+    // notifications but that's not a problem because the first frame
+    // invalidation (when it comes) will invalidate for us. So we can ignore
+    // the return value of UpdateState. This also handles the invalidation
+    // from setting the composited frame as valid below.
     mAnimationState->UpdateState(mAnimationFinished, this, mSize);
     // If the animation is finished we can draw right away because we just draw
     // the final frame all the time from now on. See comment in
     // AnimationState::UpdateState.
     if (mAnimationFinished) {
       mAnimationState->SetCompositedFrameInvalid(false);
     }
   } else {
@@ -1716,17 +1725,20 @@ RasterImage::NotifyDecodeComplete(const 
   NotifyProgress(aProgress, aInvalidRect, aFrameCount,
                  aDecoderFlags, aSurfaceFlags);
 
   if (!(aDecoderFlags & DecoderFlags::FIRST_FRAME_ONLY) &&
       mHasBeenDecoded && mAnimationState) {
     // We've finished a full decode of all animation frames and our AnimationState
     // has been notified about them all, so let it know not to expect anymore.
     mAnimationState->NotifyDecodeComplete();
-    mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+    gfx::IntRect rect = mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+    if (!rect.IsEmpty()) {
+      NotifyProgress(NoProgress, rect);
+    }
   }
 
   // Do some telemetry if this isn't a metadata decode.
   if (!aStatus.mWasMetadataDecode) {
     if (aTelemetry.mChunkCount) {
       Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, aTelemetry.mChunkCount);
     }