Bug 1465619 - Part 1. Use imgFrame directly instead of RawAccessFrameRef in FrameAnimator. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Sun, 03 Jun 2018 18:49:23 -0400
changeset 490703 b2a31a31fba6da90949b485e1728ef2dee40450e
parent 490702 8c1121739072bf560b00acd482745de2c952b33a
child 490704 847e8234669346a34af39546b1e64c93f8a8e882
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerstnikkel
bugs1465619
milestone65.0a1
Bug 1465619 - Part 1. Use imgFrame directly instead of RawAccessFrameRef in FrameAnimator. r=tnikkel When blending full frames off the main thread, FrameAnimator no longer requires access to the raw data of the frame to advance the animation. Now we only request a RawAccessFrameRef for the current/next frames when we have discovered that we need to do blending on the main thread. In addition to avoiding the mutex overhead of RawAccessFrameRef, this will also facilitate potentially optimizing the surfaces for the DrawTarget for individual animated image frames. Differential Revision: https://phabricator.services.mozilla.com/D7506
image/AnimationSurfaceProvider.cpp
image/AnimationSurfaceProvider.h
image/FrameAnimator.cpp
image/FrameAnimator.h
image/ISurfaceProvider.h
image/test/gtest/TestDecoders.cpp
image/test/gtest/TestMetadata.cpp
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -137,32 +137,29 @@ AnimationSurfaceProvider::DrawableRef(si
   imgFrame* frame = mFrames.Get(aFrame);
   if (!frame) {
     return DrawableFrameRef();
   }
 
   return frame->DrawableRef();
 }
 
