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

Proposed by Lukáš Tinkl
Status: Merged
Approved by: Gerry Boland
Approved revision: 389
Merged at revision: 396
Proposed branch: lp:~lukas-kde/qtmir/wheelEvent
Merge into: lp:qtmir
Prerequisite: lp:~unity-team/qtmir/touch_tracing
Diff against target: 268 lines (+111/-17)
8 files modified
src/modules/Unity/Application/mirsurface.cpp (+38/-8)
src/modules/Unity/Application/mirsurface.h (+1/-0)
src/modules/Unity/Application/mirsurfaceinterface.h (+1/-0)
src/modules/Unity/Application/mirsurfaceitem.cpp (+5/-1)
src/platforms/mirserver/qteventfeeder.cpp (+54/-7)
src/platforms/mirserver/qteventfeeder.h (+6/-0)
tests/mirserver/QtEventFeeder/mock_qtwindowsystem.h (+5/-1)
tests/modules/common/fake_mirsurface.h (+1/-0)
To merge this branch: bzr merge lp:~lukas-kde/qtmir/wheelEvent
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) Abstain
Gerry Boland (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+274313@code.launchpad.net

This proposal supersedes a proposal from 2015-10-01.

Commit message

Implement support for mouse wheel events; correctly pass around buttons

Description of the change

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

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

"""
-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
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote : Posted in a previous version of this proposal

> """
> -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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

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.

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote : Posted in a previous version of this proposal

> 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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

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. :)

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote : Posted in a previous version of this proposal

> 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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

> > 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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~lukas-kde/qtmir/wheelEvent updated
389. By Lukáš Tinkl

merge lp:~unity-team/qtmir/touch_tracing

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) wrote :

LGTM

