Call rgba8_byte_swap_colors_inplace on the WebGL thread

Cargo.lock

@@ -379,6 +379,7 @@ dependencies = [
  "malloc_size_of 0.0.1",
  "malloc_size_of_derive 0.0.1",
  "offscreen_gl_context 0.21.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "pixels 0.0.1",
  "serde 1.0.80 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_bytes 0.10.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "servo_config 0.0.1",

components/canvas/webgl_thread.rs

@@ -10,7 +10,7 @@ use fnv::FnvHashMap;
 use gleam::gl;
 use half::f16;
 use offscreen_gl_context::{GLContext, GLContextAttributes, GLLimits, NativeGLContextMethods};
-use pixels;
+use pixels::{self, PixelFormat};
 use std::thread;
 
 /// WebGL Threading API entry point that lives in the constellation.
@@ -1056,7 +1056,7 @@ impl WebGLImpl {
                 unpacking_alignment,
                 alpha_treatment,
                 y_axis_treatment,
-                tex_source,
+                pixel_format,
                 ref receiver,
             } => {
                 let pixels = prepare_pixels(
@@ -1066,7 +1066,7 @@ impl WebGLImpl {
                     unpacking_alignment,
                     alpha_treatment,
                     y_axis_treatment,
-                    tex_source,
+                    pixel_format,
                     receiver.recv().unwrap(),
                 );
 
@@ -1096,7 +1096,7 @@ impl WebGLImpl {
                 unpacking_alignment,
                 alpha_treatment,
                 y_axis_treatment,
-                tex_source,
+                pixel_format,
                 ref receiver,
             } => {
                 let pixels = prepare_pixels(
@@ -1106,7 +1106,7 @@ impl WebGLImpl {
                     unpacking_alignment,
                     alpha_treatment,
                     y_axis_treatment,
-                    tex_source,
+                    pixel_format,
                     receiver.recv().unwrap(),
                 );
 
@@ -1742,26 +1742,30 @@ fn prepare_pixels(
     unpacking_alignment: u32,
     alpha_treatment: Option<AlphaTreatment>,
     y_axis_treatment: YAxisTreatment,
-    tex_source: TexSource,
+    pixel_format: Option<PixelFormat>,
     mut pixels: Vec<u8>,
 ) -> Vec<u8> {
     match alpha_treatment {
         Some(AlphaTreatment::Premultiply) => {
-            if tex_source == TexSource::FromHtmlElement {
+            if let Some(pixel_format) = pixel_format {
+                match pixel_format {
+                    PixelFormat::BGRA8 | PixelFormat::RGBA8 => {},
+                    _ => unimplemented!("unsupported pixel format ({:?})", pixel_format),
+                }
                 premultiply_inplace(TexFormat::RGBA, TexDataType::UnsignedByte, &mut pixels);
             } else {
                 premultiply_inplace(internal_format, data_type, &mut pixels);
             }
         },
         Some(AlphaTreatment::Unmultiply) => {
-            assert_eq!(tex_source, TexSource::FromHtmlElement);
+            assert!(pixel_format.is_some());
             unmultiply_inplace(&mut pixels);
         },
         None => {},
     }
 
-    if tex_source == TexSource::FromHtmlElement {
+    if let Some(pixel_format) = pixel_format {
-        pixels = rgba8_image_to_tex_image_data(internal_format, data_type, pixels);
+        pixels = image_to_tex_image_data(pixel_format, internal_format, data_type, pixels);
     }
 
     if y_axis_treatment == YAxisTreatment::Flipped {
@@ -1781,7 +1785,8 @@ fn prepare_pixels(
 
 /// Translates an image in rgba8 (red in the first byte) format to
 /// the format that was requested of TexImage.
-fn rgba8_image_to_tex_image_data(
+fn image_to_tex_image_data(
+    pixel_format: PixelFormat,
     format: TexFormat,
     data_type: TexDataType,
     mut pixels: Vec<u8>,
@@ -1789,6 +1794,12 @@ fn rgba8_image_to_tex_image_data(
     // hint for vector allocation sizing.
     let pixel_count = pixels.len() / 4;
 
+    match pixel_format {
+        PixelFormat::BGRA8 => pixels::rgba8_byte_swap_colors_inplace(&mut pixels),
+        PixelFormat::RGBA8 => {},
+        _ => unimplemented!("unsupported pixel format ({:?})", pixel_format),
+    }
+
     match (format, data_type) {
         (TexFormat::RGBA, TexDataType::UnsignedByte) => pixels,
         (TexFormat::RGB, TexDataType::UnsignedByte) => {

components/canvas_traits/Cargo.toml

@@ -22,6 +22,7 @@ lazy_static = "1"
 malloc_size_of = { path = "../malloc_size_of" }
 malloc_size_of_derive = { path = "../malloc_size_of_derive" }
 offscreen_gl_context = {version = "0.21", features = ["serde"]}
+pixels = {path = "../pixels"}
 serde = "1.0"
 serde_bytes = "0.10"
 servo_config = {path = "../config"}

components/canvas_traits/webgl.rs

@@ -6,6 +6,7 @@ use euclid::{Rect, Size2D};
 use gleam::gl;
 use ipc_channel::ipc::{IpcBytesReceiver, IpcBytesSender};
 use offscreen_gl_context::{GLContextAttributes, GLLimits};
+use pixels::PixelFormat;
 use serde_bytes::ByteBuf;
 use std::borrow::Cow;
 use std::num::NonZeroU32;
@@ -282,7 +283,7 @@ pub enum WebGLCommand {
         unpacking_alignment: u32,
         alpha_treatment: Option<AlphaTreatment>,
         y_axis_treatment: YAxisTreatment,
-        tex_source: TexSource,
+        pixel_format: Option<PixelFormat>,
         receiver: IpcBytesReceiver,
     },
     TexSubImage2D {
@@ -298,7 +299,7 @@ pub enum WebGLCommand {
         unpacking_alignment: u32,
         alpha_treatment: Option<AlphaTreatment>,
         y_axis_treatment: YAxisTreatment,
-        tex_source: TexSource,
+        pixel_format: Option<PixelFormat>,
         receiver: IpcBytesReceiver,
     },
     DrawingBufferWidth(WebGLSender<i32>),
@@ -775,9 +776,3 @@ pub enum YAxisTreatment {
     AsIs,
     Flipped,
 }
-
-#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)]
-pub enum TexSource {
-    FromHtmlElement,
-    FromArray,
-}

components/net/image_cache.rs

@@ -67,7 +67,7 @@ fn set_webrender_image_key(webrender_api: &webrender_api::RenderApi, image: &mut
 
             true
         },
-        PixelFormat::K8 | PixelFormat::KA8 => {
+        PixelFormat::K8 | PixelFormat::KA8 | PixelFormat::RGBA8 => {
             panic!("Not support by webrender yet");
         },
     };

components/pixels/lib.rs

@@ -18,6 +18,8 @@ pub enum PixelFormat {
     /// RGB, 8 bits per channel
     RGB8,
     /// RGB + alpha, 8 bits per channel
+    RGBA8,
+    /// BGR + alpha, 8 bits per channel
     BGRA8,
 }
 

components/script/dom/canvasrenderingcontext2d.rs

@@ -444,9 +444,7 @@ impl CanvasRenderingContext2D {
         let image_size = Size2D::new(img.width, img.height);
         let image_data = match img.format {
             PixelFormat::BGRA8 => img.bytes.to_vec(),
-            PixelFormat::K8 => panic!("K8 color type not supported"),
+            pixel_format => unimplemented!("unsupported pixel format ({:?})", pixel_format),
-            PixelFormat::RGB8 => panic!("RGB8 color type not supported"),
-            PixelFormat::KA8 => panic!("KA8 color type not supported"),
         };
 
         Some((image_data, image_size))

components/script/dom/webglrenderingcontext.rs

@@ -7,9 +7,9 @@ use backtrace::Backtrace;
 use canvas_traits::webgl::WebGLError::*;
 use canvas_traits::webgl::{
     webgl_channel, AlphaTreatment, DOMToTextureCommand, Parameter, TexDataType, TexFormat,
-    TexParameter, TexSource, WebGLCommand, WebGLCommandBacktrace, WebGLContextShareMode,
+    TexParameter, WebGLCommand, WebGLCommandBacktrace, WebGLContextShareMode, WebGLError,
-    WebGLError, WebGLFramebufferBindingRequest, WebGLMsg, WebGLMsgSender, WebGLProgramId,
+    WebGLFramebufferBindingRequest, WebGLMsg, WebGLMsgSender, WebGLProgramId, WebGLResult,
-    WebGLResult, WebGLSLVersion, WebGLSender, WebGLVersion, WebVRCommand, YAxisTreatment,
+    WebGLSLVersion, WebGLSender, WebGLVersion, WebVRCommand, YAxisTreatment,
 };
 use crate::dom::bindings::codegen::Bindings::ANGLEInstancedArraysBinding::ANGLEInstancedArraysConstants;
 use crate::dom::bindings::codegen::Bindings::EXTBlendMinmaxBinding::EXTBlendMinmaxConstants;
@@ -504,8 +504,7 @@ impl WebGLRenderingContext {
             level,
             0,
             1,
-            TexSource::FromHtmlElement,
+            TexPixels::new(pixels, size, PixelFormat::RGBA8, true),
-            TexPixels::new(pixels, size, true),
         );
 
         false
@@ -527,9 +526,12 @@ impl WebGLRenderingContext {
 
     fn get_image_pixels(&self, source: TexImageSource) -> Fallible<Option<TexPixels>> {
         Ok(Some(match source {
-            TexImageSource::ImageData(image_data) => {
+            TexImageSource::ImageData(image_data) => TexPixels::new(
-                TexPixels::new(image_data.to_vec(), image_data.get_size(), false)
+                image_data.to_vec(),
-            },
+                image_data.get_size(),
+                PixelFormat::RGBA8,
+                false,
+            ),
             TexImageSource::HTMLImageElement(image) => {
                 let document = document_from_node(&*self.canvas);
                 if !image.same_origin(document.origin()) {
@@ -552,15 +554,7 @@ impl WebGLRenderingContext {
 
                 let size = Size2D::new(img.width, img.height);
 
-                // For now Servo's images are all stored as BGRA8 internally.
+                TexPixels::new(img.bytes.to_vec(), size, img.format, false)
-                let mut data = match img.format {
-                    PixelFormat::BGRA8 => img.bytes.to_vec(),
-                    _ => unimplemented!(),
-                };
-
-                pixels::rgba8_byte_swap_colors_inplace(&mut data);
-
-                TexPixels::new(data, size, false)
             },
             // TODO(emilio): Getting canvas data is implemented in CanvasRenderingContext2D,
             // but we need to refactor it moving it to `HTMLCanvasElement` and support
@@ -569,10 +563,8 @@ impl WebGLRenderingContext {
                 if !canvas.origin_is_clean() {
                     return Err(Error::Security);
                 }
-                if let Some((mut data, size)) = canvas.fetch_all_data() {
+                if let Some((data, size)) = canvas.fetch_all_data() {
-                    // Pixels got from Canvas have already alpha premultiplied
+                    TexPixels::new(data, size, PixelFormat::BGRA8, true)
-                    pixels::rgba8_byte_swap_colors_inplace(&mut data);
-                    TexPixels::new(data, size, true)
                 } else {
                     return Ok(None);
                 }
@@ -646,7 +638,6 @@ impl WebGLRenderingContext {
         level: u32,
         _border: u32,
         unpacking_alignment: u32,
-        tex_source: TexSource,
         pixels: TexPixels,
     ) {
         // TexImage2D depth is always equal to 1.
@@ -698,7 +689,7 @@ impl WebGLRenderingContext {
             unpacking_alignment,
             alpha_treatment,
             y_axis_treatment,
-            tex_source,
+            pixel_format: pixels.pixel_format,
             receiver,
         });
         sender.send(&pixels.data).unwrap();
@@ -718,7 +709,6 @@ impl WebGLRenderingContext {
         format: TexFormat,
         data_type: TexDataType,
         unpacking_alignment: u32,
-        tex_source: TexSource,
         pixels: TexPixels,
     ) {
         // We have already validated level
@@ -776,7 +766,7 @@ impl WebGLRenderingContext {
             unpacking_alignment,
             alpha_treatment,
             y_axis_treatment,
-            tex_source,
+            pixel_format: pixels.pixel_format,
             receiver,
         });
         sender.send(&pixels.data).unwrap();
@@ -3649,7 +3639,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             level,
             border,
             unpacking_alignment,
-            TexSource::FromArray,
             TexPixels::from_array(buff, Size2D::new(width, height)),
         );
 
@@ -3714,15 +3703,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
         }
 
         self.tex_image_2d(
-            &texture,
+            &texture, target, data_type, format, level, border, 1, pixels,
-            target,
-            data_type,
-            format,
-            level,
-            border,
-            1,
-            TexSource::FromHtmlElement,
-            pixels,
         );
         Ok(())
     }
@@ -3854,7 +3835,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             format,
             data_type,
             unpacking_alignment,
-            TexSource::FromArray,
             TexPixels::from_array(buff, Size2D::new(width, height)),
         );
         Ok(())
@@ -3900,16 +3880,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
         };
 
         self.tex_sub_image_2d(
-            texture,
+            texture, target, level, xoffset, yoffset, format, data_type, 1, pixels,
-            target,
-            level,
-            xoffset,
-            yoffset,
-            format,
-            data_type,
-            1,
-            TexSource::FromHtmlElement,
-            pixels,
         );
         Ok(())
     }
@@ -4194,14 +4165,21 @@ impl TextureUnit {
 struct TexPixels {
     data: Vec<u8>,
     size: Size2D<u32>,
+    pixel_format: Option<PixelFormat>,
     premultiplied: bool,
 }
 
 impl TexPixels {
-    fn new(data: Vec<u8>, size: Size2D<u32>, premultiplied: bool) -> Self {
+    fn new(
+        data: Vec<u8>,
+        size: Size2D<u32>,
+        pixel_format: PixelFormat,
+        premultiplied: bool,
+    ) -> Self {
         Self {
             data,
             size,
+            pixel_format: Some(pixel_format),
             premultiplied,
         }
     }
@@ -4210,6 +4188,7 @@ impl TexPixels {
         Self {
             data,
             size,
+            pixel_format: None,
             premultiplied: false,
         }
     }