Merge lp:~dandrader/qtubuntu/improve-resize into lp:qtubuntu

[Daniel d'Andrada]

Drastically improve surface resize responsiveness

QWindowSystemInterface::handle[...]() methods are thread safe, thus there's no need to have intermetiate methods between the rendering thread and such calls.

There was also some miscommunication between the now absent handleBufferResize() slot from the main thread and the swapbuffer handler from the render thread.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:259
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/208/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/34
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/34/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

=== modified file 'src/ubuntumirclient/input.cpp'
+/*
+*/
necessary change?

In testing with mir_proving_server, I notice one problem: the surface size QML requests is not the size it gets. Think the geometry.setY() call is problematic now. This used to work before.

Remainder looks good. Am worried that there are 3+ threads (Qt GUI, Qt renderers, Mir input) all interacting with this class tho.

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

On 13/05/15 09:12, Gerry Boland wrote:
> === modified file 'src/ubuntumirclient/input.cpp'
> +/*
> +*/
> necessary change?

Yes, code won't build otherwise in a debug config. I've the same change
in another top-approved-but-not-yet-landed MP.

[Daniel d'Andrada (dandrader)]

> In testing with mir_proving_server, I notice one problem: the surface size QML
> requests is not the size it gets.

This also happens with lp:qtubuntu.

I request (w=600,h=600) and get a geometry of (x=0,y=26,w=600,h=574). It's interesting that h+y=600. That's with mir_proving_server.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

LGTM

review: Approve

[lp:~dandrader/qtubuntu/improve-resize]

261. By Daniel d'Andrada on 2015-06-17

Refactor optional logging and remove trailing whitespace

[Gerry Boland (gerboland)]

review: Approve

=== modified file 'src/ubuntumirclient/input.cpp'
--- src/ubuntumirclient/input.cpp	2015-06-11 09:10:55 +0000
+++ src/ubuntumirclient/input.cpp	2015-06-17 13:08:26 +0000
@@ -157,9 +157,7 @@
   // Qt will take care of deleting mTouchDevice.
 }
 
-#ifndef QT_NO_DEBUG
-
-
+#if (LOG_EVENTS != 0)
 static const char* nativeEventTypeToStr(MirEventType t)
 {
     switch (t)
@@ -185,9 +183,7 @@
         return "invalid";
     }
 }
-
-
-#endif
+#endif // LOG_EVENTS != 0
 
 void UbuntuInput::customEvent(QEvent* event)
 {
@@ -209,10 +205,12 @@
         return;
     }
 
-    //DLOG("UbuntuInput::customEvent(type=%s)", nativeEventTypeToStr(mir_event_get_type(nativeEvent)));
+    #if (LOG_EVENTS != 0)
+    LOG("UbuntuInput::customEvent(type=%s)", nativeEventTypeToStr(mir_event_get_type(nativeEvent)));
+    #endif
 
     // Event dispatching.
-    switch (mir_event_get_type(nativeEvent)) 
+    switch (mir_event_get_type(nativeEvent))
     {
     case mir_event_type_input:
         dispatchInputEvent(ubuntuEvent->window->window(), mir_event_get_input_event(nativeEvent));
@@ -429,7 +427,7 @@
 
     auto local_point = QPointF(mir_pointer_event_axis_value(pev, mir_pointer_axis_x),
                                mir_pointer_event_axis_value(pev, mir_pointer_axis_y));
-    
+
     QWindowSystemInterface::handleMouseEvent(window, timestamp, local_point, local_point /* Should we omit global point instead? */,
                                              buttons, modifiers);
 }

=== modified file 'src/ubuntumirclient/window.cpp'
--- src/ubuntumirclient/window.cpp	2015-05-26 21:11:43 +0000
+++ src/ubuntumirclient/window.cpp	2015-06-17 13:08:26 +0000
@@ -96,7 +96,6 @@
     WId id;
     UbuntuInput* input;
     Qt::WindowState state;
-    QRect geometry;
     MirConnection *connection;
     MirSurface* surface;
     QSize bufferSize;
@@ -140,8 +139,8 @@
     d->id = id++;
 
     // Use client geometry if set explicitly, use available screen geometry otherwise.
-    d->geometry = window()->geometry() != screen->geometry() ?
-        window()->geometry() : screen->availableGeometry();
+    QPlatformWindow::setGeometry(window()->geometry() != screen->geometry() ?
+        window()->geometry() : screen->availableGeometry());
     createWindow();
     DLOG("UbuntuWindow::UbuntuWindow (this=%p, w=%p, screen=%p, input=%p)", this, w, screen, input);
 }
@@ -253,7 +252,7 @@
         geometry.setY(panelHeight);
     } else {
         printf("UbuntuWindow - regular geometry\n");
-        geometry = d->geometry;
+        geometry = this->geometry();
         geometry.setY(panelHeight);
     }
 
