Bug 1462355 - Part 3. Make FrameAnimator use the new imgFrame/RawAccessFrameRef methods. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 29 May 2018 08:36:12 -0400
changeset 420216 45406c2b9e9cfa9fc2dd8cd80a35c82cf9480efc
parent 420215 3ea8add8bacc5b2b74b03ff3ba410211000c76c5
child 420217 93bdeed04a6b61139bc1c1b12088dafc260b3599
push id103751
push useraosmond@gmail.com
push dateTue, 29 May 2018 12:36:34 +0000
treeherdermozilla-inbound@9b516954e103 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1462355
milestone62.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 1462355 - Part 3. Make FrameAnimator use the new imgFrame/RawAccessFrameRef methods. r=tnikkel
image/FrameAnimator.cpp
image/FrameAnimator.h
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -317,31 +317,28 @@ FrameAnimator::AdvanceFrame(AnimationSta
     // Similar to the above case, we could be blocked by network or decoding,
     // and so we should advance our current time rather than risk jumping
     // through the animation. We will wait until the next refresh driver tick
     // and try again.
     aState.mCurrentAnimationFrameTime = aTime;
     return ret;
   }
 
-  Maybe<FrameTimeout> nextFrameTimeout = GetTimeoutForFrame(aState, aFrames, nextFrameIndex);
-  // GetTimeoutForFrame can only return none if frame doesn't exist,
-  // but we just got it above.
-  MOZ_ASSERT(nextFrameTimeout.isSome());
-  if (*nextFrameTimeout == FrameTimeout::Forever()) {
+  if (nextFrame->GetTimeout() == FrameTimeout::Forever()) {
     ret.mAnimationFinished = true;
   }
 
   if (nextFrameIndex == 0) {
     ret.mDirtyRect = aState.FirstFrameRefreshArea();
   } else {
     MOZ_ASSERT(nextFrameIndex == currentFrameIndex + 1);
+    RawAccessFrameRef currentFrame = GetRawFrame(aFrames, currentFrameIndex);
 
     // Change frame
-    if (!DoBlend(aFrames, &ret.mDirtyRect, currentFrameIndex, nextFrameIndex)) {
+    if (!DoBlend(currentFrame, nextFrame, nextFrameIndex, &ret.mDirtyRect)) {
       // something went wrong, move on to next
       NS_WARNING("FrameAnimator::AdvanceFrame(): Compositing of frame failed");
       nextFrame->SetCompositingFailed(true);
       Maybe<TimeStamp> currentFrameEndTime = GetCurrentImgFrameEndTime(aState, aFrames);
       MOZ_ASSERT(currentFrameEndTime.isSome());
       aState.mCurrentAnimationFrameTime = *currentFrameEndTime;
       aState.mCurrentAnimationFrameIndex = nextFrameIndex;
       aState.mCompositedFrameRequested = false;
@@ -555,18 +552,17 @@ FrameAnimator::GetCompositedFrame(Animat
 
 Maybe<FrameTimeout>
 FrameAnimator::GetTimeoutForFrame(AnimationState& aState,
                                   DrawableSurface& aFrames,
                                   uint32_t aFrameNum) const
 {
   RawAccessFrameRef frame = GetRawFrame(aFrames, aFrameNum);
   if (frame) {
-    AnimationData data = frame->GetAnimationData();
-    return Some(data.mTimeout);
+    return Some(frame->GetTimeout());
   }
 
   MOZ_ASSERT(aState.mHasRequestedDecode && !aState.mIsCurrentlyDecoded);
   return Nothing();
 }
 
 static void
 DoCollectSizeOfCompositingSurfaces(const RawAccessFrameRef& aSurface,
@@ -627,90 +623,83 @@ FrameAnimator::GetRawFrame(DrawableSurfa
 
   return aFrames->RawAccessRef();
 }
 
 //******************************************************************************
 // DoBlend gets called when the timer for animation get fired and we have to
 // update the composited frame of the animation.
 bool
-FrameAnimator::DoBlend(DrawableSurface& aFrames,
-                       IntRect* aDirtyRect,
-                       uint32_t aPrevFrameIndex,
-                       uint32_t aNextFrameIndex)
+FrameAnimator::DoBlend(const RawAccessFrameRef& aPrevFrame,
+                       const RawAccessFrameRef& aNextFrame,
+                       uint32_t aNextFrameIndex,
+                       IntRect* aDirtyRect)
 {
-  RawAccessFrameRef prevFrame = GetRawFrame(aFrames, aPrevFrameIndex);
-  RawAccessFrameRef nextFrame = GetRawFrame(aFrames, aNextFrameIndex);
-
-  MOZ_ASSERT(prevFrame && nextFrame, "Should have frames here");
+  MOZ_ASSERT(aPrevFrame && aNextFrame, "Should have frames here");
 
-  AnimationData prevFrameData = prevFrame->GetAnimationData();
-  if (prevFrameData.mDisposalMethod == DisposalMethod::RESTORE_PREVIOUS &&
+  DisposalMethod prevDisposalMethod = aPrevFrame->GetDisposalMethod();
+  bool prevHasAlpha = aPrevFrame->FormatHasAlpha();
+  if (prevDisposalMethod == DisposalMethod::RESTORE_PREVIOUS &&
       !mCompositingPrevFrame) {
-    prevFrameData.mDisposalMethod = DisposalMethod::CLEAR;
+    prevDisposalMethod = DisposalMethod::CLEAR;
   }
 
-  IntRect prevRect = prevFrameData.mBlendRect
-                   ? prevFrameData.mRect.Intersect(*prevFrameData.mBlendRect)
-                   : prevFrameData.mRect;
-
+  IntRect prevRect = aPrevFrame->GetBoundedBlendRect();
   bool isFullPrevFrame = prevRect.IsEqualRect(0, 0, mSize.width, mSize.height);
 
   // Optimization: DisposeClearAll if the previous frame is the same size as
   //               container and it's clearing itself
   if (isFullPrevFrame &&
-      (prevFrameData.mDisposalMethod == DisposalMethod::CLEAR)) {
-    prevFrameData.mDisposalMethod = DisposalMethod::CLEAR_ALL;
+      (prevDisposalMethod == DisposalMethod::CLEAR)) {
+    prevDisposalMethod = DisposalMethod::CLEAR_ALL;
   }
 
-  AnimationData nextFrameData = nextFrame->GetAnimationData();
+  DisposalMethod nextDisposalMethod = aNextFrame->GetDisposalMethod();
+  bool nextHasAlpha = aNextFrame->FormatHasAlpha();
 
-  IntRect nextRect = nextFrameData.mBlendRect
-                   ? nextFrameData.mRect.Intersect(*nextFrameData.mBlendRect)
-                   : nextFrameData.mRect;
-
+  IntRect nextRect = aNextFrame->GetBoundedBlendRect();
   bool isFullNextFrame = nextRect.IsEqualRect(0, 0, mSize.width, mSize.height);
 
-  if (!nextFrame->GetIsPaletted()) {
+  if (!aNextFrame->GetIsPaletted()) {
     // Optimization: Skip compositing if the previous frame wants to clear the
     //               whole image
-    if (prevFrameData.mDisposalMethod == DisposalMethod::CLEAR_ALL) {
+    if (prevDisposalMethod == DisposalMethod::CLEAR_ALL) {
       aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
       return true;
     }
 
     // Optimization: Skip compositing if this frame is the same size as the
     //               container and it's fully drawing over prev frame (no alpha)
     if (isFullNextFrame &&
-        (nextFrameData.mDisposalMethod != DisposalMethod::RESTORE_PREVIOUS) &&
-        !nextFrameData.mHasAlpha) {
+        (nextDisposalMethod != DisposalMethod::RESTORE_PREVIOUS) &&
+        !nextHasAlpha) {
       aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
       return true;
     }
   }
 
   // Calculate area that needs updating
-  switch (prevFrameData.mDisposalMethod) {
+  switch (prevDisposalMethod) {
     default:
       MOZ_FALLTHROUGH_ASSERT("Unexpected DisposalMethod");
     case DisposalMethod::NOT_SPECIFIED:
     case DisposalMethod::KEEP:
       *aDirtyRect = nextRect;
       break;
 
     case DisposalMethod::CLEAR_ALL:
       // Whole image container is cleared
       aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
       break;
 
     case DisposalMethod::CLEAR:
       // Calc area that needs to be redrawn (the combination of previous and
       // this frame)
       // XXX - This could be done with multiple framechanged calls
-      //       Having prevFrame way at the top of the image, and nextFrame
+      //       Having aPrevFrame way at the top of the image, and aNextFrame
       //       way at the bottom, and both frames being small, we'd be
       //       telling framechanged to refresh the whole image when only two
       //       small areas are needed.
       aDirtyRect->UnionRect(nextRect, prevRect);
       break;
 
     case DisposalMethod::RESTORE_PREVIOUS:
       aDirtyRect->SetRect(0, 0, mSize.width, mSize.height);
@@ -742,27 +731,24 @@ FrameAnimator::DoBlend(DrawableSurface& 
     needToBlankComposite = true;
   } else if (int32_t(aNextFrameIndex) != mLastCompositedFrameIndex+1) {
 
     // If we are not drawing on top of last composited frame,
     // then we are building a new composite frame, so let's clear it first.
     needToBlankComposite = true;
   }
 
-  AnimationData compositingFrameData = mCompositingFrame->GetAnimationData();
-
   // More optimizations possible when next frame is not transparent
   // But if the next frame has DisposalMethod::RESTORE_PREVIOUS,
   // this "no disposal" optimization is not possible,
   // because the frame in "after disposal operation" state
   // needs to be stored in compositingFrame, so it can be
   // copied into compositingPrevFrame later.
   bool doDisposal = true;
-  if (!nextFrameData.mHasAlpha &&
-      nextFrameData.mDisposalMethod != DisposalMethod::RESTORE_PREVIOUS) {
+  if (!nextHasAlpha && nextDisposalMethod != DisposalMethod::RESTORE_PREVIOUS) {
     if (isFullNextFrame) {
       // Optimization: No need to dispose prev.frame when
       // next frame is full frame and not transparent.
       doDisposal = false;
       // No need to blank the composite frame
       needToBlankComposite = false;
     } else {
       if ((prevRect.X() >= nextRect.X()) && (prevRect.Y() >= nextRect.Y()) &&
@@ -772,141 +758,134 @@ FrameAnimator::DoBlend(DrawableSurface& 
         // next frame fully overlaps previous frame.
         doDisposal = false;
       }
     }
   }
 
   if (doDisposal) {
     // Dispose of previous: clear, restore, or keep (copy)
-    switch (prevFrameData.mDisposalMethod) {
+    switch (prevDisposalMethod) {
       case DisposalMethod::CLEAR:
         if (needToBlankComposite) {
           // If we just created the composite, it could have anything in its
           // buffer. Clear whole frame
-          ClearFrame(compositingFrameData.mRawData,
-                     compositingFrameData.mRect);
+          ClearFrame(mCompositingFrame.Data(),
+                     mCompositingFrame->GetRect());
         } else {
           // Only blank out previous frame area (both color & Mask/Alpha)
-          ClearFrame(compositingFrameData.mRawData,
-                     compositingFrameData.mRect,
+          ClearFrame(mCompositingFrame.Data(),
+                     mCompositingFrame->GetRect(),
                      prevRect);
         }
         break;
 
       case DisposalMethod::CLEAR_ALL:
-        ClearFrame(compositingFrameData.mRawData,
-                   compositingFrameData.mRect);
+        ClearFrame(mCompositingFrame.Data(),
+                   mCompositingFrame->GetRect());
         break;
 
       case DisposalMethod::RESTORE_PREVIOUS:
         // It would be better to copy only the area changed back to
         // compositingFrame.
         if (mCompositingPrevFrame) {
-          AnimationData compositingPrevFrameData =
-            mCompositingPrevFrame->GetAnimationData();
-
-          CopyFrameImage(compositingPrevFrameData.mRawData,
-                         compositingPrevFrameData.mRect,
-                         compositingFrameData.mRawData,
-                         compositingFrameData.mRect);
+          CopyFrameImage(mCompositingPrevFrame.Data(),
+                         mCompositingPrevFrame->GetRect(),
+                         mCompositingFrame.Data(),
+                         mCompositingFrame->GetRect());
 
           // destroy only if we don't need it for this frame's disposal
-          if (nextFrameData.mDisposalMethod !=
-              DisposalMethod::RESTORE_PREVIOUS) {
+          if (nextDisposalMethod != DisposalMethod::RESTORE_PREVIOUS) {
             mCompositingPrevFrame.reset();
           }
         } else {
-          ClearFrame(compositingFrameData.mRawData,
-                     compositingFrameData.mRect);
+          ClearFrame(mCompositingFrame.Data(),
+                     mCompositingFrame->GetRect());
         }
         break;
 
       default:
         MOZ_FALLTHROUGH_ASSERT("Unexpected DisposalMethod");
       case DisposalMethod::NOT_SPECIFIED:
       case DisposalMethod::KEEP:
         // Copy previous frame into compositingFrame before we put the new
         // frame on top
         // Assumes that the previous frame represents a full frame (it could be
         // smaller in size than the container, as long as the frame before it
         // erased itself)
         // Note: Frame 1 never gets into DoBlend(), so (aNextFrameIndex - 1)
         // will always be a valid frame number.
         if (mLastCompositedFrameIndex != int32_t(aNextFrameIndex - 1)) {
-          if (isFullPrevFrame && !prevFrame->GetIsPaletted()) {
+          if (isFullPrevFrame && !aPrevFrame->GetIsPaletted()) {
             // Just copy the bits
-            CopyFrameImage(prevFrameData.mRawData,
+            CopyFrameImage(aPrevFrame.Data(),
                            prevRect,
-                           compositingFrameData.mRawData,
-                           compositingFrameData.mRect);
+                           mCompositingFrame.Data(),
+                           mCompositingFrame->GetRect());
           } else {
             if (needToBlankComposite) {
               // Only blank composite when prev is transparent or not full.
-              if (prevFrameData.mHasAlpha || !isFullPrevFrame) {
-                ClearFrame(compositingFrameData.mRawData,
-                           compositingFrameData.mRect);
+              if (prevHasAlpha || !isFullPrevFrame) {
+                ClearFrame(mCompositingFrame.Data(),
+                           mCompositingFrame->GetRect());
               }
             }
-            DrawFrameTo(prevFrameData.mRawData, prevFrameData.mRect,
-                        prevFrameData.mPaletteDataLength,
-                        prevFrameData.mHasAlpha,
-                        compositingFrameData.mRawData,
-                        compositingFrameData.mRect,
-                        prevFrameData.mBlendMethod,
-                        prevFrameData.mBlendRect);
+            DrawFrameTo(aPrevFrame.Data(), aPrevFrame->GetRect(),
+                        aPrevFrame.PaletteDataLength(),
+                        prevHasAlpha,
+                        mCompositingFrame.Data(),
+                        mCompositingFrame->GetRect(),
+                        aPrevFrame->GetBlendMethod(),
+                        aPrevFrame->GetBlendRect());
           }
         }
     }
   } else if (needToBlankComposite) {
     // If we just created the composite, it could have anything in its
     // buffers. Clear them
-    ClearFrame(compositingFrameData.mRawData,
-               compositingFrameData.mRect);
+    ClearFrame(mCompositingFrame.Data(),
+               mCompositingFrame->GetRect());
   }
 
   // Check if the frame we are composing wants the previous image restored after
   // it is done. Don't store it (again) if last frame wanted its image restored
   // too
-  if ((nextFrameData.mDisposalMethod == DisposalMethod::RESTORE_PREVIOUS) &&
-      (prevFrameData.mDisposalMethod != DisposalMethod::RESTORE_PREVIOUS)) {
+  if ((nextDisposalMethod == DisposalMethod::RESTORE_PREVIOUS) &&
+      (prevDisposalMethod != DisposalMethod::RESTORE_PREVIOUS)) {
     // We are storing the whole image.
-    // It would be better if we just stored the area that nextFrame is going to
+    // It would be better if we just stored the area that aNextFrame is going to
     // overwrite.
     if (!mCompositingPrevFrame) {
       RefPtr<imgFrame> newFrame = new imgFrame;
       nsresult rv = newFrame->InitForAnimator(mSize,
                                               SurfaceFormat::B8G8R8A8);
       if (NS_FAILED(rv)) {
         mCompositingPrevFrame.reset();
         return false;
       }
 
       mCompositingPrevFrame = newFrame->RawAccessRef();
     }
 
-    AnimationData compositingPrevFrameData =
-      mCompositingPrevFrame->GetAnimationData();
-
-    CopyFrameImage(compositingFrameData.mRawData,
-                   compositingFrameData.mRect,
-                   compositingPrevFrameData.mRawData,
-                   compositingPrevFrameData.mRect);
+    CopyFrameImage(mCompositingFrame.Data(),
+                   mCompositingFrame->GetRect(),
+                   mCompositingPrevFrame.Data(),
+                   mCompositingPrevFrame->GetRect());
 
     mCompositingPrevFrame->Finish();
   }
 
   // blit next frame into it's correct spot
-  DrawFrameTo(nextFrameData.mRawData, nextFrameData.mRect,
-              nextFrameData.mPaletteDataLength,
-              nextFrameData.mHasAlpha,
-              compositingFrameData.mRawData,
-              compositingFrameData.mRect,
-              nextFrameData.mBlendMethod,
-              nextFrameData.mBlendRect);
+  DrawFrameTo(aNextFrame.Data(), aNextFrame->GetRect(),
+              aNextFrame.PaletteDataLength(),
+              nextHasAlpha,
+              mCompositingFrame.Data(),
+              mCompositingFrame->GetRect(),
+              aNextFrame->GetBlendMethod(),
+              aNextFrame->GetBlendRect());
 
   // Tell the image that it is fully 'downloaded'.
   mCompositingFrame->Finish();
 
   mLastCompositedFrameIndex = int32_t(aNextFrameIndex);
 
   return true;
 }
@@ -965,17 +944,17 @@ FrameAnimator::CopyFrameImage(const uint
 
   return true;
 }
 
 nsresult
 FrameAnimator::DrawFrameTo(const uint8_t* aSrcData, const IntRect& aSrcRect,
                            uint32_t aSrcPaletteLength, bool aSrcHasAlpha,
                            uint8_t* aDstPixels, const IntRect& aDstRect,
-                           BlendMethod aBlendMethod, const Maybe<IntRect>& aBlendRect)
+                           BlendMethod aBlendMethod, const IntRect& aBlendRect)
 {
   NS_ENSURE_ARG_POINTER(aSrcData);
   NS_ENSURE_ARG_POINTER(aDstPixels);
 
   // According to both AGIF and APNG specs, offsets are unsigned
   if (aSrcRect.X() < 0 || aSrcRect.Y() < 0) {
     NS_WARNING("FrameAnimator::DrawFrameTo: negative offsets not allowed");
     return NS_ERROR_FAILURE;
@@ -1062,18 +1041,18 @@ FrameAnimator::DrawFrameTo(const uint8_t
     // with BlendMethod::OVER, and then recopying the area that uses
     // BlendMethod::SOURCE if needed. To make this work, the decoder has to
     // provide a "blend rect" that tells us where to do this. This is just the
     // frame rect, but hidden in a way that makes it invisible to most of the
     // system, so we can keep eliminating dependencies on it.
     auto op = aBlendMethod == BlendMethod::SOURCE ? PIXMAN_OP_SRC
                                                   : PIXMAN_OP_OVER;
 
-    if (aBlendMethod == BlendMethod::OVER || !aBlendRect ||
-        (aBlendMethod == BlendMethod::SOURCE && aSrcRect.IsEqualEdges(*aBlendRect))) {
+    if (aBlendMethod == BlendMethod::OVER ||
+        (aBlendMethod == BlendMethod::SOURCE && aSrcRect.IsEqualEdges(aBlendRect))) {
       // We don't need to do anything clever. (Or, in the case where no blend
       // rect was specified, we can't.)
       pixman_image_composite32(op,
                                src,
                                nullptr,
                                dst,
                                0, 0,
                                0, 0,
@@ -1088,20 +1067,20 @@ FrameAnimator::DrawFrameTo(const uint8_t
                                0, 0,
                                0, 0,
                                aSrcRect.X(), aSrcRect.Y(),
                                aSrcRect.Width(), aSrcRect.Height());
       pixman_image_composite32(PIXMAN_OP_SRC,
                                src,
                                nullptr,
                                dst,
-                               aBlendRect->X(), aBlendRect->Y(),
+                               aBlendRect.X(), aBlendRect.Y(),
                                0, 0,
-                               aBlendRect->X(), aBlendRect->Y(),
-                               aBlendRect->Width(), aBlendRect->Height());
+                               aBlendRect.X(), aBlendRect.Y(),
+                               aBlendRect.Width(), aBlendRect.Height());
     }
 
     pixman_image_unref(src);
     pixman_image_unref(dst);
   }
 
   return NS_OK;
 }
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -365,20 +365,20 @@ private: // methods
    * 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().
    */
   Maybe<TimeStamp> GetCurrentImgFrameEndTime(AnimationState& aState,
                                              DrawableSurface& aFrames) const;
 
-  bool DoBlend(DrawableSurface& aFrames,
-               gfx::IntRect* aDirtyRect,
-               uint32_t aPrevFrameIndex,
-               uint32_t aNextFrameIndex);
+  bool DoBlend(const RawAccessFrameRef& aPrevFrame,
+               const RawAccessFrameRef& aNextFrame,
+               uint32_t aNextFrameIndex,
+               gfx::IntRect* aDirtyRect);
 
   /** Clears an area of <aFrame> with transparent black.
    *
    * @param aFrameData Target Frame data
    * @param aFrameRect The rectangle of the data pointed ot by aFrameData
    *
    * @note Does also clears the transparency mask
    */
@@ -408,17 +408,17 @@ private: // methods
    * @aBlendMethod the blend method for how to blend src on the composition
    * frame.
    */
   static nsresult DrawFrameTo(const uint8_t* aSrcData,
                               const gfx::IntRect& aSrcRect,
                               uint32_t aSrcPaletteLength, bool aSrcHasAlpha,
                               uint8_t* aDstPixels, const gfx::IntRect& aDstRect,
                               BlendMethod aBlendMethod,
-                              const Maybe<gfx::IntRect>& aBlendRect);
+                              const gfx::IntRect& aBlendRect);
 
 private: // data
   //! A weak pointer to our owning image.
   RasterImage* mImage;
 
   //! The intrinsic size of the image.
   gfx::IntSize mSize;