-RawAccessFrameRef
-AnimationSurfaceProvider::RawAccessRef(size_t aFrame)
+already_AddRefed<imgFrame>
+AnimationSurfaceProvider::GetFrame(size_t aFrame)
 {
   MutexAutoLock lock(mFramesMutex);
 
   if (Availability().IsPlaceholder()) {
-    MOZ_ASSERT_UNREACHABLE("Calling RawAccessRef() on a placeholder");
+    MOZ_ASSERT_UNREACHABLE("Calling GetFrame() on a placeholder");
     return RawAccessFrameRef();
   }
 
-  imgFrame* frame = mFrames.Get(aFrame);
-  if (!frame) {
-    return RawAccessFrameRef();
-  }
-
-  return frame->RawAccessRef(/* aOnlyFinished */ true);
+  RefPtr<imgFrame> frame = mFrames.Get(aFrame);
+  MOZ_ASSERT_IF(frame, frame->IsFinished());
+  return frame.forget();
 }
 
 bool
 AnimationSurfaceProvider::IsFinished() const
 {
   MutexAutoLock lock(mFramesMutex);
 
   if (Availability().IsPlaceholder()) {
--- a/image/AnimationSurfaceProvider.h
+++ b/image/AnimationSurfaceProvider.h
@@ -50,17 +50,17 @@ public:
   size_t LogicalSizeInBytes() const override;
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                               const AddSizeOfCb& aCallback) override;
   void Reset() override;
   void Advance(size_t aFrame) override;
 
 protected:
   DrawableFrameRef DrawableRef(size_t aFrame) override;
-  RawAccessFrameRef RawAccessRef(size_t aFrame) override;
+  already_AddRefed<imgFrame> GetFrame(size_t aFrame) override;
 
   // Animation frames are always locked. This is because we only want to release
   // their memory atomically (due to the surface cache discarding them). If they
   // were unlocked, the OS could end up releasing the memory of random frames
   // from the middle of the animation, which is not worth the complexity of
   // dealing with.
   bool IsLocked() const override { return true; }
   void SetLocked(bool) override { }
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -231,17 +231,17 @@ FrameAnimator::GetCurrentImgFrameEndTime
   TimeDuration durationOfTimeout =
     TimeDuration::FromMilliseconds(double(aCurrentTimeout.AsMilliseconds()));
   return aState.mCurrentAnimationFrameTime + durationOfTimeout;
 }
 
 RefreshResult
 FrameAnimator::AdvanceFrame(AnimationState& aState,
                             DrawableSurface& aFrames,
-                            RawAccessFrameRef& aCurrentFrame,
+                            RefPtr<imgFrame>& aCurrentFrame,
                             TimeStamp aTime)
 {
   AUTO_PROFILER_LABEL("FrameAnimator::AdvanceFrame", GRAPHICS);
 
   RefreshResult ret;
 
   // Determine what the next frame is, taking into account looping.
   uint32_t currentFrameIndex = aState.mCurrentAnimationFrameIndex;
@@ -290,17 +290,17 @@ FrameAnimator::AdvanceFrame(AnimationSta
   }
 
   // There can be frames in the surface cache with index >= KnownFrameCount()
   // which GetRawFrame() can access because an async decoder has decoded them,
   // but which AnimationState doesn't know about yet because we haven't received
   // the appropriate notification on the main thread. Make sure we stay in sync
   // with AnimationState.
   MOZ_ASSERT(nextFrameIndex < aState.KnownFrameCount());
-  RawAccessFrameRef nextFrame = aFrames.RawAccessRef(nextFrameIndex);
+  RefPtr<imgFrame> nextFrame = aFrames.GetFrame(nextFrameIndex);
 
   // We should always check to see if we have the next frame even if we have
   // previously finished decoding. If we needed to redecode (e.g. due to a draw
   // failure) we would have discarded all the old frames and may not yet have
   // the new ones. DrawableSurface::RawAccessRef promises to only return
   // finished frames.
   if (!nextFrame) {
     // Uh oh, the frame we want to show is currently being decoded (partial).
@@ -316,18 +316,23 @@ FrameAnimator::AdvanceFrame(AnimationSta
     ret.mAnimationFinished = true;
   }
 
   if (nextFrameIndex == 0) {
     MOZ_ASSERT(nextFrame->IsFullFrame());
     ret.mDirtyRect = aState.FirstFrameRefreshArea();
   } else if (!nextFrame->IsFullFrame()) {
     MOZ_ASSERT(nextFrameIndex == currentFrameIndex + 1);
+    RawAccessFrameRef currentRef =
+      aCurrentFrame->RawAccessRef(/* aFinished */ true);
+    RawAccessFrameRef nextRef =
+      nextFrame->RawAccessRef(/* aFinished */ true);
+
     // Change frame
-    if (!DoBlend(aCurrentFrame, nextFrame, nextFrameIndex, &ret.mDirtyRect)) {
+    if (!DoBlend(currentRef, nextRef, nextFrameIndex, &ret.mDirtyRect)) {
       // something went wrong, move on to next
       NS_WARNING("FrameAnimator::AdvanceFrame(): Compositing of frame failed");
       nextFrame->SetCompositingFailed(true);
       aState.mCurrentAnimationFrameTime =
         GetCurrentImgFrameEndTime(aState, aCurrentFrame->GetTimeout());
       aState.mCurrentAnimationFrameIndex = nextFrameIndex;
       aState.mCompositedFrameRequested = false;
       aCurrentFrame = std::move(nextFrame);
@@ -436,18 +441,18 @@ FrameAnimator::RequestRefresh(AnimationS
   if (aState.IsDiscarded() || !result) {
     aState.MaybeAdvanceAnimationFrameTime(aTime);
     if (!ret.mDirtyRect.IsEmpty()) {
       ret.mFrameAdvanced = true;
     }
     return ret;
   }
 
-  RawAccessFrameRef currentFrame =
-    result.Surface().RawAccessRef(aState.mCurrentAnimationFrameIndex);
+  RefPtr<imgFrame> currentFrame =
+    result.Surface().GetFrame(aState.mCurrentAnimationFrameIndex);
 
   // only advance the frame if the current time is greater than or
   // equal to the current frame's end time.
   if (!currentFrame) {
     MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
     MOZ_ASSERT(aState.GetHasRequestedDecode() && !aState.GetIsCurrentlyDecoded());
     MOZ_ASSERT(aState.mCompositedFrameInvalid);
     // Nothing we can do but wait for our previous current frame to be decoded
@@ -609,17 +614,20 @@ FrameAnimator::CollectSizeOfCompositingS
 // DoBlend gets called when the timer for animation get fired and we have to
 // update the composited frame of the animation.
 bool
 FrameAnimator::DoBlend(const RawAccessFrameRef& aPrevFrame,
                        const RawAccessFrameRef& aNextFrame,
                        uint32_t aNextFrameIndex,
                        IntRect* aDirtyRect)
 {
-  MOZ_ASSERT(aPrevFrame && aNextFrame, "Should have frames here");
+  if (!aPrevFrame || !aNextFrame) {
+    MOZ_ASSERT_UNREACHABLE("Should have RawAccessFrameRefs to blend!");
+    return false;
+  }
 
   DisposalMethod prevDisposalMethod = aPrevFrame->GetDisposalMethod();
   bool prevHasAlpha = aPrevFrame->FormatHasAlpha();
   if (prevDisposalMethod == DisposalMethod::RESTORE_PREVIOUS &&
       !mCompositingPrevFrame) {
     prevDisposalMethod = DisposalMethod::CLEAR;
   }
 
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -348,17 +348,17 @@ private: // methods
    *                      we advance, it will replace aCurrentFrame with the
    *                      new current frame we advanced to.
    *
    * @returns a RefreshResult that shows whether the frame was successfully
    *          advanced, and its resulting dirty rect.
    */
   RefreshResult AdvanceFrame(AnimationState& aState,
                              DrawableSurface& aFrames,
-                             RawAccessFrameRef& aCurrentFrame,
+                             RefPtr<imgFrame>& aCurrentFrame,
                              TimeStamp aTime);
 
   /**
    * Get the time the frame we're currently displaying is supposed to end.
    *
    * In the error case (like if the requested frame is not currently
    * decoded), returns None().
    */
--- a/image/ISurfaceProvider.h
+++ b/image/ISurfaceProvider.h
@@ -105,23 +105,22 @@ protected:
 
   virtual ~ISurfaceProvider() { }
 
   /// @return an eagerly computed drawable reference to a surface. For
   /// dynamically generated animation surfaces, @aFrame specifies the 0-based
   /// index of the desired frame.
   virtual DrawableFrameRef DrawableRef(size_t aFrame) = 0;
 
-  /// @return an eagerly computed raw access reference to a surface. For
-  /// dynamically generated animation surfaces, @aFrame specifies the 0-based
-  /// index of the desired frame.
-  virtual RawAccessFrameRef RawAccessRef(size_t aFrame)
+  /// @return an imgFrame at the 0-based index of the desired frame, as
+  /// specified by @aFrame. Only applies for animated images.
+  virtual already_AddRefed<imgFrame> GetFrame(size_t aFrame)
   {
-    MOZ_ASSERT_UNREACHABLE("Surface provider does not support raw access!");
-    return RawAccessFrameRef();
+    MOZ_ASSERT_UNREACHABLE("Surface provider does not support direct access!");
+    return nullptr;
   }
 
   /// @return true if this ISurfaceProvider is locked. (@see SetLocked())
   /// Should only be called from SurfaceCache code as it relies on SurfaceCache
   /// for synchronization.
   virtual bool IsLocked() const = 0;
 
   /// If @aLocked is true, hint that this ISurfaceProvider is in use and it
@@ -203,26 +202,26 @@ public:
       return NS_ERROR_FAILURE;
     }
 
     mDrawableRef = mProvider->DrawableRef(aFrame);
 
     return mDrawableRef ? NS_OK : NS_ERROR_FAILURE;
   }
 
-  RawAccessFrameRef RawAccessRef(size_t aFrame)
+  already_AddRefed<imgFrame> GetFrame(size_t aFrame)
   {
     MOZ_ASSERT(mHaveSurface, "Trying to get on an empty DrawableSurface?");
 
     if (!mProvider) {
       MOZ_ASSERT_UNREACHABLE("Trying to get on a static DrawableSurface?");
-      return RawAccessFrameRef();
+      return nullptr;
     }
 
-    return mProvider->RawAccessRef(aFrame);
+    return mProvider->GetFrame(aFrame);
   }
 
   void Reset()
   {
     if (!mProvider) {
       MOZ_ASSERT_UNREACHABLE("Trying to reset a static DrawableSurface?");
       return;
     }
--- a/image/test/gtest/TestDecoders.cpp
+++ b/image/test/gtest/TestDecoders.cpp
@@ -604,17 +604,17 @@ TEST_F(ImageDecoders, AnimatedGIFWithFRA
                                             DefaultSurfaceFlags(),
                                             PlaybackType::eAnimated),
                            /* aMarkUsed = */ true);
     ASSERT_EQ(MatchType::EXACT, result.Type());
 
     EXPECT_TRUE(NS_SUCCEEDED(result.Surface().Seek(0)));
     EXPECT_TRUE(bool(result.Surface()));
 
-    RawAccessFrameRef partialFrame = result.Surface().RawAccessRef(1);
+    RefPtr<imgFrame> partialFrame = result.Surface().GetFrame(1);
     EXPECT_TRUE(bool(partialFrame));
   }
 
   // Ensure that the static version is still around.
   {
     LookupResult result =
       SurfaceCache::Lookup(ImageKey(image.get()),
                            RasterSurfaceKey(imageSize,
@@ -692,17 +692,17 @@ TEST_F(ImageDecoders, AnimatedGIFWithFRA
                                             DefaultSurfaceFlags(),
                                             PlaybackType::eAnimated),
                            /* aMarkUsed = */ true);
     ASSERT_EQ(MatchType::EXACT, result.Type());
 
     EXPECT_TRUE(NS_SUCCEEDED(result.Surface().Seek(0)));
     EXPECT_TRUE(bool(result.Surface()));
 
-    RawAccessFrameRef partialFrame = result.Surface().RawAccessRef(1);
+    RefPtr<imgFrame> partialFrame = result.Surface().GetFrame(1);
     EXPECT_TRUE(bool(partialFrame));
   }
 
   // Ensure that we didn't decode the static version of the image.
   {
     LookupResult result =
       SurfaceCache::Lookup(ImageKey(image.get()),
                            RasterSurfaceKey(imageSize,
@@ -738,17 +738,17 @@ TEST_F(ImageDecoders, AnimatedGIFWithFRA
                                             DefaultSurfaceFlags(),
                                             PlaybackType::eAnimated),
                            /* aMarkUsed = */ true);
     ASSERT_EQ(MatchType::EXACT, result.Type());
 
     EXPECT_TRUE(NS_SUCCEEDED(result.Surface().Seek(0)));
     EXPECT_TRUE(bool(result.Surface()));
 
-    RawAccessFrameRef partialFrame = result.Surface().RawAccessRef(1);
+    RefPtr<imgFrame> partialFrame = result.Surface().GetFrame(1);
     EXPECT_TRUE(bool(partialFrame));
   }
 }
 
 TEST_F(ImageDecoders, AnimatedGIFWithExtraImageSubBlocks)
 {
   ImageTestCase testCase = ExtraImageSubBlocksAnimatedGIFTestCase();
 
@@ -808,17 +808,17 @@ TEST_F(ImageDecoders, AnimatedGIFWithExt
                                           DefaultSurfaceFlags(),
                                           PlaybackType::eAnimated),
                          /* aMarkUsed = */ true);
   ASSERT_EQ(MatchType::EXACT, result.Type());
 
   EXPECT_TRUE(NS_SUCCEEDED(result.Surface().Seek(0)));
   EXPECT_TRUE(bool(result.Surface()));
 
-  RawAccessFrameRef partialFrame = result.Surface().RawAccessRef(1);
+  RefPtr<imgFrame> partialFrame = result.Surface().GetFrame(1);
   EXPECT_TRUE(bool(partialFrame));
 }
 
 TEST_F(ImageDecoders, TruncatedSmallGIFSingleChunk)
 {
   CheckDecoderSingleChunk(TruncatedSmallGIFTestCase());
 }
 
--- a/image/test/gtest/TestMetadata.cpp
+++ b/image/test/gtest/TestMetadata.cpp
@@ -252,11 +252,11 @@ TEST_F(ImageDecoderMetadata, NoFrameDela
                                           DefaultSurfaceFlags(),
                                           PlaybackType::eAnimated),
                          /* aMarkUsed = */ true);
   ASSERT_EQ(MatchType::EXACT, result.Type());
 
   EXPECT_TRUE(NS_SUCCEEDED(result.Surface().Seek(0)));
   EXPECT_TRUE(bool(result.Surface()));
 
-  RawAccessFrameRef partialFrame = result.Surface().RawAccessRef(1);
+  RefPtr<imgFrame> partialFrame = result.Surface().GetFrame(1);
   EXPECT_TRUE(bool(partialFrame));
 }