Bug 56558

Summary:

Button state on emulated pointer events is incorrect

Product:

xorg

Reporter:

Peter Hutterer <peter.hutterer>

Component:

Server/Input/Core

Assignee:

Peter Hutterer <peter.hutterer>

Status:

RESOLVED FIXED

QA Contact:

Xorg Project Team <xorg-team>

Severity:

normal

Priority:

medium

CC:

carlosg, dan, peter.hutterer

Version:

git

Hardware:

Other

OS:

All

Whiteboard:

i915 platform:

i915 features:

Bug Depends on:

56557

Bug Blocks:

Attachments:

Description	Flags
0001-Xi-put-the-state-into-the-event-_before_-updating-th.patch	none
patch, update device state after delivering emulated events	none
patch, set modifier mask on touch events	none

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.