Merge lp:~nick-dedekind/qtubuntu/bad-context into lp:qtubuntu

[Nick Dedekind]

Moved setSwapInterval to glContext to fix bad context. Added better logging for egl failures.

Moved setSwapInterval to glContext to fix bad context. Added better logging for egl failures.

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:317
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/34/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1297
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1268
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1268
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1266
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1266/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1266
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1266/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1266
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1266/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1266
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1266/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1266
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1266/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1266
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1266/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/34/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Code equivalent to that in Qt's QEglPlatformContext [1], so can't complain. Have you tested all devices to be sure it has zero impact, just in case?

+++ src/ubuntumirclient/window.cpp
+ if (!ok) return;
Let's print an error instead of just ignoring

[1] https://codereview.qt-project.org/#/c/62055/

review: Needs Fixing

[Gerry Boland (gerboland)]

Thanks for this, but I inadvertently ended up fixing this in this refactoring:
https://code.launchpad.net/~gerboland/qtubuntu/adopt-more-eglconvenience2/+merge/295248

Unmerged revisions

317. By Nick Dedekind on 2016-04-06

removed global

316. By Nick Dedekind on 2016-04-05

dont onBufferswapDone if failed swap

315. By Nick Dedekind on 2016-04-05

merged crossbuild

314. By Nick Dedekind on 2016-04-05

context fixes

=== modified file 'src/ubuntumirclient/glcontext.cpp'
--- src/ubuntumirclient/glcontext.cpp	2016-01-04 17:18:51 +0000
+++ src/ubuntumirclient/glcontext.cpp	2016-04-07 14:36:32 +0000
@@ -49,6 +49,9 @@
 }
 
 UbuntuOpenGLContext::UbuntuOpenGLContext(UbuntuScreen* screen, UbuntuOpenGLContext* share)
+    : m_swapInterval(-1)
+    , m_swapIntervalEnvChecked(false)
+    , m_swapIntervalFromEnv(-1)
 {
     ASSERT(screen != NULL);
     mEglDisplay = screen->eglDisplay();
@@ -68,7 +71,9 @@
 
 UbuntuOpenGLContext::~UbuntuOpenGLContext()
 {
-    ASSERT(eglDestroyContext(mEglDisplay, mEglContext) == EGL_TRUE);
+    if (mEglContext != EGL_NO_CONTEXT) {
+        ASSERT(eglDestroyContext(mEglDisplay, mEglContext) == EGL_TRUE);
+    }
 }
 
 bool UbuntuOpenGLContext::makeCurrent(QPlatformSurface* surface)
@@ -83,29 +88,70 @@
         }
         return offscreen->buffer()->bind();
     } else {
+        ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
+
         EGLSurface eglSurface = static_cast<UbuntuWindow*>(surface)->eglSurface();
-        ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
-        ASSERT(eglMakeCurrent(mEglDisplay, eglSurface, eglSurface, mEglContext) == EGL_TRUE);
+
+        // shortcut: on some GPUs, eglMakeCurrent is not a cheap operation
+        if (eglGetCurrentContext() == mEglContext &&
+            eglGetCurrentDisplay() == mEglDisplay &&
+            eglGetCurrentSurface(EGL_READ) == eglSurface &&
+            eglGetCurrentSurface(EGL_DRAW) == eglSurface) {
+            return true;
+        }
+
+        const bool ok = eglMakeCurrent(mEglDisplay, eglSurface, eglSurface, mEglContext);
+        if (ok) {
+            if (!m_swapIntervalEnvChecked) {
+                m_swapIntervalEnvChecked = true;
+                if (qEnvironmentVariableIsSet("QTUBUNTU_SWAPINTERVAL")) {
+                    QByteArray swapIntervalString = qgetenv("QTUBUNTU_SWAPINTERVAL");
+                    bool intervalOk;
+                    const int swapInterval = swapIntervalString.toInt(&intervalOk);
+                    if (intervalOk) {
+                        m_swapIntervalFromEnv = swapInterval;
+                    }
+                }
+            }
+            const int requestedSwapInterval = m_swapIntervalFromEnv >= 0 ? m_swapIntervalFromEnv
+                                                                         : surface->format().swapInterval();
+            if (requestedSwapInterval >= 0 && m_swapInterval != requestedSwapInterval) {
+                m_swapInterval = requestedSwapInterval;
+                eglSwapInterval(mEglDisplay, m_swapInterval);
+            }
+        } else {
+            qCWarning(ubuntumirclient, "UbuntuOpenGLContext::makeCurrent: eglError: %x, this: %p", eglGetError(), this);
+        }
+
         if (ubuntumirclient().isDebugEnabled()) {
             printOpenGLESConfig();
         }
-        return true;
+        return ok;
     }
