Bug 56558 - Button state on emulated pointer events is incorrect
Summary: Button state on emulated pointer events is incorrect
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/Input/Core (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Peter Hutterer
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on: 56557
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-30 00:57 UTC by Peter Hutterer
Modified: 2012-11-06 01:51 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
0001-Xi-put-the-state-into-the-event-_before_-updating-th.patch (3.16 KB, patch)
2012-10-30 06:07 UTC, Peter Hutterer
no flags Details | Splinter Review
patch, update device state after delivering emulated events (1.66 KB, patch)
2012-10-31 18:34 UTC, Carlos Garnacho Parro
no flags Details | Splinter Review
patch, set modifier mask on touch events (1.37 KB, patch)
2012-10-31 18:35 UTC, Carlos Garnacho Parro
no flags Details | Splinter Review

Description Peter Hutterer 2012-10-30 00:57:24 UTC
Touch-emulated pointer events must have the same button state as non-emulated button events.

Touch sequence: TouchBegin TouchUpdate TouchEnd
Expected pointer events (state):
Motion(0x0) ButtonPress(0x0) Motion(0x100) ButtonRelease(0x100)

Currently the state is:
Motion(0x100) ButtonPress(0x100) Motion(0x100) ButtonRelease(0x0)

Cause is that the button events are delivered after the touch events, which update the pointer state.


Carlo's patch http://lists.freedesktop.org/archives/xorg-devel/2012-October/034229.html fixes the state for the initial Motion event.
Comment 1 Peter Hutterer 2012-10-30 06:07:47 UTC
Created attachment 69284 [details] [review]
0001-Xi-put-the-state-into-the-event-_before_-updating-th.patch

Tentative patch. I don't have a full set of tests yet, especially in regards to checking the correct states on touch events, but it passes my tests for pointer emulation checks.
Comment 2 Daniel Drake 2012-10-30 16:15:15 UTC
Thanks for finding/investigating Peter.

Carlos Garnacho has tested the above patch; it does fix a problem that we are seeing in OLPC/Sugar.
http://bugs.sugarlabs.org/ticket/4133
Comment 3 Carlos Garnacho Parro 2012-10-31 18:28:56 UTC
(In reply to comment #1)
> Created attachment 69284 [details] [review] [review]
> 0001-Xi-put-the-state-into-the-event-_before_-updating-th.patch
> 
> Tentative patch. I don't have a full set of tests yet, especially in regards
> to checking the correct states on touch events, but it passes my tests for
> pointer emulation checks.

Hey Peter :), been trying out this patch, but it brings in other issues on touch events, doing event_set_state() on the ET_Touch* InternalEvent itself (as opposed to the made up motion/button ones) makes touch events contain button 1 set in the mask, which do cause other glitches on how events get to GTK+.

Now, I can see the usefulness of setting the paired keyboard modifier mask on touch events, I'm attaching 2 patches to do that separately, although this other issue probably belongs to another bug
Comment 4 Carlos Garnacho Parro 2012-10-31 18:34:48 UTC
Created attachment 69366 [details] [review]
patch, update device state after delivering emulated events
Comment 5 Carlos Garnacho Parro 2012-10-31 18:35:22 UTC
Created attachment 69367 [details] [review]
patch, set modifier mask on touch events
Comment 6 Peter Hutterer 2012-11-02 06:24:58 UTC
Whoops, you're right. I forgot that the button state shouldn't include touch. Patches look good. I'll test them a bit more but expect them upstream soon. thanks!
Comment 7 Peter Hutterer 2012-11-06 01:50:12 UTC
commit 863f32c930d71073ee5f78452b78bd459d024867
Author: Carlos Garnacho <carlosg@gnome.org>
Date:   Wed Oct 31 19:32:57 2012 +0100

    Xi: Update the device after delivering the emulated pointer event(#56558)
Comment 8 Peter Hutterer 2012-11-06 01:51:04 UTC
commit b4e44b285ed0eee1d06514215a4b01d54f40094b
Author: Carlos Garnacho <carlosg@gnome.org>
Date:   Wed Oct 31 19:29:45 2012 +0100

    Xi: Set modifier mask on touch events


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.