Skip to content
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

Port acceleration code to fixed point math using fixedptc.h #28

Draft
wants to merge 11 commits into
base: fixedpoint
Choose a base branch
from

Conversation

Cloudperry
Copy link

@Cloudperry Cloudperry commented Mar 4, 2023

This is a port of the current float based acceleration code to fixed point math using fixedptc.h from here.

All acceleration parameters should mostly work as in the float based version, but there are a few missing features and inconveniences:

  • Parameter defaults from config.h are converted to string from the C code that generates the fixed point number. This results in a string that contains the value as a human readable decimal number, but is very messy.
  • Parameter updating at runtime is not implemented yet. You will need to recompile the module with a new config.h to change the parameters. This is a bit hard to implement as fixedptc doesn't have a string to fixed point parser. Maybe libfixmath's parser could be adapted to fixedptc.
  • Fixedptc.h defaults to 24 bits for the integer part and 8 bits for the fractional part of the number. For this module 16 bits for the int part and 16 bits for the fractional part would be better for accuracy. Fixedptc allows the user to configure these, but I got no mouse movement when trying to change these.

IMO this PR will probably be easier to make into a final polished version than #29. To be usable, this version would just need a string to fixed point parser and new parameter updating code using that parser. The libfixmath version would need a bunch of low level debugging to find out why completely valid fixed point operations cause overflows. More details in that PR.

@Cloudperry Cloudperry changed the base branch from master to fixedpoint March 4, 2023 22:22
@systemofapwne
Copy link
Owner

Judging on the code changes, this looks straight forward so far.

This define should probably be moved to accel.c, because this changes fixedptc.h unnecessarily
samuk10 added a commit to samuk10/leetmouse that referenced this pull request Mar 29, 2024
@samuk10
Copy link

samuk10 commented Mar 30, 2024

hi, im using yours PR!
i made a video a managed to work with leetmouse-gui also: https://www.youtube.com/watch?v=wBSriyVY4qc

Cloudperry added 4 commits May 2, 2024 15:33
When doing the initial port I didn't see that the frametime calculations use large numbers (nanoseconds) that don't fit into 16 bits. The libfixmath version is also broken, because of this. It could be fixed by doing integer division down to microseconds before calculating ms (frametime). This would lose some precision compared to just using the nanosecond timestamps.
…gle correction. Remove float related includes.
@Cloudperry
Copy link
Author

I added rotation, because I wanted to try it (with my grip the sensor of my mouse is quite tilted). Currently using a bit over 8 degrees of rotation. Next time I work on this driver, I will probably add updating of parameters through sysfs.

There are two ways of implementing it that I've been considering. I could make the kernel parameters 32-bit ints and do the conversion from string to fixed-point in a GUI or CLI tool. The other way would be to find or make a C function for converting a string to FP. With the string to FP C functions I have looked at online, I'm not sure if they handle conversion to the closest representable number correctly. Also they seemed to be pretty messy and there could be security problems. This is why I have been thinking about putting the conversion in a GUI/CLI that uses Zig, Nim or C++.

@nJ3ahxac
Copy link

nJ3ahxac commented Jul 15, 2024

Just want to add that for me, with my mouse set to 8000 dpi, changing the precision of fixedpt to allow more bits on the right hand side has had only positive effects (FIXEDPT_WBITS 12). Because the DPI of my mouse was so high, reasonable acceleration values were much closer to zero than what you are using (0.001 - 0.01). I noticed some strange behaviour with the standard 24 bit left hand side values where at around 0.002 there was zero mouse acceleration, but after that point it was very noticeable. I suspect this is because (1 / 2 ** 8) is around 0.004, so it was being rounded to zero when set below 0.002 and 0.004 when above. It's probably going to be necessary for anyone using this to modify FIXEDPT_WBITS to allow for more precision when using higher DPI mice, which naturally work with lower numbers.

I think it would be nice if we could just use 64 bits of precision but that only works when you link against libgcc and afaik you can't do that in a kernel module.

This is great work btw, thanks for doing this.

@Cloudperry
Copy link
Author

I noticed some strange behaviour with the standard 24 bit left hand side values where at around 0.002 there was zero mouse acceleration, but after that point it was very noticeable. I suspect this is because (1 / 2 ** 8) is around 0.004, so it was being rounded to zero when set below 0.002 and 0.004 when above.

Yeah thats exactly the reason why low mouse acceleration values don't work with the current value of 24 integer part bits. Initially I wanted to use 12 or 16 bits for the integer part as that would be optimal for the mouse acceleration calculations. However later I saw that the timestamp calculations use nanosecond timestamps from the kernel and those can be very large.

Using anything below the current WBITS value risks breaking the timestamp calculation, but it can look like it works, because it probably takes the lowest bits of the timestamp discarding the bits that don't fit. Idk what exactly happens when the WBITS is too low for the timestamps, but it seems like it would result in mouse acceleration being way above the set value.

I think it would be nice if we could just use 64 bits of precision but that only works when you link against libgcc and afaik you can't do that in a kernel module.

Using larger fixed-point numbers would be a really nice solution for this, if that worked in the kernel. The reason it doesn't work is, because some functions use int128 or uint128 types that don't work in the kernel. With the current fixedptc.h library it compiles with 64-bit fixed-point numbers, but moving the mouse causes a crash. Fixing this would "only" require adding custom 128-bit mul and div functions for use in the kernel, but idk how to do that. Other arithmetic functions don't use 128-bit types.

The solution I will probably try next for the timestamp problems is to use some arithmetic trickery to get accurate enough timestamps in ms. For example first getting the whole ms value by dividing the ns timestamp with 1000000 and then adding precision to it by using the modulo of timestamps in ns.

@N-R-K
Copy link

N-R-K commented Oct 11, 2024

However later I saw that the timestamp calculations use nanosecond timestamps from the kernel and those can be very large

The absolute nanoseconds timestamp might be large but that's not what we're interested in anyways. The delta (now - last) is the interesting part and is supposed to be small.

But if that's still not enough then you can just truncate the delta to micro-seconds and convert that to fixed-point milliseconds:

delta_us = (now - last) / 1000;
// optional, rounds the ns instead of flooring it
delta_us += ((now - last) % 1000) >= 500 ? 1 : 0;
ms = fixedpt_div(fixedpt_fromint(delta_us), fixedpt_fromint(1000));

EDIT: just realized that 16 bits won't be able to represent the divisor of 1000*1000, so truncating to micro-seconds before converting is necessary if 16.16 format is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants