diff --git a/moli-canvas/src/blit.rs b/moli-canvas/src/blit.rs index 89efafb346..e0dd7320aa 100644 --- a/moli-canvas/src/blit.rs +++ b/moli-canvas/src/blit.rs @@ -1,27 +1,22 @@ use crate::pixel::{copy_rgba8_rect, scale_rgba8}; -use crate::types::{DrawImageBlit, Rgba8Rect, ScaleFilter, byte_len, surface_matches_len}; +use crate::types::{ + DrawImageBlit, ImageDataRect, Rgba8Rect, ScaleFilter, byte_len, surface_matches_len, +}; pub fn extract_image_data( pixels: &[u8], canvas_width: u32, canvas_height: u32, - source_x: i32, - source_y: i32, - width: u32, - height: u32, + rect: ImageDataRect, ) -> Vec { + let (width, height) = (rect.width, rect.height); let mut out = vec![0; byte_len(width, height).unwrap_or(0)]; if !surface_matches_len(pixels, canvas_width, canvas_height) { return out; } - let Some((source_rect, dest_x, dest_y)) = clipped_source_rect_for_extract( - source_x, - source_y, - width, - height, - canvas_width, - canvas_height, - ) else { + let Some((source_rect, dest_x, dest_y)) = + clipped_source_rect_for_extract(rect, canvas_width, canvas_height) + else { return out; }; let _ = copy_rgba8_rect( @@ -39,17 +34,14 @@ pub fn extract_image_data( } fn clipped_source_rect_for_extract( - source_x: i32, - source_y: i32, - width: u32, - height: u32, + rect: ImageDataRect, canvas_width: u32, canvas_height: u32, ) -> Option<(Rgba8Rect, u32, u32)> { - let left = source_x as i64; - let top = source_y as i64; - let right = left.checked_add(width as i64)?; - let bottom = top.checked_add(height as i64)?; + let left = rect.x; + let top = rect.y; + let right = left + i64::from(rect.width); + let bottom = top + i64::from(rect.height); let copy_left = left.max(0).min(canvas_width as i64); let copy_top = top.max(0).min(canvas_height as i64); let copy_right = right.max(0).min(canvas_width as i64); @@ -80,15 +72,12 @@ pub fn blit_image_data( source_height: u32, dx: i32, dy: i32, - dirty_x: i32, - dirty_y: i32, - dirty_width: i32, - dirty_height: i32, + dirty_rect: ImageDataRect, ) { if !surface_matches_len(pixels, canvas_width, canvas_height) || !surface_matches_len(source, source_width, source_height) - || dirty_width <= 0 - || dirty_height <= 0 + || dirty_rect.width == 0 + || dirty_rect.height == 0 { return; } @@ -99,10 +88,7 @@ pub fn blit_image_data( canvas_height, dx, dy, - dirty_x, - dirty_y, - dirty_width, - dirty_height, + dirty_rect, ) else { return; }; @@ -119,7 +105,6 @@ pub fn blit_image_data( ); } -#[allow(clippy::too_many_arguments)] fn clipped_blit_rect( source_width: u32, source_height: u32, @@ -127,40 +112,20 @@ fn clipped_blit_rect( dest_height: u32, dest_x: i32, dest_y: i32, - dirty_x: i32, - dirty_y: i32, - dirty_width: i32, - dirty_height: i32, + dirty_rect: ImageDataRect, ) -> Option<(Rgba8Rect, u32, u32)> { - let mut source_left = (dirty_x as i64).max(0).min(source_width as i64); - let mut source_top = (dirty_y as i64).max(0).min(source_height as i64); - let mut source_right = (dirty_x as i64) - .checked_add(dirty_width as i64)? - .max(0) - .min(source_width as i64); - let mut source_bottom = (dirty_y as i64) - .checked_add(dirty_height as i64)? - .max(0) - .min(source_height as i64); - let mut target_x = dest_x as i64; - let mut target_y = dest_y as i64; - - if target_x < 0 { - let skip = -target_x; - source_left += skip; - target_x = 0; - } - if target_y < 0 { - let skip = -target_y; - source_top += skip; - target_y = 0; - } - if target_x + (source_right - source_left) > dest_width as i64 { - source_right -= target_x + (source_right - source_left) - dest_width as i64; - } - if target_y + (source_bottom - source_top) > dest_height as i64 { - source_bottom -= target_y + (source_bottom - source_top) - dest_height as i64; - } + // Intersect in source coordinates. A source pixel (x, y) is always written + // to (dest_x + x, dest_y + y), even when the dirty rectangle is clipped. + let dest_x = i64::from(dest_x); + let dest_y = i64::from(dest_y); + let source_left = dirty_rect.x.max(0).max(-dest_x); + let source_top = dirty_rect.y.max(0).max(-dest_y); + let source_right = (dirty_rect.x + i64::from(dirty_rect.width)) + .min(i64::from(source_width)) + .min(i64::from(dest_width) - dest_x); + let source_bottom = (dirty_rect.y + i64::from(dirty_rect.height)) + .min(i64::from(source_height)) + .min(i64::from(dest_height) - dest_y); if source_left >= source_right || source_top >= source_bottom { return None; } @@ -170,7 +135,11 @@ fn clipped_blit_rect( (source_right - source_left) as u32, (source_bottom - source_top) as u32, )?; - Some((source_rect, target_x as u32, target_y as u32)) + Some(( + source_rect, + (dest_x + source_left) as u32, + (dest_y + source_top) as u32, + )) } pub fn blit_draw_image( diff --git a/moli-canvas/src/lib.rs b/moli-canvas/src/lib.rs index c93966bb38..7017c23d12 100644 --- a/moli-canvas/src/lib.rs +++ b/moli-canvas/src/lib.rs @@ -17,8 +17,8 @@ pub use pixel::{ pub use rect::{canonicalize_fill_style, fill_style_rgba, normalize_rect, paint_rect}; pub use text::{draw_text, measure_text_width}; pub use types::{ - CanvasRect, DEFAULT_FILL_STYLE, DEFAULT_FONT, DrawImageBlit, MAX_RGBA8_BYTE_LENGTH, Rgba8Rect, - ScaleFilter, byte_len, + CanvasRect, DEFAULT_FILL_STYLE, DEFAULT_FONT, DrawImageBlit, ImageDataRect, + MAX_RGBA8_BYTE_LENGTH, Rgba8Rect, ScaleFilter, byte_len, }; #[cfg(test)] @@ -264,7 +264,7 @@ mod tests { .expect("copy should fit"); assert_eq!( - extract_image_data(&dest, 4, 3, 1, 1, 2, 2), + extract_image_data(&dest, 4, 3, ImageDataRect::new(1, 1, 2, 2)), vec![ 2, 0, 0, 255, 3, 0, 0, 255, // 5, 0, 0, 255, 6, 0, 0, 255, @@ -404,15 +404,28 @@ mod tests { 255, 0, 0, 255, 0, 255, 0, 255, 0, 0, 255, 255, 255, 255, 0, 255, ]; - blit_image_data(&mut dst, 2, 2, &src, 2, 2, 0, 0, 0, 0, 2, 2); - assert_eq!(extract_image_data(&dst, 2, 2, 0, 0, 2, 2), src); + blit_image_data( + &mut dst, + 2, + 2, + &src, + 2, + 2, + 0, + 0, + ImageDataRect::new(0, 0, 2, 2), + ); + assert_eq!( + extract_image_data(&dst, 2, 2, ImageDataRect::new(0, 0, 2, 2)), + src + ); } #[test] fn extract_image_data_out_of_bounds_fills_transparent_black() { let pixels = vec![255, 0, 0, 255]; assert_eq!( - extract_image_data(&pixels, 1, 1, -1, -1, 2, 2), + extract_image_data(&pixels, 1, 1, ImageDataRect::new(-1, -1, 2, 2)), vec![ 0, 0, 0, 0, 0, 0, 0, 0, // 0, 0, 0, 0, 255, 0, 0, 255, @@ -420,6 +433,44 @@ mod tests { ); } + #[test] + fn image_data_extreme_rectangles_clip_without_wrapping() { + let source = vec![255, 0, 0, 255, 0, 255, 0, 255]; + let mut target = vec![0; 8]; + blit_image_data( + &mut target, + 2, + 1, + &source, + 2, + 1, + 0, + 0, + ImageDataRect::new(1, 0, i32::MIN, 1), + ); + assert_eq!(target, [255, 0, 0, 255, 0, 0, 0, 0]); + assert_eq!( + extract_image_data(&source, 2, 1, ImageDataRect::new(2, 1, -2, -1)), + source, + ); + assert_eq!( + extract_image_data(&source, 2, 1, ImageDataRect::new(i32::MIN, 0, -2, 1)), + [0; 8], + ); + blit_image_data( + &mut target, + 2, + 1, + &source, + 2, + 1, + i32::MAX, + 0, + ImageDataRect::new(i32::MIN, 0, i32::MIN, 1), + ); + assert_eq!(target, [255, 0, 0, 255, 0, 0, 0, 0]); + } + #[test] fn blit_image_data_honors_dirty_rect_and_destination_clipping() { let source = vec![ @@ -428,17 +479,37 @@ mod tests { ]; let mut dst = vec![0; byte_len(2, 2).expect("buffer len")]; - blit_image_data(&mut dst, 2, 2, &source, 2, 2, 0, 0, 1, 0, 1, 2); + blit_image_data( + &mut dst, + 2, + 2, + &source, + 2, + 2, + 0, + 0, + ImageDataRect::new(1, 0, 1, 2), + ); assert_eq!( dst, vec![ - 0, 255, 0, 255, 0, 0, 0, 0, // - 255, 255, 0, 255, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 255, 0, 255, // + 0, 0, 0, 0, 255, 255, 0, 255, ] ); let mut clipped = vec![0; byte_len(1, 2).expect("buffer len")]; - blit_image_data(&mut clipped, 1, 2, &source, 2, 2, -1, 0, 0, 0, 2, 2); + blit_image_data( + &mut clipped, + 1, + 2, + &source, + 2, + 2, + -1, + 0, + ImageDataRect::new(0, 0, 2, 2), + ); assert_eq!( clipped, vec![ @@ -457,10 +528,7 @@ mod tests { 2, -1, 0, - 0, - 0, - 2, - 2, + ImageDataRect::new(0, 0, 2, 2), ); assert_eq!( negative_destination, @@ -488,7 +556,7 @@ mod tests { 2, DrawImageBlit::new(0.0, 0.0, 2.0, 2.0, 1.0, 1.0, 2.0, 2.0).expect("valid blit"), ); - let scaled_crop = extract_image_data(&scaled, 4, 4, 1, 1, 2, 2); + let scaled_crop = extract_image_data(&scaled, 4, 4, ImageDataRect::new(1, 1, 2, 2)); assert_eq!( scaled_crop, vec![ diff --git a/moli-canvas/src/types.rs b/moli-canvas/src/types.rs index 3d261c2843..62c2d75dd1 100644 --- a/moli-canvas/src/types.rs +++ b/moli-canvas/src/types.rs @@ -4,6 +4,35 @@ pub const MAX_RGBA8_BYTE_LENGTH: usize = 2_147_483_647; pub type CanvasRect = (i32, i32, i32, i32); +/// A pixel rectangle normalized from WebIDL long coordinates and signed spans. +/// Keep its origin wide: normalizing a negative span can leave the i32 range. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub struct ImageDataRect { + pub(crate) x: i64, + pub(crate) y: i64, + pub(crate) width: u32, + pub(crate) height: u32, +} + +impl ImageDataRect { + pub fn new(x: i32, y: i32, width: i32, height: i32) -> Self { + Self { + x: i64::from(x) + i64::from(width.min(0)), + y: i64::from(y) + i64::from(height.min(0)), + width: width.unsigned_abs(), + height: height.unsigned_abs(), + } + } + + pub fn width(self) -> u32 { + self.width + } + + pub fn height(self) -> u32 { + self.height + } +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct Rgba8Rect { pub x: u32, diff --git a/moli-renderer-v8/src/context_bootstrap/canvas/context2d.rs b/moli-renderer-v8/src/context_bootstrap/canvas/context2d.rs index 955814efe6..197708f48c 100644 --- a/moli-renderer-v8/src/context_bootstrap/canvas/context2d.rs +++ b/moli-renderer-v8/src/context_bootstrap/canvas/context2d.rs @@ -1991,15 +1991,12 @@ pub(crate) fn canvas_context_put_image_data_callback<'s>( else { return; }; - let Some(dirty_rect) = normalized_image_data_dirty_rect( - scope, + let dirty_rect = moli_canvas::ImageDataRect::new( parsed.dirty_x, parsed.dirty_y, parsed.dirty_width, parsed.dirty_height, - ) else { - return; - }; + ); (parsed.image_data, parsed.dx, parsed.dy, Some(dirty_rect)) } else { let Some(parsed) = webidl::parse_args::(scope, &args) else { @@ -2019,11 +2016,9 @@ pub(crate) fn canvas_context_put_image_data_callback<'s>( return; }; - let (dirty_x, dirty_y, dirty_width, dirty_height) = if args.length() >= 7 { - dirty_rect.unwrap_or((0, 0, 0, 0)) - } else { - (0, 0, source_width as i32, source_height as i32) - }; + let dirty_rect = dirty_rect.unwrap_or_else(|| { + moli_canvas::ImageDataRect::new(0, 0, source_width as i32, source_height as i32) + }); let _ = with_canvas_like_pixels_mut(scope, canvas, |pixels, width, height| { blit_image_data( @@ -2035,10 +2030,7 @@ pub(crate) fn canvas_context_put_image_data_callback<'s>( source_height, dx, dy, - dirty_x, - dirty_y, - dirty_width, - dirty_height, + dirty_rect, ); }); } @@ -2052,33 +2044,21 @@ pub(crate) fn canvas_context_get_image_data_callback<'s>( rv.set(v8::undefined(scope).into()); return; }; - let Some(width) = positive_image_data_dimension_from_long(scope, parsed.sw) else { - rv.set(v8::undefined(scope).into()); + let rect = moli_canvas::ImageDataRect::new(parsed.sx, parsed.sy, parsed.sw, parsed.sh); + let (width, height) = (rect.width(), rect.height()); + if width == 0 || height == 0 { + webidl::throw_index_size_error(scope); return; - }; - let Some(height) = positive_image_data_dimension_from_long(scope, parsed.sh) else { - rv.set(v8::undefined(scope).into()); - return; - }; + } if byte_len(width, height).is_none() { webidl::throw_index_size_error(scope); rv.set(v8::undefined(scope).into()); return; } - let source_x = parsed.sx; - let source_y = parsed.sy; let bytes = if let Some(canvas) = canvas_owner_from_context(scope, args.this()) { canvas_like_pixels_copy(scope, canvas) .map(|(pixels, canvas_width, canvas_height)| { - extract_image_data( - &pixels, - canvas_width, - canvas_height, - source_x, - source_y, - width, - height, - ) + extract_image_data(&pixels, canvas_width, canvas_height, rect) }) .unwrap_or_else(|| blank_image_data(width, height)) } else { @@ -2115,48 +2095,6 @@ fn image_data_dimension_from_long(scope: &mut v8::PinScope<'_, '_>, value: i32) Some(value.unsigned_abs()) } -fn positive_image_data_dimension_from_long( - scope: &mut v8::PinScope<'_, '_>, - value: i32, -) -> Option { - if value <= 0 { - webidl::throw_index_size_error(scope); - return None; - } - Some(value as u32) -} - -fn normalized_image_data_dirty_rect( - scope: &mut v8::PinScope<'_, '_>, - dirty_x: i32, - dirty_y: i32, - dirty_width: i32, - dirty_height: i32, -) -> Option<(i32, i32, i32, i32)> { - let (dirty_x, dirty_width) = normalized_dirty_axis(scope, dirty_x, dirty_width)?; - let (dirty_y, dirty_height) = normalized_dirty_axis(scope, dirty_y, dirty_height)?; - Some((dirty_x, dirty_y, dirty_width, dirty_height)) -} - -fn normalized_dirty_axis( - scope: &mut v8::PinScope<'_, '_>, - start: i32, - span: i32, -) -> Option<(i32, i32)> { - if span >= 0 { - return Some((start, span)); - } - if span == i32::MIN { - webidl::throw_index_size_error(scope); - return None; - } - let start = start.checked_add(span).or_else(|| { - webidl::throw_index_size_error(scope); - None - })?; - Some((start, -span)) -} - fn normalized_rect<'s>( scope: &mut v8::PinScope<'s, '_>, args: &v8::FunctionCallbackArguments<'s>, diff --git a/moli-renderer-v8/src/script_vm/tests/canvas_webgl.rs b/moli-renderer-v8/src/script_vm/tests/canvas_webgl.rs index 306f02c5a5..a0ae073dbd 100644 --- a/moli-renderer-v8/src/script_vm/tests/canvas_webgl.rs +++ b/moli-renderer-v8/src/script_vm/tests/canvas_webgl.rs @@ -1781,7 +1781,7 @@ fn html_canvas_put_image_data_honors_dirty_rect() { ) .expect("canvas putImageData dirty rect should evaluate"); - assert_eq!(result, "[0,255,0,255,0,0,0,0,255,255,0,255,0,0,0,0]"); + assert_eq!(result, "[0,0,0,0,0,255,0,255,0,0,0,0,255,255,0,255]"); } #[test] @@ -1813,7 +1813,10 @@ fn html_canvas_image_data_methods_parse_webidl_enforce_range_long() { }); const createZero = probe(() => ctx.createImageData(0, 1)); const createSymbol = probe(() => ctx.createImageData(Symbol('width'), 1)); - const getNegative = probe(() => ctx.getImageData(2, 2, -2, -2)); + const getNegative = probe(() => { + const data = ctx.getImageData(2, 2, -2, -2); + return [data.width, data.height, Array.from(data.data).join(',')].join('|'); + }); const getMissing = probe(() => ctx.getImageData(0, 0, 1)); const getNan = probe(() => ctx.getImageData(NaN, 0, 1, 1)); const getZero = probe(() => ctx.getImageData(0, 0, 0, 1)); @@ -1869,7 +1872,7 @@ fn html_canvas_image_data_methods_parse_webidl_enforce_range_long() { assert_eq!( result, - r#"{"created":"2:3","createZero":"IndexSizeError","createSymbol":"TypeError","getNegative":"IndexSizeError","getMissing":"TypeError","getNan":"TypeError","getZero":"IndexSizeError","constructorOversized":"IndexSizeError","getOversized":"IndexSizeError","putCoerced":"0,0,0,0,255,0,0,255,0,255,0,255","putSymbol":"TypeError","afterBadPut":true,"dirtyNegative":"255,0,0,255,0,0,0,0","putPlainObject":"TypeError"}"# + r#"{"created":"2:3","createZero":"IndexSizeError","createSymbol":"TypeError","getNegative":"2|2|255,0,0,255,255,0,0,255,255,0,0,255,255,0,0,255","getMissing":"TypeError","getNan":"TypeError","getZero":"IndexSizeError","constructorOversized":"IndexSizeError","getOversized":"IndexSizeError","putCoerced":"0,0,0,0,255,0,0,255,0,255,0,255","putSymbol":"TypeError","afterBadPut":true,"dirtyNegative":"255,0,0,255,0,0,0,0","putPlainObject":"TypeError"}"# ); }