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
InputCommon/XInput2: Added an axis output for the scroll wheel #11363
InputCommon/XInput2: Added an axis output for the scroll wheel #11363
Conversation
You broke the submodules somehow. |
ops thought I'd stripped out those changes before, I can fix that :P |
8f31242
to
e0ca33a
Compare
e0ca33a
to
c3018fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than that this looks fine to me now.
if (XIMaskIsSet(raw_event->valuators.mask, 3)) | ||
{ | ||
// Scroll wheel input gets scaled to be similar to the mouse axes | ||
delta_delta = raw_event->raw_values[0] * 8.0 / scroll_increment; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was unable to find good documentation on this, but I assume you tested this and it works as expected? I find it a bit odd that raw_event->raw_values[0]
is used here when I would expect raw_values[3]
based on the other two branches here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Searching around a bit more, I think our handling of raw_values
and valuators.mask
is wrong in general; based on https://people.freedesktop.org/~whot/xi2-recipes/part4.c it's supposed to be used like this:
size_t value_idx = 0;
for (int i = 0; i < event->valuators.mask_len * 8; ++i) {
if (XIMaskIsSet(event->valuators.mask, i)) {
const double value = event->raw_values[value_idx];
// use value here
++value_idx;
}
}
I guess it just happens to work as-is because bit 1 is never set when bit 0 is clear...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, XInput2 (as with a lot of X11 stuff) is not very well documented. I think you're correct here. In practice, what already exists was mostly fine, since (from what I've tested), events for mouse X and Y, and events for scroll axes are always sent separately, but that's mostly an implementation detail I think. It shouldn't be much trouble to change it
^ Can someone familiar with XInput2 take a look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Untested.
I'll just merge this and see if anyone yells... |
Awesome, cool :) |
On X11, mouse wheel inputs are almost impossible to produce since the ButtonPress and ButtonRelease events are sent simultaneously. I'd previously attempted to fix this by adding an input buffer (#8815) but that wasn't quite right, and doing a completely working implementation of that seems tricky. As an alternative, XInput2 does expose the scroll wheel as an axis (similar to on Windows), so this provides that as an axis (Axis Z), and from my testing seems to work fairly reliably.