feat: 添加 FramePoolWithPseudoMinimize 和 PrintWindowWithPseudoMinimize 截图方式#1153
feat: 添加 FramePoolWithPseudoMinimize 和 PrintWindowWithPseudoMinimize 截图方式#1153MistEO merged 11 commits intoMaaXYZ:mainfrom
Conversation
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并留下了一些总体反馈:
- 在
Win32ControlUnitMgr::connect中,通过对screencap_method_进行相等性判断来检测pseudo_minimize,但该类型是一个位掩码;如果以后组合多个标志,这种写法就会失效,因此建议使用按位&检查而不是==来检测伪最小化的几种变体。 PseudoMinimizeHelper在start()中保存了original_ex_style_,但在恢复时并没有使用它;要么移除这个成员,要么在恢复时使用它还原完整的原始样式,以避免携带未使用的状态。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Win32ControlUnitMgr::connect`, `pseudo_minimize` is detected via equality checks on `screencap_method_`, but the type is a bitmask; if multiple flags are ever combined this will break, so consider using bitwise `&` checks instead of `==` to detect the pseudo-minimize variants.
- `PseudoMinimizeHelper` stores `original_ex_style_` in `start()` but never uses it when reverting; either remove this member or restore the full original style from it to avoid carrying unused state.
## Individual Comments
### Comment 1
<location path="source/MaaWin32ControlUnit/Screencap/PseudoMinimizeHelper.h" line_range="41-43" />
<code_context>
+ std::thread monitor_thread_;
+
+ // 保存窗口原始属性,用于恢复
+ LONG_PTR original_ex_style_ = 0;
+ BYTE original_alpha_ = 255;
+ bool had_layered_style_ = false;
+};
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Stored original_ex_style_ is never used when restoring window styles, which can lead to subtle style loss.
Because `revert_pseudo_minimize()` rebuilds the style from `current_ex_style` by only masking out `WS_EX_TRANSPARENT` (and maybe `WS_EX_LAYERED`), any changes made to the extended style while the helper is active can cause divergence from the true original flags. Restoring from `original_ex_style_` (with only necessary adjustments) would be more robust and deterministic; otherwise, consider removing `original_ex_style_` and documenting that the current behavior is intentional.
Suggested implementation:
```c
std::thread monitor_thread_;
// 保存窗口原始属性,用于恢复
LONG_PTR original_ex_style_ = 0;
BYTE original_alpha_ = 255;
bool had_layered_style_ = false;
#include <atomic>
#include <thread>
#include "MaaUtils/SafeWindows.hpp"
#include "Common/Conf.h"
MAA_CTRL_UNIT_NS_BEGIN
// 伪最小化辅助类:当目标窗口被最小化时,将其设为透明+点击穿透并还原,
```
To fully implement your comment (restoring from `original_ex_style_` instead of a mutated current style), you should also update the corresponding implementation file, likely `source/MaaWin32ControlUnit/Screencap/PseudoMinimizeHelper.cpp`, along these lines:
1. In the method that *applies* pseudo-minimize (e.g. `PseudoMinimizeHelper::apply_pseudo_minimize()`):
- Read and store the original extended styles before modifying them, e.g.:
- `original_ex_style_ = GetWindowLongPtr(hwnd_, GWL_EXSTYLE);`
- `had_layered_style_ = (original_ex_style_ & WS_EX_LAYERED) != 0;`
- Read and store the original alpha (if you are using `GetLayeredWindowAttributes`), e.g.:
- `GetLayeredWindowAttributes(hwnd_, nullptr, &original_alpha_, &flags);`
- Then apply the transparent / click-through styles as you already do.
2. In `PseudoMinimizeHelper::revert_pseudo_minimize()`:
- Stop rebuilding the extended style from a `current_ex_style` with `~WS_EX_TRANSPARENT` and conditional `~WS_EX_LAYERED`.
- Instead, restore from `original_ex_style_` (with only necessary adjustments if some invariants must be preserved), e.g.:
- `SetWindowLongPtr(hwnd_, GWL_EXSTYLE, original_ex_style_);`
- Restore opacity using `original_alpha_` and `had_layered_style_`, e.g.:
- If `had_layered_style_` was false, optionally clear `WS_EX_LAYERED` and/or disable layered alpha.
- Otherwise, reapply `SetLayeredWindowAttributes` with `original_alpha_`.
3. Ensure these fields are correctly initialized/reset when:
- The helper is constructed or reused for a different `HWND`.
- `apply_pseudo_minimize()` fails part-way (avoid leaving inconsistent state).
These `.cpp` changes are required to make use of `original_ex_style_` and avoid the subtle divergence you described; otherwise, if you decide the current behavior is acceptable, you should instead remove `original_ex_style_` from the header and update the comment to document that only a subset of extended styles is restored.
</issue_to_address>请帮我变得更有用!欢迎在每条评论上点 👍 或 👎,我会根据你的反馈改进评审质量。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
Win32ControlUnitMgr::connect,pseudo_minimizeis detected via equality checks onscreencap_method_, but the type is a bitmask; if multiple flags are ever combined this will break, so consider using bitwise&checks instead of==to detect the pseudo-minimize variants. PseudoMinimizeHelperstoresoriginal_ex_style_instart()but never uses it when reverting; either remove this member or restore the full original style from it to avoid carrying unused state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Win32ControlUnitMgr::connect`, `pseudo_minimize` is detected via equality checks on `screencap_method_`, but the type is a bitmask; if multiple flags are ever combined this will break, so consider using bitwise `&` checks instead of `==` to detect the pseudo-minimize variants.
- `PseudoMinimizeHelper` stores `original_ex_style_` in `start()` but never uses it when reverting; either remove this member or restore the full original style from it to avoid carrying unused state.
## Individual Comments
### Comment 1
<location path="source/MaaWin32ControlUnit/Screencap/PseudoMinimizeHelper.h" line_range="41-43" />
<code_context>
+ std::thread monitor_thread_;
+
+ // 保存窗口原始属性,用于恢复
+ LONG_PTR original_ex_style_ = 0;
+ BYTE original_alpha_ = 255;
+ bool had_layered_style_ = false;
+};
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Stored original_ex_style_ is never used when restoring window styles, which can lead to subtle style loss.
Because `revert_pseudo_minimize()` rebuilds the style from `current_ex_style` by only masking out `WS_EX_TRANSPARENT` (and maybe `WS_EX_LAYERED`), any changes made to the extended style while the helper is active can cause divergence from the true original flags. Restoring from `original_ex_style_` (with only necessary adjustments) would be more robust and deterministic; otherwise, consider removing `original_ex_style_` and documenting that the current behavior is intentional.
Suggested implementation:
```c
std::thread monitor_thread_;
// 保存窗口原始属性,用于恢复
LONG_PTR original_ex_style_ = 0;
BYTE original_alpha_ = 255;
bool had_layered_style_ = false;
#include <atomic>
#include <thread>
#include "MaaUtils/SafeWindows.hpp"
#include "Common/Conf.h"
MAA_CTRL_UNIT_NS_BEGIN
// 伪最小化辅助类:当目标窗口被最小化时,将其设为透明+点击穿透并还原,
```
To fully implement your comment (restoring from `original_ex_style_` instead of a mutated current style), you should also update the corresponding implementation file, likely `source/MaaWin32ControlUnit/Screencap/PseudoMinimizeHelper.cpp`, along these lines:
1. In the method that *applies* pseudo-minimize (e.g. `PseudoMinimizeHelper::apply_pseudo_minimize()`):
- Read and store the original extended styles before modifying them, e.g.:
- `original_ex_style_ = GetWindowLongPtr(hwnd_, GWL_EXSTYLE);`
- `had_layered_style_ = (original_ex_style_ & WS_EX_LAYERED) != 0;`
- Read and store the original alpha (if you are using `GetLayeredWindowAttributes`), e.g.:
- `GetLayeredWindowAttributes(hwnd_, nullptr, &original_alpha_, &flags);`
- Then apply the transparent / click-through styles as you already do.
2. In `PseudoMinimizeHelper::revert_pseudo_minimize()`:
- Stop rebuilding the extended style from a `current_ex_style` with `~WS_EX_TRANSPARENT` and conditional `~WS_EX_LAYERED`.
- Instead, restore from `original_ex_style_` (with only necessary adjustments if some invariants must be preserved), e.g.:
- `SetWindowLongPtr(hwnd_, GWL_EXSTYLE, original_ex_style_);`
- Restore opacity using `original_alpha_` and `had_layered_style_`, e.g.:
- If `had_layered_style_` was false, optionally clear `WS_EX_LAYERED` and/or disable layered alpha.
- Otherwise, reapply `SetLayeredWindowAttributes` with `original_alpha_`.
3. Ensure these fields are correctly initialized/reset when:
- The helper is constructed or reused for a different `HWND`.
- `apply_pseudo_minimize()` fails part-way (avoid leaving inconsistent state).
These `.cpp` changes are required to make use of `original_ex_style_` and avoid the subtle divergence you described; otherwise, if you decide the current behavior is acceptable, you should instead remove `original_ex_style_` from the header and update the comment to document that only a subset of extended styles is restored.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我在这里给出了一些高层面的反馈:
- 在
Win32ControlUnitMgr::connect中,supports_minimized和截图方式的 switch 目前只区分MaaWin32ScreencapMethod_FramePool和_PrintWindow,但说明中提到了新的*_WithPseudoMinimize标志位(第 6/7 位);建议在supports_minimized的检查和 switch 中显式接入这些新的枚举值,以避免调用方传入新标志位时出现语义不清或配置错误。 - 在
PseudoMinimizeHelper::revert_pseudo_minimize中,整体恢复original_ex_style_和original_alpha_可能会覆盖应用在 helper 运行期间做出的样式更改;建议只恢复实际被修改过的标志/属性(例如,仅移除WS_EX_LAYERED/WS_EX_TRANSPARENT,或者仅在 alpha 未被外部修改时恢复原值),以降低覆盖外部更新的风险。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `Win32ControlUnitMgr::connect`, `supports_minimized` and the screencap switch only distinguish `MaaWin32ScreencapMethod_FramePool` and `_PrintWindow`, but the description mentions new `*_WithPseudoMinimize` flags (bits 6/7); consider wiring those new enum values explicitly into the `supports_minimized` check and switch to avoid ambiguity or misconfiguration when callers pass the new flags.
- In `PseudoMinimizeHelper::revert_pseudo_minimize`, restoring `original_ex_style_` and `original_alpha_` wholesale may overwrite style changes made by the application while the helper is running; consider limiting restoration to the flags/attributes actually modified (e.g., only removing `WS_EX_LAYERED`/`WS_EX_TRANSPARENT` or restoring alpha if it has not changed) to reduce the risk of clobbering external updates.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据这些反馈改进后续评审。
Original comment in English
Hey - I've left some high level feedback:
- In
Win32ControlUnitMgr::connect,supports_minimizedand the screencap switch only distinguishMaaWin32ScreencapMethod_FramePooland_PrintWindow, but the description mentions new*_WithPseudoMinimizeflags (bits 6/7); consider wiring those new enum values explicitly into thesupports_minimizedcheck and switch to avoid ambiguity or misconfiguration when callers pass the new flags. - In
PseudoMinimizeHelper::revert_pseudo_minimize, restoringoriginal_ex_style_andoriginal_alpha_wholesale may overwrite style changes made by the application while the helper is running; consider limiting restoration to the flags/attributes actually modified (e.g., only removingWS_EX_LAYERED/WS_EX_TRANSPARENTor restoring alpha if it has not changed) to reduce the risk of clobbering external updates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Win32ControlUnitMgr::connect`, `supports_minimized` and the screencap switch only distinguish `MaaWin32ScreencapMethod_FramePool` and `_PrintWindow`, but the description mentions new `*_WithPseudoMinimize` flags (bits 6/7); consider wiring those new enum values explicitly into the `supports_minimized` check and switch to avoid ambiguity or misconfiguration when callers pass the new flags.
- In `PseudoMinimizeHelper::revert_pseudo_minimize`, restoring `original_ex_style_` and `original_alpha_` wholesale may overwrite style changes made by the application while the helper is running; consider limiting restoration to the flags/attributes actually modified (e.g., only removing `WS_EX_LAYERED`/`WS_EX_TRANSPARENT` or restoring alpha if it has not changed) to reduce the risk of clobbering external updates.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 adds pseudo-minimize functionality to Win32 screencap methods to allow capturing minimized windows. When a window is minimized, the feature makes it transparent and click-through, then restores it without activation, enabling screencap to continue without disturbing the user.
However, there is a critical discrepancy: The PR description claims to add new C API constants MaaWin32ScreencapMethod_FramePoolWithPseudoMinimize (bit 6) and MaaWin32ScreencapMethod_PrintWindowWithPseudoMinimize (bit 7), but the actual implementation modifies the behavior of existing FramePool and PrintWindow methods to always use pseudo-minimize internally. No new constants are added to the C API.
Changes:
- Added
PseudoMinimizeHelperclass to manage window state manipulation with a background monitoring thread - Created wrapper classes
FramePoolWithPseudoMinimizeScreencapandPrintWindowWithPseudoMinimizeScreencapthat compose the original screencap classes with the helper - Modified
Win32ControlUnitMgrto instantiate the pseudo-minimize variants for FramePool and PrintWindow methods - Updated documentation in C header, NodeJS bindings, and English/Chinese docs to describe pseudo-minimize behavior
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| source/MaaWin32ControlUnit/Screencap/PseudoMinimizeHelper.h | New helper class header defining window state management with background thread |
| source/MaaWin32ControlUnit/Screencap/PseudoMinimizeHelper.cpp | Implementation of pseudo-minimize logic including window style manipulation and monitoring thread |
| source/MaaWin32ControlUnit/Screencap/FramePoolWithPseudoMinimizeScreencap.h | Wrapper class composing FramePoolScreencap with PseudoMinimizeHelper |
| source/MaaWin32ControlUnit/Screencap/PrintWindowWithPseudoMinimizeScreencap.h | Wrapper class composing PrintWindowScreencap with PseudoMinimizeHelper |
| source/MaaWin32ControlUnit/Manager/Win32ControlUnitMgr.cpp | Updated to instantiate pseudo-minimize variants and skip IsIconic check for supported methods |
| include/MaaFramework/MaaDef.h | Updated documentation to note FramePool and PrintWindow support pseudo-minimize |
| source/binding/NodeJS/src/apis/constant.d.ts | Updated TypeScript documentation comments to describe pseudo-minimize behavior |
| docs/en_us/2.4-ControlMethods.md | Added note about pseudo-minimize support in FramePool and PrintWindow methods |
Comments suppressed due to low confidence (1)
include/MaaFramework/MaaDef.h:326
- Discrepancy between PR description and implementation: The PR description states "新增
MaaWin32ScreencapMethod_FramePoolWithPseudoMinimize(bit 6) 和MaaWin32ScreencapMethod_PrintWindowWithPseudoMinimize(bit 7)" and mentions adding new C API constants. However, no new constants are being added to this header file. Instead, the implementation modifies the behavior of existingMaaWin32ScreencapMethod_FramePoolandMaaWin32ScreencapMethod_PrintWindowmethods to always use pseudo-minimize. This is a breaking change in behavior that should be clearly documented, or separate constants should actually be added as described in the PR.
#define MaaWin32ScreencapMethod_None 0ULL
#define MaaWin32ScreencapMethod_GDI 1ULL
#define MaaWin32ScreencapMethod_FramePool (1ULL << 1)
#define MaaWin32ScreencapMethod_DXGI_DesktopDup (1ULL << 2)
#define MaaWin32ScreencapMethod_DXGI_DesktopDup_Window (1ULL << 3)
#define MaaWin32ScreencapMethod_PrintWindow (1ULL << 4)
#define MaaWin32ScreencapMethod_ScreenDC (1ULL << 5)
| // 保存窗口原始扩展样式 | ||
| original_ex_style_ = GetWindowLongPtr(hwnd_, GWL_EXSTYLE); | ||
| had_layered_style_ = (original_ex_style_ & WS_EX_LAYERED) != 0; | ||
|
|
||
| // 如果窗口原本就有 WS_EX_LAYERED,尝试读取原始透明度 | ||
| if (had_layered_style_) { | ||
| DWORD flags = 0; | ||
| COLORREF color_key = 0; | ||
| if (GetLayeredWindowAttributes(hwnd_, &color_key, &original_alpha_, &flags)) { | ||
| // 成功读取 | ||
| } | ||
| else { | ||
| original_alpha_ = 255; | ||
| } | ||
| } | ||
| else { | ||
| original_alpha_ = 255; | ||
| } | ||
|
|
||
| running_ = true; | ||
| monitor_thread_ = std::thread(&PseudoMinimizeHelper::monitor_thread_func, this); | ||
|
|
||
| LogInfo << "PseudoMinimizeHelper started" << VAR_VOIDP(hwnd_); |
There was a problem hiding this comment.
Potential data race on non-atomic members: The non-atomic members original_ex_style_, original_alpha_, and had_layered_style_ are written in start() (lines 26-42) before the monitor thread starts, but they are read in revert_pseudo_minimize() which can be called from either the monitor thread or from stop() on the main thread. While the current code might work due to happens-before relationships through running_ and thread join, this creates a fragile dependency. Consider documenting this ordering requirement or making these members atomic/protected by a mutex for clarity and future maintainability.
| case MaaWin32ScreencapMethod_FramePool: | ||
| screencap_ = std::make_shared<FramePoolScreencap>(hwnd_); | ||
| screencap_ = std::make_shared<FramePoolWithPseudoMinimizeScreencap>(hwnd_); |
There was a problem hiding this comment.
Breaking behavioral change without opt-in: The implementation is changing existing MaaWin32ScreencapMethod_FramePool to always instantiate FramePoolWithPseudoMinimizeScreencap instead of FramePoolScreencap. This changes the behavior for all existing users of FramePool who did not opt into pseudo-minimize functionality. The pseudo-minimize feature manipulates window styles (transparent, click-through) which could interfere with applications that have their own window management logic. Consider either: (1) adding new separate enum values as the PR description suggests, (2) making this opt-in via a separate flag/option, or (3) clearly documenting this as a breaking change.
| void start(); | ||
| void stop(); | ||
|
|
||
| // 截图前同步调用:若窗口处于最小化状态,立即执行伪最小化并等待窗口就绪 |
There was a problem hiding this comment.
Unclear comment wording: The comment "截图前同步调用:若窗口处于最小化状态,立即执行伪最小化并等待窗口就绪" (line 27) says "wait for window ready" (等待窗口就绪), but the actual implementation in ensure_not_minimized() doesn't include any waiting logic. It just calls apply_pseudo_minimize() and returns immediately. Either add the waiting logic or update the comment to accurately reflect what the function does.
| // 截图前同步调用:若窗口处于最小化状态,立即执行伪最小化并等待窗口就绪 | |
| // 截图前同步调用:若窗口处于最小化状态,立即执行伪最小化以确保可截图 |
| case MaaWin32ScreencapMethod_PrintWindow: | ||
| screencap_ = std::make_shared<PrintWindowScreencap>(hwnd_); | ||
| screencap_ = std::make_shared<PrintWindowWithPseudoMinimizeScreencap>(hwnd_); |
There was a problem hiding this comment.
Same breaking behavioral change issue: Similar to FramePool, changing PrintWindow to always use pseudo-minimize affects all existing users without their consent. This could cause unexpected behavior for applications not expecting window manipulation.
| // 1. 添加 WS_EX_LAYERED 和 WS_EX_TRANSPARENT 样式 | ||
| LONG_PTR ex_style = GetWindowLongPtr(hwnd_, GWL_EXSTYLE); | ||
| SetWindowLongPtr(hwnd_, GWL_EXSTYLE, ex_style | WS_EX_LAYERED | WS_EX_TRANSPARENT); | ||
|
|
||
| // 2. 设置完全透明 | ||
| SetLayeredWindowAttributes(hwnd_, 0, 0, LWA_ALPHA); | ||
|
|
||
| // 3. 以不激活方式还原窗口,使截图方式能工作 | ||
| ShowWindow(hwnd_, SW_SHOWNOACTIVATE); | ||
|
|
||
| pseudo_minimized_ = true; |
There was a problem hiding this comment.
Missing error handling for Win32 API calls: SetWindowLongPtr, SetLayeredWindowAttributes, and ShowWindow can fail, but their return values are not checked. If these calls fail during apply_pseudo_minimize(), the window could be left in an inconsistent state (e.g., transparent but still minimized, or with WS_EX_TRANSPARENT set but visible). Consider checking return values and logging errors, and potentially reverting changes if the operation cannot complete successfully.
| if (IsIconic(hwnd_)) { | ||
| apply_pseudo_minimize(); | ||
| } |
There was a problem hiding this comment.
Race condition: ensure_not_minimized() is called from the screencap thread without synchronization, while apply_pseudo_minimize() and revert_pseudo_minimize() in the monitor thread both read and write window state and the pseudo_minimized_ flag. Multiple concurrent calls to apply_pseudo_minimize() or simultaneous calls to apply and revert could lead to corrupted window state or lost original attributes. Consider adding a mutex to protect the critical sections in apply_pseudo_minimize(), revert_pseudo_minimize(), and ensure_not_minimized().
| { | ||
| helper_.start(); | ||
| } |
There was a problem hiding this comment.
Thread started in constructor: Starting a background thread in the constructor (line 17) is generally problematic because: (1) if the constructor throws after starting the thread, cleanup becomes complex, (2) the thread begins executing while the object is still being constructed, potentially accessing uninitialized members, and (3) it makes the constructor have side effects. Consider moving helper_.start() to an explicit init() method or deferring thread creation until first use in ensure_not_minimized().
| { | ||
| helper_.start(); | ||
| } |
There was a problem hiding this comment.
Same issue: Thread started in constructor. See comment on FramePoolWithPseudoMinimizeScreencap for details.
| if (IsIconic(hwnd_)) { | ||
| apply_pseudo_minimize(); | ||
| } |
There was a problem hiding this comment.
Missing synchronization after window restoration: ensure_not_minimized() calls apply_pseudo_minimize() which does ShowWindow(hwnd_, SW_SHOWNOACTIVATE), but there's no wait or verification that the window has actually been restored and is ready for screencap. Window restoration is asynchronous - the window might not be fully ready when this function returns, potentially causing screencap to fail. Consider adding a brief wait loop to verify the window is no longer iconic, or checking the screencap result and retrying.
| if (IsIconic(hwnd_)) { | |
| apply_pseudo_minimize(); | |
| } | |
| if (!IsIconic(hwnd_)) { | |
| return; | |
| } | |
| apply_pseudo_minimize(); | |
| // ShowWindow/伪最小化恢复是异步的,这里做一个短暂的等待,确保窗口已不再是最小化状态 | |
| constexpr int kMaxWaitMs = 500; | |
| constexpr int kStepMs = 10; | |
| auto start = std::chrono::steady_clock::now(); | |
| while (IsIconic(hwnd_)) { | |
| if (!IsWindow(hwnd_)) { | |
| break; | |
| } | |
| std::this_thread::sleep_for(std::chrono::milliseconds(kStepMs)); | |
| auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>( | |
| std::chrono::steady_clock::now() - start); | |
| if (elapsed.count() >= kMaxWaitMs) { | |
| LogWarn << "Window still iconic after pseudo-minimize wait" | |
| << VAR_VOIDP(hwnd_); | |
| break; | |
| } | |
| } |
| // FramePool 和 PrintWindow 内置伪最小化支持,允许最小化窗口 | ||
| bool supports_minimized = screencap_method_ == MaaWin32ScreencapMethod_FramePool | ||
| || screencap_method_ == MaaWin32ScreencapMethod_PrintWindow; | ||
| if (!supports_minimized && IsIconic(hwnd_)) { | ||
| LogError << "hwnd_ is minimized"; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Logic inconsistency with pseudo-minimize implementation: The comment says "FramePool 和 PrintWindow 内置伪最小化支持,允许最小化窗口" and the code checks these two methods. However, this check at connection time is insufficient because: (1) users might minimize the window after connection, (2) the check allows connection with a minimized window but doesn't actually trigger pseudo-minimize restoration - that only happens on first screencap call via ensure_not_minimized(). Consider either removing this check entirely since pseudo-minimize handles it, or immediately applying pseudo-minimize if the window is iconic.
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈:
- 在
Win32ControlUnitMgr::connect中,FramePool和PrintWindow方法现在总是映射到 pseudo-minimize 变体上,这会导致新添加的*_WithPseudoMinimize枚举位实际上没有被使用;建议要么把这些新的枚举值接到 pseudo-minimize 包装函数上,要么保留可选择的非 pseudo-minimize 变体,以避免让现有调用方感到意外。 - 在
PseudoMinimizeHelper::start/apply_pseudo_minimize/revert_pseudo_minimize中,Win32 API(GetWindowLongPtr、SetWindowLongPtr、SetLayeredWindowAttributes、ShowWindow)被假定总是会调用成功;为了更安全起见,建议在失败时进行检查并记录日志,这样在问题窗口上出现 pseudo-minimize 相关问题时,会更容易诊断。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- In `Win32ControlUnitMgr::connect`, the `FramePool` and `PrintWindow` methods are now always mapped to the pseudo-minimize variants, which makes the newly added `*_WithPseudoMinimize` enum bits effectively unused; consider either wiring the new enum values to the pseudo-minimize wrappers or keeping non-pseudo-minimize variants selectable to avoid surprising existing callers.
- In `PseudoMinimizeHelper::start`/`apply_pseudo_minimize`/`revert_pseudo_minimize`, the Win32 APIs (`GetWindowLongPtr`, `SetWindowLongPtr`, `SetLayeredWindowAttributes`, `ShowWindow`) are assumed to succeed; it would be safer to check and log failures so that pseudo-minimize issues can be diagnosed more easily on problematic windows.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've left some high level feedback:
- In
Win32ControlUnitMgr::connect, theFramePoolandPrintWindowmethods are now always mapped to the pseudo-minimize variants, which makes the newly added*_WithPseudoMinimizeenum bits effectively unused; consider either wiring the new enum values to the pseudo-minimize wrappers or keeping non-pseudo-minimize variants selectable to avoid surprising existing callers. - In
PseudoMinimizeHelper::start/apply_pseudo_minimize/revert_pseudo_minimize, the Win32 APIs (GetWindowLongPtr,SetWindowLongPtr,SetLayeredWindowAttributes,ShowWindow) are assumed to succeed; it would be safer to check and log failures so that pseudo-minimize issues can be diagnosed more easily on problematic windows.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Win32ControlUnitMgr::connect`, the `FramePool` and `PrintWindow` methods are now always mapped to the pseudo-minimize variants, which makes the newly added `*_WithPseudoMinimize` enum bits effectively unused; consider either wiring the new enum values to the pseudo-minimize wrappers or keeping non-pseudo-minimize variants selectable to avoid surprising existing callers.
- In `PseudoMinimizeHelper::start`/`apply_pseudo_minimize`/`revert_pseudo_minimize`, the Win32 APIs (`GetWindowLongPtr`, `SetWindowLongPtr`, `SetLayeredWindowAttributes`, `ShowWindow`) are assumed to succeed; it would be safer to check and log failures so that pseudo-minimize issues can be diagnosed more easily on problematic windows.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…截图方式 (#1153) Co-authored-by: Z_06 <z060606060606@gmail.com>
概述
为 Win32 截图器添加两个新的截图控制器:
FramePoolWithPseudoMinimize和PrintWindowWithPseudoMinimize。这两个控制器允许用户最小化目标窗口的同时仍能正常截图。当目标窗口被最小化时,通过将窗口设为透明、开启点击穿透、以不激活方式还原来实现伪最小化。
变更
MaaWin32ScreencapMethod_FramePoolWithPseudoMinimize(bit 6) 和MaaWin32ScreencapMethod_PrintWindowWithPseudoMinimize(bit 7)PseudoMinimizeHelper辅助类及两个截图控制器包装类Win32ControlUnitMgr中注册新控制器,PseudoMinimize 方式跳过 IsIconic 检查MaaWin32ScreencapMethodEnum新增两个值工作原理
WS_EX_LAYERED | WS_EX_TRANSPARENT,透明度设为 0,SW_SHOWNOACTIVATE还原Summary by Sourcery
为 Win32 的 FramePool 和 PrintWindow 截屏方法添加伪最小化(pseudo-minimize)支持,使最小化的窗口也能在不打扰用户的情况下被捕获。
新功能:
PseudoMinimizeHelper工具类,以及针对 FramePool 和 PrintWindow 的截屏封装类,通过透明且可穿透点击的伪最小化状态,使最小化的窗口保持可被捕获。改进:
Win32ControlUnitMgr,使其使用支持伪最小化的 FramePool 和 PrintWindow 实现,并在使用这些方法时跳过对窗口最小化状态的检查。文档:
Original summary in English
Summary by Sourcery
Add pseudo-minimize support to Win32 FramePool and PrintWindow screencap methods so minimized windows can still be captured without disturbing the user.
New Features:
Enhancements:
Documentation:
新功能:
PseudoMinimizeHelper工具类,以及针对 FramePool 和 PrintWindow 的截屏包装类,通过基于透明度的伪最小化机制,使已最小化窗口保持可被捕获。增强内容:
文档:
Original summary in English
Summary by Sourcery
为 Win32 的 FramePool 和 PrintWindow 截屏方法添加伪最小化(pseudo-minimize)支持,使最小化的窗口也能在不打扰用户的情况下被捕获。
新功能:
PseudoMinimizeHelper工具类,以及针对 FramePool 和 PrintWindow 的截屏封装类,通过透明且可穿透点击的伪最小化状态,使最小化的窗口保持可被捕获。改进:
Win32ControlUnitMgr,使其使用支持伪最小化的 FramePool 和 PrintWindow 实现,并在使用这些方法时跳过对窗口最小化状态的检查。文档:
Original summary in English
Summary by Sourcery
Add pseudo-minimize support to Win32 FramePool and PrintWindow screencap methods so minimized windows can still be captured without disturbing the user.
New Features:
Enhancements:
Documentation:
新功能:
FramePoolWithPseudoMinimize和PrintWindowWithPseudoMinimize两种 Win32 屏幕捕获方法,以通过伪最小化来支持捕获已最小化窗口。增强:
Win32ControlUnitMgr中注册新的伪最小化屏幕捕获控制器,并在使用这些控制器时跳过对已最小化窗口的检查。文档:
Original summary in English
Summary by Sourcery
为 Win32 的 FramePool 和 PrintWindow 截屏方法添加伪最小化(pseudo-minimize)支持,使最小化的窗口也能在不打扰用户的情况下被捕获。
新功能:
PseudoMinimizeHelper工具类,以及针对 FramePool 和 PrintWindow 的截屏封装类,通过透明且可穿透点击的伪最小化状态,使最小化的窗口保持可被捕获。改进:
Win32ControlUnitMgr,使其使用支持伪最小化的 FramePool 和 PrintWindow 实现,并在使用这些方法时跳过对窗口最小化状态的检查。文档:
Original summary in English
Summary by Sourcery
Add pseudo-minimize support to Win32 FramePool and PrintWindow screencap methods so minimized windows can still be captured without disturbing the user.
New Features:
Enhancements:
Documentation:
新功能:
PseudoMinimizeHelper工具类,以及针对 FramePool 和 PrintWindow 的截屏包装类,通过基于透明度的伪最小化机制,使已最小化窗口保持可被捕获。增强内容:
文档:
Original summary in English
Summary by Sourcery
为 Win32 的 FramePool 和 PrintWindow 截屏方法添加伪最小化(pseudo-minimize)支持,使最小化的窗口也能在不打扰用户的情况下被捕获。
新功能:
PseudoMinimizeHelper工具类,以及针对 FramePool 和 PrintWindow 的截屏封装类,通过透明且可穿透点击的伪最小化状态,使最小化的窗口保持可被捕获。改进:
Win32ControlUnitMgr,使其使用支持伪最小化的 FramePool 和 PrintWindow 实现,并在使用这些方法时跳过对窗口最小化状态的检查。文档:
Original summary in English
Summary by Sourcery
Add pseudo-minimize support to Win32 FramePool and PrintWindow screencap methods so minimized windows can still be captured without disturbing the user.
New Features:
Enhancements:
Documentation: