winex11: Always use the Virtual Core Pointer for raw events

[Carlos Garnacho]

Hi Henri,

Thanks for the comments.

On Mon, Nov 14, 2016 at 9:04 PM, Henri Verbeet <hverbeet at gmail.com> wrote:
> On 12 November 2016 at 17:14, Carlos Garnacho <carlosg at gnome.org> wrote:
>> Second, in the unlikely case that Wine managed to detect a master
>> pointer other than the VCP (this requires user intervention, creating
>> a second pointer/keyboard pair and assigning HW devices to it) This
>> second pointer would be unaffected by the core X calls performed by
>> Wine (eg. grabs,cursor). This could eg. result in Wine grabbing/confining
>> one pointer, while listening to raw events from another, which is
>> otherwise unaffected by the grab.
>>
> I'm not sure that's necessarily wrong, but using XIGrabDevice(),
> XIWarpPointer(), etc. would probably be enough to fix that.

Indeed, but that is a major undertaking, esp. with grabs as they will
coerce XI2 events, at that point it might be better to make all event
handling go over XI2.

>
>> The Virtual Core Pointer is a given, it always has ID=2, it's created
>> by default, it can't be removed, it's the one getting HW devices attached
>> to by default, and it will be the device implicitly used by the other
>> X core calls. It's the device Wine wants to listen to despite its initial
>> state.
>>
> That probably works in practice, but I can't think of anything in the
> protocol spec that guarantees the VCP has id 2. Did I miss something
> there? How about XIGetClientPointer(), would that do the right thing?

Right, I agree it's better to get the client pointer, as that will
match the master pointer as seen by X11 core events. That said, to my
knowledge there is no WM that bothers setting different per-client
pointer devices (calling XISetClientPointer is WM responsibility, and
only matters in practice if the WM deals with multipointer), so this
is a glorified way to get "2", still more correct nonetheless.

As for VCP=2 being guaranteed, the wording in the spec is a bit
subtle, please read:
https://cgit.freedesktop.org/xorg/proto/inputproto/tree/specs/XI2proto.txt#n135

This in practice means that VCP/VCK are the first devices created
(they get ID 2 and 3 because 0/1 are reserved for
XIAllDevices/XIAllMasterDevices special IDs), and they can't be
removed to satisfy XI1 clients, effectively trying "xinput
remove-master 2" on the terminal gives me XI_BadDevice.

>
>> The main purpose of this patch is fixing master device detection over
>> Xwayland, which is affected by the relative axes check. Xwayland will
>> most usually emit pointer events as coming from a device with absolute
>> axes, only switching to emitting relative motion when a very specific
>> set of conditions are met (thought out for gaming basically). But most
>> usually these conditions don't apply yet when XI2 is enabled, so the
>> VCP is skipped and it ends up with xi2_core_pointer=0, thus ignoring
>> the relative motion that is later sent.
>>
>> But again, this is not Xwayland specific, it could happen as well with
>> the right conditions on traditional X servers.
>>
> Wouldn't the correct way to handle this kind of thing be to respond to
> XI_DeviceChanged and XI_HierarchyChanged events?

Right, using those wasn't my first option as XI2 usage in Wine seemed
as minimal as possible on purpose to me.

Actually, if XI_DeviceChanged/RawMotion is tracked on the VCP, and
relative axes are checked on the most recent XIDeviceInfo as obtained
in DeviceChanged, Wine might not need knowing much about slave devices
after all, at least as for the "raw motion" purpose goes. The VCP
would be mimicking the currently used device.

And this would solve another bug I see with Wine: plugging in a mouse
after enable_xinput2() results in no raw events from it. The slave
devices' XIDeviceInfos are obtained at enable_xinput2() time, and so
is selecting for XI_RawEvent on those, so the newly plugged mouse is
ignored until enable_xinput2() happens again while it's plugged in
(eg. unfocusing and focusing back)

If it is agreed this is a good direction, I'll make another patch(es).

Cheers,
  Carlos