Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical draft
authorRobert O'Callahan <robert@ocallahan.org>
Tue, 12 May 2015 12:56:09 +1200
changeset 275585 7ef0a10fa41cbfd95571280dad03d6c93a483ec6
parent 275584 68871db232526d0ee0b895927bfc8aca85b4c146
child 275586 b2be8062ca8a331e4a09d04a714d64018b8dee2f
push id3189
push userrocallahan@mozilla.com
push dateFri, 03 Jul 2015 11:12:01 +0000
reviewersnical
bugs1143575
milestone42.0a1
Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical When ImageContainer and ImageClient are managing a list of images, the individual Image serial numbers are no longer enough to detect whether the state has changed.
gfx/layers/ImageContainer.cpp
gfx/layers/ImageContainer.h
gfx/layers/client/ImageClient.cpp
gfx/layers/client/ImageClient.h
--- a/gfx/layers/ImageContainer.cpp
+++ b/gfx/layers/ImageContainer.cpp
@@ -43,16 +43,18 @@ namespace mozilla {
 namespace layers {
 
 using namespace mozilla::ipc;
 using namespace android;
 using namespace mozilla::gfx;
 
 Atomic<int32_t> Image::sSerialCounter(0);
 
+Atomic<uint32_t> ImageContainer::sGenerationCounter(0);
+
 already_AddRefed<Image>
 ImageFactory::CreateImage(ImageFormat aFormat,
                           const gfx::IntSize &,
                           BufferRecycleBin *aRecycleBin)
 {
   nsRefPtr<Image> img;
 #ifdef MOZ_WIDGET_GONK
   if (aFormat == ImageFormat::GRALLOC_PLANAR_YCBCR) {
@@ -135,16 +137,17 @@ BufferRecycleBin::GetBuffer(uint32_t aSi
   uint32_t last = mRecycledBuffers.Length() - 1;
   uint8_t* result = mRecycledBuffers[last].forget();
   mRecycledBuffers.RemoveElementAt(last);
   return result;
 }
 
 ImageContainer::ImageContainer(ImageContainer::Mode flag)
 : mReentrantMonitor("ImageContainer.mReentrantMonitor"),
+  mGenerationCounter(++sGenerationCounter),
   mPaintCount(0),
   mPreviousImagePainted(false),
   mImageFactory(new ImageFactory()),
   mRecycleBin(new BufferRecycleBin()),
   mImageClient(nullptr)
 {
   if (ImageBridgeChild::IsCreated()) {
     // the refcount of this ImageClient is 1. we don't use a RefPtr here because the refcount
@@ -199,16 +202,19 @@ ImageContainer::CreateImage(ImageFormat 
   return mImageFactory->CreateImage(aFormat, mScaleHint, mRecycleBin);
 }
 
 void
 ImageContainer::SetCurrentImageInternal(Image *aImage)
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
 
+  if (mActiveImage != aImage) {
+    mGenerationCounter = ++sGenerationCounter;
+  }
   mActiveImage = aImage;
   CurrentImageChanged();
 }
 
 void
 ImageContainer::ClearCurrentImage()
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
@@ -279,23 +285,27 @@ bool
 ImageContainer::HasCurrentImage()
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
 
   return !!mActiveImage.get();
 }
 
 void
-ImageContainer::GetCurrentImages(nsTArray<OwningImage>* aImages)
+ImageContainer::GetCurrentImages(nsTArray<OwningImage>* aImages,
+                                 uint32_t* aGenerationCounter)
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
 
   if (mActiveImage) {
     aImages->AppendElement()->mImage = mActiveImage;
   }
+  if (aGenerationCounter) {
+    *aGenerationCounter = mGenerationCounter;
+  }
 }
 
 gfx::IntSize
 ImageContainer::GetCurrentSize()
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
 
   if (!mActiveImage) {
--- a/gfx/layers/ImageContainer.h
+++ b/gfx/layers/ImageContainer.h
@@ -175,18 +175,19 @@ protected:
   mozilla::EnumeratedArray<mozilla::layers::LayersBackend,
                            mozilla::layers::LayersBackend::LAYERS_LAST,
                            nsAutoPtr<ImageBackendData>>
     mBackendData;
 
   void* mImplData;
   int32_t mSerial;
   ImageFormat mFormat;
+  bool mSent;
+
   static mozilla::Atomic<int32_t> sSerialCounter;
-  bool mSent;
 };
 
 /**
  * A RecycleBin is owned by an ImageContainer. We store buffers in it that we
  * want to recycle from one image to the next.It's a separate object from 
  * ImageContainer because images need to store a strong ref to their RecycleBin
  * and we must avoid creating a reference loop between an ImageContainer and
  * its active image.
@@ -375,22 +376,24 @@ public:
 
   struct OwningImage {
     nsRefPtr<Image> mImage;
   };
   /**
    * Copy the current Image list to aImages.
    * This has to add references since otherwise there are race conditions
    * where the current image is destroyed before the caller can add
-   * a reference. This lock strictly guarantees the underlying image remains
-   * valid, it does not mean the current image cannot change.
+   * a reference.
    * Can be called on any thread.
    * May return an empty list to indicate there is no current image.
+   * If aGenerationCounter is non-null, sets *aGenerationCounter to a value
+   * that's unique for this ImageContainer state.
    */
-  void GetCurrentImages(nsTArray<OwningImage>* aImages);
+  void GetCurrentImages(nsTArray<OwningImage>* aImages,
+                        uint32_t* aGenerationCounter = nullptr);
 
   /**
    * Returns the size of the image in pixels.
    * Can be called on any thread. This method takes mReentrantMonitor when accessing
    * thread-shared state.
    */
   gfx::IntSize GetCurrentSize();
 
@@ -481,16 +484,18 @@ private:
   // mReentrantMonitor held.
   void CurrentImageChanged() {
     mReentrantMonitor.AssertCurrentThreadIn();
     mPreviousImagePainted = !mPaintTime.IsNull();
     mPaintTime = TimeStamp();
   }
 
   nsRefPtr<Image> mActiveImage;
+  // Updates every time mActiveImage changes
+  uint32_t mGenerationCounter;
 
   // Number of contained images that have been painted at least once.  It's up
   // to the ImageContainer implementation to ensure accesses to this are
   // threadsafe.
   uint32_t mPaintCount;
 
   // Time stamp at which the current image was first painted.  It's up to the
   // ImageContainer implementation to ensure accesses to this are threadsafe.
@@ -511,16 +516,18 @@ private:
   // This member points to an ImageClient if this ImageContainer was
   // sucessfully created with ENABLE_ASYNC, or points to null otherwise.
   // 'unsuccessful' in this case only means that the ImageClient could not
   // be created, most likely because off-main-thread compositing is not enabled.
   // In this case the ImageContainer is perfectly usable, but it will forward
   // frames to the compositor through transactions in the main thread rather than
   // asynchronusly using the ImageBridge IPDL protocol.
   ImageClient* mImageClient;
+
+  static mozilla::Atomic<uint32_t> sGenerationCounter;
 };
 
 class AutoLockImage
 {
 public:
   explicit AutoLockImage(ImageContainer *aContainer)
   {
     aContainer->GetCurrentImages(&mImages);
--- a/gfx/layers/client/ImageClient.cpp
+++ b/gfx/layers/client/ImageClient.cpp
@@ -134,34 +134,37 @@ ImageClientSingle::FlushAllImages(bool a
     // already flushed
     aAsyncTransactionTracker->NotifyComplete();
   }
 }
 
 bool
 ImageClientSingle::UpdateImage(ImageContainer* aContainer, uint32_t aContentFlags)
 {
-  AutoLockImage autoLock(aContainer);
+  nsAutoTArray<ImageContainer::OwningImage,4> images;
+  uint32_t generationCounter;
+  aContainer->GetCurrentImages(&images, &generationCounter);
 
-  Image *image = autoLock.GetImage();
-  if (!image) {
+  if (mLastUpdateGenerationCounter == generationCounter) {
+    return true;
+  }
+  mLastUpdateGenerationCounter = generationCounter;
+
+  if (images.IsEmpty()) {
     return false;
   }
 
+  Image* image = images[0].mImage;
   // Don't try to update to an invalid image. We return true because the caller
   // would attempt to recreate the ImageClient otherwise, and that isn't going
   // to help.
   if (!image->IsValid()) {
     return true;
   }
 
-  if (mLastPaintedImageSerial == image->GetSerial()) {
-    return true;
-  }
-
   RefPtr<TextureClient> texture = image->GetTextureClient(this);
 
   AutoRemoveTexture autoRemoveTexture(this);
   if (texture != mFrontBuffer) {
     autoRemoveTexture.mTexture = mFrontBuffer;
     mFrontBuffer = nullptr;
   }
 
@@ -243,19 +246,17 @@ ImageClientSingle::UpdateImage(ImageCont
   if (!texture || !AddTextureClient(texture)) {
     return false;
   }
 
   mFrontBuffer = texture;
   IntRect pictureRect = image->GetPictureRect();
   GetForwarder()->UseTexture(this, texture, &pictureRect);
 
-  mLastPaintedImageSerial = image->GetSerial();
   aContainer->NotifyPaintedImage(image);
-
   texture->SyncWithObject(GetForwarder()->GetSyncObject());
 
   return true;
 }
 
 bool
 ImageClientSingle::AddTextureClient(TextureClient* aTexture)
 {
@@ -269,17 +270,17 @@ ImageClientSingle::OnDetach()
   mFrontBuffer = nullptr;
 }
 
 ImageClient::ImageClient(CompositableForwarder* aFwd, TextureFlags aFlags,
                          CompositableType aType)
 : CompositableClient(aFwd, aFlags)
 , mLayer(nullptr)
 , mType(aType)
-, mLastPaintedImageSerial(0)
+, mLastUpdateGenerationCounter(0)
 {}
 
 ImageClientBridge::ImageClientBridge(CompositableForwarder* aFwd,
                                      TextureFlags aFlags)
 : ImageClient(aFwd, aFlags, CompositableType::IMAGE_BRIDGE)
 , mAsyncContainerID(0)
 {
 }
@@ -334,19 +335,20 @@ ImageClientOverlay::UpdateImage(ImageCon
 {
   AutoLockImage autoLock(aContainer);
 
   Image *image = autoLock.GetImage();
   if (!image) {
     return false;
   }
 
-  if (mLastPaintedImageSerial == image->GetSerial()) {
+  if (mLastUpdateGenerationCounter == (uint32_t)image->GetSerial()) {
     return true;
   }
+  mLastUpdateGenerationCounter = (uint32_t)image->GetSerial();
 
   AutoRemoveTexture autoRemoveTexture(this);
   if (image->GetFormat() == ImageFormat::OVERLAY_IMAGE) {
     OverlayImage* overlayImage = static_cast<OverlayImage*>(image);
     uint32_t overlayId = overlayImage->GetOverlayId();
     gfx::IntSize size = overlayImage->GetSize();
 
     OverlaySource source;
--- a/gfx/layers/client/ImageClient.h
+++ b/gfx/layers/client/ImageClient.h
@@ -79,17 +79,17 @@ public:
                                 AsyncTransactionTracker* aAsyncTransactionTracker = nullptr);
 
 protected:
   ImageClient(CompositableForwarder* aFwd, TextureFlags aFlags,
               CompositableType aType);
 
   ClientLayer* mLayer;
   CompositableType mType;
-  int32_t mLastPaintedImageSerial;
+  uint32_t mLastUpdateGenerationCounter;
 };
 
 /**
  * An image client which uses a single texture client.
  */
 class ImageClientSingle : public ImageClient
 {
 public: