Mir

Merge lp://qastaging/~afrantzis/mir/always-consume-input-events-in-clients into lp://qastaging/mir

  1. always-consume-input-events-in-clients
  2. Merge into development-branch
Proposed by Alexandros Frantzis
Status: Rejected
Rejected by: Alan Griffiths
Proposed branch: lp://qastaging/~afrantzis/mir/always-consume-input-events-in-clients
Merge into: lp://qastaging/mir
Diff against target: 248 lines (+113/-82)
2 files modified
src/client/mir_surface.cpp (+13/-6)
tests/unit-tests/client/test_client_mir_surface.cpp (+100/-76)
To merge this branch: bzr merge lp://qastaging/~afrantzis/mir/always-consume-input-events-in-clients
Reviewer Review Type Date Requested Status
Alan Griffiths Needs Information
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+209313@code.qastaging.launchpad.net

Commit message

client: Process input events even in clients that don't handle input

Description of the change

client: Process input events even in clients that don't handle input

This is one alternative solution for bug 1213804. See comments in bug 1213804 for more information and pointers to other options.

Currently, focused client surfaces that don't handle input events don't process them at all. Such clients never acknowledge receiving input events, causing the input system to retry sending the events.

If in the interval between two send attempts another app gains the focus, the input system sends the event to the newly focused app/surface, leading to bug 1213804 (i.e. the target of events is always the currently focused surfaced, not a particular surface).

Ideally, the input system should be able to handle this situation better, e.g., by resending the event only if the focused surface the event was originally targeted at hasn't changed. However, the input subsystem is full of intricacies and I am not familiar enough with it to tell if this behavior is a bug or has a particular reason for being there.

If we decide that the proposed solution/workaround is not acceptable, I can dive more deeply into the abyss known as the input subsystem.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1445
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/980/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1115
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1113
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/699
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/712
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/712/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/717
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/717/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/700
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/700/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/666
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4463

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/980/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

It definitely sounds like a workaround more than a fix. It also leads to clients being woken up by events they should never receive at all. But we possibly already had that problem...

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

> It definitely sounds like a workaround more than a fix. It also leads to
> clients being woken up by events they should never receive at all. But we
> possibly already had that problem...

I guess toolkits will always have event handles set?

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

It sounds to me as though the real problem is in the server - that if a client fails to consume an event then "bad stuff happens". I'd rather fix the "bad stuff happens" - but if that is a lot of work then this attempt to mitigate the problem is OK (at least for now).

review: Needs Information
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

InputDispatcher.cpp has its own app switching logic which does a lot of interesting things, as there is something like a dedicated app switch key... Maybe we have to drop all key events when focused surface changes?

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> I'd rather fix the "bad stuff happens" - but if that is a lot of work then this attempt to mitigate the problem is OK (at least for now).

> Maybe we have to drop all key events when focused surface changes?

I am looking more into the input subsystem to see if there is a clean and safe way we can handle the issue there.

Unmerged revisions

1445. By Alexandros Frantzis

client: Process input events even in clients that don't handle input

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