Bug 1483533 - Delay texture delete for DirectMapTextureSource r=jrmuizel

I wish I understood a little better what precisely is going on
here. What seems to be the problem is calling glDeleteTextures
too early, but I can't pin down exactly when "too early" is.
In any case I can no longer reproduce the issue with this patch
applied, and I cannot observe any performance degradation, and
it's not a remarkably risky patch, so I'm opting to cut the
investigation short. Any insights would be appreciated though.

Differential Revision: https://phabricator.services.mozilla.com/D6064

--HG--
extra : moz-landing-system : lando

gfx/layers/TextureSourceProvider.cpp

@@ -46,6 +46,7 @@ TextureSourceProvider::ReadUnlockTextures()
     texture->ReadUnlock();
   }
 #endif
+  mReferenceUntilAfterComposition.Clear();
   mUnlockAfterComposition.Clear();
 }
 
@@ -55,6 +56,12 @@ TextureSourceProvider::UnlockAfterComposition(TextureHost* aTexture)
   mUnlockAfterComposition.AppendElement(aTexture);
 }
 
+void
+TextureSourceProvider::ReferenceUntilAfterComposition(DataTextureSource* aTextureSource)
+{
+  mReferenceUntilAfterComposition.AppendElement(aTextureSource);
+}
+
 bool
 TextureSourceProvider::NotifyNotUsedAfterComposition(TextureHost* aTextureHost)
 {

gfx/layers/TextureSourceProvider.h

@@ -64,6 +64,13 @@ public:
   /// the texture itself requires it.
   virtual void UnlockAfterComposition(TextureHost* aTexture);
 
+  /// This is used for client storage support on OSX. On Nvidia hardware, it
+  /// seems that if we glDeleteTextures on a texture too early, even if we've
+  /// waited on the texture with glFinishObjectAPPLE, we'll see visual defects.
+  /// This just holds a reference to the texture until ReadUnlockTextures,
+  /// but we don't need to unlock it since we have already done so.
+  void ReferenceUntilAfterComposition(DataTextureSource* aTextureSource);
+
   /// 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 NotifyNotUsed() call.
@@ -131,6 +138,9 @@ private:
   // An array of locks that will need to be unlocked after the next composition.
   nsTArray<RefPtr<TextureHost>> mUnlockAfterComposition;
 
+  // See ReferenceUntilAfterComposition.
+  nsTArray<RefPtr<DataTextureSource>> mReferenceUntilAfterComposition;
+
   // An array of TextureHosts that will need to call NotifyNotUsed() after the next composition.
   nsTArray<RefPtr<TextureHost>> mNotifyNotUsedAfterComposition;
 };

gfx/layers/composite/TextureHost.cpp

@@ -927,6 +927,10 @@ BufferTextureHost::UnbindTextureSource()
     mFirstSource->Unbind();
   }
 
+  if (mFirstSource && mFirstSource->IsDirectMap() && mProvider) {
+    mProvider->ReferenceUntilAfterComposition(mFirstSource);
+  }
+
   // This texture is not used by any layer anymore.
   // If the texture doesn't have an intermediate buffer, it means we are
   // compositing synchronously on the CPU, so we don't need to wait until