Align ctx.createImageData and ctx.getImageData with the spec

10 files changed, 54 insertions, 81 deletions

diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs
index 7ccd7b6184e..e7841d1a53c 100644
--- a/components/canvas/canvas_data.rs
+++ b/components/canvas/canvas_data.rs
@@ -11,7 +11,7 @@ use azure::azure_hl::SurfacePattern;
 use canvas_traits::canvas::*;
 use cssparser::RGBA;
 use euclid::{Transform2D, Point2D, Vector2D, Rect, Size2D};
-use ipc_channel::ipc::{IpcBytesSender, IpcSender};
+use ipc_channel::ipc::IpcSender;
 use num_traits::ToPrimitive;
 use pixels;
 use serde_bytes::ByteBuf;
@@ -440,15 +440,6 @@ impl<'a> CanvasData<'a> {
         chan.send(data).unwrap();
     }
 
-    pub fn image_data(
-        &self,
-        dest_rect: Rect<i32>,
-        canvas_size: Size2D<f64>,
-        sender: IpcBytesSender,
-    ) {
-        sender.send(&self.read_pixels(dest_rect, canvas_size)).unwrap();
-    }
-
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-putimagedata
     pub fn put_image_data(
         &mut self,
@@ -526,9 +517,8 @@ impl<'a> CanvasData<'a> {
     /// canvas_size: The size of the canvas we're reading from
     /// read_rect: The area of the canvas we want to read from
     #[allow(unsafe_code)]
-    pub fn read_pixels(&self, read_rect: Rect<i32>, canvas_size: Size2D<f64>) -> Vec<u8> {
-        let canvas_size = canvas_size.to_i32();
-        let canvas_rect = Rect::new(Point2D::new(0i32, 0i32), canvas_size);
+    pub fn read_pixels(&self, read_rect: Rect<i32>, canvas_size: Size2D<i32>) -> Vec<u8> {
+        let canvas_rect = Rect::from_size(canvas_size);
         let src_read_rect = canvas_rect.intersection(&read_rect).unwrap_or(Rect::zero());
 
         if src_read_rect.is_empty() || canvas_size.width <= 0 && canvas_size.height <= 0 {
diff --git a/components/canvas/canvas_paint_thread.rs b/components/canvas/canvas_paint_thread.rs
index 428ca8ddd9f..dae7836fd71 100644
--- a/components/canvas/canvas_paint_thread.rs
+++ b/components/canvas/canvas_paint_thread.rs
@@ -163,7 +163,7 @@ impl<'a> CanvasPaintThread <'a> {
             ) => {
                 let image_data = self.canvas(canvas_id).read_pixels(
                     source_rect.to_i32(),
-                    image_size,
+                    image_size.to_i32(),
                 );
                 self.canvas(other_canvas_id).draw_image(
                     image_data.into(),
@@ -238,8 +238,9 @@ impl<'a> CanvasPaintThread <'a> {
             Canvas2dMsg::SetGlobalComposition(op) => {
                 self.canvas(canvas_id).set_global_composition(op)
             },
-            Canvas2dMsg::GetImageData(dest_rect, canvas_size, chan) => {
-                self.canvas(canvas_id).image_data(dest_rect, canvas_size, chan)
+            Canvas2dMsg::GetImageData(dest_rect, canvas_size, sender) => {
+                let pixels = self.canvas(canvas_id).read_pixels(dest_rect, canvas_size);
+                sender.send(&pixels).unwrap();
             },
             Canvas2dMsg::PutImageData(receiver, offset, imagedata_size) => {
                 self.canvas(canvas_id).put_image_data(
diff --git a/components/canvas_traits/canvas.rs b/components/canvas_traits/canvas.rs
index 4833f26262f..8e452c93aff 100644
--- a/components/canvas_traits/canvas.rs
+++ b/components/canvas_traits/canvas.rs
@@ -50,7 +50,7 @@ pub enum Canvas2dMsg {
     Fill,
     FillText(String, f64, f64, Option<f64>),
     FillRect(Rect<f32>),
-    GetImageData(Rect<i32>, Size2D<f64>, IpcBytesSender),
+    GetImageData(Rect<i32>, Size2D<i32>, IpcBytesSender),
     IsPointInPath(f64, f64, FillRule, IpcSender<bool>),
     LineTo(Point2D<f32>),
     MoveTo(Point2D<f32>),
diff --git a/components/script/dom/canvasrenderingcontext2d.rs b/components/script/dom/canvasrenderingcontext2d.rs
index 69c7d4038cd..633fbbc3f14 100644
--- a/components/script/dom/canvasrenderingcontext2d.rs
+++ b/components/script/dom/canvasrenderingcontext2d.rs
@@ -40,12 +40,11 @@ use net_traits::image_cache::ImageOrMetadataAvailable;
 use net_traits::image_cache::ImageResponse;
 use net_traits::image_cache::ImageState;
 use net_traits::image_cache::UsePlaceholder;
-use num_traits::ToPrimitive;
 use pixels;
 use profile_traits::ipc as profiled_ipc;
 use script_traits::ScriptMsg;
 use servo_url::ServoUrl;
-use std::{cmp, fmt, mem};
+use std::{fmt, mem};
 use std::cell::Cell;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -1118,14 +1117,11 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-createimagedata
-    fn CreateImageData(&self, sw: Finite<f64>, sh: Finite<f64>) -> Fallible<DomRoot<ImageData>> {
-        if *sw == 0.0 || *sh == 0.0 {
+    fn CreateImageData(&self, sw: i32, sh: i32) -> Fallible<DomRoot<ImageData>> {
+        if sw == 0 || sh == 0 {
             return Err(Error::IndexSize);
         }
-
-        let sw = cmp::max(1, sw.abs().to_u32().unwrap());
-        let sh = cmp::max(1, sh.abs().to_u32().unwrap());
-        ImageData::new(&self.global(), sw, sh, None)
+        ImageData::new(&self.global(), sw.abs() as u32, sh.abs() as u32, None)
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-createimagedata
@@ -1136,59 +1132,49 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-getimagedata
     fn GetImageData(
         &self,
-        sx: Finite<f64>,
-        sy: Finite<f64>,
-        sw: Finite<f64>,
-        sh: Finite<f64>,
+        mut sx: i32,
+        mut sy: i32,
+        mut sw: i32,
+        mut sh: i32,
     ) -> Fallible<DomRoot<ImageData>> {
-        if !self.origin_is_clean() {
-            return Err(Error::Security);
+        if sw == 0 || sh == 0 {
+            return Err(Error::IndexSize);
         }
 
-        let mut sx = *sx;
-        let mut sy = *sy;
-        let mut sw = *sw;
-        let mut sh = *sh;
-
-        if sw == 0.0 || sh == 0.0 {
-            return Err(Error::IndexSize);
+        if !self.origin_is_clean() {
+            return Err(Error::Security);
         }
 
-        if sw < 0.0 {
+        if sw < 0 {
             sw = -sw;
             sx -= sw;
         }
-        if sh < 0.0 {
+        if sh < 0 {
             sh = -sh;
             sy -= sh;
         }
 
-        let sh = cmp::max(1, sh.to_u32().unwrap());
-        let sw = cmp::max(1, sw.to_u32().unwrap());
-
         let (sender, receiver) = ipc::bytes_channel().unwrap();
-        let dest_rect = Rect::new(
-            Point2D::new(sx.to_i32().unwrap(), sy.to_i32().unwrap()),
-            Size2D::new(sw as i32, sh as i32),
-        );
-        let canvas_size = self
-            .canvas
-            .as_ref()
-            .map(|c| c.get_size())
-            .unwrap_or(Size2D::zero());
-        let canvas_size = Size2D::new(canvas_size.width as f64, canvas_size.height as f64);
-        self.send_canvas_2d_msg(Canvas2dMsg::GetImageData(dest_rect, canvas_size, sender));
+        let dest_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());
+        self.send_canvas_2d_msg(Canvas2dMsg::GetImageData(
+            dest_rect,
+            canvas_size.to_i32(),
+            sender,
+        ));
         let mut data = receiver.recv().unwrap();
 
         // Byte swap and unmultiply alpha.
         for chunk in data.chunks_mut(4) {
-            let (b, g, r, a) = (chunk[0], chunk[1], chunk[2], chunk[3]);
-            chunk[0] = UNPREMULTIPLY_TABLE[256 * (a as usize) + r as usize];
-            chunk[1] = UNPREMULTIPLY_TABLE[256 * (a as usize) + g as usize];
-            chunk[2] = UNPREMULTIPLY_TABLE[256 * (a as usize) + b as usize];
+            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, sh, Some(data.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
@@ -1265,11 +1251,14 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
             dirty_height = imagedata_size.height - dirty_y;
         }
 
-        // We take care of ignoring any pixel that would be drawn after the end
-        // of the canvas surface.
+        // 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()).to_i32();
         let origin = Point2D::new(dest_x, dest_y);
         let drawable_size = (origin - canvas_size.to_vector().to_point()).to_size().abs();
+
+        // We take care of ignoring any pixel that would be drawn after the end
+        // of the canvas surface.
         dirty_width = dirty_width.min(drawable_size.width);
         dirty_height = dirty_height.min(drawable_size.height);
 
diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs
index c3aae4eed83..78e74641347 100644
--- a/components/script/dom/htmlcanvaselement.rs
+++ b/components/script/dom/htmlcanvaselement.rs
@@ -14,7 +14,6 @@ use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLContext
 use dom::bindings::conversions::ConversionResult;
 use dom::bindings::error::{Error, Fallible};
 use dom::bindings::inheritance::Castable;
-use dom::bindings::num::Finite;
 use dom::bindings::reflector::DomObject;
 use dom::bindings::root::{Dom, DomRoot, LayoutDom};
 use dom::bindings::str::{DOMString, USVString};
@@ -370,13 +369,7 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement {
         let raw_data = match *self.context.borrow() {
             Some(CanvasContext::Context2d(ref context)) => {
                 // FIXME(nox): This shouldn't go through ImageData etc.
-                let image_data = context.GetImageData(
-                    Finite::wrap(0f64),
-                    Finite::wrap(0f64),
-                    Finite::wrap(self.Width() as f64),
-                    Finite::wrap(self.Height() as f64),
-                )?;
-                image_data.to_vec()
+                context.GetImageData(0, 0, self.Width() as i32, self.Height() as i32)?.to_vec()
             },
             Some(CanvasContext::WebGL(ref context)) => {
                 match context.get_image_data(self.Width(), self.Height()) {
diff --git a/components/script/dom/webidls/CanvasRenderingContext2D.webidl b/components/script/dom/webidls/CanvasRenderingContext2D.webidl
index f772bd2e8db..82001645924 100644
--- a/components/script/dom/webidls/CanvasRenderingContext2D.webidl
+++ b/components/script/dom/webidls/CanvasRenderingContext2D.webidl
@@ -175,11 +175,11 @@ interface CanvasDrawImage {
 interface CanvasImageData {
   // pixel manipulation
   [Throws]
-  ImageData createImageData(double sw, double sh);
+  ImageData createImageData(long sw, long sh);
   [Throws]
   ImageData createImageData(ImageData imagedata);
   [Throws]
-  ImageData getImageData(double sx, double sy, double sw, double sh);
+  ImageData getImageData(long sx, long sy, long sw, long sh);
   void putImageData(ImageData imagedata, long dx, long dy);
   void putImageData(ImageData imagedata,
                     long dx, long dy,
diff --git a/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.create2.nonfinite.html.ini b/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.create2.nonfinite.html.ini
new file mode 100644
index 00000000000..62382c779c2
--- /dev/null
+++ b/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.create2.nonfinite.html.ini
@@ -0,0 +1,5 @@
+[2d.imageData.create2.nonfinite.html]
+  bug: https://github.com/web-platform-tests/wpt/issues/13393
+  [createImageData() throws TypeError if arguments are not finite]
+    expected: FAIL
+
diff --git a/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.create2.zero.html.ini b/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.create2.zero.html.ini
deleted file mode 100644
index ae36bc71a82..00000000000
--- a/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.create2.zero.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[2d.imageData.create2.zero.html]
-  type: testharness
-  [createImageData(sw, sh) throws INDEX_SIZE_ERR if size is zero]
-    expected: FAIL
-
diff --git a/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.get.nonfinite.html.ini b/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.get.nonfinite.html.ini
new file mode 100644
index 00000000000..dfb06a53b30
--- /dev/null
+++ b/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.get.nonfinite.html.ini
@@ -0,0 +1,5 @@
+[2d.imageData.get.nonfinite.html]
+  bug: https://github.com/web-platform-tests/wpt/issues/13393
+  [getImageData() throws TypeError if arguments are not finite]
+    expected: FAIL
+
diff --git a/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.get.zero.html.ini b/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.get.zero.html.ini
deleted file mode 100644
index 1535daa7110..00000000000
--- a/tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.get.zero.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[2d.imageData.get.zero.html]
-  type: testharness
-  [getImageData() throws INDEX_SIZE_ERR if size is zero]
-    expected: FAIL
-