-
Notifications
You must be signed in to change notification settings - Fork 220
perf(web): replace softbuffer with direct put_image_data canvas present #1374
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
Changes from all commits
5be56f9
45fb1a6
67d73ff
6fc670e
13b7b9a
eb072cc
ad0dc5d
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,93 +1,82 @@ | ||
| use core::num::NonZeroU32; | ||
|
|
||
| use anyhow::Context as _; | ||
| use ironrdp::pdu::geometry::{InclusiveRectangle, Rectangle as _}; | ||
| use softbuffer::{NoDisplayHandle, NoWindowHandle}; | ||
| use web_sys::HtmlCanvasElement; | ||
|
|
||
| #[cfg(target_arch = "wasm32")] | ||
| use anyhow::anyhow; | ||
| use ironrdp::pdu::geometry::InclusiveRectangle; | ||
| #[cfg(target_arch = "wasm32")] | ||
| use ironrdp::pdu::geometry::Rectangle as _; | ||
| #[cfg(target_arch = "wasm32")] | ||
| use wasm_bindgen::{Clamped, JsCast as _}; | ||
| #[cfg(target_arch = "wasm32")] | ||
| use web_sys::ImageData; | ||
| use web_sys::{CanvasRenderingContext2d, HtmlCanvasElement}; | ||
|
|
||
| /// Web render surface: blits each dirty region to the canvas with `put_image_data`. | ||
| pub(crate) struct Canvas { | ||
| width: NonZeroU32, | ||
| surface: softbuffer::Surface<NoDisplayHandle, NoWindowHandle>, | ||
| canvas: HtmlCanvasElement, | ||
| ctx: CanvasRenderingContext2d, | ||
| } | ||
|
|
||
| impl Canvas { | ||
| pub(crate) fn new(render_canvas: HtmlCanvasElement, width: NonZeroU32, height: NonZeroU32) -> anyhow::Result<Self> { | ||
| render_canvas.set_width(width.get()); | ||
| render_canvas.set_height(height.get()); | ||
| let ctx = context_2d(&render_canvas)?; | ||
|
|
||
| #[cfg(target_arch = "wasm32")] | ||
| let mut surface = { | ||
| use softbuffer::SurfaceExtWeb as _; | ||
| softbuffer::Surface::from_canvas(render_canvas).expect("surface") | ||
| }; | ||
|
|
||
| #[cfg(not(target_arch = "wasm32"))] | ||
| let mut surface = { | ||
| fn stub(_: HtmlCanvasElement) -> softbuffer::Surface<NoDisplayHandle, NoWindowHandle> { | ||
| unimplemented!() | ||
| } | ||
|
|
||
| stub(render_canvas) | ||
| }; | ||
|
|
||
| surface.resize(width, height).expect("surface resize"); | ||
|
|
||
| Ok(Self { width, surface }) | ||
| Ok(Self { | ||
| canvas: render_canvas, | ||
| ctx, | ||
| }) | ||
| } | ||
|
|
||
| /// Resizes the backing store. Note: this also clears the canvas and resets 2D context state; | ||
| /// the cached `ctx` stays valid. | ||
| pub(crate) fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) { | ||
| self.surface.resize(width, height).expect("surface resize"); | ||
| self.width = width; | ||
| self.canvas.set_width(width.get()); | ||
| self.canvas.set_height(height.get()); | ||
| } | ||
|
|
||
| pub(crate) fn draw(&mut self, buffer: &[u8], region: InclusiveRectangle) -> anyhow::Result<()> { | ||
| let region_width = region.width(); | ||
| 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 mut dst = self.surface.buffer_mut().expect("surface buffer"); | ||
| /// Blits a dirty region with `put_image_data`. Forces alpha opaque first: the framebuffer isn't | ||
| /// guaranteed opaque (zero-init columns, QOI-RGBA) and `put_image_data` stores alpha verbatim. | ||
| pub(crate) fn draw(&self, buffer: &mut [u8], region: InclusiveRectangle) -> anyhow::Result<()> { | ||
| for pixel in buffer.chunks_exact_mut(4) { | ||
| pixel[3] = 0xFF; | ||
| } | ||
|
Comment on lines
+42
to
44
Member
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. question: Just challenging the established pattern here: do we really need to force opaque alpha? I assume that if we don’t do that we end up with visual artifacts? My understanding is that we are just extracting a sub-image from an otherwise fully rendered image, and that we don’t really need any extra cleaning step.
Contributor
Author
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. Benoît Cortier (@CBenoit) yeah, works for most cases but it's not guaranteed. Framebuffer starts zero-filled (here) and the QOI-RGBA path keeps source alpha (here) — plus a tall update gets widened to full width, so early / post-resize frames can upload not-yet-painted columns as transparent. Best case we'd guarantee opaque upstream (init the framebuffer to
Member
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. Sounds good to me! My only suggestion then is to track this with an issue for visibility |
||
|
|
||
| #[cfg(target_arch = "wasm32")] | ||
| { | ||
| // Copy src into dst | ||
|
|
||
| let region_top_usize = usize::from(region.top); | ||
| let region_height_usize = usize::from(region_height); | ||
| let region_left_usize = usize::from(region.left); | ||
| let region_width_usize = usize::from(region_width); | ||
|
|
||
| for dst_row in dst | ||
| .chunks_exact_mut(usize::try_from(self.width.get()).context("canvas width")?) | ||
| .skip(region_top_usize) | ||
| .take(region_height_usize) | ||
| { | ||
| let src_row = src.by_ref().take(region_width_usize); | ||
|
|
||
| dst_row | ||
| .iter_mut() | ||
| .skip(region_left_usize) | ||
| .take(region_width_usize) | ||
| .zip(src_row) | ||
| .for_each(|(dst, src)| *dst = src); | ||
| } | ||
| let image = ImageData::new_with_u8_clamped_array_and_sh( | ||
| Clamped(&*buffer), | ||
| u32::from(region.width()), | ||
| u32::from(region.height()), | ||
| ) | ||
| .map_err(|err| anyhow!("ImageData::new failed: {err:?}"))?; | ||
| self.ctx | ||
| .put_image_data(&image, f64::from(region.left), f64::from(region.top)) | ||
| .map_err(|err| anyhow!("put_image_data failed: {err:?}")) | ||
| } | ||
| #[cfg(not(target_arch = "wasm32"))] | ||
| { | ||
| let _ = (&self.ctx, buffer, region); | ||
| unimplemented!("web canvas is only available on wasm32") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let damage_rect = softbuffer::Rect { | ||
| x: u32::from(region.left), | ||
| y: u32::from(region.top), | ||
| width: NonZeroU32::new(u32::from(region_width)) | ||
| .expect("per InclusiveRectangle invariants: 0 < region_width"), | ||
| height: NonZeroU32::new(u32::from(region_height)) | ||
| .expect("per InclusiveRectangle invariants: 0 < region_height"), | ||
| }; | ||
|
|
||
| dst.present_with_damage(&[damage_rect]).expect("buffer present"); | ||
|
|
||
| Ok(()) | ||
| /// Acquires the canvas 2D context (wasm only; panics on other targets). | ||
| fn context_2d(canvas: &HtmlCanvasElement) -> anyhow::Result<CanvasRenderingContext2d> { | ||
| #[cfg(target_arch = "wasm32")] | ||
| { | ||
| canvas | ||
| .get_context("2d") | ||
| .map_err(|err| anyhow!("get_context(\"2d\") failed: {err:?}"))? | ||
| .ok_or_else(|| anyhow!("canvas has no 2d context"))? | ||
| .dyn_into::<CanvasRenderingContext2d>() | ||
| .map_err(|_| anyhow!("2d context is not a CanvasRenderingContext2d")) | ||
| } | ||
| #[cfg(not(target_arch = "wasm32"))] | ||
| { | ||
| let _ = canvas; | ||
| unimplemented!("web canvas is only available on wasm32") | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,18 +2,29 @@ | |
|
|
||
| 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>) { | ||
| use ironrdp_core::WriteBuf; | ||
|
|
||
| /// Copies the dirty `region` into `buffer` from its current cursor (clear it between regions). | ||
| /// The returned rect may be wider than `region`: the whole-rows path widens to full image width. | ||
| pub(crate) fn extract_partial_image( | ||
| image: &DecodedImage, | ||
| region: InclusiveRectangle, | ||
| buffer: &mut WriteBuf, | ||
| ) -> InclusiveRectangle { | ||
| // PERF: needs actual benchmark to find a better heuristic | ||
|
Member
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. Thought: Since you have the tools for benchmarking, I think it may be worth to look into this comment as well. As a follow up 🙂 |
||
| if region.height() > 64 || region.width() > 512 { | ||
| extract_whole_rows(image, region) | ||
| extract_whole_rows(image, region, buffer) | ||
| } else { | ||
| extract_smallest_rectangle(image, region) | ||
| extract_smallest_rectangle(image, region, buffer) | ||
| } | ||
| } | ||
|
|
||
| // 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, | ||
| buffer: &mut WriteBuf, | ||
| ) -> InclusiveRectangle { | ||
| let pixel_size = usize::from(image.pixel_format().bytes_per_pixel()); | ||
|
|
||
| let image_width = usize::from(image.width()); | ||
|
|
@@ -26,7 +37,7 @@ fn extract_smallest_rectangle(image: &DecodedImage, region: InclusiveRectangle) | |
| let region_stride = region_width * pixel_size; | ||
|
|
||
| let dst_buf_size = region_width * region_height * pixel_size; | ||
| let mut dst = vec![0; dst_buf_size]; | ||
| let dst = buffer.unfilled_to(dst_buf_size); | ||
|
|
||
| let src = image.data(); | ||
|
|
||
|
|
@@ -42,11 +53,13 @@ fn extract_smallest_rectangle(image: &DecodedImage, region: InclusiveRectangle) | |
| target_slice.copy_from_slice(src_slice); | ||
| } | ||
|
|
||
| (region, dst) | ||
| buffer.advance(dst_buf_size); | ||
|
|
||
| region | ||
| } | ||
|
|
||
| // 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, buffer: &mut WriteBuf) -> InclusiveRectangle { | ||
| let pixel_size = usize::from(image.pixel_format().bytes_per_pixel()); | ||
|
|
||
| let image_width = usize::from(image.width()); | ||
|
|
@@ -59,15 +72,15 @@ 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 len = src_end - src_begin; | ||
|
|
||
| let dst = src[src_begin..src_end].to_vec(); | ||
| buffer.unfilled_to(len).copy_from_slice(&src[src_begin..src_end]); | ||
| buffer.advance(len); | ||
|
|
||
| let wider_region = InclusiveRectangle { | ||
| InclusiveRectangle { | ||
| left: 0, | ||
| top: region.top, | ||
| right: image.width() - 1, | ||
| bottom: region.bottom, | ||
| }; | ||
|
|
||
| (wider_region, dst) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.