From 0579fbe4fad276e11218d90bd935b5705f29f8f6 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 6 Sep 2018 10:41:58 +0200 Subject: [PATCH 1/5] Use WebGLResult for returns of instanced draw methods --- .../script/dom/webgl2renderingcontext.rs | 10 ++- .../ext/angleinstancedarrays.rs | 10 ++- .../script/dom/webglrenderingcontext.rs | 73 +++++++------------ 3 files changed, 43 insertions(+), 50 deletions(-) diff --git a/components/script/dom/webgl2renderingcontext.rs b/components/script/dom/webgl2renderingcontext.rs index 69ef20e4fc2..da50ed556df 100644 --- a/components/script/dom/webgl2renderingcontext.rs +++ b/components/script/dom/webgl2renderingcontext.rs @@ -961,7 +961,10 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext { count: i32, primcount: i32, ) { - self.base.draw_arrays_instanced(mode, first, count, primcount); + handle_potential_webgl_error!( + self.base, + self.base.draw_arrays_instanced(mode, first, count, primcount) + ) } /// https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.9 @@ -973,7 +976,10 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext { offset: i64, primcount: i32, ) { - self.base.draw_elements_instanced(mode, count, type_, offset, primcount); + handle_potential_webgl_error!( + self.base, + self.base.draw_elements_instanced(mode, count, type_, offset, primcount) + ) } /// https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.9 diff --git a/components/script/dom/webgl_extensions/ext/angleinstancedarrays.rs b/components/script/dom/webgl_extensions/ext/angleinstancedarrays.rs index f35abaf15af..7c93c8c6567 100644 --- a/components/script/dom/webgl_extensions/ext/angleinstancedarrays.rs +++ b/components/script/dom/webgl_extensions/ext/angleinstancedarrays.rs @@ -71,7 +71,10 @@ impl ANGLEInstancedArraysMethods for ANGLEInstancedArrays { count: i32, primcount: i32, ) { - self.ctx.draw_arrays_instanced(mode, first, count, primcount); + handle_potential_webgl_error!( + self.ctx, + self.ctx.draw_arrays_instanced(mode, first, count, primcount) + ) } // https://www.khronos.org/registry/webgl/extensions/ANGLE_instanced_arrays/ @@ -83,7 +86,10 @@ impl ANGLEInstancedArraysMethods for ANGLEInstancedArrays { offset: i64, primcount: i32, ) { - self.ctx.draw_elements_instanced(mode, count, type_, offset, primcount); + handle_potential_webgl_error!( + self.ctx, + self.ctx.draw_elements_instanced(mode, count, type_, offset, primcount) + ) } fn VertexAttribDivisorANGLE(&self, index: u32, divisor: u32) { diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 546a0950ea4..aad8148c1b4 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -1076,48 +1076,40 @@ impl WebGLRenderingContext { first: i32, count: i32, primcount: i32, - ) { + ) -> WebGLResult<()> { match mode { constants::POINTS | constants::LINE_STRIP | constants::LINE_LOOP | constants::LINES | constants::TRIANGLE_STRIP | constants::TRIANGLE_FAN | constants::TRIANGLES => {}, _ => { - return self.webgl_error(InvalidEnum); + return Err(InvalidEnum); } } if first < 0 || count < 0 || primcount < 0 { - return self.webgl_error(InvalidValue); + return Err(InvalidValue); } - let current_program = handle_potential_webgl_error!( - self, - self.current_program.get().ok_or(InvalidOperation), - return - ); + let current_program = self.current_program.get().ok_or(InvalidOperation)?; let required_len = if count > 0 { - handle_potential_webgl_error!( - self, - first.checked_add(count).map(|len| len as u32).ok_or(InvalidOperation), - return - ) + first.checked_add(count).map(|len| len as u32).ok_or(InvalidOperation)? } else { 0 }; - handle_potential_webgl_error!( - self, - self.current_vao().validate_for_draw(required_len, primcount as u32, ¤t_program.active_attribs()), - return - ); + self.current_vao().validate_for_draw( + required_len, + primcount as u32, + ¤t_program.active_attribs(), + )?; if !self.validate_framebuffer_complete() { - return; + return Ok(()); } if count == 0 || primcount == 0 { - return; + return Ok(()); } self.send_command(if primcount == 1 { @@ -1126,6 +1118,7 @@ impl WebGLRenderingContext { WebGLCommand::DrawArraysInstanced { mode, first, count, primcount } }); self.mark_as_dirty(); + Ok(()) } // https://www.khronos.org/registry/webgl/extensions/ANGLE_instanced_arrays/ @@ -1136,62 +1129,49 @@ impl WebGLRenderingContext { type_: u32, offset: i64, primcount: i32, - ) { + ) -> WebGLResult<()> { match mode { constants::POINTS | constants::LINE_STRIP | constants::LINE_LOOP | constants::LINES | constants::TRIANGLE_STRIP | constants::TRIANGLE_FAN | constants::TRIANGLES => {}, _ => { - return self.webgl_error(InvalidEnum); + return Err(InvalidEnum); } } if count < 0 || offset < 0 || primcount < 0 { - return self.webgl_error(InvalidValue); + return Err(InvalidValue); } let type_size = match type_ { constants::UNSIGNED_BYTE => 1, constants::UNSIGNED_SHORT => 2, constants::UNSIGNED_INT if self.extension_manager.is_element_index_uint_enabled() => 4, - _ => return self.webgl_error(InvalidEnum), + _ => return Err(InvalidEnum), }; if offset % type_size != 0 { - return self.webgl_error(InvalidOperation); + return Err(InvalidOperation); } - let current_program = handle_potential_webgl_error!( - self, - self.current_program.get().ok_or(InvalidOperation), - return - ); - - let array_buffer = handle_potential_webgl_error!( - self, - self.current_vao().element_array_buffer().get().ok_or(InvalidOperation), - return - ); + let current_program = self.current_program.get().ok_or(InvalidOperation)?; + let array_buffer = self.current_vao().element_array_buffer().get().ok_or(InvalidOperation)?; if count > 0 && primcount > 0 { // This operation cannot overflow in u64 and we know all those values are nonnegative. let val = offset as u64 + (count as u64 * type_size as u64); if val > array_buffer.capacity() as u64 { - return self.webgl_error(InvalidOperation); + return Err(InvalidOperation); } } // TODO(nox): Pass the correct number of vertices required. - handle_potential_webgl_error!( - self, - self.current_vao().validate_for_draw(0, primcount as u32, ¤t_program.active_attribs()), - return - ); + self.current_vao().validate_for_draw(0, primcount as u32, ¤t_program.active_attribs())?; if !self.validate_framebuffer_complete() { - return; + return Ok(()); } if count == 0 || primcount == 0 { - return; + return Ok(()); } let offset = offset as u32; @@ -1201,6 +1181,7 @@ impl WebGLRenderingContext { WebGLCommand::DrawElementsInstanced { mode, count, type_, offset, primcount } }); self.mark_as_dirty(); + Ok(()) } pub fn vertex_attrib_divisor(&self, index: u32, divisor: u32) { @@ -2328,12 +2309,12 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.11 fn DrawArrays(&self, mode: u32, first: i32, count: i32) { - self.draw_arrays_instanced(mode, first, count, 1); + handle_potential_webgl_error!(self, self.draw_arrays_instanced(mode, first, count, 1)); } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.11 fn DrawElements(&self, mode: u32, count: i32, type_: u32, offset: i64) { - self.draw_elements_instanced(mode, count, type_, offset, 1); + handle_potential_webgl_error!(self, self.draw_elements_instanced(mode, count, type_, offset, 1)); } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 From 5a206d51373bb304b80a6469b518cafb35322a93 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 6 Sep 2018 11:25:06 +0200 Subject: [PATCH 2/5] Make validate_framebuffer return a WebGLResult<()> --- .../script/dom/webglrenderingcontext.rs | 55 +++++-------------- 1 file changed, 15 insertions(+), 40 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index aad8148c1b4..647ff441e3e 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -336,17 +336,12 @@ impl WebGLRenderingContext { // // The WebGL spec mentions a couple more operations that trigger // this: clear() and getParameter(IMPLEMENTATION_COLOR_READ_*). - fn validate_framebuffer_complete(&self) -> bool { + fn validate_framebuffer(&self) -> WebGLResult<()> { match self.bound_framebuffer.get() { - Some(fb) => match fb.check_status() { - constants::FRAMEBUFFER_COMPLETE => return true, - _ => { - self.webgl_error(InvalidFramebufferOperation); - return false; - } + Some(ref fb) if fb.check_status() != constants::FRAMEBUFFER_COMPLETE => { + Err(InvalidFramebufferOperation) }, - // The default framebuffer is always complete. - None => return true, + _ => Ok(()), } } @@ -1104,9 +1099,7 @@ impl WebGLRenderingContext { ¤t_program.active_attribs(), )?; - if !self.validate_framebuffer_complete() { - return Ok(()); - } + self.validate_framebuffer()?; if count == 0 || primcount == 0 { return Ok(()); @@ -1166,9 +1159,7 @@ impl WebGLRenderingContext { // TODO(nox): Pass the correct number of vertices required. self.current_vao().validate_for_draw(0, primcount as u32, ¤t_program.active_attribs())?; - if !self.validate_framebuffer_complete() { - return Ok(()); - } + self.validate_framebuffer()?; if count == 0 || primcount == 0 { return Ok(()); @@ -1200,9 +1191,7 @@ impl WebGLRenderingContext { // // https://www.khronos.org/registry/webgl/specs/latest/1.0/#2.2 pub fn get_image_data(&self, mut width: u32, mut height: u32) -> Option> { - if !self.validate_framebuffer_complete() { - return None; - } + handle_potential_webgl_error!(self, self.validate_framebuffer(), return None); if let Some((fb_width, fb_height)) = self.get_current_framebuffer_size() { width = cmp::min(width, fb_width as u32); @@ -1406,18 +1395,12 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // GL_OES_read_format support (assuming an underlying GLES // driver. Desktop is happy to format convert for us). constants::IMPLEMENTATION_COLOR_READ_FORMAT => { - if !self.validate_framebuffer_complete() { - return NullValue(); - } else { - return Int32Value(constants::RGBA as i32); - } + handle_potential_webgl_error!(self, self.validate_framebuffer(), return NullValue()); + return Int32Value(constants::RGBA as i32); } constants::IMPLEMENTATION_COLOR_READ_TYPE => { - if !self.validate_framebuffer_complete() { - return NullValue(); - } else { - return Int32Value(constants::UNSIGNED_BYTE as i32); - } + handle_potential_webgl_error!(self, self.validate_framebuffer(), return NullValue()); + return Int32Value(constants::UNSIGNED_BYTE as i32); } constants::COMPRESSED_TEXTURE_FORMATS => { // FIXME(nox): https://github.com/servo/servo/issues/20594 @@ -1951,9 +1934,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 fn CopyTexImage2D(&self, target: u32, level: i32, internal_format: u32, x: i32, y: i32, width: i32, height: i32, border: i32) { - if !self.validate_framebuffer_complete() { - return; - } + handle_potential_webgl_error!(self, self.validate_framebuffer(), return); let validator = CommonTexImage2DValidator::new(self, target, level, internal_format, width, @@ -2008,9 +1989,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 fn CopyTexSubImage2D(&self, target: u32, level: i32, xoffset: i32, yoffset: i32, x: i32, y: i32, width: i32, height: i32) { - if !self.validate_framebuffer_complete() { - return; - } + handle_potential_webgl_error!(self, self.validate_framebuffer(), return); // NB: We use a dummy (valid) format and border in order to reuse the // common validations, but this should have its own validator. @@ -2051,9 +2030,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.11 fn Clear(&self, mask: u32) { - if !self.validate_framebuffer_complete() { - return; - } + handle_potential_webgl_error!(self, self.validate_framebuffer(), return); if mask & !(constants::DEPTH_BUFFER_BIT | constants::STENCIL_BUFFER_BIT | constants::COLOR_BUFFER_BIT) != 0 { return self.webgl_error(InvalidValue); } @@ -2836,9 +2813,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { Some(ref mut data) => (data.get_array_type(), unsafe { data.as_mut_slice() }), }; - if !self.validate_framebuffer_complete() { - return; - } + handle_potential_webgl_error!(self, self.validate_framebuffer(), return); match array_type { Type::Uint8 => (), From 1293692ef8874f4d810f9a08114bcbfe11d4ac8f Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 7 Sep 2018 10:40:30 +0200 Subject: [PATCH 3/5] Simplify WebGLBuffer::buffer_data There is no need to pass the target to that buffer method, given the buffer has been retrieved by looking up the one bound to that target in the context. --- components/script/dom/webglbuffer.rs | 13 ++----------- .../script/dom/webglrenderingcontext.rs | 19 +++++-------------- 2 files changed, 7 insertions(+), 25 deletions(-) diff --git a/components/script/dom/webglbuffer.rs b/components/script/dom/webglbuffer.rs index d874cc7a73a..138c34130a3 100644 --- a/components/script/dom/webglbuffer.rs +++ b/components/script/dom/webglbuffer.rs @@ -62,10 +62,7 @@ impl WebGLBuffer { self.id } - pub fn buffer_data(&self, target: u32, data: T, usage: u32) -> WebGLResult<()> - where - T: Into>, - { + pub fn buffer_data(&self, data: Vec, usage: u32) -> WebGLResult<()> { match usage { WebGLRenderingContextConstants::STREAM_DRAW | WebGLRenderingContextConstants::STATIC_DRAW | @@ -73,17 +70,11 @@ impl WebGLBuffer { _ => return Err(WebGLError::InvalidEnum), } - if let Some(previous_target) = self.target.get() { - if target != previous_target { - return Err(WebGLError::InvalidOperation); - } - } - let data = data.into(); self.capacity.set(data.len()); self.usage.set(usage); self.upcast::() .context() - .send_command(WebGLCommand::BufferData(target, data.into(), usage)); + .send_command(WebGLCommand::BufferData(self.target.get().unwrap(), data.into(), usage)); Ok(()) } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 647ff441e3e..82bf6d46e91 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -1859,21 +1859,15 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { }; let bound_buffer = handle_potential_webgl_error!(self, self.bound_buffer(target), return); - let bound_buffer = match bound_buffer { - Some(bound_buffer) => bound_buffer, - None => return self.webgl_error(InvalidOperation), - }; + let bound_buffer = handle_potential_webgl_error!(self, bound_buffer.ok_or(InvalidOperation), return); - handle_potential_webgl_error!(self, bound_buffer.buffer_data(target, data, usage)); + handle_potential_webgl_error!(self, bound_buffer.buffer_data(data, usage)); } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 fn BufferData_(&self, target: u32, size: i64, usage: u32) { let bound_buffer = handle_potential_webgl_error!(self, self.bound_buffer(target), return); - let bound_buffer = match bound_buffer { - Some(bound_buffer) => bound_buffer, - None => return self.webgl_error(InvalidOperation), - }; + let bound_buffer = handle_potential_webgl_error!(self, bound_buffer.ok_or(InvalidOperation), return); if size < 0 { return self.webgl_error(InvalidValue); @@ -1882,7 +1876,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // FIXME: Allocating a buffer based on user-requested size is // not great, but we don't have a fallible allocation to try. let data = vec![0u8; size as usize]; - handle_potential_webgl_error!(self, bound_buffer.buffer_data(target, data, usage)); + handle_potential_webgl_error!(self, bound_buffer.buffer_data(data, usage)); } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 @@ -1894,10 +1888,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { }; let bound_buffer = handle_potential_webgl_error!(self, self.bound_buffer(target), return); - let bound_buffer = match bound_buffer { - Some(bound_buffer) => bound_buffer, - None => return self.webgl_error(InvalidOperation), - }; + let bound_buffer = handle_potential_webgl_error!(self, bound_buffer.ok_or(InvalidOperation), return); if offset < 0 { return self.webgl_error(InvalidValue); From e37856a855185c496013bdb0009fb0a807396d25 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 7 Sep 2018 11:10:30 +0200 Subject: [PATCH 4/5] Remove Clone impl for WebGLMsg --- components/canvas_traits/webgl.rs | 4 ++-- components/canvas_traits/webgl_channel/mod.rs | 14 +++++++++++++- components/canvas_traits/webgl_channel/mpsc.rs | 7 ++++++- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/components/canvas_traits/webgl.rs b/components/canvas_traits/webgl.rs index bdffd586287..89fec5596cd 100644 --- a/components/canvas_traits/webgl.rs +++ b/components/canvas_traits/webgl.rs @@ -24,7 +24,7 @@ pub use ::webgl_channel::WebGLPipeline; pub use ::webgl_channel::WebGLChan; /// WebGL Message API -#[derive(Clone, Deserialize, Serialize)] +#[derive(Deserialize, Serialize)] pub enum WebGLMsg { /// Creates a new WebGLContext. CreateContext(WebGLVersion, Size2D, GLContextAttributes, @@ -155,7 +155,7 @@ impl WebGLMsgSender { } /// WebGL Commands for a specific WebGLContext -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize)] pub enum WebGLCommand { GetContextAttributes(WebGLSender), ActiveTexture(u32), diff --git a/components/canvas_traits/webgl_channel/mod.rs b/components/canvas_traits/webgl_channel/mod.rs index f317d116f3d..3c6bd1690b0 100644 --- a/components/canvas_traits/webgl_channel/mod.rs +++ b/components/canvas_traits/webgl_channel/mod.rs @@ -16,12 +16,24 @@ lazy_static! { static ref IS_MULTIPROCESS: bool = { opts::multiprocess() }; } -#[derive(Clone, Deserialize, Serialize)] +#[derive(Deserialize, Serialize)] pub enum WebGLSender { Ipc(ipc::WebGLSender), Mpsc(mpsc::WebGLSender), } +impl Clone for WebGLSender +where + T: Serialize, +{ + fn clone(&self) -> Self { + match *self { + WebGLSender::Ipc(ref chan) => WebGLSender::Ipc(chan.clone()), + WebGLSender::Mpsc(ref chan) => WebGLSender::Mpsc(chan.clone()), + } + } +} + impl fmt::Debug for WebGLSender { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "WebGLSender(..)") diff --git a/components/canvas_traits/webgl_channel/mpsc.rs b/components/canvas_traits/webgl_channel/mpsc.rs index 79988835b6c..92b83791d36 100644 --- a/components/canvas_traits/webgl_channel/mpsc.rs +++ b/components/canvas_traits/webgl_channel/mpsc.rs @@ -26,10 +26,15 @@ macro_rules! unreachable_serializable { }; } -#[derive(Clone)] pub struct WebGLSender(mpsc::Sender); pub struct WebGLReceiver(mpsc::Receiver); +impl Clone for WebGLSender { + fn clone(&self) -> Self { + WebGLSender(self.0.clone()) + } +} + impl WebGLSender { #[inline] pub fn send(&self, data: T) -> Result<(), mpsc::SendError> { From 9f924013bcbb840914bb6323d69d6b17addc9777 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 7 Sep 2018 11:10:44 +0200 Subject: [PATCH 5/5] Use a bytes channel in BufferData This means we don't need to copy the input ArrayBuffer at all on the DOM side. --- components/canvas/webgl_thread.rs | 10 +++-- components/canvas_traits/webgl.rs | 5 ++- components/script/dom/webglbuffer.rs | 7 +++- .../script/dom/webglrenderingcontext.rs | 37 ++++++++++++------- 4 files changed, 37 insertions(+), 22 deletions(-) diff --git a/components/canvas/webgl_thread.rs b/components/canvas/webgl_thread.rs index 7430145e5ee..f97749610aa 100644 --- a/components/canvas/webgl_thread.rs +++ b/components/canvas/webgl_thread.rs @@ -659,10 +659,12 @@ impl WebGLImpl { ctx.gl().blend_func(src, dest), WebGLCommand::BlendFuncSeparate(src_rgb, dest_rgb, src_alpha, dest_alpha) => ctx.gl().blend_func_separate(src_rgb, dest_rgb, src_alpha, dest_alpha), - WebGLCommand::BufferData(buffer_type, ref data, usage) => - gl::buffer_data(ctx.gl(), buffer_type, data, usage), - WebGLCommand::BufferSubData(buffer_type, offset, ref data) => - gl::buffer_sub_data(ctx.gl(), buffer_type, offset, data), + WebGLCommand::BufferData(buffer_type, ref receiver, usage) => { + gl::buffer_data(ctx.gl(), buffer_type, &receiver.recv().unwrap(), usage) + }, + WebGLCommand::BufferSubData(buffer_type, offset, ref receiver) => { + gl::buffer_sub_data(ctx.gl(), buffer_type, offset, &receiver.recv().unwrap()) + }, WebGLCommand::Clear(mask) => ctx.gl().clear(mask), WebGLCommand::ClearColor(r, g, b, a) => diff --git a/components/canvas_traits/webgl.rs b/components/canvas_traits/webgl.rs index 89fec5596cd..f1f8515002c 100644 --- a/components/canvas_traits/webgl.rs +++ b/components/canvas_traits/webgl.rs @@ -4,6 +4,7 @@ use euclid::Size2D; use gleam::gl; +use ipc_channel::ipc::IpcBytesReceiver; use offscreen_gl_context::{GLContextAttributes, GLLimits}; use serde_bytes::ByteBuf; use std::borrow::Cow; @@ -167,8 +168,8 @@ pub enum WebGLCommand { AttachShader(WebGLProgramId, WebGLShaderId), DetachShader(WebGLProgramId, WebGLShaderId), BindAttribLocation(WebGLProgramId, u32, String), - BufferData(u32, ByteBuf, u32), - BufferSubData(u32, isize, ByteBuf), + BufferData(u32, IpcBytesReceiver, u32), + BufferSubData(u32, isize, IpcBytesReceiver), Clear(u32), ClearColor(f32, f32, f32, f32), ClearDepth(f32), diff --git a/components/script/dom/webglbuffer.rs b/components/script/dom/webglbuffer.rs index 138c34130a3..c49a3852709 100644 --- a/components/script/dom/webglbuffer.rs +++ b/components/script/dom/webglbuffer.rs @@ -13,6 +13,7 @@ use dom::bindings::root::DomRoot; use dom::webglobject::WebGLObject; use dom::webglrenderingcontext::WebGLRenderingContext; use dom_struct::dom_struct; +use ipc_channel::ipc; use std::cell::Cell; #[dom_struct] @@ -62,7 +63,7 @@ impl WebGLBuffer { self.id } - pub fn buffer_data(&self, data: Vec, usage: u32) -> WebGLResult<()> { + pub fn buffer_data(&self, data: &[u8], usage: u32) -> WebGLResult<()> { match usage { WebGLRenderingContextConstants::STREAM_DRAW | WebGLRenderingContextConstants::STATIC_DRAW | @@ -72,9 +73,11 @@ impl WebGLBuffer { self.capacity.set(data.len()); self.usage.set(usage); + let (sender, receiver) = ipc::bytes_channel().unwrap(); self.upcast::() .context() - .send_command(WebGLCommand::BufferData(self.target.get().unwrap(), data.into(), usage)); + .send_command(WebGLCommand::BufferData(self.target.get().unwrap(), receiver, usage)); + sender.send(data).unwrap(); Ok(()) } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 82bf6d46e91..6c151486f59 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -52,6 +52,7 @@ use dom::window::Window; use dom_struct::dom_struct; use euclid::Size2D; use half::f16; +use ipc_channel::ipc; use js::jsapi::{JSContext, JSObject, Type}; use js::jsval::{BooleanValue, DoubleValue, Int32Value, UInt32Value, JSVal}; use js::jsval::{ObjectValue, NullValue, UndefinedValue}; @@ -1846,21 +1847,25 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 + #[allow(unsafe_code)] fn BufferData( &self, target: u32, data: Option, usage: u32, ) { - let data = match data { - Some(ArrayBufferViewOrArrayBuffer::ArrayBuffer(data)) => data.to_vec(), - Some(ArrayBufferViewOrArrayBuffer::ArrayBufferView(data)) => data.to_vec(), - None => return self.webgl_error(InvalidValue), - }; + let data = handle_potential_webgl_error!(self, data.ok_or(InvalidValue), return); let bound_buffer = handle_potential_webgl_error!(self, self.bound_buffer(target), return); let bound_buffer = handle_potential_webgl_error!(self, bound_buffer.ok_or(InvalidOperation), return); + let data = unsafe { + // Safe because we don't do anything with JS until the end of the method. + match data { + ArrayBufferViewOrArrayBuffer::ArrayBuffer(ref data) => data.as_slice(), + ArrayBufferViewOrArrayBuffer::ArrayBufferView(ref data) => data.as_slice(), + } + }; handle_potential_webgl_error!(self, bound_buffer.buffer_data(data, usage)); } @@ -1876,17 +1881,12 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // FIXME: Allocating a buffer based on user-requested size is // not great, but we don't have a fallible allocation to try. let data = vec![0u8; size as usize]; - handle_potential_webgl_error!(self, bound_buffer.buffer_data(data, usage)); + handle_potential_webgl_error!(self, bound_buffer.buffer_data(&data, usage)); } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 + #[allow(unsafe_code)] fn BufferSubData(&self, target: u32, offset: i64, data: ArrayBufferViewOrArrayBuffer) { - let data_vec = match data { - // Typed array is rooted, so we can safely temporarily retrieve its slice - ArrayBufferViewOrArrayBuffer::ArrayBuffer(inner) => inner.to_vec(), - ArrayBufferViewOrArrayBuffer::ArrayBufferView(inner) => inner.to_vec(), - }; - let bound_buffer = handle_potential_webgl_error!(self, self.bound_buffer(target), return); let bound_buffer = handle_potential_webgl_error!(self, bound_buffer.ok_or(InvalidOperation), return); @@ -1894,14 +1894,23 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { return self.webgl_error(InvalidValue); } - if (offset as usize) + data_vec.len() > bound_buffer.capacity() { + let data = unsafe { + // Safe because we don't do anything with JS until the end of the method. + match data { + ArrayBufferViewOrArrayBuffer::ArrayBuffer(ref data) => data.as_slice(), + ArrayBufferViewOrArrayBuffer::ArrayBufferView(ref data) => data.as_slice(), + } + }; + if (offset as u64) + data.len() as u64 > bound_buffer.capacity() as u64 { return self.webgl_error(InvalidValue); } + let (sender, receiver) = ipc::bytes_channel().unwrap(); self.send_command(WebGLCommand::BufferSubData( target, offset as isize, - data_vec.into(), + receiver, )); + sender.send(data).unwrap(); } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8