fix(win32): 修复前台截图无法激活目标窗口 - #1438
Open
he0119 wants to merge 2 commits into
Open
Conversation
恢复最小化或隐藏的目标窗口，并等待前台切换结果。 在普通激活失败时，短暂关联当前前台线程的输入队列并临时调整 Z 序后重试；通过 RAII 保证输入队列解除关联且不遗留永久置顶。 同时将截图置前冷却改为原子状态，并为 ScreenDC 与 DesktopDupWindow 补充失败诊断信息。
he0119
marked this pull request as draft
August 11, 2026 08:50
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并留下了一些总体反馈：
ensure_foreground_with_cooldown中的冷却逻辑现在在仍处于时间间隔内时会提前返回false，而不是像之前的实现那样再次检查这段时间内窗口是否已经变为前台，这在行为上有细微变化；请考虑这种回退是否可以接受，或者是否需要恢复之前的语义。- 围绕
last_foreground_attempt.compare_exchange_weak的自旋循环使用了memory_order_relaxed，在高竞争情况下理论上可能无限循环；你可以考虑用fetch_max/exchange或单次 CAS 而不手写重试循环来简化逻辑，因为这里对精确的内存序并不敏感。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The cooldown logic in `ensure_foreground_with_cooldown` now returns `false` early when within the interval instead of rechecking whether the window became foreground in the meantime (as the previous implementation did), which subtly changes behavior; consider whether this regression is acceptable or restore the prior semantics.
- The spin loop around `last_foreground_attempt.compare_exchange_weak` uses `memory_order_relaxed` and can theoretically loop indefinitely under high contention; you might simplify this by using `fetch_max`/`exchange` or a single CAS without a manual retry loop since exact ordering is not critical here.
## Individual Comments
### Comment 1
<location path="source/MaaWin32ControlUnit/Base/ForegroundUtils.cpp" line_range="129-138" />
<code_context>
+ const DWORD now = GetTickCount();
+ DWORD previous_attempt = last_foreground_attempt.load(std::memory_order_relaxed);
+ while (true) {
+ if (previous_attempt != 0 && now - previous_attempt < kForegroundRecoveryInterval) {
+ return false;
+ }
+ if (last_foreground_attempt.compare_exchange_weak(previous_attempt, now, std::memory_order_relaxed)) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Cooldown logic no longer checks if the window became foreground independently, which changes previous behavior.
Previously, within the cooldown interval we returned `hwnd == GetForegroundWindow()`, so the call still succeeded if something else had already brought the window to the foreground. Now we always return `false` in that case. If callers interpret `true` as "already in foreground" (regardless of who did it), this can introduce spurious warnings or retries. To keep the prior behavior, consider checking `GetForegroundWindow()` before returning, e.g. `return hwnd == GetForegroundWindow();`.
```suggestion
const DWORD now = GetTickCount();
DWORD previous_attempt = last_foreground_attempt.load(std::memory_order_relaxed);
while (true) {
if (previous_attempt != 0 && now - previous_attempt < kForegroundRecoveryInterval) {
// Within cooldown: preserve previous behavior by still succeeding
// if the window is already in the foreground (regardless of who did it).
return hwnd == GetForegroundWindow();
}
if (last_foreground_attempt.compare_exchange_weak(previous_attempt, now, std::memory_order_relaxed)) {
break;
}
}
```
</issue_to_address>
### Comment 2
<location path="source/MaaWin32ControlUnit/Base/ForegroundUtils.cpp" line_range="56-59" />
<code_context>
+ return;
+ }
+
+ ShowWindowAsync(hwnd, IsIconic(hwnd) ? SW_RESTORE : SW_SHOW);
+
+ const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(100);
+ while ((IsIconic(hwnd) || !IsWindowVisible(hwnd)) && std::chrono::steady_clock::now() < deadline) {
+ std::this_thread::sleep_for(std::chrono::milliseconds(5));
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Repeated calls to IsIconic/IsWindowVisible without re-checking IsWindow may lead to confusing behavior if the window is destroyed mid-loop.
Inside `restore_window`, consider checking `IsWindow(hwnd)` within the loop and breaking early if it returns false. Otherwise, if the window is destroyed while you’re polling `IsIconic` / `IsWindowVisible`, you may keep looping against a dead handle until the deadline, which can lead to surprising behavior during teardown.
Suggested implementation:
```cpp
void restore_window(HWND hwnd)
{
if (!IsWindow(hwnd)) {
return;
}
if (!IsIconic(hwnd) && IsWindowVisible(hwnd)) {
return;
}
ShowWindowAsync(hwnd, IsIconic(hwnd) ? SW_RESTORE : SW_SHOW);
const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(100);
while (IsWindow(hwnd) &&
(IsIconic(hwnd) || !IsWindowVisible(hwnd)) &&
std::chrono::steady_clock::now() < deadline) {
std::this_thread::sleep_for(std::chrono::milliseconds(5));
}
attached_ = AttachThreadInput(source_thread_id_, target_thread_id_, TRUE) != FALSE;
```
1. Ensure `ForegroundUtils.cpp` includes `<chrono>` and `<thread>` at the top of the file:
- Add `#include <chrono>` and `#include <thread>` if they are not already present.
2. If the project avoids fully-qualified names for `std::chrono` / `std::this_thread`, you may instead add appropriate `using` declarations consistent with the existing style.
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The cooldown logic in
ensure_foreground_with_cooldownnow returnsfalseearly when within the interval instead of rechecking whether the window became foreground in the meantime (as the previous implementation did), which subtly changes behavior; consider whether this regression is acceptable or restore the prior semantics. - The spin loop around
last_foreground_attempt.compare_exchange_weakusesmemory_order_relaxedand can theoretically loop indefinitely under high contention; you might simplify this by usingfetch_max/exchangeor a single CAS without a manual retry loop since exact ordering is not critical here.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The cooldown logic in `ensure_foreground_with_cooldown` now returns `false` early when within the interval instead of rechecking whether the window became foreground in the meantime (as the previous implementation did), which subtly changes behavior; consider whether this regression is acceptable or restore the prior semantics.
- The spin loop around `last_foreground_attempt.compare_exchange_weak` uses `memory_order_relaxed` and can theoretically loop indefinitely under high contention; you might simplify this by using `fetch_max`/`exchange` or a single CAS without a manual retry loop since exact ordering is not critical here.
## Individual Comments
### Comment 1
<location path="source/MaaWin32ControlUnit/Base/ForegroundUtils.cpp" line_range="129-138" />
<code_context>
+ const DWORD now = GetTickCount();
+ DWORD previous_attempt = last_foreground_attempt.load(std::memory_order_relaxed);
+ while (true) {
+ if (previous_attempt != 0 && now - previous_attempt < kForegroundRecoveryInterval) {
+ return false;
+ }
+ if (last_foreground_attempt.compare_exchange_weak(previous_attempt, now, std::memory_order_relaxed)) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Cooldown logic no longer checks if the window became foreground independently, which changes previous behavior.
Previously, within the cooldown interval we returned `hwnd == GetForegroundWindow()`, so the call still succeeded if something else had already brought the window to the foreground. Now we always return `false` in that case. If callers interpret `true` as "already in foreground" (regardless of who did it), this can introduce spurious warnings or retries. To keep the prior behavior, consider checking `GetForegroundWindow()` before returning, e.g. `return hwnd == GetForegroundWindow();`.
```suggestion
const DWORD now = GetTickCount();
DWORD previous_attempt = last_foreground_attempt.load(std::memory_order_relaxed);
while (true) {
if (previous_attempt != 0 && now - previous_attempt < kForegroundRecoveryInterval) {
// Within cooldown: preserve previous behavior by still succeeding
// if the window is already in the foreground (regardless of who did it).
return hwnd == GetForegroundWindow();
}
if (last_foreground_attempt.compare_exchange_weak(previous_attempt, now, std::memory_order_relaxed)) {
break;
}
}
```
</issue_to_address>
### Comment 2
<location path="source/MaaWin32ControlUnit/Base/ForegroundUtils.cpp" line_range="56-59" />
<code_context>
+ return;
+ }
+
+ ShowWindowAsync(hwnd, IsIconic(hwnd) ? SW_RESTORE : SW_SHOW);
+
+ const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(100);
+ while ((IsIconic(hwnd) || !IsWindowVisible(hwnd)) && std::chrono::steady_clock::now() < deadline) {
+ std::this_thread::sleep_for(std::chrono::milliseconds(5));
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Repeated calls to IsIconic/IsWindowVisible without re-checking IsWindow may lead to confusing behavior if the window is destroyed mid-loop.
Inside `restore_window`, consider checking `IsWindow(hwnd)` within the loop and breaking early if it returns false. Otherwise, if the window is destroyed while you’re polling `IsIconic` / `IsWindowVisible`, you may keep looping against a dead handle until the deadline, which can lead to surprising behavior during teardown.
Suggested implementation:
```cpp
void restore_window(HWND hwnd)
{
if (!IsWindow(hwnd)) {
return;
}
if (!IsIconic(hwnd) && IsWindowVisible(hwnd)) {
return;
}
ShowWindowAsync(hwnd, IsIconic(hwnd) ? SW_RESTORE : SW_SHOW);
const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(100);
while (IsWindow(hwnd) &&
(IsIconic(hwnd) || !IsWindowVisible(hwnd)) &&
std::chrono::steady_clock::now() < deadline) {
std::this_thread::sleep_for(std::chrono::milliseconds(5));
}
attached_ = AttachThreadInput(source_thread_id_, target_thread_id_, TRUE) != FALSE;
```
1. Ensure `ForegroundUtils.cpp` includes `<chrono>` and `<thread>` at the top of the file:
- Add `#include <chrono>` and `#include <thread>` if they are not already present.
2. If the project avoids fully-qualified names for `std::chrono` / `std::this_thread`, you may instead add appropriate `using` declarations consistent with the existing style.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
|
这个 PR ready 了吗 |
Contributor
Author
这是根据我之前遇到的 bug 让 gpt 写的，测试下来是修复了，但是我没有能力去审核。之前 @zmdyy0318 说他要来看看。 |
Contributor
|
@zmdyy0318 push一下 |
zmdyy0318
marked this pull request as ready for review
September 5, 2026 14:52
Contributor
|
测试通过 push @MistEO |
Contributor
There was a problem hiding this comment.
您好——我发现了 1 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论：
## 单独评论
### 评论 1
<location path="source/MaaWin32ControlUnit/Base/ForegroundUtils.h" line_range="31-34" />
<code_context>
+ // SetForegroundWindow 可能被系统前台限制拒绝，但 TOPMOST 切换不受此限制
if (hwnd != GetForegroundWindow()) {
- SetWindowPos(hwnd, HWND_TOP, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE);
+ SetWindowPos(hwnd, HWND_TOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE);
+ SetWindowPos(hwnd, HWND_NOTOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE);
std::this_thread::sleep_for(std::chrono::milliseconds(5));
}
}
</code_context>
<issue_to_address>
**问题 (bug_risk)：** 当 SetForegroundWindow 仍被阻止时，回退逻辑只执行了一次短暂的 TOPMOST 切换，并且从未验证或确保目标窗口已成为前台窗口；恢复为 NOTOPMOST 后，其他前台窗口或 TOPMOST 窗口仍可能覆盖它，而 ScreenDC 和 DesktopDup 继续进行捕获。
**触发条件：** 当 Windows 拒绝前台窗口请求，且另一个窗口与目标窗口重叠时。
**建议修复：** 仅将此回退逻辑作为重试的一部分，并重新检查 GetForegroundWindow；如果目标窗口仍不是前台窗口，则应让激活/捕获失败，而不是继续执行。
</issue_to_address>Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="source/MaaWin32ControlUnit/Base/ForegroundUtils.h" line_range="31-34" />
<code_context>
+ // SetForegroundWindow 可能被系统前台限制拒绝，但 TOPMOST 切换不受此限制
if (hwnd != GetForegroundWindow()) {
- SetWindowPos(hwnd, HWND_TOP, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE);
+ SetWindowPos(hwnd, HWND_TOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE);
+ SetWindowPos(hwnd, HWND_NOTOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE);
std::this_thread::sleep_for(std::chrono::milliseconds(5));
}
}
</code_context>
<issue_to_address>
**issue (bug_risk):** When SetForegroundWindow remains blocked, the fallback only performs a transient TOPMOST toggle and never verifies or establishes that the target became the foreground window; after reverting to NOTOPMOST, another foreground or TOPMOST window can still cover it while ScreenDC and DesktopDup continue capturing.
**Triggers:** When Windows rejects the foreground request and another window overlaps the target.
**Suggested fix:** Use the fallback only as part of a retry that rechecks GetForegroundWindow, and fail the activation/capture rather than proceeding if the target is still not foreground.
</issue_to_address>
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
3 participants
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.
close MaaEnd/MaaEnd#5465
close MaaEnd/MaaEnd#4820
问题
Win32 前台截图后端会在截图前尝试激活目标窗口。原实现只依次调用
SetWindowPos和SetForegroundWindow，再以固定短延迟等待；当截图进程受 Windows 前台切换限制时，调用可能被拒绝，ScreenDC 或DXGI_DesktopDup_Window随后仍会继续截图，导致目标窗口被其他窗口遮挡。此外，原有 5 秒置前冷却使用函数内静态
DWORD，多个截图线程并发访问时存在数据竞争。修改内容
.cpp，统一维护进程内冷却状态。SetWindowPos(HWND_TOP)、BringWindowToTop和SetForegroundWindow，等待 20 ms 确认结果。PeekMessageW确保当前线程拥有消息队列；SetFocus/SetActiveWindow；退出作用域时始终解除输入队列关联。std::atomic<DWORD>，消除并发数据竞争。DXGI_DesktopDup_Window的置前失败日志补充目标 HWND、实际前台 HWND、最小化状态和可见状态。验证
git diff --checkcmake --build build --config RelWithDebInfo --target MaaWin32ControlUnit --parallel 16succeeded=1、foreground_after=target、topmost_after=0。v5.13.0-beta.2及其原始 MaaUtils/OpenCV 运行完整 ScreenDC 链路：create、connect、screencap均成功，得到有效的 1920×1080 三通道截图。已知边界
目标游戏会拒绝外部
SW_MINIMIZE，因此最小化后的真实游戏恢复路径未能完成端到端验证；普通后台遮挡与完整截图链路已验证。Sourcery 总结
改进 Win32 屏幕截图的前台激活功能，确保目标窗口能够可靠地恢复并置于前台，同时避免并发竞争问题。
Bug 修复：
增强功能：
测试：
Original summary in English
Sourcery 总结
确保 Win32 截图捕获在执行捕获前可靠地激活目标窗口，同时保证并发使用时的安全性。
错误修复：
增强功能：
测试：
Original summary in English
Summary by Sourcery
Make Win32 screenshot capture reliably activate the target window before capturing while remaining safe under concurrent use.
Bug Fixes:
Enhancements:
Tests:
Bug Fixes（缺陷修复）:
Enhancements（增强功能）:
Original summary in English