review: Approve
Revision history for this message
Daniel d'Andrada (dandrader) :
review: Abstain

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
50 return m_mods;50 return m_mods;
51}51}
5252
53MirPointerButtons
54getMirButtonsFromQt(Qt::MouseButtons buttons)
55{
56 MirPointerButtons result = 0;
57 if (buttons & Qt::LeftButton)
58 result |= mir_pointer_button_primary;
59 if (buttons & Qt::RightButton)
60 result |= mir_pointer_button_secondary;
61 if (buttons & Qt::MiddleButton)
62 result |= mir_pointer_button_tertiary;
63 if (buttons & Qt::BackButton)
64 result |= mir_pointer_button_back;
65 if (buttons & Qt::ForwardButton)
66 result |= mir_pointer_button_forward;
67
68 return result;
69}
70
53mir::EventUPtr makeMirEvent(QMouseEvent *qtEvent, MirPointerAction action)71mir::EventUPtr makeMirEvent(QMouseEvent *qtEvent, MirPointerAction action)
54{72{
55 auto timestamp = uncompressTimestamp<ulong>(qtEvent->timestamp());73 auto timestamp = uncompressTimestamp<ulong>(qtEvent->timestamp());
56 auto modifiers = getMirModifiersFromQt(qtEvent->modifiers());74 auto modifiers = getMirModifiersFromQt(qtEvent->modifiers());
5775 auto buttons = getMirButtonsFromQt(qtEvent->buttons());
58 MirPointerButtons buttons = 0;
59 if (qtEvent->buttons() & Qt::LeftButton)
60 buttons |= mir_pointer_button_primary;
61 if (qtEvent->buttons() & Qt::RightButton)
62 buttons |= mir_pointer_button_secondary;
63 if (qtEvent->buttons() & Qt::MidButton)
64 buttons |= mir_pointer_button_tertiary;
6576
66 return mir::events::make_event(0 /*DeviceID */, timestamp, 0 /* mac */, modifiers, action,77 return mir::events::make_event(0 /*DeviceID */, timestamp, 0 /* mac */, modifiers, action,
67 buttons, qtEvent->x(), qtEvent->y(), 0, 0, 0, 0);78 buttons, qtEvent->x(), qtEvent->y(), 0, 0, 0, 0);
@@ -77,6 +88,18 @@
77 buttons, qtEvent->posF().x(), qtEvent->posF().y(), 0, 0, 0, 0);88 buttons, qtEvent->posF().x(), qtEvent->posF().y(), 0, 0, 0, 0);
78}89}
7990
91mir::EventUPtr makeMirEvent(QWheelEvent *qtEvent)
92{
93 auto timestamp = std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds(qtEvent->timestamp()));
94 auto modifiers = getMirModifiersFromQt(qtEvent->modifiers());
95 auto buttons = getMirButtonsFromQt(qtEvent->buttons());
96
97 return mir::events::make_event(0 /*DeviceID */, timestamp, 0 /* mac */, modifiers, mir_pointer_action_motion,
98 buttons, qtEvent->x(), qtEvent->y(),
99 qtEvent->angleDelta().x(), qtEvent->angleDelta().y(),
100 0, 0);
101}
102
80mir::EventUPtr makeMirEvent(QKeyEvent *qtEvent)103mir::EventUPtr makeMirEvent(QKeyEvent *qtEvent)
81{104{
82 MirKeyboardAction action = mir_keyboard_action_down;105 MirKeyboardAction action = mir_keyboard_action_down;
@@ -574,6 +597,13 @@
574 event->accept();597 event->accept();
575}598}
576599
600void MirSurface::wheelEvent(QWheelEvent *event)
601{
602 auto ev = makeMirEvent(event);
603 m_surface->consume(*ev);
604 event->accept();
605}
606
577void MirSurface::keyPressEvent(QKeyEvent *qtEvent)607void MirSurface::keyPressEvent(QKeyEvent *qtEvent)
578{608{
579 auto ev = makeMirEvent(qtEvent);609 auto ev = makeMirEvent(qtEvent);
580610
=== 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 @@
100 void hoverEnterEvent(QHoverEvent *event) override;100 void hoverEnterEvent(QHoverEvent *event) override;
101 void hoverLeaveEvent(QHoverEvent *event) override;101 void hoverLeaveEvent(QHoverEvent *event) override;
102 void hoverMoveEvent(QHoverEvent *event) override;102 void hoverMoveEvent(QHoverEvent *event) override;
103 void wheelEvent(QWheelEvent *event) override;
103104
104 void keyPressEvent(QKeyEvent *event) override;105 void keyPressEvent(QKeyEvent *event) override;
105 void keyReleaseEvent(QKeyEvent *event) override;106 void keyReleaseEvent(QKeyEvent *event) override;
106107
=== 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 @@
67 virtual void hoverEnterEvent(QHoverEvent *event) = 0;67 virtual void hoverEnterEvent(QHoverEvent *event) = 0;
68 virtual void hoverLeaveEvent(QHoverEvent *event) = 0;68 virtual void hoverLeaveEvent(QHoverEvent *event) = 0;
69 virtual void hoverMoveEvent(QHoverEvent *event) = 0;69 virtual void hoverMoveEvent(QHoverEvent *event) = 0;
70 virtual void wheelEvent(QWheelEvent *event) = 0;
7071
71 virtual void keyPressEvent(QKeyEvent *event) = 0;72 virtual void keyPressEvent(QKeyEvent *event) = 0;
72 virtual void keyReleaseEvent(QKeyEvent *event) = 0;73 virtual void keyReleaseEvent(QKeyEvent *event) = 0;
7374
=== 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 @@
298298
299void MirSurfaceItem::wheelEvent(QWheelEvent *event)299void MirSurfaceItem::wheelEvent(QWheelEvent *event)
300{300{
301 Q_UNUSED(event);301 if (m_consumesInput && m_surface && m_surface->live()) {
302 m_surface->wheelEvent(event);
303 } else {
304 event->ignore();
305 }
302}306}
303307
304void MirSurfaceItem::hoverEnterEvent(QHoverEvent *event)308void MirSurfaceItem::hoverEnterEvent(QHoverEvent *event)
305309
=== 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 @@
416 QWindowSystemInterface::handleTouchEvent(window, timestamp, device, points, mods);416 QWindowSystemInterface::handleTouchEvent(window, timestamp, device, points, mods);
417 }417 }
418418
419 void handleMouseEvent(ulong timestamp, QPointF movement, Qt::MouseButtons buttons, Qt::KeyboardModifiers modifiers) override419 void handleMouseEvent(ulong timestamp, QPointF movement, Qt::MouseButtons buttons,
420 Qt::KeyboardModifiers modifiers) override
420 {421 {
421 // Send to the first screen that handles the mouse event422 // Send to the first screen that handles the mouse event
422 // TODO: Have a mechanism to tell which screen currently has the logical mouse pointer423 // TODO: Have a mechanism to tell which screen currently has the logical mouse pointer
@@ -434,6 +435,25 @@
434 }435 }
435 }436 }
436437
438 void handleWheelEvent(ulong timestamp, const QPointF &localPoint, const QPointF &globalPoint,
439 QPoint pixelDelta, QPoint angleDelta,
440 Qt::KeyboardModifiers mods, Qt::ScrollPhase phase) override
441 {
442 QWindowSystemInterface::handleWheelEvent(m_screenController->getWindowForPoint(localPoint.toPoint()),
443 timestamp, localPoint, globalPoint,
444 pixelDelta, angleDelta, mods, phase);
445 }
446
447 void handleEnterEvent(const QPointF &localPoint, const QPointF &globalPoint) override
448 {
449 QWindowSystemInterface::handleEnterEvent(m_screenController->getWindowForPoint(localPoint.toPoint()), localPoint, globalPoint);
450 }
451
452 void handleLeaveEvent(const QPointF &localPoint) override
453 {
454 QWindowSystemInterface::handleLeaveEvent(m_screenController->getWindowForPoint(localPoint.toPoint()));
455 }
456
437private:457private:
438 QSharedPointer<ScreenController> m_screenController;458 QSharedPointer<ScreenController> m_screenController;
439};459};
@@ -496,7 +516,7 @@
496516
497Qt::KeyboardModifiers getQtModifiersFromMir(MirInputEventModifiers modifiers)517Qt::KeyboardModifiers getQtModifiersFromMir(MirInputEventModifiers modifiers)
498{518{
499 int qtModifiers = Qt::NoModifier;519 Qt::KeyboardModifiers qtModifiers = Qt::NoModifier;
500 if (modifiers & mir_input_event_modifier_shift) {520 if (modifiers & mir_input_event_modifier_shift) {
501 qtModifiers |= Qt::ShiftModifier;521 qtModifiers |= Qt::ShiftModifier;
502 }522 }
@@ -509,7 +529,7 @@
509 if (modifiers & mir_input_event_modifier_meta) {529 if (modifiers & mir_input_event_modifier_meta) {
510 qtModifiers |= Qt::MetaModifier;530 qtModifiers |= Qt::MetaModifier;
511 }531 }
512 return static_cast<Qt::KeyboardModifiers>(qtModifiers);532 return qtModifiers;
513}533}
514534
515Qt::MouseButtons getQtMouseButtonsfromMirPointerEvent(MirPointerEvent const* pev)535Qt::MouseButtons getQtMouseButtonsfromMirPointerEvent(MirPointerEvent const* pev)
@@ -533,17 +553,44 @@
533void QtEventFeeder::dispatchPointer(MirInputEvent const* ev)553void QtEventFeeder::dispatchPointer(MirInputEvent const* ev)
534{554{
535 auto timestamp = qtmir::compressTimestamp<ulong>(std::chrono::nanoseconds(mir_input_event_get_event_time(ev)));555 auto timestamp = qtmir::compressTimestamp<ulong>(std::chrono::nanoseconds(mir_input_event_get_event_time(ev)));
536
537 auto pev = mir_input_event_get_pointer_event(ev);556 auto pev = mir_input_event_get_pointer_event(ev);
557 auto action = mir_pointer_event_action(pev);
538 qCDebug(QTMIR_MIR_INPUT) << "Received" << qPrintable(mirPointerEventToString(pev));558 qCDebug(QTMIR_MIR_INPUT) << "Received" << qPrintable(mirPointerEventToString(pev));
539559
540 auto modifiers = getQtModifiersFromMir(mir_pointer_event_modifiers(pev));560 auto modifiers = getQtModifiersFromMir(mir_pointer_event_modifiers(pev));
541 auto buttons = getQtMouseButtonsfromMirPointerEvent(pev);
542561
543 auto movement = QPointF(mir_pointer_event_axis_value(pev, mir_pointer_axis_relative_x),562 auto movement = QPointF(mir_pointer_event_axis_value(pev, mir_pointer_axis_relative_x),
544 mir_pointer_event_axis_value(pev, mir_pointer_axis_relative_y));563 mir_pointer_event_axis_value(pev, mir_pointer_axis_relative_y));
545564 auto local_point = QPointF(mir_pointer_event_axis_value(pev, mir_pointer_axis_x),
546 mQtWindowSystem->handleMouseEvent(timestamp, movement, buttons, modifiers);565 mir_pointer_event_axis_value(pev, mir_pointer_axis_y));
566
567 switch (action) {
568 case mir_pointer_action_button_up:
569 case mir_pointer_action_button_down:
570 case mir_pointer_action_motion:
571 {
572 const float hDelta = mir_pointer_event_axis_value(pev, mir_pointer_axis_hscroll);
573 const float vDelta = mir_pointer_event_axis_value(pev, mir_pointer_axis_vscroll);
574
575 if (hDelta != 0 || vDelta != 0) {
576 const QPoint angleDelta = QPoint(hDelta * 15, vDelta * 15);
577 mQtWindowSystem->handleWheelEvent(timestamp, local_point, local_point,
578 QPoint(), angleDelta, modifiers, Qt::ScrollUpdate);
579 } else {
580 auto buttons = getQtMouseButtonsfromMirPointerEvent(pev);
581 mQtWindowSystem->handleMouseEvent(timestamp, movement, buttons, modifiers);
582 }
583 break;
584 }
585 case mir_pointer_action_enter:
586 mQtWindowSystem->handleEnterEvent(local_point, local_point);
587 break;
588 case mir_pointer_action_leave:
589 mQtWindowSystem->handleLeaveEvent(local_point);
590 break;
591 default:
592 qCDebug(QTMIR_MIR_INPUT) << "Unrecognized pointer event";
593 }
547}594}
548595
549void QtEventFeeder::dispatchKey(MirInputEvent const* event)596void QtEventFeeder::dispatchKey(MirInputEvent const* event)
550597
=== 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 @@
52 Qt::KeyboardModifiers mods = Qt::NoModifier) = 0;52 Qt::KeyboardModifiers mods = Qt::NoModifier) = 0;
53 virtual void handleMouseEvent(ulong timestamp, QPointF movement, Qt::MouseButtons buttons,53 virtual void handleMouseEvent(ulong timestamp, QPointF movement, Qt::MouseButtons buttons,
54 Qt::KeyboardModifiers modifiers) = 0;54 Qt::KeyboardModifiers modifiers) = 0;
55 virtual void handleWheelEvent(ulong timestamp, const QPointF &localPoint, const QPointF &globalPoint,
56 QPoint pixelDelta, QPoint angleDelta,
57 Qt::KeyboardModifiers mods = Qt::NoModifier,
58 Qt::ScrollPhase phase = Qt::ScrollUpdate) = 0;
59 virtual void handleEnterEvent(const QPointF &localPoint = QPointF(), const QPointF &globalPoint = QPointF()) = 0;
60 virtual void handleLeaveEvent(const QPointF &localPoint = QPointF()) = 0;
55 };61 };
5662
57 QtEventFeeder(const QSharedPointer<ScreenController> &screenController);63 QtEventFeeder(const QSharedPointer<ScreenController> &screenController);
5864
=== 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 @@
41 MOCK_METHOD5(handleTouchEvent, void(QWindow *window, ulong timestamp, QTouchDevice *device,41 MOCK_METHOD5(handleTouchEvent, void(QWindow *window, ulong timestamp, QTouchDevice *device,
42 const QList<struct QWindowSystemInterface::TouchPoint> &points,42 const QList<struct QWindowSystemInterface::TouchPoint> &points,
43 Qt::KeyboardModifiers mods));43 Qt::KeyboardModifiers mods));
44 MOCK_METHOD4(handleMouseEvent, void(ulong, QPointF, Qt::MouseButtons, Qt::KeyboardModifiers));44 MOCK_METHOD4(handleMouseEvent, void(ulong timestamp, QPointF point, Qt::MouseButtons buttons, Qt::KeyboardModifiers modifiers));
45 MOCK_METHOD7(handleWheelEvent, void(ulong timestamp, const QPointF & local, const QPointF & global, QPoint pixelDelta, QPoint angleDelta,
46 Qt::KeyboardModifiers mods, Qt::ScrollPhase phase));
47 MOCK_METHOD2(handleEnterEvent, void(const QPointF &localPoint, const QPointF& globalPoint));
48 MOCK_METHOD1(handleLeaveEvent, void(const QPointF &localPoint));
45};49};
4650
47namespace testing51namespace testing
4852
=== 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 @@
146 void hoverEnterEvent(QHoverEvent *) override {}146 void hoverEnterEvent(QHoverEvent *) override {}
147 void hoverLeaveEvent(QHoverEvent *) override {}147 void hoverLeaveEvent(QHoverEvent *) override {}
148 void hoverMoveEvent(QHoverEvent *) override {}148 void hoverMoveEvent(QHoverEvent *) override {}
149 void wheelEvent(QWheelEvent *) override {}
149150
150 void keyPressEvent(QKeyEvent *) override {}151 void keyPressEvent(QKeyEvent *) override {}
151 void keyReleaseEvent(QKeyEvent *) override {}152 void keyReleaseEvent(QKeyEvent *) override {}

Subscribers

People subscribed via source and target branches