Repository navigation
DYN-10845: Fix Dynamo window restoring off-screen when saved monitor is unavailable - #17321
johnpierson wants to merge 5 commits into
Conversation
Validate the saved window bounds against the working area of each connected display on startup instead of summing monitor widths/heights. Restore the saved position only when it overlaps a live monitor by a usable amount; otherwise recenter on the primary monitor. Handles negative coordinates and disconnected/rearranged multi-monitor layouts. Related: DynamoDS/DynamoRevit#3429 Co-Authored-By: Claude Opus 4.8 <[email protected]>
There was a problem hiding this comment.
🟡 Changes recommended
The new visibility/recenter logic mixes WPF logical units with WinForms screen pixel coordinates under DPI scaling, which can misplace or misclassify window visibility on non-100% DPI setups.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates Dynamo’s main window restore logic in DynamoView to prevent Dynamo from launching fully off-screen when the previously-saved monitor layout is no longer available, by validating/restoring the saved bounds only when they meaningfully overlap a currently-connected display and otherwise recentering on the primary display.
Changes:
- Replaces the prior “sum of monitor widths/heights” guard and
CheckVirtualScreenSize()withIsWindowVisibleOnAnyScreen(...)based on rectangle intersection with each display’s working area. - Recenters the window on the primary monitor’s working area when the saved bounds don’t overlap any connected display by at least a minimum visible extent (100px).
File summaries
| File | Description |
|---|---|
| src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs | Reworks startup window-bounds restoration to avoid off-screen launches by validating bounds against connected display working areas and recenters when not recoverable. |
Review details
Suppressed comments (1)
src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs:1210
IsWindowVisibleOnAnyScreenintersects the saved bounds (coming from WPFWindowX/Y/W/Hdoubles, i.e., WPF logical units) withScreen.WorkingArea(device pixels). On DPI-scaled displays this mixes coordinate spaces and can misclassify a window as visible/not visible. Consider converting the WPF bounds to device pixels viaPresentationSource.CompositionTarget.TransformToDevicebefore intersecting (seesrc/DynamoCoreWpf/Windows/ModelessChildWindow.csfor an existing pattern).
foreach (var screen in System.Windows.Forms.Screen.AllScreens)
{
var overlap = System.Drawing.Rectangle.Intersect(screen.WorkingArea, bounds);
if (overlap.Width >= MinVisibleWindowExtent &&
overlap.Height >= MinVisibleWindowExtent)
{
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| var workingArea = System.Windows.Forms.Screen.PrimaryScreen.WorkingArea; | ||
| Width = Math.Min(1024, workingArea.Width); | ||
| Height = Math.Min(768, workingArea.Height); | ||
| Left = workingArea.Left + (workingArea.Width - Width) / 2; | ||
| Top = workingArea.Top + (workingArea.Height - Height) / 2; |
There was a problem hiding this comment.
Good catch — fixed in 8ab87aa. All of the geometry now happens in device-independent units:
- The fallback centering uses
SystemParameters.WorkAreainstead ofScreen.PrimaryScreen.WorkingArea, so it is already in DIPs and matches whatLeft/Top/Width/Heightexpect. - The visibility check keeps
Screen.AllScreens(there is no WPF API that enumerates monitors), but eachWorkingAreais scaled bySystemParameters.PrimaryScreenWidth / Screen.PrimaryScreen.Bounds.Widthbefore being intersected with the saved bounds, and the intersection now usesSystem.Windows.Rectrather thanSystem.Drawing.Rectangleso nothing is truncated to integers.
The scale factor is derived from the primary monitor, so it is an approximation on mixed-DPI layouts. That is documented on the helper and is acceptable here because the check only needs to know roughly where the connected monitors are — it is bounded by the 100-unit overlap tolerance. It also degrades correctly for a DPI-unaware host, where the ratio is 1.
Address review feedback: System.Windows.Forms.Screen reports device pixels, while WPF Left/Top/Width/Height and the saved WindowX/Y/W/H preferences are device-independent units. On non-100% DPI the previous comparison mixed the two coordinate spaces, so the visibility check and the recentering fallback could land in the wrong place. Use SystemParameters.WorkArea (already DIPs) for the fallback bounds and scale each Screen working area by the primary monitor's DIP-to-pixel ratio before intersecting it with the saved bounds. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
johnpierson
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-10845
Findings summarySymptom. After a monitor layout change (undocking, unplugging a display, remote sessions, home/office swaps), Dynamo launches but its window is off-screen and unreachable on any connected display. The process is running, but standard Windows recovery (Win+arrow) is unreliable because Dynamo's window is modeless. Where. Root cause. On startup Dynamo restores the last-saved
As a result, a window last shown on a monitor that's since been removed keeps its stale coordinates and renders off-screen. Fix. Replace the summing logic and Behavior change to note. Previously an oversized saved window (larger than the virtual desktop) was reset. Now it's restored as-is; because its top-left still lands on a live monitor it remains reachable and resizable. Scope / limits. Core-side fix, covers Sandbox + Revit. A manual "reset window position" command was intentionally left to the Revit side, tracked in DynamoDS/DynamoRevit#3429. |
edwin-vasquez-ucaldas
left a comment
edwin-vasquez-ucaldas
left a comment
There was a problem hiding this comment.
LGTM.
Just one question: Each Screen working area is converted with the primary monitor scale, this is good to despite monitors potentially using different scales.
Reword the doc comment to explain that, because Dynamo is System-DPI-aware, Windows virtualizes every monitor's coordinates to the single system scale, so the primary-derived factor is correct for all displays rather than an approximation. Note the Per-Monitor V2 condition under which it would need a per-monitor GetDpiForMonitor lookup. Addresses review feedback from Edwin. Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
@edwin-vasquez-ucaldas thanks for the review! Good question. It's safe because Dynamo runs System-DPI-aware (no Per-Monitor V2 manifest), so Windows virtualizes every monitor's coordinates to the single system/primary DPI before we ever read them — The only case where per-monitor scales would matter is if we ever move Dynamo to Per-Monitor V2 awareness, where each display can carry its own scale — at that point this helper would need a per-monitor |
|
| if (IsWindowVisibleOnAnyScreen(savedBounds)) | ||
| { | ||
| Left = savedBounds.Left; | ||
| Top = savedBounds.Top; |
There was a problem hiding this comment.
Saved Top is restored unclamped, so the title bar can still end up off-screen.
The visibility test only requires a 100-unit overlap somewhere on a connected display, and the old WindowY < oy - 10 guard was removed, so nothing now keeps the caption row on-screen.
Scenario: two monitors stacked vertically (secondary at 0,-1080, primary at 0,0). Dynamo is left straddling the boundary at Top = -300, Height = 768, then the secondary is disconnected. IsWindowVisibleOnAnyScreen intersects (0,-300,1024,768) with the primary work area (0,0,1920,1032) and gets a 1024x468 overlap, which passes both >= 100 checks, so Top = -300 is restored. The top 300 DIPs of the window (title bar, menu bar, toolbar) are above the screen edge -- the user cannot drag it back, and per the PR description Win+Arrow is unreliable on this modeless window. The old code reset this case to 0,0 / 1024x768.
Suggest that, once the bounds are deemed visible, Top (and ideally Left) is clamped so the caption row lies inside the working area of the screen the window intersects.
| { | ||
| Left = savedBounds.Left; | ||
| Top = savedBounds.Top; | ||
| Width = savedBounds.Width; |
There was a problem hiding this comment.
An oversized saved size is now restored verbatim, which can push all of the window chrome off-screen.
The PR body calls out that the size guard was dropped, but the concrete failure mode is worth weighing: the old WindowW > VirtualScreenWidth || WindowH > VirtualScreenHeight check was the only clamp, and nothing replaced it.
Scenario: the user runs Dynamo maximized on a 3840x2160 display, so DynamoView_SizeChanged persists WindowW ~= 3856 / WindowH ~= 2176 and DynamoView_LocationChanged persists WindowX/Y = -8. They later launch on a laptop with only a 1920x1080 display. The saved rect overlaps the primary work area heavily, so this branch restores Left/Top = -8 and Width/Height = 3856x2176 -- roughly double the screen in each dimension. The minimize/maximize/close buttons (top-right) and the ResizeMode="CanResizeWithGrip" grip (bottom-right) are both off-screen, leaving only Alt+Space / Alt+F4. Previously this reset to 1024x768.
Clamping the restored Width/Height to the working area of the intersecting screen would keep the off-screen fix without this regression.
| /// on is no longer available. | ||
| /// </summary> | ||
| /// <param name="bounds">Window bounds in device-independent units.</param> | ||
| private static bool IsWindowVisibleOnAnyScreen(Rect bounds) |
There was a problem hiding this comment.
No test coverage, and the shape makes it untestable.
This predicate is the entire correctness surface of the fix -- negative coordinates, disconnected monitors, the DPI scale derivation, and the 100-unit tolerance -- but it is private static on a WPF Window, so no test can reach any of its branches. A later tweak to the tolerance or the scale factor could silently reintroduce the off-screen launch with a green CI.
Consider making it internal static bool IsWindowVisibleOnAnyScreen(Rect bounds, IEnumerable<Rect> screenWorkAreas) (with the Screen.AllScreens + scaling lookup left at the call site). That makes the interesting cases assertable from DynamoCoreWpfTests: fully on-screen, negative-coordinate maximized (-8,-8), a sliver overlap below the tolerance, and no overlap after a monitor is removed.
jasonstratton
left a comment
jasonstratton
left a comment
There was a problem hiding this comment.
Roberto had 3 comments that need to be addressed before merging:
- DynamoView.xaml.cs:200 — restored Top isn't clamped, so a window straddling two stacked monitors can still land with its title bar/menu above the visible screen edge once the second monitor is gone. Concrete repro given (secondary at 0,-1080, saved Top=-300). This is literally a variant of the bug the PR is fixing.
- DynamoView.xaml.cs:201 — the old oversized-window guard was dropped with nothing replacing it; a window maximized on a 4K display restored on a 1080p laptop keeps ~3856×2176 dimensions, pushing the resize grip and min/max/close buttons off-screen.
- DynamoView.xaml.cs:1201 — IsWindowVisibleOnAnyScreen is private static on a WPF Window, so this is the entire correctness surface of the fix and it's untestable from DynamoCoreWpfTests. Suggests internal static with Rect/IEnumerable params.
Looks like the failing PR checks are due to infrastructure failures. I will record them, but when changes are made, they should rerun and hopefully succeed on that run.
|
Just agree with the Roberto's comments, when you have a chance to address before merge. Also same suggestion from the Jason's comment. |
There was a problem hiding this comment.
See the ticket for this pull request: https://autodesk.atlassian.net/browse/DYN-10845
|
Purpose
Fixes DYN-10845.
Dynamo restores its previous window position on launch. When the monitor it was last shown on is no longer available — undocking/docking a laptop, disconnecting a display, switching between office and home setups, remote sessions, or otherwise rearranging a multi-monitor layout — the saved position can place the window entirely off-screen: running, but inaccessible, with no reliable way to recover it (Win+Arrow does not consistently work on Dynamo's modeless window).
Reported via the community here: https://forum.dynamobim.com/t/dynamo-button-idea-for-ui/115174
Related issue: DynamoDS/DynamoRevit#3429
Root cause
The startup restore logic in
DynamoViewvalidated the saved bounds by summing every monitor's width and height (leftLimit/topLimit) and only resetLeft/Topto 0 if the saved value exceeded that sum. The virtual desktop is not a simple tiling — monitors can sit at negative coordinates or with gaps between them — so that guard is geometrically meaningless.CheckVirtualScreenSize()only rejected positions past the top/left origin, so a window saved on a monitor that was later unplugged stayed off-screen. Nothing tested whether the saved rectangle actually intersects a currently connected display.Fix
Replace the summing guard and
CheckVirtualScreenSize()with a real visibility test (IsWindowVisibleOnAnyScreen) that intersects the saved bounds against each connected display's working area. The saved bounds are restored only when the window overlaps a live monitor by a usable amount (>= 100 units in both dimensions); otherwise the window is recentered on the primary monitor's work area. This handles negative coordinates and disconnected or rearranged monitors.All of the geometry is done in device-independent units, which is the unit the
WindowX/WindowY/WindowW/WindowHpreferences are written in (they are populated from WPFLeft/TopinDynamoView_LocationChanged).SystemParameters.WorkAreais already in DIPs and is used directly for the recentering fallback; theSystem.Windows.Forms.Screenrectangles are in device pixels, so they are scaled by the primary monitor's DIP-to-pixel ratio before being intersected. The scale is derived from the primary monitor, so it is an approximation on mixed-DPI layouts — acceptable here because the check only needs to know roughly where the connected monitors are, and it is bounded by the 100-unit overlap tolerance.Behavior change worth noting
Previously the window was also reset when the saved size was larger than the virtual desktop. With this change an oversized saved window is restored as-is; because its top-left corner is guaranteed to be on a visible monitor, it remains recoverable and resizable. Called out here so it is not a surprise in review.
Declarations
Check these if you believe they are true
No public API surface changes — both new members are private to
DynamoView.Release Notes
Dynamo no longer opens off-screen when the monitor it was last used on is disconnected or rearranged; the window is recentered on the primary display when the saved position is no longer visible.
Reviewers
Open to the Dynamo team — happy to add specific reviewers on request.
Manual verification: launch Dynamo on a secondary monitor, close it, disconnect that monitor, and relaunch — the window comes back centered on the primary display instead of off-screen. Repeat at 100% and 150% display scaling.
FYIs
@JacobSmall (filed the related DynamoRevit issue)