From 64566b1a092b95793480dc050765dc0aef571eb2 Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Fri, 8 May 2026 22:08:20 +0200 Subject: [PATCH 01/13] feat(screenshot): briefly highlight captured area with orange-red flash 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. --- CLAUDE.md | 1 + README.md | 3 +- src/windows_mcp/desktop/flash_overlay.py | 189 +++++++++++++++++++++++ src/windows_mcp/desktop/service.py | 68 +++++--- tests/conftest.py | 8 + tests/test_flash_overlay.py | 117 ++++++++++++++ 6 files changed, 366 insertions(+), 20 deletions(-) create mode 100644 src/windows_mcp/desktop/flash_overlay.py create mode 100644 tests/test_flash_overlay.py diff --git a/CLAUDE.md b/CLAUDE.md index 10cf4851..07c52be6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,6 +64,7 @@ The codebase follows a layered service architecture under `src/windows_mcp/`: | `WINDOWS_MCP_PROFILE_SNAPSHOT` | _(off)_ | Set to `1`/`true`/`yes`/`on` to log per-stage timing for Screenshot/Snapshot. Checked in `tools/_snapshot_helpers.py` and `desktop/service.py`. | | `ANONYMIZED_TELEMETRY` | `true` | Set to `false` to disable PostHog telemetry. Checked in `__main__.py` and `analytics.py`. | | `WINDOWS_MCP_DEBUG` | `false` | Set to `1`/`true`/`yes`/`on` to enable debug mode. Checked in `config.py`. Also available as `--debug` CLI flag. | +| `WINDOWS_MCP_DISABLE_FLASH` | _(off)_ | Set to `1`/`true`/`yes`/`on` to suppress the orange-red glowing border that briefly appears after every screenshot. Resolved in `desktop/flash_overlay.py`. | ## Security Context diff --git a/README.md b/README.md index 599c4372..0eba34ba 100755 --- a/README.md +++ b/README.md @@ -446,6 +446,7 @@ All variables are optional unless noted. Set them via the `env` key in `claude_d | `WINDOWS_MCP_SCREENSHOT_SCALE` | `1.0` | Scale factor applied to screenshots before encoding. Accepts a float in the range `0.1`–`1.0`. Useful on high-resolution displays (1440p, 4K) where the default produces images that exceed Claude Desktop's 1 MB tool-result limit. Set to `0.5` to halve both dimensions (quarter the file size). | | `WINDOWS_MCP_SCREENSHOT_BACKEND` | `auto` | Screenshot capture backend. Accepted values: `auto` (tries dxcam → mss → pillow in order), `dxcam`, `mss`, `pillow`. Use `mss` or `pillow` if `dxcam` is unavailable or causes issues on your GPU. | | `WINDOWS_MCP_PROFILE_SNAPSHOT` | _(disabled)_ | Set to `1`, `true`, `yes`, or `on` to emit per-stage timing logs for Screenshot/Snapshot calls. Useful for diagnosing slow captures. | +| `WINDOWS_MCP_DISABLE_FLASH` | _(disabled)_ | Set to `1`, `true`, `yes`, or `on` to suppress the orange-red glowing border that briefly highlights the captured area after every screenshot. The flash is rendered on a transparent always-on-top window *after* capture so it never appears in the captured image. | ### Telemetry @@ -493,7 +494,7 @@ MCP Client can access the following tools to interact with Windows: - `Move`: Move mouse pointer or drag (set drag=True) to coordinates. - `Shortcut`: Press keyboard shortcuts (`Ctrl+c`, `Alt+Tab`, etc). - `Wait`: Pause for a defined duration. -- `Screenshot`: Fast screenshot-first desktop capture with cursor position, active/open windows, and an image. Skips UI tree extraction for speed and should be the default first call when you mainly need visual context. Supports `display=[0]` or `display=[0,1]` to capture specific screens. +- `Screenshot`: Fast screenshot-first desktop capture with cursor position, active/open windows, and an image. Skips UI tree extraction for speed and should be the default first call when you mainly need visual context. Supports `display=[0]` or `display=[0,1]` to capture specific screens. After capture, a brief orange-red glowing border is drawn over the captured area as a visual confirmation (set `WINDOWS_MCP_DISABLE_FLASH=1` to disable). - `Snapshot`: Full desktop state capture for workflows that need interactive element ids, scrollable regions, or `use_dom=True` browser extraction. Supports `use_vision=True` for including screenshots and `display=[0]` or `display=[0,1]` for limiting all returned Snapshot information to specific screens. - `App`: To launch an application from the start menu, resize or move the window and switch between apps. - `Shell`: To execute PowerShell commands. diff --git a/src/windows_mcp/desktop/flash_overlay.py b/src/windows_mcp/desktop/flash_overlay.py new file mode 100644 index 00000000..1ea8eaf0 --- /dev/null +++ b/src/windows_mcp/desktop/flash_overlay.py @@ -0,0 +1,189 @@ +"""Brief on-screen visual confirmation that a screenshot was taken. + +Draws a glowing orange-red border over the captured area for ~1 second using +a transparent always-on-top Tk window on a daemon thread. The flash is started +*after* the screenshot is captured so it never appears in the captured image. +""" + +import logging +import os +import threading +import time + +logger = logging.getLogger(__name__) + +_FLASH_COLOR = "#FF4500" +_DURATION_MS = 2500 +_FRAME_INTERVAL_MS = 20 +_BORDER_THICKNESS = 6 +_FULLSCREEN_INSET = 12 + +_lock = threading.Lock() +_active_overlay: "_Overlay | None" = None + + +def _flash_disabled() -> bool: + value = os.getenv("WINDOWS_MCP_DISABLE_FLASH", "") + return value.strip().lower() in {"1", "true", "yes", "on"} + + +class _Overlay: + def __init__(self) -> None: + self.stop_event = threading.Event() + self.closed_event = threading.Event() + self.thread: threading.Thread | None = None + + +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. + """ + if _flash_disabled() or not rects: + return + rects = [tuple(r) for r in rects] + overlay = _Overlay() + overlay.thread = threading.Thread( + target=_run_overlay, + args=(rects, full_screen, overlay), + name="windows-mcp-flash", + daemon=True, + ) + with _lock: + global _active_overlay + _active_overlay = overlay + overlay.thread.start() + + +def _run_overlay( + rects: list[tuple[int, int, int, int]], + full_screen: bool, + overlay: _Overlay, +) -> None: + try: + import tkinter as tk + except Exception: + logger.debug("tkinter unavailable; skipping screenshot flash") + overlay.closed_event.set() + return + + root: "tk.Tk | None" = None + try: + left = min(r[0] for r in rects) + top = min(r[1] for r in rects) + right = max(r[2] for r in rects) + bottom = max(r[3] for r in rects) + width = right - left + height = bottom - top + if width <= 0 or height <= 0: + return + + root = tk.Tk() + root.withdraw() + root.overrideredirect(True) + root.attributes("-topmost", True) + try: + root.attributes("-disabled", True) + except tk.TclError: + pass + + transparent_color = "#010203" + try: + root.configure(bg=transparent_color) + root.attributes("-transparentcolor", transparent_color) + canvas_bg = transparent_color + except tk.TclError: + canvas_bg = root.cget("bg") + + root.geometry(f"{width}x{height}+{left}+{top}") + + canvas = tk.Canvas( + root, + width=width, + height=height, + bg=canvas_bg, + highlightthickness=0, + borderwidth=0, + ) + canvas.pack(fill="both", expand=True) + + inset = _FULLSCREEN_INSET if full_screen else 0 + for r_left, r_top, r_right, r_bottom in rects: + x1 = r_left - left + inset + y1 = r_top - top + inset + x2 = r_right - left - inset - 1 + y2 = r_bottom - top - inset - 1 + if x2 - x1 <= 0 or y2 - y1 <= 0: + continue + for i in range(_BORDER_THICKNESS): + canvas.create_rectangle( + x1 + i, + y1 + i, + x2 - i, + y2 - i, + outline=_FLASH_COLOR, + width=1, + ) + + root.deiconify() + start = time.perf_counter() + + def tick() -> None: + if overlay.stop_event.is_set(): + root.destroy() + return + elapsed_ms = (time.perf_counter() - start) * 1000 + if elapsed_ms >= _DURATION_MS: + root.destroy() + return + t = elapsed_ms / _DURATION_MS + if full_screen: + alpha = 1.0 - abs(2 * t - 1) + else: + alpha = 1.0 if t < 0.65 else max(0.0, 1.0 - (t - 0.65) / 0.35) + try: + root.attributes("-alpha", max(0.0, min(1.0, alpha))) + except tk.TclError: + pass + root.after(_FRAME_INTERVAL_MS, tick) + + root.after(0, tick) + root.mainloop() + except Exception: + logger.debug("screenshot flash overlay failed", exc_info=True) + if root is not None: + try: + root.destroy() + except Exception: + pass + finally: + with _lock: + global _active_overlay + if _active_overlay is overlay: + _active_overlay = None + overlay.closed_event.set() diff --git a/src/windows_mcp/desktop/service.py b/src/windows_mcp/desktop/service.py index 8514bd60..d6781ef2 100755 --- a/src/windows_mcp/desktop/service.py +++ b/src/windows_mcp/desktop/service.py @@ -15,6 +15,7 @@ from PIL import ImageFont, ImageDraw, Image from windows_mcp.tree.service import Tree from windows_mcp.desktop import screenshot as screenshot_capture +from windows_mcp.desktop import flash_overlay from locale import getpreferredencoding from contextlib import contextmanager from typing import Literal @@ -253,7 +254,9 @@ def get_state( screenshot_region=screenshot_region, screenshot_displays=display_indices, tree_state=tree_state, - screenshot_backend=getattr(self, "_last_screenshot_backend", None) if use_vision else None, + screenshot_backend=getattr(self, "_last_screenshot_backend", None) + if use_vision + else None, capture_sec=time() - start_time, ) if profile_enabled: @@ -341,7 +344,9 @@ def _get_apps_from_shortcuts(self) -> dict[str, str]: apps[name] = lnk_path return apps - def execute_command(self, command: str, timeout: int = 10, shell: str | None = None) -> tuple[str, int]: + def execute_command( + self, command: str, timeout: int = 10, shell: str | None = None + ) -> tuple[str, int]: return PowerShellExecutor.execute_command(command, timeout, shell) def is_window_browser(self, node: uia.Control): @@ -358,7 +363,9 @@ def get_default_language(self) -> str: reader = csv.DictReader(io.StringIO(response)) return "".join([row.get("DisplayName") for row in reader]) - def _find_window_by_name(self, name: str, refresh_state: bool = False) -> tuple["Window | None", str]: + def _find_window_by_name( + self, name: str, refresh_state: bool = False + ) -> tuple["Window | None", str]: """Find a window by fuzzy name match. Returns (window, error_msg). If the returned window is None, error_msg describes the failure reason. @@ -632,7 +639,7 @@ def get_coordinates_from_labels(self, labels: list[int]) -> list[tuple[int, int] results.append((element_node.center.x, element_node.center.y)) return results - def click(self, loc: tuple[int, int]|list[int], button: str = "left", clicks: int = 2): + def click(self, loc: tuple[int, int] | list[int], button: str = "left", clicks: int = 2): if isinstance(loc, list): x, y = loc[0], loc[1] else: @@ -714,7 +721,7 @@ def scroll( return 'Invalid type. Use "horizontal" or "vertical".' return None - def drag(self, loc: tuple[int, int]|list[int]): + def drag(self, loc: tuple[int, int] | list[int]): if isinstance(loc, list): x, y = loc[0], loc[1] else: @@ -956,10 +963,10 @@ def get_xpath_from_element(self, element: uia.Control): xpath = "/".join(path_parts) return xpath - - def get_windows_version(self) -> str: - response, status = PowerShellExecutor.execute_command("(Get-CimInstance Win32_OperatingSystem).Caption") + response, status = PowerShellExecutor.execute_command( + "(Get-CimInstance Win32_OperatingSystem).Caption" + ) if status == 0: return response.strip() return "Windows" @@ -997,14 +1004,18 @@ def parse_display_selection( return None if isinstance(display, bool): - raise ValueError("display must be a JSON array of non-negative integers, for example [0] or [0,1]") + raise ValueError( + "display must be a JSON array of non-negative integers, for example [0] or [0,1]" + ) if isinstance(display, int): values = [display] elif isinstance(display, (list, tuple)): values = list(display) else: - raise ValueError("display must be a JSON array of non-negative integers, for example [0] or [0,1]") + raise ValueError( + "display must be a JSON array of non-negative integers, for example [0] or [0,1]" + ) unique_values: list[int] = [] for value in values: @@ -1017,7 +1028,9 @@ def parse_display_selection( def get_display_union_rect(self, display_indices: list[int]) -> uia.Rect: monitor_rects = uia.GetMonitorsRect() if not monitor_rects: - logger.warning("Monitor enumeration returned no monitors while display filter was requested") + logger.warning( + "Monitor enumeration returned no monitors while display filter was requested" + ) raise ValueError("No displays detected") invalid_indices = [index for index in display_indices if index >= len(monitor_rects)] @@ -1040,8 +1053,27 @@ def get_display_union_rect(self, display_indices: list[int]) -> uia.Rect: ) def get_screenshot(self, capture_rect: uia.Rect | None = None) -> Image.Image: + flash_overlay.cancel_active_flash() image, used_backend = screenshot_capture.capture(capture_rect) self._last_screenshot_backend = used_backend + try: + if capture_rect is not None: + rects = [ + ( + capture_rect.left, + capture_rect.top, + capture_rect.right, + capture_rect.bottom, + ) + ] + full_screen = False + else: + monitor_rects = uia.GetMonitorsRect() + rects = [(m.left, m.top, m.right, m.bottom) for m in monitor_rects] + full_screen = True + flash_overlay.show_capture_flash(rects, full_screen=full_screen) + except Exception: + logger.debug("could not start screenshot flash overlay", exc_info=True) return image def get_annotated_screenshot( @@ -1151,7 +1183,9 @@ def draw_annotation(label, node: TreeElementNode): # Draw "Cursor" label c_label = "CURSOR" c_label_width = draw.textlength(c_label, font=font) - draw.rectangle([acx + r, acy - r, acx + r + c_label_width + 4, acy - r + 16], fill="red") + draw.rectangle( + [acx + r, acy - r, acx + r + c_label_width + 4, acy - r + 16], fill="red" + ) draw.text((acx + r + 2, acy - r), c_label, fill="white", font=font) if capture_rect: @@ -1200,9 +1234,7 @@ def _clip_bounding_box_to_region( height=bottom - top, ) - def _filter_window_to_region( - self, window: Window | None, region: BoundingBox - ) -> Window | None: + def _filter_window_to_region(self, window: Window | None, region: BoundingBox) -> Window | None: if window is None: return None clipped_box = self._clip_bounding_box_to_region(window.bounding_box, region) @@ -1218,9 +1250,7 @@ def _filter_window_to_region( process_id=window.process_id, ) - def _filter_windows_to_region( - self, windows: list[Window], region: BoundingBox - ) -> list[Window]: + def _filter_windows_to_region(self, windows: list[Window], region: BoundingBox) -> list[Window]: filtered_windows: list[Window] = [] for window in windows: filtered_window = self._filter_window_to_region(window, region) @@ -1338,7 +1368,7 @@ def send_notification(self, title: str, message: str, app_id: str) -> str: if status == 0: return f'Notification sent: "{title}" - {message}' else: - return f'Notification may have been sent. PowerShell output: {response[:200]}' + return f"Notification may have been sent. PowerShell output: {response[:200]}" def list_processes( self, diff --git a/tests/conftest.py b/tests/conftest.py index a7008ebb..20d68522 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,13 @@ +import os + import pytest +# Disable the post-screenshot flash overlay during the test suite. The overlay +# spawns a Tk window on a daemon thread which races with pytest teardown and +# can crash the interpreter. Tests for the flash itself set/clear this env var +# explicitly via monkeypatch. +os.environ.setdefault("WINDOWS_MCP_DISABLE_FLASH", "1") + from windows_mcp.tree.views import BoundingBox, Center, TreeElementNode, ScrollElementNode from windows_mcp.desktop.views import Window, Status, DesktopState diff --git a/tests/test_flash_overlay.py b/tests/test_flash_overlay.py new file mode 100644 index 00000000..e8100e19 --- /dev/null +++ b/tests/test_flash_overlay.py @@ -0,0 +1,117 @@ +"""Unit tests for the screenshot flash overlay. + +The actual Tk window is never created in these tests — they exercise the +public dispatch surface (env-var gating, lifecycle bookkeeping, fallthrough +when ``tkinter`` cannot be imported). +""" + +import sys +import threading +from unittest.mock import patch + +import pytest + +import windows_mcp.desktop.flash_overlay as flash_overlay + + +@pytest.fixture(autouse=True) +def _reset_active_overlay(): + """Each test starts and ends with no overlay registered.""" + with flash_overlay._lock: + flash_overlay._active_overlay = None + yield + with flash_overlay._lock: + ov = flash_overlay._active_overlay + flash_overlay._active_overlay = None + if ov is not None: + ov.stop_event.set() + + +class TestFlashDisabled: + def test_default_is_enabled(self, monkeypatch): + monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) + assert flash_overlay._flash_disabled() is False + + @pytest.mark.parametrize("value", ["1", "true", "yes", "on", "TRUE", " On "]) + def test_truthy_values_disable(self, monkeypatch, value): + monkeypatch.setenv("WINDOWS_MCP_DISABLE_FLASH", value) + assert flash_overlay._flash_disabled() is True + + @pytest.mark.parametrize("value", ["0", "false", "no", "off", ""]) + def test_falsy_values_keep_enabled(self, monkeypatch, value): + monkeypatch.setenv("WINDOWS_MCP_DISABLE_FLASH", value) + assert flash_overlay._flash_disabled() is False + + +class TestShowCaptureFlash: + def test_disabled_env_var_skips_thread(self, monkeypatch): + monkeypatch.setenv("WINDOWS_MCP_DISABLE_FLASH", "1") + with patch.object(threading, "Thread") as fake_thread: + flash_overlay.show_capture_flash([(0, 0, 100, 100)], full_screen=False) + fake_thread.assert_not_called() + assert flash_overlay._active_overlay is None + + def test_empty_rects_skips_thread(self, monkeypatch): + monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) + with patch.object(threading, "Thread") as fake_thread: + flash_overlay.show_capture_flash([], full_screen=False) + fake_thread.assert_not_called() + assert flash_overlay._active_overlay is None + + def test_registers_overlay_and_starts_daemon_thread(self, monkeypatch): + monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) + + captured = {} + + class _StubThread: + def __init__(self, target, args, name, daemon): + captured["target"] = target + captured["args"] = args + captured["name"] = name + captured["daemon"] = daemon + + def start(self): + captured["started"] = True + + monkeypatch.setattr(flash_overlay.threading, "Thread", _StubThread) + + flash_overlay.show_capture_flash([(10, 20, 110, 120)], full_screen=True) + + assert captured["started"] is True + assert captured["daemon"] is True + assert captured["name"] == "windows-mcp-flash" + assert flash_overlay._active_overlay is not None + # rects passed through; full_screen flag forwarded + rects_arg, full_screen_arg, overlay_arg = captured["args"] + assert rects_arg == [(10, 20, 110, 120)] + assert full_screen_arg is True + assert overlay_arg is flash_overlay._active_overlay + + +class TestCancelActiveFlash: + def test_no_op_when_no_active_overlay(self): + flash_overlay.cancel_active_flash() + assert flash_overlay._active_overlay is None + + def test_signals_stop_and_clears_active(self, monkeypatch): + # Install a stub overlay manually so we don't depend on Tk + overlay = flash_overlay._Overlay() + overlay.thread = threading.Thread(target=lambda: None, daemon=True) + overlay.thread.start() + overlay.thread.join() + with flash_overlay._lock: + flash_overlay._active_overlay = overlay + + flash_overlay.cancel_active_flash(timeout=0.1) + + assert overlay.stop_event.is_set() + assert flash_overlay._active_overlay is None + + +class TestRunOverlayFallthrough: + def test_missing_tkinter_sets_closed_event(self, monkeypatch): + # Force ``import tkinter`` inside _run_overlay to fail + monkeypatch.setitem(sys.modules, "tkinter", None) + overlay = flash_overlay._Overlay() + flash_overlay._run_overlay([(0, 0, 100, 100)], False, overlay) + assert overlay.closed_event.is_set() From dc5e8b091846795a75f3bfaa6c3baa86de8fb474 Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Fri, 8 May 2026 22:29:44 +0200 Subject: [PATCH 02/13] feat(screenshot): render the flash as an actual fading glow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/windows_mcp/desktop/flash_overlay.py | 147 ++++++++++++----------- tests/test_flash_overlay.py | 36 ++++++ 2 files changed, 114 insertions(+), 69 deletions(-) diff --git a/src/windows_mcp/desktop/flash_overlay.py b/src/windows_mcp/desktop/flash_overlay.py index 1ea8eaf0..2314a832 100644 --- a/src/windows_mcp/desktop/flash_overlay.py +++ b/src/windows_mcp/desktop/flash_overlay.py @@ -1,8 +1,12 @@ """Brief on-screen visual confirmation that a screenshot was taken. -Draws a glowing orange-red border over the captured area for ~1 second using -a transparent always-on-top Tk window on a daemon thread. The flash is started -*after* the screenshot is captured so it never appears in the captured image. +Renders a glowing orange-red halo around the captured area for ~2.5 s and +then fades out. Implementation uses one ``tk.Toplevel`` per glow layer per +side (top/bottom/left/right) so per-window ``-alpha`` works reliably — the +``-alpha`` + ``-transparentcolor`` combination is unreliable on Windows. + +The flash is started *after* capture and any active overlay is torn down +before the next capture, so it never appears in a captured image. """ import logging @@ -15,8 +19,8 @@ _FLASH_COLOR = "#FF4500" _DURATION_MS = 2500 _FRAME_INTERVAL_MS = 20 -_BORDER_THICKNESS = 6 -_FULLSCREEN_INSET = 12 +_GLOW_LAYERS = 8 +_FULLSCREEN_INSET = 4 _lock = threading.Lock() _active_overlay: "_Overlay | None" = None @@ -55,12 +59,12 @@ def show_capture_flash( *, full_screen: bool, ) -> None: - """Show a fade-in/out orange-red border over each rect. + """Show a fade-in/out orange-red glow 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. + coordinates. ``full_screen=True`` draws an inner glow that radiates + inward from each monitor edge. ``full_screen=False`` draws an outer + halo around the captured region. Returns immediately; rendering happens on a daemon thread. """ @@ -80,6 +84,39 @@ def show_capture_flash( overlay.thread.start() +def _build_strip_defs( + rects: list[tuple[int, int, int, int]], + full_screen: bool, +) -> list[tuple[int, int, int, int, float]]: + """Return ``(l, t, r, b, base_alpha)`` strip rects for every glow layer. + + For region mode the strips spread *outward* from the rect edge; for + full-screen mode they spread *inward* from each monitor edge. Layer 0 + sits exactly on the rect/edge boundary at full opacity; subsequent + layers step further away with a quadratic falloff. + """ + strips: list[tuple[int, int, int, int, float]] = [] + base_inset = _FULLSCREEN_INSET if full_screen else 0 + for r_left, r_top, r_right, r_bottom in rects: + for layer in range(_GLOW_LAYERS): + falloff = (1.0 - layer / _GLOW_LAYERS) ** 2 + if full_screen: + offset = base_inset + layer + else: + offset = -layer + left = r_left + offset + top = r_top + offset + right = r_right - offset + bottom = r_bottom - offset + if right - left < 2 or bottom - top < 2: + continue + strips.append((left, top, right, top + 1, falloff)) + strips.append((left, bottom - 1, right, bottom, falloff)) + strips.append((left, top, left + 1, bottom, falloff)) + strips.append((right - 1, top, right, bottom, falloff)) + return strips + + def _run_overlay( rects: list[tuple[int, int, int, int]], full_screen: bool, @@ -94,63 +131,30 @@ def _run_overlay( root: "tk.Tk | None" = None try: - left = min(r[0] for r in rects) - top = min(r[1] for r in rects) - right = max(r[2] for r in rects) - bottom = max(r[3] for r in rects) - width = right - left - height = bottom - top - if width <= 0 or height <= 0: + strip_defs = _build_strip_defs(rects, full_screen) + if not strip_defs: return root = tk.Tk() root.withdraw() - root.overrideredirect(True) - root.attributes("-topmost", True) - try: - root.attributes("-disabled", True) - except tk.TclError: - pass - - transparent_color = "#010203" - try: - root.configure(bg=transparent_color) - root.attributes("-transparentcolor", transparent_color) - canvas_bg = transparent_color - except tk.TclError: - canvas_bg = root.cget("bg") - - root.geometry(f"{width}x{height}+{left}+{top}") - - canvas = tk.Canvas( - root, - width=width, - height=height, - bg=canvas_bg, - highlightthickness=0, - borderwidth=0, - ) - canvas.pack(fill="both", expand=True) - - inset = _FULLSCREEN_INSET if full_screen else 0 - for r_left, r_top, r_right, r_bottom in rects: - x1 = r_left - left + inset - y1 = r_top - top + inset - x2 = r_right - left - inset - 1 - y2 = r_bottom - top - inset - 1 - if x2 - x1 <= 0 or y2 - y1 <= 0: - continue - for i in range(_BORDER_THICKNESS): - canvas.create_rectangle( - x1 + i, - y1 + i, - x2 - i, - y2 - i, - outline=_FLASH_COLOR, - width=1, - ) - - root.deiconify() + + strip_windows: list[tuple["tk.Toplevel", float]] = [] + for left, top, right, bottom, base_alpha in strip_defs: + w = tk.Toplevel(root) + w.overrideredirect(True) + w.attributes("-topmost", True) + try: + w.attributes("-toolwindow", True) + except tk.TclError: + pass + w.configure(bg=_FLASH_COLOR) + w.geometry(f"{right - left}x{bottom - top}+{left}+{top}") + try: + w.attributes("-alpha", base_alpha) + except tk.TclError: + pass + strip_windows.append((w, base_alpha)) + start = time.perf_counter() def tick() -> None: @@ -161,15 +165,20 @@ def tick() -> None: if elapsed_ms >= _DURATION_MS: root.destroy() return - t = elapsed_ms / _DURATION_MS + t_norm = elapsed_ms / _DURATION_MS if full_screen: - alpha = 1.0 - abs(2 * t - 1) + time_alpha = 1.0 - abs(2 * t_norm - 1) + elif t_norm < 0.15: + time_alpha = t_norm / 0.15 + elif t_norm < 0.65: + time_alpha = 1.0 else: - alpha = 1.0 if t < 0.65 else max(0.0, 1.0 - (t - 0.65) / 0.35) - try: - root.attributes("-alpha", max(0.0, min(1.0, alpha))) - except tk.TclError: - pass + time_alpha = max(0.0, 1.0 - (t_norm - 0.65) / 0.35) + for w, base_alpha in strip_windows: + try: + w.attributes("-alpha", max(0.0, min(1.0, base_alpha * time_alpha))) + except tk.TclError: + pass root.after(_FRAME_INTERVAL_MS, tick) root.after(0, tick) diff --git a/tests/test_flash_overlay.py b/tests/test_flash_overlay.py index e8100e19..cac1769b 100644 --- a/tests/test_flash_overlay.py +++ b/tests/test_flash_overlay.py @@ -108,6 +108,42 @@ def test_signals_stop_and_clears_active(self, monkeypatch): assert flash_overlay._active_overlay is None +class TestBuildStripDefs: + def test_region_strips_spread_outward_from_rect(self): + defs = flash_overlay._build_strip_defs([(100, 200, 500, 400)], full_screen=False) + # Layer 0 (innermost glow ring) sits exactly on the rect edge with full alpha. + layer0 = [d for d in defs if d[4] == 1.0] + assert len(layer0) == 4 + top, bottom, left, right = layer0 + assert top == (100, 200, 500, 201, 1.0) + assert bottom == (100, 399, 500, 400, 1.0) + assert left == (100, 200, 101, 400, 1.0) + assert right == (499, 200, 500, 400, 1.0) + + def test_region_outer_layer_has_lower_alpha_and_grows_outward(self): + defs = flash_overlay._build_strip_defs([(100, 200, 500, 400)], full_screen=False) + last_layer_alpha = (1.0 - (flash_overlay._GLOW_LAYERS - 1) / flash_overlay._GLOW_LAYERS) ** 2 + outer = [d for d in defs if abs(d[4] - last_layer_alpha) < 1e-9] + assert len(outer) == 4 + # Outermost top strip sits ABOVE the rect edge (smaller y) by N-1 pixels. + top = next(d for d in outer if d[3] - d[1] == 1 and d[2] - d[0] > 1) + assert top[1] == 200 - (flash_overlay._GLOW_LAYERS - 1) + + def test_full_screen_strips_spread_inward(self): + defs = flash_overlay._build_strip_defs([(0, 0, 1000, 800)], full_screen=True) + layer0 = [d for d in defs if d[4] == 1.0] + # Inner-most ring is offset by _FULLSCREEN_INSET from the screen edge. + inset = flash_overlay._FULLSCREEN_INSET + top = next(d for d in layer0 if d[3] - d[1] == 1) + assert top[1] == inset + assert top[3] == inset + 1 + + def test_full_screen_too_small_rect_produces_no_strips(self): + # Full-screen mode strips inset inward; a 1x1 rect can't fit any layer. + defs = flash_overlay._build_strip_defs([(0, 0, 1, 1)], full_screen=True) + assert defs == [] + + class TestRunOverlayFallthrough: def test_missing_tkinter_sets_closed_event(self, monkeypatch): # Force ``import tkinter`` inside _run_overlay to fail From 0142126fbe1864c72496ac4bcc822e208efa9783 Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Fri, 8 May 2026 23:12:02 +0200 Subject: [PATCH 03/13] fix(screenshot): make the glow render reliably on a single Tk window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- src/windows_mcp/desktop/flash_overlay.py | 150 ++++++++++++----------- tests/test_flash_overlay.py | 49 +++----- 2 files changed, 94 insertions(+), 105 deletions(-) diff --git a/src/windows_mcp/desktop/flash_overlay.py b/src/windows_mcp/desktop/flash_overlay.py index 2314a832..1c4feb76 100644 --- a/src/windows_mcp/desktop/flash_overlay.py +++ b/src/windows_mcp/desktop/flash_overlay.py @@ -1,9 +1,13 @@ """Brief on-screen visual confirmation that a screenshot was taken. -Renders a glowing orange-red halo around the captured area for ~2.5 s and -then fades out. Implementation uses one ``tk.Toplevel`` per glow layer per -side (top/bottom/left/right) so per-window ``-alpha`` works reliably — the -``-alpha`` + ``-transparentcolor`` combination is unreliable on Windows. +Renders a glowing orange-red halo around the captured area for ~2.5 s on a +single transparent always-on-top Tk window. The "glow" is produced by +drawing concentric border rectangles whose RGB is blended from full +orange-red toward pure black; the canvas's transparent-colour key is set to +pure black, so pixels that fade to black become genuinely transparent. The +time fade is achieved by re-rendering with a scaled intensity each frame — +``-alpha`` is deliberately avoided because it combines unreliably with +``-transparentcolor`` on Windows. The flash is started *after* capture and any active overlay is torn down before the next capture, so it never appears in a captured image. @@ -16,11 +20,13 @@ logger = logging.getLogger(__name__) -_FLASH_COLOR = "#FF4500" +_FLASH_RGB = (0xFF, 0x45, 0x00) +_TRANSPARENT_COLOR = "#000000" _DURATION_MS = 2500 _FRAME_INTERVAL_MS = 20 -_GLOW_LAYERS = 8 +_GLOW_LAYERS = 12 _FULLSCREEN_INSET = 4 +_MIN_VISIBLE_INTENSITY = 0.04 _lock = threading.Lock() _active_overlay: "_Overlay | None" = None @@ -39,11 +45,7 @@ def __init__(self) -> None: 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. - """ + """Tear down any flash overlay currently on screen.""" global _active_overlay with _lock: ov = _active_overlay @@ -59,14 +61,13 @@ def show_capture_flash( *, full_screen: bool, ) -> None: - """Show a fade-in/out orange-red glow over each rect. + """Show a fade-in/out orange-red glow around each rect. ``rects`` are ``(left, top, right, bottom)`` tuples in virtual-screen coordinates. ``full_screen=True`` draws an inner glow that radiates inward from each monitor edge. ``full_screen=False`` draws an outer - halo around the captured region. - - Returns immediately; rendering happens on a daemon thread. + halo around the captured region. Returns immediately; rendering happens + on a daemon thread. """ if _flash_disabled() or not rects: return @@ -84,37 +85,15 @@ def show_capture_flash( overlay.thread.start() -def _build_strip_defs( - rects: list[tuple[int, int, int, int]], - full_screen: bool, -) -> list[tuple[int, int, int, int, float]]: - """Return ``(l, t, r, b, base_alpha)`` strip rects for every glow layer. - - For region mode the strips spread *outward* from the rect edge; for - full-screen mode they spread *inward* from each monitor edge. Layer 0 - sits exactly on the rect/edge boundary at full opacity; subsequent - layers step further away with a quadratic falloff. - """ - strips: list[tuple[int, int, int, int, float]] = [] - base_inset = _FULLSCREEN_INSET if full_screen else 0 - for r_left, r_top, r_right, r_bottom in rects: - for layer in range(_GLOW_LAYERS): - falloff = (1.0 - layer / _GLOW_LAYERS) ** 2 - if full_screen: - offset = base_inset + layer - else: - offset = -layer - left = r_left + offset - top = r_top + offset - right = r_right - offset - bottom = r_bottom - offset - if right - left < 2 or bottom - top < 2: - continue - strips.append((left, top, right, top + 1, falloff)) - strips.append((left, bottom - 1, right, bottom, falloff)) - strips.append((left, top, left + 1, bottom, falloff)) - strips.append((right - 1, top, right, bottom, falloff)) - return strips +def _layer_color(intensity: float) -> str: + """Return a Tk hex colour string for orange-red scaled by ``intensity``.""" + r = int(_FLASH_RGB[0] * intensity) + g = int(_FLASH_RGB[1] * intensity) + b = int(_FLASH_RGB[2] * intensity) + # Avoid landing exactly on the transparent-colour key for very-dim layers. + if r + g + b == 0: + r = 1 + return f"#{r:02X}{g:02X}{b:02X}" def _run_overlay( @@ -131,29 +110,61 @@ def _run_overlay( root: "tk.Tk | None" = None try: - strip_defs = _build_strip_defs(rects, full_screen) - if not strip_defs: + union_left = min(r[0] for r in rects) + union_top = min(r[1] for r in rects) + union_right = max(r[2] for r in rects) + union_bottom = max(r[3] for r in rects) + # Region mode expands outward by up to _GLOW_LAYERS pixels; widen the + # window so the outer layers stay inside the canvas. + if not full_screen: + union_left -= _GLOW_LAYERS + union_top -= _GLOW_LAYERS + union_right += _GLOW_LAYERS + union_bottom += _GLOW_LAYERS + width = union_right - union_left + height = union_bottom - union_top + if width <= 0 or height <= 0: return root = tk.Tk() - root.withdraw() - - strip_windows: list[tuple["tk.Toplevel", float]] = [] - for left, top, right, bottom, base_alpha in strip_defs: - w = tk.Toplevel(root) - w.overrideredirect(True) - w.attributes("-topmost", True) - try: - w.attributes("-toolwindow", True) - except tk.TclError: - pass - w.configure(bg=_FLASH_COLOR) - w.geometry(f"{right - left}x{bottom - top}+{left}+{top}") - try: - w.attributes("-alpha", base_alpha) - except tk.TclError: - pass - strip_windows.append((w, base_alpha)) + root.overrideredirect(True) + root.attributes("-topmost", True) + root.configure(bg=_TRANSPARENT_COLOR) + try: + root.attributes("-transparentcolor", _TRANSPARENT_COLOR) + except tk.TclError: + pass + root.geometry(f"{width}x{height}+{union_left}+{union_top}") + + canvas = tk.Canvas( + root, + width=width, + height=height, + bg=_TRANSPARENT_COLOR, + highlightthickness=0, + borderwidth=0, + ) + canvas.pack(fill="both", expand=True) + + base_inset = _FULLSCREEN_INSET if full_screen else 0 + + def render(time_alpha: float) -> None: + canvas.delete("glow") + for layer in range(_GLOW_LAYERS): + falloff = (1.0 - layer / _GLOW_LAYERS) ** 2 + intensity = falloff * time_alpha + if intensity < _MIN_VISIBLE_INTENSITY: + continue + color = _layer_color(intensity) + offset = base_inset + layer if full_screen else -layer + for r_left, r_top, r_right, r_bottom in rects: + x1 = r_left - union_left + offset + y1 = r_top - union_top + offset + x2 = r_right - union_left - offset - 1 + y2 = r_bottom - union_top - offset - 1 + if x2 - x1 <= 0 or y2 - y1 <= 0: + continue + canvas.create_rectangle(x1, y1, x2, y2, outline=color, width=1, tags="glow") start = time.perf_counter() @@ -174,13 +185,10 @@ def tick() -> None: time_alpha = 1.0 else: time_alpha = max(0.0, 1.0 - (t_norm - 0.65) / 0.35) - for w, base_alpha in strip_windows: - try: - w.attributes("-alpha", max(0.0, min(1.0, base_alpha * time_alpha))) - except tk.TclError: - pass + render(time_alpha) root.after(_FRAME_INTERVAL_MS, tick) + render(1.0) root.after(0, tick) root.mainloop() except Exception: diff --git a/tests/test_flash_overlay.py b/tests/test_flash_overlay.py index cac1769b..f75fb463 100644 --- a/tests/test_flash_overlay.py +++ b/tests/test_flash_overlay.py @@ -108,40 +108,21 @@ def test_signals_stop_and_clears_active(self, monkeypatch): assert flash_overlay._active_overlay is None -class TestBuildStripDefs: - def test_region_strips_spread_outward_from_rect(self): - defs = flash_overlay._build_strip_defs([(100, 200, 500, 400)], full_screen=False) - # Layer 0 (innermost glow ring) sits exactly on the rect edge with full alpha. - layer0 = [d for d in defs if d[4] == 1.0] - assert len(layer0) == 4 - top, bottom, left, right = layer0 - assert top == (100, 200, 500, 201, 1.0) - assert bottom == (100, 399, 500, 400, 1.0) - assert left == (100, 200, 101, 400, 1.0) - assert right == (499, 200, 500, 400, 1.0) - - def test_region_outer_layer_has_lower_alpha_and_grows_outward(self): - defs = flash_overlay._build_strip_defs([(100, 200, 500, 400)], full_screen=False) - last_layer_alpha = (1.0 - (flash_overlay._GLOW_LAYERS - 1) / flash_overlay._GLOW_LAYERS) ** 2 - outer = [d for d in defs if abs(d[4] - last_layer_alpha) < 1e-9] - assert len(outer) == 4 - # Outermost top strip sits ABOVE the rect edge (smaller y) by N-1 pixels. - top = next(d for d in outer if d[3] - d[1] == 1 and d[2] - d[0] > 1) - assert top[1] == 200 - (flash_overlay._GLOW_LAYERS - 1) - - def test_full_screen_strips_spread_inward(self): - defs = flash_overlay._build_strip_defs([(0, 0, 1000, 800)], full_screen=True) - layer0 = [d for d in defs if d[4] == 1.0] - # Inner-most ring is offset by _FULLSCREEN_INSET from the screen edge. - inset = flash_overlay._FULLSCREEN_INSET - top = next(d for d in layer0 if d[3] - d[1] == 1) - assert top[1] == inset - assert top[3] == inset + 1 - - def test_full_screen_too_small_rect_produces_no_strips(self): - # Full-screen mode strips inset inward; a 1x1 rect can't fit any layer. - defs = flash_overlay._build_strip_defs([(0, 0, 1, 1)], full_screen=True) - assert defs == [] +class TestLayerColor: + def test_full_intensity_returns_orange_red(self): + assert flash_overlay._layer_color(1.0).upper() == "#FF4500" + + def test_half_intensity_blends_toward_black(self): + c = flash_overlay._layer_color(0.5) + # 0xFF * 0.5 = 127 -> 7F; 0x45 * 0.5 = 34 -> 22; 0x00 * 0.5 = 0 -> 00. + assert c.upper() == "#7F2200" + + def test_zero_intensity_avoids_pure_black(self): + # Pure black is the transparent-colour key; the helper must not return it + # because a 0-intensity layer would otherwise punch a hole in the canvas. + c = flash_overlay._layer_color(0.0) + assert c.upper() != "#000000" + assert c.upper() == "#010000" class TestRunOverlayFallthrough: From 699fa0a768e00ccbacbce102691c3969620bfc62 Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Fri, 8 May 2026 23:23:35 +0200 Subject: [PATCH 04/13] fix(screenshot): make the flash actually visible (single solid border) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/windows_mcp/desktop/flash_overlay.py | 162 +++++++++++------------ tests/test_flash_overlay.py | 42 +++--- 2 files changed, 107 insertions(+), 97 deletions(-) diff --git a/src/windows_mcp/desktop/flash_overlay.py b/src/windows_mcp/desktop/flash_overlay.py index 1c4feb76..c4cbfa42 100644 --- a/src/windows_mcp/desktop/flash_overlay.py +++ b/src/windows_mcp/desktop/flash_overlay.py @@ -1,16 +1,15 @@ """Brief on-screen visual confirmation that a screenshot was taken. -Renders a glowing orange-red halo around the captured area for ~2.5 s on a -single transparent always-on-top Tk window. The "glow" is produced by -drawing concentric border rectangles whose RGB is blended from full -orange-red toward pure black; the canvas's transparent-colour key is set to -pure black, so pixels that fade to black become genuinely transparent. The -time fade is achieved by re-rendering with a scaled intensity each frame — -``-alpha`` is deliberately avoided because it combines unreliably with -``-transparentcolor`` on Windows. +Renders an orange-red glow halo around the captured area for ~2.5 s using +small **opaque** Tk Toplevel windows arranged as concentric border strips +(top/bottom/left/right per glow layer). Per-window ``-alpha`` gives a +real fade and a stack of decreasing alphas creates the soft halo. Tk's +``-transparentcolor`` is deliberately *not* used because some Windows +configurations refuse to render a window when it is set, so the original +canvas-based approach was invisible on those systems. The flash is started *after* capture and any active overlay is torn down -before the next capture, so it never appears in a captured image. +before the next capture so it never appears in a captured image. """ import logging @@ -20,13 +19,15 @@ logger = logging.getLogger(__name__) -_FLASH_RGB = (0xFF, 0x45, 0x00) -_TRANSPARENT_COLOR = "#000000" +_FLASH_COLOR = "#FF4500" _DURATION_MS = 2500 -_FRAME_INTERVAL_MS = 20 -_GLOW_LAYERS = 12 -_FULLSCREEN_INSET = 4 -_MIN_VISIBLE_INTENSITY = 0.04 +_FRAME_INTERVAL_MS = 25 +# Tk on Windows hangs its mainloop when ~8+ Toplevels are created back-to-back +# on a non-main thread (overrideredirect + alpha), so the halo is rendered with +# a single ring of 4 thick strips rather than a multi-layer gradient. +_GLOW_LAYERS = 1 +_LAYER_THICKNESS = 8 +_FULLSCREEN_INSET = 6 _lock = threading.Lock() _active_overlay: "_Overlay | None" = None @@ -64,10 +65,9 @@ def show_capture_flash( """Show a fade-in/out orange-red glow around each rect. ``rects`` are ``(left, top, right, bottom)`` tuples in virtual-screen - coordinates. ``full_screen=True`` draws an inner glow that radiates - inward from each monitor edge. ``full_screen=False`` draws an outer - halo around the captured region. Returns immediately; rendering happens - on a daemon thread. + coordinates. ``full_screen=True`` draws an inner halo radiating inward + from each monitor edge; ``full_screen=False`` draws an outer halo. + Returns immediately; rendering happens on a daemon thread. """ if _flash_disabled() or not rects: return @@ -85,15 +85,36 @@ def show_capture_flash( overlay.thread.start() -def _layer_color(intensity: float) -> str: - """Return a Tk hex colour string for orange-red scaled by ``intensity``.""" - r = int(_FLASH_RGB[0] * intensity) - g = int(_FLASH_RGB[1] * intensity) - b = int(_FLASH_RGB[2] * intensity) - # Avoid landing exactly on the transparent-colour key for very-dim layers. - if r + g + b == 0: - r = 1 - return f"#{r:02X}{g:02X}{b:02X}" +def _build_strip_defs( + rects: list[tuple[int, int, int, int]], + full_screen: bool, +) -> list[tuple[int, int, int, int, float]]: + """Return ``(l, t, r, b, base_alpha)`` for every strip window. + + The first layer sits on the rect edge at full alpha (1.0); each + subsequent layer steps further away (outward for region, inward for + full-screen) at quadratically falling base alpha. + """ + strips: list[tuple[int, int, int, int, float]] = [] + base_inset = _FULLSCREEN_INSET if full_screen else 0 + for r_left, r_top, r_right, r_bottom in rects: + for layer in range(_GLOW_LAYERS): + base_alpha = (1.0 - layer / _GLOW_LAYERS) ** 2 + if full_screen: + offset = base_inset + layer * _LAYER_THICKNESS + else: + offset = -(layer + 1) * _LAYER_THICKNESS + left = r_left + offset + top = r_top + offset + right = r_right - offset + bottom = r_bottom - offset + if right - left < _LAYER_THICKNESS * 2 or bottom - top < _LAYER_THICKNESS * 2: + continue + strips.append((left, top, right, top + _LAYER_THICKNESS, base_alpha)) + strips.append((left, bottom - _LAYER_THICKNESS, right, bottom, base_alpha)) + strips.append((left, top, left + _LAYER_THICKNESS, bottom, base_alpha)) + strips.append((right - _LAYER_THICKNESS, top, right, bottom, base_alpha)) + return strips def _run_overlay( @@ -110,62 +131,36 @@ def _run_overlay( root: "tk.Tk | None" = None try: - union_left = min(r[0] for r in rects) - union_top = min(r[1] for r in rects) - union_right = max(r[2] for r in rects) - union_bottom = max(r[3] for r in rects) - # Region mode expands outward by up to _GLOW_LAYERS pixels; widen the - # window so the outer layers stay inside the canvas. - if not full_screen: - union_left -= _GLOW_LAYERS - union_top -= _GLOW_LAYERS - union_right += _GLOW_LAYERS - union_bottom += _GLOW_LAYERS - width = union_right - union_left - height = union_bottom - union_top - if width <= 0 or height <= 0: + strip_defs = _build_strip_defs(rects, full_screen) + if not strip_defs: return root = tk.Tk() - root.overrideredirect(True) - root.attributes("-topmost", True) - root.configure(bg=_TRANSPARENT_COLOR) - try: - root.attributes("-transparentcolor", _TRANSPARENT_COLOR) - except tk.TclError: - pass - root.geometry(f"{width}x{height}+{union_left}+{union_top}") - - canvas = tk.Canvas( - root, - width=width, - height=height, - bg=_TRANSPARENT_COLOR, - highlightthickness=0, - borderwidth=0, - ) - canvas.pack(fill="both", expand=True) - - base_inset = _FULLSCREEN_INSET if full_screen else 0 - - def render(time_alpha: float) -> None: - canvas.delete("glow") - for layer in range(_GLOW_LAYERS): - falloff = (1.0 - layer / _GLOW_LAYERS) ** 2 - intensity = falloff * time_alpha - if intensity < _MIN_VISIBLE_INTENSITY: - continue - color = _layer_color(intensity) - offset = base_inset + layer if full_screen else -layer - for r_left, r_top, r_right, r_bottom in rects: - x1 = r_left - union_left + offset - y1 = r_top - union_top + offset - x2 = r_right - union_left - offset - 1 - y2 = r_bottom - union_top - offset - 1 - if x2 - x1 <= 0 or y2 - y1 <= 0: - continue - canvas.create_rectangle(x1, y1, x2, y2, outline=color, width=1, tags="glow") + root.withdraw() + windows: list[tuple["tk.Toplevel", float]] = [] + for left, top, right, bottom, base_alpha in strip_defs: + w = tk.Toplevel(root) + w.overrideredirect(True) + w.attributes("-topmost", True) + try: + w.attributes("-toolwindow", True) + except tk.TclError: + pass + w.configure(bg=_FLASH_COLOR) + w.geometry(f"{right - left}x{bottom - top}+{left}+{top}") + try: + w.attributes("-alpha", base_alpha) + except tk.TclError: + pass + windows.append((w, base_alpha)) + + logger.info( + "screenshot flash overlay started: %d strip windows for %d rect(s) (full_screen=%s)", + len(windows), + len(rects), + full_screen, + ) start = time.perf_counter() def tick() -> None: @@ -185,10 +180,13 @@ def tick() -> None: time_alpha = 1.0 else: time_alpha = max(0.0, 1.0 - (t_norm - 0.65) / 0.35) - render(time_alpha) + for w, base_alpha in windows: + try: + w.attributes("-alpha", max(0.05, min(1.0, base_alpha * time_alpha))) + except tk.TclError: + pass root.after(_FRAME_INTERVAL_MS, tick) - render(1.0) root.after(0, tick) root.mainloop() except Exception: diff --git a/tests/test_flash_overlay.py b/tests/test_flash_overlay.py index f75fb463..252348d8 100644 --- a/tests/test_flash_overlay.py +++ b/tests/test_flash_overlay.py @@ -108,21 +108,33 @@ def test_signals_stop_and_clears_active(self, monkeypatch): assert flash_overlay._active_overlay is None -class TestLayerColor: - def test_full_intensity_returns_orange_red(self): - assert flash_overlay._layer_color(1.0).upper() == "#FF4500" - - def test_half_intensity_blends_toward_black(self): - c = flash_overlay._layer_color(0.5) - # 0xFF * 0.5 = 127 -> 7F; 0x45 * 0.5 = 34 -> 22; 0x00 * 0.5 = 0 -> 00. - assert c.upper() == "#7F2200" - - def test_zero_intensity_avoids_pure_black(self): - # Pure black is the transparent-colour key; the helper must not return it - # because a 0-intensity layer would otherwise punch a hole in the canvas. - c = flash_overlay._layer_color(0.0) - assert c.upper() != "#000000" - assert c.upper() == "#010000" +class TestBuildStripDefs: + def test_region_layer_zero_sits_on_rect_edge_with_full_alpha(self): + defs = flash_overlay._build_strip_defs([(100, 200, 500, 400)], full_screen=False) + # First strip stack is layer 0 (full alpha); 4 strips per layer. + layer0 = [d for d in defs if d[4] == 1.0] + assert len(layer0) == 4 + # Layer 0 is offset outward by 1 * _LAYER_THICKNESS so the inner edge + # of the strip aligns with the rect edge. + thickness = flash_overlay._LAYER_THICKNESS + top, bottom, left, right = layer0 + assert top == (100 - thickness, 200 - thickness, 500 + thickness, 200, 1.0) + assert bottom == (100 - thickness, 400, 500 + thickness, 400 + thickness, 1.0) + + def test_full_screen_strips_inset_inward_from_edge(self): + defs = flash_overlay._build_strip_defs([(0, 0, 1000, 800)], full_screen=True) + layer0 = [d for d in defs if d[4] == 1.0] + assert len(layer0) == 4 + inset = flash_overlay._FULLSCREEN_INSET + thickness = flash_overlay._LAYER_THICKNESS + # Top strip: y range is [inset, inset+thickness] + top = next(d for d in layer0 if d[3] - d[1] == thickness and d[2] - d[0] > thickness) + assert top[1] == inset + assert top[3] == inset + thickness + + def test_too_small_full_screen_rect_produces_no_strips(self): + defs = flash_overlay._build_strip_defs([(0, 0, 4, 4)], full_screen=True) + assert defs == [] class TestRunOverlayFallthrough: From 4e46068a188b76863f37b1952e60e44184a65acf Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Fri, 8 May 2026 23:33:36 +0200 Subject: [PATCH 05/13] feat(screenshot): replace Tk overlay with Win32 layered window glow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- src/windows_mcp/desktop/flash_overlay.py | 531 ++++++++++++++++++----- tests/test_flash_overlay.py | 64 +-- 2 files changed, 469 insertions(+), 126 deletions(-) diff --git a/src/windows_mcp/desktop/flash_overlay.py b/src/windows_mcp/desktop/flash_overlay.py index c4cbfa42..ae1aa51b 100644 --- a/src/windows_mcp/desktop/flash_overlay.py +++ b/src/windows_mcp/desktop/flash_overlay.py @@ -1,33 +1,34 @@ """Brief on-screen visual confirmation that a screenshot was taken. -Renders an orange-red glow halo around the captured area for ~2.5 s using -small **opaque** Tk Toplevel windows arranged as concentric border strips -(top/bottom/left/right per glow layer). Per-window ``-alpha`` gives a -real fade and a stack of decreasing alphas creates the soft halo. Tk's -``-transparentcolor`` is deliberately *not* used because some Windows -configurations refuse to render a window when it is set, so the original -canvas-based approach was invisible on those systems. +Renders a soft orange-red glow halo around the captured area for ~2.5 s +using a Win32 layered window with per-pixel alpha (``UpdateLayeredWindow`` ++ premultiplied BGRA DIB section). Tk was tried twice and abandoned — +``-transparentcolor`` rendered nothing on some Windows configs and a +multi-Toplevel "strip" approach hung Tk's mainloop on a non-main thread +once more than ~6 windows were created back-to-back. The flash is started *after* capture and any active overlay is torn down before the next capture so it never appears in a captured image. """ +import ctypes import logging import os import threading import time +from ctypes import wintypes logger = logging.getLogger(__name__) -_FLASH_COLOR = "#FF4500" +_FLASH_RGB = (0xFF, 0x45, 0x00) _DURATION_MS = 2500 -_FRAME_INTERVAL_MS = 25 -# Tk on Windows hangs its mainloop when ~8+ Toplevels are created back-to-back -# on a non-main thread (overrideredirect + alpha), so the halo is rendered with -# a single ring of 4 thick strips rather than a multi-layer gradient. -_GLOW_LAYERS = 1 -_LAYER_THICKNESS = 8 +_FRAME_INTERVAL_MS = 30 +_GLOW_BORDER_THICKNESS = 4 +_GLOW_BLUR_RADIUS = 14 +_GLOW_MARGIN = _GLOW_BLUR_RADIUS * 3 _FULLSCREEN_INSET = 6 +_MIN_VISIBLE_INTENSITY = 0.04 +_INTENSITY_QUANT = 32 _lock = threading.Lock() _active_overlay: "_Overlay | None" = None @@ -66,8 +67,9 @@ def show_capture_flash( ``rects`` are ``(left, top, right, bottom)`` tuples in virtual-screen coordinates. ``full_screen=True`` draws an inner halo radiating inward - from each monitor edge; ``full_screen=False`` draws an outer halo. - Returns immediately; rendering happens on a daemon thread. + from each monitor edge; ``full_screen=False`` draws an outer halo + around the captured region. Returns immediately; rendering happens on + a daemon thread. """ if _flash_disabled() or not rects: return @@ -85,36 +87,351 @@ def show_capture_flash( overlay.thread.start() -def _build_strip_defs( - rects: list[tuple[int, int, int, int]], - full_screen: bool, -) -> list[tuple[int, int, int, int, float]]: - """Return ``(l, t, r, b, base_alpha)`` for every strip window. +# --------------------------------------------------------------------------- +# Win32 plumbing +# --------------------------------------------------------------------------- + +_user32 = ctypes.windll.user32 +_gdi32 = ctypes.windll.gdi32 +_kernel32 = ctypes.windll.kernel32 + +_WS_POPUP = 0x80000000 +_WS_EX_LAYERED = 0x00080000 +_WS_EX_TRANSPARENT = 0x00000020 +_WS_EX_TOPMOST = 0x00000008 +_WS_EX_TOOLWINDOW = 0x00000080 +_WS_EX_NOACTIVATE = 0x08000000 +_ULW_ALPHA = 0x00000002 +_AC_SRC_OVER = 0x00 +_AC_SRC_ALPHA = 0x01 +_BI_RGB = 0 +_DIB_RGB_COLORS = 0 +_SW_SHOWNA = 8 +_HWND_TOPMOST = -1 +_SWP_NOSIZE = 0x0001 +_SWP_NOMOVE = 0x0002 +_SWP_NOACTIVATE = 0x0010 +_SWP_SHOWWINDOW = 0x0040 +_PM_REMOVE = 0x0001 +_WM_DESTROY = 0x0002 + + +class _POINT(ctypes.Structure): + _fields_ = [("x", ctypes.c_long), ("y", ctypes.c_long)] + + +class _SIZE(ctypes.Structure): + _fields_ = [("cx", ctypes.c_long), ("cy", ctypes.c_long)] + + +class _BLENDFUNCTION(ctypes.Structure): + _fields_ = [ + ("BlendOp", ctypes.c_byte), + ("BlendFlags", ctypes.c_byte), + ("SourceConstantAlpha", ctypes.c_byte), + ("AlphaFormat", ctypes.c_byte), + ] + + +class _BITMAPINFOHEADER(ctypes.Structure): + _fields_ = [ + ("biSize", ctypes.c_uint32), + ("biWidth", ctypes.c_long), + ("biHeight", ctypes.c_long), + ("biPlanes", ctypes.c_uint16), + ("biBitCount", ctypes.c_uint16), + ("biCompression", ctypes.c_uint32), + ("biSizeImage", ctypes.c_uint32), + ("biXPelsPerMeter", ctypes.c_long), + ("biYPelsPerMeter", ctypes.c_long), + ("biClrUsed", ctypes.c_uint32), + ("biClrImportant", ctypes.c_uint32), + ] + + +class _BITMAPINFO(ctypes.Structure): + _fields_ = [ + ("bmiHeader", _BITMAPINFOHEADER), + ("bmiColors", ctypes.c_uint32 * 3), + ] + + +# LRESULT is signed pointer-sized integer on Windows (use c_ssize_t for x64). +_LRESULT = ctypes.c_ssize_t - The first layer sits on the rect edge at full alpha (1.0); each - subsequent layer steps further away (outward for region, inward for - full-screen) at quadratically falling base alpha. +_WNDPROC = ctypes.WINFUNCTYPE( + _LRESULT, + wintypes.HWND, + ctypes.c_uint, + wintypes.WPARAM, + wintypes.LPARAM, +) + + +class _WNDCLASSEX(ctypes.Structure): + _fields_ = [ + ("cbSize", ctypes.c_uint), + ("style", ctypes.c_uint), + ("lpfnWndProc", _WNDPROC), + ("cbClsExtra", ctypes.c_int), + ("cbWndExtra", ctypes.c_int), + ("hInstance", wintypes.HINSTANCE), + ("hIcon", wintypes.HICON), + ("hCursor", wintypes.HANDLE), + ("hbrBackground", wintypes.HBRUSH), + ("lpszMenuName", wintypes.LPCWSTR), + ("lpszClassName", wintypes.LPCWSTR), + ("hIconSm", wintypes.HICON), + ] + + +_user32.CreateWindowExW.restype = wintypes.HWND +_user32.RegisterClassExW.restype = ctypes.c_ushort +_user32.DefWindowProcW.restype = _LRESULT +_user32.DefWindowProcW.argtypes = [ + wintypes.HWND, + ctypes.c_uint, + wintypes.WPARAM, + wintypes.LPARAM, +] +_user32.GetDC.restype = wintypes.HDC +_user32.GetDC.argtypes = [wintypes.HWND] +_user32.ReleaseDC.restype = ctypes.c_int +_user32.ReleaseDC.argtypes = [wintypes.HWND, wintypes.HDC] +_user32.UpdateLayeredWindow.restype = wintypes.BOOL +_user32.UpdateLayeredWindow.argtypes = [ + wintypes.HWND, + wintypes.HDC, + ctypes.POINTER(_POINT), + ctypes.POINTER(_SIZE), + wintypes.HDC, + ctypes.POINTER(_POINT), + wintypes.COLORREF, + ctypes.POINTER(_BLENDFUNCTION), + wintypes.DWORD, +] +_user32.DestroyWindow.argtypes = [wintypes.HWND] +_user32.ShowWindow.argtypes = [wintypes.HWND, ctypes.c_int] +_user32.SetWindowPos.argtypes = [ + wintypes.HWND, + wintypes.HWND, + ctypes.c_int, + ctypes.c_int, + ctypes.c_int, + ctypes.c_int, + ctypes.c_uint, +] +_gdi32.CreateCompatibleDC.restype = wintypes.HDC +_gdi32.CreateCompatibleDC.argtypes = [wintypes.HDC] +_gdi32.CreateDIBSection.restype = wintypes.HBITMAP +_gdi32.CreateDIBSection.argtypes = [ + wintypes.HDC, + ctypes.POINTER(_BITMAPINFO), + wintypes.UINT, + ctypes.POINTER(ctypes.c_void_p), + wintypes.HANDLE, + wintypes.DWORD, +] +_gdi32.SelectObject.restype = wintypes.HGDIOBJ +_gdi32.SelectObject.argtypes = [wintypes.HDC, wintypes.HGDIOBJ] +_gdi32.DeleteObject.argtypes = [wintypes.HGDIOBJ] +_gdi32.DeleteDC.argtypes = [wintypes.HDC] + + +# --------------------------------------------------------------------------- +# Rendering +# --------------------------------------------------------------------------- + + +def _render_glow_rgba( + width: int, + height: int, + rect_list: list[tuple[int, int, int, int]], +) -> "object": + """Return a PIL RGBA image with a soft halo ring around each rect. + + Each rect is in window-local coordinates. A sharp solid border is drawn + on the rect edge, then the layer is gaussian-blurred to spread the + glow, and the sharp ring is composited back on top so the inner edge + stays crisp. """ - strips: list[tuple[int, int, int, int, float]] = [] - base_inset = _FULLSCREEN_INSET if full_screen else 0 - for r_left, r_top, r_right, r_bottom in rects: - for layer in range(_GLOW_LAYERS): - base_alpha = (1.0 - layer / _GLOW_LAYERS) ** 2 - if full_screen: - offset = base_inset + layer * _LAYER_THICKNESS - else: - offset = -(layer + 1) * _LAYER_THICKNESS - left = r_left + offset - top = r_top + offset - right = r_right - offset - bottom = r_bottom - offset - if right - left < _LAYER_THICKNESS * 2 or bottom - top < _LAYER_THICKNESS * 2: + from PIL import Image, ImageDraw, ImageFilter + + sharp = Image.new("RGBA", (width, height), (0, 0, 0, 0)) + draw = ImageDraw.Draw(sharp) + color = (*_FLASH_RGB, 255) + for x1, y1, x2, y2 in rect_list: + for i in range(_GLOW_BORDER_THICKNESS): + draw.rectangle( + [x1 + i, y1 + i, x2 - i - 1, y2 - i - 1], + outline=color, + width=1, + ) + blurred = sharp.filter(ImageFilter.GaussianBlur(radius=_GLOW_BLUR_RADIUS)) + return Image.alpha_composite(blurred, sharp) + + +def _premultiplied_bgra(rgba_image, intensity: float) -> bytes: + """Convert PIL RGBA to BGRA premultiplied bytes scaled by ``intensity``.""" + bgra = bytearray(rgba_image.tobytes("raw", "BGRA")) + if intensity >= 1.0: + for i in range(0, len(bgra), 4): + a = bgra[i + 3] + if a == 0: continue - strips.append((left, top, right, top + _LAYER_THICKNESS, base_alpha)) - strips.append((left, bottom - _LAYER_THICKNESS, right, bottom, base_alpha)) - strips.append((left, top, left + _LAYER_THICKNESS, bottom, base_alpha)) - strips.append((right - _LAYER_THICKNESS, top, right, bottom, base_alpha)) - return strips + bgra[i] = (bgra[i] * a) // 255 + bgra[i + 1] = (bgra[i + 1] * a) // 255 + bgra[i + 2] = (bgra[i + 2] * a) // 255 + else: + for i in range(0, len(bgra), 4): + a = (bgra[i + 3] * int(intensity * 255)) // 255 + bgra[i + 3] = a + if a == 0: + bgra[i] = 0 + bgra[i + 1] = 0 + bgra[i + 2] = 0 + continue + bgra[i] = (bgra[i] * a) // 255 + bgra[i + 1] = (bgra[i + 1] * a) // 255 + bgra[i + 2] = (bgra[i + 2] * a) // 255 + return bytes(bgra) + + +# --------------------------------------------------------------------------- +# Window management +# --------------------------------------------------------------------------- + + +@_WNDPROC +def _wnd_proc(hwnd, msg, wparam, lparam): + if msg == _WM_DESTROY: + _user32.PostQuitMessage(0) + return 0 + return _user32.DefWindowProcW(hwnd, msg, wparam, lparam) + + +def _create_layered_window(class_name: str, x: int, y: int, w: int, h: int): + h_instance = _kernel32.GetModuleHandleW(None) + wc = _WNDCLASSEX() + wc.cbSize = ctypes.sizeof(_WNDCLASSEX) + wc.style = 0 + wc.lpfnWndProc = _wnd_proc + wc.cbClsExtra = 0 + wc.cbWndExtra = 0 + wc.hInstance = h_instance + wc.hIcon = None + wc.hCursor = None + wc.hbrBackground = None + wc.lpszMenuName = None + wc.lpszClassName = class_name + wc.hIconSm = None + + atom = _user32.RegisterClassExW(ctypes.byref(wc)) + if not atom: + raise OSError(f"RegisterClassExW failed: {ctypes.get_last_error()}") + + ex_style = ( + _WS_EX_LAYERED | _WS_EX_TRANSPARENT | _WS_EX_TOPMOST | _WS_EX_TOOLWINDOW | _WS_EX_NOACTIVATE + ) + hwnd = _user32.CreateWindowExW( + ex_style, + class_name, + "windows-mcp-flash", + _WS_POPUP, + x, + y, + w, + h, + None, + None, + h_instance, + None, + ) + if not hwnd: + _user32.UnregisterClassW(class_name, h_instance) + raise OSError(f"CreateWindowExW failed: {ctypes.get_last_error()}") + return hwnd, h_instance + + +def _push_bitmap(hwnd, x: int, y: int, w: int, h: int, bgra: bytes) -> None: + screen_dc = _user32.GetDC(None) + if not screen_dc: + raise OSError("GetDC failed") + try: + mem_dc = _gdi32.CreateCompatibleDC(screen_dc) + if not mem_dc: + raise OSError("CreateCompatibleDC failed") + try: + bmi = _BITMAPINFO() + bmi.bmiHeader.biSize = ctypes.sizeof(_BITMAPINFOHEADER) + bmi.bmiHeader.biWidth = w + bmi.bmiHeader.biHeight = -h # top-down DIB + bmi.bmiHeader.biPlanes = 1 + bmi.bmiHeader.biBitCount = 32 + bmi.bmiHeader.biCompression = _BI_RGB + + bits_ptr = ctypes.c_void_p() + hbm = _gdi32.CreateDIBSection( + screen_dc, + ctypes.byref(bmi), + _DIB_RGB_COLORS, + ctypes.byref(bits_ptr), + None, + 0, + ) + if not hbm: + raise OSError("CreateDIBSection failed") + try: + ctypes.memmove(bits_ptr, bgra, len(bgra)) + old_bmp = _gdi32.SelectObject(mem_dc, hbm) + try: + pos = _POINT(x, y) + size = _SIZE(w, h) + src_pos = _POINT(0, 0) + blend = _BLENDFUNCTION(_AC_SRC_OVER, 0, 255, _AC_SRC_ALPHA) + ok = _user32.UpdateLayeredWindow( + hwnd, + screen_dc, + ctypes.byref(pos), + ctypes.byref(size), + mem_dc, + ctypes.byref(src_pos), + 0, + ctypes.byref(blend), + _ULW_ALPHA, + ) + if not ok: + raise OSError(f"UpdateLayeredWindow failed: {ctypes.get_last_error()}") + finally: + _gdi32.SelectObject(mem_dc, old_bmp) + finally: + _gdi32.DeleteObject(hbm) + finally: + _gdi32.DeleteDC(mem_dc) + finally: + _user32.ReleaseDC(None, screen_dc) + + +def _pump_messages(hwnd) -> None: + msg = wintypes.MSG() + while _user32.PeekMessageW(ctypes.byref(msg), hwnd, 0, 0, _PM_REMOVE): + _user32.TranslateMessage(ctypes.byref(msg)) + _user32.DispatchMessageW(ctypes.byref(msg)) + + +def _intensity_at(t_norm: float, full_screen: bool) -> float: + if full_screen: + return 1.0 - abs(2 * t_norm - 1) + if t_norm < 0.15: + return t_norm / 0.15 + if t_norm < 0.65: + return 1.0 + return max(0.0, 1.0 - (t_norm - 0.65) / 0.35) + + +# --------------------------------------------------------------------------- +# Daemon thread entry point +# --------------------------------------------------------------------------- def _run_overlay( @@ -123,80 +440,96 @@ def _run_overlay( overlay: _Overlay, ) -> None: try: - import tkinter as tk + from PIL import Image # noqa: F401 — fail fast if Pillow missing except Exception: - logger.debug("tkinter unavailable; skipping screenshot flash") + logger.debug("Pillow unavailable; skipping screenshot flash") overlay.closed_event.set() return - root: "tk.Tk | None" = None + hwnd = None + h_instance = None + class_name = f"WindowsMCPFlash_{id(overlay):x}" + try: - strip_defs = _build_strip_defs(rects, full_screen) - if not strip_defs: + union_left = min(r[0] for r in rects) + union_top = min(r[1] for r in rects) + union_right = max(r[2] for r in rects) + union_bottom = max(r[3] for r in rects) + if not full_screen: + union_left -= _GLOW_MARGIN + union_top -= _GLOW_MARGIN + union_right += _GLOW_MARGIN + union_bottom += _GLOW_MARGIN + width = union_right - union_left + height = union_bottom - union_top + if width <= 0 or height <= 0: return - root = tk.Tk() - root.withdraw() + local_rects = [] + for r_left, r_top, r_right, r_bottom in rects: + inset = _FULLSCREEN_INSET if full_screen else 0 + local_rects.append( + ( + r_left - union_left + inset, + r_top - union_top + inset, + r_right - union_left - inset, + r_bottom - union_top - inset, + ) + ) - windows: list[tuple["tk.Toplevel", float]] = [] - for left, top, right, bottom, base_alpha in strip_defs: - w = tk.Toplevel(root) - w.overrideredirect(True) - w.attributes("-topmost", True) - try: - w.attributes("-toolwindow", True) - except tk.TclError: - pass - w.configure(bg=_FLASH_COLOR) - w.geometry(f"{right - left}x{bottom - top}+{left}+{top}") - try: - w.attributes("-alpha", base_alpha) - except tk.TclError: - pass - windows.append((w, base_alpha)) + hwnd, h_instance = _create_layered_window(class_name, union_left, union_top, width, height) + _user32.ShowWindow(hwnd, _SW_SHOWNA) + _user32.SetWindowPos( + hwnd, + _HWND_TOPMOST, + 0, + 0, + 0, + 0, + _SWP_NOSIZE | _SWP_NOMOVE | _SWP_NOACTIVATE | _SWP_SHOWWINDOW, + ) + + glow_rgba = _render_glow_rgba(width, height, local_rects) logger.info( - "screenshot flash overlay started: %d strip windows for %d rect(s) (full_screen=%s)", - len(windows), + "screenshot flash overlay started: %dx%d layered window at (%d,%d) for %d rect(s)", + width, + height, + union_left, + union_top, len(rects), - full_screen, ) - start = time.perf_counter() - def tick() -> None: - if overlay.stop_event.is_set(): - root.destroy() - return + start = time.perf_counter() + last_intensity_q = -1 + while not overlay.stop_event.is_set(): elapsed_ms = (time.perf_counter() - start) * 1000 if elapsed_ms >= _DURATION_MS: - root.destroy() - return - t_norm = elapsed_ms / _DURATION_MS - if full_screen: - time_alpha = 1.0 - abs(2 * t_norm - 1) - elif t_norm < 0.15: - time_alpha = t_norm / 0.15 - elif t_norm < 0.65: - time_alpha = 1.0 - else: - time_alpha = max(0.0, 1.0 - (t_norm - 0.65) / 0.35) - for w, base_alpha in windows: - try: - w.attributes("-alpha", max(0.05, min(1.0, base_alpha * time_alpha))) - except tk.TclError: - pass - root.after(_FRAME_INTERVAL_MS, tick) - - root.after(0, tick) - root.mainloop() + break + intensity = _intensity_at(elapsed_ms / _DURATION_MS, full_screen) + intensity_q = round(intensity * _INTENSITY_QUANT) + if intensity_q != last_intensity_q: + if intensity < _MIN_VISIBLE_INTENSITY: + bgra = b"\x00" * (width * height * 4) + else: + bgra = _premultiplied_bgra(glow_rgba, intensity) + _push_bitmap(hwnd, union_left, union_top, width, height, bgra) + last_intensity_q = intensity_q + _pump_messages(hwnd) + time.sleep(_FRAME_INTERVAL_MS / 1000) except Exception: logger.debug("screenshot flash overlay failed", exc_info=True) - if root is not None: - try: - root.destroy() - except Exception: - pass finally: + try: + if hwnd: + _user32.DestroyWindow(hwnd) + except Exception: + pass + try: + if h_instance: + _user32.UnregisterClassW(class_name, h_instance) + except Exception: + pass with _lock: global _active_overlay if _active_overlay is overlay: diff --git a/tests/test_flash_overlay.py b/tests/test_flash_overlay.py index 252348d8..b842f121 100644 --- a/tests/test_flash_overlay.py +++ b/tests/test_flash_overlay.py @@ -108,33 +108,43 @@ def test_signals_stop_and_clears_active(self, monkeypatch): assert flash_overlay._active_overlay is None -class TestBuildStripDefs: - def test_region_layer_zero_sits_on_rect_edge_with_full_alpha(self): - defs = flash_overlay._build_strip_defs([(100, 200, 500, 400)], full_screen=False) - # First strip stack is layer 0 (full alpha); 4 strips per layer. - layer0 = [d for d in defs if d[4] == 1.0] - assert len(layer0) == 4 - # Layer 0 is offset outward by 1 * _LAYER_THICKNESS so the inner edge - # of the strip aligns with the rect edge. - thickness = flash_overlay._LAYER_THICKNESS - top, bottom, left, right = layer0 - assert top == (100 - thickness, 200 - thickness, 500 + thickness, 200, 1.0) - assert bottom == (100 - thickness, 400, 500 + thickness, 400 + thickness, 1.0) - - def test_full_screen_strips_inset_inward_from_edge(self): - defs = flash_overlay._build_strip_defs([(0, 0, 1000, 800)], full_screen=True) - layer0 = [d for d in defs if d[4] == 1.0] - assert len(layer0) == 4 - inset = flash_overlay._FULLSCREEN_INSET - thickness = flash_overlay._LAYER_THICKNESS - # Top strip: y range is [inset, inset+thickness] - top = next(d for d in layer0 if d[3] - d[1] == thickness and d[2] - d[0] > thickness) - assert top[1] == inset - assert top[3] == inset + thickness - - def test_too_small_full_screen_rect_produces_no_strips(self): - defs = flash_overlay._build_strip_defs([(0, 0, 4, 4)], full_screen=True) - assert defs == [] +class TestIntensityCurve: + def test_full_screen_is_bell_curve(self): + # Symmetric peak at t=0.5, zero at t=0 and t=1 + assert flash_overlay._intensity_at(0.0, full_screen=True) == 0.0 + assert flash_overlay._intensity_at(0.5, full_screen=True) == 1.0 + assert abs(flash_overlay._intensity_at(1.0, full_screen=True)) < 1e-9 + + def test_region_holds_then_fades(self): + # Pre-peak fade-in + assert flash_overlay._intensity_at(0.0, full_screen=False) == 0.0 + # Held at peak in the middle + assert flash_overlay._intensity_at(0.4, full_screen=False) == 1.0 + # Fading in last segment + assert flash_overlay._intensity_at(1.0, full_screen=False) == 0.0 + + +class TestPremultipliedBgra: + def test_full_intensity_premultiplies_color_by_alpha(self): + from PIL import Image + + # 1×1 pixel: orange-red 50% alpha → premult should be (R*128/255, G*128/255, B*128/255, 128) in BGRA order + img = Image.new("RGBA", (1, 1), (255, 69, 0, 128)) + out = flash_overlay._premultiplied_bgra(img, 1.0) + b, g, r, a = out + assert a == 128 + assert b == 0 + assert g == (69 * 128) // 255 + assert r == (255 * 128) // 255 + + def test_intensity_scales_alpha(self): + from PIL import Image + + img = Image.new("RGBA", (1, 1), (255, 69, 0, 255)) + out = flash_overlay._premultiplied_bgra(img, 0.5) + # Alpha was 255; intensity 0.5 → effective alpha ≈ 127. + _, _, _, a = out + assert 124 <= a <= 128 class TestRunOverlayFallthrough: From b5ce1301447b2a92cc684b2050454664dc3a8d77 Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Mon, 11 May 2026 08:07:01 +0200 Subject: [PATCH 06/13] feat(flash): draw halo ring outside rect for region/window captures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/windows_mcp/desktop/flash_overlay.py | 29 ++++++++++++++++-------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/windows_mcp/desktop/flash_overlay.py b/src/windows_mcp/desktop/flash_overlay.py index ae1aa51b..334658e3 100644 --- a/src/windows_mcp/desktop/flash_overlay.py +++ b/src/windows_mcp/desktop/flash_overlay.py @@ -247,13 +247,17 @@ def _render_glow_rgba( width: int, height: int, rect_list: list[tuple[int, int, int, int]], + *, + outward: bool = True, ) -> "object": """Return a PIL RGBA image with a soft halo ring around each rect. Each rect is in window-local coordinates. A sharp solid border is drawn - on the rect edge, then the layer is gaussian-blurred to spread the - glow, and the sharp ring is composited back on top so the inner edge - stays crisp. + just outside the rect edge (``outward=True``) so the captured area stays + clean and the halo reads as a surround, then the layer is + gaussian-blurred to spread the glow, and the sharp ring is composited + back on top so the inner edge stays crisp. ``outward=False`` nests the + ring inward — used for the full-screen inner halo. """ from PIL import Image, ImageDraw, ImageFilter @@ -262,11 +266,18 @@ def _render_glow_rgba( color = (*_FLASH_RGB, 255) for x1, y1, x2, y2 in rect_list: for i in range(_GLOW_BORDER_THICKNESS): - draw.rectangle( - [x1 + i, y1 + i, x2 - i - 1, y2 - i - 1], - outline=color, - width=1, - ) + if outward: + draw.rectangle( + [x1 - i - 1, y1 - i - 1, x2 + i, y2 + i], + outline=color, + width=1, + ) + else: + draw.rectangle( + [x1 + i, y1 + i, x2 - i - 1, y2 - i - 1], + outline=color, + width=1, + ) blurred = sharp.filter(ImageFilter.GaussianBlur(radius=_GLOW_BLUR_RADIUS)) return Image.alpha_composite(blurred, sharp) @@ -489,7 +500,7 @@ def _run_overlay( _SWP_NOSIZE | _SWP_NOMOVE | _SWP_NOACTIVATE | _SWP_SHOWWINDOW, ) - glow_rgba = _render_glow_rgba(width, height, local_rects) + glow_rgba = _render_glow_rgba(width, height, local_rects, outward=not full_screen) logger.info( "screenshot flash overlay started: %dx%d layered window at (%d,%d) for %d rect(s)", From 94cc55f055ca34b33548e46532168d4dade4ca93 Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Mon, 11 May 2026 09:57:52 +0200 Subject: [PATCH 07/13] feat(flash): thicker halo (8px) and longer duration (3.5s) for visibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/windows_mcp/desktop/flash_overlay.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/windows_mcp/desktop/flash_overlay.py b/src/windows_mcp/desktop/flash_overlay.py index 334658e3..830b6b6a 100644 --- a/src/windows_mcp/desktop/flash_overlay.py +++ b/src/windows_mcp/desktop/flash_overlay.py @@ -21,9 +21,9 @@ logger = logging.getLogger(__name__) _FLASH_RGB = (0xFF, 0x45, 0x00) -_DURATION_MS = 2500 +_DURATION_MS = 3500 _FRAME_INTERVAL_MS = 30 -_GLOW_BORDER_THICKNESS = 4 +_GLOW_BORDER_THICKNESS = 8 _GLOW_BLUR_RADIUS = 14 _GLOW_MARGIN = _GLOW_BLUR_RADIUS * 3 _FULLSCREEN_INSET = 6 From b08ffdd68612eeb98aaa847064cdce8022e68423 Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Mon, 11 May 2026 15:12:09 +0200 Subject: [PATCH 08/13] refactor(flash): own rect resolution and cancel prior overlay atomically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review asks on PR #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) --- src/windows_mcp/desktop/flash_overlay.py | 53 +++++++++++---- src/windows_mcp/desktop/service.py | 19 +----- tests/test_flash_overlay.py | 82 ++++++++++++++++++++++-- 3 files changed, 115 insertions(+), 39 deletions(-) diff --git a/src/windows_mcp/desktop/flash_overlay.py b/src/windows_mcp/desktop/flash_overlay.py index 830b6b6a..831a6717 100644 --- a/src/windows_mcp/desktop/flash_overlay.py +++ b/src/windows_mcp/desktop/flash_overlay.py @@ -58,22 +58,40 @@ def cancel_active_flash(timeout: float = 0.25) -> None: 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 glow around each rect. - - ``rects`` are ``(left, top, right, bottom)`` tuples in virtual-screen - coordinates. ``full_screen=True`` draws an inner halo radiating inward - from each monitor edge; ``full_screen=False`` draws an outer halo - around the captured region. Returns immediately; rendering happens on - a daemon thread. +def show_capture_flash(capture_rect: "object | None" = None) -> None: + """Show a fade-in/out orange-red glow for a screenshot capture. + + Pass the same ``capture_rect`` (``uia.Rect`` with ``left/top/right/ + bottom``) used for the screenshot — or ``None`` for a full-desktop + capture, in which case every monitor gets an inner-edge halo. Returns + immediately; rendering happens on a daemon thread, and any previously + active overlay is cancelled atomically so a follow-up capture never + leaks a stale overlay. """ - if _flash_disabled() or not rects: + if _flash_disabled(): + return + try: + if capture_rect is not None: + rects: list[tuple[int, int, int, int]] = [ + ( + int(capture_rect.left), + int(capture_rect.top), + int(capture_rect.right), + int(capture_rect.bottom), + ) + ] + full_screen = False + else: + import windows_mcp.uia as uia + + monitor_rects = uia.GetMonitorsRect() + rects = [(m.left, m.top, m.right, m.bottom) for m in monitor_rects] + full_screen = True + except Exception: + logger.debug("could not resolve flash overlay rects", exc_info=True) + return + if not rects: return - rects = [tuple(r) for r in rects] overlay = _Overlay() overlay.thread = threading.Thread( target=_run_overlay, @@ -81,9 +99,16 @@ def show_capture_flash( name="windows-mcp-flash", daemon=True, ) + # Atomic swap: save the previous overlay under the lock, install the new + # one, then signal the prior overlay outside the lock so it can tear down + # without blocking new callers. Without this, an overwrite would orphan + # the prior overlay — cancel_active_flash() could no longer reach it. with _lock: global _active_overlay + prev = _active_overlay _active_overlay = overlay + if prev is not None: + prev.stop_event.set() overlay.thread.start() diff --git a/src/windows_mcp/desktop/service.py b/src/windows_mcp/desktop/service.py index d6781ef2..bffa61b7 100755 --- a/src/windows_mcp/desktop/service.py +++ b/src/windows_mcp/desktop/service.py @@ -1056,24 +1056,7 @@ def get_screenshot(self, capture_rect: uia.Rect | None = None) -> Image.Image: flash_overlay.cancel_active_flash() image, used_backend = screenshot_capture.capture(capture_rect) self._last_screenshot_backend = used_backend - try: - if capture_rect is not None: - rects = [ - ( - capture_rect.left, - capture_rect.top, - capture_rect.right, - capture_rect.bottom, - ) - ] - full_screen = False - else: - monitor_rects = uia.GetMonitorsRect() - rects = [(m.left, m.top, m.right, m.bottom) for m in monitor_rects] - full_screen = True - flash_overlay.show_capture_flash(rects, full_screen=full_screen) - except Exception: - logger.debug("could not start screenshot flash overlay", exc_info=True) + flash_overlay.show_capture_flash(capture_rect) return image def get_annotated_screenshot( diff --git a/tests/test_flash_overlay.py b/tests/test_flash_overlay.py index b842f121..eb907947 100644 --- a/tests/test_flash_overlay.py +++ b/tests/test_flash_overlay.py @@ -43,22 +43,35 @@ def test_falsy_values_keep_enabled(self, monkeypatch, value): assert flash_overlay._flash_disabled() is False +class _FakeRect: + """Stand-in for ``uia.Rect`` with the four corner attributes.""" + + def __init__(self, left, top, right, bottom): + self.left = left + self.top = top + self.right = right + self.bottom = bottom + + class TestShowCaptureFlash: def test_disabled_env_var_skips_thread(self, monkeypatch): monkeypatch.setenv("WINDOWS_MCP_DISABLE_FLASH", "1") with patch.object(threading, "Thread") as fake_thread: - flash_overlay.show_capture_flash([(0, 0, 100, 100)], full_screen=False) + flash_overlay.show_capture_flash(_FakeRect(0, 0, 100, 100)) fake_thread.assert_not_called() assert flash_overlay._active_overlay is None - def test_empty_rects_skips_thread(self, monkeypatch): + def test_empty_monitor_rects_skips_thread(self, monkeypatch): + """Full-screen path with zero monitors must not start a thread.""" monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) + fake_uia = type("fake_uia", (), {"GetMonitorsRect": staticmethod(lambda: [])}) + monkeypatch.setitem(sys.modules, "windows_mcp.uia", fake_uia) with patch.object(threading, "Thread") as fake_thread: - flash_overlay.show_capture_flash([], full_screen=False) + flash_overlay.show_capture_flash(None) fake_thread.assert_not_called() assert flash_overlay._active_overlay is None - def test_registers_overlay_and_starts_daemon_thread(self, monkeypatch): + def test_region_capture_passes_single_rect(self, monkeypatch): monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) captured = {} @@ -75,18 +88,73 @@ def start(self): monkeypatch.setattr(flash_overlay.threading, "Thread", _StubThread) - flash_overlay.show_capture_flash([(10, 20, 110, 120)], full_screen=True) + flash_overlay.show_capture_flash(_FakeRect(10, 20, 110, 120)) assert captured["started"] is True assert captured["daemon"] is True assert captured["name"] == "windows-mcp-flash" assert flash_overlay._active_overlay is not None - # rects passed through; full_screen flag forwarded rects_arg, full_screen_arg, overlay_arg = captured["args"] assert rects_arg == [(10, 20, 110, 120)] - assert full_screen_arg is True + assert full_screen_arg is False assert overlay_arg is flash_overlay._active_overlay + def test_full_screen_capture_enumerates_monitors(self, monkeypatch): + """When capture_rect is None the helper must read uia.GetMonitorsRect.""" + monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) + fake_uia = type( + "fake_uia", + (), + { + "GetMonitorsRect": staticmethod( + lambda: [_FakeRect(0, 0, 1920, 1080), _FakeRect(1920, 0, 3840, 1080)] + ) + }, + ) + monkeypatch.setitem(sys.modules, "windows_mcp.uia", fake_uia) + + captured = {} + + class _StubThread: + def __init__(self, target, args, name, daemon): + captured["args"] = args + + def start(self): + pass + + monkeypatch.setattr(flash_overlay.threading, "Thread", _StubThread) + + flash_overlay.show_capture_flash(None) + + rects_arg, full_screen_arg, _ = captured["args"] + assert rects_arg == [(0, 0, 1920, 1080), (1920, 0, 3840, 1080)] + assert full_screen_arg is True + + def test_overlapping_calls_cancel_prior_overlay(self, monkeypatch): + """Second call must signal the prior overlay's stop_event so it can be torn down.""" + monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) + + class _StubThread: + def __init__(self, *a, **kw): + pass + + def start(self): + pass + + monkeypatch.setattr(flash_overlay.threading, "Thread", _StubThread) + + flash_overlay.show_capture_flash(_FakeRect(0, 0, 100, 100)) + first = flash_overlay._active_overlay + assert first is not None + assert not first.stop_event.is_set() + + flash_overlay.show_capture_flash(_FakeRect(0, 0, 100, 100)) + second = flash_overlay._active_overlay + assert second is not None + assert second is not first + # Critical: the prior overlay must be signalled so cancel_active_flash semantics survive + assert first.stop_event.is_set() + class TestCancelActiveFlash: def test_no_op_when_no_active_overlay(self): From 50ad3ecfdfbe9476a946334023df52bec48f6f04 Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Mon, 11 May 2026 15:19:20 +0200 Subject: [PATCH 09/13] test(flash): patch uia on real module to survive cross-suite import order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- tests/test_flash_overlay.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tests/test_flash_overlay.py b/tests/test_flash_overlay.py index eb907947..f3bad33d 100644 --- a/tests/test_flash_overlay.py +++ b/tests/test_flash_overlay.py @@ -64,8 +64,12 @@ def test_disabled_env_var_skips_thread(self, monkeypatch): def test_empty_monitor_rects_skips_thread(self, monkeypatch): """Full-screen path with zero monitors must not start a thread.""" monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) - fake_uia = type("fake_uia", (), {"GetMonitorsRect": staticmethod(lambda: [])}) - monkeypatch.setitem(sys.modules, "windows_mcp.uia", fake_uia) + # Patch on the real uia module — ``import windows_mcp.uia as uia`` resolves + # via the cached parent-package attribute once another test has imported + # the package, so monkeypatching sys.modules is not reliable on its own. + import windows_mcp.uia + + monkeypatch.setattr(windows_mcp.uia, "GetMonitorsRect", lambda: []) with patch.object(threading, "Thread") as fake_thread: flash_overlay.show_capture_flash(None) fake_thread.assert_not_called() @@ -102,16 +106,13 @@ def start(self): def test_full_screen_capture_enumerates_monitors(self, monkeypatch): """When capture_rect is None the helper must read uia.GetMonitorsRect.""" monkeypatch.delenv("WINDOWS_MCP_DISABLE_FLASH", raising=False) - fake_uia = type( - "fake_uia", - (), - { - "GetMonitorsRect": staticmethod( - lambda: [_FakeRect(0, 0, 1920, 1080), _FakeRect(1920, 0, 3840, 1080)] - ) - }, + import windows_mcp.uia + + monkeypatch.setattr( + windows_mcp.uia, + "GetMonitorsRect", + lambda: [_FakeRect(0, 0, 1920, 1080), _FakeRect(1920, 0, 3840, 1080)], ) - monkeypatch.setitem(sys.modules, "windows_mcp.uia", fake_uia) captured = {} From 8f0dfb0eaa4fe0825077de2e2a682a9f484c6bfc Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Fri, 8 May 2026 22:14:47 +0200 Subject: [PATCH 10/13] feat(screenshot): support window_name / window_pid targeting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds optional window_name (fuzzy title match) and window_pid (exact process id) parameters to the Screenshot and Snapshot tools so an agent can capture a single window's bounding rectangle without doing a full-desktop screenshot or guessing a display index. The targeted window is brought to the foreground first by default; pass focus_window=False to skip that. window_name/window_pid and display are mutually exclusive. Implementation: - New module desktop/window_resolver.py: enumerate_visible_windows, resolve_window (PID/exact + name/fuzzy), and get_window_rect using DwmGetWindowAttribute(DWMWA_EXTENDED_FRAME_BOUNDS) with a GetWindowRect fallback so DWM drop-shadow doesn't pollute the rect. - Desktop.resolve_window_capture_rect ties resolver + focus + rect read together and reuses the existing bring_window_to_top logic. - Desktop.get_state now accepts a capture_rect override that takes precedence over display_indices (and rejects passing both). - capture_desktop_state (snapshot helper) plumbs window_name / window_pid / focus_window through, validates mutual exclusion with display, and surfaces the resolved title in the Screenshot/Snapshot metadata as "Target Window: ...". Tests: tests/test_window_resolver.py — 12 cases covering enumeration filtering, PID-vs-name precedence, fuzzy cutoff, and the DWM/ GetWindowRect fallback chain. --- CLAUDE.md | 1 + README.md | 4 +- src/windows_mcp/desktop/service.py | 39 +++++- src/windows_mcp/desktop/window_resolver.py | 115 ++++++++++++++++ src/windows_mcp/tools/_snapshot_helpers.py | 35 +++-- src/windows_mcp/tools/snapshot.py | 29 ++-- tests/test_window_resolver.py | 148 +++++++++++++++++++++ 7 files changed, 352 insertions(+), 19 deletions(-) create mode 100644 src/windows_mcp/desktop/window_resolver.py create mode 100644 tests/test_window_resolver.py diff --git a/CLAUDE.md b/CLAUDE.md index 07c52be6..aaf6f881 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,6 +50,7 @@ The codebase follows a layered service architecture under `src/windows_mcp/`: - Screenshots are capped to 1920x1080 for token efficiency - Mouse/keyboard input uses UIA (same coordinate space as BoundingRectangle; no DPI mismatch) - Screenshot is the preferred fast visual-context tool; Snapshot is the heavier path for UI element ids and DOM extraction +- Both Screenshot and Snapshot accept `window_name` (fuzzy title) or `window_pid` (exact pid) to capture a single window via `desktop/window_resolver.py`; the rect is read with `DwmGetWindowAttribute(DWMWA_EXTENDED_FRAME_BOUNDS)` falling back to `GetWindowRect` - Browser detection (Chrome, Edge, Firefox) triggers special DOM extraction mode in Snapshot - Fuzzy string matching (`thefuzz`) is used for element name matching - UI element fetching has retry logic (`THREAD_MAX_RETRIES=3` in tree service) diff --git a/README.md b/README.md index 0eba34ba..bd2cce5b 100755 --- a/README.md +++ b/README.md @@ -494,8 +494,8 @@ MCP Client can access the following tools to interact with Windows: - `Move`: Move mouse pointer or drag (set drag=True) to coordinates. - `Shortcut`: Press keyboard shortcuts (`Ctrl+c`, `Alt+Tab`, etc). - `Wait`: Pause for a defined duration. -- `Screenshot`: Fast screenshot-first desktop capture with cursor position, active/open windows, and an image. Skips UI tree extraction for speed and should be the default first call when you mainly need visual context. Supports `display=[0]` or `display=[0,1]` to capture specific screens. After capture, a brief orange-red glowing border is drawn over the captured area as a visual confirmation (set `WINDOWS_MCP_DISABLE_FLASH=1` to disable). -- `Snapshot`: Full desktop state capture for workflows that need interactive element ids, scrollable regions, or `use_dom=True` browser extraction. Supports `use_vision=True` for including screenshots and `display=[0]` or `display=[0,1]` for limiting all returned Snapshot information to specific screens. +- `Screenshot`: Fast screenshot-first desktop capture with cursor position, active/open windows, and an image. Skips UI tree extraction for speed and should be the default first call when you mainly need visual context. Supports `display=[0]` or `display=[0,1]` to capture specific screens, or `window_name="..."` (fuzzy title match) / `window_pid=12345` (exact process id) to capture just one window's bounding rectangle. The targeted window is brought to the foreground first unless `focus_window=False`. `window_name` / `window_pid` and `display` are mutually exclusive. After capture, a brief orange-red glowing border is drawn over the captured area as a visual confirmation (set `WINDOWS_MCP_DISABLE_FLASH=1` to disable). +- `Snapshot`: Full desktop state capture for workflows that need interactive element ids, scrollable regions, or `use_dom=True` browser extraction. Supports `use_vision=True` for including screenshots, `display=[0]` or `display=[0,1]` for limiting all returned Snapshot information to specific screens, and `window_name` / `window_pid` for limiting capture to a specific window's bounding rectangle (mutually exclusive with `display`). - `App`: To launch an application from the start menu, resize or move the window and switch between apps. - `Shell`: To execute PowerShell commands. - `Scrape`: To scrape the entire webpage for information. diff --git a/src/windows_mcp/desktop/service.py b/src/windows_mcp/desktop/service.py index bffa61b7..4af74dea 100755 --- a/src/windows_mcp/desktop/service.py +++ b/src/windows_mcp/desktop/service.py @@ -16,6 +16,7 @@ from windows_mcp.tree.service import Tree from windows_mcp.desktop import screenshot as screenshot_capture from windows_mcp.desktop import flash_overlay +from windows_mcp.desktop import window_resolver from locale import getpreferredencoding from contextlib import contextmanager from typing import Literal @@ -92,6 +93,7 @@ def get_state( grid_lines: tuple[int, int] | None = None, display_indices: list[int] | None = None, max_image_size: Size | None = None, + capture_rect: "uia.Rect | None" = None, ) -> DesktopState: use_annotation = use_annotation is True or ( isinstance(use_annotation, str) and use_annotation.lower() == "true" @@ -118,7 +120,10 @@ def get_state( screenshot_capture_ms = 0.0 screenshot_resize_ms = 0.0 state_build_ms = 0.0 - capture_rect = self.get_display_union_rect(display_indices) if display_indices else None + if capture_rect is None and display_indices: + capture_rect = self.get_display_union_rect(display_indices) + elif capture_rect is not None and display_indices: + raise ValueError("capture_rect and display_indices are mutually exclusive") screenshot_region = self._rect_to_bounding_box(capture_rect) if capture_rect else None # Fast path for Screenshot tool (use_ui_tree=False): skip window enumeration. @@ -1052,6 +1057,38 @@ def get_display_union_rect(self, display_indices: list[int]) -> uia.Rect: bottom=max(rect.bottom for rect in selected_rects), ) + def resolve_window_capture_rect( + self, + *, + name: str | None = None, + pid: int | None = None, + focus: bool = True, + ) -> tuple[uia.Rect, str]: + """Resolve a top-level window to a capture rectangle. + + Returns ``(rect, title)``. If ``focus`` is True, the window is brought + to the foreground and unminimized before its rect is read so the + screenshot will show the actual on-screen content. + """ + hwnd, title = window_resolver.resolve_window(name=name, pid=pid) + if focus: + try: + self.bring_window_to_top(hwnd) + except Exception: + logger.debug("bring_window_to_top failed for %s", title, exc_info=True) + window_resolver.restore_if_minimized(hwnd) + sleep(0.05) + elif window_resolver.is_iconic(hwnd): + raise window_resolver.WindowNotFoundError( + f"Window {title!r} is minimized; pass focus_window=True to restore it" + ) + rect = window_resolver.get_window_rect(hwnd) + if rect.isempty(): + raise window_resolver.WindowNotFoundError( + f"Window {title!r} has an empty bounding rectangle" + ) + return rect, title + def get_screenshot(self, capture_rect: uia.Rect | None = None) -> Image.Image: flash_overlay.cancel_active_flash() image, used_backend = screenshot_capture.capture(capture_rect) diff --git a/src/windows_mcp/desktop/window_resolver.py b/src/windows_mcp/desktop/window_resolver.py new file mode 100644 index 00000000..905f3dac --- /dev/null +++ b/src/windows_mcp/desktop/window_resolver.py @@ -0,0 +1,115 @@ +"""Resolve a top-level window by title or PID into a capture rectangle. + +Used by the Screenshot/Snapshot tools to support targeting a specific +window without taking a full-desktop screenshot. The resolver is decoupled +from ``Desktop`` so it can be unit-tested without spinning up UIA. +""" + +import ctypes +import ctypes.wintypes +import logging + +import win32con +import win32gui +import win32process +from fuzzywuzzy import process + +import windows_mcp.uia as uia + +logger = logging.getLogger(__name__) + +DWMWA_EXTENDED_FRAME_BOUNDS = 9 +_FUZZY_SCORE_CUTOFF = 70 + + +class WindowNotFoundError(ValueError): + """Raised when no visible top-level window matches the supplied criteria.""" + + +def enumerate_visible_windows() -> list[tuple[int, str, int]]: + """Return ``(hwnd, title, pid)`` for every visible, non-cloaked top-level window. + + The list is intended for matching, not display, so untitled windows are + included for PID-based lookup. + """ + results: list[tuple[int, str, int]] = [] + + def callback(hwnd: int, _: object) -> bool: + try: + if not win32gui.IsWindow(hwnd) or not win32gui.IsWindowVisible(hwnd): + return True + title = win32gui.GetWindowText(hwnd) + _, pid = win32process.GetWindowThreadProcessId(hwnd) + results.append((hwnd, title, pid)) + except Exception: + pass + return True + + win32gui.EnumWindows(callback, None) + return results + + +def get_window_rect(hwnd: int) -> uia.Rect: + """Return the window's frame rect, preferring DWM extended bounds.""" + rect = ctypes.wintypes.RECT() + try: + hr = ctypes.windll.dwmapi.DwmGetWindowAttribute( + ctypes.wintypes.HWND(hwnd), + ctypes.wintypes.DWORD(DWMWA_EXTENDED_FRAME_BOUNDS), + ctypes.byref(rect), + ctypes.sizeof(rect), + ) + except Exception: + hr = 1 + if hr == 0: + return uia.Rect(rect.left, rect.top, rect.right, rect.bottom) + left, top, right, bottom = win32gui.GetWindowRect(hwnd) + return uia.Rect(left, top, right, bottom) + + +def resolve_window( + *, + name: str | None = None, + pid: int | None = None, + windows: list[tuple[int, str, int]] | None = None, +) -> tuple[int, str]: + """Resolve a window by exact PID or fuzzy title match. + + PID takes precedence when both are given. Returns ``(hwnd, title)``. + Raises :class:`WindowNotFoundError` if nothing matches. + """ + if name is None and pid is None: + raise ValueError("resolve_window requires either name or pid") + + if windows is None: + windows = enumerate_visible_windows() + + if pid is not None: + candidates = [(hwnd, title) for hwnd, title, win_pid in windows if win_pid == pid] + if not candidates: + raise WindowNotFoundError(f"No visible window found for PID {pid}") + # Prefer windows that have a title; fall back to the first match. + candidates.sort(key=lambda t: 0 if t[1] else 1) + return candidates[0] + + titled = [(hwnd, title) for hwnd, title, _ in windows if title] + if not titled: + raise WindowNotFoundError("No titled windows available for name match") + titles = [title for _, title in titled] + match = process.extractOne(name, titles, score_cutoff=_FUZZY_SCORE_CUTOFF) + if match is None: + raise WindowNotFoundError(f"No window title matched {name!r} (score cutoff 70)") + matched_title, _ = match + for hwnd, title in titled: + if title == matched_title: + return hwnd, title + raise WindowNotFoundError(f"No window title matched {name!r}") + + +def is_iconic(hwnd: int) -> bool: + return bool(win32gui.IsIconic(hwnd)) + + +def restore_if_minimized(hwnd: int) -> None: + if is_iconic(hwnd): + win32gui.ShowWindow(hwnd, win32con.SW_RESTORE) diff --git a/src/windows_mcp/tools/_snapshot_helpers.py b/src/windows_mcp/tools/_snapshot_helpers.py index baed9936..0ffa5da4 100644 --- a/src/windows_mcp/tools/_snapshot_helpers.py +++ b/src/windows_mcp/tools/_snapshot_helpers.py @@ -53,6 +53,9 @@ def capture_desktop_state( height_reference_line: int | None, display: list[int] | None, tool_name: str, + window_name: str | None = None, + window_pid: int | None = None, + focus_window: bool = True, ): profile_enabled = _snapshot_profile_enabled() profile_started_at = time.perf_counter() @@ -66,6 +69,17 @@ def capture_desktop_state( display_indices = Desktop.parse_display_selection(display) + capture_rect = None + target_window_title = None + if window_name or window_pid is not None: + if display_indices: + raise ValueError("window_name/window_pid and display are mutually exclusive") + capture_rect, target_window_title = desktop.resolve_window_capture_rect( + name=window_name, + pid=window_pid, + focus=focus_window, + ) + grid_lines = None if width_reference_line and height_reference_line: grid_lines = (int(width_reference_line), int(height_reference_line)) @@ -80,6 +94,7 @@ def capture_desktop_state( grid_lines=grid_lines, display_indices=display_indices, max_image_size=Size(width=MAX_IMAGE_WIDTH, height=MAX_IMAGE_HEIGHT), + capture_rect=capture_rect, ) if profile_enabled: desktop_state_ms = (time.perf_counter() - stage_started_at) * 1000 @@ -128,6 +143,7 @@ def capture_desktop_state( "active_desktop": active_desktop, "all_desktops": all_desktops, "screenshot_bytes": screenshot_bytes, + "target_window_title": target_window_title, } @@ -163,18 +179,21 @@ def build_snapshot_response( " for click, move and other mouse actions)\n" ) if desktop_state.screenshot_region: + metadata_text += f"Screenshot Region: {desktop_state.screenshot_region.xyxy_to_string()}\n" + if desktop_state.screenshot_displays: metadata_text += ( - f"Screenshot Region: {desktop_state.screenshot_region.xyxy_to_string()}\n" + f"Displays: {','.join(str(index) for index in desktop_state.screenshot_displays)}\n" ) - if desktop_state.screenshot_displays: - metadata_text += f"Displays: {','.join(str(index) for index in desktop_state.screenshot_displays)}\n" metadata_text += "Coordinate Space: Virtual desktop coordinates\n" if desktop_state.screenshot_backend: metadata_text += f"Screenshot Backend: {desktop_state.screenshot_backend}\n" + target_window_title = capture_result.get("target_window_title") + if target_window_title: + metadata_text += f"Target Window: {target_window_title}\n" if ui_detail_note: metadata_text += f"{ui_detail_note}\n" - response_text = dedent(f''' + response_text = dedent(f""" {metadata_text} Active Desktop: {active_desktop} @@ -187,14 +206,14 @@ def build_snapshot_response( Opened Windows: {windows} - ''') + """) if include_ui_details: - response_text += dedent(f''' + response_text += dedent(f""" UI Tree: - {semantic_tree or "No elements found."}''') + {semantic_tree or "No elements found."}""") response = [response_text] if screenshot_bytes: - response.append(Image(data=screenshot_bytes, format='png')) + response.append(Image(data=screenshot_bytes, format="png")) return response diff --git a/src/windows_mcp/tools/snapshot.py b/src/windows_mcp/tools/snapshot.py index 9f1a7b2b..a9d16d12 100644 --- a/src/windows_mcp/tools/snapshot.py +++ b/src/windows_mcp/tools/snapshot.py @@ -21,9 +21,10 @@ def register(mcp, *, get_desktop, get_analytics): global state_tool, screenshot_tool + @mcp.tool( - name='Snapshot', - description="Take a screenshot and inspect the screen. Keywords: screenshot, screen capture, see screen, observe, look, inspect, UI elements, what's on screen. Captures complete desktop state including: system language, focused/opened windows, interactive elements (buttons, text fields, links, menus with coordinates), and scrollable areas. Set use_vision=True to include screenshot with cursor highlight. Set use_annotation=False to get a clean screenshot without bounding box overlays on UI elements (default: True, draws colored rectangles around detected elements). Set use_ui_tree=False for a faster screenshot-only snapshot when you do not need interactive or scrollable element extraction. Set width_reference_lines/height_reference_lines to overlay a grid for better spatial reasoning (make sure vision is enabled to use it). Set use_dom=True for browser content to get web page elements instead of browser UI. Set display=[0] or display=[0,1] to limit all returned Snapshot information to specific screens; omit it to keep the default full-desktop behavior. Always call this first to understand the current desktop state before taking actions.", + name="Snapshot", + description="Take a screenshot and inspect the screen. Keywords: screenshot, screen capture, see screen, observe, look, inspect, UI elements, what's on screen. Captures complete desktop state including: system language, focused/opened windows, interactive elements (buttons, text fields, links, menus with coordinates), and scrollable areas. Set use_vision=True to include screenshot with cursor highlight. Set use_annotation=False to get a clean screenshot without bounding box overlays on UI elements (default: True, draws colored rectangles around detected elements). Set use_ui_tree=False for a faster screenshot-only snapshot when you do not need interactive or scrollable element extraction. Set width_reference_lines/height_reference_lines to overlay a grid for better spatial reasoning (make sure vision is enabled to use it). Set use_dom=True for browser content to get web page elements instead of browser UI. Set display=[0] or display=[0,1] to limit all returned Snapshot information to specific screens; omit it to keep the default full-desktop behavior. Set window_name (fuzzy title match) or window_pid (exact process id) to capture only that window's bounding rectangle; the window is brought to the foreground first unless focus_window=False. window_name/window_pid and display are mutually exclusive. Always call this first to understand the current desktop state before taking actions.", annotations=ToolAnnotations( title="Snapshot", readOnlyHint=True, @@ -41,6 +42,9 @@ def _state_tool( width_reference_line: int | None = None, height_reference_line: int | None = None, display: list[int] | None = None, + window_name: str | None = None, + window_pid: int | None = None, + focus_window: bool | str = True, ctx: Context = None, ): try: @@ -54,22 +58,25 @@ def _state_tool( height_reference_line=height_reference_line, display=display, tool_name="Snapshot tool", + window_name=window_name, + window_pid=window_pid, + focus_window=_as_bool(focus_window), ) except Exception as e: logger.warning( "Snapshot failed with display=%s use_vision=%s use_dom=%s", display, - use_vision if 'use_vision' in locals() else None, - use_dom if 'use_dom' in locals() else None, + use_vision if "use_vision" in locals() else None, + use_dom if "use_dom" in locals() else None, exc_info=True, ) - return [f'Error capturing desktop state: {str(e)}. Please try again.'] + return [f"Error capturing desktop state: {str(e)}. Please try again."] return build_snapshot_response(capture_result, include_ui_details=True) @mcp.tool( - name='Screenshot', - description="Captures a fast screenshot-first desktop snapshot with cursor position, desktop/window summaries, and an image. This path skips UI tree extraction for speed. Use Snapshot when you need interactive element ids, scrollable regions, or browser DOM extraction. Note: the returned image may be downscaled for efficiency; when it is, multiply image coordinates by the ratio of original size to displayed size to get the actual screen coordinates for mouse actions (Click, Move, etc.).", + name="Screenshot", + description="Captures a fast screenshot-first desktop snapshot with cursor position, desktop/window summaries, and an image. This path skips UI tree extraction for speed. Use Snapshot when you need interactive element ids, scrollable regions, or browser DOM extraction. Set window_name (fuzzy title match) or window_pid (exact process id) to capture just that window's bounding rectangle; the window is brought to the foreground first unless focus_window=False. window_name/window_pid and display are mutually exclusive. Note: the returned image may be downscaled for efficiency; when it is, multiply image coordinates by the ratio of original size to displayed size to get the actual screen coordinates for mouse actions (Click, Move, etc.).", annotations=ToolAnnotations( title="Screenshot", readOnlyHint=True, @@ -84,6 +91,9 @@ def _screenshot_tool( width_reference_line: int | None = None, height_reference_line: int | None = None, display: list[int] | None = None, + window_name: str | None = None, + window_pid: int | None = None, + focus_window: bool | str = True, ctx: Context = None, ): try: @@ -97,6 +107,9 @@ def _screenshot_tool( height_reference_line=height_reference_line, display=display, tool_name="Screenshot tool", + window_name=window_name, + window_pid=window_pid, + focus_window=_as_bool(focus_window), ) except Exception as e: logger.warning( @@ -104,7 +117,7 @@ def _screenshot_tool( display, exc_info=True, ) - return [f'Error capturing screenshot: {str(e)}. Please try again.'] + return [f"Error capturing screenshot: {str(e)}. Please try again."] return build_snapshot_response( capture_result, diff --git a/tests/test_window_resolver.py b/tests/test_window_resolver.py new file mode 100644 index 00000000..3c96502c --- /dev/null +++ b/tests/test_window_resolver.py @@ -0,0 +1,148 @@ +"""Unit tests for window resolution used by the Screenshot/Snapshot tools.""" + +import ctypes +from unittest.mock import MagicMock + +import pytest + +from windows_mcp.desktop import window_resolver +from windows_mcp.desktop.window_resolver import ( + WindowNotFoundError, + enumerate_visible_windows, + get_window_rect, + resolve_window, +) +from windows_mcp.uia import Rect + + +def _windows(): + return [ + (101, "Notepad - Untitled", 1000), + (202, "Cotire — Columbus Time Reporting", 60972), + (303, "", 60972), + (404, "Visual Studio Code", 5555), + ] + + +class TestEnumerateVisibleWindows: + def test_filters_invisible_and_invalid(self, monkeypatch): + def fake_enum(callback, _): + for hwnd in (1, 2, 3): + callback(hwnd, None) + + monkeypatch.setattr(window_resolver.win32gui, "EnumWindows", fake_enum) + monkeypatch.setattr( + window_resolver.win32gui, + "IsWindow", + lambda hwnd: hwnd != 2, + ) + monkeypatch.setattr( + window_resolver.win32gui, + "IsWindowVisible", + lambda hwnd: hwnd != 3, + ) + monkeypatch.setattr( + window_resolver.win32gui, + "GetWindowText", + lambda hwnd: f"win-{hwnd}", + ) + monkeypatch.setattr( + window_resolver.win32process, + "GetWindowThreadProcessId", + lambda hwnd: (0, hwnd * 10), + ) + + results = enumerate_visible_windows() + + assert results == [(1, "win-1", 10)] + assert 2 not in [r[0] for r in results] + assert 3 not in [r[0] for r in results] + + +class TestResolveWindow: + def test_requires_name_or_pid(self): + with pytest.raises(ValueError, match="name or pid"): + resolve_window(windows=_windows()) + + def test_resolves_by_pid_prefers_titled(self): + hwnd, title = resolve_window(pid=60972, windows=_windows()) + assert hwnd == 202 + assert title.startswith("Cotire") + + def test_resolves_by_pid_returns_first_match_when_no_titled(self): + windows = [ + (1, "", 555), + (2, "", 555), + ] + hwnd, title = resolve_window(pid=555, windows=windows) + assert hwnd == 1 + assert title == "" + + def test_pid_not_found_raises(self): + with pytest.raises(WindowNotFoundError, match="PID 999"): + resolve_window(pid=999, windows=_windows()) + + def test_resolves_by_fuzzy_name(self): + hwnd, title = resolve_window(name="cotire", windows=_windows()) + assert hwnd == 202 + assert title.startswith("Cotire") + + def test_name_not_found_raises(self): + with pytest.raises(WindowNotFoundError, match="cutoff"): + resolve_window(name="zzzzzzzzzzz", windows=_windows()) + + def test_no_titled_windows_raises(self): + only_untitled = [(1, "", 1)] + with pytest.raises(WindowNotFoundError, match="No titled windows"): + resolve_window(name="anything", windows=only_untitled) + + def test_pid_takes_precedence_over_name(self): + hwnd, _ = resolve_window(name="visual studio code", pid=60972, windows=_windows()) + assert hwnd == 202 + + +class TestGetWindowRect: + def test_uses_dwm_when_call_succeeds(self, monkeypatch): + captured = {} + + def fake_dwm(hwnd, attr, rect_ptr, size): + captured["hwnd"] = hwnd.value + r = ctypes.cast(rect_ptr, ctypes.POINTER(ctypes.wintypes.RECT)).contents + r.left, r.top, r.right, r.bottom = 100, 200, 600, 700 + return 0 + + fake_dwmapi = MagicMock() + fake_dwmapi.DwmGetWindowAttribute.side_effect = fake_dwm + monkeypatch.setattr(ctypes, "windll", MagicMock(dwmapi=fake_dwmapi)) + + rect = get_window_rect(12345) + + assert isinstance(rect, Rect) + assert (rect.left, rect.top, rect.right, rect.bottom) == (100, 200, 600, 700) + assert captured["hwnd"] == 12345 + + def test_falls_back_to_get_window_rect_when_dwm_fails(self, monkeypatch): + fake_dwmapi = MagicMock() + fake_dwmapi.DwmGetWindowAttribute.return_value = 1 # nonzero HRESULT + monkeypatch.setattr(ctypes, "windll", MagicMock(dwmapi=fake_dwmapi)) + monkeypatch.setattr( + window_resolver.win32gui, + "GetWindowRect", + lambda hwnd: (10, 20, 30, 40), + ) + + rect = get_window_rect(99) + assert (rect.left, rect.top, rect.right, rect.bottom) == (10, 20, 30, 40) + + def test_falls_back_when_dwm_raises(self, monkeypatch): + fake_dwmapi = MagicMock() + fake_dwmapi.DwmGetWindowAttribute.side_effect = OSError("oops") + monkeypatch.setattr(ctypes, "windll", MagicMock(dwmapi=fake_dwmapi)) + monkeypatch.setattr( + window_resolver.win32gui, + "GetWindowRect", + lambda hwnd: (1, 2, 3, 4), + ) + + rect = get_window_rect(1) + assert (rect.left, rect.top, rect.right, rect.bottom) == (1, 2, 3, 4) From 9dac51de7aecfab6049d5d70e161179d587fed3e Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Fri, 8 May 2026 22:32:31 +0200 Subject: [PATCH 11/13] feat(screenshot): refuse to capture an unfocused targeted window When window_name/window_pid is supplied with focus_window=False, the resolver now refuses unless the target is actually the foreground window. Otherwise the screenshot would just capture whatever happened to be on top of it, which is misleading. - focus_window=True (default): after bring_window_to_top, verify the window became foreground and log a warning if it didn't (e.g. when Windows blocks SetForegroundWindow for an elevated/UIPI target). - focus_window=False: raise WindowNotFoundError with a clear message if the target is not foreground, in addition to the existing minimized check. Adds is_foreground(hwnd) helper in window_resolver.py and three tests for it. --- src/windows_mcp/desktop/service.py | 24 +++++++++++++++++----- src/windows_mcp/desktop/window_resolver.py | 8 ++++++++ tests/test_window_resolver.py | 17 +++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/windows_mcp/desktop/service.py b/src/windows_mcp/desktop/service.py index 4af74dea..5143e344 100755 --- a/src/windows_mcp/desktop/service.py +++ b/src/windows_mcp/desktop/service.py @@ -1077,11 +1077,25 @@ def resolve_window_capture_rect( except Exception: logger.debug("bring_window_to_top failed for %s", title, exc_info=True) window_resolver.restore_if_minimized(hwnd) - sleep(0.05) - elif window_resolver.is_iconic(hwnd): - raise window_resolver.WindowNotFoundError( - f"Window {title!r} is minimized; pass focus_window=True to restore it" - ) + sleep(0.1) + if not window_resolver.is_foreground(hwnd): + logger.warning( + "Window %r did not become foreground after focus attempt; " + "screenshot may be obscured by another window", + title, + ) + else: + if window_resolver.is_iconic(hwnd): + raise window_resolver.WindowNotFoundError( + f"Window {title!r} is minimized; pass focus_window=True to restore it" + ) + if not window_resolver.is_foreground(hwnd): + raise window_resolver.WindowNotFoundError( + f"Window {title!r} is not the foreground window; " + "screenshot would capture whatever is on top instead. " + "Pass focus_window=True (default) to bring it forward, " + "or focus the window manually first." + ) rect = window_resolver.get_window_rect(hwnd) if rect.isempty(): raise window_resolver.WindowNotFoundError( diff --git a/src/windows_mcp/desktop/window_resolver.py b/src/windows_mcp/desktop/window_resolver.py index 905f3dac..0204ec88 100644 --- a/src/windows_mcp/desktop/window_resolver.py +++ b/src/windows_mcp/desktop/window_resolver.py @@ -110,6 +110,14 @@ def is_iconic(hwnd: int) -> bool: return bool(win32gui.IsIconic(hwnd)) +def is_foreground(hwnd: int) -> bool: + """True if ``hwnd`` is currently the system foreground window.""" + try: + return win32gui.GetForegroundWindow() == hwnd + except Exception: + return False + + def restore_if_minimized(hwnd: int) -> None: if is_iconic(hwnd): win32gui.ShowWindow(hwnd, win32con.SW_RESTORE) diff --git a/tests/test_window_resolver.py b/tests/test_window_resolver.py index 3c96502c..c3784772 100644 --- a/tests/test_window_resolver.py +++ b/tests/test_window_resolver.py @@ -101,6 +101,23 @@ def test_pid_takes_precedence_over_name(self): assert hwnd == 202 +class TestIsForeground: + def test_true_when_foreground_handle_matches(self, monkeypatch): + monkeypatch.setattr(window_resolver.win32gui, "GetForegroundWindow", lambda: 4242) + assert window_resolver.is_foreground(4242) is True + + def test_false_when_foreground_handle_differs(self, monkeypatch): + monkeypatch.setattr(window_resolver.win32gui, "GetForegroundWindow", lambda: 999) + assert window_resolver.is_foreground(4242) is False + + def test_false_when_call_raises(self, monkeypatch): + def boom(): + raise OSError("denied") + + monkeypatch.setattr(window_resolver.win32gui, "GetForegroundWindow", boom) + assert window_resolver.is_foreground(1) is False + + class TestGetWindowRect: def test_uses_dwm_when_call_succeeds(self, monkeypatch): captured = {} From 957174ef7a5795378da1c85205a4741a9b8b635a Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Fri, 8 May 2026 23:14:10 +0200 Subject: [PATCH 12/13] fix(screenshot): fall back to SwitchToThisWindow when SetForegroundWindow is blocked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bring_window_to_top relies on SetForegroundWindow, which Windows silently rejects when the calling process didn't own the last input event — even after the AttachThreadInput dance. The targeted window is raised in z-order but the active foreground window stays put, so the screenshot grabs whichever app the user happens to be looking at instead. When the post-focus is_foreground check fails, retry once with SwitchToThisWindow (the undocumented Win32 API the shell uses for Alt-Tab), which bypasses the foreground lock without injecting keyboard input. If that still doesn't work, log the existing warning. Tests cover the new force_foreground helper: it calls SwitchToThisWindow with (hwnd, True) and silently swallows OSError from misbehaving drivers. --- src/windows_mcp/desktop/service.py | 3 +++ src/windows_mcp/desktop/window_resolver.py | 17 +++++++++++++++++ tests/test_window_resolver.py | 18 ++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/src/windows_mcp/desktop/service.py b/src/windows_mcp/desktop/service.py index 5143e344..a7deaaba 100755 --- a/src/windows_mcp/desktop/service.py +++ b/src/windows_mcp/desktop/service.py @@ -1078,6 +1078,9 @@ def resolve_window_capture_rect( logger.debug("bring_window_to_top failed for %s", title, exc_info=True) window_resolver.restore_if_minimized(hwnd) sleep(0.1) + if not window_resolver.is_foreground(hwnd): + window_resolver.force_foreground(hwnd) + sleep(0.1) if not window_resolver.is_foreground(hwnd): logger.warning( "Window %r did not become foreground after focus attempt; " diff --git a/src/windows_mcp/desktop/window_resolver.py b/src/windows_mcp/desktop/window_resolver.py index 0204ec88..4beb71b6 100644 --- a/src/windows_mcp/desktop/window_resolver.py +++ b/src/windows_mcp/desktop/window_resolver.py @@ -121,3 +121,20 @@ def is_foreground(hwnd: int) -> bool: def restore_if_minimized(hwnd: int) -> None: if is_iconic(hwnd): win32gui.ShowWindow(hwnd, win32con.SW_RESTORE) + + +def force_foreground(hwnd: int) -> None: + """Last-resort focus attempt via SwitchToThisWindow. + + ``SetForegroundWindow`` is silently rejected when the calling process + didn't receive the last input event (Windows foreground lock), even + after the AttachThreadInput dance. ``SwitchToThisWindow`` is the + undocumented Win32 API the shell uses for Alt-Tab; it works around the + lock without injecting keyboard input. + """ + try: + ctypes.windll.user32.SwitchToThisWindow( + ctypes.wintypes.HWND(hwnd), ctypes.wintypes.BOOL(True) + ) + except Exception: + logger.debug("SwitchToThisWindow failed for hwnd %s", hwnd, exc_info=True) diff --git a/tests/test_window_resolver.py b/tests/test_window_resolver.py index c3784772..b25ada79 100644 --- a/tests/test_window_resolver.py +++ b/tests/test_window_resolver.py @@ -118,6 +118,24 @@ def boom(): assert window_resolver.is_foreground(1) is False +class TestForceForeground: + def test_invokes_switch_to_this_window(self, monkeypatch): + calls: list[tuple] = [] + fake_user32 = MagicMock() + fake_user32.SwitchToThisWindow.side_effect = lambda hwnd, fAlt: calls.append( + (hwnd.value, fAlt.value) + ) + monkeypatch.setattr(ctypes, "windll", MagicMock(user32=fake_user32)) + window_resolver.force_foreground(7777) + assert calls and calls[0] == (7777, True) + + def test_swallows_exception(self, monkeypatch): + fake_user32 = MagicMock() + fake_user32.SwitchToThisWindow.side_effect = OSError("nope") + monkeypatch.setattr(ctypes, "windll", MagicMock(user32=fake_user32)) + window_resolver.force_foreground(1) + + class TestGetWindowRect: def test_uses_dwm_when_call_succeeds(self, monkeypatch): captured = {} From eba890d57aa45f64f188073eefdcf7f9e22891cb Mon Sep 17 00:00:00 2001 From: Alex Schick Date: Mon, 11 May 2026 15:15:08 +0200 Subject: [PATCH 13/13] fix(screenshot): raise when targeted window cannot be focused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo review on PR #233 pointed out that resolve_window_capture_rect relied on catching exceptions from bring_window_to_top, but bring_window_to_top swallows its own exceptions and only logs — so the try/except was never a reliable failure signal. f4a8ff5 added an explicit is_foreground post-condition check plus a SwitchToThisWindow fallback, but on final failure it only warned and then captured whatever was on top anyway — exactly the silent-wrong- content case Qodo flagged. When focus_window=True and both bring_window_to_top + force_foreground fail to bring the target forward, raise WindowNotFoundError with a message that tells the user to focus it manually or pass focus_window=False. focus_window=False still bypasses this path. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/windows_mcp/desktop/service.py | 14 +++++-- tests/test_window_resolver.py | 63 ++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/windows_mcp/desktop/service.py b/src/windows_mcp/desktop/service.py index a7deaaba..cde51840 100755 --- a/src/windows_mcp/desktop/service.py +++ b/src/windows_mcp/desktop/service.py @@ -1082,10 +1082,16 @@ def resolve_window_capture_rect( window_resolver.force_foreground(hwnd) sleep(0.1) if not window_resolver.is_foreground(hwnd): - logger.warning( - "Window %r did not become foreground after focus attempt; " - "screenshot may be obscured by another window", - title, + # bring_window_to_top swallows its own exceptions, so we can't rely on + # them as a failure signal. The explicit foreground check above is the + # only reliable signal — refuse the capture rather than silently shoot + # whatever happens to be on top. The user can pass focus_window=False + # to accept that risk explicitly. + raise window_resolver.WindowNotFoundError( + f"Could not bring window {title!r} to the foreground after multiple " + "attempts (likely blocked by elevation or another foreground-locking " + "process). Focus it manually and retry, or pass focus_window=False to " + "capture its current rect even though it may be obscured." ) else: if window_resolver.is_iconic(hwnd): diff --git a/tests/test_window_resolver.py b/tests/test_window_resolver.py index b25ada79..ae326a7b 100644 --- a/tests/test_window_resolver.py +++ b/tests/test_window_resolver.py @@ -181,3 +181,66 @@ def test_falls_back_when_dwm_raises(self, monkeypatch): rect = get_window_rect(1) assert (rect.left, rect.top, rect.right, rect.bottom) == (1, 2, 3, 4) + + +class TestResolveWindowCaptureRectFocusFailure: + """Regression test for PR #233 review feedback: when focus=True and the + post-condition foreground check fails for both bring_window_to_top and + force_foreground, the method must raise instead of capturing wrong content. + bring_window_to_top swallows its own exceptions, so the try/except path is + not a reliable failure signal — only the explicit ``is_foreground`` check is. + """ + + def test_raises_when_window_never_becomes_foreground(self, monkeypatch): + from windows_mcp.desktop.service import Desktop + + # Stand up a Desktop without running its full __init__ (Tree/UIA setup + # would otherwise pull in heavy Windows COM state in a unit test). + desktop = Desktop.__new__(Desktop) + + # Stub Desktop.bring_window_to_top so it neither raises nor focuses + # — same as real-world failure (it logs and returns). + monkeypatch.setattr(Desktop, "bring_window_to_top", lambda self, hwnd: None) + + # Resolver returns a fake (hwnd, title). + monkeypatch.setattr( + window_resolver, "resolve_window", lambda name=None, pid=None: (12345, "Stub Title") + ) + # is_foreground always returns False — focus attempt "fails". + monkeypatch.setattr(window_resolver, "is_foreground", lambda hwnd: False) + # force_foreground does nothing (also a no-op). + monkeypatch.setattr(window_resolver, "force_foreground", lambda hwnd: None) + # Make sleep a no-op so the test is fast. + monkeypatch.setattr("windows_mcp.desktop.service.sleep", lambda s: None) + + with pytest.raises(WindowNotFoundError) as excinfo: + desktop.resolve_window_capture_rect(name="Stub") + assert "foreground" in str(excinfo.value).lower() + assert "focus_window=False" in str(excinfo.value) + + def test_returns_rect_when_force_foreground_succeeds(self, monkeypatch): + from windows_mcp.desktop.service import Desktop + + desktop = Desktop.__new__(Desktop) + + monkeypatch.setattr(Desktop, "bring_window_to_top", lambda self, hwnd: None) + monkeypatch.setattr( + window_resolver, "resolve_window", lambda name=None, pid=None: (12345, "Stub Title") + ) + # First check fails, second (after force_foreground) succeeds. + calls = {"is_foreground": 0} + + def fake_is_foreground(hwnd): + calls["is_foreground"] += 1 + return calls["is_foreground"] >= 2 + + monkeypatch.setattr(window_resolver, "is_foreground", fake_is_foreground) + monkeypatch.setattr(window_resolver, "force_foreground", lambda hwnd: None) + monkeypatch.setattr("windows_mcp.desktop.service.sleep", lambda s: None) + monkeypatch.setattr( + window_resolver, "get_window_rect", lambda hwnd: Rect(10, 20, 110, 120) + ) + + rect, title = desktop.resolve_window_capture_rect(name="Stub") + assert title == "Stub Title" + assert (rect.left, rect.top, rect.right, rect.bottom) == (10, 20, 110, 120)