fix: 修复未处理 WM_SYSKEYDOWN/WM_SYSKEYUP 导致的异常 - #374
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough扩展系统键盘消息的解析、窗口分发及按键状态更新，使 Changes系统按键处理
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #373 where key state could get “stuck” (e.g., Ctrl) when Alt is involved, by ensuring system key messages (WM_SYSKEYDOWN/WM_SYSKEYUP) are handled consistently alongside normal key messages in the Windows message pipeline.
Changes:
- Treat
WM_SYSKEYDOWN/WM_SYSKEYUPas key down/up events when consuming keyboard messages (src/keyboard.cpp). - Extend UI/control key dispatch and internal key state tracking to include system key down/up messages (
src/graphics.cpp). - Ensure the window procedure routes
WM_SYSKEYDOWN/WM_SYSKEYUPinto the existing key handling path (src/graphics.cpp).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/keyboard.cpp | Adds WM_SYSKEYDOWN/WM_SYSKEYUP support when reading key messages from msgkey_queue. |
| src/graphics.cpp | Routes system key messages through on_key() and UI control key dispatch to keep keystatemap accurate. |
| while (pg->msgkey_queue->pop(msg)) { | ||
| if (msg.message == WM_CHAR || msg.message == WM_KEYDOWN) { | ||
| if (msg.message == WM_KEYDOWN) { | ||
| if (msg.message == WM_CHAR || msg.message == WM_KEYDOWN || msg.message == WM_SYSKEYDOWN) { | ||
| if (msg.message == WM_KEYDOWN || msg.message == WM_SYSKEYDOWN) { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/graphics.cpp`:
- Around line 640-642: Update the WM_SYSKEYDOWN and WM_SYSKEYUP handling in the
window procedure so these messages are forwarded to DefWindowProcW after the
on_key callback, rather than exiting through return 0; preserve existing
handling for ordinary key messages and any intentional WM_SYSCOMMAND
interception.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 75a6fb38-844a-44c5-8bca-ee690149700d
📒 Files selected for processing (2)
src/graphics.cppsrc/keyboard.cpp
| case WM_SYSKEYDOWN: | ||
| case WM_KEYUP: | ||
| case WM_SYSKEYUP: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)src/graphics\.cpp$|src/graphics\.(h|hpp)$' || true
echo
echo "Outline:"
ast-grep outline src/graphics.cpp --view compact 2>/dev/null | sed -n '1,220p' || true
echo
echo "Relevant message handling region:"
nl -ba src/graphics.cpp | sed -n '600,730p'
echo
echo "on_key usages/definition:"
rg -n "on_key|WM_SYSKEYDOWN|WM_SYSKEYUP|WM_KEYUP|DefWindowProcW|WM_SYSCOMMAND|WM_MENU|WM_ACTIVATE" src/graphics.cppRepository: x-ege/xege
Length of output: 258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Relevant message handling region:"
awk '{printf "%6d\t%s\n", NR, $0}' src/graphics.cpp | sed -n '600,730p'
echo
echo "on_key usages/def and relevant Windows messages:"
grep -n "on_key\|WM_SYSKEYDOWN\|WM_SYSKEYUP\|WM_KEYUP\|DefWindowProcW\|WM_SYSCOMMAND\|WM_MENU\|WM_ACTIVATE" src/graphics.cpp || true
echo
echo "Search on_key declaration/implementation:"
grep -Rn "on_key" src | sed -n '1,120p'Repository: x-ege/xege
Length of output: 6841
🌐 Web query:
Microsoft Windows WM_SYSKEYDOWN message DefWindowProc Alt F10 system menu documentation
💡 Result:
The WM_SYSKEYDOWN message is sent to a window when a system key is pressed—specifically when the user presses the F10 key (which activates the menu bar) or holds down the ALT key while pressing another key [1][2]. Regarding the interaction with DefWindowProc and the activation of the system menu: 1. Handling of F10 and ALT: When a window procedure receives the WM_SYSKEYDOWN message, it is critical to pass the message to the DefWindowProc function if the application does not intend to handle the keystroke itself [2]. The system uses the WM_SYSKEYDOWN message to track the state of these keys for system-level commands [3]. 2. Mechanism of Menu Activation: DefWindowProc does not typically trigger the menu activation immediately upon receiving the WM_SYSKEYDOWN message for F10. Instead, it tracks the state of the F10 key [3]. When the subsequent WM_KEYUP message for F10 is received, DefWindowProc checks that internal state and, if F10 was pressed, sends a WM_SYSCOMMAND message with the SC_KEYMENU parameter to the window [3]. This WM_SYSCOMMAND message is what actually activates the menu bar [4][3]. 3. Recommendations: If you need to intercept or prevent the default menu activation triggered by ALT or F10, you should handle the WM_SYSCOMMAND message in your window procedure [5][4]. By catching WM_SYSCOMMAND where wParam is equal to SC_KEYMENU, you can prevent the default behavior (by returning 0 without calling DefWindowProc) or implement custom logic to display your own menu or handle the event differently [5][4]. If you simply "block" the WM_SYSKEYDOWN message without letting it reach DefWindowProc, you may interfere with the operating system's ability to process standard system commands [2]. Therefore, the recommended practice is to allow the system to process the keys via DefWindowProc and instead override the resulting action by intercepting the WM_SYSCOMMAND message [5][4][3].
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/inputdev/wm-syskeydown
- 2: https://learn.microsoft.com/en-us/windows/win32/learnwin32/keyboard-input
- 3: https://ofekshilon.com/2011/12/03/three-f-keys-gotchas/
- 4: https://stackoverflow.com/questions/66364979/how-to-capture-the-activation-of-the-menubar
- 5: https://stackoverflow.com/questions/66437715/how-to-handle-vk-menu-alt-keypresses-properly-using-winapi
不要吞掉 WM_SYSKEYDOWN/WM_SYSKEYUP 的默认窗口处理。
这条分支调用完 on_key 后直接 break，最终从 return 0 退出；WM_SYSKEYDOWN/SM_SYSKEYUP 需要传给 DefWindowProcW 才能让系统继续处理 F10/Alt 等系统键盘状态，否则对应 WM_SYSCOMMAND 行为（如菜单激活）可能失效。转发消息前如需拦截，可先处理业务回调并额外转发/拦截 WM_SYSCOMMAND。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graphics.cpp` around lines 640 - 642, Update the WM_SYSKEYDOWN and
WM_SYSKEYUP handling in the window procedure so these messages are forwarded to
DefWindowProcW after the on_key callback, rather than exiting through return 0;
preserve existing handling for ordinary key messages and any intentional
WM_SYSCOMMAND interception.
#373
Summary by CodeRabbit