Bug 1388733 - Ensure animations resume when the image surfaces are discarded while still decoding. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Wed, 09 Aug 2017 22:26:55 -0400
changeset 373923 0f0344d938ce5131228c70825d1087fc3c9c0e25
parent 373922 59c5bd42ea9c77acd3a734de4c1b897bf1137822
child 373924 65a321ca7e672004006462e7f755dc686aff8994
push id32311
push userkwierso@gmail.com
push dateFri, 11 Aug 2017 01:14:57 +0000
treeherdermozilla-central@253a8560dc34 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1388733
milestone57.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 1388733 - Ensure animations resume when the image surfaces are discarded while still decoding. r=tnikkel When an animated image has been discarded, we avoided marking the composited frame invalid unless it had been previously decoded. Most of the time this was fine, but if the animated image was still decoding for the first time, then we still had a composited frame lingering that we did not mark as invalid. As a result, when we called RasterImage::LookupFrame (and indirectly FrameAnimator::GetCompositedFrame), it would always return the composited frame. This meant that RasterImage::Decode would never be called to trigger a redecode. At the same time, FrameAnimator::RequestRefresh would not cause us to advance the frame because the state was still discarded. With this patch we separate out the concepts of "has ever requested to be decoded" and "has ever completed decoding." The former is now used to control whether or not a composited frame is marked as invalid after we discover we currently have no surface for the animation -- this solves the animation remaining frozen as we now request the redecode as expected. The latter remains used to determine if we actually know the total number of frames.
image/FrameAnimator.cpp
image/FrameAnimator.h
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -52,19 +52,21 @@ AnimationState::UpdateStateInternal(Look
   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.
     mDiscarded = false;
     mIsCurrentlyDecoded = false;
+    mHasRequestedDecode = true;
   } else {
     MOZ_ASSERT(aResult.Type() == MatchType::EXACT);
     mDiscarded = false;
+    mHasRequestedDecode = true;
 
     // If mHasBeenDecoded is true then we know the true total frame count and
     // we can use it to determine if we have all the frames now so we know if
     // we are currently fully decoded.
     // If mHasBeenDecoded is false then we'll get another UpdateState call
     // when the decode finishes.
     if (mHasBeenDecoded) {
       Maybe<uint32_t> frameCount = FrameCount();
@@ -94,17 +96,17 @@ AnimationState::UpdateStateInternal(Look
       // 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) {
+      if (mHasRequestedDecode) {
         MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
         mCompositedFrameInvalid = true;
       }
     }
     // Otherwise don't change the value of mCompositedFrameInvalid, it will be
     // updated by RequestRefresh.
   }
 
@@ -204,17 +206,17 @@ Maybe<TimeStamp>
 FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState,
                                          DrawableSurface& aFrames) const
 {
   TimeStamp currentFrameTime = aState.mCurrentAnimationFrameTime;
   Maybe<FrameTimeout> timeout =
     GetTimeoutForFrame(aState, aFrames, aState.mCurrentAnimationFrameIndex);
 
   if (timeout.isNothing()) {
-    MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded());
+    MOZ_ASSERT(aState.GetHasRequestedDecode() && !aState.GetIsCurrentlyDecoded());
     return Nothing();
   }
 
   if (*timeout == FrameTimeout::Forever()) {
     // We need to return a sentinel value in this case, because our logic
     // doesn't work correctly if we have an infinitely long timeout. We use one
     // year in the future as the sentinel because it works with the loop in
     // RequestRefresh() below.
@@ -411,17 +413,17 @@ FrameAnimator::RequestRefresh(AnimationS
   }
 
   // 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()) {
     MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
-    MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded());
+    MOZ_ASSERT(aState.GetHasRequestedDecode() && !aState.GetIsCurrentlyDecoded());
     MOZ_ASSERT(aState.mCompositedFrameInvalid);
     // Nothing we can do but wait for our previous current frame to be decoded
     // again so we can determine what to do next.
     return ret;
   }
 
   while (*currentFrameEndTime <= aTime) {
     TimeStamp oldFrameEndTime = *currentFrameEndTime;
@@ -460,17 +462,17 @@ FrameAnimator::GetCompositedFrame(Animat
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
                                           PlaybackType::eAnimated));
 
   if (aState.mCompositedFrameInvalid) {
     MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
-    MOZ_ASSERT(aState.GetHasBeenDecoded());
+    MOZ_ASSERT(aState.GetHasRequestedDecode());
     MOZ_ASSERT(!aState.GetIsCurrentlyDecoded());
     if (result.Type() == MatchType::NOT_FOUND) {
       return result;
     }
     return LookupResult(MatchType::PENDING);
   }
 
   // If we have a composited version of this frame, return that.
@@ -507,17 +509,17 @@ FrameAnimator::GetTimeoutForFrame(Animat
                                   uint32_t aFrameNum) const
 {
   RawAccessFrameRef frame = GetRawFrame(aFrames, aFrameNum);
   if (frame) {
     AnimationData data = frame->GetAnimationData();
     return Some(data.mTimeout);
   }
 
-  MOZ_ASSERT(aState.mHasBeenDecoded && !aState.mIsCurrentlyDecoded);
+  MOZ_ASSERT(aState.mHasRequestedDecode && !aState.mIsCurrentlyDecoded);
   return Nothing();
 }
 
 static void
 DoCollectSizeOfCompositingSurfaces(const RawAccessFrameRef& aSurface,
                                    SurfaceMemoryCounterType aType,
                                    nsTArray<SurfaceMemoryCounter>& aCounters,
                                    MallocSizeOf aMallocSizeOf)
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -29,16 +29,17 @@ public:
   explicit AnimationState(uint16_t aAnimationMode)
     : mFrameCount(0)
     , mCurrentAnimationFrameIndex(0)
     , mLoopRemainingCount(-1)
     , mLoopCount(-1)
     , mFirstFrameTimeout(FrameTimeout::FromRawMilliseconds(0))
     , mAnimationMode(aAnimationMode)
     , mHasBeenDecoded(false)
+    , mHasRequestedDecode(false)
     , 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,
@@ -62,16 +63,21 @@ public:
   void NotifyDecodeComplete();
 
   /**
    * Returns true if this image has been fully decoded before.
    */
   bool GetHasBeenDecoded() { return mHasBeenDecoded; }
 
   /**
+   * Returns true if this image has ever requested a decode before.
+   */
+  bool GetHasRequestedDecode() { return mHasRequestedDecode; }
+
+  /**
    * Returns true if this image has been discarded and a decoded has not yet
    * been created to redecode it.
    */
   bool IsDiscarded() { return mDiscarded; }
 
   /**
    * Sets the composited frame as valid or invalid.
    */
@@ -216,16 +222,19 @@ private:
    * false when we are able to advance to the frame that should be showing
    * for the current time. mIsCurrentlyDecoded gets set to true when the
    * redecode finishes.
    */
 
   //! Whether this image has been decoded at least once.
   bool mHasBeenDecoded;
 
+  //! Whether this image has ever requested a decode.
+  bool mHasRequestedDecode;
+
   //! Whether this image is currently fully decoded.
   bool mIsCurrentlyDecoded;
 
   //! Whether the composited frame is valid to draw to the screen, note that
   //! the composited frame can exist and be filled with image data but not
   //! valid to draw to the screen.
   bool mCompositedFrameInvalid;