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.

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

260. By Daniel d'Andrada on 2015-06-02

Merge lp:~mir-team/qtubuntu/track-mir-deprecations

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

Refactor optional logging and remove trailing whitespace

Unmerged revisions

=== modified file 'debian/control'
--- debian/control	2015-03-05 19:54:57 +0000
+++ debian/control	2015-06-03 13:24:47 +0000
@@ -17,7 +17,7 @@
                qtbase5-private-dev,
                qtdeclarative5-dev,
                libqt5sensors5-dev,
-               libmirclient-dev (>= 0.11.0),
+               libmirclient-dev (>= 0.13.0),
 # if you don't have have commit access to this branch but would like to upload
 # directly to Ubuntu, don't worry: your changes will be merged back into the
 # upstream branch

=== modified file 'src/ubuntumirclient/input.cpp'
--- src/ubuntumirclient/input.cpp	2015-02-17 17:39:47 +0000
+++ src/ubuntumirclient/input.cpp	2015-06-03 13:24:47 +0000
@@ -160,6 +160,7 @@
 #ifndef QT_NO_DEBUG
 
 
+/*
 static const char* nativeEventTypeToStr(MirEventType t)
 {
     switch (t)
@@ -185,7 +186,7 @@
         return "invalid";
     }
 }
-
+*/
 
 #endif
 
