InfiniTime icon indicating copy to clipboard operation
InfiniTime copied to clipboard

touchhandler: custom gesture detection

Open KaffeinatedKat opened this issue 1 year ago • 10 comments

This PR implements InfiniTime's own code for detecting gestures within the TouchHandler, instead of using the gesture detection built-in to the Cst816S. The only gesture that hasn't been re-implemented is the DoubleTap because the built in DoubleTap detection works just fine as is.

This PR allows us to easily re-configure the gestures. Swiping works much more consistently now due to a lower distance required to be detected. The LongTap gesture is also greatly improved here, with a much lower hold duration, making it activate much more consistently.

I also plan to implement smooth scrolling in places like the app drawer and the settings screen. Re-implementing the gesture system is required for that, and it will depend on this PR

KaffeinatedKat avatar Feb 04 '24 22:02 KaffeinatedKat

Build size and comparison to main:

Section Size Difference
text 373224B 116B
data 940B 0B
bss 63532B 16B

github-actions[bot] avatar Feb 04 '24 22:02 github-actions[bot]

Seems to work nicely. Short and fast flicks don't get registered as swipes easily though. Maybe velocity could be taken into account? Example: Try scrolling the settings menus with fast flicks. Most of the time it registers it as a tap and you end up inside a settings page rather than still being on the list

mark9064 avatar Feb 09 '24 01:02 mark9064

not a bad idea to take velocity into account. I'll continue to play with this to make it easier to use. Going to make this PR a draft until I can implement everything better than what we have right now. Going to also re-implement the DoubleTap gesture and then we can just completely remove the Cst816S gestures from the driver code.

Another benefit of having the TouchHandler do all of the gestures is that it makes it much easier to add more watch support. We won't have worry about the gesture system of the touch controller, and converting those into the TouchHandler events, because the TouchHandler is what is handling the gestures regardless of what the hardware provides.

A touch controller will simply need to provide x and y coordinates of a touch location, and a "isTouching" state (basic touch controller functionality). This will greatly improve the maintainability when InfiniTime eventually supports a wider range of watch models.

KaffeinatedKat avatar Feb 09 '24 21:02 KaffeinatedKat

Oh also another thought. This also breaks tap to wake right? As the gesture events are generated even when the touch panel is in sleep mode, but if we do gestures in software we lose that (and would have to keep the touch panel powered 24/7 like we do right now for double tap to wake)

mark9064 avatar Feb 10 '24 12:02 mark9064

this is true, I didn't think about that. A simple fix is to keep the Cst816S Tap gesture in the TouchHandler to keep the battery life benefits of the display being asleep

KaffeinatedKat avatar Feb 10 '24 18:02 KaffeinatedKat

What would be the added value of re-implementing in software those gestures when the hardware does it automatically for us, in a probably more energy efficient way?

JF002 avatar Feb 11 '24 13:02 JF002

the main reason is that the hardware LongTap is just annoyingly inconsistent, I hate using it. It takes too long to activate and sometimes it just doesn't activate multiple times in a row. I went ahead and re-implemented everything else while I was at it, but other than the LongTap the hardware gestures work fine.

I could always change this PR to just re-implement the long tap, and leave the rest of the gestures as is if that's better

KaffeinatedKat avatar Feb 11 '24 17:02 KaffeinatedKat

I wasn't aware we had issues with long taps. Could you give more detail about those inconsistencies?

JF002 avatar Feb 11 '24 18:02 JF002

the LongTap action takes quite a while to activate, and I feel this hold duration is too long. Sometimes the LongTap action just never triggers, despite holding for a long time, and because of how long it takes it takes me a second to realize that it's not going to trigger.

The long duration also makes it not really useful outside of pulling up watchface settings. With a lower duration it could easily be used as an option to clear all the notifications in the notification screen (https://github.com/InfiniTimeOrg/InfiniTime/pull/2000#issuecomment-1925449522). The long duration makes it impractical for this, as it's not much faster than just manually dismissing them all

KaffeinatedKat avatar Feb 13 '24 07:02 KaffeinatedKat

Swiping is quite inconsistent for me right now, so being able to customize it a bit would be great.

pipe01 avatar Apr 23 '24 22:04 pipe01