@@ -314,22 +313,16 @@
 
 void UbuntuWindow::handleSurfaceResize(int width, int height)
 {
+    QMutexLocker(&d->mutex);
     LOG("UbuntuWindow::handleSurfaceResize(width=%d, height=%d)", width, height);
 
     // The current buffer size hasn't actually changed. so just render on it and swap
     // buffers until we render on a buffer with the target size.
 
-    bool shouldSwapBuffers;
-
-    {
-        QMutexLocker(&d->mutex);
-        d->targetBufferSize.rwidth() = width;
-        d->targetBufferSize.rheight() = height;
-
-        shouldSwapBuffers = d->bufferSize != d->targetBufferSize;
-    }
-
-    if (shouldSwapBuffers) {
+    d->targetBufferSize.rwidth() = width;
+    d->targetBufferSize.rheight() = height;
+
+    if (d->bufferSize != d->targetBufferSize) {
         QWindowSystemInterface::handleExposeEvent(window(), geometry());
     } else {
         qWarning("[ubuntumirclient QPA] UbuntuWindow::handleSurfaceResize"
@@ -356,35 +349,6 @@
     QWindowSystemInterface::handleWindowActivated(activatedWindow, Qt::ActiveWindowFocusReason);
 }
 
-void UbuntuWindow::handleBufferResize(int width, int height)
-{
-    DLOG("UbuntuWindow::handleBufferResize(width=%d, height=%d)", width, height);
-
-    QRect oldGeometry;
-    QRect newGeometry;
-
-    {
-        QMutexLocker(&d->mutex);
-        oldGeometry = geometry();
-        newGeometry = oldGeometry;
-        newGeometry.setWidth(width);
-        newGeometry.setHeight(height);
-
-        d->bufferSize.rwidth() = width;
-        d->bufferSize.rheight() = height;
-        d->geometry = newGeometry;
-    }
-
-    QPlatformWindow::setGeometry(newGeometry);
-    QWindowSystemInterface::handleGeometryChange(window(), newGeometry, oldGeometry);
-    QWindowSystemInterface::handleExposeEvent(window(), newGeometry);
-}
-
-void UbuntuWindow::forceRedraw()
-{
-    QWindowSystemInterface::handleExposeEvent(window(), geometry());
-}
-
 void UbuntuWindow::setWindowState(Qt::WindowState state)
 {
     QMutexLocker(&d->mutex);
@@ -406,7 +370,7 @@
 
     {
         QMutexLocker(&d->mutex);
-        d->geometry = rect;
+        QPlatformWindow::setGeometry(rect);
         doMoveResize = d->state != Qt::WindowFullScreen && d->state != Qt::WindowMaximized;
     }
 
@@ -450,16 +414,30 @@
 
     if (sizeKnown && (d->bufferSize.width() != newBufferWidth ||
                 d->bufferSize.height() != newBufferHeight)) {
-        QMetaObject::invokeMethod(this, "handleBufferResize",
-                Qt::QueuedConnection,
-                Q_ARG(int, newBufferWidth), Q_ARG(int, newBufferHeight));
+
+        DLOG("UbuntuWindow::onBuffersSwapped_threadSafe - buffer size changed from (%d,%d) to (%d,%d)",
+                d->bufferSize.width(), d->bufferSize.height(), newBufferWidth, newBufferHeight);
+
+        d->bufferSize.rwidth() = newBufferWidth;
+        d->bufferSize.rheight() = newBufferHeight;
+
+        QRect newGeometry;
+
+        newGeometry = geometry();
+        newGeometry.setWidth(d->bufferSize.width());
+        newGeometry.setHeight(d->bufferSize.height());
+
+        QPlatformWindow::setGeometry(newGeometry);
+        QWindowSystemInterface::handleGeometryChange(window(), newGeometry, QRect());
+        QWindowSystemInterface::handleExposeEvent(window(), newGeometry);
+
     } else {
         // buffer size hasn't changed
         if (d->targetBufferSize.isValid()) {
             if (d->bufferSize != d->targetBufferSize) {
                 // but we still didn't reach the promised buffer size from the mir resize event.
                 // thus keep swapping buffers
-                QMetaObject::invokeMethod(this, "forceRedraw", Qt::QueuedConnection);
+                QWindowSystemInterface::handleExposeEvent(window(), geometry());
             } else {
                 // target met. we have just provided a render with the target size and
                 // can therefore finally rest.

=== modified file 'src/ubuntumirclient/window.h'
--- src/ubuntumirclient/window.h	2015-02-04 16:14:43 +0000
+++ src/ubuntumirclient/window.h	2015-06-17 13:08:26 +0000
@@ -49,10 +49,6 @@
 
     UbuntuWindowPrivate* priv() { return d; }
 
-public Q_SLOTS:
-    void handleBufferResize(int width, int height);
-    void forceRedraw();
-
 private:
     void createWindow();
     void moveResize(const QRect& rect);