From 796ee705976f8072032a0fc88579ca18aed85999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1ty=C3=A1s=20Mustoha?= Date: Fri, 14 Feb 2020 11:57:11 +0100 Subject: [PATCH] Add initial support for WebGL2 read framebuffer Adds support for binding to the read framebuffer slot and querying its status. --- .../script/dom/webgl2renderingcontext.rs | 49 +++++- components/script/dom/webglrenderbuffer.rs | 24 +-- .../script/dom/webglrenderingcontext.rs | 159 +++++++++++------- components/script/dom/webgltexture.rs | 20 ++- .../renderbuffers/framebuffer-test.html.ini | 39 ++++- 5 files changed, 202 insertions(+), 89 deletions(-) diff --git a/components/script/dom/webgl2renderingcontext.rs b/components/script/dom/webgl2renderingcontext.rs index 71f1f108b395..0e9d24075a62 100644 --- a/components/script/dom/webgl2renderingcontext.rs +++ b/components/script/dom/webgl2renderingcontext.rs @@ -503,6 +503,13 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext { self.current_transform_feedback.get() ); }, + // NOTE: DRAW_FRAMEBUFFER_BINDING is the same as FRAMEBUFFER_BINDING, handled on the WebGL1 side + constants::READ_FRAMEBUFFER_BINDING => unsafe { + return optional_root_object_to_js_or_null!( + *cx, + &self.base.get_read_framebuffer_slot().get() + ); + }, _ => {}, } @@ -650,7 +657,32 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext { /// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6 fn BindFramebuffer(&self, target: u32, framebuffer: Option<&WebGLFramebuffer>) { - self.base.BindFramebuffer(target, framebuffer) + handle_potential_webgl_error!( + self.base, + self.base.validate_new_framebuffer_binding(framebuffer), + return + ); + + let (bind_read, bind_draw) = match target { + constants::FRAMEBUFFER => (true, true), + constants::READ_FRAMEBUFFER => (true, false), + constants::DRAW_FRAMEBUFFER => (false, true), + _ => return self.base.webgl_error(InvalidEnum), + }; + if bind_read { + self.base.bind_framebuffer_to( + target, + framebuffer, + &self.base.get_read_framebuffer_slot(), + ); + } + if bind_draw { + self.base.bind_framebuffer_to( + target, + framebuffer, + &self.base.get_draw_framebuffer_slot(), + ); + } } /// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7 @@ -2221,7 +2253,20 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext { /// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6 fn CheckFramebufferStatus(&self, target: u32) -> u32 { - self.base.CheckFramebufferStatus(target) + let fb_slot = match target { + constants::FRAMEBUFFER | constants::DRAW_FRAMEBUFFER => { + self.base.get_draw_framebuffer_slot() + }, + constants::READ_FRAMEBUFFER => &self.base.get_read_framebuffer_slot(), + _ => { + self.base.webgl_error(InvalidEnum); + return 0; + }, + }; + match fb_slot.get() { + Some(fb) => fb.check_status(), + None => constants::FRAMEBUFFER_COMPLETE, + } } /// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7 diff --git a/components/script/dom/webglrenderbuffer.rs b/components/script/dom/webglrenderbuffer.rs index ca75f83c3362..8c8164f62430 100644 --- a/components/script/dom/webglrenderbuffer.rs +++ b/components/script/dom/webglrenderbuffer.rs @@ -96,20 +96,24 @@ impl WebGLRenderbuffer { if !self.is_deleted.get() { self.is_deleted.set(true); + let context = self.upcast::().context(); + /* - If a renderbuffer object is deleted while its image is attached to the currently - bound framebuffer, then it is as if FramebufferRenderbuffer had been called, with - a renderbuffer of 0, for each attachment point to which this image was attached - in the currently bound framebuffer. - - GLES 2.0, 4.4.3, "Attaching Renderbuffer Images to a Framebuffer" - */ - let currently_bound_framebuffer = - self.upcast::().context().bound_framebuffer(); - if let Some(fb) = currently_bound_framebuffer { + If a renderbuffer object is deleted while its image is attached to one or more + attachment points in a currently bound framebuffer object, then it is as if + FramebufferRenderbuffer had been called, with a renderbuffer of zero, for each + attachment point to which this image was attached in that framebuffer object. + In other words,the renderbuffer image is first detached from all attachment points + in that frame-buffer object. + - GLES 3.0, 4.4.2.3, "Attaching Renderbuffer Images to a Framebuffer" + */ + if let Some(fb) = context.get_draw_framebuffer_slot().get() { + let _ = fb.detach_renderbuffer(self); + } + if let Some(fb) = context.get_read_framebuffer_slot().get() { let _ = fb.detach_renderbuffer(self); } - let context = self.upcast::().context(); let cmd = WebGLCommand::DeleteRenderbuffer(self.id); if fallible { context.send_command_ignored(cmd); diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 460f50aaadad..874a089a6376 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -163,7 +163,10 @@ pub struct WebGLRenderingContext { texture_unpacking_settings: Cell, // TODO(nox): Should be Cell. texture_unpacking_alignment: Cell, - bound_framebuffer: MutNullableDom, + bound_draw_framebuffer: MutNullableDom, + // TODO(mmatyas): This was introduced in WebGL2, but listed here because it's used by + // Textures and Renderbuffers, but such WebGLObjects have access only to the GL1 context. + bound_read_framebuffer: MutNullableDom, bound_renderbuffer: MutNullableDom, bound_buffer_array: MutNullableDom, current_program: MutNullableDom, @@ -223,7 +226,8 @@ impl WebGLRenderingContext { texture_packing_alignment: Cell::new(4), texture_unpacking_settings: Cell::new(TextureUnpacking::CONVERT_COLORSPACE), texture_unpacking_alignment: Cell::new(4), - bound_framebuffer: MutNullableDom::new(None), + bound_draw_framebuffer: MutNullableDom::new(None), + bound_read_framebuffer: MutNullableDom::new(None), bound_buffer_array: MutNullableDom::new(None), bound_renderbuffer: MutNullableDom::new(None), current_program: MutNullableDom::new(None), @@ -328,7 +332,7 @@ impl WebGLRenderingContext { // Bound framebuffer must not change when the canvas is resized. // Right now surfman generates a new FBO on resize. // Send a command to re-bind the framebuffer, if any. - if let Some(fbo) = self.bound_framebuffer.get() { + if let Some(fbo) = self.bound_draw_framebuffer.get() { let id = WebGLFramebufferBindingRequest::Explicit(fbo.id()); self.send_command(WebGLCommand::BindFramebuffer(constants::FRAMEBUFFER, id)); } @@ -402,7 +406,7 @@ impl WebGLRenderingContext { // The WebGL spec mentions a couple more operations that trigger // this: clear() and getParameter(IMPLEMENTATION_COLOR_READ_*). pub fn validate_framebuffer(&self) -> WebGLResult<()> { - match self.bound_framebuffer.get() { + match self.bound_draw_framebuffer.get() { Some(fb) => match fb.check_status_for_rendering() { CompleteForRendering::Complete => Ok(()), CompleteForRendering::Incomplete => Err(InvalidFramebufferOperation), @@ -478,7 +482,7 @@ impl WebGLRenderingContext { fn mark_as_dirty(&self) { // If we have a bound framebuffer, then don't mark the canvas as dirty. - if self.bound_framebuffer.get().is_some() { + if self.bound_draw_framebuffer.get().is_some() { return; } @@ -503,7 +507,7 @@ impl WebGLRenderingContext { } pub fn get_current_framebuffer_size(&self) -> Option<(i32, i32)> { - match self.bound_framebuffer.get() { + match self.bound_draw_framebuffer.get() { Some(fb) => return fb.size(), // The window system framebuffer is bound @@ -757,7 +761,7 @@ impl WebGLRenderingContext { data: pixels.data.into(), }); - if let Some(fb) = self.bound_framebuffer.get() { + if let Some(fb) = self.bound_draw_framebuffer.get() { fb.invalidate_texture(&*texture); } } @@ -1124,10 +1128,6 @@ impl WebGLRenderingContext { }); } - pub fn bound_framebuffer(&self) -> Option> { - self.bound_framebuffer.get() - } - pub fn extension_manager(&self) -> &WebGLExtensions { &self.extension_manager } @@ -1341,6 +1341,50 @@ impl WebGLRenderingContext { } self.uniform_vec_section::(vec, offset, length, uniform_size, uniform_location) } + + pub fn get_draw_framebuffer_slot(&self) -> &MutNullableDom { + &self.bound_draw_framebuffer + } + + pub fn get_read_framebuffer_slot(&self) -> &MutNullableDom { + &self.bound_read_framebuffer + } + + pub fn validate_new_framebuffer_binding( + &self, + framebuffer: Option<&WebGLFramebuffer>, + ) -> WebGLResult<()> { + if let Some(fb) = framebuffer { + self.validate_ownership(fb)?; + if fb.is_deleted() { + // From the WebGL spec: + // + // "An attempt to bind a deleted framebuffer will + // generate an INVALID_OPERATION error, and the + // current binding will remain untouched." + return Err(InvalidOperation); + } + } + Ok(()) + } + + pub fn bind_framebuffer_to( + &self, + target: u32, + framebuffer: Option<&WebGLFramebuffer>, + slot: &MutNullableDom, + ) { + match framebuffer { + Some(framebuffer) => framebuffer.bind(target), + None => { + // Bind the default framebuffer + let cmd = + WebGLCommand::BindFramebuffer(target, WebGLFramebufferBindingRequest::Default); + self.send_command(cmd); + }, + } + slot.set(framebuffer); + } } #[cfg(not(feature = "webgl_backtrace"))] @@ -1442,7 +1486,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { return optional_root_object_to_js_or_null!(*cx, buffer); }, constants::FRAMEBUFFER_BINDING => unsafe { - return optional_root_object_to_js_or_null!(*cx, &self.bound_framebuffer.get()); + return optional_root_object_to_js_or_null!( + *cx, + &self.bound_draw_framebuffer.get() + ); }, constants::RENDERBUFFER_BINDING => unsafe { return optional_root_object_to_js_or_null!(*cx, &self.bound_renderbuffer.get()); @@ -1845,33 +1892,17 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6 fn BindFramebuffer(&self, target: u32, framebuffer: Option<&WebGLFramebuffer>) { - if let Some(fb) = framebuffer { - handle_potential_webgl_error!(self, self.validate_ownership(fb), return); - } + handle_potential_webgl_error!( + self, + self.validate_new_framebuffer_binding(framebuffer), + return + ); if target != constants::FRAMEBUFFER { return self.webgl_error(InvalidEnum); } - if let Some(framebuffer) = framebuffer { - if framebuffer.is_deleted() { - // From the WebGL spec: - // - // "An attempt to bind a deleted framebuffer will - // generate an INVALID_OPERATION error, and the - // current binding will remain untouched." - return self.webgl_error(InvalidOperation); - } else { - framebuffer.bind(target); - self.bound_framebuffer.set(Some(framebuffer)); - } - } else { - // Bind the default framebuffer - let cmd = - WebGLCommand::BindFramebuffer(target, WebGLFramebufferBindingRequest::Default); - self.send_command(cmd); - self.bound_framebuffer.set(framebuffer); - } + self.bind_framebuffer_to(target, framebuffer, &self.bound_draw_framebuffer) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7 @@ -2009,7 +2040,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { data: pixels.data.into(), }); - if let Some(fb) = self.bound_framebuffer.get() { + if let Some(fb) = self.bound_draw_framebuffer.get() { fb.invalidate_texture(&*texture); } } @@ -2100,7 +2131,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { Err(_) => return, }; - let framebuffer_format = match self.bound_framebuffer.get() { + let framebuffer_format = match self.bound_draw_framebuffer.get() { Some(fb) => match fb.attachment(constants::COLOR_ATTACHMENT0) { Some(WebGLFramebufferAttachmentRoot::Renderbuffer(rb)) => { TexFormat::from_gl_constant(rb.internal_format()) @@ -2419,10 +2450,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { handle_potential_webgl_error!(self, self.validate_ownership(framebuffer), return); handle_object_deletion!( self, - self.bound_framebuffer, + self.bound_draw_framebuffer, framebuffer, Some(WebGLCommand::BindFramebuffer( - constants::FRAMEBUFFER, + framebuffer.target().unwrap(), WebGLFramebufferBindingRequest::Default )) ); @@ -2575,7 +2606,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { pname: u32, ) -> JSVal { // Check if currently bound framebuffer is non-zero as per spec. - if let Some(fb) = self.bound_framebuffer.get() { + if let Some(fb) = self.bound_draw_framebuffer.get() { // Opaque framebuffers cannot have their attachments inspected // https://immersive-web.github.io/webxr/#opaque-framebuffer handle_potential_webgl_error!(self, fb.validate_transparent(), return NullValue()); @@ -2617,27 +2648,31 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { _ => false, }; - let bound_attachment_matches = - match self.bound_framebuffer.get().unwrap().attachment(attachment) { - Some(attachment_root) => match attachment_root { - WebGLFramebufferAttachmentRoot::Renderbuffer(_) => match pname { - constants::FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE | - constants::FRAMEBUFFER_ATTACHMENT_OBJECT_NAME => true, - _ => false, - }, - WebGLFramebufferAttachmentRoot::Texture(_) => match pname { - constants::FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE | - constants::FRAMEBUFFER_ATTACHMENT_OBJECT_NAME | - constants::FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL | - constants::FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE => true, - _ => false, - }, + let bound_attachment_matches = match self + .bound_draw_framebuffer + .get() + .unwrap() + .attachment(attachment) + { + Some(attachment_root) => match attachment_root { + WebGLFramebufferAttachmentRoot::Renderbuffer(_) => match pname { + constants::FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE | + constants::FRAMEBUFFER_ATTACHMENT_OBJECT_NAME => true, + _ => false, }, - _ => match pname { - constants::FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE => true, + WebGLFramebufferAttachmentRoot::Texture(_) => match pname { + constants::FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE | + constants::FRAMEBUFFER_ATTACHMENT_OBJECT_NAME | + constants::FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL | + constants::FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE => true, _ => false, }, - }; + }, + _ => match pname { + constants::FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE => true, + _ => false, + }, + }; if !target_matches || !attachment_matches || !pname_matches || !bound_attachment_matches { self.webgl_error(InvalidEnum); @@ -2653,7 +2688,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { if pname == constants::FRAMEBUFFER_ATTACHMENT_OBJECT_NAME { // if fb is None, an INVALID_OPERATION is returned // at the beggining of the function, so `.unwrap()` will never panic - let fb = self.bound_framebuffer.get().unwrap(); + let fb = self.bound_draw_framebuffer.get().unwrap(); if let Some(webgl_attachment) = fb.attachment(attachment) { match webgl_attachment { WebGLFramebufferAttachmentRoot::Renderbuffer(rb) => unsafe { @@ -4158,7 +4193,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { return 0; } - match self.bound_framebuffer.get() { + match self.bound_draw_framebuffer.get() { Some(fb) => return fb.check_status(), None => return constants::FRAMEBUFFER_COMPLETE, } @@ -4185,7 +4220,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { self, rb.storage(self.api_type, internal_format, width, height) ); - if let Some(fb) = self.bound_framebuffer.get() { + if let Some(fb) = self.bound_draw_framebuffer.get() { fb.invalidate_renderbuffer(&*rb); } @@ -4208,7 +4243,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { return self.webgl_error(InvalidEnum); } - match self.bound_framebuffer.get() { + match self.bound_draw_framebuffer.get() { Some(fb) => handle_potential_webgl_error!(self, fb.renderbuffer(attachment, rb)), None => self.webgl_error(InvalidOperation), }; @@ -4231,7 +4266,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { return self.webgl_error(InvalidEnum); } - match self.bound_framebuffer.get() { + match self.bound_draw_framebuffer.get() { Some(fb) => handle_potential_webgl_error!( self, fb.texture2d(attachment, textarget, texture, level) diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index f7d52c8708a8..682f04f0bd83 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -197,15 +197,17 @@ impl WebGLTexture { } /* - If a texture object is deleted while its image is attached to the currently - bound framebuffer, then it is as if FramebufferTexture2D had been called, with - a texture of 0, for each attachment point to which this image was attached - in the currently bound framebuffer. - - GLES 2.0, 4.4.3, "Attaching Texture Images to a Framebuffer" - */ - let currently_bound_framebuffer = - self.upcast::().context().bound_framebuffer(); - if let Some(fb) = currently_bound_framebuffer { + If a texture object is deleted while its image is attached to one or more attachment + points in a currently bound framebuffer, then it is as if FramebufferTexture had been + called, with a texture of zero, for each attachment point to which this im-age was + attached in that framebuffer. In other words, this texture image is firstdetached from + all attachment points in a currently bound framebuffer. + - GLES 3.0, 4.4.2.3, "Attaching Texture Images to a Framebuffer" + */ + if let Some(fb) = context.get_draw_framebuffer_slot().get() { + let _ = fb.detach_texture(self); + } + if let Some(fb) = context.get_read_framebuffer_slot().get() { let _ = fb.detach_texture(self); } diff --git a/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-test.html.ini b/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-test.html.ini index ca03fddc1faa..4018c71869f5 100644 --- a/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-test.html.ini +++ b/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-test.html.ini @@ -50,9 +50,6 @@ [WebGL test #11: getError expected: NO_ERROR. Was INVALID_ENUM : checkFramebufferStatus(READ_FRAMEBUFFER).] expected: FAIL - [WebGL test #10: checkFramebufferStatus(READ_FRAMEBUFFER) should succeed.] - expected: FAIL - [WebGL test #27: getError expected: NO_ERROR. Was INVALID_OPERATION : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer.] expected: FAIL @@ -80,9 +77,6 @@ [WebGL test #56: getError expected: NO_ERROR. Was INVALID_ENUM : bind draw framebuffer to default (null) framebuffer.] expected: FAIL - [WebGL test #13: bindFramebuffer(READ_FRAMEBUFFER) should change READ_FRAMEBUFFER_BINDING.] - expected: FAIL - [WebGL test #12: getError expected: NO_ERROR. Was INVALID_ENUM : bindFramebuffer(READ_FRAMEBUFFER).] expected: FAIL @@ -167,3 +161,36 @@ [WebGL test #28: getError expected: NO_ERROR. Was INVALID_ENUM : attach a texture to read/draw framebuffer binding point.] expected: FAIL + [WebGL test #33: getError expected: NO_ERROR. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer.] + expected: FAIL + + [WebGL test #42: getError expected: NO_ERROR. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer.] + expected: FAIL + + [WebGL test #46: getError expected: NO_ERROR. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer.] + expected: FAIL + + [WebGL test #53: getError expected: INVALID_OPERATION. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_RED_SIZE) on draw framebuffer without color attachment.] + expected: FAIL + + [WebGL test #29: getError expected: NO_ERROR. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer.] + expected: FAIL + + [WebGL test #43: getError expected: INVALID_OPERATION. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer with no attachment.] + expected: FAIL + + [WebGL test #34: getError expected: INVALID_OPERATION. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer with no attachment.] + expected: FAIL + + [WebGL test #47: getError expected: INVALID_OPERATION. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer with no attachment.] + expected: FAIL + + [WebGL test #39: getError expected: NO_ERROR. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on draw framebuffer.] + expected: FAIL + + [WebGL test #56: getError expected: INVALID_OPERATION. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_DEPTH_SIZE) on draw framebuffer with no attachment.] + expected: FAIL + + [WebGL test #30: getError expected: INVALID_OPERATION. Was INVALID_ENUM : getFramebufferAttachmentParameter(FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) on read/draw framebuffer with no attachment.] + expected: FAIL +