Merge lp:~gerboland/qtmir/fix-glcontext-loss-on-reconfigure into lp:qtmir

[Gerry Boland]

ScreenModel: Only expose windows on displays that are turned on

It seems the GL context Mir gives us for a display that is turned off is invalid. But there is no point rendering to a display that is turned off anyway.

[Gerry Boland (gerboland)]

Handy test, grab
https://code.launchpad.net/~gerboland/+git/display-configurator
Run it with
./display-configurator -- --desktop_file_hint=gedit
and hit the letter "O" - big o. (I find you need to click on the surface for it to get input correctly)

The display should turn off.

P isn't turning it on yet - need to figure out why, if input is disabled or if QtMir isn't passing input to non-exposed windows or something.

To turn display on, kill the display-configurator process. Mir/U8 should revert to the previous state

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

PASSED: Continuous integration, rev:587
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/449/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3847
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3875
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3718
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3718/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3718
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3718/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3718
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3718/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3718
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3718/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3718
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3718/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3718
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3718/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

should probably remove the binary and object file from the git repo of display-configurator

[Daniel d'Andrada (dandrader)]

Code looks ok. Passes manual test.

review: Approve

=== modified file 'src/platforms/mirserver/screensmodel.cpp'
--- src/platforms/mirserver/screensmodel.cpp	2016-11-03 20:17:46 +0000
+++ src/platforms/mirserver/screensmodel.cpp	2017-01-18 18:40:03 +0000
@@ -79,10 +79,9 @@
     qCDebug(QTMIR_SCREENS) << "ScreensModel::onCompositorStarting";
     m_compositing = true;
 
-    update();
+    update(); // must handle all hardware changes before starting the renderer
 
-    // (Re)Start Qt's render thread by setting all windows with a corresponding screen to exposed.
-    allWindowsSetExposed(true);
+    startRenderer();
 }
 
 void ScreensModel::onCompositorStopping()
@@ -90,9 +89,7 @@
     qCDebug(QTMIR_SCREENS) << "ScreensModel::onCompositorStopping";
     m_compositing = false;
 
-    // Stop Qt's render threads by setting all its windows it obscured. Must
-    // block until all windows have their GL contexts released.
-    allWindowsSetExposed(false);
+    haltRenderer(); // must stop all rendering before handling any hardware changes
 
     update();
 }
@@ -221,12 +218,36 @@
     return canUpdateExisting;
 }
 
-void ScreensModel::allWindowsSetExposed(bool exposed)
+/*
+ * ScreensModel::startRenderer()
+ * (Re)Start Qt's render thread by setting all windows with a corresponding screen to exposed.
+ * It is asynchronous, it returns before the render thread(s) have started.
+ */
+void ScreensModel::startRenderer()
+{
+    Q_FOREACH (const auto screen, m_screenList) {
+        // Only set windows exposed on displays which are turned on, as the GL context Mir provided
+        // is invalid in that situation
+        if (screen->powerMode() == mir_power_mode_on) {
+            const auto window = static_cast<ScreenWindow *>(screen->window());
+            if (window && window->window()) {
+                window->setExposed(true);
+            }
+        }
+    }
+}
+
+/*
+ * ScreensModel::haltRenderer()
+ * Stop Qt's render thread(s) by setting all windows with a corresponding screen to not exposed.
+ * It is blocking, it returns after the render thread(s) have all stopped.
+ */
+void ScreensModel::haltRenderer()
 {
     Q_FOREACH (const auto screen, m_screenList) {
         const auto window = static_cast<ScreenWindow *>(screen->window());
         if (window && window->window()) {
-            window->setExposed(exposed);
+            window->setExposed(false);
         }
     }
 }

=== modified file 'src/platforms/mirserver/screensmodel.h'
--- src/platforms/mirserver/screensmodel.h	2016-11-03 20:17:46 +0000
+++ src/platforms/mirserver/screensmodel.h	2017-01-18 18:40:03 +0000
@@ -90,7 +90,8 @@
 private:
     Screen* findScreenWithId(const QList<Screen*> &list, const mir::graphics::DisplayConfigurationOutputId id);
     bool canUpdateExistingScreen(const Screen *screen, const mir::graphics::DisplayConfigurationOutput &output);
-    void allWindowsSetExposed(bool exposed);
+    void startRenderer();
+    void haltRenderer();
 
     std::weak_ptr<mir::graphics::Display> m_display;
     std::shared_ptr<QtCompositor> m_compositor;

=== modified file 'src/platforms/mirserver/screenwindow.cpp'
--- src/platforms/mirserver/screenwindow.cpp	2016-06-07 20:52:43 +0000
+++ src/platforms/mirserver/screenwindow.cpp	2017-01-18 18:40:03 +0000
@@ -109,7 +109,6 @@
     myScreen->setWindow(this);
 
     QWindowSystemInterface::handleWindowScreenChanged(window(), myScreen->screen());
-    setExposed(true); //GERRY - assumption setScreen only called while compositor running
 
     qCDebug(QTMIR_SCREENS) << "ScreenWindow" << this << "with window ID" << uint(m_winId) << "NEWLY backed by" << myScreen;
 }