[Willie Abrams (willie)]

ActiveStage composites the EGFX output after each PDU. That only works when the GraphicsPipelineClient lives inside it.

Why run it somewhere else:

• H.264 decoding can be slow (a busy GPU).
• While it decodes, the session loop doesn't read the socket.
• Audio waits behind video, and GNOME Remote Desktop drops audio older than 50 ms.
• Running the pipeline on its own thread fixes that, but then nothing composites its output: the reset and composite steps were private.

What this adds:

• composite_graphics_pipeline(gfx, image): follows a pending ResetGraphics, applies the finished output regions, and takes the RemoteApp window regions.
• CompositedGraphics: whether it reset, the union region, each region, and the window updates.
• ActiveStage calls it too, so its behavior doesn't change.
• ironrdp-egfx is now a # public dependency, since GraphicsPipelineClient is in the signature. CompositedGraphics is #[non_exhaustive].

Using it off the session loop, the caller also:

• registers a channel processor named ironrdp_egfx::CHANNEL_NAME that forwards each message to the pipeline;
• sends the pipeline's replies (capabilities, frame acks) through the tunnel dvc_tunnel_for_channel names, or over TCP with ActiveStage::encode_dvc_messages;
• shares the image with the session under a lock (bitmaps and the pointer write to it too);
• paints the window updates into the RemoteApp windows they name, as ActiveStage returns them in WindowSurfaceUpdates;
• remembers a reset until the next paint, and tracks damage itself.

The function's docs say the same.

🤖 Generated with Claude Code

[Willie Abrams (willie)]

Validation at 31a3331:

• New test composite_graphics_pipeline_follows_a_reset_then_paints_the_frame (ironrdp-testsuite-core, session::active_stage): a pipeline fed ResetGraphics, CreateSurface, MapSurfaceToOutput and a solid fill in one frame. Checks the image is resized, the fill's pixels land, the region covers them, and a second call returns nothing. It fails with either the reset or the compositing step removed.
• New test active_stage_widens_after_a_reset_and_records_damage: a pipeline registered on DRDYNVC, fed through ActiveStage::process as X.224 frames. Checks the update after a reset covers the whole image and that a later fill is recorded as damage. It fails with the damage step removed. Removing the reset flag alone still passes: mapping a surface after the reset already outputs the whole surface.
• cargo test -p ironrdp-testsuite-core: 1837 passed
• RUSTDOCFLAGS=-D warnings cargo doc -p ironrdp-session: clean
• cargo xtask check fmt, cargo xtask check lints, typos: clean

Where it's used: a macOS client I work on runs the pipeline on its own thread and calls this after each EGFX message. Against a headless GNOME Remote Desktop with 50 ms and 150 ms of artificial decode delay per update, all of a 17 s tone arrived; with the pipeline on the session loop, audio stopped after 3 s and the server dropped the connection.

Note

Human-tuned, LLM-assisted content.

[Copilot]

🟡 Changes recommended

The usage documentation incorrectly routes tunneled EGFX replies through the TCP-only encoder.

1 open finding

• Route EGFX replies through the active DVC transport · New

What changed in this PR

Exposes EGFX compositing so pipelines can run outside the session loop without blocking other channels.

Changes:

• Adds public compositing results and helper API.
• Reuses the helper within ActiveStage.
• Adds integration coverage and marks EGFX as public.

File	Description
crates/​ironrdp-session/​src/​active_stage.rs	Adds and integrates the compositing API.
crates/​ironrdp-session/​src/​lib.rs	Re-exports the new public API.
crates/​ironrdp-session/​Cargo.toml	Marks EGFX as a public dependency.
crates/​ironrdp-testsuite-core/​tests/​session/​active_stage.rs	Tests reset, compositing, and damage behavior.

🧠 Review effort: Balanced

[github-actions]