@@ -275,7 +276,7 @@
 
 void UbuntuInput::dispatchTouchEvent(QWindow *window, const MirInputEvent *ev)
 {
-    const MirTouchInputEvent *tev = mir_input_event_get_touch_input_event(ev);
+    const MirTouchEvent *tev = mir_input_event_get_touch_event(ev);
 
     // FIXME(loicm) Max pressure is device specific. That one is for the Samsung Galaxy Nexus. That
     //     needs to be fixed as soon as the compat input lib adds query support.
@@ -286,30 +287,30 @@
 
     // TODO: Is it worth setting the Qt::TouchPointStationary ones? Currently they are left
     //       as Qt::TouchPointMoved
-    const unsigned int kPointerCount = mir_touch_input_event_get_touch_count(tev);
+    const unsigned int kPointerCount = mir_touch_event_point_count(tev);
     for (unsigned int i = 0; i < kPointerCount; ++i) {
         QWindowSystemInterface::TouchPoint touchPoint;
 
-        const float kX = mir_touch_input_event_get_touch_axis_value(tev, i, mir_touch_input_axis_x) + kWindowGeometry.x();
-        const float kY = mir_touch_input_event_get_touch_axis_value(tev, i, mir_touch_input_axis_y) + kWindowGeometry.y(); // see bug lp:1346633 workaround comments elsewhere
-        const float kW = mir_touch_input_event_get_touch_axis_value(tev, i, mir_touch_input_axis_touch_major);
-        const float kH = mir_touch_input_event_get_touch_axis_value(tev, i, mir_touch_input_axis_touch_minor);
-        const float kP = mir_touch_input_event_get_touch_axis_value(tev, i, mir_touch_input_axis_pressure);
-        touchPoint.id = mir_touch_input_event_get_touch_id(tev, i);
+        const float kX = mir_touch_event_axis_value(tev, i, mir_touch_axis_x) + kWindowGeometry.x();
+        const float kY = mir_touch_event_axis_value(tev, i, mir_touch_axis_y) + kWindowGeometry.y(); // see bug lp:1346633 workaround comments elsewhere
+        const float kW = mir_touch_event_axis_value(tev, i, mir_touch_axis_touch_major);
+        const float kH = mir_touch_event_axis_value(tev, i, mir_touch_axis_touch_minor);
+        const float kP = mir_touch_event_axis_value(tev, i, mir_touch_axis_pressure);
+        touchPoint.id = mir_touch_event_id(tev, i);
         touchPoint.normalPosition = QPointF(kX / kWindowGeometry.width(), kY / kWindowGeometry.height());
         touchPoint.area = QRectF(kX - (kW / 2.0), kY - (kH / 2.0), kW, kH);
         touchPoint.pressure = kP / kMaxPressure;
 
-        MirTouchInputEventTouchAction touch_action = mir_touch_input_event_get_touch_action(tev, i);
+        MirTouchAction touch_action = mir_touch_event_action(tev, i);
         switch (touch_action)
         {
-        case mir_touch_input_event_action_down:
+        case mir_touch_action_down:
             touchPoint.state = Qt::TouchPointPressed;
             break;
-        case mir_touch_input_event_action_up:
+        case mir_touch_action_up:
             touchPoint.state = Qt::TouchPointReleased;
             break;
-        case mir_touch_input_event_action_change:
+        case mir_touch_action_change:
         default:
             touchPoint.state = Qt::TouchPointMoved;
         }
@@ -363,23 +364,23 @@
 
 void UbuntuInput::dispatchKeyEvent(QWindow *window, const MirInputEvent *event)
 {
-    const MirKeyInputEvent *key_event = mir_input_event_get_key_input_event(event);
+    const MirKeyboardEvent *key_event = mir_input_event_get_keyboard_event(event);
 
     ulong timestamp = mir_input_event_get_event_time(event) / 1000000;
-    xkb_keysym_t xk_sym = mir_key_input_event_get_key_code(key_event);
+    xkb_keysym_t xk_sym = mir_keyboard_event_key_code(key_event);
 
     // Key modifier and unicode index mapping.
-    auto modifiers = qt_modifiers_from_mir(mir_key_input_event_get_modifiers(key_event));
+    auto modifiers = qt_modifiers_from_mir(mir_keyboard_event_modifiers(key_event));
 
-    MirKeyInputEventAction action = mir_key_input_event_get_action(key_event);
-    QEvent::Type keyType = action == mir_key_input_event_action_up
+    MirKeyboardAction action = mir_keyboard_event_action(key_event);
+    QEvent::Type keyType = action == mir_keyboard_action_up
         ? QEvent::KeyRelease : QEvent::KeyPress;
 
     char s[2];
     int sym = translateKeysym(xk_sym, s, sizeof(s));
     QString text = QString::fromLatin1(s);
 
-    bool is_auto_rep = action == mir_key_input_event_action_repeat;
+    bool is_auto_rep = action == mir_keyboard_action_repeat;
 
     QPlatformInputContext *context = QGuiApplicationPrivate::platformIntegration()->inputContext();
     if (context) {
@@ -396,14 +397,14 @@
 
 namespace
 {
-Qt::MouseButtons extract_buttons(const MirPointerInputEvent *pev)
+Qt::MouseButtons extract_buttons(const MirPointerEvent *pev)
 {
     Qt::MouseButtons buttons = Qt::NoButton;
-    if (mir_pointer_input_event_get_button_state(pev, mir_pointer_input_button_primary))
+    if (mir_pointer_event_button_state(pev, mir_pointer_button_primary))
         buttons |= Qt::LeftButton;
-    if (mir_pointer_input_event_get_button_state(pev, mir_pointer_input_button_secondary))
+    if (mir_pointer_event_button_state(pev, mir_pointer_button_secondary))
         buttons |= Qt::RightButton;
-    if (mir_pointer_input_event_get_button_state(pev, mir_pointer_input_button_tertiary))
+    if (mir_pointer_event_button_state(pev, mir_pointer_button_tertiary))
         buttons |= Qt::MidButton;
 
     // TODO: Should mir back and forward buttons exist?
@@ -416,12 +417,12 @@
 {
     auto timestamp = mir_input_event_get_event_time(ev) / 1000000;
 
-    auto pev = mir_input_event_get_pointer_input_event(ev);
-    auto modifiers = qt_modifiers_from_mir(mir_pointer_input_event_get_modifiers(pev));
+    auto pev = mir_input_event_get_pointer_event(ev);
+    auto modifiers = qt_modifiers_from_mir(mir_pointer_event_modifiers(pev));
     auto buttons = extract_buttons(pev);
 
-    auto local_point = QPointF(mir_pointer_input_event_get_axis_value(pev, mir_pointer_input_axis_x),
-                               mir_pointer_input_event_get_axis_value(pev, mir_pointer_input_axis_y));
+    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-02-23 19:53:45 +0000
+++ src/ubuntumirclient/window.cpp	2015-06-03 13:24:47 +0000
@@ -96,7 +96,6 @@
     WId id;
     UbuntuInput* input;
     Qt::WindowState state;
-    QRect geometry;
     MirConnection *connection;
     MirSurface* surface;
     QSize bufferSize;
@@ -119,8 +118,7 @@
     UbuntuWindow* platformWindow = static_cast<UbuntuWindow*>(context);
     platformWindow->priv()->surface = surface;
 
-    MirEventDelegate handler = {eventCallback, context};
-    mir_surface_set_event_handler(surface, &handler);
+    mir_surface_set_event_handler(surface, eventCallback, context);
 }
 
 UbuntuWindow::UbuntuWindow(QWindow* w, QSharedPointer<UbuntuClipboard> clipboard, UbuntuScreen* screen,
@@ -141,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);
 }
@@ -254,7 +252,7 @@
         geometry.setY(panelHeight);
     } else {
         printf("UbuntuWindow - regular geometry\n");
-        geometry = d->geometry;
+        geometry = this->geometry();
         geometry.setY(panelHeight);
     }
 
@@ -279,7 +277,7 @@
     mir_surface_spec_release(spec);
 
     DASSERT(d->surface != NULL);
-    d->createEGLSurface((EGLNativeWindowType)mir_surface_get_egl_native_window(d->surface));
+    d->createEGLSurface((EGLNativeWindowType)mir_buffer_stream_get_egl_native_window(mir_surface_get_buffer_stream(d->surface)));
 
     if (d->state == Qt::WindowFullScreen) {
     // TODO: We could set this on creation once surface spec supports it (mps already up)
@@ -315,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"
@@ -357,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);
@@ -407,7 +370,7 @@
 
     {
         QMutexLocker(&d->mutex);
-        d->geometry = rect;
+        QPlatformWindow::setGeometry(rect);
         doMoveResize = d->state != Qt::WindowFullScreen && d->state != Qt::WindowMaximized;
     }
 
@@ -451,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-03 13:24:47 +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);