Avoid copying pixels in ctx.putImageData sometimes

components/canvas/canvas_paint_thread.rs

@@ -241,8 +241,12 @@ impl<'a> CanvasPaintThread <'a> {
             Canvas2dMsg::GetImageData(dest_rect, canvas_size, chan) => {
                 self.canvas(canvas_id).image_data(dest_rect, canvas_size, chan)
             },
-            Canvas2dMsg::PutImageData(imagedata, offset, imagedata_size) => {
+            Canvas2dMsg::PutImageData(receiver, offset, imagedata_size) => {
-                self.canvas(canvas_id).put_image_data(imagedata.into(), offset, imagedata_size)
+                self.canvas(canvas_id).put_image_data(
+                    receiver.recv().unwrap(),
+                    offset,
+                    imagedata_size,
+                )
             },
             Canvas2dMsg::SetShadowOffsetX(value) => {
                 self.canvas(canvas_id).set_shadow_offset_x(value)

components/canvas_traits/canvas.rs

@@ -4,7 +4,7 @@
 
 use cssparser::RGBA;
 use euclid::{Transform2D, Point2D, Vector2D, Rect, Size2D};
-use ipc_channel::ipc::{IpcBytesSender, IpcSender};
+use ipc_channel::ipc::{IpcBytesReceiver, IpcBytesSender, IpcSender};
 use serde_bytes::ByteBuf;
 use std::default::Default;
 use std::str::FromStr;
@@ -19,7 +19,7 @@ pub enum FillRule {
 #[derive(Clone, Copy, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)]
 pub struct CanvasId(pub u64);
 
-#[derive(Clone, Deserialize, Serialize)]
+#[derive(Deserialize, Serialize)]
 pub enum CanvasMsg {
     Canvas2d(Canvas2dMsg, CanvasId),
     Create(IpcSender<CanvasId>, Size2D<u32>, webrender_api::RenderApiSender, bool),
@@ -34,7 +34,7 @@ pub struct CanvasImageData {
     pub image_key: webrender_api::ImageKey,
 }
 
-#[derive(Clone, Deserialize, Serialize)]
+#[derive(Deserialize, Serialize)]
 pub enum Canvas2dMsg {
     Arc(Point2D<f32>, f32, f32, f32, bool),
     ArcTo(Point2D<f32>, Point2D<f32>, f32),
@@ -54,7 +54,7 @@ pub enum Canvas2dMsg {
     IsPointInPath(f64, f64, FillRule, IpcSender<bool>),
     LineTo(Point2D<f32>),
     MoveTo(Point2D<f32>),
-    PutImageData(ByteBuf, Vector2D<i32>, Size2D<i32>),
+    PutImageData(IpcBytesReceiver, Vector2D<i32>, Size2D<i32>),
     QuadraticCurveTo(Point2D<f32>, Point2D<f32>),
     Rect(Rect<f32>),
     RestoreContext,

components/script/dom/canvasrenderingcontext2d.rs

@@ -1197,6 +1197,7 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-putimagedata
+    #[allow(unsafe_code)]
     fn PutImageData_(
         &self,
         imagedata: &ImageData,
@@ -1281,11 +1282,13 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
         let dirty_rect = Rect::new(Point2D::new(dirty_x, dirty_y), dirty_size);
 
         // Step 7.
+        let (sender, receiver) = ipc::bytes_channel().unwrap();
         self.send_canvas_2d_msg(Canvas2dMsg::PutImageData(
-            imagedata.get_rect(dirty_rect.try_cast().unwrap()).into(),
+            receiver,
             origin.to_vector(),
             dirty_size,
         ));
+        sender.send(unsafe { &imagedata.get_rect(dirty_rect.try_cast().unwrap()) }).unwrap();
         self.mark_as_dirty();
     }
 

components/script/dom/htmlcanvaselement.rs

@@ -369,13 +369,14 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement {
         // Step 3.
         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.get_data_array()
+                image_data.to_vec()
             },
             Some(CanvasContext::WebGL(ref context)) => {
                 match context.get_image_data(self.Width(), self.Height()) {

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 std::borrow::Cow;
 use std::default::Default;
 use std::ptr;
 use std::ptr::NonNull;
@@ -137,43 +138,46 @@ impl ImageData {
         Self::new_with_jsobject(global, width, opt_height, Some(jsobject))
     }
 
+    /// Nothing must change the array on the JS side while the slice is live.
     #[allow(unsafe_code)]
-    pub fn get_data_array(&self) -> Vec<u8> {
+    pub unsafe fn as_slice(&self) -> &[u8] {
-        unsafe {
+        assert!(!self.data.get().is_null());
-            assert!(!self.data.get().is_null());
+        let cx = Runtime::get();
-            let cx = Runtime::get();
+        assert!(!cx.is_null());
-            assert!(!cx.is_null());
+        typedarray!(in(cx) let array: Uint8ClampedArray = self.data.get());
-            typedarray!(in(cx) let array: Uint8ClampedArray = self.data.get());
+        let array = array.as_ref().unwrap();
-            let vec = array.unwrap().as_slice().to_vec();
+        // NOTE(nox): This is just as unsafe as `as_slice` itself even though we
-            vec
+        // are extending the lifetime of the slice, because the data in
-        }
+        // this ImageData instance will never change. The method is thus unsafe
+        // because the array may be manipulated from JS while the reference
+        // is live.
+        let ptr = array.as_slice() as *const _;
+        &*ptr
     }
 
     #[allow(unsafe_code)]
-    pub fn get_rect(&self, rect: Rect<u32>) -> Vec<u8> {
+    pub fn to_vec(&self) -> Vec<u8> {
-        unsafe {
+        unsafe { self.as_slice().into() }
-            assert!(!rect.is_empty());
+    }
-            assert!(self.rect().contains_rect(&rect));
+
-            assert!(!self.data.get().is_null());
+    #[allow(unsafe_code)]
-            let cx = Runtime::get();
+    pub unsafe fn get_rect(&self, rect: Rect<u32>) -> Cow<[u8]> {
-            assert!(!cx.is_null());
+        assert!(!rect.is_empty());
-            typedarray!(in(cx) let array: Uint8ClampedArray = self.data.get());
+        assert!(self.rect().contains_rect(&rect));
-            let slice = array.as_ref().unwrap().as_slice();
+        let slice = self.as_slice();
-            let area = rect.size.area() as usize;
+        let area = rect.size.area() as usize;
-            let first_column_start = rect.origin.x as usize * 4;
+        let first_column_start = rect.origin.x as usize * 4;
-            let row_length = self.width as usize * 4;
+        let row_length = self.width as usize * 4;
-            let first_row_start = rect.origin.y as usize * row_length;
+        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 {
+        if rect.origin.x == 0 && rect.size.width == self.width || rect.size.height == 1 {
-                let start = first_column_start + first_row_start;
+            let start = first_column_start + first_row_start;
-                // FIXME(nox): This should be a borrow.
+            return Cow::Borrowed(&slice[start..start + area * 4]);
-                return slice[start..start + area * 4].into();
-            }
-            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
         }
+        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> {

components/script/dom/webglrenderingcontext.rs

@@ -520,7 +520,7 @@ impl WebGLRenderingContext {
     ) -> Fallible<Option<(Vec<u8>, Size2D<u32>, bool)>> {
         Ok(Some(match source {
             TexImageSource::ImageData(image_data) => {
-                (image_data.get_data_array(), image_data.get_size(), false)
+                (image_data.to_vec(), image_data.get_size(), false)
             },
             TexImageSource::HTMLImageElement(image) => {
                 let document = document_from_node(&*self.canvas);