Repository navigation
feat(displaycontrol)!: multi-monitor layouts with orientation and DPI - #2182
Conversation
…r caps Build complete MS-RDPEDISP monitor layouts that the server can accept: - `DisplayControlMonitorLayout::new` now rejects overlapping monitors and, for multi-monitor layouts, monitors not adjacent to another one (MS-RDPEDISP 3.1.5.2). - `fit_to_capabilities` enforces MaxNumMonitors and the maximum monitor area: extra monitors are dropped (the primary is kept) and a single monitor is scaled down, preserving its aspect ratio. - `DisplayControlClient` stores the server capabilities and gains `encode_monitor_layout`; `encode_single_primary_monitor` now fits its layout to the capabilities too. - `new_single_primary_monitor` clamps the desktop scale factor to 100..=500, derives the device scale factor from it instead of hard-coding 100%, and reports a square monitor as landscape. - New helpers: capability getters, `area`, `desktop_size`, `MonitorLayoutEntry::adjust_desktop_scale_factor`, `DeviceScaleFactor::from_value` and `from_desktop_scale_factor`. BREAKING CHANGE: `DisplayControlMonitorLayout::new` returns an error for overlapping or disconnected monitors, which it previously accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add `ActiveStage::prepare_monitor_layout`, `encode_monitor_layout` and `display_control_capabilities`, so clients can send several monitors with their positions, orientation, physical size and scale factors during the session instead of a single primary monitor size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add `RdpInputEvent::MonitorLayout`, which shares the resize queue: it is deferred until Display Control is ready, and its reconnection fallback advertises the requested monitors in the GCC Client Monitor Data block. A layout the server capabilities cannot accept is logged and dropped rather than ending the session. BREAKING CHANGE: `RdpInputEvent` has a new variant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Handle `WindowEvent::ScaleFactorChanged` through the debounced resize path, so the session follows the window's DPI when it moves to another monitor or the monitor scale changes. The RPC-backed viewer now passes the window scale too. The viewer is a single window, so it keeps sending one primary monitor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Multimonitor sessions no longer reject dynamic display updates. Like mstsc, they mirror the local monitors: `UpdateSessionDisplaySettings`, `WM_DISPLAYCHANGE` and `WM_DPICHANGED` (debounced) send the full local topology as a Display Control layout, with each monitor's position, orientation and effective DPI. The sent topology becomes the configured one, so it stays consistent with the GCC monitors advertised at connection time and is confirmed by the server's monitor layout. Single-monitor updates now accept rotation and the 140%/180% device scales by sending an explicit layout; values MS-RDPEDISP does not define are rejected with E_INVALIDARG. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Display metadata preservation, reconnect handling, and confirmed topology state have unresolved correctness defects.
6 open issues
Correct reversed 90/270 monitor rotation mappings · New Avoid overriding DPI during automatic renderer resizes · New Preserve explicit display scale and orientation metadata · New Preserve confirmed topology when layout updates are rejected · New Validate GCC monitor limits before fallback disconnect · New Clear stale monitor topology on plain resize · New
What changed in this PR
Extends Display Control to support multi-monitor layouts, orientation, and DPI across the session API, client, viewer, and ActiveX control.
Changes:
- Adds layout validation, server-capability fitting, and encoding APIs.
- Integrates monitor updates with resize queues and reconnect fallback.
- Adds DPI handling and regression tests.
| File | Description |
|---|---|
| crates/ironrdp-viewer/src/app.rs | Debounces DPI changes and forwards window scale. |
| crates/ironrdp-testsuite-core/tests/displaycontrol/mod.rs | Tests layout validation, fitting, and scaling. |
| crates/ironrdp-session/src/active_stage.rs | Exposes capabilities and monitor-layout APIs. |
| crates/ironrdp-displaycontrol/src/pdu/mod.rs | Adds geometry validation and capability fitting. |
| crates/ironrdp-displaycontrol/src/client.rs | Stores capabilities and encodes complete layouts. |
| crates/ironrdp-daemon/src/daemon.rs | Accepts scale factors for resize requests. |
| crates/ironrdp-client/src/rdp.rs | Queues layouts and adds reconnect conversion. |
| crates/ironrdp-activex/src/control.rs | Integrates local topology, rotation, and DPI updates. |
| crates/ironrdp-activex/Cargo.toml | Adds the Display Control dependency. |
| Cargo.lock | Records the ActiveX dependency change. |
Review effort: Balanced
…t updates Address review feedback on multimonitor Display Control layouts: - client: a layout fallback reconnection fits the GCC monitor limit (16), keeping the primary monitor, and a plain resize fallback clears any previously configured monitor topology. - activex: renderer-driven resizes keep each monitor's DPI and skip an unchanged topology; explicit updates use the resize shortcut only when its derived orientation and device scale match the request; sending a layout keeps the confirmed topology until the server reports another. - activex: correct the DMDO rotation comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
The PR coherently enables full multi-monitor DISPLAYCONTROL_MONITOR_LAYOUT_PDUs with orientation and DPI: pdu-level overlap/adjacency validation and capability fitting, session plumbing via ActiveStage::prepare_monitor_layout, a new RdpInputEvent::MonitorLayout sharing the resize queue with GCC-fallback reconnection, and ActiveX mirroring of the local topology. The protocol reviewer found no wire-conformance defect and independent inspection of fit_to_capabilities, the session/client plumbing and the ActiveX layout code surfaced no session-corrupting bug. Published findings: one unverified spec interpretation in fit_to_capabilities' total-area limit (concrete impact: a server-rejected layout is only logged and dropped), one unresolved behavior question in mirrors_local_monitors for single-monitor 'use multimon' sessions, one overclaimed doc/behavior gap in the capability fitting (refined), and four small behavior-preserving compression nits in the new ActiveX code. The parallel-attrib…
- [code-compressor] adjust_display_size is computed twice for the same desktop size in update_display_layout_impl — low 🟡 — crates/ironrdp-activex/src/control.rs
The pure MonitorLayoutEntry::adjust_display_size(desktop_width, desktop_height) is called inside the derived_orientation block and again in the full-layout else branch. Hoisting a single let binding above the event conditional removes the duplicated derivation and the extra block scope with no behavior change.
Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.
Address review feedback: - displaycontrol: when a layout exceeds MaxNumMonitors, keep the monitors reachable from the primary first, so the reduced layout stays adjacent whatever order the monitors are listed in; document that an error means the caller should keep the current layout. - activex: with 'use multimon' alone, mirror the local monitors only once the local topology has several monitors, so a single-monitor session keeps honoring explicit sizes and orientations. - activex: send the multimonitor layout through one call, reuse the derived size and scales, and log every invalid local monitor the same way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
The PR is a well-tested, protocol-conformant implementation of multi-monitor Display Control layouts: wire format, overlap/adjacency validation, capability fitting, the GCC reconnect fallback, and ActiveX multimon mirroring all check out against MS-RDPEDISP/MS-RDPBCGR. I independently confirmed the five skeptical findings: Reconnect() routes hardcoded 100% scale factors through the explicit multimon path, an explicit scale override is recorded as the base topology so later DPI refreshes are skipped as unchanged, capability-fit failures silently drop layouts while the ActiveX control has already recorded the topology as configured (and the non-forced refresh skip prevents retries), RAIL is told the unfitted desktop size, and the reconnect fallback doc overstates the code. The two code-compressor findings are confirmed as unused/test-only public API additions; both are low-severity speculative-surface items.
- [skeptical] Reconnect() forces 100% desktop and device scale on every monitor of a multimon session — medium 🟠 ❓ — crates/ironrdp-activex/src/control.rs
reconnect() calls update_display_layout with hardcoded scale factors of 100, which forward to update_display_layout_impl(layout, explicit=true). In the new multimon branch, explicit=true treats those hardcoded defaults as caller choices and forces the update, sending the local topology with every monitor's DPI overridden to 100% while the requested width/height is ignored. This is precisely the 'defaults, not caller choices' case explicit=false was introduced for on the renderer path. The lines are pre-existing but the behavior becomes reachable only with this PR; intent (mstsc parity) is unverifiable, hence question=true.
Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.
Address review feedback: - client: fit a monitor layout to the server capabilities before sending it, so RAIL and the reconnection fallback use the size the server applies, and report a layout that cannot fit with the new `RdpOutputEvent::MonitorLayoutRejected` instead of dropping it silently. Correct the fallback doc: an unfittable layout reconnects without monitor data. - activex: callers without scale arguments (Reconnect, the two-argument UpdateSessionDisplaySettings, renderer resizes) leave the scale unspecified instead of forcing 100% on every monitor; remember the last layout sent, scale overrides included, so a refresh restores per-monitor DPI; roll back to the confirmed topology when a layout is rejected. - displaycontrol: drop the unused MaxMonitorAreaFactorA/B getters. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
The PR adds complete multi-monitor Display Control layouts with orientation/DPI, capability fitting, overlap/adjacency validation, an ActiveX local-topology mirroring path, and a GCC Client Monitor Data fallback. All 9 validated candidates were independently confirmed against the head and accepted: one protocol edge case (missing 32,766-pixel virtual desktop bound in the GCC fallback), four behavioral/state-consistency concerns of low severity, and four maintainability items. The two double-fit findings (skeptical and code-compressor) share one root cause at rdp.rs:669-675 and are merged into a single final finding with both sources. No high or critical issues were found; the double-fit and duplicated rejection-handler items are the most actionable maintainability cleanups.
- [code-compressor] Zeroed 'unspecified scale' DisplayLayout literal repeated at six call sites — low 🟡 — crates/ironrdp-activex/src/control.rs
The tail {physical_width: 0, physical_height: 0, orientation: 0, desktop_scale_factor: 0, device_scale_factor: 0} is repeated at six DisplayLayout construction sites (plus display_layout_from_renderer_size), each differing only in desktop_width/height. The PR added doc comments documenting the 0-means-unspecified convention, but it still has no single definition; a small constructor such as DisplayLayout::unspecified(desktop_width, desktop_height) would collapse each literal to two arguments so a future field cannot be missed at one site. Behavior is identical.
Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.
Address review feedback: - client: a layout fallback whose monitors span more than 32,766 pixels reconnects without monitor data (MS-RDPBCGR 2.2.1.3.6); a plain resize fallback keeps a single configured monitor, resized, instead of dropping it; report rejected layouts through one helper. - activex: a rejected layout always rolls back to the confirmed topology, even when none exists, and stays the last layout sent so the identical layout is not retried; add `DisplayLayout::unscaled` for the six callers without scale arguments. - displaycontrol: drop the test-only `DisplayControlMonitorLayout::area`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0784fce
into
master

