Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/ironrdp-session/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ ironrdp-error = { path = "../ironrdp-error", version = "0.2" } # public
ironrdp-graphics = { path = "../ironrdp-graphics", version = "0.10" } # public
ironrdp-pdu = { path = "../ironrdp-pdu", version = "0.10", features = ["std"] } # public
ironrdp-displaycontrol = { path = "../ironrdp-displaycontrol", version = "0.9" }
ironrdp-egfx = { path = "../ironrdp-egfx", version = "0.4" }
ironrdp-egfx = { path = "../ironrdp-egfx", version = "0.4" } # public
ironrdp-rdpei = { path = "../ironrdp-rdpei", version = "0.2" } # public
tracing = { version = "0.1", features = ["log"] }
qoicoubeh = { version = "0.5", optional = true }
Expand Down
80 changes: 68 additions & 12 deletions crates/ironrdp-session/src/active_stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,21 +307,18 @@ impl ActiveStage {
&mut self,
image: &mut DecodedImage,
) -> SessionResult<(Option<InclusiveRectangle>, Vec<WindowUpdate>)> {
let (output_reset, graphics_updates, window_updates) = self
let Some(composited) = self
.get_dvc_mut::<GraphicsPipelineClient>()
.map(|mut gfx| {
let gfx = gfx.processor_mut();
(gfx.take_output_reset(), gfx.drain_output(), gfx.drain_window_updates())
})
.unwrap_or_default();
if let Some((width, height)) = output_reset {
image.reset_preserving_pointer(width, height)?;
.map(|mut gfx| composite_graphics_pipeline(gfx.processor_mut(), image))
.transpose()?
else {
return Ok((None, Vec::new()));
};
if composited.reset {
self.graphics_output_needs_full_refresh = true;
}
let (region, damage_regions) =
composite_graphics_updates(image, graphics_updates.into_iter().map(|u| (u.region, u.data)))?;
self.damage_regions.extend(damage_regions);
Ok((region, window_updates))
self.damage_regions.extend(composited.regions);
Ok((composited.region, composited.window_updates))
}

/// Composites RGBA pixels rendered outside the session, such as client-side video, into
Expand Down Expand Up @@ -1126,6 +1123,65 @@ fn process_slow_path_pointer(
fast_path_processor.process_pointer_update(image, pointer)
}

/// What [`composite_graphics_pipeline`] did to the image.
#[derive(Debug)]
#[non_exhaustive]
pub struct CompositedGraphics {
/// The image was resized and cleared for a `ResetGraphics`. `region` covers
/// only what was painted since, and is `None` when nothing was, so the
/// caller remembers the reset and repaints the whole image at its next
/// paint, as [`ActiveStage`] does with its next graphics update.
pub reset: bool,
/// The union of the regions changed, or `None` when nothing changed.
pub region: Option<InclusiveRectangle>,
/// Each region changed.
pub regions: Vec<InclusiveRectangle>,
/// The RemoteApp window regions, which are not part of the image.
pub window_updates: Vec<WindowUpdate>,
}

/// Composite what `gfx` has decoded since the last call into `image`: follow a
/// pending `ResetGraphics`, then apply each completed-frame output region.
/// Also take the RemoteApp window regions, which are not part of `image`.
///
/// [`ActiveStage`] does this after each PDU for a [`GraphicsPipelineClient`]
/// registered on its dynamic channels. Call it directly for one that runs
/// elsewhere, such as on a thread of its own so that a slow H.264 decode
/// doesn't hold up the session's other channels. The caller then also:
///
/// - registers in its place a [`DvcClientProcessor`] named
/// [`ironrdp_egfx::CHANNEL_NAME`] that forwards each message to the
/// pipeline;
/// - sends the pipeline's replies (capabilities, frame acknowledgements):
/// wraps them with [`ironrdp_dvc::encode_dvc_messages`] for the channel,
/// then, if [`ActiveStage::dvc_tunnel_for_channel`] names a Soft-Sync
/// tunnel, sends each one's [`SvcMessage::encode_unframed_pdu`] through
/// it, and otherwise encodes them for TCP with
/// [`ActiveStage::encode_dvc_messages`];
/// - shares `image` with the session under a lock, since other updates
/// (bitmaps, the pointer) write to it too;
/// - paints each of `window_updates` into the window it names, as
/// [`ActiveStage`] returns them in [`ActiveStageOutput::WindowSurfaceUpdates`];
/// - tracks the regions itself: they don't reach
/// [`ActiveStage::take_damage_regions`].
Comment on lines +1147 to +1166

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

pub fn composite_graphics_pipeline(
gfx: &mut GraphicsPipelineClient,
image: &mut DecodedImage,
) -> SessionResult<CompositedGraphics> {
let output_reset = gfx.take_output_reset();
if let Some((width, height)) = output_reset {
image.reset_preserving_pointer(width, height)?;
}
let (region, regions) =
composite_graphics_updates(image, gfx.drain_output().into_iter().map(|u| (u.region, u.data)))?;
Ok(CompositedGraphics {
reset: output_reset.is_some(),
region,
regions,
window_updates: gfx.drain_window_updates(),
})
}

/// Apply every compositor delta to `image` and return their union and exact applied regions.
///
/// The union preserves the single full-frame copy used by existing consumers.
Expand Down
5 changes: 4 additions & 1 deletion crates/ironrdp-session/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ use core::fmt;

#[cfg(feature = "__test")]
pub use active_stage::composite_graphics_updates;
pub use active_stage::{ActiveStage, ActiveStageBuilder, ActiveStageOutput, GracefulDisconnectReason};
pub use active_stage::{
ActiveStage, ActiveStageBuilder, ActiveStageOutput, CompositedGraphics, GracefulDisconnectReason,
composite_graphics_pipeline,
};
pub use fast_path::{BulkDecompressionErrorKind, FastPathBulkDecompressionFailure};

pub type SessionResult<T> = Result<T, SessionError>;
Expand Down
Loading
Loading