Merge lp:~lukas-kde/qtmir/wheelEvent into lp:qtmir

[Lukáš Tinkl]

Implement support for mouse wheel events; correctly pass around buttons

Implement support for mouse wheel events; also correctly pass around buttons

Cf. https://bugs.launchpad.net/ubuntu/+source/qtmir/+bug/1497091

* Are there any related MPs required for this MP to build/function as expected? Please list.

https://code.launchpad.net/~lukas-kde/qtubuntu/wheelEvent/+merge/273149

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

Yes

* Did you make sure that your branch does not contain spurious tags?

Yes

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

Yes

* If you changed the UI, has there been a design review?

N/A

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:383
http://jenkins.qa.ubuntu.com/job/qtmir-ci/475/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/171
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/171
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/171/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/53
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/208
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/208
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/208/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/53

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

"""
-pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=8)
+pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application>=8)
"""

The unity-api versioning scheme is made so that qtmir and unity8 depend on a *specific* version of the API. That's meant solely ensure that qtmir and unity8 are kept in sync. There's absolutely no guarantee (or effort to towards it) that a given version is backward or forward compatible.

review: Needs Fixing

[Lukáš Tinkl (lukas-kde)]

> """
> -pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application=8)
> +pkg_check_modules(APPLICATION_API REQUIRED unity-shell-application>=8)
> """
>
> The unity-api versioning scheme is made so that qtmir and unity8 depend on a
> *specific* version of the API. That's meant solely ensure that qtmir and
> unity8 are kept in sync. There's absolutely no guarantee (or effort to towards
> it) that a given version is backward or forward compatible.

Got it, fixed

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:384
http://jenkins.qa.ubuntu.com/job/qtmir-ci/482/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/178
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/178
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/178/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/60
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/215
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/215
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/215/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/60

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

Could you please move "getMirButtonsFromQt(qtEvent->buttons())" into a separate variable like it's being done for timestamp and modifiers?

The mir::events::make_event() call is already messy from having such a large number of parameters. Using those helper variables improve readability and I think won't impact performance as I bet they will be optimized away by the compiler.

[Lukáš Tinkl (lukas-kde)]

> Could you please move "getMirButtonsFromQt(qtEvent->buttons())" into a
> separate variable like it's being done for timestamp and modifiers?
>
> The mir::events::make_event() call is already messy from having such a large
> number of parameters. Using those helper variables improve readability and I
> think won't impact performance as I bet they will be optimized away by the
> compiler.

Done

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:385
http://jenkins.qa.ubuntu.com/job/qtmir-ci/485/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/181
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/181
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/181/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/218
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/218
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/218/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/63

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

the handleWheelEvent() line in qteventfeeder.h is way too long. Please avoid going beyond 120 chars. It's our coding style (which, by the way, I couldn't find it right now).

That's the last nitpick I have with this MP. :)

[Lukáš Tinkl (lukas-kde)]

> the handleWheelEvent() line in qteventfeeder.h is way too long. Please avoid
> going beyond 120 chars. It's our coding style (which, by the way, I couldn't
> find it right now).
>
> That's the last nitpick I have with this MP. :)

Fixed

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:386
http://jenkins.qa.ubuntu.com/job/qtmir-ci/486/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/182
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/182
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/182/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/64
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/219
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/219
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/219/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/64

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

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

> > the handleWheelEvent() line in qteventfeeder.h is way too long. Please avoid
> > going beyond 120 chars. It's our coding style (which, by the way, I couldn't
> > find it right now).
> >
> > That's the last nitpick I have with this MP. :)
>
> Fixed

Thanks!

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:388
http://jenkins.qa.ubuntu.com/job/qtmir-ci/511/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/207/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/207/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/89/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/244/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/244/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/89/console

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

review: Needs Fixing (continuous-integration)

[lp:~lukas-kde/qtmir/wheelEvent]

389. By Lukáš Tinkl on 2015-10-14

merge lp:~unity-team/qtmir/touch_tracing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:389
http://jenkins.qa.ubuntu.com/job/qtmir-ci/515/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/211/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/211/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/93/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/248/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/248/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/93/console

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

LGTM

review: Approve

[Daniel d'Andrada (dandrader)]

review: Abstain

=== modified file 'src/modules/Unity/Application/mirsurface.cpp'
--- src/modules/Unity/Application/mirsurface.cpp	2015-10-14 13:36:55 +0000
+++ src/modules/Unity/Application/mirsurface.cpp	2015-10-14 13:36:55 +0000
@@ -50,18 +50,29 @@
     return m_mods;
 }
 
