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

[Lukáš Tinkl]

Fix inconsistent mouse wheel scrolling behavior

Fix inconsistent mouse wheel scrolling behavior

Need to pass a nullptr as a target, QWindowSystemInterface will internally choose the window to send the event to based on the globalPos only (QCursor::pos() in this case).

Related branch: https://code.launchpad.net/~unity-team/qtubuntu/fixWheel/+merge/277680

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:399
http://jenkins.qa.ubuntu.com/job/qtmir-ci/540/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/236
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/236
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/236/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/118
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/273/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/273/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/118/console

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

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

I doesn't make sense to handle enter and leave events in qtmir right now.

This might actually be harmful as the qt mouse pointer is defined and controlled by the Cursor qml plugin in unity8. Sending enter and leave events directly to Qt ourselves contradicts the mouse events generated by that qml element.

If unity8's QGuiApplication is to get any mouse enter and leave events, they should come from the Cursor plugin, not from qtmir.

Those enter and leave events are from the mir cursor, which we are ignoring right now.

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

Thus I think the way to go is to remove this enter and leave event handling altogether. We should have it only once we ditch the qml Cursor approach and start using the mir cursor.

[Daniel d'Andrada (dandrader)]

The wheel events should be given to the platform cursor so it can be handled over to the qml Cursor just like we do with motion events.

review: Needs Fixing

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

400. By Lukáš Tinkl on 2015-11-06

merge trunk

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:400
http://jenkins.qa.ubuntu.com/job/qtmir-ci/552/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/248
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/248
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/248/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/130
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/285/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/285/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/130/console

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

review: Needs Fixing (continuous-integration)

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

401. By Lukáš Tinkl on 2015-11-09

remove handling of enter/leave events

[Lukáš Tinkl (lukas-kde)]

> Thus I think the way to go is to remove this enter and leave event handling
> altogether. We should have it only once we ditch the qml Cursor approach and
> start using the mir cursor.

Removed, still working fine

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

402. By Lukáš Tinkl on 2015-11-09

also remove mock enter/leave events

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:402
http://jenkins.qa.ubuntu.com/job/qtmir-ci/553/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/249
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/249
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/249/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/131
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/286/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/286/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/131/console

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

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

> The wheel events should be given to the platform cursor so it can be handled
> over to the qml Cursor just like we do with motion events.

hmmm... not really need in this case since you're using the static QCursor::pos() function which returns the position of our QML Cursor already.

[Daniel d'Andrada (dandrader)]

> The wheel events should be given to the platform cursor so it can be handled
> over to the qml Cursor just like we do with motion events.

hmmm... not really need in this case since you're using the static QCursor::pos() function which returns the position of our QML Cursor already.

[Daniel d'Andrada (dandrader)]

Please remove the now supeefluous code and document behavior a bit:
http://bazaar.launchpad.net/~dandrader/qtmir/fixWheel/revision/406

review: Needs Fixing

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

403. By Lukáš Tinkl on 2015-11-10

remove superfluous code, document the wheel behavior

merge lp:~dandrader/qtmir/fixWheel

[Lukáš Tinkl (lukas-kde)]

> Please remove the now supeefluous code and document behavior a bit:
> http://bazaar.launchpad.net/~dandrader/qtmir/fixWheel/revision/406

Thanks for that, fixed

[Daniel d'Andrada (dandrader)]

Looking good. Didn't test though.

review: Approve (code)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:403
http://jenkins.qa.ubuntu.com/job/qtmir-ci/556/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/252
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/252
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/252/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-i386-ci/134
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/289/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/289/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-wily-i386-ci/134/console

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

review: Needs Fixing (continuous-integration)

[Daniel d'Andrada (dandrader)]

And works too

review: Approve

[Gerry Boland (gerboland)]

This is in silo48, and in testing, mouse wheel on external display is not working. It is working well on internal display though

review: Needs Fixing

Unmerged revisions

=== modified file 'src/platforms/mirserver/qteventfeeder.cpp'
--- src/platforms/mirserver/qteventfeeder.cpp	2015-10-29 11:58:29 +0000
+++ src/platforms/mirserver/qteventfeeder.cpp	2015-11-10 18:49:24 +0000
@@ -435,25 +435,16 @@
         }
     }
 
-    void handleWheelEvent(ulong timestamp, const QPointF &localPoint, const QPointF &globalPoint,
+    void handleWheelEvent(ulong timestamp, const QPointF &globalPoint,
                           QPoint pixelDelta, QPoint angleDelta,
                           Qt::KeyboardModifiers mods, Qt::ScrollPhase phase) override
     {
-        QWindowSystemInterface::handleWheelEvent(m_screenController->getWindowForPoint(localPoint.toPoint()),
-                                                 timestamp, localPoint, globalPoint,
+        // NB: The target QWindow is deduced by Qt from globalPoint when null is passed.
+        //     localPoint is irrelevant as Qt will calculate it from the QWindow it sends it to.
+        QWindowSystemInterface::handleWheelEvent(nullptr /* window */, timestamp, QPointF() /* 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;
 };
@@ -561,8 +552,6 @@
 
     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));
-    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:
@@ -574,20 +563,13 @@
 
         if (hDelta != 0 || vDelta != 0) {
             const QPoint angleDelta = QPoint(hDelta * 15, vDelta * 15);
-            mQtWindowSystem->handleWheelEvent(timestamp.count(), local_point, local_point,
+            mQtWindowSystem->handleWheelEvent(timestamp.count(), QCursor::pos(),
                                               QPoint(), angleDelta, modifiers, Qt::ScrollUpdate);
-        } else {
-            auto buttons = getQtMouseButtonsfromMirPointerEvent(pev);
-            mQtWindowSystem->handleMouseEvent(timestamp.count(), movement, buttons, modifiers);
         }
+        auto buttons = getQtMouseButtonsfromMirPointerEvent(pev);
+        mQtWindowSystem->handleMouseEvent(timestamp.count(), 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";
     }

=== modified file 'src/platforms/mirserver/qteventfeeder.h'
--- src/platforms/mirserver/qteventfeeder.h	2015-10-14 13:36:25 +0000
+++ src/platforms/mirserver/qteventfeeder.h	2015-11-10 18:49:24 +0000
@@ -52,12 +52,10 @@
                 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,
+        virtual void handleWheelEvent(ulong timestamp, 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:25 +0000
+++ tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h	2015-11-10 18:49:24 +0000
@@ -42,10 +42,8 @@
             const QList<struct QWindowSystemInterface::TouchPoint> &points,
             Qt::KeyboardModifiers mods));
     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,
+    MOCK_METHOD6(handleWheelEvent, void(ulong timestamp, 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