On 12 November 2016 at 17:14, Carlos Garnacho carlosg@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.
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?
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?
Hi Henri,
Thanks for the comments.
On Mon, Nov 14, 2016 at 9:04 PM, Henri Verbeet hverbeet@gmail.com wrote:
On 12 November 2016 at 17:14, Carlos Garnacho carlosg@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#n...
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
On 14 November 2016 at 23:40, Carlos Garnacho carlosg@gnome.org wrote:
On Mon, Nov 14, 2016 at 9:04 PM, Henri Verbeet hverbeet@gmail.com wrote:
On 12 November 2016 at 17:14, Carlos Garnacho carlosg@gnome.org wrote:
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.
Sure. I don't think there's an issue with always using the VCP at this point, although I don't necessarily have the last word on that. Note though that when the code was added in bd3ec1a973fef5ff46a6242aa4e04dd4b7a40799 we effectively always used the VCP, and that was later disabled for absolute devices. I think the reasons for that no longer apply because we're more careful about checking things in X11DRV_RawMotion(), but it's something to keep in mind. The assumption that the VCP always has ID 2 seems more questionable to me, even if it's probably true in practice.
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#n...
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.
That implies the VCP and VCK are listed first, but I don't think it makes guarantees about what IDs they get.
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.
It's minimal in the sense that we try to avoid unnecessary code, but it's not an attempt to limit usage of XI2 itself.
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.
I'm not sure if it's still true on current X servers, but it used to be the case that relative motion events only got reported on the slave devices, and never on the master 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)
Yeah, that's a known issue. It's uncommon enough that not a lot of people worry about it, but it would be nice to get that fixed as well.
Hi Henri,
On Wed, Nov 16, 2016 at 5:14 PM, Henri Verbeet hverbeet@gmail.com wrote:
On 14 November 2016 at 23:40, Carlos Garnacho carlosg@gnome.org wrote:
On Mon, Nov 14, 2016 at 9:04 PM, Henri Verbeet hverbeet@gmail.com wrote:
On 12 November 2016 at 17:14, Carlos Garnacho carlosg@gnome.org wrote:
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.
Sure. I don't think there's an issue with always using the VCP at this point, although I don't necessarily have the last word on that. Note though that when the code was added in bd3ec1a973fef5ff46a6242aa4e04dd4b7a40799 we effectively always used the VCP, and that was later disabled for absolute devices. I think the reasons for that no longer apply because we're more careful about checking things in X11DRV_RawMotion(), but it's something to keep in mind. The assumption that the VCP always has ID 2 seems more questionable to me, even if it's probably true in practice.
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#n...
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.
That implies the VCP and VCK are listed first, but I don't think it makes guarantees about what IDs they get.
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.
It's minimal in the sense that we try to avoid unnecessary code, but it's not an attempt to limit usage of XI2 itself.
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.
I'm not sure if it's still true on current X servers, but it used to be the case that relative motion events only got reported on the slave devices, and never on the master device.
Right, kindof... As far as I tried this is the case for Wine, because the XGrabPointer call coerces the VCP into "core events" state, so that means no XI2 events in the mean time.
If there were no grab (that's not desired of course), or if the grab was a XI2 one, there would be no such issue in receiving raw motion events coming from the master pointer.
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)
Yeah, that's a known issue. It's uncommon enough that not a lot of people worry about it, but it would be nice to get that fixed as well.
I changed the approach slightly given the restriction above, but still catering for the hotplug case. Made it listen to XIAllDevices, and made the RawMotion handler react exclusively to the current slave driving the client pointer. Although it'll be unlikely it receives anything else anyway, the master pointer itself is taken away by the grab, and extra master pointers or slave pointers not attached to the VCP are highly unlikely.
The patches are in wine-patches ML.
Cheers, Carlos
-
Carlos Garnacho -
Henri Verbeet