Bug 1293472 (Part 1) - Make it possible to seek DecodedSurfaces. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Thu, 18 Aug 2016 13:21:20 -0700
changeset 310053 6d4e430fbd45ed519c81bc6fde02dd9e31d56251
parent 310052 83bbd356da976384d6971b5ef83d415b02c7193b
child 310054 02f9e27b988dd63dce023fbca505ac74a1e8a1f6
push id80769
push userseth.bugzilla@blackhail.net
push dateThu, 18 Aug 2016 22:43:16 +0000
treeherdermozilla-inbound@856b1b82372a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1293472
milestone51.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 1293472 (Part 1) - Make it possible to seek DecodedSurfaces. r=dholbert
image/DecodedSurfaceProvider.cpp
image/DecodedSurfaceProvider.h
image/ISurfaceProvider.h
--- a/image/DecodedSurfaceProvider.cpp
+++ b/image/DecodedSurfaceProvider.cpp
@@ -48,18 +48,21 @@ DecodedSurfaceProvider::DropImageReferen
   // destructor may call into the surface cache while whatever code caused us to
   // get evicted is holding the surface cache lock, causing deadlock.
   RefPtr<RasterImage> image = mImage;
   mImage = nullptr;
   NS_ReleaseOnMainThread(image.forget(), /* aAlwaysProxy = */ true);
 }
 
 DrawableFrameRef
-DecodedSurfaceProvider::DrawableRef()
+DecodedSurfaceProvider::DrawableRef(size_t aFrame)
 {
+  MOZ_ASSERT(aFrame == 0,
+             "Requesting an animation frame from a DecodedSurfaceProvider?");
+
   // We depend on SurfaceCache::SurfaceAvailable() to provide synchronization
   // for methods that touch |mSurface|; after SurfaceAvailable() is called,
   // |mSurface| should be non-null and shouldn't be mutated further until we get
   // destroyed. That means that the assertions below are very important; we'll
   // end up with data races if these assumptions are violated.
   if (Availability().IsPlaceholder()) {
     MOZ_ASSERT_UNREACHABLE("Calling DrawableRef() on a placeholder");
     return DrawableFrameRef();
--- a/image/DecodedSurfaceProvider.h
+++ b/image/DecodedSurfaceProvider.h
@@ -37,17 +37,17 @@ public:
   // ISurfaceProvider implementation.
   //////////////////////////////////////////////////////////////////////////////
 
 public:
   bool IsFinished() const override;
   size_t LogicalSizeInBytes() const override;
 
 protected:
-  DrawableFrameRef DrawableRef() override;
+  DrawableFrameRef DrawableRef(size_t aFrame) override;
   bool IsLocked() const override { return bool(mLockRef); }
   void SetLocked(bool aLocked) override;
 
 
   //////////////////////////////////////////////////////////////////////////////
   // IDecodingTask implementation.
   //////////////////////////////////////////////////////////////////////////////
 
--- a/image/ISurfaceProvider.h
+++ b/image/ISurfaceProvider.h
@@ -59,18 +59,20 @@ public:
 
 protected:
   explicit ISurfaceProvider(AvailabilityState aAvailability)
     : mAvailability(aAvailability)
   { }
 
   virtual ~ISurfaceProvider() { }
 
-  /// @return an eagerly computed drawable reference to a surface.
-  virtual DrawableFrameRef DrawableRef() = 0;
+  /// @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 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
   /// should avoid releasing its resources. Should only be called from
@@ -123,49 +125,79 @@ public:
     MOZ_ASSERT(this != &aOther, "Self-moves are prohibited");
     mDrawableRef = Move(aOther.mDrawableRef);
     mProvider = Move(aOther.mProvider);
     mHaveSurface = aOther.mHaveSurface;
     aOther.mHaveSurface = false;
     return *this;
   }
 
+  /**
+   * If this DrawableSurface is dynamically generated from an animation, attempt
+   * to seek to frame @aFrame, where @aFrame is a 0-based index into the frames
+   * of the animation. Otherwise, nothing will blow up at runtime, but we assert
+   * in debug builds, since calling this in an unexpected situation probably
+   * indicates a bug.
+   *
+   * @return a successful result if we could obtain frame @aFrame. Note that
+   * |mHaveSurface| being true means that we're guaranteed to have *some* frame,
+   * so the caller can dereference this DrawableSurface even if Seek() fails,
+   * but while nothing will blow up, the frame won't be the one they expect.
+   */
+  nsresult Seek(size_t aFrame)
+  {
+    MOZ_ASSERT(mHaveSurface, "Trying to seek an empty DrawableSurface?");
+
+    if (!mProvider) {
+      MOZ_ASSERT_UNREACHABLE("Trying to seek a static DrawableSurface?");
+      return NS_ERROR_FAILURE;
+    }
+
+    mDrawableRef = mProvider->DrawableRef(aFrame);
+
+    return mDrawableRef ? NS_OK : NS_ERROR_FAILURE;
+  }
+
   explicit operator bool() const { return mHaveSurface; }
   imgFrame* operator->() { return DrawableRef().get(); }
 
 private:
   DrawableSurface(const DrawableSurface& aOther) = delete;
   DrawableSurface& operator=(const DrawableSurface& aOther) = delete;
 
   DrawableFrameRef& DrawableRef()
   {
     MOZ_ASSERT(mHaveSurface);
 
     // If we weren't created with a DrawableFrameRef directly, we should've been
-    // created with an ISurfaceProvider which can give us one.
+    // created with an ISurfaceProvider which can give us one. Note that if
+    // Seek() has been called, we'll already have a DrawableFrameRef, so we
+    // won't need to get one here.
     if (!mDrawableRef) {
       MOZ_ASSERT(mProvider);
-      mDrawableRef = mProvider->DrawableRef();
+      mDrawableRef = mProvider->DrawableRef(/* aFrame = */ 0);
     }
 
     MOZ_ASSERT(mDrawableRef);
     return mDrawableRef;
   }
 
   DrawableFrameRef mDrawableRef;
   RefPtr<ISurfaceProvider> mProvider;
   bool mHaveSurface;
 };
 
 
-// Surface() is implemented here so that DrawableSurface's definition is visible.
+// Surface() is implemented here so that DrawableSurface's definition is
+// visible. This default implementation eagerly obtains a DrawableFrameRef for
+// the first frame and is intended for static ISurfaceProviders.
 inline DrawableSurface
 ISurfaceProvider::Surface()
 {
-  return DrawableSurface(DrawableRef());
+  return DrawableSurface(DrawableRef(/* aFrame = */ 0));
 }
 
 
 /**
  * An ISurfaceProvider that stores a single surface.
  */
 class SimpleSurfaceProvider final : public ISurfaceProvider
 {
@@ -181,17 +213,23 @@ public:
 
   size_t LogicalSizeInBytes() const override
   {
     gfx::IntSize size = mSurface->GetSize();
     return size.width * size.height * mSurface->GetBytesPerPixel();
   }
 
 protected:
-  DrawableFrameRef DrawableRef() override { return mSurface->DrawableRef(); }
+  DrawableFrameRef DrawableRef(size_t aFrame) override
+  {
+    MOZ_ASSERT(aFrame == 0,
+               "Requesting an animation frame from a SimpleSurfaceProvider?");
+    return mSurface->DrawableRef();
+  }
+
   bool IsLocked() const override { return bool(mLockRef); }
 
   void SetLocked(bool aLocked) override
   {
     if (aLocked == IsLocked()) {
       return;  // Nothing changed.
     }