fix: 修复 WithWindowPos 模式鼠标干扰并移除阻塞限制#1147
Conversation
解决在 `WithWindowPos` 模式执行任务时，玩家移动物理鼠标可能导致点击位置偏移、单击变成滑动等干扰问题。 修复方案与核心重构： - 引入 WH_MOUSE_LL 底层钩子：拦截真实的硬件鼠标移动并转化为内部位移累加，防止与程序的模拟点击产生位置冲突。 - 采用独立追踪线程（60fps 节流）：平滑同步光标和窗口位置，彻底消除异步调用拉扯造成的累积误差。 - 引入目标进程挂起机制（NtSuspendProcess）：在重新对齐窗口和光标的极短瞬间双向挂起目标进程，避免游戏捕获到重置过程中的失效帧。 - 完善锚点算法与多显示器适配：基于系统视区边缘限制坐标，提升了多屏或屏幕边缘拖拽时的精确度和鲁棒性。
由于底层已经实现了零延迟的窗口和光标同步追踪，彻底解决了鼠标移动带来的干扰和点击错位问题。 因此不再需要通过 `block_input = true` 来强制阻塞玩家的物理鼠标输入，恢复完整的后台无感体验。
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 新增的 WH_MOUSE_LL 钩子以及静态全局变量
s_active_instance_/hook_block_mouse_假设同一时间只有一个激活的MessageInput；如果将来同时创建多个带有with_window_pos的实例，行为可能会变得不可预测——建议将钩子管理显式设计为单例，或增加对多实例使用场景的防护。 tracking_thread_func中对目标进程的挂起/恢复逻辑不是基于 RAII 的，如果之后引入了提前返回或类似异常的失败路径，有可能导致进程被一直挂起；可以考虑用一个小的守卫对象封装挂起/恢复，或者使用类似ScopeGuard的辅助工具来让这部分逻辑更健壮。- 跟踪线程当前在安装 WH_MOUSE_LL 钩子时使用
GetModuleHandleW(NULL)；为了在代码以后迁移到 DLL 时依然保持清晰和正确，建议显式传入包含MouseHookProc的模块句柄，而不是默认假设为 EXE 模块。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- The new WH_MOUSE_LL hook and static `s_active_instance_`/`hook_block_mouse_` globals assume only a single active `MessageInput` at a time; if multiple instances with `with_window_pos` are ever created concurrently this will behave unpredictably—consider making the hook management explicitly singleton or guarding against multi-instance use.
- The suspend/resume logic for the target process in `tracking_thread_func` is non-RAII and can leave the process suspended if an early return or exception-like failure path is introduced later; wrapping suspend/resume in a small guard object or using `ScopeGuard`-style helpers would make this more robust.
- The tracking thread currently uses `GetModuleHandleW(NULL)` when installing the WH_MOUSE_LL hook; for clarity and correctness if this code ever moves to a DLL, consider explicitly passing the module handle that contains `MouseHookProc` rather than assuming the EXE module.
## Individual Comments
### Comment 1
<location path="source/MaaWin32ControlUnit/Input/MessageInput.cpp" line_range="311-319" />
<code_context>
+ // 预先打开目标进程句柄（用于挂起/恢复）
+ open_target_process();
+
+ HHOOK hHook = SetWindowsHookExW(WH_MOUSE_LL, MouseHookProc, GetModuleHandleW(NULL), 0);
+
+ // 60fps 节流：每帧间隔 ~16.67ms
+ using clock = std::chrono::steady_clock;
+ auto last_frame = clock::now();
+ const auto frame_interval = std::chrono::microseconds(16667);
+
+ MSG msg;
+ while (!tracking_exit_) {
+ // 消息泵：WH_MOUSE_LL 钩子回调在此线程上被系统调度，必须保持消息循环活跃
+ DWORD res = MsgWaitForMultipleObjects(0, NULL, FALSE, 1, QS_ALLINPUT);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Handle SetWindowsHookExW failure explicitly to avoid a busy tracking loop that never receives mouse events.
If `SetWindowsHookExW` fails and `hHook` is null, this loop will still run at high priority with the 60fps timing but never receive mouse events, wasting resources and appearing to do nothing. Please check `hHook` after the call and either log and exit the thread early or disable tracking when the hook cannot be installed.
Suggested implementation:
```cpp
// 预先打开目标进程句柄（用于挂起/恢复）
open_target_process();
HHOOK hHook = SetWindowsHookExW(WH_MOUSE_LL, MouseHookProc, GetModuleHandleW(NULL), 0);
if (!hHook) {
// 钩子安装失败时避免进入忙循环：记录失败并退出跟踪线程
// TODO: 根据项目日志系统替换为合适的日志输出
// e.g. LOG_ERROR("SetWindowsHookExW(WH_MOUSE_LL) failed: {}", GetLastError());
tracking_exit_ = true;
timeEndPeriod(1);
return;
}
// 60fps 节流：每帧间隔 ~16.67ms
```
1. Replace the TODO comment with the project's actual logging / error-reporting macro or mechanism (for example, `LOG_ERROR`, `spdlog::error`, or your existing trace facility).
2. If there is a centralized cleanup path for this thread (e.g., unhooking and restoring timer precision), you may prefer to factor the `timeEndPeriod(1); return;` into that path instead of returning early here, while still ensuring the thread exits when the hook cannot be installed.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The new WH_MOUSE_LL hook and static
s_active_instance_/hook_block_mouse_globals assume only a single activeMessageInputat a time; if multiple instances withwith_window_posare ever created concurrently this will behave unpredictably—consider making the hook management explicitly singleton or guarding against multi-instance use. - The suspend/resume logic for the target process in
tracking_thread_funcis non-RAII and can leave the process suspended if an early return or exception-like failure path is introduced later; wrapping suspend/resume in a small guard object or usingScopeGuard-style helpers would make this more robust. - The tracking thread currently uses
GetModuleHandleW(NULL)when installing the WH_MOUSE_LL hook; for clarity and correctness if this code ever moves to a DLL, consider explicitly passing the module handle that containsMouseHookProcrather than assuming the EXE module.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new WH_MOUSE_LL hook and static `s_active_instance_`/`hook_block_mouse_` globals assume only a single active `MessageInput` at a time; if multiple instances with `with_window_pos` are ever created concurrently this will behave unpredictably—consider making the hook management explicitly singleton or guarding against multi-instance use.
- The suspend/resume logic for the target process in `tracking_thread_func` is non-RAII and can leave the process suspended if an early return or exception-like failure path is introduced later; wrapping suspend/resume in a small guard object or using `ScopeGuard`-style helpers would make this more robust.
- The tracking thread currently uses `GetModuleHandleW(NULL)` when installing the WH_MOUSE_LL hook; for clarity and correctness if this code ever moves to a DLL, consider explicitly passing the module handle that contains `MouseHookProc` rather than assuming the EXE module.
## Individual Comments
### Comment 1
<location path="source/MaaWin32ControlUnit/Input/MessageInput.cpp" line_range="311-319" />
<code_context>
+ // 预先打开目标进程句柄（用于挂起/恢复）
+ open_target_process();
+
+ HHOOK hHook = SetWindowsHookExW(WH_MOUSE_LL, MouseHookProc, GetModuleHandleW(NULL), 0);
+
+ // 60fps 节流：每帧间隔 ~16.67ms
+ using clock = std::chrono::steady_clock;
+ auto last_frame = clock::now();
+ const auto frame_interval = std::chrono::microseconds(16667);
+
+ MSG msg;
+ while (!tracking_exit_) {
+ // 消息泵：WH_MOUSE_LL 钩子回调在此线程上被系统调度，必须保持消息循环活跃
+ DWORD res = MsgWaitForMultipleObjects(0, NULL, FALSE, 1, QS_ALLINPUT);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Handle SetWindowsHookExW failure explicitly to avoid a busy tracking loop that never receives mouse events.
If `SetWindowsHookExW` fails and `hHook` is null, this loop will still run at high priority with the 60fps timing but never receive mouse events, wasting resources and appearing to do nothing. Please check `hHook` after the call and either log and exit the thread early or disable tracking when the hook cannot be installed.
Suggested implementation:
```cpp
// 预先打开目标进程句柄（用于挂起/恢复）
open_target_process();
HHOOK hHook = SetWindowsHookExW(WH_MOUSE_LL, MouseHookProc, GetModuleHandleW(NULL), 0);
if (!hHook) {
// 钩子安装失败时避免进入忙循环：记录失败并退出跟踪线程
// TODO: 根据项目日志系统替换为合适的日志输出
// e.g. LOG_ERROR("SetWindowsHookExW(WH_MOUSE_LL) failed: {}", GetLastError());
tracking_exit_ = true;
timeEndPeriod(1);
return;
}
// 60fps 节流：每帧间隔 ~16.67ms
```
1. Replace the TODO comment with the project's actual logging / error-reporting macro or mechanism (for example, `LOG_ERROR`, `spdlog::error`, or your existing trace facility).
2. If there is a centralized cleanup path for this thread (e.g., unhooking and restoring timer precision), you may prefer to factor the `timeEndPeriod(1); return;` into that path instead of returning early here, while still ensuring the thread exits when the hook cannot be installed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR fundamentally redesigns the WithWindowPos input mechanism for Win32 control to eliminate mouse interference. The old approach temporarily blocked all user input during operations; the new implementation uses a sophisticated background tracking system that continuously monitors and compensates for physical mouse movement, allowing users to move their mouse freely while automation runs.
Changes:
- Replaced input blocking with a continuous tracking thread that uses a low-level mouse hook to intercept and accumulate real mouse movements
- Implemented process suspend/resume mechanism to prevent visual glitches during window repositioning
- Changed default
block_inputparameter fromtruetofalsefor all WithWindowPos and WithCursorPos input methods - Added multi-monitor awareness with virtual screen boundary clamping
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
| Win32ControlUnitMgr.cpp | Changed default block_input from true to false for WithCursorPos and WithWindowPos input methods |
| MessageInput.h | Added tracking thread infrastructure, hook management, process suspension members, and atomic synchronization primitives |
| MessageInput.cpp | Implemented background tracking thread with WH_MOUSE_LL hook, 60fps mouse delta processing, process suspend/resume, enhanced error handling, and redesigned window alignment algorithm |
| HHOOK hHook = SetWindowsHookExW(WH_MOUSE_LL, MouseHookProc, GetModuleHandleW(NULL), 0); | ||
| if (!hHook) { | ||
| LogError << "SetWindowsHookExW failed, tracking disabled" << VAR(GetLastError()); | ||
| return; | ||
| } | ||
| OnScopeLeave([&]() { UnhookWindowsHookEx(hHook); }); |
There was a problem hiding this comment.
Hook lifetime issue: The WH_MOUSE_LL hook is installed on the tracking thread at line 398, but the static callback MouseHookProc accesses the global s_active_instance_. If multiple MessageInput instances exist simultaneously (even if only one is tracking at a time), the hook from the first instance's thread will still be active when a second instance is created and starts its own tracking thread with its own hook.
This means there could be multiple hooks installed simultaneously, all sharing the same s_active_instance_. When the first instance is destroyed and unhooks, the second instance's hook is still active. This design appears intentional (only one instance can be active at a time via s_active_instance_), but it's not immediately clear and could cause issues if assumptions change.
Consider adding documentation explaining this single-active-instance design, or implementing explicit checks to prevent multiple tracking instances.
| s_active_instance_ = this; | ||
| tracking_active_ = true; | ||
|
|
||
| move_window_to_align_cursor(x, y); |
There was a problem hiding this comment.
Potential race condition: Multiple MessageInput instances could be created simultaneously (from different threads or sequentially), and the global s_active_instance_ will be overwritten. If one instance starts tracking after another has already set s_active_instance_, the hook will suddenly start processing mouse events for the new instance while the old instance may still have tracking_active_ set to true.
Consider adding a check to prevent multiple instances from being active simultaneously, or implement proper instance management to handle this scenario safely.
| s_active_instance_ = this; | |
| tracking_active_ = true; | |
| move_window_to_align_cursor(x, y); | |
| // 仅在当前没有其他活动实例（或已经是当前实例）时，才设置为活动实例 | |
| MessageInput* expected = nullptr; | |
| MessageInput* desired = this; | |
| if (s_active_instance_.compare_exchange_strong(expected, desired, std::memory_order_acq_rel, std::memory_order_acquire) || | |
| expected == this) { | |
| // 成功成为活动实例（或已经是活动实例），启动追踪 | |
| tracking_active_ = true; | |
| move_window_to_align_cursor(x, y); | |
| } else { | |
| // 已经有其他活动实例，避免抢占其追踪权 | |
| LogWarn << "MessageInput::prepare_mouse_position skipped activating new instance because another instance is already active." | |
| << VAR(expected) << VAR(this); | |
| } |
| template <typename Fn> | ||
| static Fn resolve_nt_function(const char* name) | ||
| { | ||
| static Fn func = nullptr; | ||
| static bool resolved = false; | ||
| if (!resolved) { | ||
| HMODULE ntdll = GetModuleHandleW(L"ntdll.dll"); | ||
| if (ntdll) { | ||
| func = reinterpret_cast<Fn>(GetProcAddress(ntdll, name)); | ||
| } | ||
| if (!func) { | ||
| LogWarn << "Failed to resolve ntdll function" << VAR(name); | ||
| } | ||
| resolved = true; | ||
| } | ||
| return func; | ||
| } |
There was a problem hiding this comment.
Thread-local static can cause issues: The resolve_nt_function template uses function-local static variables that are initialized once per template instantiation. However, this is being called from a background thread, and C++11 guarantees thread-safe initialization of function-local statics. This is correct, but if multiple threads call this simultaneously during first use, it could block.
More critically, if GetModuleHandleW or GetProcAddress fail silently (func remains null), subsequent calls won't retry. Consider logging a warning only on the first resolution attempt, not on every call where func is null.
| if (config_.with_window_pos) { | ||
| tracking_thread_ = std::thread(&MessageInput::tracking_thread_func, this); | ||
| } |
There was a problem hiding this comment.
Missing synchronization on thread startup: The constructor starts the tracking thread at line 30, but the thread immediately begins executing and may attempt to use hwnd_ before the constructor has fully initialized all member variables. While hwnd_ is initialized in the member initializer list before the constructor body, there's no guarantee that the thread won't start executing before the object is fully constructed.
Consider using a separate start() method or ensuring all initialization is complete in the member initializer list before starting the thread. Alternatively, use a flag to signal when initialization is complete.
| restore_cursor_pos(); | ||
| } | ||
| else if (config_.with_window_pos) { | ||
| tracking_active_ = false; |
There was a problem hiding this comment.
Critical race condition: s_active_instance_ is set to nullptr without synchronization with the tracking thread. The tracking thread may still be running and accessing inst from s_active_instance_ in MouseHookProc (line 227-243) when it's set to nullptr here. This can cause undefined behavior if the hook callback reads the nullptr and then this instance is destroyed before the callback completes.
The tracking thread should be signaled to stop and joined before clearing s_active_instance_, or proper synchronization is needed to ensure the hook callback isn't accessing the instance during destruction.
| tracking_active_ = false; | |
| tracking_active_ = false; | |
| if (tracking_thread_.joinable()) { | |
| tracking_thread_.join(); | |
| } |
| return CallNextHookEx(NULL, nCode, wParam, lParam); | ||
| } | ||
|
|
||
| MessageInput* inst = s_active_instance_.load(std::memory_order_acquire); |
There was a problem hiding this comment.
Memory order inconsistency: The hook callback uses memory_order_acquire to load s_active_instance_ (line 227), but restore_pos() sets it to nullptr without any memory order specification (line 144 uses default memory_order_seq_cst). For consistency and to match the acquire-release semantics, line 144 should use memory_order_release when setting to nullptr:
s_active_instance_.store(nullptr, std::memory_order_release);
Similarly, line 203 should use memory_order_release when storing the instance pointer.
| // 1. 挂起目标进程，使其看不到中间态 | ||
| suspend_target_process(); | ||
|
|
||
| // 2. 移动窗口（SWP_ASYNCWINDOWPOS 避免阻塞 + SWP_NOSENDCHANGING 跳过同步通知） | ||
| SetWindowPos(hwnd_, nullptr, new_left, new_top, 0, 0, | ||
| SWP_NOSIZE | SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOSENDCHANGING | SWP_ASYNCWINDOWPOS); | ||
|
|
||
| // 3. 释放光标到累积后的真实目标位置 | ||
| SetCursorPos(mx, my); | ||
|
|
||
| // 4. 恢复目标进程（它醒来时看到的窗口和光标已完美对齐） | ||
| resume_target_process(); |
There was a problem hiding this comment.
Potential deadlock: The tracking thread runs with THREAD_PRIORITY_HIGHEST (line 386) and calls suspend_target_process() which suspends all threads in the target process. If the target process holds system-wide locks (e.g., a window manager lock) when suspended, and the tracking thread needs to acquire that same lock through a Win32 API call (e.g., SetWindowPos, GetCursorPos), this could cause a deadlock.
This is a known issue with process suspension in general. Consider adding a timeout mechanism or reconsidering the suspend/resume approach if target processes commonly hold system locks.
| // 1. 挂起目标进程，使其看不到中间态 | |
| suspend_target_process(); | |
| // 2. 移动窗口（SWP_ASYNCWINDOWPOS 避免阻塞 + SWP_NOSENDCHANGING 跳过同步通知） | |
| SetWindowPos(hwnd_, nullptr, new_left, new_top, 0, 0, | |
| SWP_NOSIZE | SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOSENDCHANGING | SWP_ASYNCWINDOWPOS); | |
| // 3. 释放光标到累积后的真实目标位置 | |
| SetCursorPos(mx, my); | |
| // 4. 恢复目标进程（它醒来时看到的窗口和光标已完美对齐） | |
| resume_target_process(); | |
| // 直接移动窗口（SWP_ASYNCWINDOWPOS 避免阻塞 + SWP_NOSENDCHANGING 跳过同步通知） | |
| SetWindowPos(hwnd_, nullptr, new_left, new_top, 0, 0, | |
| SWP_NOSIZE | SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOSENDCHANGING | SWP_ASYNCWINDOWPOS); | |
| // 释放光标到累积后的真实目标位置 | |
| SetCursorPos(mx, my); |
| MSG msg; | ||
| while (!tracking_exit_) { | ||
| // 消息泵：WH_MOUSE_LL 钩子回调在此线程上被系统调度，必须保持消息循环活跃 | ||
| DWORD res = MsgWaitForMultipleObjects(0, NULL, FALSE, 1, QS_ALLINPUT); |
There was a problem hiding this comment.
Performance concern: The tracking thread calls MsgWaitForMultipleObjects with a 1ms timeout and then checks frame timing with a 16.67ms target. This means the thread will wake up approximately every 1ms, check the frame timing, and go back to sleep, resulting in high CPU usage for no benefit during the ~15ms between actual frame processing.
Consider using a longer wait timeout (e.g., 15ms) or implementing a smarter wait strategy that wakes only when messages are available or when the frame interval has elapsed.
|
|
Co-authored-by: Z_06 <z060606060606@gmail.com>
概述
本次 PR 主要为了解决在使用
WithWindowPos模式时，玩家移动物理鼠标导致模拟点击发生错位的问题。随着在底层实现零延迟的追踪机制后，原有的通过block_input强行剥夺玩家鼠标控制权的临时做法也被一并移除。终于可以毫无顾忌地边挂机边用鼠标操作其他区域了！ 🎉
变更内容
WH_MOUSE_LL鼠标钩子来拦截由玩家产生的真实硬件级别输入，将其提取并累加成内部增量偏移量，保证物理输入和程序的模拟点击彻底隔离。timeBeginPeriod(1)定时器的高精度），以极低延迟平滑同步光标中心与目标客户端位置。SetWindowPos和SetCursorPos进行重新对齐的极致瞬间，通过底层挂起和恢复操作（Suspend/Resume Process）防范由于异步操作导致的游戏内渲染出光标未对齐的中间失效帧，实现完美的“瞬移”效果。SM_XVIRTUALSCREEN等），防范由于拉扯导致目标和光标完全飞出不同屏幕引起的死锁或目标丢失问题。block_input构造默认参数下调为了false，真正实现了无感的后台控制。Summary by Sourcery
改进 WithWindowPos 的鼠标输入处理，将模拟点击与物理鼠标移动解耦，并消除对全局输入阻塞的需求。
Bug 修复：
功能增强：
杂项：
Original summary in English
Summary by Sourcery
Improve WithWindowPos mouse input handling to decouple simulated clicks from physical mouse movement and remove the need for global input blocking.
Bug Fixes:
Enhancements:
Chores: