QtMir

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

Undecided

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Daniel d'Andrada (community)		2015-10-13	Abstain on 2015-10-21
Gerry Boland (community)			Approve on 2015-10-15
PS Jenkins bot (community)	continuous-integration	2015-10-13	Needs Fixing on 2015-10-14
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

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-01: Posted in a previous version of this proposal

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

review: Approve (continuous-integration)

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2015-10-06: 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 on 2015-10-07: 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 on 2015-10-07: Posted in a previous version of this proposal

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

review: Approve (continuous-integration)

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2015-10-07: 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 on 2015-10-07: 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 on 2015-10-07: Posted in a previous version of this proposal

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

review: Approve (continuous-integration)

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2015-10-07: 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 on 2015-10-08: 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 on 2015-10-08: Posted in a previous version of this proposal

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

review: Approve (continuous-integration)

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2015-10-08: 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 on 2015-10-13:

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 updated on 2015-10-14

389. By Lukáš Tinkl on 2015-10-14: merge lp:~unity-team/qtmir/touch_tracing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-10-14:

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)

Revision history for this message

Gerry Boland (gerboland) wrote on 2015-10-15:

LGTM

review: Approve

Revision history for this message

Daniel d'Andrada (dandrader) on 2015-10-21:

review: Abstain

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Daniel d'Andrada

Lukáš Tinkl

Mir development team

 === 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 {}