Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ fn_to_numeric_cast_any = "warn"
ptr_cast_constness = "warn"

# == Correctness == #
#indexing_slicing = "warn" TODO: enable this lint project wide.
#as_conversions = "warn"
cast_lossless = "warn"
cast_possible_truncation = "warn"
Expand Down
6 changes: 2 additions & 4 deletions crates/ironrdp-web/src/canvas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ impl Canvas {
let region_height = region.height();

let mut src = buffer.chunks_exact(4).map(|pixel| {
let r = pixel[0];
let g = pixel[1];
let b = pixel[2];
u32::from_be_bytes([0, r, g, b])
let [r, g, b] = pixel.first_chunk::<3>().expect("cannot be out of bounds");
u32::from_be_bytes([0, *r, *g, *b])
});

let mut dst = self.surface.buffer_mut().expect("surface buffer");
Expand Down
43 changes: 35 additions & 8 deletions crates/ironrdp-web/src/image.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
#![allow(clippy::arithmetic_side_effects)]

use anyhow::Context as _;
use ironrdp::pdu::geometry::{InclusiveRectangle, Rectangle as _};
use ironrdp::session::image::DecodedImage;

pub(crate) fn extract_partial_image(image: &DecodedImage, region: InclusiveRectangle) -> (InclusiveRectangle, Vec<u8>) {
pub(crate) fn extract_partial_image(
image: &DecodedImage,
region: InclusiveRectangle,
) -> anyhow::Result<(InclusiveRectangle, Vec<u8>)> {
// PERF: needs actual benchmark to find a better heuristic
if region.height() > 64 || region.width() > 512 {
extract_whole_rows(image, region)
Expand All @@ -13,7 +17,10 @@ pub(crate) fn extract_partial_image(image: &DecodedImage, region: InclusiveRecta
}

// Faster for low-height and smaller images
fn extract_smallest_rectangle(image: &DecodedImage, region: InclusiveRectangle) -> (InclusiveRectangle, Vec<u8>) {
fn extract_smallest_rectangle(
image: &DecodedImage,
region: InclusiveRectangle,
) -> anyhow::Result<(InclusiveRectangle, Vec<u8>)> {
let pixel_size = usize::from(image.pixel_format().bytes_per_pixel());

let image_width = usize::from(image.width());
Expand All @@ -33,20 +40,31 @@ fn extract_smallest_rectangle(image: &DecodedImage, region: InclusiveRectangle)
for row in 0..region_height {
let src_begin = image_stride * (region_top + row) + region_left * pixel_size;
let src_end = src_begin + region_stride;
let src_slice = &src[src_begin..src_end];
let src_slice = src.get(src_begin..src_end).with_context(|| {
format!(
"invalid region {region:?} for image with dimensions {}x{}",
image.width(),
image.height()
)
})?;
Comment on lines +43 to +49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preciously this was subject to panics upon bad input?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I think it was possible to trigger panic if passing an empty image and any valid region.


let target_begin = region_stride * row;
let target_end = target_begin + region_stride;
let target_slice = &mut dst[target_begin..target_end];
let target_slice = dst
.get_mut(target_begin..target_end)
.expect("slice index cannot be out of bounds");

target_slice.copy_from_slice(src_slice);
}

(region, dst)
Ok((region, dst))
}

// Faster for high-height and bigger images
fn extract_whole_rows(image: &DecodedImage, region: InclusiveRectangle) -> (InclusiveRectangle, Vec<u8>) {
fn extract_whole_rows(
image: &DecodedImage,
region: InclusiveRectangle,
) -> anyhow::Result<(InclusiveRectangle, Vec<u8>)> {
let pixel_size = usize::from(image.pixel_format().bytes_per_pixel());

let image_width = usize::from(image.width());
Expand All @@ -60,7 +78,16 @@ fn extract_whole_rows(image: &DecodedImage, region: InclusiveRectangle) -> (Incl
let src_begin = region_top * image_stride;
let src_end = (region_bottom + 1) * image_stride;

let dst = src[src_begin..src_end].to_vec();
let dst = src
.get(src_begin..src_end)
.with_context(|| {
format!(
"invalid region {region:?} for image with dimensions {}x{}",
image.width(),
image.height()
)
})?
.to_vec();

let wider_region = InclusiveRectangle {
left: 0,
Expand All @@ -69,5 +96,5 @@ fn extract_whole_rows(image: &DecodedImage, region: InclusiveRectangle) -> (Incl
bottom: region.bottom,
};

(wider_region, dst)
Ok((wider_region, dst))
}
2 changes: 1 addition & 1 deletion crates/ironrdp-web/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ impl iron_remote_desktop::Session for Session {
}
ActiveStageOutput::GraphicsUpdate(region) => {
// PERF: some copies and conversion could be optimized
let (region, buffer) = extract_partial_image(&image, region);
let (region, buffer) = extract_partial_image(&image, region)?;
gui.draw(&buffer, region).context("draw updated region")?;
}
ActiveStageOutput::PointerDefault => {
Expand Down
Loading