Conversation
notgull
added
B - bug
| const KEYCODE_OFFSET: u8 = 8; | ||
|
|
||
| /// Minimum delta for a device event to be considered. | ||
| const NOTGULL_THRESHOLD: f32 = 4.0e-8; |
There was a problem hiding this comment.
I should insert my username in more source code! 😁
(I've tried hard with the mtm: MainThreadMarker, but while mtm is my initials, nothing beats just spelling out the whole thing)
There was a problem hiding this comment.
Should be renamed to something useful. Also it should be explained why the given constant was picked in the first place, otherwise it may not work for some cases.
There's also a case where you move the mouse really slowly, thus filtering may not actually move the mouse where it should or accumulate in error.
There was a problem hiding this comment.
I've elected to use the float epsilon instead. Since float comparison breaks down past the epsilon, for all intentions and purposes any number less than the epsilon is zero.
| const KEYCODE_OFFSET: u8 = 8; | ||
|
|
||
| /// Minimum delta for a device event to be considered. | ||
| const NOTGULL_THRESHOLD: f32 = 4.0e-8; |
There was a problem hiding this comment.
Should be renamed to something useful. Also it should be explained why the given constant was picked in the first place, otherwise it may not work for some cases.
There's also a case where you move the mouse really slowly, thus filtering may not actually move the mouse where it should or accumulate in error.
| if mouse_delta.0.abs() as f32 >= NOTGULL_THRESHOLD | ||
| || mouse_delta.1.abs() as f32 >= NOTGULL_THRESHOLD |
There was a problem hiding this comment.
I don't quite like this logic in-particular. Could you instead check if the given value is small enough and null it? Then just send the thing if something is not null, since those low values don't make much sense in the first place.
|
This wouldn't fix the underlying issue. Interestingly, I didn't had any issues with Bevy for example, but its also not a regression. Didn't want to use thresholds because normally raw mouse input events should not be less than 1.0 But I also don't know how to trace X11 ^^' |
I think the underlying issue is that the underlying driver is making an error somewhere in the floating point calculation and sending us a very tiny number instead of zero. |
|
Then this would definitely be a valid fix, but the value is always near zero, even if there is movement, that's why I don't think it's a floating point error. |
My understanding of Xinput's raw mouse events is that they are always multiples of 1. So we shouldn't be seeing any floating point numbers anyways. So it's fine for us to assume that a value less than the epsilon is, for all intents and purposes, supposed to be zero. |
notgull
force-pushed
the
notgull/delta
branch
from
6fcac66 to
2893de5
Compare
Usually, if mouse events are equal to (0, 0) we filter them out. However, if the event is very close to zero it will still be given to the user. In some cases this can be caused by bad float math on the X11 server side. Fix it by filtering absolute values smaller than floating point epsilon. Signed-off-by: John Nunley <dev@notgull.net> Closes: #3500
| (true, false) => (this.x, 0.0), | ||
| (false, true) => (0.0, this.y), |
There was a problem hiding this comment.
Hey yall :) The code here is mistakenly returning the axis that's lower than the epsilon.
It's the likely cause of this issue in Bevy: bevyengine/bevy#12198.
| (true, false) => (this.x, 0.0), | |
| (false, true) => (0.0, this.y), | |
| (false, true) => (this.x, 0.0), | |
| (true, false) => (0.0, this.y), |
5 participants
Usually, if mouse events are equal to (0, 0) we filter them out.
However, if the event is very close to zero it will still be given to
the user. In some cases this can be caused by bad float math on the X11
server side.
I fix this by refusing to forward events where both of their deltas fall
below a certain threshold.
Closes #3500
CHANGELOG.mdif knowledge of this change could be valuable to users