Bug 1390386 - Remove duplicate IsCurrent checks in MakeCurrentImpls. - r=jrmuizel
authorJeff Gilbert <jgilbert@mozilla.com>
Tue, 15 Aug 2017 13:21:37 -0700
changeset 394037 b98d0d835d1250875edbbdaa7e5faa0ba8e847a6
parent 394036 3b910891a85bf4c99f765b76e695e1430b4253e2
child 394038 d0c82eb2cea989f0e8658f3a8200c9e5e091bba2
push id97799
push userjgilbert@mozilla.com
push dateWed, 29 Nov 2017 01:14:08 +0000
treeherdermozilla-inbound@a3d350120375 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1390386
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1390386 - Remove duplicate IsCurrent checks in MakeCurrentImpls. - r=jrmuizel MozReview-Commit-ID: LZeLbciWnic
gfx/gl/GLContext.cpp
gfx/gl/GLContext.h
gfx/gl/GLContextCGL.h
gfx/gl/GLContextEAGL.h
gfx/gl/GLContextEGL.h
gfx/gl/GLContextGLX.h
gfx/gl/GLContextProviderCGL.mm
gfx/gl/GLContextProviderEAGL.mm
gfx/gl/GLContextProviderEGL.cpp
gfx/gl/GLContextProviderGLX.cpp
gfx/gl/GLContextProviderWGL.cpp
gfx/gl/GLContextWGL.h
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -3026,28 +3026,35 @@ GetBytesPerTexel(GLenum format, GLenum t
 }
 
 bool
 GLContext::MakeCurrent(bool aForce) const
 {
     if (MOZ_UNLIKELY( IsDestroyed() ))
         return false;
 
-    if (mUseTLSIsCurrent && !aForce && sCurrentContext.get() == this) {
-        MOZ_ASSERT(IsCurrent());
-        return true;
+    if (MOZ_LIKELY( !aForce )) {
+        bool isCurrent;
+        if (mUseTLSIsCurrent) {
+            isCurrent = (sCurrentContext.get() == this);
+        } else {
+            isCurrent = IsCurrentImpl();
+        }
+        if (MOZ_LIKELY( isCurrent )) {
+            MOZ_ASSERT(IsCurrentImpl());
+            return true;
+        }
     }
 
-    if (!MakeCurrentImpl(aForce))
+    if (!MakeCurrentImpl())
         return false;
 
     if (mUseTLSIsCurrent) {
         sCurrentContext.set(this);
     }
-
     return true;
 }
 
 void
 GLContext::ResetSyncCallCount(const char* resetReason) const
 {
     if (ShouldSpew()) {
         printf_stderr("On %s, mSyncGLCallCount = %" PRIu64 "\n",
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -293,24 +293,27 @@ public:
      * If this context is double-buffered, returns TRUE.
      */
     virtual bool IsDoubleBuffered() const {
         return false;
     }
 
     virtual GLContextType GetContextType() const = 0;
 
+    virtual bool IsCurrentImpl() const = 0;
+    virtual bool MakeCurrentImpl() const = 0;
+
     bool IsCurrent() const {
         if (mImplicitMakeCurrent)
             return MakeCurrent();
 
         return IsCurrentImpl();
     }
 
-    virtual bool IsCurrentImpl() const = 0;
+    bool MakeCurrent(bool aForce = false) const;
 
     /**
      * Get the default framebuffer for this context.
      */
     virtual GLuint GetDefaultFramebuffer() {
         return 0;
     }
 
@@ -675,17 +678,17 @@ private:
 // Record the name of the GL call for better hang stacks on Android.
 #define ANDROID_ONLY_PROFILER_LABEL AUTO_PROFILER_LABEL(__func__, GRAPHICS);
 #else
 #define ANDROID_ONLY_PROFILER_LABEL
 #endif
 
 #define BEFORE_GL_CALL \
         ANDROID_ONLY_PROFILER_LABEL \
-        if (BeforeGLCall(MOZ_FUNCTION_NAME)) { \
+        if (MOZ_LIKELY( BeforeGLCall(MOZ_FUNCTION_NAME) )) { \
             do { } while (0)
 
 #define AFTER_GL_CALL \
             AfterGLCall(MOZ_FUNCTION_NAME); \
         } \
         do { } while (0)
 
     void BeforeGLCall_Debug(const char* funcName) const;
@@ -694,17 +697,17 @@ private:
 
     bool BeforeGLCall(const char* const funcName) const {
         if (mImplicitMakeCurrent) {
             if (MOZ_UNLIKELY( !MakeCurrent() )) {
                 OnImplicitMakeCurrentFailure(funcName);
                 return false;
             }
         }
-        MOZ_ASSERT(IsCurrent());
+        MOZ_ASSERT(IsCurrentImpl());
 
         if (mDebugFlags) {
             BeforeGLCall_Debug(funcName);
         }
         return true;
     }
 
     void AfterGLCall(const char* const funcName) const {
@@ -3311,21 +3314,17 @@ public:
     // the GL function pointers!
     void MarkDestroyed();
 
 // -----------------------------------------------------------------------------
 // Everything that isn't standard GL APIs
 protected:
     typedef gfx::SurfaceFormat SurfaceFormat;
 
-    virtual bool MakeCurrentImpl(bool aForce) const = 0;
-
 public:
-    bool MakeCurrent(bool aForce = false) const;
-
     virtual bool Init() = 0;
 
     virtual bool SetupLookupFunction() = 0;
 
     virtual void ReleaseSurface() {}
 
     bool IsDestroyed() const {
         // MarkDestroyed will mark all these as null.
--- a/gfx/gl/GLContextCGL.h
+++ b/gfx/gl/GLContextCGL.h
@@ -40,17 +40,17 @@ public:
         return static_cast<GLContextCGL*>(gl);
     }
 
     bool Init() override;
 
     NSOpenGLContext* GetNSOpenGLContext() const { return mContext; }
     CGLContextObj GetCGLContext() const;
 
-    virtual bool MakeCurrentImpl(bool aForce) const override;
+    virtual bool MakeCurrentImpl() const override;
 
     virtual bool IsCurrentImpl() const override;
 
     virtual GLenum GetPreferredARGB32Format() const override;
 
     virtual bool SetupLookupFunction() override;
 
     virtual bool IsDoubleBuffered() const override;
--- a/gfx/gl/GLContextEAGL.h
+++ b/gfx/gl/GLContextEAGL.h
@@ -38,17 +38,17 @@ public:
     }
 
     bool Init() override;
 
     bool AttachToWindow(nsIWidget* aWidget);
 
     EAGLContext* GetEAGLContext() const { return mContext; }
 
-    virtual bool MakeCurrentImpl(bool aForce) const override;
+    virtual bool MakeCurrentImpl() const override;
 
     virtual bool IsCurrentImpl() const override;
 
     virtual bool SetupLookupFunction() override;
 
     virtual bool IsDoubleBuffered() const override;
 
     virtual bool SwapBuffers() override;
--- a/gfx/gl/GLContextEGL.h
+++ b/gfx/gl/GLContextEGL.h
@@ -68,17 +68,17 @@ public:
 
     virtual bool ReleaseTexImage() override;
 
     void SetEGLSurfaceOverride(EGLSurface surf);
     EGLSurface GetEGLSurfaceOverride() {
         return mSurfaceOverride;
     }
 
-    virtual bool MakeCurrentImpl(bool aForce) const override;
+    virtual bool MakeCurrentImpl() const override;
 
     virtual bool IsCurrentImpl() const override;
 
     virtual bool RenewSurface(widget::CompositorWidget* aWidget) override;
 
     virtual void ReleaseSurface() override;
 
     virtual bool SetupLookupFunction() override;
--- a/gfx/gl/GLContextGLX.h
+++ b/gfx/gl/GLContextGLX.h
@@ -41,17 +41,17 @@ public:
 
     static GLContextGLX* Cast(GLContext* gl) {
         MOZ_ASSERT(gl->GetContextType() == GLContextType::GLX);
         return static_cast<GLContextGLX*>(gl);
     }
 
     bool Init() override;
 
-    virtual bool MakeCurrentImpl(bool aForce) const override;
+    virtual bool MakeCurrentImpl() const override;
 
     virtual bool IsCurrentImpl() const override;
 
     virtual bool SetupLookupFunction() override;
 
     virtual bool IsDoubleBuffered() const override;
 
     virtual bool SwapBuffers() override;
--- a/gfx/gl/GLContextProviderCGL.mm
+++ b/gfx/gl/GLContextProviderCGL.mm
@@ -105,25 +105,21 @@ GLContextCGL::Init()
 
 CGLContextObj
 GLContextCGL::GetCGLContext() const
 {
     return static_cast<CGLContextObj>([mContext CGLContextObj]);
 }
 
 bool
-GLContextCGL::MakeCurrentImpl(bool aForce) const
+GLContextCGL::MakeCurrentImpl() const
 {
-    if (!aForce && [NSOpenGLContext currentContext] == mContext) {
-        return true;
-    }
-
     if (mContext) {
         [mContext makeCurrentContext];
-        MOZ_ASSERT(IsCurrent());
+        MOZ_ASSERT(IsCurrentImpl());
         // Use non-blocking swap in "ASAP mode".
         // ASAP mode means that rendering is iterated as fast as possible.
         // ASAP mode is entered when layout.frame_rate=0 (requires restart).
         // If swapInt is 1, then glSwapBuffers will block and wait for a vblank signal.
         // When we're iterating as fast as possible, however, we want a non-blocking
         // glSwapBuffers, which will happen when swapInt==0.
         GLint swapInt = gfxPrefs::LayoutFrameRate() == 0 ? 0 : 1;
         [mContext setValues:&swapInt forParameter:NSOpenGLCPSwapInterval];
--- a/gfx/gl/GLContextProviderEAGL.mm
+++ b/gfx/gl/GLContextProviderEAGL.mm
@@ -108,22 +108,18 @@ GLContextEAGL::RecreateRB()
     fBindFramebuffer(LOCAL_GL_FRAMEBUFFER, mBackbufferFB);
     fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, LOCAL_GL_COLOR_ATTACHMENT0,
                              LOCAL_GL_RENDERBUFFER, mBackbufferRB);
 
     return LOCAL_GL_FRAMEBUFFER_COMPLETE == fCheckFramebufferStatus(LOCAL_GL_FRAMEBUFFER);
 }
 
 bool
-GLContextEAGL::MakeCurrentImpl(bool aForce) const
+GLContextEAGL::MakeCurrentImpl() const
 {
-    if (!aForce && [EAGLContext currentContext] == mContext) {
-        return true;
-    }
-
     if (mContext) {
         if(![EAGLContext setCurrentContext:mContext]) {
             return false;
         }
     }
     return true;
 }
 
--- a/gfx/gl/GLContextProviderEGL.cpp
+++ b/gfx/gl/GLContextProviderEGL.cpp
@@ -351,45 +351,37 @@ GLContextEGL::SetEGLSurfaceOverride(EGLS
     }
 
     mSurfaceOverride = surf;
     DebugOnly<bool> ok = MakeCurrent(true);
     MOZ_ASSERT(ok);
 }
 
 bool
-GLContextEGL::MakeCurrentImpl(bool aForce) const
+GLContextEGL::MakeCurrentImpl() const
 {
-    bool succeeded = true;
+    const EGLSurface surface = (mSurfaceOverride != EGL_NO_SURFACE) ? mSurfaceOverride
+                                                                    : mSurface;
+    if (surface == EGL_NO_SURFACE) {
+        MOZ_CRASH("EGL_NO_SURFACE");
+        return false;
+    }
 
-    // Assume that EGL has the same problem as WGL does,
-    // where MakeCurrent with an already-current context is
-    // still expensive.
-    bool needsMakeCurrent = (aForce || sEGLLibrary.fGetCurrentContext() != mContext);
-    if (needsMakeCurrent) {
-        EGLSurface surface = mSurfaceOverride != EGL_NO_SURFACE
-                              ? mSurfaceOverride
-                              : mSurface;
-        if (surface == EGL_NO_SURFACE) {
-            return false;
-        }
-        succeeded = sEGLLibrary.fMakeCurrent(EGL_DISPLAY(),
-                                              surface, surface,
-                                              mContext);
-        if (!succeeded) {
-            int eglError = sEGLLibrary.fGetError();
-            if (eglError == LOCAL_EGL_CONTEXT_LOST) {
-                mContextLost = true;
-                NS_WARNING("EGL context has been lost.");
-            } else {
-                NS_WARNING("Failed to make GL context current!");
+    const bool succeeded = sEGLLibrary.fMakeCurrent(EGL_DISPLAY(), surface, surface,
+                                                    mContext);
+    if (!succeeded) {
+        const auto eglError = sEGLLibrary.fGetError();
+        if (eglError == LOCAL_EGL_CONTEXT_LOST) {
+            mContextLost = true;
+            NS_WARNING("EGL context has been lost.");
+        } else {
+            NS_WARNING("Failed to make GL context current!");
 #ifdef DEBUG
-                printf_stderr("EGL Error: 0x%04x\n", eglError);
+            printf_stderr("EGL Error: 0x%04x\n", eglError);
 #endif
-            }
         }
     }
 
     return succeeded;
 }
 
 bool
 GLContextEGL::IsCurrentImpl() const
--- a/gfx/gl/GLContextProviderGLX.cpp
+++ b/gfx/gl/GLContextProviderGLX.cpp
@@ -601,45 +601,34 @@ GLContextGLX::Init()
     // so we'll also check for ARB_framebuffer_object
     if (!IsExtensionSupported(EXT_framebuffer_object) && !IsSupported(GLFeature::framebuffer_object))
         return false;
 
     return true;
 }
 
 bool
-GLContextGLX::MakeCurrentImpl(bool aForce) const
+GLContextGLX::MakeCurrentImpl() const
 {
-    bool succeeded = true;
-
-    // With the ATI FGLRX driver, glxMakeCurrent is very slow even when the context doesn't change.
-    // (This is not the case with other drivers such as NVIDIA).
-    // So avoid calling it more than necessary. Since GLX documentation says that:
-    //     "glXGetCurrentContext returns client-side information.
-    //      It does not make a round trip to the server."
-    // I assume that it's not worth using our own TLS slot here.
-    if (aForce || mGLX->fGetCurrentContext() != mContext) {
-        if (mGLX->IsMesa()) {
-          // Read into the event queue to ensure that Mesa receives a
-          // DRI2InvalidateBuffers event before drawing. See bug 1280653.
-          Unused << XPending(mDisplay);
-        }
-
-        succeeded = mGLX->fMakeCurrent(mDisplay, mDrawable, mContext);
-        NS_ASSERTION(succeeded, "Failed to make GL context current!");
-
-        if (!IsOffscreen() && mGLX->SupportsSwapControl()) {
-            // Many GLX implementations default to blocking until the next
-            // VBlank when calling glXSwapBuffers. We want to run unthrottled
-            // in ASAP mode. See bug 1280744.
-            const bool isASAP = (gfxPrefs::LayoutFrameRate() == 0);
-            mGLX->fSwapInterval(mDisplay, mDrawable, isASAP ? 0 : 1);
-        }
+    if (mGLX->IsMesa()) {
+        // Read into the event queue to ensure that Mesa receives a
+        // DRI2InvalidateBuffers event before drawing. See bug 1280653.
+        Unused << XPending(mDisplay);
     }
 
+    const bool succeeded = mGLX->fMakeCurrent(mDisplay, mDrawable, mContext);
+    NS_ASSERTION(succeeded, "Failed to make GL context current!");
+
+    if (!IsOffscreen() && mGLX->SupportsSwapControl()) {
+        // Many GLX implementations default to blocking until the next
+        // VBlank when calling glXSwapBuffers. We want to run unthrottled
+        // in ASAP mode. See bug 1280744.
+        const bool isASAP = (gfxPrefs::LayoutFrameRate() == 0);
+        mGLX->fSwapInterval(mDisplay, mDrawable, isASAP ? 0 : 1);
+    }
     return succeeded;
 }
 
 bool
 GLContextGLX::IsCurrentImpl() const
 {
     return mGLX->fGetCurrentContext() == mContext;
 }
--- a/gfx/gl/GLContextProviderWGL.cpp
+++ b/gfx/gl/GLContextProviderWGL.cpp
@@ -318,29 +318,20 @@ GLContextWGL::Init()
     SetupLookupFunction();
     if (!InitWithPrefix("gl", true))
         return false;
 
     return true;
 }
 
 bool
-GLContextWGL::MakeCurrentImpl(bool aForce) const
+GLContextWGL::MakeCurrentImpl() const
 {
-    BOOL succeeded = true;
-
-    // wglGetCurrentContext seems to just pull the HGLRC out
-    // of its TLS slot, so no need to do our own tls slot.
-    // You would think that wglMakeCurrent would avoid doing
-    // work if mContext was already current, but not so much..
-    if (aForce || sWGLLib.mSymbols.fGetCurrentContext() != mContext) {
-        succeeded = sWGLLib.mSymbols.fMakeCurrent(mDC, mContext);
-        NS_ASSERTION(succeeded, "Failed to make GL context current!");
-    }
-
+    const bool succeeded = sWGLLib.mSymbols.fMakeCurrent(mDC, mContext);
+    NS_ASSERTION(succeeded, "Failed to make GL context current!");
     return succeeded;
 }
 
 bool
 GLContextWGL::IsCurrentImpl() const
 {
     return sWGLLib.mSymbols.fGetCurrentContext() == mContext;
 }
--- a/gfx/gl/GLContextWGL.h
+++ b/gfx/gl/GLContextWGL.h
@@ -40,17 +40,17 @@ public:
 
     static GLContextWGL* Cast(GLContext* gl) {
         MOZ_ASSERT(gl->GetContextType() == GLContextType::WGL);
         return static_cast<GLContextWGL*>(gl);
     }
 
     bool Init() override;
 
-    virtual bool MakeCurrentImpl(bool aForce) const override;
+    virtual bool MakeCurrentImpl() const override;
 
     virtual bool IsCurrentImpl() const override;
 
     void SetIsDoubleBuffered(bool aIsDB);
 
     virtual bool IsDoubleBuffered() const override;
 
     virtual bool SwapBuffers() override;