Opened 8 years ago

Last modified 7 years ago

#4132 assigned defect

Xorg pointer emulation issues

Reported by: garnacho Owned by: garnacho
Priority: Urgent Milestone:
Component: Write Version: 0.97.x
Severity: Critical Keywords: upstream
Cc: dsd Distribution/OS: OLPC
Bug Status: Assigned

Description

Certain emulated pointer events resulting from touch events may contain a bogus button mask, concretely the first virtual motion event, and the ButtonPress/Release emulated events. this breaks invariants in clients (gtk+ and abiword known to be affected) that button 1 mask will be only set while between the corresponding ButtonPress/Release events.

An initial patch was submitted to xorg-devel:
http://lists.x.org/archives/xorg-devel/2012-October/034229.html

But other miscellaneous problems arose, that also need fixing:
http://lists.x.org/archives/xorg-devel/2012-October/034238.html

Attachments (1)

0001-Xi-update-device-state-even-if-no-emulated-events-ar.patch (963 bytes) - added by garnacho 8 years ago.
Patch to fix wrong state after touch capture

Download all attachments as: .zip

Change History (17)

comment:1 Changed 8 years ago by erikos

  • Bug Status changed from Unconfirmed to Assigned
  • Cc garnacho added
  • Distribution/OS changed from Unspecified to OLPC
  • Milestone changed from Unspecified by Release Team to 0.98
  • Priority changed from Unspecified by Maintainer to Urgent
  • Severity changed from Unspecified to Critical
  • Version changed from Unspecified to 0.97.x

comment:2 Changed 8 years ago by erikos

  • Cc garnacho removed
  • Keywords upstream added
  • Owner changed from godiard to garnacho
  • Status changed from new to assigned

comment:3 follow-up: Changed 8 years ago by dsd

  • Cc dsd added

The initial patch will be included in the next OLPC build.

If the other problems found are affecting us, please advise me when they go upstream so that I can add them as well.

Thanks for your work here!

comment:4 in reply to: ↑ 3 Changed 8 years ago by garnacho

Replying to dsd:

If the other problems found are affecting us, please advise me when they go upstream so that I can add them as well.

Ticket #4133 has been found to be a side effect of the button mask issues on button events, so I think we're only unaffected by the "button events on root window" issue, as all we do there is passive grabs, and those work fine

comment:5 Changed 8 years ago by garnacho

Work continued on https://bugs.freedesktop.org/show_bug.cgi?id=56558 , 2 patches have just been submitted there

comment:6 Changed 8 years ago by dsd

I've added those 2 patches to xorg-x11-server-1.13.0-7.fc18.olpc1 for the next 13.1.0 build.

comment:7 Changed 8 years ago by erikos

Daniel, the xorg-x11-server* has some noticeable regressions:

  • undesired selections (e.g. Write #4265)
  • undesired selections in Home
  • scrolling in Write does trigger an input focus change

I did a downgrade, and those issues seem to be gone: http://armpkgs.fedoraproject.org/packages/xorg-x11-server/1.13.0/7.fc18/armv7hl/xorg-x11-server-common-1.13.0-7.fc18.armv7hl.rpm http://armpkgs.fedoraproject.org/packages/xorg-x11-server/1.13.0/7.fc18/armv7hl/xorg-x11-server-Xorg-1.13.0-7.fc18.armv7hl.rpm (for reproduction, installed with sudo rpm -U --force)

comment:8 Changed 8 years ago by godiard

Is the gdk patch from #4133 needed or already included?

comment:9 follow-up: Changed 8 years ago by dsd

The difference between xorg-x11-server-1.13.0-7.fc18 (old good version from build 13) and xorg-x11-server-Xorg-1.13.0-7.fc18.olpc1 (new buggy? version included in build 14+) is:

Patch removed: Xi: Call UpdateDeviceState() after the first emulated motion event

Patch added: Xi: Set modifier mask on touch events

Patch added: Xi: Update the device after delivering the emulated pointer event(#56558)

My understanding was that the patch we removed never went upstream, and was instead replaced by the 2 patches that were added (and are now upstream). So we will need Carlos Garnacho to clarify the situation here and/or figure out whats going on, if we have found a new bug.

comment:10 in reply to: ↑ 9 Changed 8 years ago by garnacho

Replying to dsd:

My understanding was that the patch we removed never went upstream, and was instead replaced by the 2 patches that were added (and are now upstream). So we will need Carlos Garnacho to clarify the situation here and/or figure out whats going on, if we have found a new bug.

Effectively that has been the case. I'll check the behavior issues, although those look pretty much unrelated to these patches, as the code these patches touch only get triggered through touch devices, and the code that does non-touch devices also updates the device state on its own

Changed 8 years ago by garnacho

Patch to fix wrong state after touch capture

comment:11 Changed 8 years ago by garnacho

Passive grabs were hitting a different codepath on emulated events delivery that didn't get the device updated, this has been visible on sugar due to the global gesture grabber, with this patch the button1 mask is unset correctly after those, so no extraneous selections happen

comment:12 Changed 8 years ago by erikos

comment:13 Changed 8 years ago by dsd

Patch added to xorg-x11-server-1.13.0-7.fc18.olpc2 for next build

comment:14 Changed 8 years ago by dsd

I can reproduce the Write issue also in that new version (1.13.0-7.fc18.olpc2). The test case is something like this:

  • Start Sugar
  • Do some touchy stuff - open/close the frame
  • Open Write
  • Type in a few lines of text
  • Create a text selection with the touchscreen
  • Use the touchscreen to drag and move the selected text to another position in the document. The text should still be selected at the end.
  • Move the mouse. Without clicking or touching, the text will act as if it is being dragged by the mouse cursor.
  • Close write with the mouse
  • Open Paint with the mouse
  • Move the mouse cursor around the screen. A line will be drawn as if the mouse button is being held down.

xorg-x11-server-1.13.0-7.fc18 includes: Xi: Call UpdateDeviceState() after the first emulated motion event

This version is working well, w.r.t. this issue. But that patch didn't go upstream.

xorg-x11-server-1.13.0-7.fc18.olpc1
drops: Xi: Call UpdateDeviceState() after the first emulated motion event
adds: Xi: Set modifier mask on touch events
adds: Xi: Update the device after delivering the emulated pointer event(#56558)

The non-upstream patch was dropped, and was replaced by the 2 upstream patches which are supposed to be equivalent.

This introduced the bug described here.

xorg-x11-server-1.13.0-7.fc18.olpc1 adds Xi: update device state even if no emulated events are sent

The bug described here is still present.


So, I think for the time being, we can just downgrade to xorg-x11-server-1.13.0-7.fc18, going back to the non-upstream " Xi: Call UpdateDeviceState() after the first emulated motion event" approach. I believe the only reason we started shuffling the patches around in .olpc1 (causing breakage) was to adopt the way that upstream fixed the problem. But the upstream fixes now don't seem to be equivalent, and I don't think adopting those patches fixes any other issues we were seeing. Am I missing anything?

Obviously we do want this fixed properly and fixed upstream, but just in terms of restoring the non-buggy behaviour we had a couple of weeks ago, downgrading this package for the time being could make sense for us.

comment:15 Changed 8 years ago by dsd

For 13.1.0 build 16 I went back to xorg-x11-server-1.13.0-7.fc18 as described above, which includes a non-upstream patch. I think we will stay with this for 13.1.0 final. We do need Garnacho's continued help getting this fixed properly upstream though (not urgent) for the future.

comment:16 Changed 7 years ago by dnarvaez

  • Milestone 0.98 deleted

Milestone 0.98 deleted

Note: See TracTickets for help on using tickets.