-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add GUI screenshot support with headless X11 capture #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3cd658b
e2defe4
d7e63a3
3fadc91
a83e49e
1ef4e5b
87016dd
2255348
1341ce8
007c6fc
ddc96bc
c79ef38
5ba4c78
d4d62bb
71fc21a
94daca0
c0317fd
9234954
33a19e1
3043345
b9a1579
2e01c66
9133182
6583bf5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -123,6 +123,11 @@ Once configured, the following tools are available: | |||||||
| - `get_session_metrics` - Get performance metrics | ||||||||
| - `list_report_images` - List ORFS report directory images | ||||||||
| - `read_report_image` - Read a ORFS report image | ||||||||
| - `gui_screenshot` - Capture a screenshot from the OpenROAD GUI (headless via Xvfb) | ||||||||
|
|
||||||||
| ### Headless GUI Support | ||||||||
|
|
||||||||
| The `gui_screenshot` tool launches OpenROAD's GUI under a persistent **Xvfb** virtual display and captures the screen using ImageMagick's `import -window root` command — no physical display is required. When called without a `session_id` the tool automatically starts Xvfb, launches `openroad -gui -no_init` on that display, waits for the GUI window to render, and returns the screenshot as base64-encoded JPEG by default (configurable to PNG or WebP). You can reuse a previously created GUI session by passing its `session_id` (only sessions created by `gui_screenshot` can be reused). This feature requires `xvfb` and ImageMagick to be installed (`apt-get install -y xvfb imagemagick`) and is pre-configured in the Docker test image. | ||||||||
|
Comment on lines
+128
to
+130
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This should be removed - would like to keep README lean.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright @luarss . |
||||||||
|
|
||||||||
| ## Troubleshooting | ||||||||
|
|
||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| from pydantic import BaseModel, Field | ||
| from pydantic import BaseModel, Field, model_validator | ||
|
|
||
|
|
||
| class Settings(BaseModel): | ||
|
|
@@ -41,6 +41,98 @@ class Settings(BaseModel): | |
| description="Enable command validation to prevent command injection", | ||
| ) | ||
|
|
||
| # GUI screenshot settings | ||
| GUI_DISPLAY_RESOLUTION: str = Field( | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate please |
||
| default="1280x1024x24", | ||
| description="Default Xvfb virtual display resolution (WxHxDepth)", | ||
| ) | ||
| GUI_CAPTURE_TIMEOUT_MS: int = Field( | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Timeout could potentially be a constant? |
||
| default=8000, | ||
| description="Timeout in milliseconds for the screenshot capture", | ||
| ) | ||
| GUI_MAX_SCREENSHOT_SIZE_MB: int = Field( | ||
| default=50, | ||
| description="Maximum allowed screenshot file size in megabytes", | ||
| ) | ||
| GUI_IMPORT_TIMEOUT_S: float = Field( | ||
| default=15.0, | ||
| description="Timeout in seconds for the ImageMagick import subprocess", | ||
| ) | ||
| GUI_DISPLAY_START: int = Field( | ||
| default=42, | ||
| description="Start of the X11 display number range for Xvfb", | ||
| ) | ||
| GUI_DISPLAY_END: int = Field( | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need a start and end? Don't you just need one? |
||
| default=100, | ||
| description="End (exclusive) of the X11 display number range for Xvfb", | ||
| ) | ||
| GUI_STARTUP_TIMEOUT_S: float = Field( | ||
| default=15.0, | ||
| description="Maximum seconds to wait for the GUI to become ready", | ||
| ) | ||
| GUI_STARTUP_POLL_INTERVAL_S: float = Field( | ||
| default=0.5, | ||
| description="Seconds between xdpyinfo readiness polls during GUI startup", | ||
| ) | ||
| GUI_APP_READY_TIMEOUT_S: float = Field( | ||
| default=15.0, | ||
| description="Max seconds to wait for the OpenROAD GUI window to render after Xvfb is ready", | ||
| ) | ||
| GUI_APP_READY_POLL_INTERVAL_S: float = Field( | ||
| default=0.5, | ||
| description="Polling interval (seconds) when waiting for GUI application window", | ||
| ) | ||
| GUI_DEFAULT_IMAGE_FORMAT: str = Field( | ||
| default="jpeg", | ||
| description="Default image format for screenshots ('png', 'jpeg', or 'webp')", | ||
| ) | ||
| GUI_DEFAULT_JPEG_QUALITY: int = Field( | ||
| default=85, | ||
| description="Default JPEG/WebP quality (1-100). Lower = smaller file, more artifacts", | ||
| ) | ||
| GUI_PREVIEW_SIZE_PX: int = Field( | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need a preview?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @luarss preview is not important that much for mcp , just added if we want to use it later. |
||
| default=256, | ||
| description="Maximum dimension (px) of the longest side for preview thumbnails", | ||
| ) | ||
| GUI_XVFB_SETTLE_S: float = Field( | ||
| default=1.0, | ||
| description="Seconds to wait after starting Xvfb before checking it is alive", | ||
| ) | ||
| GUI_SUBPROCESS_TIMEOUT_S: float = Field( | ||
| default=3.0, | ||
| description="Timeout (seconds) for short-lived helper subprocesses (xdpyinfo, xwininfo)", | ||
| ) | ||
| GUI_DISPLAY_FALLBACK_RANGE: int = Field( | ||
| default=200, | ||
| description="Random range added to GUI_DISPLAY_START when no free display is found", | ||
| ) | ||
| GUI_ERROR_TRUNCATE_CHARS: int = Field( | ||
| default=500, | ||
| description="Max characters of stderr kept in error messages", | ||
| ) | ||
| GUI_TEMP_UUID_LENGTH: int = Field( | ||
| default=12, | ||
| description="Hex characters from UUID used in temporary screenshot filenames", | ||
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_display_range(self) -> "Settings": | ||
| if self.GUI_DISPLAY_START >= self.GUI_DISPLAY_END: | ||
| raise ValueError( | ||
| f"GUI_DISPLAY_START ({self.GUI_DISPLAY_START}) must be less than " | ||
| f"GUI_DISPLAY_END ({self.GUI_DISPLAY_END})" | ||
| ) | ||
| if self.GUI_DISPLAY_FALLBACK_RANGE <= 0: | ||
| raise ValueError( | ||
| "GUI_DISPLAY_FALLBACK_RANGE must be greater than 0 " | ||
| "(used as modulo divisor in _find_free_display fallback)" | ||
| ) | ||
| if self.GUI_STARTUP_POLL_INTERVAL_S <= 0: | ||
| raise ValueError("GUI_STARTUP_POLL_INTERVAL_S must be greater than 0") | ||
| if self.GUI_APP_READY_POLL_INTERVAL_S <= 0: | ||
| raise ValueError("GUI_APP_READY_POLL_INTERVAL_S must be greater than 0") | ||
| return self | ||
|
Pavankumar07s marked this conversation as resolved.
|
||
|
|
||
| # ORFS integration settings | ||
| ORFS_FLOW_PATH: str = Field( | ||
| default=os.path.expanduser("~/OpenROAD-flow-scripts/flow"), | ||
|
|
@@ -83,6 +175,24 @@ def from_env(cls) -> "Settings": | |
| "READ_CHUNK_SIZE": ("OPENROAD_READ_CHUNK_SIZE", int), | ||
| "LOG_LEVEL": ("LOG_LEVEL", str), | ||
| "LOG_FORMAT": ("LOG_FORMAT", str), | ||
| "GUI_DISPLAY_RESOLUTION": ("OPENROAD_GUI_DISPLAY_RESOLUTION", str), | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need all these config variables? There's like almost 20 of them. |
||
| "GUI_CAPTURE_TIMEOUT_MS": ("OPENROAD_GUI_CAPTURE_TIMEOUT_MS", int), | ||
| "GUI_MAX_SCREENSHOT_SIZE_MB": ("OPENROAD_GUI_MAX_SCREENSHOT_SIZE_MB", int), | ||
| "GUI_IMPORT_TIMEOUT_S": ("OPENROAD_GUI_IMPORT_TIMEOUT_S", float), | ||
| "GUI_DISPLAY_START": ("OPENROAD_GUI_DISPLAY_START", int), | ||
| "GUI_DISPLAY_END": ("OPENROAD_GUI_DISPLAY_END", int), | ||
| "GUI_STARTUP_TIMEOUT_S": ("OPENROAD_GUI_STARTUP_TIMEOUT_S", float), | ||
| "GUI_STARTUP_POLL_INTERVAL_S": ("OPENROAD_GUI_STARTUP_POLL_INTERVAL_S", float), | ||
| "GUI_APP_READY_TIMEOUT_S": ("OPENROAD_GUI_APP_READY_TIMEOUT_S", float), | ||
| "GUI_APP_READY_POLL_INTERVAL_S": ("OPENROAD_GUI_APP_READY_POLL_INTERVAL_S", float), | ||
| "GUI_DEFAULT_IMAGE_FORMAT": ("OPENROAD_GUI_DEFAULT_IMAGE_FORMAT", str), | ||
| "GUI_DEFAULT_JPEG_QUALITY": ("OPENROAD_GUI_DEFAULT_JPEG_QUALITY", int), | ||
| "GUI_PREVIEW_SIZE_PX": ("OPENROAD_GUI_PREVIEW_SIZE_PX", int), | ||
| "GUI_XVFB_SETTLE_S": ("OPENROAD_GUI_XVFB_SETTLE_S", float), | ||
| "GUI_SUBPROCESS_TIMEOUT_S": ("OPENROAD_GUI_SUBPROCESS_TIMEOUT_S", float), | ||
| "GUI_DISPLAY_FALLBACK_RANGE": ("OPENROAD_GUI_DISPLAY_FALLBACK_RANGE", int), | ||
| "GUI_ERROR_TRUNCATE_CHARS": ("OPENROAD_GUI_ERROR_TRUNCATE_CHARS", int), | ||
| "GUI_TEMP_UUID_LENGTH": ("OPENROAD_GUI_TEMP_UUID_LENGTH", int), | ||
| "ORFS_FLOW_PATH": ("ORFS_FLOW_PATH", str), | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,15 @@ | ||
| """Main MCP server setup and tool registration.""" | ||
|
|
||
| import asyncio | ||
| from typing import Annotated | ||
|
|
||
| from fastmcp import FastMCP | ||
| from pydantic import Field | ||
|
|
||
| from openroad_mcp.config.cli import CLIConfig | ||
|
|
||
| from .core.manager import OpenROADManager | ||
| from .tools.gui import GuiScreenshotTool | ||
| from .tools.interactive import ( | ||
| CreateSessionTool, | ||
| InspectSessionTool, | ||
|
|
@@ -41,6 +44,9 @@ | |
| list_report_images_tool = ListReportImagesTool(manager) | ||
| read_report_image_tool = ReadReportImageTool(manager) | ||
|
|
||
| # Initialize GUI tool instances | ||
|
luarss marked this conversation as resolved.
|
||
| gui_screenshot_tool = GuiScreenshotTool(manager) | ||
|
|
||
|
|
||
| # Interactive session tools | ||
| @mcp.tool() | ||
|
|
@@ -103,11 +109,119 @@ async def read_report_image(platform: str, design: str, run_slug: str, image_nam | |
| return await read_report_image_tool.execute(platform, design, run_slug, image_name) | ||
|
|
||
|
|
||
| # GUI tools | ||
| @mcp.tool() | ||
| async def gui_screenshot( | ||
| session_id: Annotated[ | ||
| str, | ||
| Field(description="Existing GUI session ID to reuse. Leave empty to auto-create a new headless session."), | ||
| ] = "", | ||
| resolution: Annotated[ | ||
| str, | ||
| Field(description="Virtual display resolution, e.g. '1920x1080x24'. Defaults to '1280x1024x24'."), | ||
| ] = "", | ||
| output_path: Annotated[ | ||
| str, | ||
| Field(description="File path to save the screenshot on disk. A temp file is used when omitted."), | ||
| ] = "", | ||
| timeout_ms: Annotated[ | ||
| str, | ||
| Field(description="Timeout in milliseconds for the screenshot capture. Defaults to 8000."), | ||
| ] = "", | ||
| image_format: Annotated[ | ||
| str, | ||
| Field(description="Output format: 'png', 'jpeg', or 'webp'. Defaults to 'jpeg' (smaller, saves tokens)."), | ||
| ] = "", | ||
| quality: Annotated[ | ||
| str, | ||
| Field(description="Compression quality for JPEG/WebP (1-100). Ignored for PNG. Defaults to 85."), | ||
| ] = "", | ||
| scale: Annotated[ | ||
| str, | ||
| Field(description="Downscale factor (0.0-1.0]. 0.5 = half size. Defaults to 1.0 (no scaling)."), | ||
| ] = "", | ||
| crop: Annotated[ | ||
| str, | ||
| Field( | ||
| description=( | ||
| "Pixel region to crop: 'x0,y0,x1,y1' or 'x0 y0 x1 y1'. " | ||
| "Applied before scaling. Leave empty for full image." | ||
| ) | ||
| ), | ||
| ] = "", | ||
| return_mode: Annotated[ | ||
| str, | ||
| Field( | ||
| description=( | ||
| "How to return the result: " | ||
| "'base64' (full image, default), " | ||
| "'path' (file path only, saves tokens), " | ||
| "'preview' (256px thumbnail + file path)." | ||
| ) | ||
| ), | ||
| ] = "", | ||
| ) -> str: | ||
| """Capture a screenshot of the OpenROAD GUI running in a headless display. | ||
|
|
||
| Auto-creates a session if session_id is not provided. Use return_mode='path' | ||
| or 'preview' to save tokens. JPEG with quality=60-85 reduces size by 70-90%. | ||
| """ | ||
|
|
||
| # Normalise inputs: empty strings → None so execute() applies defaults. | ||
| def _clean(v: str) -> str | None: | ||
| v = str(v).strip() | ||
| return v if v else None | ||
|
|
||
| def _int(v: str, name: str = "parameter") -> int | None: | ||
| v = str(v).strip() | ||
| if not v: | ||
| return None | ||
| try: | ||
| return int(v) | ||
| except ValueError: | ||
| raise ValueError(f"Invalid integer value for {name}: '{v}'") from None | ||
|
|
||
| def _float(v: str, name: str = "parameter") -> float | None: | ||
| v = str(v).strip() | ||
| if not v: | ||
| return None | ||
| try: | ||
| return float(v) | ||
| except ValueError: | ||
| raise ValueError(f"Invalid numeric value for {name}: '{v}'") from None | ||
|
Comment on lines
+171
to
+191
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should not have to validate tools inside this scope. It should be consolidated higher up in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure @luarss |
||
|
|
||
| try: | ||
| return await gui_screenshot_tool.execute( | ||
| session_id=_clean(session_id), | ||
| resolution=_clean(resolution), | ||
| output_path=_clean(output_path), | ||
| timeout_ms=_int(timeout_ms, "timeout_ms"), | ||
| image_format=_clean(image_format), | ||
| quality=_int(quality, "quality"), | ||
| scale=_float(scale, "scale"), | ||
| crop=_clean(crop), | ||
| return_mode=_clean(return_mode), | ||
| ) | ||
| except ValueError as e: | ||
| import json | ||
|
|
||
| from .core.models import GuiScreenshotResult | ||
|
|
||
| return json.dumps( | ||
| GuiScreenshotResult(error="InvalidParameter", message=str(e)).model_dump(), | ||
| indent=2, | ||
| ) | ||
|
|
||
|
|
||
| async def shutdown_openroad() -> None: | ||
| """Gracefully shutdown interactive OpenROAD sessions.""" | ||
| """Gracefully shutdown interactive OpenROAD sessions and GUI displays.""" | ||
| try: | ||
| logger.info("Initiating graceful shutdown of OpenROAD services...") | ||
|
|
||
| # Clean up any Xvfb displays managed by the GUI tool | ||
| for sid in list(gui_screenshot_tool._session_displays): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typically - internal attributes (prefixed by underscore) should not be accessed externally. |
||
| gui_screenshot_tool.cleanup_display(sid) | ||
|
|
||
| await manager.cleanup_all() | ||
|
|
||
| logger.info("OpenROAD services shutdown completed successfully") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this dep is not going to be changed often we should put it earlier to benefit from layer caching
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes @luarss great idea, we should change it inorder to get benefits from layer caching.