Skip to content

Commit 21e12d3

Browse files
elmarcoelmarco
committed
refactor(server): make UpdateEncoder::update() an iterator
A single display update can now result in multiple commands / update code (FastPathUpdate). The update dispatching and bitmap encoding is now done by the UpdateEncoder itself. Signed-off-by: Marc-André Lureau <[email protected]>
1 parent 5068740 commit 21e12d3

File tree

4 files changed

+73
-43
lines changed

4 files changed

+73
-43
lines changed

‎crates/ironrdp-server/src/encoder/bitmap.rs

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use ironrdp_pdu::geometry::InclusiveRectangle;
77
use crate::BitmapUpdate;
88

99
// PERF: we could also remove the need for this buffer
10+
#[derive(Clone)]
1011
pub(crate) struct BitmapEncoder {
1112
buffer: Vec<u8>,
1213
}

‎crates/ironrdp-server/src/encoder/mod.rs

+49-11
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use ironrdp_pdu::surface_commands::{ExtendedBitmapDataPdu, SurfaceBitsPdu, Surfa
1212
use self::bitmap::BitmapEncoder;
1313
use self::rfx::RfxEncoder;
1414
use super::BitmapUpdate;
15-
use crate::{ColorPointer, Framebuffer, RGBAPointer};
15+
use crate::{time_warn, ColorPointer, DisplayUpdate, Framebuffer, RGBAPointer};
1616

1717
mod bitmap;
1818
mod fast_path;
@@ -59,12 +59,19 @@ impl UpdateEncoder {
5959
}
6060
}
6161

62+
pub(crate) fn update(&mut self, update: DisplayUpdate) -> EncoderIter<'_> {
63+
EncoderIter {
64+
encoder: self,
65+
update: Some(update),
66+
}
67+
}
68+
6269
pub(crate) fn set_desktop_size(&mut self, size: DesktopSize) {
6370
self.desktop_size = size;
6471
}
6572