+MirPointerButtons
+getMirButtonsFromQt(Qt::MouseButtons buttons)
+{
+    MirPointerButtons result = 0;
+    if (buttons & Qt::LeftButton)
+        result |= mir_pointer_button_primary;
+    if (buttons & Qt::RightButton)
+        result |= mir_pointer_button_secondary;
+    if (buttons & Qt::MiddleButton)
+        result |= mir_pointer_button_tertiary;
+    if (buttons & Qt::BackButton)
+        result |= mir_pointer_button_back;
+    if (buttons & Qt::ForwardButton)
+        result |= mir_pointer_button_forward;
+
+    return result;
+}
+
 mir::EventUPtr makeMirEvent(QMouseEvent *qtEvent, MirPointerAction action)
 {
     auto timestamp = uncompressTimestamp<ulong>(qtEvent->timestamp());
     auto modifiers = getMirModifiersFromQt(qtEvent->modifiers());
-
-    MirPointerButtons buttons = 0;
-    if (qtEvent->buttons() & Qt::LeftButton)
-        buttons |= mir_pointer_button_primary;
-    if (qtEvent->buttons() & Qt::RightButton)
-        buttons |= mir_pointer_button_secondary;
-    if (qtEvent->buttons() & Qt::MidButton)
-        buttons |= mir_pointer_button_tertiary;
+    auto buttons = getMirButtonsFromQt(qtEvent->buttons());
 
     return mir::events::make_event(0 /*DeviceID */, timestamp, 0 /* mac */, modifiers, action,
                                    buttons, qtEvent->x(), qtEvent->y(), 0, 0, 0, 0);
@@ -77,6 +88,18 @@
                                    buttons, qtEvent->posF().x(), qtEvent->posF().y(), 0, 0, 0, 0);
 }
 
+mir::EventUPtr makeMirEvent(QWheelEvent *qtEvent)
+{
+    auto timestamp = std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds(qtEvent->timestamp()));
+    auto modifiers = getMirModifiersFromQt(qtEvent->modifiers());
+    auto buttons = getMirButtonsFromQt(qtEvent->buttons());
+
+    return mir::events::make_event(0 /*DeviceID */, timestamp, 0 /* mac */, modifiers, mir_pointer_action_motion,
+                                   buttons, qtEvent->x(), qtEvent->y(),
+                                   qtEvent->angleDelta().x(), qtEvent->angleDelta().y(),
+                                   0, 0);
+}
+
 mir::EventUPtr makeMirEvent(QKeyEvent *qtEvent)
 {
     MirKeyboardAction action = mir_keyboard_action_down;
@@ -574,6 +597,13 @@
     event->accept();
 }
 
+void MirSurface::wheelEvent(QWheelEvent *event)
+{
+    auto ev = makeMirEvent(event);
+    m_surface->consume(*ev);
+    event->accept();
+}
+
 void MirSurface::keyPressEvent(QKeyEvent *qtEvent)
 {
     auto ev = makeMirEvent(qtEvent);

=== modified file 'src/modules/Unity/Application/mirsurface.h'
--- src/modules/Unity/Application/mirsurface.h	2015-10-14 13:36:55 +0000
+++ src/modules/Unity/Application/mirsurface.h	2015-10-14 13:36:55 +0000
@@ -100,6 +100,7 @@
     void hoverEnterEvent(QHoverEvent *event) override;
     void hoverLeaveEvent(QHoverEvent *event) override;
     void hoverMoveEvent(QHoverEvent *event) override;
+    void wheelEvent(QWheelEvent *event) override;
 
     void keyPressEvent(QKeyEvent *event) override;
     void keyReleaseEvent(QKeyEvent *event) override;

=== modified file 'src/modules/Unity/Application/mirsurfaceinterface.h'
--- src/modules/Unity/Application/mirsurfaceinterface.h	2015-10-14 13:36:55 +0000
+++ src/modules/Unity/Application/mirsurfaceinterface.h	2015-10-14 13:36:55 +0000
@@ -67,6 +67,7 @@
     virtual void hoverEnterEvent(QHoverEvent *event) = 0;
     virtual void hoverLeaveEvent(QHoverEvent *event) = 0;
     virtual void hoverMoveEvent(QHoverEvent *event) = 0;
+    virtual void wheelEvent(QWheelEvent *event) = 0;
 
     virtual void keyPressEvent(QKeyEvent *event) = 0;
     virtual void keyReleaseEvent(QKeyEvent *event) = 0;

=== modified file 'src/modules/Unity/Application/mirsurfaceitem.cpp'
--- src/modules/Unity/Application/mirsurfaceitem.cpp	2015-10-14 13:36:55 +0000
+++ src/modules/Unity/Application/mirsurfaceitem.cpp	2015-10-14 13:36:55 +0000
@@ -298,7 +298,11 @@
 
 void MirSurfaceItem::wheelEvent(QWheelEvent *event)
 {
-    Q_UNUSED(event);
+    if (m_consumesInput && m_surface && m_surface->live()) {
+        m_surface->wheelEvent(event);
+    } else {
+        event->ignore();
+    }
 }
 
 void MirSurfaceItem::hoverEnterEvent(QHoverEvent *event)

=== modified file 'src/platforms/mirserver/qteventfeeder.cpp'
--- src/platforms/mirserver/qteventfeeder.cpp	2015-10-14 13:36:55 +0000
+++ src/platforms/mirserver/qteventfeeder.cpp	2015-10-14 13:36:55 +0000
@@ -416,7 +416,8 @@
         QWindowSystemInterface::handleTouchEvent(window, timestamp, device, points, mods);
     }
 
