From 908408ba802ac661ac494f6be19873f5d68547f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Cortier?= Date: Sat, 9 Sep 2023 11:30:38 -0400 Subject: [PATCH] fix(web): optimize and fix present_with_damage (#150) Co-authored-by: daxpedda --- CHANGELOG.md | 1 + src/util.rs | 39 ++++++++++++++++++++++++++++++++++ src/web.rs | 59 +++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 85 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ed55f4..3819c23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # UNRELEASED * On X11, fix the length of the returned buffer when using the wire-transferred buffer. +* On Web, fix incorrect starting coordinates when handling buffer damage. # 0.3.0 diff --git a/src/util.rs b/src/util.rs index cce639e..c31c575 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,6 +1,10 @@ // Not needed on all platforms #![allow(dead_code)] +use std::cmp; +use std::num::NonZeroU32; + +use crate::Rect; use crate::SoftBufferError; /// Takes a mutable reference to a container and a function deriving a @@ -44,6 +48,41 @@ impl<'a, T: 'static + ?Sized, U: 'static + ?Sized> BorrowStack<'a, T, U> { } } +/// Calculates the smallest `Rect` necessary to represent all damaged `Rect`s. +pub(crate) fn union_damage(damage: &[Rect]) -> Option { + struct Region { + left: u32, + top: u32, + bottom: u32, + right: u32, + } + + let region = damage + .iter() + .map(|rect| Region { + left: rect.x, + top: rect.y, + right: rect.x + rect.width.get(), + bottom: rect.y + rect.height.get(), + }) + .reduce(|mut prev, next| { + prev.left = cmp::min(prev.left, next.left); + prev.top = cmp::min(prev.top, next.top); + prev.right = cmp::max(prev.right, next.right); + prev.bottom = cmp::max(prev.bottom, next.bottom); + prev + })?; + + Some(Rect { + x: region.left, + y: region.top, + width: NonZeroU32::new(region.right - region.left) + .expect("`right` must always be bigger then `left`"), + height: NonZeroU32::new(region.bottom - region.top) + .expect("`bottom` must always be bigger then `top`"), + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/web.rs b/src/web.rs index bbd1e71..eaf4da9 100644 --- a/src/web.rs +++ b/src/web.rs @@ -2,6 +2,10 @@ #![allow(clippy::uninlined_format_args)] +use std::convert::TryInto; +use std::marker::PhantomData; +use std::num::NonZeroU32; + use js_sys::Object; use raw_window_handle::WebWindowHandle; use wasm_bindgen::{JsCast, JsValue}; @@ -10,10 +14,7 @@ use web_sys::{CanvasRenderingContext2d, HtmlCanvasElement}; use web_sys::{OffscreenCanvas, OffscreenCanvasRenderingContext2d}; use crate::error::SwResultExt; -use crate::{Rect, SoftBufferError}; -use std::convert::TryInto; -use std::marker::PhantomData; -use std::num::NonZeroU32; +use crate::{util, Rect, SoftBufferError}; /// Display implementation for the web platform. /// @@ -138,19 +139,40 @@ impl WebImpl { } fn present_with_damage(&mut self, damage: &[Rect]) -> Result<(), SoftBufferError> { - let (width, _height) = self + let (buffer_width, _buffer_height) = self .size .expect("Must set size of surface before calling `present_with_damage()`"); + + let union_damage = if let Some(rect) = util::union_damage(damage) { + rect + } else { + return Ok(()); + }; + // Create a bitmap from the buffer. let bitmap: Vec<_> = self .buffer - .iter() + .chunks_exact(buffer_width.get() as usize) + .skip(union_damage.y as usize) + .take(union_damage.height.get() as usize) + .flat_map(|row| { + row.iter() + .skip(union_damage.x as usize) + .take(union_damage.width.get() as usize) + }) .copied() .flat_map(|pixel| [(pixel >> 16) as u8, (pixel >> 8) as u8, pixel as u8, 255]) .collect(); + debug_assert_eq!( + bitmap.len() as u32, + union_damage.width.get() * union_damage.height.get() * 4 + ); + #[cfg(target_feature = "atomics")] let result = { + // When using atomics, the underlying memory becomes `SharedArrayBuffer`, + // which can't be shared with `ImageData`. use js_sys::{Uint8Array, Uint8ClampedArray}; use wasm_bindgen::prelude::wasm_bindgen; @@ -166,13 +188,15 @@ impl WebImpl { let array = Uint8Array::new_with_length(bitmap.len() as u32); array.copy_from(&bitmap); let array = Uint8ClampedArray::new(&array); - ImageDataExt::new(array, width.get()) + ImageDataExt::new(array, union_damage.width.get()) .map(JsValue::from) .map(ImageData::unchecked_from_js) }; #[cfg(not(target_feature = "atomics"))] - let result = - ImageData::new_with_u8_clamped_array(wasm_bindgen::Clamped(&bitmap), width.get()); + let result = ImageData::new_with_u8_clamped_array( + wasm_bindgen::Clamped(&bitmap), + union_damage.width.get(), + ); // This should only throw an error if the buffer we pass's size is incorrect. let image_data = result.unwrap(); @@ -181,8 +205,10 @@ impl WebImpl { self.canvas .put_image_data( &image_data, - rect.x.into(), - rect.y.into(), + union_damage.x.into(), + union_damage.y.into(), + (rect.x - union_damage.x).into(), + (rect.y - union_damage.y).into(), rect.width.get().into(), rect.height.get().into(), ) @@ -274,22 +300,27 @@ impl Canvas { } } + // NOTE: suppress the lint because we mirror `CanvasRenderingContext2D.putImageData()`, and + // this is just an internal API used by this module only, so it's not too relevant. + #[allow(clippy::too_many_arguments)] fn put_image_data( &self, imagedata: &ImageData, dx: f64, dy: f64, - widht: f64, + dirty_x: f64, + dirty_y: f64, + width: f64, height: f64, ) -> Result<(), JsValue> { match self { Self::Canvas { ctx, .. } => ctx .put_image_data_with_dirty_x_and_dirty_y_and_dirty_width_and_dirty_height( - imagedata, dx, dy, dx, dy, widht, height, + imagedata, dx, dy, dirty_x, dirty_y, width, height, ), Self::OffscreenCanvas { ctx, .. } => ctx .put_image_data_with_dirty_x_and_dirty_y_and_dirty_width_and_dirty_height( - imagedata, dx, dy, dx, dy, widht, height, + imagedata, dx, dy, dirty_x, dirty_y, width, height, ), } }