From 6ec2c41df8a07c81ada5d4d1d6f5ec647fb9868b Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Mon, 15 Aug 2016 01:02:36 -0700 Subject: [PATCH 1/3] webgl: Protect against GL error on glBindRenderbuffer(deleted rbo). On a GLES or compatibility underlying GL context, we were fine because an underlying renderbuffer object would just get re-created, and nothing too bad happened because we aren't tracking the currently bound renderbuffer at the DOM level. However, on a desktop GL core context, binding non-genned or deleted names is an error. Fixes a crash in object-deletion-behavior.html. --- .../script/dom/webglrenderingcontext.rs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 059c6941bc1..cd336e02e50 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -705,13 +705,19 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { return self.webgl_error(InvalidEnum); } - if let Some(renderbuffer) = renderbuffer { - renderbuffer.bind(target) - } else { - // Unbind the currently bound renderbuffer - self.ipc_renderer - .send(CanvasMsg::WebGL(WebGLCommand::BindRenderbuffer(target, None))) - .unwrap() + match renderbuffer { + // Implementations differ on what to do in the deleted + // case: Chromium currently unbinds, and Gecko silently + // returns. The conformance tests don't cover this case. + Some(renderbuffer) if !renderbuffer.is_deleted() => { + renderbuffer.bind(target) + } + _ => { + // Unbind the currently bound renderbuffer + self.ipc_renderer + .send(CanvasMsg::WebGL(WebGLCommand::BindRenderbuffer(target, None))) + .unwrap() + } } } From db9fe23903d53193a969df35c5a142def6b6893f Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Sun, 14 Aug 2016 18:35:32 -0700 Subject: [PATCH 2/3] webgl: Remove objects from binding points on object deletion. We keep bindings that shadow what's mapped in the GL state currently, and so we need to remove the objects from our binding points when they get implicitly removed from the GL binding points during object deletion. --- .../script/dom/webglrenderingcontext.rs | 29 +++++++++++++++---- .../misc/object-deletion-behaviour.html.ini | 3 -- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index cd336e02e50..96eb0ae98df 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -61,6 +61,24 @@ macro_rules! handle_potential_webgl_error { }; } +// From the GLES 2.0.25 spec, page 85: +// +// "If a texture that is currently bound to one of the targets +// TEXTURE_2D, or TEXTURE_CUBE_MAP is deleted, it is as though +// BindTexture had been executed with the same target and texture +// zero." +// +// and similar text occurs for other object types. +macro_rules! handle_object_deletion { + ($binding:expr, $object:ident) => { + if let Some(bound_object) = $binding.get() { + if bound_object.id() == $object.id() { + $binding.set(None); + } + } + }; +} + /// Set of bitflags for texture unpacking (texImage2d, etc...) bitflags! { #[derive(HeapSizeOf, JSTraceable)] @@ -1101,6 +1119,8 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5 fn DeleteBuffer(&self, buffer: Option<&WebGLBuffer>) { if let Some(buffer) = buffer { + handle_object_deletion!(self.bound_buffer_array, buffer); + handle_object_deletion!(self.bound_buffer_element_array, buffer); buffer.delete() } } @@ -1108,11 +1128,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6 fn DeleteFramebuffer(&self, framebuffer: Option<&WebGLFramebuffer>) { if let Some(framebuffer) = framebuffer { - if let Some(bound_fb) = self.bound_framebuffer.get() { - if bound_fb.id() == framebuffer.id() { - self.bound_framebuffer.set(None); - } - } + handle_object_deletion!(self.bound_framebuffer, framebuffer); framebuffer.delete() } } @@ -1127,6 +1143,8 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 fn DeleteTexture(&self, texture: Option<&WebGLTexture>) { if let Some(texture) = texture { + handle_object_deletion!(self.bound_texture_2d, texture); + handle_object_deletion!(self.bound_texture_cube_map, texture); texture.delete() } } @@ -1134,6 +1152,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 fn DeleteProgram(&self, program: Option<&WebGLProgram>) { if let Some(program) = program { + handle_object_deletion!(self.current_program, program); program.delete() } } diff --git a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/misc/object-deletion-behaviour.html.ini b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/misc/object-deletion-behaviour.html.ini index 2dc8b16d510..04cdda15902 100644 --- a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/misc/object-deletion-behaviour.html.ini +++ b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/misc/object-deletion-behaviour.html.ini @@ -68,9 +68,6 @@ [WebGL test #48: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindTexture(gl.TEXTURE_2D, t)] expected: FAIL - [WebGL test #52: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_S, gl.CLAMP_TO_EDGE)] - expected: FAIL - [WebGL test #56: gl.getParameter(gl.TEXTURE_BINDING_2D) should be [object WebGLTexture\]. Was null.] expected: FAIL From e8b5f70429b70e779e959f616fc88456a52bc7a4 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 18 Aug 2016 01:18:45 -0700 Subject: [PATCH 3/3] webgl: Add getters for other GL_*_BINDING enums. Fixes many subcases of object-deletion-behaviour.html. --- .../script/dom/webglrenderingcontext.rs | 42 ++++++++++---- .../misc/object-deletion-behaviour.html.ini | 55 +------------------ 2 files changed, 33 insertions(+), 64 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 96eb0ae98df..7814296c38a 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -79,6 +79,20 @@ macro_rules! handle_object_deletion { }; } +macro_rules! object_binding_to_js_or_null { + ($cx: expr, $binding:expr) => { + { + rooted!(in($cx) let mut rval = NullValue()); + if let Some(bound_object) = $binding.get() { + unsafe { + bound_object.to_jsval($cx, rval.handle_mut()); + } + } + rval.get() + } + }; +} + /// Set of bitflags for texture unpacking (texImage2d, etc...) bitflags! { #[derive(HeapSizeOf, JSTraceable)] @@ -101,6 +115,7 @@ pub struct WebGLRenderingContext { last_error: Cell>, texture_unpacking_settings: Cell, bound_framebuffer: MutNullableHeap>, + bound_renderbuffer: MutNullableHeap>, bound_texture_2d: MutNullableHeap>, bound_texture_cube_map: MutNullableHeap>, bound_buffer_array: MutNullableHeap>, @@ -135,6 +150,7 @@ impl WebGLRenderingContext { bound_texture_cube_map: MutNullableHeap::new(None), bound_buffer_array: MutNullableHeap::new(None), bound_buffer_element_array: MutNullableHeap::new(None), + bound_renderbuffer: MutNullableHeap::new(None), current_program: MutNullableHeap::new(None), current_vertex_attrib_0: Cell::new((0f32, 0f32, 0f32, 1f32)), } @@ -531,19 +547,22 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3 fn GetParameter(&self, cx: *mut JSContext, parameter: u32) -> JSVal { - // Handle the GL_FRAMEBUFFER_BINDING without going all the way + // Handle the GL_*_BINDING without going all the way // to the GL, since we would just need to map back from GL's - // returned ID to the WebGLFramebuffer we're tracking. + // returned ID to the WebGL* object we're tracking. match parameter { - constants::FRAMEBUFFER_BINDING => { - rooted!(in(cx) let mut rval = NullValue()); - if let Some(bound_fb) = self.bound_framebuffer.get() { - unsafe { - bound_fb.to_jsval(cx, rval.handle_mut()); - } - } - return rval.get() - } + constants::ARRAY_BUFFER_BINDING => + return object_binding_to_js_or_null!(cx, &self.bound_buffer_array), + constants::ELEMENT_ARRAY_BUFFER_BINDING => + return object_binding_to_js_or_null!(cx, &self.bound_buffer_element_array), + constants::FRAMEBUFFER_BINDING => + return object_binding_to_js_or_null!(cx, &self.bound_framebuffer), + constants::RENDERBUFFER_BINDING => + return object_binding_to_js_or_null!(cx, &self.bound_renderbuffer), + constants::TEXTURE_BINDING_2D => + return object_binding_to_js_or_null!(cx, &self.bound_texture_2d), + constants::TEXTURE_BINDING_CUBE_MAP => + return object_binding_to_js_or_null!(cx, &self.bound_texture_cube_map), _ => {} } @@ -1136,6 +1155,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7 fn DeleteRenderbuffer(&self, renderbuffer: Option<&WebGLRenderbuffer>) { if let Some(renderbuffer) = renderbuffer { + handle_object_deletion!(self.bound_renderbuffer, renderbuffer); renderbuffer.delete() } } diff --git a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/misc/object-deletion-behaviour.html.ini b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/misc/object-deletion-behaviour.html.ini index 04cdda15902..4e4a3963ca9 100644 --- a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/misc/object-deletion-behaviour.html.ini +++ b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/misc/object-deletion-behaviour.html.ini @@ -23,9 +23,6 @@ [WebGL test #21: gl.isShader(fragmentShader) should be false. Threw exception TypeError: gl.isShader is not a function] expected: FAIL - [WebGL test #28: gl.getParameter(gl.TEXTURE_BINDING_2D) should be [object WebGLTexture\]. Was null.] - expected: FAIL - [WebGL test #29: gl.framebufferTexture2D(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.TEXTURE_2D, tex, 0) threw exception TypeError: gl.framebufferTexture2D is not a function] expected: FAIL @@ -35,9 +32,6 @@ [WebGL test #31: gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) should be 5890. Threw exception TypeError: gl.getFramebufferAttachmentParameter is not a function] expected: FAIL - [WebGL test #32: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.deleteTexture(tex)] - expected: FAIL - [WebGL test #33: gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) should be 0. Threw exception TypeError: gl.getFramebufferAttachmentParameter is not a function] expected: FAIL @@ -47,43 +41,13 @@ [WebGL test #35: gl.isTexture(tex) should be false. Threw exception TypeError: gl.isTexture is not a function] expected: FAIL - [WebGL test #37: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindTexture(gl.TEXTURE_2D, tex)] - expected: FAIL - - [WebGL test #40: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindTexture(gl.TEXTURE_CUBE_MAP, texCubeMap)] - expected: FAIL - - [WebGL test #41: gl.getParameter(gl.TEXTURE_BINDING_CUBE_MAP) should be [object WebGLTexture\]. Was null.] - expected: FAIL - - [WebGL test #42: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.deleteTexture(texCubeMap)] + [WebGL test #37: getError expected: NO_ERROR. Was INVALID_OPERATION : after evaluating: gl.bindTexture(gl.TEXTURE_2D, tex)] expected: FAIL [WebGL test #43: gl.isTexture(texCubeMap) should be false. Threw exception TypeError: gl.isTexture is not a function] expected: FAIL - [WebGL test #45: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindTexture(gl.TEXTURE_CUBE_MAP, texCubeMap)] - expected: FAIL - - [WebGL test #48: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindTexture(gl.TEXTURE_2D, t)] - expected: FAIL - - [WebGL test #56: gl.getParameter(gl.TEXTURE_BINDING_2D) should be [object WebGLTexture\]. Was null.] - expected: FAIL - - [WebGL test #57: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.activeTexture(gl.TEXTURE1)] - expected: FAIL - - [WebGL test #59: gl.getParameter(gl.TEXTURE_BINDING_2D) should be [object WebGLTexture\]. Was null.] - expected: FAIL - - [WebGL test #60: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.deleteTexture(t2)] - expected: FAIL - - [WebGL test #62: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.activeTexture(gl.TEXTURE0)] - expected: FAIL - - [WebGL test #67: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindRenderbuffer(gl.RENDERBUFFER, rbo)] + [WebGL test #45: getError expected: NO_ERROR. Was INVALID_OPERATION : after evaluating: gl.bindTexture(gl.TEXTURE_CUBE_MAP, texCubeMap)] expected: FAIL [WebGL test #68: gl.getParameter(gl.RENDERBUFFER_BINDING) should be [object WebGLRenderbuffer\]. Was null.] @@ -95,9 +59,6 @@ [WebGL test #70: gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) should be [object WebGLRenderbuffer\]. Threw exception TypeError: gl.getFramebufferAttachmentParameter is not a function] expected: FAIL - [WebGL test #71: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.deleteRenderbuffer(rbo)] - expected: FAIL - [WebGL test #72: gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) should be 0. Threw exception TypeError: gl.getFramebufferAttachmentParameter is not a function] expected: FAIL @@ -107,24 +68,12 @@ [WebGL test #74: gl.isRenderbuffer(rbo) should be false. Threw exception TypeError: gl.isRenderbuffer is not a function] expected: FAIL - [WebGL test #76: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindRenderbuffer(gl.RENDERBUFFER, rbo)] - expected: FAIL - - [WebGL test #78: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindRenderbuffer(gl.RENDERBUFFER, rbo2)] - expected: FAIL - [WebGL test #79: gl.getParameter(gl.RENDERBUFFER_BINDING) should be [object WebGLRenderbuffer\]. Was null.] expected: FAIL - [WebGL test #80: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.deleteRenderbuffer(rbo3)] - expected: FAIL - [WebGL test #81: gl.getParameter(gl.RENDERBUFFER_BINDING) should be [object WebGLRenderbuffer\]. Was null.] expected: FAIL - [WebGL test #82: getError expected: NO_ERROR. Was INVALID_ENUM : after evaluating: gl.bindRenderbuffer(gl.RENDERBUFFER, rbo)] - expected: FAIL - [WebGL test #83: gl.renderbufferStorage(gl.RENDERBUFFER, gl.RGBA4, 16, 16) threw exception TypeError: gl.renderbufferStorage is not a function] expected: FAIL