Bug 1462355 - Part 2. Expose imgFrame's data pointers via RawAccessFrameRef. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 29 May 2018 08:36:12 -0400
changeset 476999 3ea8add8bacc5b2b74b03ff3ba410211000c76c5
parent 476998 eb8ec97c5f5fa8555c8c9370b029807a707fc15f
child 477000 45406c2b9e9cfa9fc2dd8cd80a35c82cf9480efc
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [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 2. Expose imgFrame's data pointers via RawAccessFrameRef. r=tnikkel RawAccessFrameRef ensures there is a valid data pointer to the pixel data for the frame. It is a common pattern for users of RawAccessFrameRef to follow up with a request for the data pointer shortly after creation. We can avoid an extra lock by exposing this data pointer from RawAccessFrameRef, and populating it via imgFrame::LockImageData.
image/imgFrame.cpp
image/imgFrame.h
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -490,28 +490,28 @@ imgFrame::Optimize(DrawTarget* aTarget)
 
 DrawableFrameRef
 imgFrame::DrawableRef()
 {
   return DrawableFrameRef(this);
 }
 
 RawAccessFrameRef
-imgFrame::RawAccessRef()
+imgFrame::RawAccessRef(bool aOnlyFinished /*= false*/)
 {
-  return RawAccessFrameRef(this);
+  return RawAccessFrameRef(this, aOnlyFinished);
 }
 
 void
 imgFrame::SetRawAccessOnly()
 {
   AssertImageDataLocked();
 
   // Lock our data and throw away the key.
-  LockImageData();
+  LockImageData(false);
 }
 
 
 imgFrame::SurfaceWithFormat
 imgFrame::SurfaceForDrawing(bool               aDoPartialDecode,
                             bool               aDoTile,
                             ImageRegion&       aRegion,
                             SourceSurface*     aSurface)
@@ -742,45 +742,43 @@ uint32_t*
 imgFrame::GetPaletteData() const
 {
   uint32_t* data;
   uint32_t length;
   GetPaletteData(&data, &length);
   return data;
 }
 
-nsresult
-imgFrame::LockImageData()
+uint8_t*
+imgFrame::LockImageData(bool aOnlyFinished)
 {
   MonitorAutoLock lock(mMonitor);
 
   MOZ_ASSERT(mLockCount >= 0, "Unbalanced locks and unlocks");
-  if (mLockCount < 0) {
-    return NS_ERROR_FAILURE;
-  }
-
-  mLockCount++;
-
-  // If we are not the first lock, there's nothing to do.
-  if (mLockCount != 1) {
-    return NS_OK;
+  if (mLockCount < 0 || (aOnlyFinished && !mFinished)) {
+    return nullptr;
   }
 
-  // If we're the first lock, but have the locked surface, we're OK.
-  if (mLockedSurface) {
-    return NS_OK;
+  uint8_t* data;
+  if (mPalettedImageData) {
+    data = mPalettedImageData;
+  } else if (mLockedSurface) {
+    data = mLockedSurface->GetData();
+  } else {
+    data = nullptr;
   }
 
-  // Paletted images don't have surfaces, so there's nothing to do.
-  if (mPalettedImageData) {
-    return NS_OK;
+  // If the raw data is still available, we should get a valid pointer for it.
+  if (!data) {
+    MOZ_ASSERT_UNREACHABLE("It's illegal to re-lock an optimized imgFrame");
+    return nullptr;
   }
 
-  MOZ_ASSERT_UNREACHABLE("It's illegal to re-lock an optimized imgFrame");
-  return NS_ERROR_FAILURE;
+  ++mLockCount;
+  return data;
 }
 
 void
 imgFrame::AssertImageDataLocked() const
 {
 #ifdef DEBUG
   MonitorAutoLock lock(mMonitor);
   MOZ_ASSERT(mLockCount > 0, "Image data should be locked");
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -124,17 +124,24 @@ public:
   nsresult InitWithDrawable(gfxDrawable* aDrawable,
                             const nsIntSize& aSize,
                             const SurfaceFormat aFormat,
                             SamplingFilter aSamplingFilter,
                             uint32_t aImageFlags,
                             gfx::BackendType aBackend);
 
   DrawableFrameRef DrawableRef();
-  RawAccessFrameRef RawAccessRef();
+
+  /**
+   * Create a RawAccessFrameRef for the frame.
+   *
+   * @param aOnlyFinished If true, only return a valid RawAccessFrameRef if
+   *                      imgFrame::Finish has been called.
+   */
+  RawAccessFrameRef RawAccessRef(bool aOnlyFinished = false);
 
   /**
    * Make this imgFrame permanently available for raw access.
    *
    * This is irrevocable, and should be avoided whenever possible, since it
    * prevents this imgFrame from being optimized and makes it impossible for its
    * volatile buffer to be freed.
    *
@@ -229,17 +236,25 @@ public:
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf, size_t& aHeapSizeOut,
                               size_t& aNonHeapSizeOut,
                               size_t& aExtHandlesOut) const;
 
 private: // methods
 
   ~imgFrame();
 
-  nsresult LockImageData();
+  /**
+   * Used when the caller desires raw access to the underlying frame buffer.
+   * If the locking succeeds, the data pointer to the start of the buffer is
+   * returned, else it returns nullptr.
+   *
+   * @param aOnlyFinished If true, only attempt to lock if imgFrame::Finish has
+   *                      been called.
+   */
+  uint8_t* LockImageData(bool aOnlyFinished);
   nsresult UnlockImageData();
   nsresult Optimize(gfx::DrawTarget* aTarget);
 
   void AssertImageDataLocked() const;
 
   bool AreAllPixelsWritten() const;
   nsresult ImageUpdatedInternal(const nsIntRect& aUpdateRect);
   void GetImageDataInternal(uint8_t** aData, uint32_t* length) const;
@@ -432,32 +447,37 @@ private:
  * that the imgFrame holds.
  *
  * Once all an imgFrame's RawAccessFrameRefs go out of scope, new
  * RawAccessFrameRefs cannot be created.
  */
 class RawAccessFrameRef final
 {
 public:
-  RawAccessFrameRef() { }
+  RawAccessFrameRef() : mData(nullptr) { }
 
-  explicit RawAccessFrameRef(imgFrame* aFrame)
+  explicit RawAccessFrameRef(imgFrame* aFrame,
+                             bool aOnlyFinished)
     : mFrame(aFrame)
+    , mData(nullptr)
   {
     MOZ_ASSERT(mFrame, "Need a frame");
 
-    if (NS_FAILED(mFrame->LockImageData())) {
-      mFrame->UnlockImageData();
+    mData = mFrame->LockImageData(aOnlyFinished);
+    if (!mData) {
       mFrame = nullptr;
     }
   }
 
   RawAccessFrameRef(RawAccessFrameRef&& aOther)
     : mFrame(aOther.mFrame.forget())
-  { }
+    , mData(aOther.mData)
+  {
+    aOther.mData = nullptr;
+  }
 
   ~RawAccessFrameRef()
   {
     if (mFrame) {
       mFrame->UnlockImageData();
     }
   }
 
@@ -465,16 +485,18 @@ public:
   {
     MOZ_ASSERT(this != &aOther, "Self-moves are prohibited");
 
     if (mFrame) {
       mFrame->UnlockImageData();
     }
 
     mFrame = aOther.mFrame.forget();
+    mData = aOther.mData;
+    aOther.mData = nullptr;
 
     return *this;
   }
 
   explicit operator bool() const { return bool(mFrame); }
 
   imgFrame* operator->()
   {
@@ -492,20 +514,26 @@ public:
   const imgFrame* get() const { return mFrame; }
 
   void reset()
   {
     if (mFrame) {
       mFrame->UnlockImageData();
     }
     mFrame = nullptr;
+    mData = nullptr;
   }
 
+  uint8_t* Data() const { return mData; }
+  uint32_t PaletteDataLength() const { return mFrame->PaletteDataLength(); }
+
 private:
   RawAccessFrameRef(const RawAccessFrameRef& aOther) = delete;
+  RawAccessFrameRef& operator=(const RawAccessFrameRef& aOther) = delete;
 
   RefPtr<imgFrame> mFrame;
+  uint8_t* mData;
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_imgFrame_h