-    void handleMouseEvent(ulong timestamp, QPointF movement, Qt::MouseButtons buttons, Qt::KeyboardModifiers modifiers) override
+    void handleMouseEvent(ulong timestamp, QPointF movement, Qt::MouseButtons buttons,
+                          Qt::KeyboardModifiers modifiers) override
     {
         // Send to the first screen that handles the mouse event
         // TODO: Have a mechanism to tell which screen currently has the logical mouse pointer
@@ -434,6 +435,25 @@
         }
     }
 
+    void handleWheelEvent(ulong timestamp, const QPointF &localPoint, const QPointF &globalPoint,
+                          QPoint pixelDelta, QPoint angleDelta,
+                          Qt::KeyboardModifiers mods, Qt::ScrollPhase phase) override
+    {
+        QWindowSystemInterface::handleWheelEvent(m_screenController->getWindowForPoint(localPoint.toPoint()),
+                                                 timestamp, localPoint, globalPoint,
+                                                 pixelDelta, angleDelta, mods, phase);
+    }
+
+    void handleEnterEvent(const QPointF &localPoint, const QPointF &globalPoint) override
+    {
+        QWindowSystemInterface::handleEnterEvent(m_screenController->getWindowForPoint(localPoint.toPoint()), localPoint, globalPoint);
+    }
+
+    void handleLeaveEvent(const QPointF &localPoint) override
+    {
+        QWindowSystemInterface::handleLeaveEvent(m_screenController->getWindowForPoint(localPoint.toPoint()));
+    }
+
 private:
     QSharedPointer<ScreenController> m_screenController;
 };
@@ -496,7 +516,7 @@
 
 Qt::KeyboardModifiers getQtModifiersFromMir(MirInputEventModifiers modifiers)
 {
-    int qtModifiers = Qt::NoModifier;
+    Qt::KeyboardModifiers qtModifiers = Qt::NoModifier;
     if (modifiers & mir_input_event_modifier_shift) {
         qtModifiers |= Qt::ShiftModifier;
     }
@@ -509,7 +529,7 @@
     if (modifiers & mir_input_event_modifier_meta) {
         qtModifiers |= Qt::MetaModifier;
     }
-    return static_cast<Qt::KeyboardModifiers>(qtModifiers);
+    return qtModifiers;
 }
 
 Qt::MouseButtons getQtMouseButtonsfromMirPointerEvent(MirPointerEvent const* pev)
