From d76e9ba35f472a7c405d3b669020512a1abeac74 Mon Sep 17 00:00:00 2001
From: Markus Stange <mstange@themasta.com>
Date: Tue, 15 Oct 2019 02:14:50 +0000
Subject: [PATCH] Bug 1586627 - Work around a bug in
 AppleIntelHD3000GraphicsGLDriver in CompositorOGL. r=jrmuizel

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

--HG--
extra : moz-landing-system : lando
---
 gfx/layers/opengl/CompositorOGL.cpp | 63 +++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/gfx/layers/opengl/CompositorOGL.cpp b/gfx/layers/opengl/CompositorOGL.cpp
index b8ee153bd707..fdd885f59082 100644
--- a/gfx/layers/opengl/CompositorOGL.cpp
+++ b/gfx/layers/opengl/CompositorOGL.cpp
@@ -1056,6 +1056,67 @@ void CompositorOGL::CreateFBOWithTexture(const gfx::IntRect& aRect,
   mGLContext->fGenFramebuffers(1, aFBO);
 }
 
+// Should be called after calls to fReadPixels or fCopyTexImage2D, and other
+// GL read calls.
+static void WorkAroundAppleIntelHD3000GraphicsGLDriverBug(GLContext* aGL) {
+#ifdef XP_MACOSX
+  if (aGL->WorkAroundDriverBugs() &&
+      aGL->Renderer() == GLRenderer::IntelHD3000) {
+    // Work around a bug in the Apple Intel HD Graphics 3000 driver (bug
+    // 1586627, filed with Apple as FB7379358). This bug has been found present
+    // on 10.9.3 and on 10.13.6, so it likely affects all shipped versions of
+    // this driver. (macOS 10.14 does not support this GPU.)
+    // The bug manifests as follows: Reading from a framebuffer puts that
+    // framebuffer into a state such that deleting that framebuffer can break
+    // other framebuffers in certain cases. More specifically, if you have two
+    // framebuffers A and B, the following sequence of events breaks subsequent
+    // drawing to B:
+    //  1. A becomes "most recently read-from framebuffer".
+    //  2. B is drawn to.
+    //  3. A is deleted, and other GL state (such as GL_SCISSOR enabled state)
+    //     is touched.
+    //  4. B is drawn to again.
+    // Now all draws to framebuffer B, including the draw from step 4, will
+    // render at the wrong position and upside down.
+    //
+    // When AfterGLReadCall() is called, the currently bound framebuffer is the
+    // framebuffer that has been read from most recently. So in the presence of
+    // this bug, deleting this framebuffer has now become dangerous. We work
+    // around the bug by creating a new short-lived framebuffer, making that new
+    // framebuffer the most recently read-from framebuffer (using
+    // glCopyTexImage2D), and then deleting it under controlled circumstances.
+    // This deletion is not affected by the bug because our deletion call is not
+    // interleaved with draw calls to another framebuffer and a touching of the
+    // GL scissor enabled state.
+
+    ScopedTexture texForReading(aGL);
+    {
+      // Initialize a 1x1 texture.
+      ScopedBindTexture autoBindTexForReading(aGL, texForReading);
+      aGL->fTexImage2D(LOCAL_GL_TEXTURE_2D, 0, LOCAL_GL_RGBA, 1, 1, 0,
+                       LOCAL_GL_RGBA, LOCAL_GL_UNSIGNED_BYTE, nullptr);
+      aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MIN_FILTER,
+                          LOCAL_GL_LINEAR);
+      aGL->fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MAG_FILTER,
+                          LOCAL_GL_LINEAR);
+    }
+    // Make a framebuffer around the texture.
+    ScopedFramebufferForTexture autoFBForReading(aGL, texForReading);
+    if (autoFBForReading.IsComplete()) {
+      // "Read" from the framebuffer, by initializing a new texture using
+      // glCopyTexImage2D. This flips the bad bit on autoFBForReading.FB().
+      ScopedBindFramebuffer autoFB(aGL, autoFBForReading.FB());
+      ScopedTexture texReadingDest(aGL);
+      ScopedBindTexture autoBindTexReadingDest(aGL, texReadingDest);
+      aGL->fCopyTexImage2D(LOCAL_GL_TEXTURE_2D, 0, LOCAL_GL_RGBA, 0, 0, 1, 1,
+                           0);
+    }
+    // When autoFBForReading goes out of scope, the "poisoned" framebuffer is
+    // deleted, and the bad state seems to go away along with it.
+  }
+#endif
+}
+
 GLuint CompositorOGL::CreateTexture(const IntRect& aRect, bool aCopyFromSource,
                                     GLuint aSourceFrameBuffer,
                                     IntSize* aAllocSize) {
@@ -1100,6 +1161,7 @@ GLuint CompositorOGL::CreateTexture(const IntRect& aRect, bool aCopyFromSource,
       mGLContext->fCopyTexImage2D(mFBOTextureTarget, 0, LOCAL_GL_RGBA,
                                   clampedRect.X(), FlipY(clampedRect.YMost()),
                                   clampedRectWidth, clampedRectHeight, 0);
+      WorkAroundAppleIntelHD3000GraphicsGLDriverBug(mGLContext);
     } else {
       // Curses, incompatible formats.  Take a slow path.
 
@@ -1110,6 +1172,7 @@ GLuint CompositorOGL::CreateTexture(const IntRect& aRect, bool aCopyFromSource,
       mGLContext->fReadPixels(clampedRect.X(), clampedRect.Y(),
                               clampedRectWidth, clampedRectHeight,
                               LOCAL_GL_RGBA, LOCAL_GL_UNSIGNED_BYTE, buf.get());
+      WorkAroundAppleIntelHD3000GraphicsGLDriverBug(mGLContext);
       mGLContext->fTexImage2D(mFBOTextureTarget, 0, LOCAL_GL_RGBA,
                               clampedRectWidth, clampedRectHeight, 0,
                               LOCAL_GL_RGBA, LOCAL_GL_UNSIGNED_BYTE, buf.get());