Bug 1272600 - Part 7 - Add some documentation to TextureReadLock. r=sotaro

gfx/layers/Compositor.h

@@ -533,6 +533,12 @@ public:
     return mParent;
   }
 
+  /// Most compositor backends operate asynchronously under the hood. This
+  /// means that when a layer stops using a texture it is often desirable to
+  /// wait for the end of the next composition before releasing the texture's
+  /// ReadLock.
+  /// This function provides a convenient way to do this delayed unlocking, if
+  /// the texture itself requires it.
   void UnlockAfterComposition(already_AddRefed<TextureReadLock> aLock)
   {
     mUnlockAfterComposition.AppendElement(aLock);

gfx/layers/client/TextureClient.cpp

@@ -452,6 +452,12 @@ TextureClient::UnlockActor() const
 bool
 TextureClient::IsReadLocked() const
 {
+  // mPendingReadUnlock is true when the texture has been written into (and as 
+  // a result the read-count already incremented on the behalf of the compositor),
+  // but we haven't sent the notification for the compositor to use the texture,
+  // so it is still OK to access the texture data on this side.
+  // If we didn't take mPendingReadUnlock into account here, we would not be able
+  // to Lock and Unlock a texture twice before sending it.
   return mReadLock && mReadLock->GetReadCount() > 1 && !mPendingReadUnlock;
 }
 

gfx/layers/composite/ImageHost.h

@@ -127,6 +127,9 @@ protected:
     int32_t mInputFrameID;
   };
 
+  // Use a simple RefPtr because the same texture is already held by a
+  // a CompositableTextureHostRef in the array of TimedImage.
+  // See the comment in CompositableTextureRef for more details.
   RefPtr<TextureHost> mCurrentTextureHost;
   CompositableTextureSourceRef mCurrentTextureSource;
   // When doing texture uploads it's best to alternate between two (or three)

gfx/layers/composite/TextureHost.cpp

@@ -312,6 +312,10 @@ TextureHost::TextureHost(TextureFlags aFlags)
 
 TextureHost::~TextureHost()
 {
+  // If we still have a ReadLock, unlock it. At this point we don't care about
+  // the texture client being written into on the other side since it should be
+  // destroyed by now. But we will hit assertions if we don't ReadUnlock before
+  // destroying the lock itself.
   ReadUnlock();
   MOZ_COUNT_DTOR(TextureHost);
 }
@@ -693,6 +697,8 @@ BufferTextureHost::MaybeUpload(nsIntRegion *aRegion)
   }
 
   if (mHasIntermediateBuffer) {
+    // We just did the texture upload, the content side can now freely write
+    // into the shared buffer.
     ReadUnlock();
   }
 

gfx/layers/composite/TextureHost.h

@@ -168,10 +168,33 @@ protected:
   int mCompositableCount;
 };
 
-/**
- * equivalent of a RefPtr<TextureSource>, that calls AddCompositableRef and
- * ReleaseCompositableRef in addition to the usual AddRef and Release.
- */
+/// Equivalent of a RefPtr<TextureSource>, that calls AddCompositableRef and
+/// ReleaseCompositableRef in addition to the usual AddRef and Release.
+///
+/// The semantoics of these CompositableTextureRefs are important because they
+/// are used both as a synchronization/safety mechanism, and as an optimization
+/// mechanism. They are also tricky and subtle because we use them in a very
+/// implicit way (assigning to a CompositableTextureRef is less visible than
+/// explicitly calling a method or whatnot).
+/// It is Therefore important to be careful about the way we use this tool.
+///
+/// CompositableTextureRef is a mechanism that lets us count how many compositables
+/// are using a given texture (for TextureSource and TextureHost).
+/// We use it to run specific code when a texture is not used anymore, and also
+/// we trigger fast paths on some operations when we can see that the texture's
+/// CompositableTextureRef counter is equal to 1 (the texture is not shared
+/// between compositables).
+/// This means that it is important to observe the following rules:
+/// * CompositableHosts that receive UseTexture and similar messages *must* store
+/// all of the TextureHosts they receive in CompositableTextureRef slots for as
+/// long as they may be using them.
+/// * CompositableHosts must store each texture in a *single* CompositableTextureRef
+/// slot to ensure that the counter properly reflects how many compositables are
+/// using the texture.
+/// If a compositable needs to hold two references to a given texture (for example
+/// to have a pointer to the current texture in a list of textures that may be
+/// used), it can hold its extra references with RefPtr or whichever pointer type
+/// makes sense.
 template<typename T>
 class CompositableTextureRef {
 public:
@@ -581,11 +604,11 @@ public:
 
   TextureReadLock* GetReadLock() { return mReadLock; }
 
-  void ReadUnlock();
-
   virtual Compositor* GetCompositor() = 0;
 
 protected:
+  void ReadUnlock();
+
   FenceHandle mReleaseFenceHandle;
 
   FenceHandle mAcquireFenceHandle;

gfx/layers/composite/TiledContentHost.h

@@ -93,12 +93,6 @@ public:
 
   bool IsPlaceholderTile() const { return mTextureHost == nullptr; }
 
-  void ReadUnlock() {
-    if (mTextureHost) {
-      mTextureHost->ReadUnlock();
-    }
-  }
-
   void Dump(std::stringstream& aStream) {
     aStream << "TileHost(...)"; // fill in as needed
   }