Handle some transparent black cases in ctx.getImageData

Cargo.lock

@@ -2663,6 +2663,9 @@ dependencies = [
 [[package]]
 name = "pixels"
 version = "0.0.1"
+dependencies = [
+ "euclid 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)",
+]
 
 [[package]]
 name = "pkg-config"

components/canvas/canvas_data.rs

@@ -526,22 +526,7 @@ impl<'a> CanvasData<'a> {
         }
 
         let data_surface = self.drawtarget.snapshot().get_data_surface();
-        let src_data = unsafe { data_surface.data() };
+        pixels::get_rect(unsafe { data_surface.data() }, canvas_size.to_u32(), read_rect.to_u32()).into_owned()
-        let stride = data_surface.stride();
-
-        //start offset of the copyable rectangle
-        let mut src = (src_read_rect.origin.y * stride + src_read_rect.origin.x * 4) as usize;
-        let mut image_data = Vec::with_capacity(
-            (src_read_rect.size.width * src_read_rect.size.height * 4) as usize,
-        );
-        //copy the data to the destination vector
-        for _ in 0..src_read_rect.size.height {
-            let row = &src_data[src .. src + (4 * src_read_rect.size.width) as usize];
-            image_data.extend_from_slice(row);
-            src += stride as usize;
-        }
-
-        image_data
     }
 }
 

components/pixels/Cargo.toml

@@ -8,3 +8,6 @@ publish = false
 [lib]
 name = "pixels"
 path = "lib.rs"
+
+[dependencies]
+euclid = "0.19"

components/pixels/lib.rs

@@ -2,6 +2,31 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+extern crate euclid;
+
+use euclid::{Rect, Size2D};
+use std::borrow::Cow;
+
+pub fn get_rect(pixels: &[u8], size: Size2D<u32>, rect: Rect<u32>) -> Cow<[u8]> {
+    assert!(!rect.is_empty());
+    assert!(Rect::from_size(size).contains_rect(&rect));
+    assert_eq!(pixels.len() % 4, 0);
+    assert_eq!(size.area() as usize, pixels.len() / 4);
+    let area = rect.size.area() as usize;
+    let first_column_start = rect.origin.x as usize * 4;
+    let row_length = size.width as usize * 4;
+    let first_row_start = rect.origin.y as usize * row_length;
+    if rect.origin.x == 0 && rect.size.width == size.width || rect.size.height == 1 {
+        let start = first_column_start + first_row_start;
+        return Cow::Borrowed(&pixels[start..start + area * 4]);
+    }
+    let mut data = Vec::with_capacity(area * 4);
+    for row in pixels[first_row_start..].chunks(row_length).take(rect.size.height as usize) {
+        data.extend_from_slice(&row[first_column_start..][..rect.size.width as usize * 4]);
+    }
+    data.into()
+}
+
 // TODO(pcwalton): Speed up with SIMD, or better yet, find some way to not do this.
 pub fn byte_swap_colors_inplace(pixels: &mut [u8]) {
     assert!(pixels.len() % 4 == 0);

components/script/dom/canvasrenderingcontext2d.rs

@@ -1137,6 +1137,9 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
         mut sw: i32,
         mut sh: i32,
     ) -> Fallible<DomRoot<ImageData>> {
+        // FIXME(nox): There are many arithmetic operations here that can
+        // overflow or underflow, this should probably be audited.
+
         if sw == 0 || sh == 0 {
             return Err(Error::IndexSize);
         }
@@ -1154,27 +1157,48 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
             sy -= sh;
         }
 
+        let data_width = sw;
+        let data_height = sh;
+
+        if sx < 0 {
+            sw += sx;
+            sx = 0;
+        }
+        if sy < 0 {
+            sh += sy;
+            sy = 0;
+        }
+
+        if sw <= 0 || sh <= 0 {
+            // All the pixels are before the start of the canvas surface.
+            return ImageData::new(&self.global(), data_width as u32, data_height as u32, None);
+        }
+
         let (sender, receiver) = ipc::bytes_channel().unwrap();
