Created attachment 288392 [details] [review]
core: Unset "pointer emulating" sequence after event processing

meta_display_is_pointer_emulating_sequence() must hold valid for the
sequence during all its lifetime, which is not currently true during
the processing of CLUTTER_TOUCH_END, so perform the update after that
event is processed.

(In reply to comment #0)
> Testing a wayland 3.14.0 session, I don't get any touch input for applications
> using the wayland backend. Using X wayland, they do get touch input

FTR, I currently do expect the opposite... Xwayland has no implementation/translation of wl_touch events yet, which makes X11 apps insensitive to touch. Maybe there was some mishap on the expected/used environment variables?

I tested and input only works with native Wayland for me.

You are correct. Retesting with 3.14.1, I get touch even when using the wayland backend, no touch input when using xwayland.

the sticky drag with touch still persists in 3.14.1

Jasper, does the patch in comment 1 make sense to you ?

Review of attachment 288392 [details] [review]:

Ah, I didn't notice this patch. Nice catch.

As for this, are we sure this is what we want? Should we update slot_serial *after* we send out the up event to all clients? That seems wrong to me.

(In reply to comment #7)
> Review of attachment 288392 [details] [review]:
> 
> As for this, are we sure this is what we want? Should we update slot_serial
> *after* we send out the up event to all clients? That seems wrong to me.

Not sure what you mean :), maybe you've mistaken meta_wayland_touch_update() with meta_display_update_pointer_emulating_sequence()? The latter updates the sequence used for "pointer emulation" within the shell (event if wayland doesn't have that concept). This is so certain operations like window dragging are consistently restricted to one touch at a time.

slot_serial is still being updated in the former function, called inside meta_wayland_compositor_update() early in meta_display_handle_event().

(In reply to comment #8)
> (In reply to comment #7)
> > Review of attachment 288392 [details] [review] [details]:
> > 
> > As for this, are we sure this is what we want? Should we update slot_serial
> > *after* we send out the up event to all clients? That seems wrong to me.
> 
> Not sure what you mean :), maybe you've mistaken meta_wayland_touch_update()
> with meta_display_update_pointer_emulating_sequence()?

Oops, yes I did.

Re-review: meta_display_update_pointer_emulating_sequence() has two paths: one for BEGIN and one for END. There is no need to call it on UPDATE at all from what I can tell. If we're going to be already branching at two different spots, perhaps it just makes sense to inline the two paths and add a comment.

(In reply to comment #9)
> Re-review: meta_display_update_pointer_emulating_sequence() has two paths: one
> for BEGIN and one for END. There is no need to call it on UPDATE at all from
> what I can tell. If we're going to be already branching at two different spots,
> perhaps it just makes sense to inline the two paths and add a comment.

Sure, having this separate in a function felt a bit more verbose to me, but the variable to be updated is also descriptive enough.

FWIW, I've send some patches to xorg-devel to have xwayland proxy touch events to X11 clients, which would fix the former first part of this bug:
http://lists.x.org/archives/xorg-devel/2014-October/044373.html

Created attachment 289212 [details] [review]
core: Unset "pointer emulating" sequence after event processing

The set/unset branches of meta_display_update_pointer_emulating_sequence()
have been split and put directly where it makes sense. The pointer emulated
sequence will be updated before processing the CLUTTER_TOUCH_BEGIN, and
after processing the CLUTTER_TOUCH_END, this way the checks on this hold
true during all the sequence lifetime.

Review of attachment 289212 [details] [review]:

Yep.

Attachment 289212 [details] pushed as df38496 - core: Unset "pointer emulating" sequence after event processing