+    return false;
 }
 
 void UbuntuOpenGLContext::doneCurrent()
 {
     ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
-    ASSERT(eglMakeCurrent(mEglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT) == EGL_TRUE);
+
+    bool ok = eglMakeCurrent(mEglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
+    if (!ok) {
+        qCWarning(ubuntumirclient, "UbuntuOpenGLContext::doneCurrent(): eglError: %d, this: %p", eglGetError(), this);
+    }
 }
 
 void UbuntuOpenGLContext::swapBuffers(QPlatformSurface* surface)
 {
     UbuntuWindow *ubuntuWindow = static_cast<UbuntuWindow*>(surface);
 
+    ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
     EGLSurface eglSurface = ubuntuWindow->eglSurface();
-    ASSERT(eglBindAPI(api_in_use()) == EGL_TRUE);
-    ASSERT(eglSwapBuffers(mEglDisplay, eglSurface) == EGL_TRUE);
+    bool ok = eglSwapBuffers(mEglDisplay, eglSurface);
+    if (!ok) {
+        qCWarning(ubuntumirclient, "UbuntuOpenGLContext::swapBuffers(): eglError: %d, this: %p", eglGetError(), this);
+        return;
+    }
 
     ubuntuWindow->onSwapBuffersDone();
 }

=== modified file 'src/ubuntumirclient/glcontext.h'
--- src/ubuntumirclient/glcontext.h	2015-09-29 13:09:01 +0000
+++ src/ubuntumirclient/glcontext.h	2016-04-07 14:36:32 +0000
@@ -40,6 +40,10 @@
     UbuntuScreen* mScreen;
     EGLContext mEglContext;
     EGLDisplay mEglDisplay;
+
+    int m_swapInterval;
+    bool m_swapIntervalEnvChecked;
+    int m_swapIntervalFromEnv;
 };
 
 #endif // UBUNTU_OPENGL_CONTEXT_H

=== modified file 'src/ubuntumirclient/screen.cpp'
--- src/ubuntumirclient/screen.cpp	2015-12-09 13:01:28 +0000
+++ src/ubuntumirclient/screen.cpp	2016-04-07 14:36:32 +0000
@@ -31,8 +31,6 @@
 
 #include <memory>
 
-static const int kSwapInterval = 1;
-
 static const char *orientationToStr(Qt::ScreenOrientation orientation) {
     switch (orientation) {
         case Qt::PrimaryOrientation:
@@ -163,18 +161,6 @@
         printEglConfig(mEglDisplay, mEglConfig);
     }
 
-    // Set vblank swap interval.
-    int swapInterval = kSwapInterval;
-    QByteArray swapIntervalString = qgetenv("QTUBUNTU_SWAPINTERVAL");
-    if (!swapIntervalString.isEmpty()) {
-        bool ok;
-        swapInterval = swapIntervalString.toInt(&ok);
-        if (!ok)
-            swapInterval = kSwapInterval;
-    }
-    qCDebug(ubuntumirclient, "setting swap interval to %d", swapInterval);
-    eglSwapInterval(mEglDisplay, swapInterval);
-
     // Get screen resolution.
     auto configDeleter = [](MirDisplayConfiguration *config) { mir_display_config_destroy(config); };
     using configUp = std::unique_ptr<MirDisplayConfiguration, decltype(configDeleter)>;

=== modified file 'src/ubuntumirclient/window.cpp'
--- src/ubuntumirclient/window.cpp	2016-03-14 17:37:19 +0000
+++ src/ubuntumirclient/window.cpp	2016-04-07 14:36:32 +0000
@@ -438,8 +438,9 @@
 
     EGLint eglSurfaceWidth = -1;
     EGLint eglSurfaceHeight = -1;
-    eglQuerySurface(mEglDisplay, mEglSurface, EGL_WIDTH, &eglSurfaceWidth);
-    eglQuerySurface(mEglDisplay, mEglSurface, EGL_HEIGHT, &eglSurfaceHeight);
+    bool ok = eglQuerySurface(mEglDisplay, mEglSurface, EGL_WIDTH, &eglSurfaceWidth);
+    ok &= eglQuerySurface(mEglDisplay, mEglSurface, EGL_HEIGHT, &eglSurfaceHeight);
+    if (!ok) return;
 
     const bool validSize = eglSurfaceWidth > 0 && eglSurfaceHeight > 0;