Merge lp:~nick-dedekind/qtubuntu/lp1475678.surface-occlude into lp:qtubuntu

[Nick Dedekind]

Support server->client visibility change to stop rendering (lp:#1475678)

Support server->client visibility change to stop rendering (lp:#1475678)

related MPs:
https://code.launchpad.net/~nick-dedekind/unity-api/lp1475678.surface-occlude/+merge/273425
https://code.launchpad.net/~nick-dedekind/unity8/lp1475678.surface-occlude/+merge/273427
https://code.launchpad.net/~nick-dedekind/qtmir/lp1475678.surface-occlude/+merge/273426

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:283
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/254/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/45
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/45/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

In src/ubuntumirclient/window.cpp:

"""
+ window()->setVisible(exposed);
"""

You sure this is necessary?

It doesn't look right that a backend is calling the client-side API. Since, QWindow::visible property is under control of application code, it would be weird that it suddenly turn false without app code having explicitly set it (either via setVisible(false) or hide()).

review: Needs Information

[lp:~nick-dedekind/qtubuntu/lp1475678.surface-occlude]

284. By Nick Dedekind on 2015-10-12

better exposure handling

[Nick Dedekind (nick-dedekind)]

> In src/ubuntumirclient/window.cpp:
>
> """
> + window()->setVisible(exposed);
> """
>
> You sure this is necessary?
>
> It doesn't look right that a backend is calling the client-side API. Since,
> QWindow::visible property is under control of application code, it would be
> weird that it suddenly turn false without app code having explicitly set it
> (either via setVisible(false) or hide()).

Apparently not. Looks like it's the isExposed that stops the redraw.
I've fixed it up now.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:284
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/261/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/52
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/52/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

Code looks ok. Didn't test yet.

review: Approve (code)

[Daniel d'Andrada (dandrader)]

And works fine.

review: Approve

[Daniel d'Andrada (dandrader)]

review: Approve

[lp:~nick-dedekind/qtubuntu/lp1475678.surface-occlude]

285. By Nick Dedekind on 2015-10-26

merge with trunk

286. By Nick Dedekind on 2015-11-05

Exposure initialization

[Daniel d'Andrada (dandrader)]

something like pendingOcclusion or occlusionCatchup would be clearer than exposeCatchUp as this is meant specifically to occlude the window. Corner cases like this should be well explained.

review: Needs Fixing

[Nick Dedekind (nick-dedekind)]

> something like pendingOcclusion or occlusionCatchup would be clearer than
> exposeCatchUp as this is meant specifically to occlude the window. Corner
> cases like this should be well explained.

Done and added a comment explaining.

review: Disapprove

[Nick Dedekind (nick-dedekind)]

review: Abstain

[Daniel d'Andrada (dandrader)]

Thanks! But looks like it was too late. This last change didn't show up in the merged branch.

=== modified file 'src/ubuntumirclient/input.cpp'
--- src/ubuntumirclient/input.cpp	2015-10-21 11:47:48 +0000
+++ src/ubuntumirclient/input.cpp	2015-11-06 09:06:33 +0000
@@ -232,9 +232,15 @@
     case mir_event_type_surface:
     {
         auto surfaceEvent = mir_event_get_surface_event(nativeEvent);
-        if (mir_surface_event_get_attribute(surfaceEvent) == mir_surface_attrib_focus) {
+        auto surfaceEventAttribute = mir_surface_event_get_attribute(surfaceEvent);
+
+        if (surfaceEventAttribute == mir_surface_attrib_focus) {
             ubuntuEvent->window->handleSurfaceFocusChange(mir_surface_event_get_attribute_value(surfaceEvent) ==
                 mir_surface_focused);
+
+        } else if (surfaceEventAttribute == mir_surface_attrib_visibility) {
+            ubuntuEvent->window->handleSurfaceExposeChange(mir_surface_event_get_attribute_value(surfaceEvent) ==
+                                                           mir_surface_visibility_exposed);
         }
         break;
     }

=== modified file 'src/ubuntumirclient/window.cpp'
--- src/ubuntumirclient/window.cpp	2015-09-29 18:10:51 +0000
+++ src/ubuntumirclient/window.cpp	2015-11-06 09:06:33 +0000
@@ -99,7 +99,9 @@
     QSize bufferSize;
     QMutex mutex;
     QSharedPointer<UbuntuClipboard> clipboard;
+    bool exposed;
     int resizeCatchUpAttempts;
+    bool exposeCatchUp;
 #if !defined(QT_NO_DEBUG)
     int frameNumber;
 #endif
@@ -136,6 +138,8 @@
     d->connection = connection;
     d->clipboard = clipboard;
     d->resizeCatchUpAttempts = 0;
+    d->exposed = true;
+    d->exposeCatchUp = false;
 
     static int id = 1;
     d->id = id++;
@@ -283,6 +287,7 @@
     // Create platform window
     mir_wait_for(mir_surface_create(spec, surfaceCreateCallback, this));
     mir_surface_spec_release(spec);
+    d->exposeCatchUp = mir_surface_get_visibility(d->surface) == mir_surface_visibility_occluded;
 
     DASSERT(d->surface != NULL);
     d->createEGLSurface((EGLNativeWindowType)mir_buffer_stream_get_egl_native_window(mir_surface_get_buffer_stream(d->surface)));
@@ -339,7 +344,7 @@
         // swap more than once to get a buffer with the new size!
         d->resizeCatchUpAttempts = 2;
 
-        QWindowSystemInterface::handleExposeEvent(window(), geometry());
+        QWindowSystemInterface::handleExposeEvent(window(), d->exposed ? QRect(QPoint(), geometry().size()) : QRect());
         QWindowSystemInterface::flushWindowSystemEvents();
     }
 }
@@ -362,6 +367,19 @@
     QWindowSystemInterface::handleWindowActivated(activatedWindow, Qt::ActiveWindowFocusReason);
 }
 
+void UbuntuWindow::handleSurfaceExposeChange(bool exposed)
+{
+    QMutexLocker(&d->mutex);
+    DLOG("UbuntuWindow::handleSurfaceExposeChange(exposed=%s)", exposed ? "true" : "false");
+
+    if (d->exposed != exposed) {
+        d->exposed = exposed;
+
+        QWindowSystemInterface::handleExposeEvent(window(), d->exposed ? QRect(QPoint(), geometry().size()) : QRect());
+        QWindowSystemInterface::flushWindowSystemEvents();
+    }
+}
+
 void UbuntuWindow::setWindowState(Qt::WindowState state)
 {
     QMutexLocker(&d->mutex);
@@ -399,14 +417,19 @@
 
     if (visible) {
         mir_wait_for(mir_surface_set_state(d->surface, qtWindowStateToMirSurfaceState(d->state)));
-
-        QWindowSystemInterface::handleExposeEvent(window(), QRect());
-        QWindowSystemInterface::flushWindowSystemEvents();
     } else {
         // TODO: Use the new mir_surface_state_hidden state instead of mir_surface_state_minimized.
         //       Will have to change qtmir and unity8 for that.
         mir_wait_for(mir_surface_set_state(d->surface, mir_surface_state_minimized));
     }
+
+    QWindowSystemInterface::handleExposeEvent(window(), d->exposed ? QRect(QPoint(), geometry().size()) : QRect());
+    QWindowSystemInterface::flushWindowSystemEvents();
+}
+
+bool UbuntuWindow::isExposed() const
+{
+    return d->exposed && window()->isVisible();
 }
 
 void UbuntuWindow::setWindowTitle(const QString &title)
@@ -437,6 +460,13 @@
     ++d->frameNumber;
 #endif
 
+    bool exposureChanged = false;
+    if (d->exposeCatchUp) {
+        d->exposeCatchUp = false;
+        d->exposed = false;
+        exposureChanged = true;
+    }
+
     if (sizeKnown && (d->bufferSize.width() != newBufferWidth ||
                 d->bufferSize.height() != newBufferHeight)) {
         d->resizeCatchUpAttempts = 0;
@@ -462,9 +492,13 @@
         DLOG("UbuntuWindow::onBuffersSwapped_threadSafe [%d] - buffer size (%d,%d). Redrawing to catch up a resized buffer."
                " resizeCatchUpAttempts=%d",
                d->frameNumber, d->bufferSize.width(), d->bufferSize.height(), d->resizeCatchUpAttempts);
-        QWindowSystemInterface::handleExposeEvent(window(), geometry());
+        QWindowSystemInterface::handleExposeEvent(window(), d->exposed ? QRect(QPoint(), geometry().size()) : QRect());
     } else {
         DLOG("UbuntuWindow::onBuffersSwapped_threadSafe [%d] - buffer size (%d,%d). resizeCatchUpAttempts=%d",
                d->frameNumber, d->bufferSize.width(), d->bufferSize.height(), d->resizeCatchUpAttempts);
+
+        if (exposureChanged) {
+            QWindowSystemInterface::handleExposeEvent(window(), d->exposed ? QRect(QPoint(), geometry().size()) : QRect());
+        }
     }
 }

=== modified file 'src/ubuntumirclient/window.h'
--- src/ubuntumirclient/window.h	2015-09-29 18:10:51 +0000
+++ src/ubuntumirclient/window.h	2015-11-06 09:06:33 +0000
@@ -40,12 +40,14 @@
     void setGeometry(const QRect&) override;
     void setWindowState(Qt::WindowState state) override;
     void setVisible(bool visible) override;
+    bool isExposed() const override;
     void setWindowTitle(const QString &title) override;
 
     // New methods.
     void* eglSurface() const;
     void handleSurfaceResize(int width, int height);
     void handleSurfaceFocusChange(bool focused);
+    void handleSurfaceExposeChange(bool visible);
     void onBuffersSwapped_threadSafe(int newBufferWidth, int newBufferHeight);
 
     UbuntuWindowPrivate* priv() { return d; }