Merge lp:~dandrader/qtmir/dontOccludeFreshSurfaces into lp:qtmir

[Daniel d'Andrada]

Don't occlude newly created surfaces until some time has passed

Ensures apps are able to render the initial state of their UIs straight away.

The main use case is the dash being created behing the greeter/lockscreen (and therefore occluded) on device start up.

* Are there any related MPs required for this MP to build/function as expected? Please list.
No. Had exposing the age property left as a TODO exactly to avoid the pain of having companion unity-api and unity8 branches, which would, besides slowing down the release of the fix, also cause new conflicts on the ongoing https://requests.ci-train.ubuntu.com/#/ticket/1636

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

[Daniel d'Andrada (dandrader)]

Turns out dash takes longer than I expected to get itself together during device startup, hence the 10 seconds, which has some spare room. Tried 3.5 seconds before and it wasn't enough, for instance (on mako).

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

PASSED: Continuous integration, rev:559
https://unity8-jenkins.ubuntu.com/job/lp-qtmir-ci/367/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2760
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2788
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2648/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2648/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2648/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2648/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2648/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2648/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2648/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2648/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2648
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2648/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

+ // TODO: Make it configurable, exposing it as a QML property to shell.
I'm not keen on this, as it pushes the responsibility to the shell. How can shell ever get this value right? IMO It's the apps problem that it isn't drawing a perfect first frame - and I don't understand why Dash/Qt is failing to do this.

However in the meantime, this fix does do the job. Let's use it for now

review: Approve

=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
--- src/modules/Unity/Application/mirsurface.cpp	2016-08-30 12:24:19 +0000
+++ src/modules/Unity/Application/mirsurface.cpp	2016-09-06 18:20:32 +0000
@@ -251,6 +251,11 @@
     QQmlEngine::setObjectOwnership(this, QQmlEngine::CppOwnership);
 
     setCloseTimer(new Timer);
+
+    QTimer::singleShot(m_minimumAgeForOcclusion, this, [this]() {
+        m_oldEnoughToBeOccluded = true;
+        updateVisibility();
+    });
 }
 
 MirSurface::~MirSurface()
@@ -787,10 +792,17 @@
 void MirSurface::updateVisibility()
 {
     bool newVisible = false;
-    QHashIterator<qintptr, View> i(m_views);
-    while (i.hasNext()) {
-        i.next();
-        newVisible |= i.value().visible;
+
+    if (m_oldEnoughToBeOccluded) {
+        QHashIterator<qintptr, View> i(m_views);
+        while (i.hasNext()) {
+            i.next();
+            newVisible |= i.value().visible;
+        }
+    } else {
+        // Surface is too young to get occluded. Let it remain exposed for a bit to ensure that it displays
+        // a properly formed UI on start up.
+        newVisible = true;
     }
 
     if (newVisible != visible()) {

=== modified file 'src/modules/Unity/Application/mirsurface.h'
--- src/modules/Unity/Application/mirsurface.h	2016-08-30 12:24:19 +0000
+++ src/modules/Unity/Application/mirsurface.h	2016-09-06 18:20:32 +0000
@@ -22,6 +22,7 @@
 
 // Qt
 #include <QCursor>
+#include <QElapsedTimer>
 #include <QMutex>
 #include <QPointer>
 #include <QRect>
@@ -246,6 +247,11 @@
     };
     ClosingState m_closingState{NotClosing};
     AbstractTimer *m_closeTimer{nullptr};
+
+    // TODO: Make it configurable, exposing it as a QML property to shell.
+    // In milliseconds.
+    const int m_minimumAgeForOcclusion{10000};
+    bool m_oldEnoughToBeOccluded{false};
 };
 
 } // namespace qtmir