-
Notifications
You must be signed in to change notification settings - Fork 948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x11: Always use correct window ID for XInput2 events #372
Conversation
The intention of the artificial |
Right, and that seems like a good way to go. |
f2ac868
to
36d6d86
Compare
A |
Is there a need for additional handling of cursors that were positioned within the window when focus was lost, but are not when focus is regained? |
36d6d86
to
51ad6a8
Compare
Well, I tried sending Since we're using I think the most correct implementation in terms of the spec would be to use the While what I've implemented is more usable for more people, I feel that trying to compensate for the inadequacies of multi-pointer support is a dangerous road to go down. From what I've read, basically none of the WMs in use are designed with multi-pointer in mind, and we can't fight the WM's notion of focus. |
I'm feeling increasingly confused about the behaviors expected by X, the users, and window managers in turn. I agree that mpx probably doesn't deserve intense care, being (yet more) X esoterica that will be rendered further irrelevant as Wayland adoption grows. If there's no obvious way to be both correct and useful, I have no argument against doing the relatively straightforward and mostly sane thing. |
51ad6a8
to
a888cdc
Compare
Alright, looks like this is finally done. As an added bonus, in light of the changes made when modifiers were added to mouse events, I've:
|
Do we want that though? A |
Come to think of it, all the relevant XInput2 events have a While keyboard handling doesn't use XInput2 yet, that only accounts for a single call, and it will be removed anyway by my upcoming libxkbcommon PR, so I can just revert that to doing the logic inline. |
The `CursorMoved` events that are used to send position updates alongside `Focused` and `CursorEntered` events were using incorrect values for the window ID. This is a direct result of the X11 backend being hard to understand, as those values came from variables in the top-level scope of the function, which one would assume to be valid throughout the entirety of their scope. In reality, their validity is dependent on the event belonging to the `XEvent` union, so very surprising things can happen if those variables are read in the case of XInput2/XKB/etc. events. To prevent future accidents, the aforementioned variables have been removed, and are now defined per-event instead. Additionally, the `CursorMoved` event sent alongside `Focused` now uses the correct device ID; it previously used the ID of a master keyboard, but now uses the ID of the pointer paired to that keyboard. Note that for those using multi-pointer X, the correctness of this ID is dependent on the correctness of the window manager's focus model.
a888cdc
to
515b6b9
Compare
This should be good to go now, if there aren't any other objections. |
Thanks! |
Partially addresses rust-windowing#372.
Implement nested SVG clip paths in the D3D11 backend. Partially addresses rust-windowing#372.
… r=pcwalton Allow clip paths to nest in the canvas API. Closes rust-windowing#372.
The
CursorMoved
events that are used to send position updates alongsideFocused
andCursorEntered
events were using incorrect values for the window ID. This is a direct result of the X11 backend being hard to understand, as those values came from variables in the top-level scope of the function, which one would assume to be valid throughout the entirety of their scope. In reality, their validity is dependent on the event belonging to theXEvent
union, so very surprising things can happen if those variables are read in the case of XInput2/XKB/etc. events. To help prevent future accidents, the aforementioned variables have been removed, and are now defined per-event instead.However, the
CursorMoved
event sent alongsideFocused
events is still imperfect. The associatedDeviceId
is for the core virtual keyboard, as opposed to the core virtual pointer, which is the expected value on all otherCursorMoved
events. I'm not aware of any straightforward solution to this, as window focus isn't inherently tied to pointer devices.