Display Control (MS-RDPEDISP) was only usable for a single primary monitor at 100% device scale. This lets clients send complete monitor layouts during the session, with real orientation and DPI, and wires it into the viewer and the ActiveX control.
Changes
ironrdp-displaycontrol(breaking)DisplayControlMonitorLayout::newnow rejects overlapping monitors and, for multi-monitor layouts, monitors not adjacent to another one (MS-RDPEDISP 3.1.5.2). Layouts it previously accepted can now fail.fit_to_capabilitiesenforces the server'sMaxNumMonitorsand maximum monitor area (MS-RDPEDISP 3.2.5.2): extra monitors are dropped (the primary is kept), a single oversized monitor is scaled down keeping its aspect ratio, and an oversized multi-monitor layout is rejected rather than shrunk (which would move monitors relative to each other).DisplayControlClientstores the server capabilities and gainsencode_monitor_layout;encode_single_primary_monitorfits to the capabilities too.new_single_primary_monitorclamps the desktop scale to 100..=500 and derives the device scale (100/140/180) from it instead of hard-coding 100%; a square monitor is landscape.area,desktop_size,MonitorLayoutEntry::adjust_desktop_scale_factor,DeviceScaleFactor::from_value/from_desktop_scale_factor.ironrdp-session:ActiveStage::prepare_monitor_layout,encode_monitor_layout,display_control_capabilities.ironrdp-client(breaking, new enum variant):RdpInputEvent::MonitorLayoutshares the resize queue. Without Display Control it reconnects advertising the requested monitors in the GCC Client Monitor Data block. A layout the server capabilities cannot accept is logged and dropped instead of ending the session.Viewer: handles
WindowEvent::ScaleFactorChangedthrough the debounced resize path, so the session follows the window DPI. The viewer is a single window, so it still sends one primary monitor. The RPC-backed path passes the window scale too (Daemon::try_resizetakes a scale; the daemon crate is unpublished).ActiveX
UpdateSessionDisplaySettings,WM_DISPLAYCHANGEandWM_DPICHANGED(debounced) send the full local topology with each monitor's position, rotation and effective DPI. Explicit scale arguments override every monitor; width/height/orientation/physical size describe a single monitor and are ignored in multimon.E_INVALIDARG(wasE_NOTIMPL).active_multimon_layout_rejects_dynamic_display_updatesis replaced byactive_multimon_layout_sends_full_display_control_layouts.Testing
ironrdp-testsuite-coretests for overlap/adjacency validation, capability fitting, scale clamping and client encoding; unit tests in session, client and ActiveX.cargo xtask check fmt -vandcargo xtask check lints -vpass; tests pass for every touched crate.Known limitations
WM_DISPLAYCHANGEis only broadcast to top-level windows, so the ActiveX control sees it only when its window is top-level..rdpkeysuse multimon,selectedmonitorsanddynamic resolutionare not handled yet.🤖 Generated with Claude Code