Skip to content

Commit a1c0315

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 973f325 commit a1c0315

4 files changed

Lines changed: 324 additions & 35 deletions

File tree

crates/ironrdp-egfx/src/client.rs

Lines changed: 73 additions & 2 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,6 +57,7 @@ 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;
6162
use ironrdp_pdu::geometry::{InclusiveRectangle, Rectangle as _};
6263
use ironrdp_pdu::{PduResult, decode_cursor, decode_err, pdu_other_err};
@@ -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
}
@@ -751,6 +761,36 @@ impl GraphicsPipelineClient {
751761
Ok(())
752762
}
753763

764+
fn decode_clearcodec(
765+
&mut self,
766+
surface_id: u16,
767+
dest_rect: &InclusiveRectangle,
768+
bitmap_data: &[u8],
769+
) -> PduResult<()> {
770+
let dest_width = dest_rect.width();
771+
let dest_height = dest_rect.height();
772+
773+
let bgra = self
774+
.clearcodec_decoder
775+
.decode(bitmap_data, dest_width, dest_height)
776+
.map_err(|e| pdu_other_err!("ClearCodec decode", source: e))?;
777+
778+
// ClearCodec outputs BGRA; convert to RGBA for the uniform BitmapUpdate format
779+
let rgba = convert_bgra_to_rgba(&bgra);
780+
781+
let update = BitmapUpdate {
782+
surface_id,
783+
destination_rectangle: dest_rect.clone(),
784+
codec_id: Codec1Type::ClearCodec,
785+
data: rgba,
786+
width: dest_width,
787+
height: dest_height,
788+
};
789+
790+
self.handler.on_bitmap_updated(&update);
791+
Ok(())
792+
}
793+
754794
fn handle_uncompressed(&mut self, pdu: crate::pdu::WireToSurface1Pdu) {
755795
let dest_width = pdu.destination_rectangle.width();
756796
let dest_height = pdu.destination_rectangle.height();
@@ -870,6 +910,19 @@ impl DvcClientProcessor for GraphicsPipelineClient {}
870910
// Frame Cropping
871911
// ============================================================================
872912

913+
/// Convert BGRA pixel data to RGBA8888
914+
///
915+
/// ClearCodec produces BGRA output per [MS-RDPEGFX 2.2.4.1]. Reorder to
916+
/// [R, G, B, A] for the uniform `BitmapUpdate` pixel format.
917+
fn convert_bgra_to_rgba(src: &[u8]) -> Vec<u8> {
918+
debug_assert!(src.len() % 4 == 0, "BGRA input length not aligned to 4 bytes");
919+
let mut dst = Vec::with_capacity(src.len());
920+
for pixel in src.chunks_exact(4) {
921+
dst.extend_from_slice(&[pixel[2], pixel[1], pixel[0], pixel[3]]);
922+
}
923+
dst
924+
}
925+
873926
/// Convert uncompressed 32bpp little-endian pixels to RGBA8888
874927
///
875928
/// The wire format for uncompressed graphics is 0xAARRGGBB in a 32-bit
@@ -1031,6 +1084,24 @@ mod tests {
10311084
assert_eq!(cropped.len(), 1920 * 1080 * 4);
10321085
}
10331086

1087+
#[test]
1088+
fn convert_bgra_to_rgba_reorders_channels() {
1089+
// BGRA input: [B, G, R, A] per pixel
1090+
let bgra = vec![
1091+
0xFF, 0x00, 0x00, 0xCC, // B=255, G=0, R=0, A=204 (blue)
1092+
0x00, 0xFF, 0x00, 0x80, // B=0, G=255, R=0, A=128 (green)
1093+
];
1094+
let rgba = convert_bgra_to_rgba(&bgra);
1095+
// Expected: [R, G, B, A] per pixel
1096+
assert_eq!(
1097+
rgba,
1098+
vec![
1099+
0x00, 0x00, 0xFF, 0xCC, // R=0, G=0, B=255, A=204
1100+
0x00, 0xFF, 0x00, 0x80, // R=0, G=255, B=0, A=128
1101+
]
1102+
);
1103+
}
1104+
10341105
#[test]
10351106
fn convert_uncompressed_bgrx_to_rgba() {
10361107
// Wire format: [B, G, R, A] per pixel (0xAARRGGBB little-endian)

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

crates/ironrdp-pdu/src/codecs/clearcodec/rlex.rs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,14 @@ pub fn decode_rlex(data: &[u8]) -> DecodeResult<RlexData> {
8383
// Each byte is a run length factor for palette[0]
8484
decode_single_palette_segments(&mut src, &mut segments)?;
8585
} else {
86-
decode_multi_palette_segments(remaining, &mut src, stop_index_bits, suite_depth_bits, &mut segments)?;
86+
decode_multi_palette_segments(
87+
remaining,
88+
&mut src,
89+
stop_index_bits,
90+
suite_depth_bits,
91+
palette_count,
92+
&mut segments,
93+
)?;
8794
}
8895

8996
Ok(RlexData { palette, segments })
@@ -106,6 +113,7 @@ fn decode_multi_palette_segments(
106113
src: &mut ReadCursor<'_>,
107114
stop_index_bits: u8,
108115
suite_depth_bits: u8,
116+
palette_count: u8,
109117
segments: &mut Vec<RlexSegment>,
110118
) -> DecodeResult<()> {
111119
let stop_mask = (1u8 << stop_index_bits) - 1;
@@ -116,6 +124,10 @@ fn decode_multi_palette_segments(
116124
let stop_index = packed & stop_mask;
117125
let suite_depth = (packed >> stop_index_bits) & depth_mask;
118126

127+
if stop_index >= palette_count {
128+
return Err(invalid_field_err!("rlexStopIndex", "stop_index exceeds palette count"));
129+
}
130+
119131
let start_index = stop_index.saturating_sub(suite_depth);
120132

121133
let run_length = decode_run_length(src)?;
@@ -211,4 +223,27 @@ mod tests {
211223
let data = [128]; // palette_count = 128 > 127
212224
assert!(decode_rlex(&data).is_err());
213225
}
226+
227+
#[test]
228+
fn reject_stop_index_beyond_palette() {
229+
// palette_count=2, stop_index_bits=1, so valid stop_index is 0 or 1.
230+
// Craft a packed byte with stop_index=1 (valid) then one with stop_index
231+
// that exceeds palette_count by using a 4-palette setup where the mask
232+
// allows index 3 but only 2 entries exist.
233+
//
234+
// palette_count=2, stop_index_bits = bit_length(1) = 1, stop_mask = 0x01
235+
// Maximum stop_index from 1 bit = 1, which equals palette_count-1. Valid.
236+
// So use palette_count=3 instead: stop_index_bits = bit_length(2) = 2,
237+
// stop_mask = 0x03. Maximum stop_index = 3, but palette only has 3 entries
238+
// (indices 0..2). stop_index=3 is invalid.
239+
let mut data = Vec::new();
240+
data.push(3); // palette_count
241+
data.extend_from_slice(&[0, 0, 0]); // palette[0]
242+
data.extend_from_slice(&[1, 1, 1]); // palette[1]
243+
data.extend_from_slice(&[2, 2, 2]); // palette[2]
244+
// packed byte: stop_index=3 (bits 1:0), suite_depth=0 (bits 7:2)
245+
data.push(0x03);
246+
data.push(1); // run_length
247+
assert!(decode_rlex(&data).is_err());
248+
}
214249
}

0 commit comments

Comments
 (0)