fix(input): resolve alt-tab and shift-tab bugs#1196
Conversation
WalkthroughThe changes refactor focus handling in the menu system. Focus events in the window procedure now set a flag rather than directly clearing input. The menu class replaces its focus loss handler with a more general focus change handler that resets keyboard input and ImGui keys. Focus changes are processed during the settings drawing phase. Changes
Sequence Diagram(s)sequenceDiagram
participant OS as Operating System
participant WndProc as Window Procedure Hook
participant Menu as Menu
participant Input as Input Device Manager
OS->>WndProc: WM_KILLFOCUS or WM_SETFOCUS
WndProc->>Menu: Set focusChanged = true
loop On DrawSettings()
Menu->>Menu: Check focusChanged
alt focusChanged == true
Menu->>Menu: OnFocusChanged()
Menu->>Input: Reset keyboard device
Menu->>Menu: Clear ImGui input keys
Menu->>Menu: Set focusChanged = false
end
end
Possibly related issues
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Hooks.cpp(1 hunks)src/Menu.cpp(2 hunks)src/Menu.h(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (3)
src/Menu.h (1)
50-53: LGTM! Clean implementation of deferred focus handling.The flag-based approach properly decouples window message handling from the actual input reset logic. The comment clearly explains the purpose, and the naming is descriptive.
src/Hooks.cpp (1)
617-619: Excellent refactor to lightweight window procedure handling.Moving from immediate processing to flag-based deferred handling is the right approach. Capturing both
WM_KILLFOCUSandWM_SETFOCUSproperly addresses the alt-tab scenario where focus is lost and regained.src/Menu.cpp (1)
308-311: LGTM! Good approach for thread-safe focus handling.The deferred focus change processing is a good design choice. By checking the flag during the rendering phase rather than handling focus changes immediately in the window message handler, this avoids potential thread safety issues with ImGui and input device operations.
|
✅ A pre-release build is available for this PR: |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
To merge on CI confirmation. |
closes 902
closes 862
Addresses an issue where input keys, particularly the tab key, would become
stuck or unresponsive after alt-tabbing out of and back into the application.
This is resolved by:
focusChangedflag and OnFocusChanged method.focusChangedonWM_KILLFOCUSandWM_SETFOCUSwindow messages via aWndProcHandler_Hook.RE::BSInputDeviceManager::GetSingleton()->GetKeyboard()->Reset().ImGui::GetIO().ClearInputKeys()to ensure proper tab key functionality within the UI.Additionally:
GetAsyncKeyStateto force-release stuck Shift and Tab keys_keyEventQueue.clear()to prevent duplicate Tab events on single tapsSummary by CodeRabbit
Summary by CodeRabbit