Auto merge of #21868 - servo:webgl, r=jdm

Improve ctx.putImageData

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21868)
<!-- Reviewable:end -->

components/canvas/canvas_data.rs

@@ -452,62 +452,15 @@ impl<'a> CanvasData<'a> {
     pub fn put_image_data(
         &mut self,
         imagedata: Vec<u8>,
-        offset: Vector2D<f64>,
+        offset: Vector2D<i32>,
-        image_data_size: Size2D<f64>,
+        image_data_size: Size2D<i32>,
-        mut dirty_rect: Rect<f64>
+        dest_rect: Rect<i32>,
     ) {
-        if image_data_size.width <= 0.0 || image_data_size.height <= 0.0 {
+        assert_eq!(image_data_size.width * image_data_size.height * 4, imagedata.len() as i32);
-            return
-        }
 
-        assert_eq!(image_data_size.width * image_data_size.height * 4.0, imagedata.len() as f64);
+        let image_size = image_data_size;
 
-        // Step 1. TODO (neutered data)
+        let first_pixel = dest_rect.origin;
-
-        // Step 2.
-        if dirty_rect.size.width < 0.0f64 {
-            dirty_rect.origin.x += dirty_rect.size.width;
-            dirty_rect.size.width = -dirty_rect.size.width;
-        }
-
-        if dirty_rect.size.height < 0.0f64 {
-            dirty_rect.origin.y += dirty_rect.size.height;
-            dirty_rect.size.height = -dirty_rect.size.height;
-        }
-
-        // Step 3.
-        if dirty_rect.origin.x < 0.0f64 {
-            dirty_rect.size.width += dirty_rect.origin.x;
-            dirty_rect.origin.x = 0.0f64;
-        }
-
-        if dirty_rect.origin.y < 0.0f64 {
-            dirty_rect.size.height += dirty_rect.origin.y;
-            dirty_rect.origin.y = 0.0f64;
-        }
-
-        // Step 4.
-        if dirty_rect.max_x() > image_data_size.width {
-            dirty_rect.size.width = image_data_size.width - dirty_rect.origin.x;
-        }
-
-        if dirty_rect.max_y() > image_data_size.height {
-            dirty_rect.size.height = image_data_size.height - dirty_rect.origin.y;
-        }
-
-        // 5) If either dirtyWidth or dirtyHeight is negative or zero,
-        // stop without affecting any bitmaps
-        if dirty_rect.size.width <= 0.0 || dirty_rect.size.height <= 0.0 {
-            return
-        }
-
-        // Step 6.
-        let dest_rect = dirty_rect.translate(&offset).to_i32();
-
-        // azure_hl operates with integers. We need to cast the image size
-        let image_size = image_data_size.to_i32();
-
-        let first_pixel = dest_rect.origin - offset.to_i32();
         let mut src_line = (first_pixel.y * (image_size.width * 4) + first_pixel.x * 4) as usize;
 
         let mut dest =
@@ -533,9 +486,7 @@ impl<'a> CanvasData<'a> {
                 dest_rect.size,
                 dest_rect.size.width * 4,
                 SurfaceFormat::B8G8R8A8) {
-            self.drawtarget.copy_surface(source_surface,
+            self.drawtarget.copy_surface(source_surface, Rect::from_size(dest_rect.size), offset.to_point());
-                                         Rect::new(Point2D::new(0, 0), dest_rect.size),
-                                         dest_rect.origin);
         }
     }
 

components/canvas_traits/canvas.rs

@@ -54,7 +54,7 @@ pub enum Canvas2dMsg {
     IsPointInPath(f64, f64, FillRule, IpcSender<bool>),
     LineTo(Point2D<f32>),
     MoveTo(Point2D<f32>),
-    PutImageData(ByteBuf, Vector2D<f64>, Size2D<f64>, Rect<f64>),
+    PutImageData(ByteBuf, Vector2D<i32>, Size2D<i32>, Rect<i32>),
     QuadraticCurveTo(Point2D<f32>, Point2D<f32>),
     Rect(Rect<f32>),
     RestoreContext,

components/script/dom/canvasrenderingcontext2d.rs

@@ -31,7 +31,7 @@ use dom::htmlcanvaselement::{CanvasContext, HTMLCanvasElement};
 use dom::imagedata::ImageData;
 use dom::node::{Node, NodeDamage, window_from_node};
 use dom_struct::dom_struct;
-use euclid::{Transform2D, Point2D, Vector2D, Rect, Size2D, vec2};
+use euclid::{Transform2D, Point2D, Rect, Size2D, vec2};
 use ipc_channel::ipc::{self, IpcSender};
 use net_traits::image::base::PixelFormat;
 use net_traits::image_cache::CanRequestImages;
@@ -1191,42 +1191,104 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-putimagedata
-    fn PutImageData(&self, imagedata: &ImageData, dx: Finite<f64>, dy: Finite<f64>) {
+    fn PutImageData(&self, imagedata: &ImageData, dx: i32, dy: i32) {
-        self.PutImageData_(
+        self.PutImageData_(imagedata, dx, dy, 0, 0, imagedata.Width() as i32, imagedata.Height() as i32)
-            imagedata,
-            dx,
-            dy,
-            Finite::wrap(0f64),
-            Finite::wrap(0f64),
-            Finite::wrap(imagedata.Width() as f64),
-            Finite::wrap(imagedata.Height() as f64),
-        )
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-putimagedata
     fn PutImageData_(
         &self,
         imagedata: &ImageData,
-        dx: Finite<f64>,
+        dx: i32,
-        dy: Finite<f64>,
+        dy: i32,
-        dirty_x: Finite<f64>,
+        mut dirty_x: i32,
-        dirty_y: Finite<f64>,
+        mut dirty_y: i32,
-        dirty_width: Finite<f64>,
+        mut dirty_width: i32,
-        dirty_height: Finite<f64>,
+        mut dirty_height: i32,
     ) {
-        let data = imagedata.get_data_array();
+        // FIXME(nox): There are many arithmetic operations here that can
-        let offset = Vector2D::new(*dx, *dy);
+        // overflow or underflow, this should probably be audited.
-        let image_data_size = Size2D::new(imagedata.Width() as f64, imagedata.Height() as f64);
 
-        let dirty_rect = Rect::new(
+        let imagedata_size = Size2D::new(imagedata.Width() as i32, imagedata.Height() as i32);
-            Point2D::new(*dirty_x, *dirty_y),
+        if imagedata_size.width <= 0 || imagedata_size.height <= 0 {
-            Size2D::new(*dirty_width, *dirty_height),
+            return;
-        );
+        }
+
+        // Step 1.
+        // Done later.
+
+        // Step 2.
+        // TODO: throw InvalidState if buffer is detached.
+
+        // Step 3.
+        if dirty_width < 0 {
+            dirty_x += dirty_width;
+            dirty_width = -dirty_width;
+        }
+        if dirty_height < 0 {
+            dirty_y += dirty_height;
+            dirty_height = -dirty_height;
+        }
+
+        // Ignore any pixel that would be drawn before the beginning of the
+        // canvas surface.
+        let mut dest_x = dx + dirty_x;
+        let mut dest_y = dy + dirty_y;
+        if dest_x < 0 {
+            dirty_x -= dest_x;
+            dirty_width += dest_x;
+            dest_x = 0;
+        }
+        if dest_y < 0 {
+            dirty_y -= dest_y;
+            dirty_height += dest_y;
+            dest_y = 0;
+        }
+
+        // Step 4.
+        if dirty_x < 0 {
+            dirty_width += dirty_x;
+            dirty_x = 0;
+        }
+        if dirty_y < 0 {
+            dirty_height += dirty_y;
+            dirty_y = 0;
+        }
+
+        // Step 5.
+        if dirty_x + dirty_width > imagedata_size.width {
+            dirty_width = imagedata_size.width - dirty_x;
+        }
+        if dirty_y + dirty_height > imagedata_size.height {
+            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.
+        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();
+        dirty_width = dirty_width.min(drawable_size.width);
+        dirty_height = dirty_height.min(drawable_size.height);
+
+        // Step 6.
+        if dirty_width <= 0 || dirty_height <= 0 {
+            return;
+        }
+
+        // FIXME(nox): There is no need to make a Vec<u8> of all the pixels
+        // if we didn't want to put the entire image.
+        let buffer = imagedata.get_data_array();
+
+        // Step 7.
         self.send_canvas_2d_msg(Canvas2dMsg::PutImageData(
-            data.into(),
+            buffer.into(),
-            offset,
+            origin.to_vector(),
-            image_data_size,
+            imagedata_size,
-            dirty_rect,
+            Rect::new(
+                Point2D::new(dirty_x, dirty_y),
+                Size2D::new(dirty_width, dirty_height),
+            ),
         ));
         self.mark_as_dirty();
     }

components/script/dom/webidls/CanvasRenderingContext2D.webidl

@@ -180,11 +180,11 @@ interface CanvasImageData {
   ImageData createImageData(ImageData imagedata);
   [Throws]
   ImageData getImageData(double sx, double sy, double sw, double sh);
-  void putImageData(ImageData imagedata, double dx, double dy);
+  void putImageData(ImageData imagedata, long dx, long dy);
   void putImageData(ImageData imagedata,
-                    double dx, double dy,
+                    long dx, long dy,
-                    double dirtyX, double dirtyY,
+                    long dirtyX, long dirtyY,
-                    double dirtyWidth, double dirtyHeight);
+                    long dirtyWidth, long dirtyHeight);
 };
 
 enum CanvasLineCap { "butt", "round", "square" };

tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.put.nonfinite.html.ini

@@ -0,0 +1,5 @@
+[2d.imageData.put.nonfinite.html]
+  bug: https://github.com/web-platform-tests/wpt/issues/13393
+  [putImageData() throws TypeError if arguments are not finite]
+    expected: FAIL
+