feat(screenshot): briefly highlight captured area with orange-red flash#232
Open
exalsch wants to merge 9 commits into
Open
feat(screenshot): briefly highlight captured area with orange-red flash#232exalsch wants to merge 9 commits into
exalsch wants to merge 9 commits into
Conversation
After every screenshot, draw a glowing orange-red border over the captured area on a transparent always-on-top Tk overlay for ~2.5s. Region captures get a solid border that fades out at the end; full- screen captures show a bell-fading inner border on the union of all monitor rects. The overlay is started *after* capture and cancelled before any subsequent capture so it never appears in the captured image. Set WINDOWS_MCP_DISABLE_FLASH=1/true/yes/on to suppress the effect. The conftest disables the flash for the test suite to avoid Tk threads racing with pytest teardown.
Review Summary by QodoAdd orange-red flash overlay for screenshot visual confirmation
WalkthroughsDescription• Add visual feedback flash overlay after screenshot capture - Orange-red glowing border highlights captured area for ~2.5s - Region captures show solid border that fades out; full-screen captures show bell-curve fade • Overlay rendered on transparent always-on-top Tk window on daemon thread - Started after capture so never appears in captured image - Cancelled before next capture to prevent bleed-through • Disable with WINDOWS_MCP_DISABLE_FLASH=1 environment variable • Comprehensive test coverage with 18 unit tests for flash overlay module Diagramflowchart LR
A["Screenshot/Snapshot<br/>Capture"] --> B["cancel_active_flash()"]
B --> C["capture_image()"]
C --> D["show_capture_flash()"]
D --> E["Daemon Thread:<br/>Tk Overlay"]
E --> F["Fade Animation<br/>~2.5s"]
F --> G["Destroy Overlay"]
File Changes1. src/windows_mcp/desktop/flash_overlay.py
|
Code Review by Qodo
1. Tests lack type hints
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def _reset_active_overlay(): |
There was a problem hiding this comment.
1. Tests lack type hints 📘 Rule violation ⚙ Maintainability
New test functions/fixtures are added without parameter and return type hints. This violates the requirement that all new or modified function signatures include type hints, reducing maintainability and static analyzability.
Agent Prompt
## Issue description
Newly added test functions and fixtures do not include type hints for parameters and return types.
## Issue Context
Compliance requires type hints on all new/modified function signatures, including in test code.
## Fix Focus Areas
- tests/test_flash_overlay.py[18-117]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+37
to
+66
| def cancel_active_flash(timeout: float = 0.25) -> None: | ||
| """Tear down any flash overlay currently on screen. | ||
|
|
||
| Call this immediately before taking a screenshot so the previous flash | ||
| can never bleed into the new capture. | ||
| """ | ||
| global _active_overlay | ||
| with _lock: | ||
| ov = _active_overlay | ||
| _active_overlay = None | ||
| if ov is None: | ||
| return | ||
| ov.stop_event.set() | ||
| ov.closed_event.wait(timeout=timeout) | ||
|
|
||
|
|
||
| def show_capture_flash( | ||
| rects: list[tuple[int, int, int, int]], | ||
| *, | ||
| full_screen: bool, | ||
| ) -> None: | ||
| """Show a fade-in/out orange-red border over each rect. | ||
|
|
||
| ``rects`` are ``(left, top, right, bottom)`` tuples in virtual-screen | ||
| coordinates. ``full_screen=True`` draws an inner border that fades in | ||
| then out (used when no region was specified). ``full_screen=False`` keeps | ||
| the border solid for most of the duration and fades out at the end. | ||
|
|
||
| Returns immediately; rendering happens on a daemon thread. | ||
| """ |
There was a problem hiding this comment.
2. Flash api docstrings non-google 📘 Rule violation ⚙ Maintainability
Public functions cancel_active_flash and show_capture_flash have docstrings that are not in Google-style format (missing Args:/Returns: sections). This violates the docstring standard for public APIs and makes generated docs inconsistent.
Agent Prompt
## Issue description
Public APIs in `flash_overlay.py` use non-Google-style docstrings.
## Issue Context
Compliance requires Google-style docstrings for public functions/classes to keep documentation consistent.
## Fix Focus Areas
- src/windows_mcp/desktop/flash_overlay.py[37-66]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
7 tasks
Replace the single transparent-canvas overlay with stacked thin Toplevel "strip" windows along each side of the rect (top, bottom, left, right). Each strip is a solid orange-red Toplevel with its own -alpha, so per-layer fade is reliable — the previous -alpha + -transparentcolor combination renders inconsistently on Windows. Eight layers per side with quadratic alpha falloff: layer 0 sits on the rect edge at full opacity, outer layers radiate further out (or inward, for full-screen) with progressively lower alpha to produce a soft halo. Time-based modulation still gives a quick fade-in, hold, and slow fade-out for region captures, and a bell-curve fade for full-screen. Tests: tests/test_flash_overlay.py adds TestBuildStripDefs covering strip placement (region outward, full-screen inward), alpha falloff, and the no-room edge case.
The previous strip-Toplevels rewrite hung Tk's mainloop when launched on a non-main thread (creating ~32 Toplevels with overrideredirect froze the loop), so the flash never appeared even though the thread was alive. Switch back to a single transparent Tk window with a Canvas, but produce the halo by drawing concentric border rectangles whose RGB fades from full orange-red toward pure black. The canvas's transparent-colour key is set to pure black, so dim outer layers genuinely become transparent — no -alpha + -transparentcolor combo (which Windows renders unreliably) is needed. The time fade is done by re-rendering each frame with a scaled intensity, so the glow fades in/out smoothly. 12 layers, ~4 px outer halo for region captures, inner glow inset by 4 px from the monitor edge for full-screen captures. Tests: replace TestBuildStripDefs with TestLayerColor covering full intensity, half-intensity blend, and the zero-intensity safeguard (never returns pure black, which would punch through the transparent key).
Two prior rewrites failed in different ways on the user's machine: 1. The single-window transparent canvas approach (-transparentcolor + per-frame redraw) rendered nothing — some Windows configurations refuse to map the layered window when -transparentcolor is set. 2. The multi-layer Toplevel-strip approach hung Tk's mainloop when ~8 or more Toplevels were created back-to-back on a non-main thread (overrideredirect + alpha + topmost). Reduce the halo to a single ring of 4 thick (8 px) opaque Toplevel strips per rect — the only configuration confirmed to actually render and complete cleanly. No transparentcolor, no glow gradient — just a solid orange-red border that fades in (~15% of duration), holds, and fades out (~35%). Drop the test that asserted falling alpha across layers since there is now only one layer; the remaining strip-placement tests still exercise the geometry. Logs the strip count at INFO level so it's possible to verify the overlay actually fired from server logs.
Drop Tk entirely — neither -transparentcolor (rendered nothing on some Windows configs) nor multi-Toplevel strips (hangs Tk's mainloop on a non-main thread once ~6+ Toplevels are created back-to-back) delivered a usable result. The new implementation creates a Win32 layered window (WS_EX_LAYERED | WS_EX_TRANSPARENT | WS_EX_TOPMOST | WS_EX_TOOLWINDOW | WS_EX_NOACTIVATE) and feeds UpdateLayeredWindow a 32-bit BGRA top-down DIB section with premultiplied alpha. The halo itself is rendered with PIL: solid orange-red ring plus a Gaussian-blurred copy composited underneath, giving a real soft glow that fades from the rect edge outward. Time fade is a linear scale of the alpha channel re-pushed each frame (~30 ms cadence), quantised to 32 levels to skip redundant uploads. ctypes argtypes/restypes are set explicitly on every Win32 entry point used because the pointer-sized handle/LRESULT defaults overflow on x64. Tests: replace TestBuildStripDefs with TestIntensityCurve (verifies the bell-curve / hold-then-fade envelopes) and TestPremultipliedBgra (verifies BGRA byte order, premultiplication, and alpha scaling for the per-frame intensity multiplier).
Collaborator
|
Looks good. Do you have a screenshot or a short screen recording/GIF to demonstrate how the orange-red flash looks in practice? |
JezaChen
reviewed
May 9, 2026
| flash_overlay.cancel_active_flash() | ||
| image, used_backend = screenshot_capture.capture(capture_rect) | ||
| self._last_screenshot_backend = used_backend | ||
| try: |
Collaborator
There was a problem hiding this comment.
Since desktop/service.py is already quite large, we want to keep it as clean as possible. Please consider moving the code added here into the show_capture_flash function.
Member
|
Could you please share a video demonstration of the working |
The orange ring was nested inward inside the captured rect, so for window- targeted screenshots (especially borderless Tauri / WebView2 apps where the DWM extended frame matches the content edge) the halo visibly overlapped the window content instead of reading as a surround. Switch the ring direction to outward by default — it now grows into the existing 42px margin around the rect — and keep the inward nesting only for the full-screen inner halo via outward=False. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lity User feedback: the glow was easy to miss for window/region captures — the 4px band against a 14px blur radius wasn't bright enough to register when the bulk of the halo sits outside the captured rect on desktop background. Double the sharp-ring thickness to 8px and bump the total duration to 3.5s so the user has time to spot it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two review asks on PR CursorTouch#232: JezaChen — desktop/service.py is already large; move the flash setup code into show_capture_flash. Done: show_capture_flash now takes a single uia.Rect (or None for full-desktop) and resolves rects / enumerates monitors internally. get_screenshot collapses to a one-liner. Qodo — show_capture_flash overwrote _active_overlay without signalling the prior one, so a follow-up call would orphan the running overlay and cancel_active_flash() could no longer reach it. Fixed with an atomic swap: save prev under _lock, install new, then signal prev outside the lock so the prior overlay tears down without blocking the new caller. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rder The two new full-screen tests monkeypatched ``sys.modules['windows_mcp.uia']`` but that is bypassed once another test in the suite imports the real uia — ``import windows_mcp.uia as uia`` then resolves via the cached parent-package attribute rather than sys.modules. Patch ``GetMonitorsRect`` on the real module instead so the override holds regardless of import order. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
There you go :-) |
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.
Summary
After every
Screenshot/Snapshot(use_vision=True)capture, briefly draw a glowing orange-red border over the captured area as a visual confirmation. Useful for end-users supervising agent activity — you can see at a glance which area was just sent to the LLM.#FF4500. Border: 6 px (concentric rectangles for soft glow).tkinterwindow on a daemon thread, started after capture, so it never appears in the captured image. Any active overlay is cancelled before the next capture so it never bleeds into a follow-up screenshot.WINDOWS_MCP_DISABLE_FLASH=1(also acceptstrue/yes/on).Why
Right now there is no visual indication that a screenshot just happened — the agent silently captures the screen and the user only finds out via tool logs. A subtle, time-bounded glow makes the action observable without being intrusive.
Implementation notes
src/windows_mcp/desktop/flash_overlay.pywithshow_capture_flash(rects, full_screen)andcancel_active_flash(). Module-level lock and_active_overlaytrack the currently visible overlay so cancellation is synchronous (uses anEventthe Tk loop polls each frame).Desktop.get_screenshot(the single funnel for bothScreenshotandSnapshot(use_vision=True)) callscancel_active_flash()before capture andshow_capture_flash(...)after. For region captures, the rect is the existingcapture_rect. For full-screen,uia.GetMonitorsRect()is used so each monitor gets its own inner border.tkinteris used for the overlay window. If the import fails (e.g., headless build), the module logs and silently skips.tests/conftest.pysetsWINDOWS_MCP_DISABLE_FLASH=1so the unit suite never starts a Tk thread (which would race with pytest teardown). The flash-overlay tests themselves manage the env var viamonkeypatch.Tests
tests/test_flash_overlay.py— 18 tests covering env-var gating, dispatch behaviour (no-op when disabled / empty rects), overlay registration/cancellation lifecycle, and graceful fallthrough whentkinterimport fails.mainand unrelated —ScrollElementNode.to_row/TreeElementNode.to_rownot yet implemented).Test plan
Screenshot/Snapshot(use_vision=True)from the MCP client and observe the orange-red border on the captured areaWINDOWS_MCP_DISABLE_FLASH=1and confirm no flash appearspytest tests/test_flash_overlay.pypasses (18/18)🤖 Generated with Claude Code