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

Improve some byte-swap and premultiply operations

<!-- 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/21877)
<!-- Reviewable:end -->

Cargo.lock

@@ -323,6 +323,7 @@ dependencies = [
  "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-traits 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "offscreen_gl_context 0.21.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "pixels 0.0.1",
  "serde_bytes 0.10.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "servo_config 0.0.1",
  "webrender 0.57.2 (git+https://github.com/servo/webrender)",
@@ -2277,6 +2278,7 @@ dependencies = [
  "msg 0.0.1",
  "net_traits 0.0.1",
  "openssl 0.9.24 (registry+https://github.com/rust-lang/crates.io-index)",
+ "pixels 0.0.1",
  "profile_traits 0.0.1",
  "serde 1.0.66 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_json 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2321,6 +2323,7 @@ dependencies = [
  "malloc_size_of_derive 0.0.1",
  "msg 0.0.1",
  "num-traits 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
+ "pixels 0.0.1",
  "serde 1.0.66 (registry+https://github.com/rust-lang/crates.io-index)",
  "servo_arc 0.1.1",
  "servo_config 0.0.1",
@@ -2611,6 +2614,13 @@ dependencies = [
  "unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[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"
 version = "0.3.14"
@@ -2981,6 +2991,7 @@ dependencies = [
  "phf 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "phf_codegen 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)",
  "phf_shared 0.7.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "pixels 0.0.1",
  "profile_traits 0.0.1",
  "ref_filter_map 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "ref_slice 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)",

components/canvas/Cargo.toml

@@ -24,6 +24,7 @@ ipc-channel = "0.11"
 log = "0.4"
 num-traits = "0.2"
 offscreen_gl_context = {version = "0.21", features = ["serde", "osmesa"]}
+pixels = {path = "../pixels"}
 serde_bytes = "0.10"
 servo_config = {path = "../config"}
 webrender = {git = "https://github.com/servo/webrender"}

components/canvas/canvas_data.rs

@@ -11,8 +11,9 @@ 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;
 use std::mem;
 use std::sync::Arc;
@@ -439,55 +440,22 @@ 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(
+    pub fn put_image_data(&mut self, mut imagedata: Vec<u8>, rect: Rect<u32>) {
-        &mut self,
+        assert_eq!(imagedata.len() % 4, 0);
-        imagedata: Vec<u8>,
+        assert_eq!(rect.size.area() as usize, imagedata.len() / 4);
-        offset: Vector2D<i32>,
+        pixels::byte_swap_and_premultiply_inplace(&mut imagedata);
-        image_data_size: Size2D<i32>,
+        let source_surface = self.drawtarget.create_source_surface_from_data(
-        dest_rect: Rect<i32>,
+            &imagedata,
-    ) {
+            rect.size.to_i32(),
-        assert_eq!(image_data_size.width * image_data_size.height * 4, imagedata.len() as i32);
+            rect.size.width as i32 * 4,
-
+            SurfaceFormat::B8G8R8A8,
-        let image_size = image_data_size;
+        ).unwrap();
-
+        self.drawtarget.copy_surface(
-        let first_pixel = dest_rect.origin;
+            source_surface,
-        let mut src_line = (first_pixel.y * (image_size.width * 4) + first_pixel.x * 4) as usize;
+            Rect::from_size(rect.size.to_i32()),
-
+            rect.origin.to_i32(),
-        let mut dest =
+        );
-            Vec::with_capacity((dest_rect.size.width * dest_rect.size.height * 4) as usize);
-
-        for _ in 0 .. dest_rect.size.height {
-            let mut src_offset = src_line;
-            for _ in 0 .. dest_rect.size.width {
-                let alpha = imagedata[src_offset + 3] as u16;
-                // add 127 before dividing for more accurate rounding
-                let premultiply_channel = |channel: u8| (((channel as u16 * alpha) + 127) / 255) as u8;
-                dest.push(premultiply_channel(imagedata[src_offset + 2]));
-                dest.push(premultiply_channel(imagedata[src_offset + 1]));
-                dest.push(premultiply_channel(imagedata[src_offset + 0]));
-                dest.push(imagedata[src_offset + 3]);
-                src_offset += 4;
-            }
-            src_line += (image_size.width * 4) as usize;
-        }
-
-        if let Some(source_surface) = self.drawtarget.create_source_surface_from_data(
-                &dest,
-                dest_rect.size,
-                dest_rect.size.width * 4,
-                SurfaceFormat::B8G8R8A8) {
-            self.drawtarget.copy_surface(source_surface, Rect::from_size(dest_rect.size), offset.to_point());
-        }
     }
 
     pub fn set_shadow_offset_x(&mut self, value: f64) {
@@ -545,32 +513,13 @@ 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> {
+    pub fn read_pixels(&self, read_rect: Rect<u32>, canvas_size: Size2D<u32>) -> Vec<u8> {
-        let canvas_size = canvas_size.to_i32();
+        let canvas_rect = Rect::from_size(canvas_size);
-        let canvas_rect = Rect::new(Point2D::new(0i32, 0i32), canvas_size);
+        if canvas_rect.intersection(&read_rect).map_or(true, |rect| rect.is_empty()) {
-        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 {
             return vec![];
         }
-
         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
     }
 }
 
@@ -641,6 +590,9 @@ fn crop_image(
     // We're going to iterate over a pixel values array so we need integers
     let crop_rect = crop_rect.to_i32();
     let image_size = image_size.to_i32();
+    if crop_rect == Rect::from_size(image_size) {
+        return image_data;
+    }
     // Assuming 4 bytes per pixel and row-major order for storage
     // (consecutive elements in a pixel row of the image are contiguous in memory)
     let stride = image_size.width * 4;
@@ -690,23 +642,23 @@ fn write_image(
     } else {
         Filter::Point
     };
-    // azure_hl operates with integers. We need to cast the image size
     let image_size = image_size.to_i32();
 
-    if let Some(source_surface) =
+    let source_surface = draw_target.create_source_surface_from_data(
-            draw_target.create_source_surface_from_data(&image_data,
+        &image_data,
         image_size,
         image_size.width * 4,
-                                                        SurfaceFormat::B8G8R8A8) {
+        SurfaceFormat::B8G8R8A8,
+    ).unwrap();
     let draw_surface_options = DrawSurfaceOptions::new(filter, true);
     let draw_options = DrawOptions::new(global_alpha, composition_op, AntialiasMode::None);
-
+    draw_target.draw_surface(
-        draw_target.draw_surface(source_surface,
+        source_surface,
         dest_rect.to_azure_style(),
         image_rect.to_azure_style(),
         draw_surface_options,
-                                 draw_options);
+        draw_options,
-    }
+    );
 }
 
 pub trait PointToi32 {

components/canvas/canvas_paint_thread.rs

@@ -7,6 +7,7 @@ use canvas_data::*;
 use canvas_traits::canvas::*;
 use euclid::Size2D;
 use ipc_channel::ipc::{self, IpcSender};
+use pixels;
 use std::borrow::ToOwned;
 use std::collections::HashMap;
 use std::thread;
@@ -141,7 +142,7 @@ impl<'a> CanvasPaintThread <'a> {
                 let data = match imagedata {
                     None => vec![0; image_size.width as usize * image_size.height as usize * 4],
                     Some(mut data) => {
-                        byte_swap(&mut data);
+                        pixels::byte_swap_colors_inplace(&mut data);
                         data.into()
                     },
                 };
@@ -161,8 +162,8 @@ impl<'a> CanvasPaintThread <'a> {
                 smoothing
             ) => {
                 let image_data = self.canvas(canvas_id).read_pixels(
-                    source_rect.to_i32(),
+                    source_rect.to_u32(),
-                    image_size,
+                    image_size.to_u32(),
                 );
                 self.canvas(other_canvas_id).draw_image(
                     image_data.into(),
@@ -237,21 +238,12 @@ impl<'a> CanvasPaintThread <'a> {
             Canvas2dMsg::SetGlobalComposition(op) => {
                 self.canvas(canvas_id).set_global_composition(op)
             },
-            Canvas2dMsg::GetImageData(dest_rect, canvas_size, chan) => {
+            Canvas2dMsg::GetImageData(dest_rect, canvas_size, sender) => {
-                self.canvas(canvas_id).image_data(dest_rect, canvas_size, chan)
+                let pixels = self.canvas(canvas_id).read_pixels(dest_rect, canvas_size);
+                sender.send(&pixels).unwrap();
             },
-            Canvas2dMsg::PutImageData(
+            Canvas2dMsg::PutImageData(rect, receiver) => {
-                imagedata,
+                self.canvas(canvas_id).put_image_data(receiver.recv().unwrap(), rect);
-                offset,
-                image_data_size,
-                dirty_rect,
-            ) => {
-                self.canvas(canvas_id).put_image_data(
-                    imagedata.into(),
-                    offset,
-                    image_data_size,
-                    dirty_rect,
-                )
             },
             Canvas2dMsg::SetShadowOffsetX(value) => {
                 self.canvas(canvas_id).set_shadow_offset_x(value)

components/canvas/lib.rs

@@ -15,6 +15,7 @@ extern crate ipc_channel;
 #[macro_use] extern crate log;
 extern crate num_traits;
 extern crate offscreen_gl_context;
+extern crate pixels;
 extern crate serde_bytes;
 extern crate servo_config;
 extern crate webrender;

components/canvas/webgl_thread.rs

@@ -2,13 +2,12 @@
  * 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/. */
 
-use canvas_traits::canvas::byte_swap;
 use canvas_traits::webgl::*;
 use euclid::Size2D;
 use fnv::FnvHashMap;
 use gleam::gl;
-use ipc_channel::ipc::IpcBytesSender;
 use offscreen_gl_context::{GLContext, GLContextAttributes, GLLimits, NativeGLContextMethods};
+use pixels;
 use std::thread;
 use super::gl_context::{GLContextFactory, GLContextWrapper};
 use webrender;
@@ -562,7 +561,7 @@ impl<VR: WebVRRenderHandler + 'static> WebGLThread<VR> {
             let src_slice = &orig_pixels[src_start .. src_start + stride];
             (&mut pixels[dst_start .. dst_start + stride]).clone_from_slice(&src_slice[..stride]);
         }
-        byte_swap(&mut pixels);
+        pixels::byte_swap_colors_inplace(&mut pixels);
         pixels
     }
 
@@ -791,8 +790,16 @@ impl WebGLImpl {
                 ctx.gl().pixel_store_i(name, val),
             WebGLCommand::PolygonOffset(factor, units) =>
                 ctx.gl().polygon_offset(factor, units),
-            WebGLCommand::ReadPixels(x, y, width, height, format, pixel_type, ref chan) => {
+            WebGLCommand::ReadPixels(rect, format, pixel_type, ref sender) => {
-                Self::read_pixels(ctx.gl(), x, y, width, height, format, pixel_type, chan)
+                let pixels = ctx.gl().read_pixels(
+                    rect.origin.x as i32,
+                    rect.origin.y as i32,
+                    rect.size.width as i32,
+                    rect.size.height as i32,
+                    format,
+                    pixel_type,
+                );
+                sender.send(&pixels).unwrap();
             }
             WebGLCommand::RenderbufferStorage(target, format, width, height) =>
                 ctx.gl().renderbuffer_storage(target, format, width, height),
@@ -1335,20 +1342,6 @@ impl WebGLImpl {
         }
     }
 
-    fn read_pixels(
-        gl: &gl::Gl,
-        x: i32,
-        y: i32,
-        width: i32,
-        height: i32,
-        format: u32,
-        pixel_type: u32,
-        chan: &IpcBytesSender,
-    ) {
-      let result = gl.read_pixels(x, y, width, height, format, pixel_type);
-      chan.send(&result).unwrap()
-    }
-
     fn finish(gl: &gl::Gl, chan: &WebGLSender<()>) {
         gl.finish();
         chan.send(()).unwrap();

components/canvas_traits/canvas.rs

@@ -3,8 +3,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cssparser::RGBA;
-use euclid::{Transform2D, Point2D, Vector2D, Rect, Size2D};
+use euclid::{Transform2D, Point2D, 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),
@@ -50,11 +50,11 @@ pub enum Canvas2dMsg {
     Fill,
     FillText(String, f64, f64, Option<f64>),
     FillRect(Rect<f32>),
-    GetImageData(Rect<i32>, Size2D<f64>, IpcBytesSender),
+    GetImageData(Rect<u32>, Size2D<u32>, IpcBytesSender),
     IsPointInPath(f64, f64, FillRule, IpcSender<bool>),
     LineTo(Point2D<f32>),
     MoveTo(Point2D<f32>),
-    PutImageData(ByteBuf, Vector2D<i32>, Size2D<i32>, Rect<i32>),
+    PutImageData(Rect<u32>, IpcBytesReceiver),
     QuadraticCurveTo(Point2D<f32>, Point2D<f32>),
     Rect(Rect<f32>),
     RestoreContext,
@@ -382,38 +382,3 @@ impl FromStr for CompositionOrBlending {
         Err(())
     }
 }
-
-// TODO(pcwalton): Speed up with SIMD, or better yet, find some way to not do this.
-pub fn byte_swap(data: &mut [u8]) {
-    let length = data.len();
-    // FIXME(rust #27741): Range::step_by is not stable yet as of this writing.
-    let mut i = 0;
-    while i < length {
-        let r = data[i + 2];
-        data[i + 2] = data[i + 0];
-        data[i + 0] = r;
-        i += 4;
-    }
-}
-
-pub fn multiply_u8_pixel(a: u8, b: u8) -> u8 {
-    return (a as u32 * b as u32 / 255) as u8;
-}
-
-pub fn byte_swap_and_premultiply(data: &mut [u8]) {
-    let length = data.len();
-
-    let mut i = 0;
-    while i < length {
-        let r = data[i + 2];
-        let g = data[i + 1];
-        let b = data[i + 0];
-        let a = data[i + 3];
-
-        data[i + 0] = multiply_u8_pixel(r, a);
-        data[i + 1] = multiply_u8_pixel(g, a);
-        data[i + 2] = multiply_u8_pixel(b, a);
-
-        i += 4;
-    }
-}

components/canvas_traits/webgl.rs

@@ -2,7 +2,7 @@
  * 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/. */
 
-use euclid::Size2D;
+use euclid::{Rect, Size2D};
 use gleam::gl;
 use ipc_channel::ipc::{IpcBytesReceiver, IpcBytesSender};
 use offscreen_gl_context::{GLContextAttributes, GLLimits};
@@ -227,7 +227,7 @@ pub enum WebGLCommand {
     GetRenderbufferParameter(u32, u32, WebGLSender<i32>),
     PolygonOffset(f32, f32),
     RenderbufferStorage(u32, u32, i32, i32),
-    ReadPixels(i32, i32, i32, i32, u32, u32, IpcBytesSender),
+    ReadPixels(Rect<u32>, u32, u32, IpcBytesSender),
     SampleCoverage(f32, bool),
     Scissor(i32, i32, u32, u32),
     StencilFunc(u32, i32, u32),

components/net/Cargo.toml

@@ -34,6 +34,7 @@ mime_guess = "1.8.0"
 msg = {path = "../msg"}
 net_traits = {path = "../net_traits"}
 openssl = "0.9"
+pixels = {path = "../pixels"}
 profile_traits = {path = "../profile_traits"}
 serde = "1.0"
 serde_json = "1.0"

components/net/image_cache.rs

@@ -9,6 +9,7 @@ use net_traits::image::base::{Image, ImageMetadata, PixelFormat, load_from_memor
 use net_traits::image_cache::{CanRequestImages, ImageCache, ImageResponder};
 use net_traits::image_cache::{ImageOrMetadataAvailable, ImageResponse, ImageState};
 use net_traits::image_cache::{PendingImageId, UsePlaceholder};
+use pixels;
 use servo_url::ServoUrl;
 use std::collections::HashMap;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
@@ -52,7 +53,7 @@ fn set_webrender_image_key(webrender_api: &webrender_api::RenderApi, image: &mut
     let is_opaque = match image.format {
         PixelFormat::BGRA8 => {
             bytes.extend_from_slice(&*image.bytes);
-            premultiply(bytes.as_mut_slice())
+            pixels::premultiply_inplace(bytes.as_mut_slice())
         }
         PixelFormat::RGB8 => {
             for bgr in image.bytes.chunks(3) {
@@ -86,30 +87,6 @@ fn set_webrender_image_key(webrender_api: &webrender_api::RenderApi, image: &mut
     image.id = Some(image_key);
 }
 
-// Returns true if the image was found to be
-// completely opaque.
-fn premultiply(data: &mut [u8]) -> bool {
-    let mut is_opaque = true;
-    let length = data.len();
-
-    let mut i = 0;
-    while i < length {
-        let b = data[i + 0] as u32;
-        let g = data[i + 1] as u32;
-        let r = data[i + 2] as u32;
-        let a = data[i + 3] as u32;
-
-        data[i + 0] = (b * a / 255) as u8;
-        data[i + 1] = (g * a / 255) as u8;
-        data[i + 2] = (r * a / 255) as u8;
-
-        i += 4;
-        is_opaque = is_opaque && a == 255;
-    }
-
-    is_opaque
-}
-
 // ======================================================================
 // Aux structs and enums.
 // ======================================================================

components/net/lib.rs

@@ -27,6 +27,7 @@ extern crate mime_guess;
 extern crate msg;
 extern crate net_traits;
 extern crate openssl;
+extern crate pixels;
 #[macro_use]
 extern crate profile_traits;
 #[macro_use] extern crate serde;

components/net_traits/Cargo.toml

@@ -24,6 +24,7 @@ malloc_size_of = { path = "../malloc_size_of" }
 malloc_size_of_derive = { path = "../malloc_size_of_derive" }
 msg = {path = "../msg"}
 num-traits = "0.2"
+pixels = {path = "../pixels"}
 serde = "1.0"
 servo_arc = {path = "../servo_arc"}
 servo_config = {path = "../config"}

components/net_traits/image/base.rs

@@ -4,6 +4,7 @@
 
 use ipc_channel::ipc::IpcSharedMemory;
 use piston_image::{self, DynamicImage, ImageFormat};
+use pixels;
 use std::fmt;
 use webrender_api;
 
@@ -46,24 +47,6 @@ pub struct ImageMetadata {
 // FIXME: Images must not be copied every frame. Instead we should atomically
 // reference count them.
 
-// TODO(pcwalton): Speed up with SIMD, or better yet, find some way to not do this.
-fn byte_swap_and_premultiply(data: &mut [u8]) {
-    let length = data.len();
-
-    let mut i = 0;
-    while i < length {
-        let r = data[i + 2];
-        let g = data[i + 1];
-        let b = data[i + 0];
-
-        data[i + 0] = r;
-        data[i + 1] = g;
-        data[i + 2] = b;
-
-        i += 4;
-    }
-}
-
 pub fn load_from_memory(buffer: &[u8]) -> Option<Image> {
     if buffer.is_empty() {
         return None;
@@ -82,7 +65,7 @@ pub fn load_from_memory(buffer: &[u8]) -> Option<Image> {
                         DynamicImage::ImageRgba8(rgba) => rgba,
                         image => image.to_rgba(),
                     };
-                    byte_swap_and_premultiply(&mut *rgba);
+                    pixels::byte_swap_colors_inplace(&mut *rgba);
                     Some(Image {
                         width: rgba.width(),
                         height: rgba.height(),

components/net_traits/lib.rs

@@ -17,6 +17,7 @@ extern crate ipc_channel;
 #[macro_use] extern crate malloc_size_of_derive;
 extern crate msg;
 extern crate num_traits;
+extern crate pixels;
 #[macro_use] extern crate serde;
 extern crate servo_arc;
 extern crate servo_url;

components/pixels/Cargo.toml

@@ -0,0 +1,13 @@
+[package]
+name = "pixels"
+version = "0.0.1"
+authors = ["The Servo Project Developers"]
+license = "MPL-2.0"
+publish = false
+
+[lib]
+name = "pixels"
+path = "lib.rs"
+
+[dependencies]
+euclid = "0.19"

components/pixels/lib.rs

@@ -0,0 +1,83 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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::{Point2D, 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);
+    for rgba in pixels.chunks_mut(4) {
+        let b = rgba[0];
+        rgba[0] = rgba[2];
+        rgba[2] = b;
+    }
+}
+
+pub fn byte_swap_and_premultiply_inplace(pixels: &mut [u8]) {
+    assert!(pixels.len() % 4 == 0);
+    for rgba in pixels.chunks_mut(4) {
+        let b = rgba[0];
+        rgba[0] = multiply_u8_color(rgba[2], rgba[3]);
+        rgba[1] = multiply_u8_color(rgba[1], rgba[3]);
+        rgba[2] = multiply_u8_color(b, rgba[3]);
+    }
+}
+
+/// Returns true if the pixels were found to be completely opaque.
+pub fn premultiply_inplace(pixels: &mut [u8]) -> bool {
+    assert!(pixels.len() % 4 == 0);
+    let mut is_opaque = true;
+    for rgba in pixels.chunks_mut(4) {
+        rgba[0] = multiply_u8_color(rgba[0], rgba[3]);
+        rgba[1] = multiply_u8_color(rgba[1], rgba[3]);
+        rgba[2] = multiply_u8_color(rgba[2], rgba[3]);
+        is_opaque = is_opaque && rgba[3] == 255;
+    }
+    is_opaque
+}
+
+pub fn multiply_u8_color(a: u8, b: u8) -> u8 {
+    return (a as u32 * b as u32 / 255) as u8;
+}
+
+pub fn clip(
+    mut origin: Point2D<i32>,
+    mut size: Size2D<u32>,
+    surface: Size2D<u32>,
+) -> Option<Rect<u32>> {
+    if origin.x < 0 {
+        size.width = size.width.saturating_sub(-origin.x as u32);
+        origin.x = 0;
+    }
+    if origin.y < 0 {
+        size.height = size.height.saturating_sub(-origin.y as u32);
+        origin.y = 0;
+    }
+    Rect::new(origin.to_u32(), size)
+        .intersection(&Rect::from_size(surface))
+        .filter(|rect| !rect.is_empty())
+}

components/script/Cargo.toml

@@ -74,6 +74,7 @@ num-traits = "0.2"
 offscreen_gl_context = {version = "0.21", features = ["serde"]}
 parking_lot = "0.6"
 phf = "0.7.18"
+pixels = {path = "../pixels"}
 profile_traits = {path = "../profile_traits"}
 ref_filter_map = "1.0.1"
 ref_slice = "1.0"

components/script/dom/canvasrenderingcontext2d.rs

@@ -5,7 +5,7 @@
 use canvas_traits::canvas::{Canvas2dMsg, CanvasMsg, CanvasId};
 use canvas_traits::canvas::{CompositionOrBlending, FillOrStrokeStyle, FillRule};
 use canvas_traits::canvas::{LineCapStyle, LineJoinStyle, LinearGradientStyle};
-use canvas_traits::canvas::{RadialGradientStyle, RepetitionStyle, byte_swap_and_premultiply};
+use canvas_traits::canvas::{RadialGradientStyle, RepetitionStyle};
 use cssparser::{Parser, ParserInput, RGBA};
 use cssparser::Color as CSSColor;
 use dom::bindings::cell::DomRefCell;
@@ -40,11 +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;
@@ -410,7 +410,7 @@ impl CanvasRenderingContext2D {
             Some((mut data, size)) => {
                 // Pixels come from cache in BGRA order and drawImage expects RGBA so we
                 // have to swap the color values
-                byte_swap_and_premultiply(&mut data);
+                pixels::byte_swap_and_premultiply_inplace(&mut data);
                 let size = Size2D::new(size.width as f64, size.height as f64);
                 (data, size)
             },
@@ -575,6 +575,28 @@ impl CanvasRenderingContext2D {
     fn set_origin_unclean(&self) {
         self.origin_clean.set(false)
     }
+
+    pub fn get_rect(&self, rect: Rect<u32>) -> Vec<u8> {
+        assert!(self.origin_is_clean());
+
+        // 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());
+        assert!(Rect::from_size(canvas_size).contains_rect(&rect));
+
+        let (sender, receiver) = ipc::bytes_channel().unwrap();
+        self.send_canvas_2d_msg(Canvas2dMsg::GetImageData(rect, canvas_size, sender));
+        let mut pixels = receiver.recv().unwrap().to_vec();
+
+        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];
+        }
+
+        pixels
+    }
 }
 
 pub trait LayoutCanvasRenderingContext2DHelpers {
@@ -1117,14 +1139,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>> {
+    fn CreateImageData(&self, sw: i32, sh: i32) -> Fallible<DomRoot<ImageData>> {
-        if *sw == 0.0 || *sh == 0.0 {
+        if sw == 0 || sh == 0 {
             return Err(Error::IndexSize);
         }
-
+        ImageData::new(&self.global(), sw.abs() as u32, sh.abs() as u32, None)
-        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)
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-createimagedata
@@ -1133,61 +1152,31 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-getimagedata
-    fn GetImageData(
+    fn GetImageData(&self, sx: i32, sy: i32, sw: i32, sh: i32) -> Fallible<DomRoot<ImageData>> {
-        &self,
+        // FIXME(nox): There are many arithmetic operations here that can
-        sx: Finite<f64>,
+        // overflow or underflow, this should probably be audited.
-        sy: Finite<f64>,
+
-        sw: Finite<f64>,
+        if sw == 0 || sh == 0 {
-        sh: Finite<f64>,
+            return Err(Error::IndexSize);
-    ) -> Fallible<DomRoot<ImageData>> {
+        }
+
         if !self.origin_is_clean() {
             return Err(Error::Security);
         }
 
-        let mut sx = *sx;
+        let (origin, size) = adjust_size_sign(Point2D::new(sx, sy), Size2D::new(sw, sh));
-        let mut sy = *sy;
+        // FIXME(nox): This is probably wrong when this is a context for an
-        let mut sw = *sw;
+        // offscreen canvas.
-        let mut sh = *sh;
+        let canvas_size = self.canvas.as_ref().map_or(Size2D::zero(), |c| c.get_size());
+        let read_rect = match pixels::clip(origin, size, canvas_size) {
+            Some(rect) => rect,
+            None => {
+                // All the pixels are outside the canvas surface.
+                return ImageData::new(&self.global(), size.width, size.height, None);
+            },
+        };
 
-        if sw == 0.0 || sh == 0.0 {
+        ImageData::new(&self.global(), size.width, size.height, Some(self.get_rect(read_rect)))
-            return Err(Error::IndexSize);
-        }
-
-        if sw < 0.0 {
-            sw = -sw;
-            sx -= sw;
-        }
-        if sh < 0.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 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];
-        }
-
-        ImageData::new(&self.global(), sw, sh, Some(data.to_vec()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-putimagedata
@@ -1196,21 +1185,23 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-context-2d-putimagedata
+    #[allow(unsafe_code)]
     fn PutImageData_(
         &self,
         imagedata: &ImageData,
         dx: i32,
         dy: i32,
-        mut dirty_x: i32,
+        dirty_x: i32,
-        mut dirty_y: i32,
+        dirty_y: i32,
-        mut dirty_width: i32,
+        dirty_width: i32,
-        mut dirty_height: i32,
+        dirty_height: i32,
     ) {
         // 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 {
+        let imagedata_size = Size2D::new(imagedata.Width(), imagedata.Height());
+        if imagedata_size.area() == 0 {
             return;
         }
 
@@ -1220,76 +1211,37 @@ impl CanvasRenderingContext2DMethods for CanvasRenderingContext2D {
         // Step 2.
         // TODO: throw InvalidState if buffer is detached.
 
-        // Step 3.
+        // FIXME(nox): This is probably wrong when this is a context for an
-        if dirty_width < 0 {
+        // offscreen canvas.
-            dirty_x += dirty_width;
+        let canvas_size = self.canvas.as_ref().map_or(Size2D::zero(), |c| c.get_size());
-            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
+        // Steps 3-6.
-        // canvas surface.
+        let (src_origin, src_size) = adjust_size_sign(
-        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(
-            buffer.into(),
-            origin.to_vector(),
-            imagedata_size,
-            Rect::new(
             Point2D::new(dirty_x, dirty_y),
             Size2D::new(dirty_width, dirty_height),
-            ),
+        );
-        ));
+        let src_rect = match pixels::clip(src_origin, src_size, imagedata_size) {
+            Some(rect) => rect,
+            None => return,
+        };
+        let (dst_origin, _) = adjust_size_sign(
+            Point2D::new(dirty_x.saturating_add(dx), dirty_y.saturating_add(dy)),
+            Size2D::new(dirty_width, dirty_height),
+        );
+        // By clipping to the canvas surface, we avoid sending any pixel
+        // that would fall outside it.
+        let dst_rect = match pixels::clip(dst_origin, src_rect.size, canvas_size) {
+            Some(rect) => rect,
+            None => return,
+        };
+
+        // Step 7.
+        let (sender, receiver) = ipc::bytes_channel().unwrap();
+        let pixels = unsafe {
+            &imagedata.get_rect(Rect::new(src_rect.origin, dst_rect.size))
+        };
+        self.send_canvas_2d_msg(Canvas2dMsg::PutImageData(dst_rect, receiver));
+        sender.send(pixels).unwrap();
         self.mark_as_dirty();
     }
 
@@ -1539,7 +1491,7 @@ fn is_rect_valid(rect: Rect<f64>) -> bool {
     rect.size.width > 0.0 && rect.size.height > 0.0
 }
 
-// https://html.spec.whatwg.org/multipage/#serialisation-of-a-colour
+// https://html.spec.whatwg.org/multipage/#serialisation-of-a-color
 fn serialize<W>(color: &RGBA, dest: &mut W) -> fmt::Result
 where
     W: fmt::Write,
@@ -1570,3 +1522,18 @@ where
         )
     }
 }
+
+fn adjust_size_sign(
+    mut origin: Point2D<i32>,
+    mut size: Size2D<i32>,
+) -> (Point2D<i32>, Size2D<u32>) {
+    if size.width < 0 {
+        size.width = -size.width;
+        origin.x = origin.x.saturating_sub(size.width);
+    }
+    if size.height < 0 {
+        size.height = -size.height;
+        origin.y = origin.y.saturating_sub(size.height);
+    }
+    (origin, size.to_u32())
+}

components/script/dom/htmlcanvaselement.rs

@@ -7,14 +7,12 @@ use canvas_traits::canvas::{CanvasMsg, CanvasId, FromScriptMsg};
 use canvas_traits::webgl::WebGLVersion;
 use dom::attr::Attr;
 use dom::bindings::cell::DomRefCell;
-use dom::bindings::codegen::Bindings::CanvasRenderingContext2DBinding::CanvasRenderingContext2DMethods;
 use dom::bindings::codegen::Bindings::HTMLCanvasElementBinding;
 use dom::bindings::codegen::Bindings::HTMLCanvasElementBinding::{HTMLCanvasElementMethods, RenderingContext};
 use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLContextAttributes;
 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};
@@ -28,7 +26,7 @@ use dom::virtualmethods::VirtualMethods;
 use dom::webgl2renderingcontext::WebGL2RenderingContext;
 use dom::webglrenderingcontext::{LayoutCanvasWebGLRenderingContextHelpers, WebGLRenderingContext};
 use dom_struct::dom_struct;
-use euclid::Size2D;
+use euclid::{Rect, Size2D};
 use html5ever::{LocalName, Prefix};
 use image::ColorType;
 use image::png::PNGEncoder;
@@ -355,11 +353,9 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement {
         _quality: HandleValue,
     ) -> Fallible<USVString> {
         // Step 1.
-        if let Some(CanvasContext::Context2d(ref context)) = *self.context.borrow() {
+        if !self.origin_is_clean() {
-            if !context.origin_is_clean() {
             return Err(Error::Security);
         }
-        }
 
         // Step 2.
         if self.Width() == 0 || self.Height() == 0 {
@@ -367,24 +363,18 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement {
         }
 
         // Step 3.
-        let raw_data = match *self.context.borrow() {
+        let file = match *self.context.borrow() {
             Some(CanvasContext::Context2d(ref context)) => {
-                let image_data = context.GetImageData(
+                context.get_rect(Rect::from_size(self.get_size()))
-                    Finite::wrap(0f64),
-                    Finite::wrap(0f64),
-                    Finite::wrap(self.Width() as f64),
-                    Finite::wrap(self.Height() as f64),
-                )?;
-                image_data.get_data_array()
             },
             Some(CanvasContext::WebGL(ref context)) => {
-                match context.get_image_data(self.Width(), self.Height()) {
+                match context.get_image_data(self.get_size()) {
                     Some(data) => data,
                     None => return Ok(USVString("data:,".into())),
                 }
             },
             Some(CanvasContext::WebGL2(ref context)) => {
-                match context.base_context().get_image_data(self.Width(), self.Height()) {
+                match context.base_context().get_image_data(self.get_size()) {
                     Some(data) => data,
                     None => return Ok(USVString("data:,".into())),
                 }
@@ -397,8 +387,10 @@ impl HTMLCanvasElementMethods for HTMLCanvasElement {
 
         // FIXME: Only handle image/png for now.
         let mut png = Vec::new();
+        // FIXME(nox): https://github.com/PistonDevelopers/image-png/issues/86
+        // FIXME(nox): https://github.com/PistonDevelopers/image-png/issues/87
         PNGEncoder::new(&mut png)
-            .encode(&raw_data, self.Width(), self.Height(), ColorType::RGBA(8))
+            .encode(&file, self.Width(), self.Height(), ColorType::RGBA(8))
             .unwrap();
         let mut url = "data:image/png;base64,".to_owned();
         // FIXME(nox): Should this use base64::URL_SAFE?

components/script/dom/imagedata.rs

@@ -9,10 +9,12 @@ use dom::bindings::reflector::{Reflector, reflect_dom_object};
 use dom::bindings::root::DomRoot;
 use dom::globalscope::GlobalScope;
 use dom_struct::dom_struct;
-use euclid::Size2D;
+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;
 use std::ptr::NonNull;
@@ -137,16 +139,31 @@ 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());
         let cx = Runtime::get();
         assert!(!cx.is_null());
         typedarray!(in(cx) let array: Uint8ClampedArray = self.data.get());
-            let vec = array.unwrap().as_slice().to_vec();
+        let array = array.as_ref().unwrap();
-            vec
+        // NOTE(nox): This is just as unsafe as `as_slice` itself even though we
+        // 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 to_vec(&self) -> Vec<u8> {
+        unsafe { self.as_slice().into() }
+    }
+
+    #[allow(unsafe_code)]
+    pub unsafe fn get_rect(&self, rect: Rect<u32>) -> Cow<[u8]> {
+        pixels::get_rect(self.as_slice(), self.get_size(), rect)
     }
 
     pub fn get_size(&self) -> Size2D<u32> {

components/script/dom/webglrenderingcontext.rs

@@ -5,7 +5,6 @@
 #[cfg(feature = "webgl_backtrace")]
 use backtrace::Backtrace;
 use byteorder::{ByteOrder, NativeEndian, WriteBytesExt};
-use canvas_traits::canvas::{byte_swap, multiply_u8_pixel};
 use canvas_traits::webgl::{DOMToTextureCommand, Parameter, WebGLCommandBacktrace};
 use canvas_traits::webgl::{TexParameter, WebGLCommand, WebGLContextShareMode, WebGLError};
 use canvas_traits::webgl::{WebGLFramebufferBindingRequest, WebGLMsg, WebGLMsgSender};
@@ -53,7 +52,7 @@ use dom::webgluniformlocation::WebGLUniformLocation;
 use dom::webglvertexarrayobjectoes::WebGLVertexArrayObjectOES;
 use dom::window::Window;
 use dom_struct::dom_struct;
-use euclid::Size2D;
+use euclid::{Point2D, Rect, Size2D};
 use half::f16;
 use ipc_channel::ipc;
 use js::jsapi::{JSContext, JSObject, Type};
@@ -65,6 +64,7 @@ use js::typedarray::{TypedArray, TypedArrayElementCreator};
 use net_traits::image::base::PixelFormat;
 use net_traits::image_cache::ImageResponse;
 use offscreen_gl_context::{GLContextAttributes, GLLimits};
+use pixels;
 use script_layout_interface::HTMLCanvasDataSource;
 use serde::{Deserialize, Serialize};
 use servo_config::prefs::PREFS;
@@ -524,7 +524,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);
@@ -554,7 +554,7 @@ impl WebGLRenderingContext {
                     _ => unimplemented!(),
                 };
 
-                byte_swap(&mut data);
+                pixels::byte_swap_colors_inplace(&mut data);
 
                 (data, size, false)
             },
@@ -567,7 +567,7 @@ impl WebGLRenderingContext {
                 }
                 if let Some((mut data, size)) = canvas.fetch_all_data() {
                     // Pixels got from Canvas have already alpha premultiplied
-                    byte_swap(&mut data);
+                    pixels::byte_swap_colors_inplace(&mut data);
                     (data, size, true)
                 } else {
                     return Ok(None);
@@ -683,15 +683,11 @@ impl WebGLRenderingContext {
 
         match (format, data_type) {
             (TexFormat::RGBA, TexDataType::UnsignedByte) => {
-                for rgba in pixels.chunks_mut(4) {
+                pixels::premultiply_inplace(pixels);
-                    rgba[0] = multiply_u8_pixel(rgba[0], rgba[3]);
-                    rgba[1] = multiply_u8_pixel(rgba[1], rgba[3]);
-                    rgba[2] = multiply_u8_pixel(rgba[2], rgba[3]);
-                }
             },
             (TexFormat::LuminanceAlpha, TexDataType::UnsignedByte) => {
                 for la in pixels.chunks_mut(2) {
-                    la[0] = multiply_u8_pixel(la[0], la[1]);
+                    la[0] = pixels::multiply_u8_color(la[0], la[1]);
                 }
             },
             (TexFormat::RGBA, TexDataType::UnsignedShort5551) => {
@@ -711,9 +707,9 @@ impl WebGLRenderingContext {
                     let a = extend_to_8_bits(pix & 0x0f);
                     NativeEndian::write_u16(
                         rgba,
-                        ((multiply_u8_pixel(r, a) & 0xf0) as u16) << 8 |
+                        ((pixels::multiply_u8_color(r, a) & 0xf0) as u16) << 8 |
-                            ((multiply_u8_pixel(g, a) & 0xf0) as u16) << 4 |
+                            ((pixels::multiply_u8_color(g, a) & 0xf0) as u16) << 4 |
-                            ((multiply_u8_pixel(b, a) & 0xf0) as u16) |
+                            ((pixels::multiply_u8_color(b, a) & 0xf0) as u16) |
                             ((a & 0x0f) as u16),
                     );
                 }
@@ -1078,7 +1074,7 @@ impl WebGLRenderingContext {
     // can fail and that it is UB what happens in that case.
     //
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#2.2
-    pub fn get_image_data(&self, width: u32, height: u32) -> Option<Vec<u8>> {
+    pub fn get_image_data(&self, mut size: Size2D<u32>) -> Option<Vec<u8>> {
         handle_potential_webgl_error!(self, self.validate_framebuffer(), return None);
 
         let (fb_width, fb_height) = handle_potential_webgl_error!(
@@ -1086,15 +1082,12 @@ impl WebGLRenderingContext {
             self.get_current_framebuffer_size().ok_or(InvalidOperation),
             return None
         );
-        let width = cmp::min(width, fb_width as u32);
+        size.width = cmp::min(size.width, fb_width as u32);
-        let height = cmp::min(height, fb_height as u32);
+        size.height = cmp::min(size.height, fb_height as u32);
 
         let (sender, receiver) = ipc::bytes_channel().unwrap();
         self.send_command(WebGLCommand::ReadPixels(
-            0,
+            Rect::from_size(size),
-            0,
-            width as i32,
-            height as i32,
             constants::RGBA,
             constants::UNSIGNED_BYTE,
             sender,
@@ -2887,45 +2880,29 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             return self.webgl_error(InvalidOperation);
         }
 
-        let mut src_x = x;
+        let src_origin = Point2D::new(x, y);
-        let mut src_y = y;
+        let src_size = Size2D::new(width as u32, height as u32);
-        let mut src_width = width;
+        let fb_size = Size2D::new(fb_width as u32, fb_height as u32);
-        let mut src_height = height;
+        let src_rect = match pixels::clip(src_origin, src_size, fb_size) {
+            Some(rect) => rect,
+            None => return,
+        };
+
         let mut dest_offset = 0;
-
+        if x < 0 {
-        if src_x < 0 {
+            dest_offset += -x * bytes_per_pixel;
-            if src_width <= -src_x {
-                return;
         }
-            dest_offset += bytes_per_pixel * -src_x;
+        if y < 0 {
-            src_width += src_x;
+            dest_offset += -y * row_len;
-            src_x = 0;
-        }
-        if src_y < 0 {
-            if src_height <= -src_y {
-                return;
-            }
-            dest_offset += row_len * -src_y;
-            src_height += src_y;
-            src_y = 0;
-        }
-
-        if src_x + src_width > fb_width {
-            src_width = fb_width - src_x;
-        }
-        if src_y + src_height > fb_height {
-            src_height = fb_height - src_y;
         }
 
         let (sender, receiver) = ipc::bytes_channel().unwrap();
-        self.send_command(WebGLCommand::ReadPixels(
+        self.send_command(WebGLCommand::ReadPixels(src_rect, format, pixel_type, sender));
-            src_x, src_y, src_width, src_height, format, pixel_type, sender,
-        ));
-
         let src = receiver.recv().unwrap();
-        let src_row_len = (src_width * bytes_per_pixel) as usize;
+
-        for i in 0..src_height {
+        let src_row_len = src_rect.size.width as usize * bytes_per_pixel as usize;
-            let dest_start = (dest_offset + i * dest_stride) as usize;
+        for i in 0..src_rect.size.height {
+            let dest_start = dest_offset as usize + i as usize * dest_stride as usize;
             let dest_end = dest_start + src_row_len;
             let src_start = i as usize * src_row_len;
             let src_end = src_start + src_row_len;

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,

components/script/lib.rs

@@ -77,6 +77,7 @@ extern crate num_traits;
 extern crate offscreen_gl_context;
 extern crate parking_lot;
 extern crate phf;
+extern crate pixels;
 #[macro_use]
 extern crate profile_traits;
 extern crate ref_filter_map;

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
+

tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.create2.zero.html.ini

@@ -1,5 +0,0 @@
-[2d.imageData.create2.zero.html]
-  type: testharness
-  [createImageData(sw, sh) throws INDEX_SIZE_ERR if size is zero]
-    expected: FAIL
-

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
+

tests/wpt/metadata/2dcontext/pixel-manipulation/2d.imageData.get.zero.html.ini

@@ -1,5 +0,0 @@
-[2d.imageData.get.zero.html]
-  type: testharness
-  [getImageData() throws INDEX_SIZE_ERR if size is zero]
-    expected: FAIL
-