InfiniTime icon indicating copy to clipboard operation
InfiniTime copied to clipboard

Clear ongoing taps when going to sleep

Open mark9064 opened this issue 1 year ago • 10 comments

Currently if the touch panel is pressed when DisplayApp goes to sleep, the finger up event gets dropped. This causes LVGL to think the finger is never released from the touch panel, so it prevents the device from sleeping. This PR clears the tapped state when going to sleep so the internal LVGL state remains consistent with the real touch panel state.

Fixes part of #1790 #2012 (there may be other issues contributing to these, but this is probably part of it)

mark9064 avatar Aug 23 '24 13:08 mark9064

Build size and comparison to main:

Section Size Difference
text 374512B -48B
data 948B 0B
bss 63488B 0B

github-actions[bot] avatar Aug 23 '24 13:08 github-actions[bot]

Also fixes #1992

mark9064 avatar Aug 25 '24 21:08 mark9064

Actually the same problem exists with TouchHandler. Should we clear the tap state there too? Or do we define calling its methods as UB when the touch panel is sleeping (as when it's sleeping, it's not possible to know whether the screen is touched or not)

TouchHandler doesn't cause any bugs, but it could certainly in the future. If we choose to define it as UB then we should guard

// DisplayApp.cpp
  if (touchHandler.IsTouching()) {
    currentScreen->OnTouchEvent(touchHandler.GetX(), touchHandler.GetY());
  }

with a state == Running check

mark9064 avatar Aug 28 '24 23:08 mark9064

I would avoid any UB.

minacode avatar Aug 29 '24 09:08 minacode

I mean defining calling the methods as UB by documentation i.e the documentation for the function IsTouching() saying "if the device is sleeping, the value returned is undefined". There is no UB C++ spec wise

mark9064 avatar Aug 29 '24 11:08 mark9064

I think the perfect type of that function would not be bool but std::optional<bool>. The value std::nullopt would represent the case where no measurement can be made (screen asleep) and the bool would be the measurement, if possible (screen awake).

Otherwise return always false if the screen is asleep. Because no screen, no touch.

minacode avatar Aug 29 '24 18:08 minacode

Excellent idea, optional makes perfect sense here semantically. I will try it out

mark9064 avatar Aug 29 '24 22:08 mark9064

Actually the same problem exists with TouchHandler. Should we clear the tap state there too? Or do we define calling its methods as UB when the touch panel is sleeping (as when it's sleeping, it's not possible to know whether the screen is touched or not)

I think we could just clear the tap in TouchHandler as soon as the touch panel is sleeping as well, since I don't think we want to handle touch while the device is sleeping.

JF002 avatar Sep 14 '24 09:09 JF002

So this has turned out to be a bit trickier than I thought. TLDR the touch panel still delivers events when sleeping, so trying to pretend it doesn't with TouchController having a null state is incompatible with double tap wake / tap wake. So instead I've opted to unify the touch processing into something a little more sensible, so the TouchController always has up to date touch state even when the device is sleeping.

So the original problem is addressed with DisplayApp not receiving or processing any touch events while sleeping, and clearing touch state on sleep entry

mark9064 avatar Sep 27 '24 23:09 mark9064

Cleaned up the OnTouchEvent method as it was just an alias for PushMessage after Unify touch handling, no other changes

mark9064 avatar Oct 17 '24 23:10 mark9064