@@ -533,17 +553,44 @@
 void QtEventFeeder::dispatchPointer(MirInputEvent const* ev)
 {
     auto timestamp = qtmir::compressTimestamp<ulong>(std::chrono::nanoseconds(mir_input_event_get_event_time(ev)));
-
     auto pev = mir_input_event_get_pointer_event(ev);
+    auto action = mir_pointer_event_action(pev);
     qCDebug(QTMIR_MIR_INPUT) << "Received" << qPrintable(mirPointerEventToString(pev));
 
     auto modifiers = getQtModifiersFromMir(mir_pointer_event_modifiers(pev));
-    auto buttons = getQtMouseButtonsfromMirPointerEvent(pev);
 
     auto movement = QPointF(mir_pointer_event_axis_value(pev, mir_pointer_axis_relative_x),
                             mir_pointer_event_axis_value(pev, mir_pointer_axis_relative_y));
-
-    mQtWindowSystem->handleMouseEvent(timestamp, movement, buttons, modifiers);
+    auto local_point = QPointF(mir_pointer_event_axis_value(pev, mir_pointer_axis_x),
+                               mir_pointer_event_axis_value(pev, mir_pointer_axis_y));
+
+    switch (action) {
+    case mir_pointer_action_button_up:
+    case mir_pointer_action_button_down:
+    case mir_pointer_action_motion:
+    {
+        const float hDelta = mir_pointer_event_axis_value(pev, mir_pointer_axis_hscroll);
+        const float vDelta = mir_pointer_event_axis_value(pev, mir_pointer_axis_vscroll);
+
+        if (hDelta != 0 || vDelta != 0) {
+            const QPoint angleDelta = QPoint(hDelta * 15, vDelta * 15);
+            mQtWindowSystem->handleWheelEvent(timestamp, local_point, local_point,
+                                              QPoint(), angleDelta, modifiers, Qt::ScrollUpdate);
+        } else {
+            auto buttons = getQtMouseButtonsfromMirPointerEvent(pev);
+            mQtWindowSystem->handleMouseEvent(timestamp, movement, buttons, modifiers);
+        }
+        break;
+    }
+    case mir_pointer_action_enter:
+        mQtWindowSystem->handleEnterEvent(local_point, local_point);
+        break;
+    case mir_pointer_action_leave:
+        mQtWindowSystem->handleLeaveEvent(local_point);
+        break;
+    default:
+        qCDebug(QTMIR_MIR_INPUT) << "Unrecognized pointer event";
+    }
 }
 
 void QtEventFeeder::dispatchKey(MirInputEvent const* event)

=== modified file 'src/platforms/mirserver/qteventfeeder.h'
--- src/platforms/mirserver/qteventfeeder.h	2015-10-14 13:36:55 +0000
+++ src/platforms/mirserver/qteventfeeder.h	2015-10-14 13:36:55 +0000
@@ -52,6 +52,12 @@
                 Qt::KeyboardModifiers mods = Qt::NoModifier) = 0;
         virtual void handleMouseEvent(ulong timestamp, QPointF movement, Qt::MouseButtons buttons,
                                       Qt::KeyboardModifiers modifiers) = 0;
+        virtual void handleWheelEvent(ulong timestamp, const QPointF &localPoint, const QPointF &globalPoint,
+                                      QPoint pixelDelta, QPoint angleDelta,
+                                      Qt::KeyboardModifiers mods = Qt::NoModifier,
+                                      Qt::ScrollPhase phase = Qt::ScrollUpdate) = 0;
+        virtual void handleEnterEvent(const QPointF &localPoint = QPointF(), const QPointF &globalPoint = QPointF()) = 0;
+        virtual void handleLeaveEvent(const QPointF &localPoint = QPointF()) = 0;
     };
 
     QtEventFeeder(const QSharedPointer<ScreenController> &screenController);

=== modified file 'tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h'
--- tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h	2015-10-14 13:36:55 +0000
+++ tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h	2015-10-14 13:36:55 +0000
@@ -41,7 +41,11 @@
     MOCK_METHOD5(handleTouchEvent, void(QWindow *window, ulong timestamp, QTouchDevice *device,
             const QList<struct QWindowSystemInterface::TouchPoint> &points,
             Qt::KeyboardModifiers mods));
-    MOCK_METHOD4(handleMouseEvent, void(ulong, QPointF, Qt::MouseButtons, Qt::KeyboardModifiers));
+    MOCK_METHOD4(handleMouseEvent, void(ulong timestamp, QPointF point, Qt::MouseButtons buttons, Qt::KeyboardModifiers modifiers));
+    MOCK_METHOD7(handleWheelEvent, void(ulong timestamp, const QPointF & local, const QPointF & global, QPoint pixelDelta, QPoint angleDelta,
+                                        Qt::KeyboardModifiers mods, Qt::ScrollPhase phase));
+    MOCK_METHOD2(handleEnterEvent, void(const QPointF &localPoint, const QPointF& globalPoint));
+    MOCK_METHOD1(handleLeaveEvent, void(const QPointF &localPoint));
 };
 
 namespace testing

=== modified file 'tests/modules/common/fake_mirsurface.h'
--- tests/modules/common/fake_mirsurface.h	2015-10-14 13:36:55 +0000
+++ tests/modules/common/fake_mirsurface.h	2015-10-14 13:36:55 +0000
@@ -146,6 +146,7 @@
     void hoverEnterEvent(QHoverEvent *) override {}
     void hoverLeaveEvent(QHoverEvent *) override {}
     void hoverMoveEvent(QHoverEvent *) override {}
+    void wheelEvent(QWheelEvent *) override {}
 
     void keyPressEvent(QKeyEvent *) override {}
     void keyReleaseEvent(QKeyEvent *) override {}