6673
#[allow(clippy::unused_self)]
67-
pub(crate) fn rgba_pointer(&mut self, ptr: RGBAPointer) -> Result<UpdateFragmenter> {
74+
fn rgba_pointer(&self, ptr: RGBAPointer) -> Result<UpdateFragmenter> {
6875
let xor_mask = ptr.data;
6976

7077
let hot_spot = Point16 {
@@ -87,7 +94,7 @@ impl UpdateEncoder {
8794
}
8895

8996
#[allow(clippy::unused_self)]
90-
pub(crate) fn color_pointer(&mut self, ptr: ColorPointer) -> Result<UpdateFragmenter> {
97+
fn color_pointer(&self, ptr: ColorPointer) -> Result<UpdateFragmenter> {
9198
let hot_spot = Point16 {
9299
x: ptr.hot_x,
93100
y: ptr.hot_y,
@@ -104,22 +111,28 @@ impl UpdateEncoder {
104111
}
105112

106113
#[allow(clippy::unused_self)]
107-
pub(crate) fn default_pointer(&mut self) -> Result<UpdateFragmenter> {
114+
fn default_pointer(&self) -> Result<UpdateFragmenter> {
108115
Ok(UpdateFragmenter::new(UpdateCode::DefaultPointer, vec![]))
109116
}
110117

111118
#[allow(clippy::unused_self)]
112-
pub(crate) fn hide_pointer(&mut self) -> Result<UpdateFragmenter> {
119+
fn hide_pointer(&self) -> Result<UpdateFragmenter> {
113120
Ok(UpdateFragmenter::new(UpdateCode::HiddenPointer, vec![]))
114121
}
115122

116123
#[allow(clippy::unused_self)]
117-
pub(crate) fn pointer_position(&mut self, pos: PointerPositionAttribute) -> Result<UpdateFragmenter> {
124+
fn pointer_position(&self, pos: PointerPositionAttribute) -> Result<UpdateFragmenter> {
118125
Ok(UpdateFragmenter::new(UpdateCode::PositionPointer, encode_vec(&pos)?))
119126
}
120127

121-
pub(crate) fn bitmap(&mut self, bitmap: BitmapUpdate) -> Result<UpdateFragmenter> {
122-
let res = self.bitmap_updater.handle(&bitmap);
128+
async fn bitmap(&mut self, bitmap: BitmapUpdate) -> Result<UpdateFragmenter> {
129+
// Clone to satisfy spawn_blocking 'static requirement
130+
// this should be cheap, even if using bitmap, since vec![] will be empty
131+
let mut updater = self.bitmap_updater.clone();
132+
let (res, bitmap) =
133+
tokio::task::spawn_blocking(move || time_warn!("Encoding bitmap", 10, (updater.handle(&bitmap), bitmap)))
134+
.await
135+
.unwrap();
123136
if bitmap.x == 0
124137
&& bitmap.y == 0
125138
&& bitmap.width.get() == self.desktop_size.width
@@ -134,7 +147,31 @@ impl UpdateEncoder {
134147
}
135148
}
136149

137-
#[derive(Debug)]
150+
pub(crate) struct EncoderIter<'a> {
151+
encoder: &'a mut UpdateEncoder,
152+
update: Option<DisplayUpdate>,
153+
}
154+
155+
impl EncoderIter<'_> {
156+
pub(crate) async fn next(&mut self) -> Option<Result<UpdateFragmenter>> {
157+
let update = self.update.take()?;
158+
let encoder = &mut self.encoder;
159+
160+
let res = match update {
161+
DisplayUpdate::Bitmap(bitmap) => encoder.bitmap(bitmap).await,
162+
DisplayUpdate::PointerPosition(pos) => encoder.pointer_position(pos),
163+
DisplayUpdate::RGBAPointer(ptr) => encoder.rgba_pointer(ptr),
164+
DisplayUpdate::ColorPointer(ptr) => encoder.color_pointer(ptr),
165+
DisplayUpdate::HidePointer => encoder.hide_pointer(),
166+
DisplayUpdate::DefaultPointer => encoder.default_pointer(),
167+
DisplayUpdate::Resize(_) => return None,
168+
};
169+
170+
Some(res)
171+
}
172+
}
173+
174+
#[derive(Debug, Clone)]
138175
enum BitmapUpdater {
139176
None(NoneHandler),
140177
Bitmap(BitmapHandler),
@@ -155,7 +192,7 @@ trait BitmapUpdateHandler {
155192
fn handle(&mut self, bitmap: &BitmapUpdate) -> Result<UpdateFragmenter>;
156193
}
157194

158-
#[derive(Debug)]
195+
#[derive(Clone, Debug)]
159196
struct NoneHandler;
160197

161198
impl BitmapUpdateHandler for NoneHandler {
@@ -169,6 +206,7 @@ impl BitmapUpdateHandler for NoneHandler {
169206
}
170207
}
171208

209+
#[derive(Clone)]
172210
struct BitmapHandler {
173211
bitmap: BitmapEncoder,
174212
}
@@ -209,7 +247,7 @@ impl BitmapUpdateHandler for BitmapHandler {
209247
}
210248
}
211249

212-
#[derive(Debug)]
250+
#[derive(Debug, Clone)]
213251
struct RemoteFxHandler {
214252
remotefx: RfxEncoder,
215253
codec_id: u8,

‎crates/ironrdp-server/src/encoder/rfx.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use ironrdp_pdu::WriteCursor;
1111

1212
use crate::BitmapUpdate;
1313

14-
#[derive(Debug)]
14+
#[derive(Debug, Clone)]
1515
pub(crate) struct RfxEncoder {
1616
entropy_algorithm: rfx::EntropyAlgorithm,
1717
}

‎crates/ironrdp-server/src/server.rs

+22-31
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use crate::clipboard::CliprdrServerFactory;
3232
use crate::display::{DisplayUpdate, RdpServerDisplay};
3333
use crate::encoder::UpdateEncoder;
3434
use crate::handler::RdpServerInputHandler;
35-
use crate::{builder, capabilities, time_warn, SoundServerFactory};
35+
use crate::{builder, capabilities, SoundServerFactory};
3636

3737
#[derive(Clone)]
3838
pub struct RdpServerOptions {
@@ -423,39 +423,30 @@ impl RdpServer {
423423
buffer: &mut Vec<u8>,
424424
mut encoder: UpdateEncoder,
425425
) -> Result<(RunState, UpdateEncoder)> {
426-
let mut fragmenter = match update {
427-
DisplayUpdate::Bitmap(bitmap) => {
428-
let (enc, res) = task::spawn_blocking(move || {
429-
let res = time_warn!("Encoding bitmap", 10, encoder.bitmap(bitmap));
430-
(encoder, res)
431-
})
432-
.await?;
433-
encoder = enc;
434-
res
435-
}
436-
DisplayUpdate::PointerPosition(pos) => encoder.pointer_position(pos),
437-
DisplayUpdate::Resize(desktop_size) => {
438-
debug!(?desktop_size, "Display resize");
439-
encoder.set_desktop_size(desktop_size);
440-
deactivate_all(io_channel_id, user_channel_id, writer).await?;
441-
return Ok((RunState::DeactivationReactivation { desktop_size }, encoder));
442-
}
443-
DisplayUpdate::RGBAPointer(ptr) => encoder.rgba_pointer(ptr),
444-
DisplayUpdate::ColorPointer(ptr) => encoder.color_pointer(ptr),
445-
DisplayUpdate::HidePointer => encoder.hide_pointer(),
446-
DisplayUpdate::DefaultPointer => encoder.default_pointer(),
426+
if let DisplayUpdate::Resize(desktop_size) = update {
427+
debug!(?desktop_size, "Display resize");
428+
encoder.set_desktop_size(desktop_size);
429+
deactivate_all(io_channel_id, user_channel_id, writer).await?;
430+
return Ok((RunState::DeactivationReactivation { desktop_size }, encoder));
447431
}
448-
.context("error during update encoding")?;
449432

450-
if fragmenter.size_hint() > buffer.len() {
451-
buffer.resize(fragmenter.size_hint(), 0);
452-
}
433+
let mut encoder_iter = encoder.update(update);
434+
loop {
435+
let Some(fragmenter) = encoder_iter.next().await else {
436+
break;
437+
};
453438

454-
while let Some(len) = fragmenter.next(buffer) {
455-
writer
456-
.write_all(&buffer[..len])
457-
.await
458-
.context("failed to write display update")?;
439+
let mut fragmenter = fragmenter.context("error while encoding")?;
440+
if fragmenter.size_hint() > buffer.len() {
441+
buffer.resize(fragmenter.size_hint(), 0);
442+
}
443+
444+
while let Some(len) = fragmenter.next(buffer) {
445+
writer
446+
.write_all(&buffer[..len])
447+
.await
448+
.context("failed to write display update")?;
449+
}
459450
}
460451

461452
Ok((RunState::Continue, encoder))

0 commit comments

Comments
 (0)