Bug 1286847 - Remove calls to XGetGeometry from the compositor thread. r?jgilbert draft
authorAndrew Comminos <andrew@comminos.com>
Tue, 12 Jul 2016 15:01:21 -0400
changeset 389580 8eb1822146aa3ac90616a30f6710e7d3f1b793ae
parent 389550 5a91e5b49be3c1ba401b057e90c92d7488e3647d
child 389581 4d018303dcff66923efebd2b3f1d82644f308e78
push id23460
push userbmo:andrew@comminos.com
push dateTue, 19 Jul 2016 17:59:41 +0000
reviewersjgilbert
bugs1286847
milestone50.0a1
Bug 1286847 - Remove calls to XGetGeometry from the compositor thread. r?jgilbert MozReview-Commit-ID: IAd2y1FgiFn
gfx/gl/GLContext.h
gfx/gl/GLContextGLX.h
gfx/gl/GLContextProviderGLX.cpp
gfx/layers/opengl/CompositorOGL.cpp
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -24,17 +24,16 @@
 #endif
 
 // Define MOZ_GL_DEBUG unconditionally to enable GL debugging in opt
 // builds.
 #ifdef DEBUG
 #define MOZ_GL_DEBUG 1
 #endif
 
-#include "../../mfbt/Maybe.h"
 #include "../../mfbt/RefPtr.h"
 #include "../../mfbt/UniquePtr.h"
 
 #include "GLDefs.h"
 #include "GLLibraryLoader.h"
 #include "nsISupportsImpl.h"
 #include "plstr.h"
 #include "GLContextTypes.h"
@@ -3304,23 +3303,16 @@ public:
     }
 
     GLuint GetDrawFB();
 
     GLuint GetReadFB();
 
     GLuint GetFB();
 
-    /*
-     * Retrieves the size of the native windowing system drawable.
-     */
-    virtual Maybe<gfx::IntSize> GetTargetSize() {
-        return Maybe<gfx::IntSize>();
-    };
-
 private:
     void GetShaderPrecisionFormatNonES2(GLenum shadertype, GLenum precisiontype, GLint* range, GLint* precision) {
         switch (precisiontype) {
             case LOCAL_GL_LOW_FLOAT:
             case LOCAL_GL_MEDIUM_FLOAT:
             case LOCAL_GL_HIGH_FLOAT:
                 // Assume IEEE 754 precision
                 range[0] = 127;
--- a/gfx/gl/GLContextGLX.h
+++ b/gfx/gl/GLContextGLX.h
@@ -61,18 +61,16 @@ public:
 
     // Overrides the current GLXDrawable backing the context and makes the
     // context current.
     bool OverrideDrawable(GLXDrawable drawable);
 
     // Undoes the effect of a drawable override.
     bool RestoreDrawable();
 
-    virtual Maybe<gfx::IntSize> GetTargetSize() override;
-
 private:
     friend class GLContextProviderGLX;
 
     GLContextGLX(CreateContextFlags flags,
                  const SurfaceCaps& caps,
                  GLContext* shareContext,
                  bool isOffscreen,
                  Display* aDisplay,
--- a/gfx/gl/GLContextProviderGLX.cpp
+++ b/gfx/gl/GLContextProviderGLX.cpp
@@ -982,30 +982,16 @@ GLContextGLX::SwapBuffers()
 {
     if (!mDoubleBuffered)
         return false;
     mGLX->xSwapBuffers(mDisplay, mDrawable);
     mGLX->xWaitGL();
     return true;
 }
 
-Maybe<gfx::IntSize>
-GLContextGLX::GetTargetSize()
-{
-    unsigned int width = 0, height = 0;
-    Window root;
-    int x, y;
-    unsigned int border, depth;
-    XGetGeometry(mDisplay, mDrawable, &root, &x, &y, &width, &height,
-                 &border, &depth);
-    Maybe<gfx::IntSize> size;
-    size.emplace(width, height);
-    return size;
-}
-
 bool
 GLContextGLX::OverrideDrawable(GLXDrawable drawable)
 {
     if (Screen())
         Screen()->AssureBlitted();
     Bool result = mGLX->xMakeCurrent(mDisplay, drawable, mContext);
     return result;
 }
--- a/gfx/layers/opengl/CompositorOGL.cpp
+++ b/gfx/layers/opengl/CompositorOGL.cpp
@@ -701,25 +701,19 @@ CompositorOGL::BeginFrame(const nsIntReg
   mGLContext->fBlendFuncSeparate(LOCAL_GL_ONE, LOCAL_GL_ONE_MINUS_SRC_ALPHA,
                                  LOCAL_GL_ONE, LOCAL_GL_ONE_MINUS_SRC_ALPHA);
   mGLContext->fEnable(LOCAL_GL_BLEND);
 
   // Make sure SCISSOR is enabled before setting the render target, since the RT
   // assumes scissor is enabled while it does clears.
   mGLContext->fEnable(LOCAL_GL_SCISSOR_TEST);
 
-  // Prefer the native windowing system's provided window size for the viewport.
-  IntSize viewportSize =
-    mGLContext->GetTargetSize().valueOr(mWidgetSize.ToUnknownSize());
-  if (viewportSize != mWidgetSize.ToUnknownSize()) {
-    mGLContext->fScissor(0, 0, viewportSize.width, viewportSize.height);
-  }
-
   RefPtr<CompositingRenderTargetOGL> rt =
-    CompositingRenderTargetOGL::RenderTargetForWindow(this, viewportSize);
+    CompositingRenderTargetOGL::RenderTargetForWindow(this,
+                                                      IntSize(width, height));
   SetRenderTarget(rt);
 
 #ifdef DEBUG
   mWindowRenderTarget = mCurrentRenderTarget;
 #endif
 
   if (aClipRectOut && !aClipRectIn) {
     aClipRectOut->SetRect(0, 0, width, height);
@@ -1495,31 +1489,23 @@ CompositorOGL::EndFrame()
 
   if (mTarget) {
     CopyToTarget(mTarget, mTargetBounds.TopLeft(), Matrix());
     mGLContext->fBindBuffer(LOCAL_GL_ARRAY_BUFFER, 0);
     mCurrentRenderTarget = nullptr;
     return;
   }
 
+  mCurrentRenderTarget = nullptr;
+
   if (mTexturePool) {
     mTexturePool->EndFrame();
   }
 
-  // If our window size changed during composition, we should discard the frame.
-  // We don't need to worry about rescheduling a composite, as widget
-  // implementations handle this in their expose event listeners.
-  // See bug 1184534. TODO: implement this for single-buffered targets?
-  IntSize targetSize = mGLContext->GetTargetSize().valueOr(mViewportSize);
-  if (!(mCurrentRenderTarget->IsWindow() && targetSize != mViewportSize)) {
-    mGLContext->SwapBuffers();
-  }
-
-  mCurrentRenderTarget = nullptr;
-
+  mGLContext->SwapBuffers();
   mGLContext->fBindBuffer(LOCAL_GL_ARRAY_BUFFER, 0);
 
   // Unbind all textures
   for (GLuint i = 0; i <= 4; i++) {
     mGLContext->fActiveTexture(LOCAL_GL_TEXTURE0 + i);
     mGLContext->fBindTexture(LOCAL_GL_TEXTURE_2D, 0);
     if (!mGLContext->IsGLES()) {
       mGLContext->fBindTexture(LOCAL_GL_TEXTURE_RECTANGLE_ARB, 0);