From 9f924013bcbb840914bb6323d69d6b17addc9777 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 7 Sep 2018 11:10:44 +0200 Subject: [PATCH] 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