PR #2120 extracts ActiveStage's private EGFX drain logic into a public composite_graphics_pipeline plus a #[non_exhaustive] CompositedGraphics result, making ironrdp-egfx a public dependency so off-loop callers (e.g., a dedicated decode thread) can composite without reimplementing reset/compositing subtleties. Verified behaviorally equivalent to the old inline path for all cases (no pipeline, reset only, updates only, both), including the Ok(None) channel-absent case. Two new tests cover the public path and the preserved ActiveStage path (reset widening, damage recording). No correctness or protocol defects found. Published one low-severity doc gap (docs omit the failure mode of compositing a pipeline ActiveStage still owns via the public get_dvc_mut), one low-severity control-flow style note on the rewritten drain method, and one low-severity test-fixture duplication note; rejected the suggestion to drop CompositedGraphics::region because the union helper is private and the field is …

1. [code-compressor] Rewritten drain_graphics_pipeline chains Option combinators where a flat let-else reads smaller — low 🟡 — crates/ironrdp-session/src/active_stage.rs
   The rewrite replaces the old map(...).unwrap_or_default() with map(...).transpose()? plus a let-else, keeping the combinator chain only to thread the channel-absent case. Since get_dvc_mut already returns Option<DynamicChannelMut<T>>, the same behavior is shorter and flat: bind the channel with a let-else returning Ok(None), then call composite_graphics_pipeline(gfx.processor_mut(), image)? directly. Error propagation, the None return, and lifetimes are identical. Optional style refinement; the current form is correct.

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.

[github-actions]

[skeptical] Docs omit the failure mode of compositing a pipeline that ActiveStage still owns — low 🟡 — ActiveStage::get_dvc_mut::<GraphicsPipelineClient>() is already public (active_stage.rs:493), so a caller can fetch the in-stage pipeline and pass it to the now-public composite_graphics_pipeline. That drains take_output_reset() and drain_output() before ActiveStage::process runs, so the in-stage drain sees no reset and no updates: graphics_output_needs_full_refresh is never set, widen_to_full_refresh never fires, and regions never reach take_damage_regions. The image is cleared and painted while the caller repaints only the returned region, leaving pre-reset content as silent visual corruption. The documented caller obligations cover the intended off-loop setup but do not warn that one pipeline must not be driven from both modes; a one-sentence doc guard (or debug-time detection) would close this.

[github-actions]

[code-compressor] Two new tests rebuild identical EGFX PDU fixtures instead of sharing a helper — low 🟡 — Both new tests inline the same StartFrame/SolidFill/EndFrame triple, the all-zero Timestamp, the same fill color, and the same ResetGraphics/CreateSurface/MapSurfaceToOutput sequence; test 2 builds the fill triple via a local closure while test 1 repeats it inline. Hoisting fill (and optionally the surface-setup list) to file scope as small helpers returning Vec<GfxPdu> would cut roughly 40 lines and keep the fixture in one place. Test 2's gfx() frame wrapper is transport-specific and stays per-test.

[Marc-André Moreau (mamoreau-devolutions)]

Rebased onto master (8668111b) and force-pushed.
The previous head was 3c4c95c8.

What the rebase changed

• feat(egfx)!: composite surfaces mapped to RemoteApp windows #2197 made drain_graphics_pipeline return the RemoteApp WindowUpdates too, and added ActiveStageOutput::WindowSurfaceUpdates.
  composite_graphics_pipeline now takes them as well (CompositedGraphics::window_updates), so ActiveStage still forwards them and an off-loop caller can paint them.
  They count against the compositor's shared byte budget, so a caller that never drained them would starve the surfaces and the cache.
• ActiveStage::process gained a received_at argument on master, so the second test passes None for it now.
• Cargo.toml takes master's dependency versions and keeps the # public marker on ironrdp-egfx.
• New test: composite_graphics_pipeline_takes_the_window_updates.
• The commits keep Willie as author.
  The two whose content changed carry a Co-authored-by trailer for the rebase.

Validation at 397fc69e

• cargo xtask check fmt, lints, tests (whole workspace), locks and typos pass.
• RUSTDOCFLAGS='-D warnings' cargo doc -p ironrdp-session --no-deps passes, so the intra-doc links resolve against current master.
• The session::active_stage group runs 12 tests.

Lines changed against master

	Added	Removed
Production (ironrdp-session/src)	72	13
Tests (ironrdp-testsuite-core)	348	0
Other (ironrdp-session/Cargo.toml)	1	1
Total	421	14

32 of the added production lines are rustdoc.

Note

LLM-assisted content (no human feedback).