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

Prevent false positive in secondary axis for gestures #343

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

chomosuke
Copy link

@chomosuke chomosuke commented Nov 24, 2022

The problem:

  • I like to map up and down gesture to volume and left and right gesture to zoom. However, sometimes when I'm adjusting volume, zooming would be triggered. This does not happen on LogiOptions.
  • Sometimes I overshoot the volume control and would like to move the other way to bring my volume to the desired value. On LogiOptions I can simply move the other way, but with logiops I have to move all the way back to origin or release and press the gesture button again to adjust volume the other way.

This PR fixes the first problem by passing both x and y movement to Gesture classes as primary and secondary axis. Then each Gesture classes can add their own logic to determine whether itself is the indented gesture or not.

New behavior of each type of gestures:

  • ReleaseGesture and ThresholdGesture will only be triggered if the total distance moved towards the primary direction is larger than the sum of all absolute value of secondary axis movement.
  • IntervalGesture will only be triggered if the sum of all absolute value of primary axis movement is larger than the sum of all absolute value of secondary axis movement.

This PR fixes the second problem trivially by only increasing _axis when primary axis movement is positive.

I've also made noneAction trigger even if some threshold are met as long as no actions have been performed by any of the gesture. This let me set the threshold to 1 for my interval gestures. I also think this makes more sense.

The behavior for scroll wheel should be unmodified though I have not tested this.
Edit: The behavior for scroll wheel is unchanged. The thumb wheel was broken for interval > 8 because press() was called immediately before move(). I removed that line to fix it.

I marked this PR previously as Help Wanted because I am not sure about the desirable behavior for AxisGesture. I'm concerned about the rare (possibly non existent) use case of mapping Left to a AxisGesture and Right to something else such as a ReleaseGesture or even an IntervalGesture. Should I make it similar to IntervalGesture where only positive movement will be recorded? Should I add false positive prevention like I did in IntervalGesture? What is the indented use case for this gesture? What's @PixlOne and everyone else's opinion on this?

Edit: For now I'll be leaving AxisGesture's behavior unchanged as I don't know its common use case and don't want to break someone else's workflow. Personally I think we should make it similar to IntervalGesture (i.e. only record positive movement and have false positive prevention) for consistency sake. However, I'm open to suggestions.

Thank you

Changed Gesture.h to allow fixture of other secondary axis false positive prevention.
Other Gesture broken
For secondary-axis-false-positive. Tested NullGesture
@chomosuke chomosuke marked this pull request as draft November 24, 2022 15:45
@chomosuke chomosuke changed the title [WIP, Help Wanted] Prevent false positive in secondary axis for gestures [Help Wanted] Prevent false positive in secondary axis for gestures Nov 24, 2022
@chomosuke chomosuke changed the title [Help Wanted] Prevent false positive in secondary axis for gestures Prevent false positive in secondary axis for gestures Nov 25, 2022
Fixed AxisGesture still triggering noneAction after reaching and then going below threshold again
@chomosuke chomosuke marked this pull request as ready for review November 25, 2022 07:16
@chomosuke chomosuke mentioned this pull request Dec 28, 2022
@PixlOne PixlOne force-pushed the main branch 14 times, most recently from a062ed0 to a77b328 Compare May 4, 2023 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant