-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: hook up resize for ironrdp-client crate #430
Changes from all commits
7776094
4dbb2b3
08dfb44
55159db
89bef14
717d020
d07d522
5c22b53
8a5c5f1
ffb4465
783b1d4
e1ec8eb
d72c77c
e93cea0
b382e81
8ec38e1
c72391a
c384c40
28c9f68
266c735
856a0aa
0dea439
69cc89b
ead24a5
e9cc83a
efaaf36
c4afa98
4b7725d
422a436
5f0840b
bc57654
205eacf
dd4906a
6e45da9
3c493d2
1e2dd93
0a16e2b
ceb44b8
28b5dc3
b9669f3
fb89407
bc26ef0
9503901
368df95
567e537
83be7c8
764abdd
56c05b6
f4658d9
bab4181
074f47d
66ceda2
d2e315e
371ac3a
db346b3
3cd8167
1de1666
af26842
6e51da5
89b28ac
94c176b
6837e94
10c2415
2359d00
3abe20f
9921483
50c649e
fdd2a6b
6945c5d
3a30629
0df68b5
622a4f4
a888bb1
4811deb
030469f
58049ec
5229b08
d57965e
2440098
83e18b8
91c357a
7ec0048
347a2d9
8793b16
1fd4b38
a07b8ec
6a7c4f8
bda1a96
4d878fb
1644ff7
a40adbb
ab2a8a3
09daceb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,8 @@ | ||||||||||||||||
use ironrdp::cliprdr::backend::{ClipboardMessage, CliprdrBackendFactory}; | ||||||||||||||||
use ironrdp::connector::connection_activation::ConnectionActivationState; | ||||||||||||||||
use ironrdp::connector::{ConnectionResult, ConnectorResult}; | ||||||||||||||||
use ironrdp::displaycontrol::client::DisplayControlClient; | ||||||||||||||||
use ironrdp::displaycontrol::pdu::MonitorLayoutEntry; | ||||||||||||||||
use ironrdp::graphics::image_processing::PixelFormat; | ||||||||||||||||
use ironrdp::pdu::input::fast_path::FastPathInputEvent; | ||||||||||||||||
use ironrdp::pdu::write_buf::WriteBuf; | ||||||||||||||||
|
@@ -28,7 +30,13 @@ pub enum RdpOutputEvent { | |||||||||||||||
|
||||||||||||||||
#[derive(Debug)] | ||||||||||||||||
pub enum RdpInputEvent { | ||||||||||||||||
Resize { width: u16, height: u16 }, | ||||||||||||||||
Resize { | ||||||||||||||||
width: u16, | ||||||||||||||||
height: u16, | ||||||||||||||||
scale_factor: u32, | ||||||||||||||||
physical_width: u32, | ||||||||||||||||
physical_height: u32, | ||||||||||||||||
}, | ||||||||||||||||
FastPath(SmallVec<[FastPathInputEvent; 2]>), | ||||||||||||||||
Close, | ||||||||||||||||
Clipboard(ClipboardMessage), | ||||||||||||||||
|
@@ -107,7 +115,9 @@ async fn connect( | |||||||||||||||
|
||||||||||||||||
let mut connector = connector::ClientConnector::new(config.connector.clone()) | ||||||||||||||||
.with_server_addr(server_addr) | ||||||||||||||||
.with_static_channel(ironrdp::dvc::DrdynvcClient::new()) | ||||||||||||||||
.with_static_channel( | ||||||||||||||||
ironrdp::dvc::DrdynvcClient::new().with_dynamic_channel(DisplayControlClient::new(|_| Ok(Vec::new()))), | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From our doc comment:
I think we need to block any dynamic resolution changes from the client until this callback (passed in new) receives some caps. Also, based on caps we should prevent setting resolution to values greater than max monitor area received in caps. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is possibility is already handled here: IronRDP/crates/ironrdp-client/src/rdp.rs Lines 207 to 213 in 4d878fb
I added this callback for Teleport purposes, where we do things a little differently:
Something similar to that could be implemented in IronRDP, but it's a substantial piece of work compared to what's there now (reconnect if the channel isn't ready).
Seems like a fine check but in practice it's not a big risk, I just added a TODO for this for now 1644ff7 |
||||||||||||||||
) | ||||||||||||||||
.with_static_channel(rdpsnd::Rdpsnd::new()) | ||||||||||||||||
.with_static_channel(rdpdr::Rdpdr::new(Box::new(NoopRdpdrBackend {}), "IronRDP".to_owned()).with_smartcard(0)); | ||||||||||||||||
|
||||||||||||||||
|
@@ -177,24 +187,30 @@ async fn active_session( | |||||||||||||||
let input_event = input_event.ok_or_else(|| session::general_err!("GUI is stopped"))?; | ||||||||||||||||
|
||||||||||||||||
match input_event { | ||||||||||||||||
RdpInputEvent::Resize { mut width, mut height } => { | ||||||||||||||||
// TODO(#105): Add support for Display Update Virtual Channel Extension | ||||||||||||||||
// https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpedisp/d2954508-f487-48bc-8731-39743e0854a9 | ||||||||||||||||
// One approach when this extension is not available is to perform a connection from scratch again. | ||||||||||||||||
|
||||||||||||||||
RdpInputEvent::Resize { mut width, mut height, .. } => { | ||||||||||||||||
// Find the last resize event | ||||||||||||||||
while let Ok(newer_event) = input_event_receiver.try_recv() { | ||||||||||||||||
if let RdpInputEvent::Resize { width: newer_width, height: newer_height } = newer_event { | ||||||||||||||||
if let RdpInputEvent::Resize { | ||||||||||||||||
width: newer_width, | ||||||||||||||||
height: newer_height, | ||||||||||||||||
.. | ||||||||||||||||
} = newer_event { | ||||||||||||||||
width = newer_width; | ||||||||||||||||
height = newer_height; | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
// TODO(#271): use the "auto-reconnect cookie": https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/15b0d1c9-2891-4adb-a45e-deb4aeeeab7c | ||||||||||||||||
|
||||||||||||||||
info!(width, height, "resize event"); | ||||||||||||||||
|
||||||||||||||||
return Ok(RdpControlFlow::ReconnectWithNewSize { width, height }) | ||||||||||||||||
let (width, height) = MonitorLayoutEntry::adjust_display_size(width.into(), height.into()); | ||||||||||||||||
debug!(width, height, "Adjusted display size"); | ||||||||||||||||
|
||||||||||||||||
if let Some(response_frame) = active_stage.encode_resize(width, height, None, Some((width, height))) { // Set physical width and height to the same as the pixel width and heighbbt per FreeRDP | ||||||||||||||||
vec![ActiveStageOutput::ResponseFrame(response_frame?)] | ||||||||||||||||
} else { | ||||||||||||||||
// TODO(#271): use the "auto-reconnect cookie": https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpbcgr/15b0d1c9-2891-4adb-a45e-deb4aeeeab7c | ||||||||||||||||
debug!("Reconnecting with new size"); | ||||||||||||||||
return Ok(RdpControlFlow::ReconnectWithNewSize { width: width.try_into().unwrap(), height: height.try_into().unwrap() }) | ||||||||||||||||
} | ||||||||||||||||
}, | ||||||||||||||||
RdpInputEvent::FastPath(events) => { | ||||||||||||||||
trace!(?events); | ||||||||||||||||
|
@@ -307,10 +323,10 @@ async fn active_session( | |||||||||||||||
pointer_software_rendering, | ||||||||||||||||
} = connection_activation.state | ||||||||||||||||
{ | ||||||||||||||||
debug!("Deactivation-Reactivation Sequence completed"); | ||||||||||||||||
debug!(?desktop_size, "Deactivation-Reactivation Sequence completed"); | ||||||||||||||||
// Update image size with the new desktop size. | ||||||||||||||||
image = DecodedImage::new(PixelFormat::RgbA32, desktop_size.width, desktop_size.height); | ||||||||||||||||
// Create a new [`FastPathProcessor`] with potentially updated | ||||||||||||||||
// io/user channel ids. | ||||||||||||||||
// Update the active stage with the new channel IDs and pointer settings. | ||||||||||||||||
active_stage.set_fastpath_processor( | ||||||||||||||||
fast_path::ProcessorBuilder { | ||||||||||||||||
io_channel_id, | ||||||||||||||||
|
@@ -320,6 +336,7 @@ async fn active_session( | |||||||||||||||
} | ||||||||||||||||
.build(), | ||||||||||||||||
); | ||||||||||||||||
active_stage.set_no_server_pointer(no_server_pointer); | ||||||||||||||||
break 'activation_seq; | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Can’t you use this? https://docs.rs/winit/latest/winit/window/struct.Window.html#method.inner_size
Is it really fine to feed in 0s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inner_size
gives you aPhysicalSize
, but that only gives you a "size represented in physical pixels." However without knowing the DPI (or other relation to a unit of length), we can't calculate the "physical size" that RDP wants, which is the width/height in millimeters.If we hand back
0
s here, they're simply ignored per the spec:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ibeckermayer
I think passing in client
(0, 0)
is not optimal (it is not obvious that they will be ignored), maybe it would be great to changeRdpInputEvent::Resize
fields to havephysical_dimensions: Option<(u32, u32)>
instead of separate u32 fields? This will keep this implementation detail as close to PDU encode/decode code as possibleThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be fixed in follow-up PR, merging PR to unblock @ibeckermayer