diff --git a/components/canvas_traits/lib.rs b/components/canvas_traits/lib.rs index b98cef63fd6a..90ff7d0daacb 100644 --- a/components/canvas_traits/lib.rs +++ b/components/canvas_traits/lib.rs @@ -134,7 +134,7 @@ pub enum CanvasWebGLMsg { BlendFuncSeparate(u32, u32, u32, u32), AttachShader(u32, u32), BindAttribLocation(u32, u32, String), - BufferData(u32, Vec, u32), + BufferData(u32, Vec, u32), BufferSubData(u32, isize, Vec), Clear(u32), ClearColor(f32, f32, f32, f32), diff --git a/components/script/dom/webglbuffer.rs b/components/script/dom/webglbuffer.rs index 3932eb56dc8a..cfb9cdc4810b 100644 --- a/components/script/dom/webglbuffer.rs +++ b/components/script/dom/webglbuffer.rs @@ -18,6 +18,7 @@ pub struct WebGLBuffer { id: u32, /// The target to which this buffer was bound the first time target: Cell>, + capacity: Cell, is_deleted: Cell, #[ignore_heap_size_of = "Defined in ipc-channel"] renderer: IpcSender, @@ -29,6 +30,7 @@ impl WebGLBuffer { webgl_object: WebGLObject::new_inherited(), id: id, target: Cell::new(None), + capacity: Cell::new(0), is_deleted: Cell::new(false), renderer: renderer, } @@ -68,6 +70,24 @@ impl WebGLBuffer { Ok(()) } + pub fn buffer_data(&self, target: u32, data: &[u8], usage: u32) -> WebGLResult<()> { + if let Some(previous_target) = self.target.get() { + if target != previous_target { + return Err(WebGLError::InvalidOperation); + } + } + self.capacity.set(data.len()); + self.renderer + .send(CanvasMsg::WebGL(CanvasWebGLMsg::BufferData(target, data.to_vec(), usage))) + .unwrap(); + + Ok(()) + } + + pub fn capacity(&self) -> usize { + self.capacity.get() + } + pub fn delete(&self) { if !self.is_deleted.get() { self.is_deleted.set(true); diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 22e134b1f309..25ec30657ea2 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -80,6 +80,8 @@ pub struct WebGLRenderingContext { texture_unpacking_settings: Cell, bound_texture_2d: MutNullableHeap>, bound_texture_cube_map: MutNullableHeap>, + bound_buffer_array: MutNullableHeap>, + bound_buffer_element_array: MutNullableHeap>, } impl WebGLRenderingContext { @@ -106,6 +108,8 @@ impl WebGLRenderingContext { texture_unpacking_settings: Cell::new(CONVERT_COLORSPACE), bound_texture_2d: MutNullableHeap::new(None), bound_texture_cube_map: MutNullableHeap::new(None), + bound_buffer_array: MutNullableHeap::new(None), + bound_buffer_element_array: MutNullableHeap::new(None), } }) } @@ -177,6 +181,20 @@ impl WebGLRenderingContext { } } + #[allow(unsafe_code)] + fn byte_array_to_slice(&self, data: *mut JSObject) -> Option> { + unsafe { + let mut length = 0; + let mut ptr = ptr::null_mut(); + let buffer_data = JS_GetObjectAsArrayBufferView(data, &mut length, &mut ptr); + if buffer_data.is_null() { + self.webgl_error(InvalidValue); // https://github.com/servo/servo/issues/5014 + return None; + } + Some(slice::from_raw_parts(ptr, length as usize).to_vec()) + } + } + fn vertex_attrib(&self, indx: u32, x: f32, y: f32, z: f32, w: f32) { self.ipc_renderer .send(CanvasMsg::WebGL(CanvasWebGLMsg::VertexAttrib(indx, x, y, z, w))) @@ -358,15 +376,18 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 fn BindBuffer(&self, target: u32, buffer: Option<&WebGLBuffer>) { - match target { - constants::ARRAY_BUFFER | - constants::ELEMENT_ARRAY_BUFFER => (), + let slot = match target { + constants::ARRAY_BUFFER => &self.bound_buffer_array, + constants::ELEMENT_ARRAY_BUFFER => &self.bound_buffer_element_array, _ => return self.webgl_error(InvalidEnum), - } + }; if let Some(buffer) = buffer { - handle_potential_webgl_error!(self, buffer.bind(target)) + match buffer.bind(target) { + Ok(_) => slot.set(Some(buffer)), + Err(e) => return self.webgl_error(e), + } } else { // Unbind the current buffer self.ipc_renderer @@ -431,11 +452,15 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 fn BufferData(&self, _cx: *mut JSContext, target: u32, data: Option<*mut JSObject>, usage: u32) { - match target { - constants::ARRAY_BUFFER | - constants::ELEMENT_ARRAY_BUFFER => (), + let bound_buffer = match target { + constants::ARRAY_BUFFER => self.bound_buffer_array.get(), + constants::ELEMENT_ARRAY_BUFFER => self.bound_buffer_element_array.get(), _ => return self.webgl_error(InvalidEnum), - } + }; + let bound_buffer = match bound_buffer { + Some(bound_buffer) => bound_buffer, + None => return self.webgl_error(InvalidValue), + }; match usage { constants::STREAM_DRAW | constants::STATIC_DRAW | @@ -446,21 +471,23 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { Some(data) => data, None => return self.webgl_error(InvalidValue), }; - if let Some(data_vec) = self.float32_array_to_slice(data) { - self.ipc_renderer - .send(CanvasMsg::WebGL(CanvasWebGLMsg::BufferData(target, data_vec, usage))) - .unwrap() + if let Some(data_vec) = self.byte_array_to_slice(data) { + handle_potential_webgl_error!(self, bound_buffer.buffer_data(target, &data_vec, usage)); } } #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 fn BufferSubData(&self, _cx: *mut JSContext, target: u32, offset: i64, data: Option<*mut JSObject>) { - match target { - constants::ARRAY_BUFFER | - constants::ELEMENT_ARRAY_BUFFER => (), + let bound_buffer = match target { + constants::ARRAY_BUFFER => self.bound_buffer_array.get(), + constants::ELEMENT_ARRAY_BUFFER => self.bound_buffer_element_array.get(), _ => return self.webgl_error(InvalidEnum), - } + }; + let bound_buffer = match bound_buffer { + Some(bound_buffer) => bound_buffer, + None => return self.webgl_error(InvalidOperation), + }; let data = match data { Some(data) => data, None => return self.webgl_error(InvalidValue), @@ -468,20 +495,14 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { if offset < 0 { return self.webgl_error(InvalidValue); } - let data_vec = unsafe { - let mut length = 0; - let mut ptr = ptr::null_mut(); - let buffer_data = JS_GetObjectAsArrayBufferView(data, &mut length, &mut ptr); - if buffer_data.is_null() { - return self.webgl_error(InvalidValue) // https://github.com/servo/servo/issues/5014 + if let Some(data_vec) = self.byte_array_to_slice(data) { + if (offset as usize) + data_vec.len() > bound_buffer.capacity() { + return self.webgl_error(InvalidValue); } - slice::from_raw_parts(ptr, length as usize).to_vec() - }; - // FIXME(simartin) Check that the defined region is inside the allocated one - // https://github.com/servo/servo/issues/8738 - self.ipc_renderer - .send(CanvasMsg::WebGL(CanvasWebGLMsg::BufferSubData(target, offset as isize, data_vec))) - .unwrap() + self.ipc_renderer + .send(CanvasMsg::WebGL(CanvasWebGLMsg::BufferSubData(target, offset as isize, data_vec))) + .unwrap() + } } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 @@ -1061,13 +1082,15 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { format: u32, data_type: u32, source: Option) { - // TODO(ecoal95): Check for bound WebGLTexture, and validate more parameters - match target { - constants::TEXTURE_2D | - constants::TEXTURE_CUBE_MAP => (), - + let texture = match target { + constants::TEXTURE_2D => self.bound_texture_2d.get(), + constants::TEXTURE_CUBE_MAP => self.bound_texture_cube_map.get(), _ => return self.webgl_error(InvalidEnum), + }; + if texture.is_none() { + return self.webgl_error(InvalidOperation); } + // TODO(ecoal95): Validate more parameters let source = match source { Some(s) => s, diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 5c9ded6ec901..8b080816beda 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -5853,6 +5853,12 @@ "url": "/_mozilla/mozilla/webgl/bufferData.html" } ], + "mozilla/webgl/bufferSubData.html": [ + { + "path": "mozilla/webgl/bufferSubData.html", + "url": "/_mozilla/mozilla/webgl/bufferSubData.html" + } + ], "mozilla/webgl/context_creation_error.html": [ { "path": "mozilla/webgl/context_creation_error.html", diff --git a/tests/wpt/mozilla/tests/mozilla/webgl/bufferData.html b/tests/wpt/mozilla/tests/mozilla/webgl/bufferData.html index ca151d6202ac..855b1ae5ff30 100644 --- a/tests/wpt/mozilla/tests/mozilla/webgl/bufferData.html +++ b/tests/wpt/mozilla/tests/mozilla/webgl/bufferData.html @@ -1,6 +1,6 @@ -bufferData and bufferSubData input array type check (issue #6791, #6601) +bufferData input array type check (issue #6791, #6601) + +