Improve Windows screenshot behavior for multi-monitor setups#281
Improve Windows screenshot behavior for multi-monitor setups#281AstroAir wants to merge 2 commits intoopenai:mainfrom
Conversation
…and Linux; add VirtualDesktop option and improve error handling
There was a problem hiding this comment.
Pull request overview
This PR updates the Windows PowerShell screenshot helper to behave better on multi-monitor setups by defaulting full-screen capture to one image per display (with an opt-in -VirtualDesktop stitched capture), and aligns the skill documentation with the new Windows behavior and troubleshooting guidance.
Changes:
- Windows PowerShell helper: multi-monitor full-screen now outputs one file per display by default; adds
-VirtualDesktopto force a stitched virtual-desktop capture. - Windows PowerShell helper: improves robustness around nullable window handles, DPI-awareness setup, and clearer validation/errors for invalid/minimized bounds.
- Skill docs: document Windows multi-display output behavior,
-VirtualDesktopusage, and troubleshooting tips.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| skills/.curated/screenshot/scripts/take_screenshot.ps1 | Implements per-display full-screen capture on Windows (default), -VirtualDesktop override, test-mode behavior, and capture hardening. |
| skills/.curated/screenshot/SKILL.md | Documents Windows multi-monitor behavior, -VirtualDesktop, output naming, and troubleshooting guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| } | ||
|
|
||
| $hasWindowHandle = $PSBoundParameters.ContainsKey("WindowHandle") | ||
| $hasExplicitPath = $PSBoundParameters.ContainsKey("Path") |
There was a problem hiding this comment.
Fixed in 067ffc8.
I aligned the “explicit path” check with Resolve-OutputPath so only non-empty, non-whitespace -Path values are
treated as explicit. That means -Path "" now follows the implicit-path flow and produces suffixed multi-display outputs
(-d1, -d2, etc.) instead of preserving the unsuffixed first path.
I did not broaden this to silently rewrite whitespace-only paths into defaults; those still fail as invalid input rather
than changing a malformed user-provided path behind the caller’s back. I also added a regression check for the empty-
string case.
| [NativeMethods]::SetProcessDpiAwarenessContext([IntPtr]::new(-4)) | Out-Null | ||
| } catch { | ||
| try { | ||
| [NativeMethods]::SetProcessDPIAware() | Out-Null | ||
| } catch { | ||
| # Best effort only; continue when APIs are unavailable. |
There was a problem hiding this comment.
Fixed in 067ffc8.
The DPI initialization now falls back to SetProcessDPIAware() whenever SetProcessDpiAwarenessContext(...) either
throws or returns false, so the legacy path still runs on hosts where PMv2 is unsupported but the API exists.
I added regression coverage for the false return path to make sure we keep the fallback behavior.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 003a53b7bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [NativeMethods]::SetProcessDpiAwarenessContext([IntPtr]::new(-4)) | Out-Null | ||
| } catch { | ||
| try { | ||
| [NativeMethods]::SetProcessDPIAware() | Out-Null |
There was a problem hiding this comment.
SetProcessDpiAwarenessContext returns false
On Windows 10 1607 / Server 2016, DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2 (-4) is not supported even though SetProcessDpiAwarenessContext exists, so this call fails by returning false rather than throwing. Because the code only enters the fallback path on exceptions, those hosts stay DPI-unaware and the new per-display capture can still read virtualized bounds, leaving mixed-DPI screenshots offset instead of applying the older SetProcessDPIAware() fallback.
Useful? React with 👍 / 👎.
- treat empty or whitespace -Path values as implicit output paths for multi-display suffixing - fall back to SetProcessDPIAware when per-monitor v2 DPI setup returns false - add PowerShell regression coverage for explicit path preservation and DPI fallback
Summary
-VirtualDesktopto opt into a single stitched image when needed-Pathfor the first display while writing additional displays as suffixed sibling files such as-d2-VirtualDesktopusage, and troubleshooting guidanceTest Plan
CODEX_SCREENSHOT_TEST_MODE=1CODEX_SCREENSHOT_TEST_DISPLAYS=1,2powershell.exe -NoProfile -ExecutionPolicy Bypass -File skills/.curated/screenshot/scripts/take_screenshot.ps1 -Path "$env:TEMP\\codex-pr-smoke.png"C:\Users\qwdma\AppData\Local\Temp\codex-pr-smoke.pngC:\Users\qwdma\AppData\Local\Temp\codex-pr-smoke-d2.pngpython skills/.system/skill-creator/scripts/quick_validate.py skills/.curated/screenshot, butpythonis not installed on this machine, so that validation could not be completed here