Skip to content

Commit fa7c7c1

Browse files
Brooooooklynclaude
andauthored
fix: use signed integers for canvas APIs that accept negative coordinates (#1227)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 897b1c5 commit fa7c7c1

File tree

4 files changed

+128
-23
lines changed

4 files changed

+128
-23
lines changed

__test__/regression.spec.ts

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { fileURLToPath } from 'node:url'
55
import test from 'ava'
66
import PNG from '@jimp/png'
77

8-
import { createCanvas, loadImage, GlobalFonts, Image, DOMMatrix, DOMPoint } from '../index'
8+
import { createCanvas, loadImage, GlobalFonts, Image, ImageData, DOMMatrix, DOMPoint } from '../index'
99
import { snapshotImage } from './image-snapshot'
1010

1111
const __dirname = dirname(fileURLToPath(import.meta.url))
@@ -456,6 +456,99 @@ test('putImageData double free', (t) => {
456456
})
457457
})
458458

459+
// https://github.com/Brooooooklyn/canvas/issues/1226
460+
test('putImageData with negative dx/dy should draw correctly', (t) => {
461+
const canvas = createCanvas(200, 200)
462+
const ctx = canvas.getContext('2d')
463+
// Fill canvas with black
464+
ctx.fillStyle = 'black'
465+
ctx.fillRect(0, 0, 200, 200)
466+
// Create a 100x100 white ImageData
467+
const imageData = new ImageData(100, 100)
468+
imageData.data.fill(255)
469+
// putImageData at (-20, -20): should place an 80x80 white region at (0,0)
470+
ctx.putImageData(imageData, -20, -20)
471+
// Pixel at (0,0) should be white (within the drawn region)
472+
const topLeft = ctx.getImageData(0, 0, 1, 1).data
473+
t.is(topLeft[0], 255)
474+
t.is(topLeft[1], 255)
475+
t.is(topLeft[2], 255)
476+
t.is(topLeft[3], 255)
477+
// Pixel at (79,79) should be white (last pixel of the drawn region)
478+
const edge = ctx.getImageData(79, 79, 1, 1).data
479+
t.is(edge[0], 255)
480+
t.is(edge[1], 255)
481+
t.is(edge[2], 255)
482+
t.is(edge[3], 255)
483+
// Pixel at (80,80) should be black (outside the drawn region)
484+
const outside = ctx.getImageData(80, 80, 1, 1).data
485+
t.is(outside[0], 0)
486+
t.is(outside[1], 0)
487+
t.is(outside[2], 0)
488+
t.is(outside[3], 255)
489+
})
490+
491+
// https://github.com/Brooooooklyn/canvas/issues/1226
492+
test('getImageData with negative x/y should return correct pixels', (t) => {
493+
const canvas = createCanvas(100, 100)
494+
const ctx = canvas.getContext('2d')
495+
// Fill entire canvas red
496+
ctx.fillStyle = 'red'
497+
ctx.fillRect(0, 0, 100, 100)
498+
// getImageData(-10, -10, 20, 20): top-left 10x10 is outside canvas (transparent black),
499+
// bottom-right 10x10 is canvas pixels (red)
500+
const data = ctx.getImageData(-10, -10, 20, 20)
501+
t.is(data.width, 20)
502+
t.is(data.height, 20)
503+
// Pixel at (0,0) in the ImageData corresponds to canvas (-10,-10) — outside, should be transparent black
504+
const outOfBounds = [data.data[0], data.data[1], data.data[2], data.data[3]]
505+
t.deepEqual(outOfBounds, [0, 0, 0, 0])
506+
// Pixel at (5,5) in the ImageData is still outside canvas (-5,-5) — transparent black
507+
const stillOutside = 4 * (5 * 20 + 5)
508+
t.deepEqual([data.data[stillOutside], data.data[stillOutside + 1], data.data[stillOutside + 2], data.data[stillOutside + 3]], [0, 0, 0, 0])
509+
// Pixel at (10,10) in the ImageData corresponds to canvas (0,0) — should be red
510+
const insideOffset = 4 * (10 * 20 + 10)
511+
t.is(data.data[insideOffset], 255) // R
512+
t.is(data.data[insideOffset + 1], 0) // G
513+
t.is(data.data[insideOffset + 2], 0) // B
514+
t.is(data.data[insideOffset + 3], 255) // A
515+
// Pixel at (19,19) in the ImageData corresponds to canvas (9,9) — should be red
516+
const cornerOffset = 4 * (19 * 20 + 19)
517+
t.is(data.data[cornerOffset], 255) // R
518+
t.is(data.data[cornerOffset + 1], 0) // G
519+
t.is(data.data[cornerOffset + 2], 0) // B
520+
t.is(data.data[cornerOffset + 3], 255) // A
521+
})
522+
523+
test('getImageData with negative width/height should flip the region per spec', (t) => {
524+
const canvas = createCanvas(100, 100)
525+
const ctx = canvas.getContext('2d')
526+
// Paint a 10x10 green square at (5,5)
527+
ctx.fillStyle = 'green'
528+
ctx.fillRect(5, 5, 10, 10)
529+
// getImageData(15, 15, -10, -10) per spec: sx=15+(-10)=5, sy=15+(-10)=5, sw=10, sh=10
530+
// Should return the 10x10 region starting at (5,5) — the green square
531+
const data = ctx.getImageData(15, 15, -10, -10)
532+
t.is(data.width, 10)
533+
t.is(data.height, 10)
534+
t.is(data.data.length, 10 * 10 * 4)
535+
// Center pixel (5,5) in the ImageData should be green
536+
const centerOffset = 4 * (5 * 10 + 5)
537+
t.is(data.data[centerOffset], 0) // R
538+
t.is(data.data[centerOffset + 1], 128) // G (green is 0,128,0)
539+
t.is(data.data[centerOffset + 2], 0) // B
540+
t.is(data.data[centerOffset + 3], 255) // A
541+
})
542+
543+
test('createImageData with negative dimensions should use absolute values', (t) => {
544+
const canvas = createCanvas(100, 100)
545+
const ctx = canvas.getContext('2d')
546+
const imageData = ctx.createImageData(-50, -30)
547+
t.is(imageData.width, 50)
548+
t.is(imageData.height, 30)
549+
t.is(imageData.data.length, 50 * 30 * 4)
550+
})
551+
459552
// https://github.com/Brooooooklyn/canvas/issues/987
460553
test('draw-canvas-on-canvas', async (t) => {
461554
const backCanvas = createCanvas(1920, 1080)

src/ctx.rs

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -969,13 +969,13 @@ impl Context {
969969
if let Some(ref recorder) = self.page_recorder {
970970
return recorder
971971
.borrow_mut()
972-
.get_pixels(x as u32, y as u32, w as u32, h as u32, color_type);
972+
.get_pixels(x as i32, y as i32, w as u32, h as u32, color_type);
973973
}
974974

975975
// Direct mode - read from main surface
976976
self
977977
.surface
978-
.read_pixels(x as u32, y as u32, w as u32, h as u32, color_type)
978+
.read_pixels(x as i32, y as i32, w as u32, h as u32, color_type)
979979
}
980980

981981
pub fn set_line_dash(&mut self, line_dash_list: Vec<f32>) {
@@ -2016,14 +2016,15 @@ impl CanvasRenderingContext2D {
20162016
pub fn create_image_data<'scope>(
20172017
&'scope mut self,
20182018
env: &'scope Env,
2019-
width_or_data: Either<u32, Uint8ClampedSlice<'scope>>,
2020-
width_or_height: u32,
2021-
height_or_settings: Option<Either<u32, Settings>>,
2019+
width_or_data: Either<i32, Uint8ClampedSlice<'scope>>,
2020+
width_or_height: i32,
2021+
height_or_settings: Option<Either<i32, Settings>>,
20222022
maybe_settings: Option<Settings>,
20232023
) -> Result<ClassInstance<'scope, ImageData>> {
20242024
match width_or_data {
20252025
Either::A(width) => {
2026-
let height = width_or_height;
2026+
let width = width.unsigned_abs();
2027+
let height = width_or_height.unsigned_abs();
20272028
let color_space = match height_or_settings {
20282029
Some(Either::B(settings)) => {
20292030
ColorSpace::from_str(&settings.color_space).unwrap_or_default()
@@ -2045,9 +2046,9 @@ impl CanvasRenderingContext2D {
20452046
}
20462047
Either::B(mut data_object) => {
20472048
let input_data_length = data_object.len();
2048-
let width = width_or_height;
2049+
let width = width_or_height.unsigned_abs();
20492050
let height = match &height_or_settings {
2050-
Some(Either::A(height)) => *height,
2051+
Some(Either::A(height)) => height.unsigned_abs(),
20512052
_ => (input_data_length as u32) / 4 / width,
20522053
};
20532054
let data = unsafe { data_object.as_mut() }.as_mut_ptr();
@@ -2717,9 +2718,20 @@ impl CanvasRenderingContext2D {
27172718
let color_space = color_space
27182719
.and_then(|cs| cs.parse().ok())
27192720
.unwrap_or(ColorSpace::Srgb);
2721+
// Per spec: if sw/sh is negative, flip the origin and use abs value
2722+
let (sx, sw) = if width < 0.0 {
2723+
(x + width, -width)
2724+
} else {
2725+
(x, width)
2726+
};
2727+
let (sy, sh) = if height < 0.0 {
2728+
(y + height, -height)
2729+
} else {
2730+
(y, height)
2731+
};
27202732
let image_data = self
27212733
.context
2722-
.get_image_data(x as f32, y as f32, width as f32, height as f32, color_space)
2734+
.get_image_data(sx as f32, sy as f32, sw as f32, sh as f32, color_space)
27232735
.ok_or_else(|| {
27242736
Error::new(
27252737
Status::GenericFailure,
@@ -2728,8 +2740,8 @@ impl CanvasRenderingContext2D {
27282740
})?;
27292741
let mut data_object = Uint8ClampedSlice::from_data(env, image_data)?;
27302742
let mut instance = ImageData {
2731-
width: width as usize,
2732-
height: height as usize,
2743+
width: sw as usize,
2744+
height: sh as usize,
27332745
color_space,
27342746
data: unsafe { data_object.as_mut() }.as_mut_ptr(),
27352747
}
@@ -2759,8 +2771,8 @@ impl CanvasRenderingContext2D {
27592771
pub fn put_image_data(
27602772
&mut self,
27612773
image_data: &ImageData,
2762-
dx: u32,
2763-
dy: u32,
2774+
dx: i32,
2775+
dy: i32,
27642776
dirty_x: Option<f64>,
27652777
dirty_y: Option<f64>,
27662778
dirty_width: Option<f64>,

src/page_recorder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,8 @@ impl PageRecorder {
283283
/// Returns pixel data for the requested region.
284284
pub fn get_pixels(
285285
&mut self,
286-
x: u32,
287-
y: u32,
286+
x: i32,
287+
y: i32,
288288
width: u32,
289289
height: u32,
290290
color_space: ColorSpace,

src/sk.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2144,8 +2144,8 @@ impl Surface {
21442144

21452145
pub fn read_pixels(
21462146
&self,
2147-
x: u32,
2148-
y: u32,
2147+
x: i32,
2148+
y: i32,
21492149
width: u32,
21502150
height: u32,
21512151
color_space: ColorSpace,
@@ -2155,8 +2155,8 @@ impl Surface {
21552155
ffi::skiac_surface_read_pixels_rect(
21562156
self.ptr,
21572157
result.as_mut_ptr(),
2158-
x as i32,
2159-
y as i32,
2158+
x,
2159+
y,
21602160
width as i32,
21612161
height as i32,
21622162
color_space as u8,
@@ -2802,16 +2802,16 @@ impl Canvas {
28022802
}
28032803
}
28042804

2805-
pub fn write_pixels(&mut self, image: &ImageData, x: u32, y: u32) {
2805+
pub fn write_pixels(&mut self, image: &ImageData, x: i32, y: i32) {
28062806
unsafe {
28072807
ffi::skiac_canvas_write_pixels(
28082808
self.0,
28092809
image.width as i32,
28102810
image.height as i32,
28112811
image.data,
28122812
image.width * 4,
2813-
x as i32,
2814-
y as i32,
2813+
x,
2814+
y,
28152815
);
28162816
}
28172817
}

0 commit comments

Comments
 (0)