Switch to using touch state instead of most recent event #97
Conversation
Previously, the wacom code would emit WacomEvent::Hover iff the last event seen was ToolPen. This has the wrong behaviour when the pen is leaving the screen, since a Touch event was emitted as the pen lifts but another ToolPen won't be emitted until the pen is no longer near. Instead, we use the current touch state to decide if we're hovering or not. So why wasn't this always broken? In the ABS_DIST code farther down in the file, it would update last_tool as part of some pressure / distance corrections I don't fully understand. When we hit this codepath, the resulting last-tool seems to be correct. That code seems to reliably fire on most tablets, but it seems that it's not guaranteed to happen. Anyways this is all a bit sketchy, and we should test on a couple devices before calling it good.
There was a problem hiding this comment.
Nice finding. Did a quick check with the input example on both models.
Seems to behave as expected. One thing of note is that the rM 1 doesn't emit any distance events anymore. I'm pretty sure it used to. And does so on the rM 2 and isn't related to these changes.
|
Oh, that's interesting. Just to clarify... the RM1 doesn't get hover events either before or after this patch? (Wonder if it has something to do with the semi-recent evdev bump...) |
|
It sounds like this was good on your end, so I'm merging it in... but I'm happy to back it out if I got that wrong! Additional fun fact: I was still a bit puzzled why the standard libremarkable demo wasn't drawing after lifting the pen, and it seems to be the case that - it actually was still "drawing," but because line thickness was based on the pressure and the pressure was zero, the line it was drawing had zero width. |
This should fix a weird edge-case bug in the event handling code. (Thanks to
okehon the discord for finding this issue and helping me debug.)I've included the commit-message description below, but let me know if anything's not clear. (Or if you know what the "interesting behaviour" in the
ecodes::ABS_DISTANCEhandler is!)Previously, the wacom code would emit WacomEvent::Hover iff the last event seen was ToolPen. This has the wrong behaviour when the pen is leaving the screen, since a Touch event gets emitted as the pen lifts but another ToolPen won't be emitted until the pen is no longer near the screen at all. Instead, we use the current touch state to decide if we're hovering or
not.
So why wasn't this always broken? In the ABS_DIST code farther down in the file, it would update last_tool as part of some pressure / distance corrections I don't fully understand. When we hit this codepath, the resulting last_tool value seems to be correct. That code seems to reliably fire pretty often on my tablet, masking the bug, but it seems that it's not guaranteed to happen.
Anyways this is all a bit sketchy, and we should probably test on a couple devices before calling it good.