Merge lp:~albaguirre/qtmir/hook-done-current into lp:qtmir

[Alberto Aguirre]

Hook MirOpenGLContext::doneCurrent to Screen::doneCurrent

Actually unbind the opengl/egl context when requested.

When the display configuration changes, the Qt compositor is stopped and ScreenWindows::setExposed(false) is called which eventually makes the renderer thread call MirOpenGLContext::doneCurrent - which does nothing.

After restarting the compositor, new rendering threads are created and eventually MirOpenGLContext::makeCurrent will be called on the new QPlatformSurface/ScreenWindow objects, which will eventually result in an eglMakeCurrent call.

Since doneCurrent does not unbind the context from the previous rendering thread, some drivers (for example mx4) will return EGL_BAD_ACCESS (with good reason, since the context is bound to another thread).

Hook MirOpenGLContext::doneCurrent so that it eventually calls Screen::doneCurrent() to properly unbind the context.

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

FAILED: Continuous integration, rev:439
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/65/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/408/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/431
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/449
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/449
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/445/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/445/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/445/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/445/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/445/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/445/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-1-ci/65/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:439
http://jenkins.qa.ubuntu.com/job/qtmir-ci/686/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/380
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/382
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/382/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/262
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/417/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/419/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/262/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/686/rebuild

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

I need to convince myself that the window will either live longer than the glcontext, or will set m_currentWindow null correctly.

[Alberto Aguirre (albaguirre)]

Logically, I hope that the window outlives the gl context as if you are tearing down, you probably don't want to tear down a surface/window while there's a context active.

[Gerry Boland (gerboland)]

Yeah, I think this is ok, for all shell needs it for. Thanks

review: Approve

=== modified file 'src/platforms/mirserver/miropenglcontext.cpp'
--- src/platforms/mirserver/miropenglcontext.cpp	2015-09-14 22:38:20 +0000
+++ src/platforms/mirserver/miropenglcontext.cpp	2016-02-05 22:58:24 +0000
@@ -36,8 +36,9 @@
 // (i.e. individual display output buffers) to use as a common base context.
 
 MirOpenGLContext::MirOpenGLContext(const QSharedPointer<MirServer> &server, const QSurfaceFormat &format)
+    : m_currentWindow(nullptr)
 #ifndef QT_NO_DEBUG
-    : m_logger(new QOpenGLDebugLogger(this))
+      , m_logger(new QOpenGLDebugLogger(this))
 #endif
 {
     auto display = server->the_display();
@@ -131,6 +132,7 @@
     // ultimately calls Mir's DisplayBuffer::make_current()
     ScreenWindow *screenWindow = static_cast<ScreenWindow*>(surface);
     if (screenWindow) {
+        m_currentWindow = screenWindow;
         screenWindow->makeCurrent();
 
 #ifndef QT_NO_DEBUG
@@ -148,7 +150,10 @@
 
 void MirOpenGLContext::doneCurrent()
 {
-    // FIXME: create a temporary GL context just to release? Would be better to get existing one.
+    if (m_currentWindow) {
+        m_currentWindow->doneCurrent();
+        m_currentWindow = nullptr;
+    }
 }
 
 QFunctionPointer MirOpenGLContext::getProcAddress(const QByteArray &procName)

=== modified file 'src/platforms/mirserver/miropenglcontext.h'
--- src/platforms/mirserver/miropenglcontext.h	2015-09-14 22:38:20 +0000
+++ src/platforms/mirserver/miropenglcontext.h	2016-02-05 22:58:24 +0000
@@ -25,6 +25,7 @@
 
 
 class MirServer;
+class ScreenWindow;
 
 class MirOpenGLContext : public QObject, public QPlatformOpenGLContext
 {
@@ -49,6 +50,7 @@
 
 private:
     QSurfaceFormat m_format;
+    ScreenWindow *m_currentWindow;
 #ifndef QT_NO_DEBUG
     QOpenGLDebugLogger *m_logger;
 #endif