From cfca906ee2325fad74896b7647db26625099cf66 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 19 Nov 2018 11:17:00 +0100 Subject: [PATCH] Call rgba8_byte_swap_colors_inplace on the WebGL thread --- Cargo.lock | 1 + components/canvas/webgl_thread.rs | 33 ++++++--- components/canvas_traits/Cargo.toml | 1 + components/canvas_traits/webgl.rs | 11 +-- components/net/image_cache.rs | 2 +- components/pixels/lib.rs | 2 + .../script/dom/canvasrenderingcontext2d.rs | 4 +- .../script/dom/webglrenderingcontext.rs | 73 +++++++------------ 8 files changed, 57 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ebacff050df..940caabd9c4 100644 --- a/Cargo.lock +++ b/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", diff --git a/components/canvas/webgl_thread.rs b/components/canvas/webgl_thread.rs index a9d8275417d..9aef15d26fd 100644 --- a/components/canvas/webgl_thread.rs +++ b/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, y_axis_treatment: YAxisTreatment, - tex_source: TexSource, + pixel_format: Option, mut pixels: Vec, ) -> Vec { 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 { - pixels = rgba8_image_to_tex_image_data(internal_format, data_type, pixels); + if let Some(pixel_format) = pixel_format { + 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, @@ -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) => { diff --git a/components/canvas_traits/Cargo.toml b/components/canvas_traits/Cargo.toml index 52dc95170f7..59a11368c5b 100644 --- a/components/canvas_traits/Cargo.toml +++ b/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"} diff --git a/components/canvas_traits/webgl.rs b/components/canvas_traits/webgl.rs index c628fd403e7..7e0efd67bcd 100644 --- a/components/canvas_traits/webgl.rs +++ b/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, y_axis_treatment: YAxisTreatment, - tex_source: TexSource, + pixel_format: Option, receiver: IpcBytesReceiver, }, TexSubImage2D { @@ -298,7 +299,7 @@ pub enum WebGLCommand { unpacking_alignment: u32, alpha_treatment: Option, y_axis_treatment: YAxisTreatment, - tex_source: TexSource, + pixel_format: Option, receiver: IpcBytesReceiver, }, DrawingBufferWidth(WebGLSender), @@ -775,9 +776,3 @@ pub enum YAxisTreatment { AsIs, Flipped, } - -#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)] -pub enum TexSource { - FromHtmlElement, - FromArray, -} diff --git a/components/net/image_cache.rs b/components/net/image_cache.rs index 6fd45801c3b..050712a3d01 100644 --- a/components/net/image_cache.rs +++ b/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"); }, }; diff --git a/components/pixels/lib.rs b/components/pixels/lib.rs index 04c81d27f6c..155a53b4b4b 100644 --- a/components/pixels/lib.rs +++ b/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, } diff --git a/components/script/dom/canvasrenderingcontext2d.rs b/components/script/dom/canvasrenderingcontext2d.rs index b9377a654fe..4840966dfb4 100644 --- a/components/script/dom/canvasrenderingcontext2d.rs +++ b/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"), - PixelFormat::RGB8 => panic!("RGB8 color type not supported"), - PixelFormat::KA8 => panic!("KA8 color type not supported"), + pixel_format => unimplemented!("unsupported pixel format ({:?})", pixel_format), }; Some((image_data, image_size)) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index d63fa9b8017..49d3a9d05ff 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/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, - WebGLError, WebGLFramebufferBindingRequest, WebGLMsg, WebGLMsgSender, WebGLProgramId, - WebGLResult, WebGLSLVersion, WebGLSender, WebGLVersion, WebVRCommand, YAxisTreatment, + TexParameter, WebGLCommand, WebGLCommandBacktrace, WebGLContextShareMode, WebGLError, + WebGLFramebufferBindingRequest, WebGLMsg, WebGLMsgSender, WebGLProgramId, WebGLResult, + 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, true), + TexPixels::new(pixels, size, PixelFormat::RGBA8, true), ); false @@ -527,9 +526,12 @@ impl WebGLRenderingContext { fn get_image_pixels(&self, source: TexImageSource) -> Fallible> { Ok(Some(match source { - TexImageSource::ImageData(image_data) => { - TexPixels::new(image_data.to_vec(), image_data.get_size(), false) - }, + TexImageSource::ImageData(image_data) => TexPixels::new( + 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. - 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) + TexPixels::new(img.bytes.to_vec(), size, img.format, 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() { - // Pixels got from Canvas have already alpha premultiplied - pixels::rgba8_byte_swap_colors_inplace(&mut data); - TexPixels::new(data, size, true) + if let Some((data, size)) = canvas.fetch_all_data() { + TexPixels::new(data, size, PixelFormat::BGRA8, 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, - target, - data_type, - format, - level, - border, - 1, - TexSource::FromHtmlElement, - pixels, + &texture, target, data_type, format, level, border, 1, 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, - target, - level, - xoffset, - yoffset, - format, - data_type, - 1, - TexSource::FromHtmlElement, - pixels, + texture, target, level, xoffset, yoffset, format, data_type, 1, pixels, ); Ok(()) } @@ -4194,14 +4165,21 @@ impl TextureUnit { struct TexPixels { data: Vec, size: Size2D, + pixel_format: Option, premultiplied: bool, } impl TexPixels { - fn new(data: Vec, size: Size2D, premultiplied: bool) -> Self { + fn new( + data: Vec, + size: Size2D, + 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, } }