rerun-io
draft: correct unwanted eye (camera) movement upon firing shortcuts
What
This PR addressed a UI glitch that eye could move slightly upon firing up a shortcut, e.g., ctrl + s. As shown in the video below, across all the platforms, due to the view still listening for any keyboard_navigation, there is a small backwards (due to "S" presence") move before (at 0:03 in the video below) and after (0:05) the save dialog.
https://github.com/user-attachments/assets/c4f4e95e-2430-41f9-8d4d-50ed8f0e2b2a
Besides, there is also a platform-dependent issue: when testing in Ubuntu 22.04.5 LTS, after closed the save dialog, the egui input context still registered the ctrl modifier and "S" keys_pressed, causing the eye moved backwards as long as cursor hovers the view (0:14 and 0:19 in the video below). I've tested with Windows 11 and web-version, it worked without such issue.
https://github.com/user-attachments/assets/0f6698aa-08ae-4584-8fc5-4f35358ac9eb
The fix
To address the issue, a shortcut key clearing from input has been implemented (https://github.com/rerun-io/rerun/pull/8975/files#diff-daa0aae659935f597d1eb9ce8af241e875d7e61e03cf725151ec26f0c75efd30R462-R463) to stop the shortcut to propagate. But maybe an event propagation control could be implemented on egui? @emilk love to hear your thoughts.
~~To address the issue no. 1 above, checked for modifiers_pressed is added so eye movement won't be activated if any of the modifier key pressed (https://github.com/rerun-io/rerun/pull/8975/files#diff-242e5fb1310bf1a865d6c25402303099d00d6a81e76137ccfaaf36db8987b24bR448-R453). I find this should be a clean approach, do let me know if there is any such case that the eye movement should be carried out with a modifier key @emilk.~~
~~To address the issue no. 2, I'm thinking to clear all modifier and key input from the egui context, before triggering the UICommands (https://github.com/rerun-io/rerun/pull/8975/files#diff-46de70906d858ca551e573c4bc0e2f1d5415e792d3b91d6e7653ec3d44b78527R705-R710). One part I'm not so sure about the current implementation is how the keyboard input is cleared. But I find it make sense to clear them before the UICommands as proposed in this fix. Let me know if there is more clever way for this issue, or any concerns about my fix.~~
@kailiuca are you planning to follow-up on this or do you want someone else to take over?
@kailiuca are you planning to follow-up on this or do you want someone else to take over?
Apologize @Wumpf , was super busy in the past few weeks. I've put up an alternative implementation.
Didn't notice this was ready for review again. input.keys_down.remove(&kb_shortcut.logical_key); seems to work fine. So let's just clean up the commented out code and land it 🤷