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

Implement kinetic mouse movement algorithm #6739

Merged
merged 9 commits into from
Dec 24, 2020

Conversation

jceb
Copy link
Contributor

@jceb jceb commented Sep 15, 2019

Description

I implemented the kinetic/quadratic mouse acceleration algorithm. The algorithm is not the one from the UHK but a newly created one that's able to compute the current speed at any given time instead of relying on a compounding mechanism that just computes the increment.
Overall, the cursor acceleration feels very similar to the UHK. However, feedback and improvements are welcome in order to make it as usable as possible.

This algorithm requires a very low MOUSEKEY_INTERVAL in order to produce smooth movements. What's possible highly depends on the micro processor. I did my tests on the elite-c which is pro micro compatible. It was able to deliver 125 events per second which I set as the default value if MK_KINETIC_SPEED is defined.

I wrote a small utility https://github.com/jceb/bin/blob/master/mouseevents that helps to measure the maximum number of mouse events that can be sent by the micro processor. If you try out this algorithm it should help find the right setting for MOUSEKEY_INTERVAL.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

tmk_core/common/mousekey.c Show resolved Hide resolved
@jceb
Copy link
Contributor Author

jceb commented Sep 29, 2019

Any updates on this pull request?

@drashna drashna requested a review from a team September 30, 2019 19:26
@jackhumbert jackhumbert added the hacktoberfest-accepted All the good issues to tackle during Hacktoberfest! label Sep 30, 2019
Copy link

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the docs and comments, all look good, just a teeny-tiny suggestion

tmk_core/common/mousekey.c Show resolved Hide resolved
Copy link

@thiagoyeds thiagoyeds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jceb, I reviewed your code, very well documented and implemented.

Copy link

@thiagoyeds thiagoyeds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a suggestion, please review it.

tmk_core/common/mousekey.c Show resolved Hide resolved
Copy link
Contributor

@lf- lf- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, documentation confused me before I read the code, which I have suggested changes for improving to make it clear how kinetic mode is different from acceleration mode.

docs/feature_mouse_keys.md Outdated Show resolved Hide resolved
docs/feature_mouse_keys.md Outdated Show resolved Hide resolved
@qmk qmk deleted a comment from rishav142k Oct 8, 2019
@qmk qmk deleted a comment from SyntaxError-exe Oct 8, 2019
@qmk qmk deleted a comment from D3v3sh5ingh Oct 9, 2019
@qmk qmk deleted a comment from chamodshehanka Oct 11, 2019
@qmk qmk deleted a comment from sumitkharche Oct 13, 2019
Copy link

@samagragupta samagragupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qmk qmk deleted a comment from PrathamGoyal Oct 22, 2019
@qmk qmk deleted a comment from raghavdhingra Oct 23, 2019
@qmk qmk deleted a comment from raghavdhingra Oct 23, 2019
@qmk qmk deleted a comment from raghavdhingra Oct 23, 2019
@jceb
Copy link
Contributor Author

jceb commented Oct 27, 2019

Docs are in place, no requests have been raised. I think it just needs a little more time. @drashna what do you think about merging it?

@drashna
Copy link
Member

drashna commented Dec 17, 2020

This PR needs to be rebased.

@jceb jceb force-pushed the feature_kinetic_mouse_movement branch from 229e7f7 to 1e40fb5 Compare December 21, 2020 15:57
@jceb
Copy link
Contributor Author

jceb commented Dec 21, 2020

Rebase done.
@sahalsaad somehow github says that there are still open comments from you but I can't find them. Could you point me to the open items or close them?
@tzarc @zvecr do you have any questions regarding this PR that are standing in the way of a sign-off?

@Erovia
Copy link
Member

Erovia commented Dec 21, 2020

Not sure if the github ui is slow a bit today or something, but it still shows one conflict (tmk_core/common/mousekey.c) for me.

@jceb jceb force-pushed the feature_kinetic_mouse_movement branch from 1e40fb5 to 56e6906 Compare December 22, 2020 09:31
@jceb
Copy link
Contributor Author

jceb commented Dec 22, 2020

Not sure if the github ui is slow a bit today or something, but it still shows one conflict (tmk_core/common/mousekey.c) for me.

Thank you for the hint. Somehow I couldn't get my git to force-push the changes. Very strange but now it worked :-D

@drashna drashna merged commit 010271d into qmk:develop Dec 24, 2020
@kareltucek
Copy link

kareltucek commented Mar 28, 2021

Overall, the cursor acceleration feels very similar to the UHK. However, feedback and improvements are welcome in order to make it as usable as possible.

If I may:

  • I see you are not handling your float remainders, which means that achievability of low speed and high smoothness is governed by the Heisenberg's principle (literally). While I agree that the sweet spot (where both the properties are acceptable for regular use) can be achieved, finding feasible combination of parameters would be much easier if you simply stashed the remainders between the calls.

  • For some reason, changing interval changes acceleration. I may have some local problems in the code due to which this observation might be incorrect, but if the observation is correct, it would be better if acceleration did not depend on the mousekey interval.

  • Documentation (https://beta.docs.qmk.fm/using-qmk/advanced-keycodes/feature_mouse_keys) is missing interpretation of the mouse delta, so figuring out that it is actually the linear step per 50 ms is up to the reader. Two ways out:

    • (easier) to fix the docs
    • (conceptually correct) to expose the acceleration in some human interprettable units whose interpretation won't change depending on internal implementation details (and fix the docs)

(I am looking into the older qmk/master code, fixed by hand in order to expose your algorithm... thence the possibility of errors.)

@jceb
Copy link
Contributor Author

jceb commented Mar 30, 2021

Thank you for your feedback. There's another kinetic implementation at https://github.com/liyang/qmk_firmware that might be of interest to you.

@kareltucek
Copy link

kareltucek commented Mar 30, 2021

Honestly? I've got your implementation running and am happy with it, and don't want to waste any more time on another O:-).

But thanks nevertheless!

@Lilalas
Copy link

Lilalas commented Aug 18, 2021

How could the double space in line 114 slip through? It was one of the very first suggestions to remove it.

@fauxpark
Copy link
Member

Unfortunately GitHub doesn't tell you when a suggestion is resolved manually or by it actually being applied.

@Lilalas
Copy link

Lilalas commented Aug 19, 2021

@fauxpark Am I looking in the wrong branch? What I meant to say is it’s not actually applied. Or rather it had been applied and somebody must have undone it.

@fauxpark
Copy link
Member

fauxpark commented Aug 19, 2021

That's right, it doesn't seem to have been applied, but the suggestion is marked as resolved. Maybe it was applied but got lost during one of the rebases that appear to have happened here. Or maybe the PR author marked it resolved and forgot to do it themselves. It's impossible to tell at this point because either way GitHub just shows this:
image

BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Implement kinetic mouse movement algorithm

* Adjust mouse wheel speed

* Remove unused math.h include

* Wrap mouse_timer definition in ifdef

* Replace double space by single space

* Clarify documentation of kinetic mouse speed

Co-Authored-By: lf <software@lfcode.ca>

* Clarify documentation of kinetic mouse speed

Co-Authored-By: lf <software@lfcode.ca>

* Remove superfluous definition of speed

* fix(variable): remove unused variable

Co-authored-by: lf <software@lfcode.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.