-        let dest_rect = Rect::new(Point2D::new(sx, sy), Size2D::new(sw, sh));
+        let src_rect = Rect::new(Point2D::new(sx, sy), Size2D::new(sw, sh));
         // FIXME(nox): This is probably wrong when this is a context for an
         // offscreen canvas.
-        let canvas_size = self.canvas.as_ref().map_or(Size2D::zero(), |c| c.get_size());
+        let canvas_size = self.canvas
-        self.send_canvas_2d_msg(Canvas2dMsg::GetImageData(
+            .as_ref()
-            dest_rect,
+            .map_or(Size2D::zero(), |c| c.get_size())
-            canvas_size.to_i32(),
+            .try_cast().unwrap();
-            sender,
+        let canvas_rect = Rect::from_size(canvas_size);
-        ));
+        let read_rect = match src_rect.intersection(&canvas_rect) {
-        let mut data = receiver.recv().unwrap();
+            Some(rect) if !rect.is_empty() => rect,
-
+            _ => {
-        // Byte swap and unmultiply alpha.
+                // All the pixels are past the end of the canvas surface.
-        for chunk in data.chunks_mut(4) {
+                return ImageData::new(&self.global(), data_width as u32, data_height as u32, None);
+            }
+        };
+        self.send_canvas_2d_msg(Canvas2dMsg::GetImageData(read_rect, canvas_size, sender));
+        let mut pixels = receiver.recv().unwrap();
+        for chunk in pixels.chunks_mut(4) {
             let b = chunk[0];
             chunk[0] = UNPREMULTIPLY_TABLE[256 * (chunk[3] as usize) + chunk[2] as usize];
             chunk[1] = UNPREMULTIPLY_TABLE[256 * (chunk[3] as usize) + chunk[1] as usize];
             chunk[2] = UNPREMULTIPLY_TABLE[256 * (chunk[3] as usize) + b as usize];
         }
-
+        ImageData::new(&self.global(), sw as u32, sh as u32, Some(pixels.to_vec()))
-        ImageData::new(&self.global(), sw as u32, sh as u32, Some(data.to_vec()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-putimagedata
@@ -1197,6 +1221,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
         // FIXME(nox): There are many arithmetic operations here that can
         // overflow or underflow, this should probably be audited.
 
+
         let imagedata_size = Size2D::new(imagedata.Width() as i32, imagedata.Height() as i32);
         if imagedata_size.width <= 0 || imagedata_size.height <= 0 {
             return;

components/script/dom/imagedata.rs

@@ -13,6 +13,7 @@ use euclid::{Rect, Size2D};
 use js::jsapi::{Heap, JSContext, JSObject};
 use js::rust::Runtime;
 use js::typedarray::{Uint8ClampedArray, CreateWith};
+use pixels;
 use std::borrow::Cow;
 use std::default::Default;
 use std::ptr;
@@ -162,31 +163,12 @@ impl ImageData {
 
     #[allow(unsafe_code)]
     pub unsafe fn get_rect(&self, rect: Rect<u32>) -> Cow<[u8]> {
-        assert!(!rect.is_empty());
+        pixels::get_rect(self.as_slice(), self.get_size(), rect)
-        assert!(self.rect().contains_rect(&rect));
-        let slice = self.as_slice();
-        let area = rect.size.area() as usize;
-        let first_column_start = rect.origin.x as usize * 4;
-        let row_length = self.width as usize * 4;
-        let first_row_start = rect.origin.y as usize * row_length;
-        if rect.origin.x == 0 && rect.size.width == self.width || rect.size.height == 1 {
-            let start = first_column_start + first_row_start;
-            return Cow::Borrowed(&slice[start..start + area * 4]);
-        }
-        let mut data = Vec::with_capacity(area * 4);
-        for row in slice[first_row_start..].chunks(row_length).take(rect.size.height as usize) {
-            data.extend_from_slice(&row[first_column_start..][..rect.size.width as usize * 4]);
-        }
-        data.into()
     }
 
     pub fn get_size(&self) -> Size2D<u32> {
         Size2D::new(self.Width(), self.Height())
     }
-
-    pub fn rect(&self) -> Rect<u32> {
-        Rect::from_size(self.get_size())
-    }
 }
 
 impl ImageDataMethods for ImageData {