feat: add a callback mechanism to the DisplayControlClient
#424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 30 commits
March 19, 2024 20:51
…receiving a ServerDeactivateAll on the I/O channel
sequences into a separate connection_activation module. This allows us to reuse them when we receive a Server Deactivate All.
`single_connect_step_read` and `single_connect_step_write`. Also adds a `ConnectionActivationSequence::reset_clone` method to aid in reusing the sequence for Deactivate All PDU handling. Passes `ConnectionActivationSequence` to `x224::Processor` to hand back upon receiving a Deactivate All PDU.
…:Windows::RDS::DisplayControl DVC to work.
…to us from the server via the BlockType in the BlockHeader, so it needn't be rigidly tracked explicitly in DecodingContext. This makes the RFX pipeline more flexible, which in turn allows us to seamlessly use it when we get another sync sequence mid-stream due to having fielded a Server Deactivate PDU.
…ess_dyvc to DrdynvcClient::process
client codebase. This uses `encode_dvc_data` to encode the DVC data responses. Currently no dynamic channels are added so the code is untested (WIP).
in the form of `DisplayControlClient`
I attempted to transfer everything in ironrdp_pdu::dvc::display into it, but it required more refactoring that I'd expected to get it to play nicely with the no_std possibility. This also removes the requirement of keeping track of the channel_id in each `DynamicVirtualChannel`, and instead lets the `DynamicChannelSet` take care of returning that as needed.
`DvcPduEncode` trait simply inherits `PduEncode`. It is a marker trait to indicate that the implementor, when encoded, is ready to be wrapped in DataFirst and/or Data PDU(s). `MonitorLayoutPdu` is a copy of the pdu by the same name in the `ironrdp-pdu` crate. It is moved (well, copied for now) to the `ironrdp-dvc` crate and implements `DvcPduEncode`.
added previously. Also creates a `DrdynvcPdu` enum that implements `SvcPduEncode`, and begins migrating `DataFirstPdu` and `DataPdu` to the new `PduEncode` paradigm. In the process, also: - Moved `DynamicVirtualChannel` and `DynamicChannelSet` from `client.rs` to `lib.rs` since they can ultimately be used by both client and server.
…id of now superfluous DvcMessage<'a>
… This has been tested to work for screen resize. The refactored server side code has not been tested.
…bles testing in ironrdp-dvc by eliminating the test = false line in Cargo.toml
- Abstracts the CapabilitiesExchange and ConnectionFinalization sequences into a separate connection_activation module. This allows us to reuse them when we receive a Server Deactivate All. - Creates IoChannelPdu enum to be used to account for the potential of receiving a ServerDeactivateAll on the I/O channel
`single_connect_step_read` and `single_connect_step_write`. Also adds a `ConnectionActivationSequence::reset_clone` method to aid in reusing the sequence for Deactivate All PDU handling. Passes `ConnectionActivationSequence` to `x224::Processor` to hand back upon receiving a Deactivate All PDU.
…:Windows::RDS::DisplayControl DVC to work.
…to us from the server via the BlockType in the BlockHeader, so it needn't be rigidly tracked explicitly in DecodingContext. This makes the RFX pipeline more flexible, which in turn allows us to seamlessly use it when we get another sync sequence mid-stream due to having fielded a Server Deactivate PDU.
ibeckermayer
pushed a commit
to gravitational/teleport
that referenced
this pull request
Mar 27, 2024
pacmancoder
reviewed
Mar 27, 2024
pacmancoder
reviewed
Mar 27, 2024
pacmancoder
reviewed
Mar 27, 2024
pacmancoder
reviewed
Mar 27, 2024
|
For consistency with other APIs and flexibility to add more callbacks, shouldn't it use trait object instead? |
CBenoit
reviewed
Apr 2, 2024
| @@ -5,6 +5,7 @@ | |||
| use ironrdp_dvc::DvcPduEncode; | |||
| use ironrdp_pdu::cursor::{ReadCursor, WriteCursor}; | |||
| use ironrdp_pdu::{ensure_fixed_part_size, invalid_message_err, PduDecode, PduEncode, PduResult}; | |||
| use tracing::warn; | |||
There was a problem hiding this comment.
suggestion: Add #[macro_use] extern crate tracing in the lib.rs file of this crate instead. The logging macros are used pervasively so it’s fine.
@ibeckermayer If more callbacks are to be added in the future, I also recommend a trait instead of increasing the number of parameters. |
CBenoit
changed the title
DisplayControlClientDisplayControlClient
github-merge-queue bot
pushed a commit
to gravitational/teleport
that referenced
this pull request
Apr 29, 2024
* Resize * ??? * ??? * Fixes to compile * Updates to latest IronRDP with dvc fix, cleans up some of the debug code * Hooks up resize listener on the client and sends changes as ClientScreenSpecs to the backend. Currently just gets logged in the WDS * Handles the new DeactivateAll ProcessorOutput * hooks up client_write_screen_resize * Renames tdp Connection Initialized to Connection Activated This message is now used both after the connection is first established and after the connection executes a Deactivation-Reactivation Sequence. This fits snugly in to our existing message flow and ensures that the fast path processor is updated appropriately whenever a Deactivation-Reactivation Sequence is executed due to a resize event. * Switch monitor orientation based on width/height * remove superfluous canvas size sync * Updates to the most up-to-date hash for IronRDP (not merged to master). Screen resize works in this commit, though its still the case that if you do so before the channel is announced by the rdp server, it will end the session * Updates to the most recent hash for IronRDP feat/dynamic-resize * Adds a mechanism to withhold sending resize requests to the RDP server until we're alerted by a new callback mechanism of the DisplayControlClient that the server has sent us its capabilities. This prevents an error where we send a resize before this event, which results either in an RDP error (if the channel is not opened) or a dropped event (if the server has not sent its capabilities yet). * Refactoring after update to the tip of Devolutions/IronRDP#424 * Adds windowOnResize to story * adds Determine Toolchain Versions and cache paths and Print versions steps to os-compatibility-test.yaml * fixes debug log formatting * Updates to local IronRDP and later IronRDP api * updates to IronRDP latest rev on Devolutions/IronRDP#430 * reverting mistaken changes * throttle -> debounce and naming changes * js renamings * updates to IronRDP master --------- Co-authored-by: Przemko Robakowski <[email protected]>
ibeckermayer
pushed a commit
to gravitational/teleport
that referenced
this pull request
Apr 29, 2024
* Resize * ??? * ??? * Fixes to compile * Updates to latest IronRDP with dvc fix, cleans up some of the debug code * Hooks up resize listener on the client and sends changes as ClientScreenSpecs to the backend. Currently just gets logged in the WDS * Handles the new DeactivateAll ProcessorOutput * hooks up client_write_screen_resize * Renames tdp Connection Initialized to Connection Activated This message is now used both after the connection is first established and after the connection executes a Deactivation-Reactivation Sequence. This fits snugly in to our existing message flow and ensures that the fast path processor is updated appropriately whenever a Deactivation-Reactivation Sequence is executed due to a resize event. * Switch monitor orientation based on width/height * remove superfluous canvas size sync * Updates to the most up-to-date hash for IronRDP (not merged to master). Screen resize works in this commit, though its still the case that if you do so before the channel is announced by the rdp server, it will end the session * Updates to the most recent hash for IronRDP feat/dynamic-resize * Adds a mechanism to withhold sending resize requests to the RDP server until we're alerted by a new callback mechanism of the DisplayControlClient that the server has sent us its capabilities. This prevents an error where we send a resize before this event, which results either in an RDP error (if the channel is not opened) or a dropped event (if the server has not sent its capabilities yet). * Refactoring after update to the tip of Devolutions/IronRDP#424 * Adds windowOnResize to story * adds Determine Toolchain Versions and cache paths and Print versions steps to os-compatibility-test.yaml * fixes debug log formatting * Updates to local IronRDP and later IronRDP api * updates to IronRDP latest rev on Devolutions/IronRDP#430 * reverting mistaken changes * throttle -> debounce and naming changes * js renamings * updates to IronRDP master --------- Co-authored-by: Przemko Robakowski <[email protected]>
github-merge-queue bot
pushed a commit
to gravitational/teleport
that referenced
this pull request
Apr 30, 2024
* Resize * ??? * ??? * Fixes to compile * Updates to latest IronRDP with dvc fix, cleans up some of the debug code * Hooks up resize listener on the client and sends changes as ClientScreenSpecs to the backend. Currently just gets logged in the WDS * Handles the new DeactivateAll ProcessorOutput * hooks up client_write_screen_resize * Renames tdp Connection Initialized to Connection Activated This message is now used both after the connection is first established and after the connection executes a Deactivation-Reactivation Sequence. This fits snugly in to our existing message flow and ensures that the fast path processor is updated appropriately whenever a Deactivation-Reactivation Sequence is executed due to a resize event. * Switch monitor orientation based on width/height * remove superfluous canvas size sync * Updates to the most up-to-date hash for IronRDP (not merged to master). Screen resize works in this commit, though its still the case that if you do so before the channel is announced by the rdp server, it will end the session * Updates to the most recent hash for IronRDP feat/dynamic-resize * Adds a mechanism to withhold sending resize requests to the RDP server until we're alerted by a new callback mechanism of the DisplayControlClient that the server has sent us its capabilities. This prevents an error where we send a resize before this event, which results either in an RDP error (if the channel is not opened) or a dropped event (if the server has not sent its capabilities yet). * Refactoring after update to the tip of Devolutions/IronRDP#424 * Adds windowOnResize to story * adds Determine Toolchain Versions and cache paths and Print versions steps to os-compatibility-test.yaml * fixes debug log formatting * Updates to local IronRDP and later IronRDP api * updates to IronRDP latest rev on Devolutions/IronRDP#430 * reverting mistaken changes * throttle -> debounce and naming changes * js renamings * updates to IronRDP master --------- Co-authored-by: Przemko Robakowski <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
which is called when the server sends the
DisplayControlCapabilities(indicating that the client is ready for use).(Replacement for #422)