Merged
Conversation
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并给出了一些高层次的反馈:
- 为所有
touch_down调用共用同一个saved_cursor_pos_,在多点触控或手势重叠的情况下可能会产生异常行为;建议为每个触点分别跟踪光标位置,或者仅在对应的touch_down/touch_up配对完成时再恢复光标。 - 在
touch_up中调用CGDisplayMoveCursorToPoint之前,最好先记录是否确实保存过有效的光标位置(例如使用一个布尔标记),以避免在touch_down被跳过或失败时,将光标恢复到未初始化或过期的位置。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- 为所有 `touch_down` 调用共用同一个 `saved_cursor_pos_`,在多点触控或手势重叠的情况下可能会产生异常行为;建议为每个触点分别跟踪光标位置,或者仅在对应的 `touch_down` / `touch_up` 配对完成时再恢复光标。
- 在 `touch_up` 中调用 `CGDisplayMoveCursorToPoint` 之前,最好先记录是否确实保存过有效的光标位置(例如使用一个布尔标记),以避免在 `touch_down` 被跳过或失败时,将光标恢复到未初始化或过期的位置。
## Individual Comments
### Comment 1
<location path="source/MaaMacOSControlUnit/Input/GlobalEventInput.mm" line_range="104-105" />
<code_context>
return false;
}
+ // 恢复鼠标到 touch_down 之前的位置
+ CGDisplayMoveCursorToPoint(CGMainDisplayID(), saved_cursor_pos_);
+
return true;
</code_context>
<issue_to_address>
**issue:** 考虑一下在没有先调用 touch_down 的情况下调用 touch_up 会发生什么(此时 saved_cursor_pos_ 仍是默认初始化)。
由于 `saved_cursor_pos_` 是默认初始化的,并且在 `touch_up` 中总会被使用,如果在没有先成功调用 `touch_down` 的情况下调用 `touch_up`,光标会被重置到 (0,0)。请在恢复前增加一个检查,确认确实保存过有效的位置(例如使用布尔标记,或跟踪对应触点是否已经开始)。
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Using a single
saved_cursor_pos_for alltouch_downcalls may misbehave with multi-touch or overlapping gestures; consider tracking cursor position per contact or only restoring when the correspondingtouch_down/touch_uppair completes. - Before calling
CGDisplayMoveCursorToPointintouch_up, it may be safer to track whether a valid cursor position was actually saved (e.g., with a boolean flag) to avoid restoring to an uninitialized or stale position iftouch_downwas skipped or failed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using a single `saved_cursor_pos_` for all `touch_down` calls may misbehave with multi-touch or overlapping gestures; consider tracking cursor position per contact or only restoring when the corresponding `touch_down`/`touch_up` pair completes.
- Before calling `CGDisplayMoveCursorToPoint` in `touch_up`, it may be safer to track whether a valid cursor position was actually saved (e.g., with a boolean flag) to avoid restoring to an uninitialized or stale position if `touch_down` was skipped or failed.
## Individual Comments
### Comment 1
<location path="source/MaaMacOSControlUnit/Input/GlobalEventInput.mm" line_range="104-105" />
<code_context>
return false;
}
+ // 恢复鼠标到 touch_down 之前的位置
+ CGDisplayMoveCursorToPoint(CGMainDisplayID(), saved_cursor_pos_);
+
return true;
</code_context>
<issue_to_address>
**issue:** Consider what happens if touch_up is called without a prior touch_down (saved_cursor_pos_ is default-initialized).
Because `saved_cursor_pos_` is default-initialized and always used on `touch_up`, calling `touch_up` without a prior successful `touch_down` will reset the cursor to (0,0). Please gate the restore behind a check that a valid position was actually saved (e.g., a boolean flag or tracking that the corresponding contact started).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines macOS control-unit behavior for input injection and screencap to better preserve user cursor state and prevent the cursor from affecting image recognition.
Changes:
- Disable cursor rendering in ScreenCaptureKit screenshots (
showsCursor = NO). - Refresh target window info on
touch_downfor GlobalEvent input to handle window movement before injection. - Save and restore system cursor position around GlobalEvent touch interactions; standardize unused
pressurehandling viastd::ignore.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| source/MaaMacOSControlUnit/Screencap/ScreenCaptureKitScreencap.mm | Configure ScreenCaptureKit capture to not include the cursor. |
| source/MaaMacOSControlUnit/Input/PostToPidInput.mm | Replace (void)pressure with std::ignore = pressure. |
| source/MaaMacOSControlUnit/Input/GlobalEventInput.mm | Update window info on touch down; save/restore cursor position; use std::ignore for pressure. |
| source/MaaMacOSControlUnit/Input/GlobalEventInput.h | Add member to store saved cursor position. |
Contributor
|
看起来没啥问题,ai提到的可以保护下 |
…ork into MacOSGlobalEventPatch
Co-authored-by: Copilot <[email protected]>
Contributor
Author
|
ai提到的问题都修了 这些建议提的很可以 看下没问题的话可以合并了 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
几点更新:
由 Sourcery 提供的摘要
调整 macOS 全局输入处理和截图行为,以更好地保持光标状态并避免将光标捕捉到截图中。
错误修复:
增强改进:
std::ignore统一处理未使用的压力(pressure)参数。Original summary in English
Summary by Sourcery
Adjust macOS global input handling and screenshot behavior to better preserve cursor state and avoid capturing the cursor.
Bug Fixes:
Enhancements: