QtMir

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

  1. wheelEvent
  2. Merge into trunk
Proposed by Lukáš Tinkl
Status: Merged
Approved by: Gerry Boland
Approved revision: 389
Merged at revision: 396
Proposed branch: lp://staging/~lukas-kde/qtmir/wheelEvent
Merge into: lp://staging/qtmir
Prerequisite: lp://staging/~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://staging/~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.staging.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

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

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

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

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

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://staging/~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 :

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 :

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
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches