From 03f6ce292eee9fe2ad62129fcb48ee74213436c1 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 15 Aug 2018 13:13:26 -0400 Subject: [PATCH 01/13] webgl: Remove unnecessary Option from texture API. --- components/script/dom/webgltexture.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index 2ecfd0bfab2..83378519848 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -146,7 +146,7 @@ impl WebGLTexture { } }; - let base_image_info = self.base_image_info().unwrap(); + let base_image_info = self.base_image_info(); if !base_image_info.is_initialized() { return Err(WebGLError::InvalidOperation); } @@ -327,7 +327,7 @@ impl WebGLTexture { fn is_cube_complete(&self) -> bool { debug_assert_eq!(self.face_count.get(), 6); - let image_info = self.base_image_info().unwrap(); + let image_info = self.base_image_info(); if !image_info.is_defined() { return false; } @@ -389,10 +389,10 @@ impl WebGLTexture { self.image_info_array.borrow_mut()[pos as usize] = image_info; } - fn base_image_info(&self) -> Option { + fn base_image_info(&self) -> ImageInfo { assert!((self.base_mipmap_level as usize) < MAX_LEVEL_COUNT); - Some(self.image_info_at_face(0, self.base_mipmap_level)) + self.image_info_at_face(0, self.base_mipmap_level) } pub fn set_attached_to_dom(&self) { From 80ed8292415d3fba6201c47061a8252e09e9ac68 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 15 Aug 2018 13:14:39 -0400 Subject: [PATCH 02/13] webgl: Mark framebuffers as incomplete if attachments do not meet format requirements. --- components/script/dom/webglframebuffer.rs | 24 ++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index 2d9e567d9bf..c2798ba4620 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -127,6 +127,12 @@ impl WebGLFramebuffer { let has_s = s.is_some(); let has_zs = zs.is_some(); let attachments = [&*c, &*z, &*s, &*zs]; + let attachment_constraints = [ + &[constants::RGBA4, constants::RGB5_A1, constants::RGB565][..], + &[constants::DEPTH_COMPONENT16][..], + &[constants::STENCIL_INDEX8][..], + &[constants::DEPTH_STENCIL][..], + ]; // From the WebGL spec, 6.6 ("Framebuffer Object Attachments"): // @@ -148,17 +154,18 @@ impl WebGLFramebuffer { } let mut fb_size = None; - for attachment in &attachments { + for (attachment, constraints) in attachments.iter().zip(&attachment_constraints) { // Get the size of this attachment. - let size = match **attachment { + let (format, size) = match **attachment { Some(WebGLFramebufferAttachment::Renderbuffer(ref att_rb)) => { - att_rb.size() + (Some(att_rb.internal_format()), att_rb.size()) } Some(WebGLFramebufferAttachment::Texture { texture: ref att_tex, level } ) => { let info = att_tex.image_info_at_face(0, level as u32); - Some((info.width() as i32, info.height() as i32)) + (info.data_type().map(|t| t.as_gl_constant()), + Some((info.width() as i32, info.height() as i32))) } - None => None, + None => (None, None), }; // Make sure that, if we've found any other attachment, @@ -171,6 +178,13 @@ impl WebGLFramebuffer { fb_size = size; } } + + if let Some(format) = format { + if constraints.iter().all(|c| *c != format) { + self.status.set(constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT); + return; + } + } } self.size.set(fb_size); From d179435eabbcb2038a8c1c33f86465c10334eebc Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 15 Aug 2018 13:27:44 -0400 Subject: [PATCH 03/13] webgl: Ensure that framebuffers have a color attachment before reading or writing. --- components/script/dom/webglframebuffer.rs | 10 +++ .../script/dom/webglrenderingcontext.rs | 2 +- .../framebuffer-object-attachment.html.ini | 90 ++++++++++++++++++- .../framebuffer-object-attachment.html.ini | 31 ++++++- 4 files changed, 129 insertions(+), 4 deletions(-) diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index c2798ba4620..53d72857b54 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -203,6 +203,16 @@ impl WebGLFramebuffer { return self.status.get(); } + pub fn check_status_for_rendering(&self) -> u32 { + let result = self.check_status(); + if result == constants::FRAMEBUFFER_COMPLETE { + if self.color.borrow().is_none() { + return constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT; + } + } + result + } + pub fn renderbuffer(&self, attachment: u32, rb: Option<&WebGLRenderbuffer>) -> WebGLResult<()> { let binding = match attachment { constants::COLOR_ATTACHMENT0 => &self.color, diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index d227af7ea1a..c4ea4667cbd 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -339,7 +339,7 @@ impl WebGLRenderingContext { // this: clear() and getParameter(IMPLEMENTATION_COLOR_READ_*). fn validate_framebuffer(&self) -> WebGLResult<()> { match self.bound_framebuffer.get() { - Some(ref fb) if fb.check_status() != constants::FRAMEBUFFER_COMPLETE => { + Some(ref fb) if fb.check_status_for_rendering() != constants::FRAMEBUFFER_COMPLETE => { Err(InvalidFramebufferOperation) }, _ => Ok(()), diff --git a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini index 13211175427..c887d27d401 100644 --- a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini +++ b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini @@ -1,5 +1,91 @@ [framebuffer-object-attachment.html] - expected: CRASH - [WebGL test #1: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] + [WebGL test #474: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + expected: FAIL + + [WebGL test #475: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + expected: FAIL + + [WebGL test #465: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + expected: FAIL + + [WebGL test #547: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexSubImage2D from missing attachment] + expected: FAIL + + [WebGL test #463: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + expected: FAIL + + [WebGL test #478: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + expected: FAIL + + [WebGL test #470: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + expected: FAIL + + [WebGL test #1: gl.checkFramebufferStatus(gl.FRAMEBUFFER) returned 36054] + expected: FAIL + + [WebGL test #543: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After ReadPixels from missing attachment] + expected: FAIL + + [WebGL test #2: gl.checkFramebufferStatus(gl.FRAMEBUFFER) returned 36054] + expected: FAIL + + [WebGL test #469: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + expected: FAIL + + [WebGL test #483: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] + expected: FAIL + + [WebGL test #477: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + expected: FAIL + + [WebGL test #476: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] + expected: FAIL + + [WebGL test #473: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + expected: FAIL + + [WebGL test #467: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + expected: FAIL + + [WebGL test #468: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + expected: FAIL + + [WebGL test #3: gl.checkFramebufferStatus(gl.FRAMEBUFFER) returned 36054] + expected: FAIL + + [WebGL test #466: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] + expected: FAIL + + [WebGL test #471: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] + expected: FAIL + + [WebGL test #545: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexImage2D from missing attachment] + expected: FAIL + + [WebGL test #481: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] + expected: FAIL + + [WebGL test #507: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] + expected: FAIL + + [WebGL test #482: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + expected: FAIL + + [WebGL test #510: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] + expected: FAIL + + [WebGL test #480: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + expected: FAIL + + [WebGL test #472: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + expected: FAIL + + [WebGL test #479: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + expected: FAIL + + [WebGL test #464: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + expected: FAIL + + [WebGL test #525: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] expected: FAIL diff --git a/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini b/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini index 28519ac2e8b..256dc0faeea 100644 --- a/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini +++ b/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini @@ -1,2 +1,31 @@ [framebuffer-object-attachment.html] - expected: CRASH + [WebGL test #13: checkFramebufferStatus expects [FRAMEBUFFER_COMPLETE\], was FRAMEBUFFER_INCOMPLETE_ATTACHMENT] + expected: FAIL + + [WebGL test #20: checkFramebufferStatus expects [FRAMEBUFFER_COMPLETE\], was FRAMEBUFFER_INCOMPLETE_ATTACHMENT] + expected: FAIL + + [WebGL test #30: getError expected: NO_ERROR. Was INVALID_ENUM : ] + expected: FAIL + + [WebGL test #21: checkFramebufferStatus expects [FRAMEBUFFER_COMPLETE\], was FRAMEBUFFER_UNSUPPORTED] + expected: FAIL + + [WebGL test #25: checkFramebufferStatus expects [FRAMEBUFFER_COMPLETE\], was FRAMEBUFFER_UNSUPPORTED] + expected: FAIL + + [WebGL test #23: checkFramebufferStatus expects [FRAMEBUFFER_COMPLETE\], was FRAMEBUFFER_UNSUPPORTED] + expected: FAIL + + [WebGL test #24: checkFramebufferStatus expects [FRAMEBUFFER_COMPLETE\], was FRAMEBUFFER_UNSUPPORTED] + expected: FAIL + + [WebGL test #22: checkFramebufferStatus expects [FRAMEBUFFER_COMPLETE\], was FRAMEBUFFER_UNSUPPORTED] + expected: FAIL + + [WebGL test #14: getError expected: NO_ERROR. Was INVALID_ENUM : ] + expected: FAIL + + [WebGL test #16: gl.getParameter(gl.RED_BITS) + gl.getParameter(gl.GREEN_BITS) + gl.getParameter(gl.BLUE_BITS) + gl.getParameter(gl.ALPHA_BITS) >= 16 should be true. Was false.] + expected: FAIL + From 15e2af0fea20a8141ea2eec4774607c434af0929 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 15 Aug 2018 14:38:28 -0400 Subject: [PATCH 04/13] webgl: Support DEPTH_STENCIL_ATTACHMENT on osmesa. --- components/canvas/webgl_thread.rs | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/components/canvas/webgl_thread.rs b/components/canvas/webgl_thread.rs index efbb4ca71c5..baa7abe2362 100644 --- a/components/canvas/webgl_thread.rs +++ b/components/canvas/webgl_thread.rs @@ -693,12 +693,31 @@ impl WebGLImpl { ctx.gl().disable(cap), WebGLCommand::Enable(cap) => ctx.gl().enable(cap), - WebGLCommand::FramebufferRenderbuffer(target, attachment, renderbuffertarget, rb) => - ctx.gl().framebuffer_renderbuffer(target, attachment, renderbuffertarget, - rb.map_or(0, WebGLRenderbufferId::get)), - WebGLCommand::FramebufferTexture2D(target, attachment, textarget, texture, level) => - ctx.gl().framebuffer_texture_2d(target, attachment, textarget, - texture.map_or(0, WebGLTextureId::get), level), + WebGLCommand::FramebufferRenderbuffer(target, attachment, renderbuffertarget, rb) => { + let attach = |attachment| { + ctx.gl().framebuffer_renderbuffer(target, attachment, + renderbuffertarget, + rb.map_or(0, WebGLRenderbufferId::get)) + }; + if attachment == gl::DEPTH_STENCIL_ATTACHMENT { + attach(gl::DEPTH_ATTACHMENT); + attach(gl::STENCIL_ATTACHMENT); + } else { + attach(attachment); + } + } + WebGLCommand::FramebufferTexture2D(target, attachment, textarget, texture, level) => { + let attach = |attachment| { + ctx.gl().framebuffer_texture_2d(target, attachment, textarget, + texture.map_or(0, WebGLTextureId::get), level) + }; + if attachment == gl::DEPTH_STENCIL_ATTACHMENT { + attach(gl::DEPTH_ATTACHMENT); + attach(gl::STENCIL_ATTACHMENT); + } else { + attach(attachment) + } + } WebGLCommand::FrontFace(mode) => ctx.gl().front_face(mode), WebGLCommand::DisableVertexAttribArray(attrib_id) => From 944d1d1f291f878cbb472ca77c473076f1d7a1ab Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 15 Aug 2018 14:59:19 -0400 Subject: [PATCH 05/13] webgl: Check internal format of textures when determining attachment completeness. --- components/script/dom/webglframebuffer.rs | 4 +- .../framebuffer-object-attachment.html.ini | 76 ++++--------------- 2 files changed, 16 insertions(+), 64 deletions(-) diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index 53d72857b54..fa66d61429f 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -128,7 +128,7 @@ impl WebGLFramebuffer { let has_zs = zs.is_some(); let attachments = [&*c, &*z, &*s, &*zs]; let attachment_constraints = [ - &[constants::RGBA4, constants::RGB5_A1, constants::RGB565][..], + &[constants::RGBA4, constants::RGB5_A1, constants::RGB565, constants::RGBA][..], &[constants::DEPTH_COMPONENT16][..], &[constants::STENCIL_INDEX8][..], &[constants::DEPTH_STENCIL][..], @@ -162,7 +162,7 @@ impl WebGLFramebuffer { } Some(WebGLFramebufferAttachment::Texture { texture: ref att_tex, level } ) => { let info = att_tex.image_info_at_face(0, level as u32); - (info.data_type().map(|t| t.as_gl_constant()), + (info.internal_format().map(|t| t.as_gl_constant()), Some((info.width() as i32, info.height() as i32))) } None => (None, None), diff --git a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini index c887d27d401..46af4d0a93b 100644 --- a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini +++ b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini @@ -1,91 +1,43 @@ [framebuffer-object-attachment.html] - [WebGL test #474: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + [WebGL test #522: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] expected: FAIL - [WebGL test #475: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + [WebGL test #483: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] expected: FAIL - [WebGL test #465: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + [WebGL test #537: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] expected: FAIL - [WebGL test #547: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexSubImage2D from missing attachment] + [WebGL test #476: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] expected: FAIL - [WebGL test #463: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + [WebGL test #491: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] expected: FAIL - [WebGL test #478: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + [WebGL test #478: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] expected: FAIL - [WebGL test #470: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] + [WebGL test #488: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] expected: FAIL - [WebGL test #1: gl.checkFramebufferStatus(gl.FRAMEBUFFER) returned 36054] + [WebGL test #481: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] expected: FAIL - [WebGL test #543: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After ReadPixels from missing attachment] + [WebGL test #486: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] expected: FAIL - [WebGL test #2: gl.checkFramebufferStatus(gl.FRAMEBUFFER) returned 36054] + [WebGL test #559: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexSubImage2D from missing attachment] expected: FAIL - [WebGL test #469: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + [WebGL test #555: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After ReadPixels from missing attachment] expected: FAIL - [WebGL test #483: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] + [WebGL test #473: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] expected: FAIL - [WebGL test #477: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] + [WebGL test #519: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] expected: FAIL - [WebGL test #476: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] - expected: FAIL - - [WebGL test #473: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] - expected: FAIL - - [WebGL test #467: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - - [WebGL test #468: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] - expected: FAIL - - [WebGL test #3: gl.checkFramebufferStatus(gl.FRAMEBUFFER) returned 36054] - expected: FAIL - - [WebGL test #466: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] - expected: FAIL - - [WebGL test #471: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] - expected: FAIL - - [WebGL test #545: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexImage2D from missing attachment] - expected: FAIL - - [WebGL test #481: getError expected: NO_ERROR. Was INVALID_FRAMEBUFFER_OPERATION : ] - expected: FAIL - - [WebGL test #507: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] - expected: FAIL - - [WebGL test #482: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - - [WebGL test #510: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] - expected: FAIL - - [WebGL test #480: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.] - expected: FAIL - - [WebGL test #472: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - - [WebGL test #479: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - - [WebGL test #464: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - - [WebGL test #525: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] + [WebGL test #557: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexImage2D from missing attachment] expected: FAIL From da3b0ef88f52d61f4f94f2fa70099d42aebae061 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 22 Aug 2018 14:50:11 -0400 Subject: [PATCH 06/13] webgl: Clear renderbuffers on first read/write operation. --- components/script/dom/webglframebuffer.rs | 47 +++++++++++++++++-- .../framebuffer-object-attachment.html.ini | 28 ++--------- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index fa66d61429f..de659e6ebd3 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -25,6 +25,15 @@ enum WebGLFramebufferAttachment { Texture { texture: Dom, level: i32 }, } +impl WebGLFramebufferAttachment { + fn needs_initialization(&self) -> bool { + match *self { + WebGLFramebufferAttachment::Renderbuffer(_) => true, + WebGLFramebufferAttachment::Texture { .. } => false, + } + } +} + #[derive(Clone, JSTraceable, MallocSizeOf)] pub enum WebGLFramebufferAttachmentRoot { Renderbuffer(DomRoot), @@ -46,6 +55,7 @@ pub struct WebGLFramebuffer { depth: DomRefCell>, stencil: DomRefCell>, depthstencil: DomRefCell>, + is_initialized: Cell, } impl WebGLFramebuffer { @@ -61,6 +71,7 @@ impl WebGLFramebuffer { depth: DomRefCell::new(None), stencil: DomRefCell::new(None), depthstencil: DomRefCell::new(None), + is_initialized: Cell::new(false), } } @@ -205,12 +216,36 @@ impl WebGLFramebuffer { pub fn check_status_for_rendering(&self) -> u32 { let result = self.check_status(); - if result == constants::FRAMEBUFFER_COMPLETE { - if self.color.borrow().is_none() { - return constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT; - } + if result != constants::FRAMEBUFFER_COMPLETE { + return result; } - result + + if self.color.borrow().is_none() { + return constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT; + } + + if !self.is_initialized.get() { + let attachments = [ + (&self.color, constants::COLOR_BUFFER_BIT), + (&self.depth, constants::DEPTH_BUFFER_BIT), + (&self.stencil, constants::STENCIL_BUFFER_BIT), + (&self.depthstencil, constants::DEPTH_BUFFER_BIT | constants::STENCIL_BUFFER_BIT) + ]; + let mut clear_bits = 0; + for &(attachment, bits) in &attachments { + if attachment.borrow().as_ref().map_or(false, |att| att.needs_initialization()) { + clear_bits |= bits; + } + } + if clear_bits != 0 { + self.upcast::().context().send_command( + WebGLCommand::Clear(clear_bits) + ); + } + self.is_initialized.set(true); + } + + constants::FRAMEBUFFER_COMPLETE } pub fn renderbuffer(&self, attachment: u32, rb: Option<&WebGLRenderbuffer>) -> WebGLResult<()> { @@ -244,6 +279,7 @@ impl WebGLFramebuffer { ); self.update_status(); + self.is_initialized.set(false); Ok(()) } @@ -344,6 +380,7 @@ impl WebGLFramebuffer { ); self.update_status(); + self.is_initialized.set(false); Ok(()) } diff --git a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini index 46af4d0a93b..06d68f8e7ed 100644 --- a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini +++ b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini @@ -1,43 +1,21 @@ [framebuffer-object-attachment.html] + [WebGL test #139: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] + expected: fail + [WebGL test #522: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] expected: FAIL - [WebGL test #483: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - [WebGL test #537: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] expected: FAIL - [WebGL test #476: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] - expected: FAIL - - [WebGL test #491: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] - expected: FAIL - - [WebGL test #478: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - - [WebGL test #488: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - - [WebGL test #481: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] - expected: FAIL - - [WebGL test #486: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] - expected: FAIL - [WebGL test #559: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexSubImage2D from missing attachment] expected: FAIL [WebGL test #555: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After ReadPixels from missing attachment] expected: FAIL - [WebGL test #473: at (0, 0) expected: 0,255,0,255 was 0,0,0,0] - expected: FAIL - [WebGL test #519: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] expected: FAIL [WebGL test #557: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexImage2D from missing attachment] expected: FAIL - From 690c98dda7d618d9f0a6553ca17d1dfc4a97555b Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 22 Aug 2018 15:09:05 -0400 Subject: [PATCH 07/13] webgl: return missing attachment status from framebuffers with no attachments. --- components/script/dom/webglframebuffer.rs | 4 ++-- .../renderbuffers/framebuffer-object-attachment.html.ini | 9 --------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index de659e6ebd3..83814bd2f30 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -66,7 +66,7 @@ impl WebGLFramebuffer { target: Cell::new(None), is_deleted: Cell::new(false), size: Cell::new(None), - status: Cell::new(constants::FRAMEBUFFER_UNSUPPORTED), + status: Cell::new(constants::FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT), color: DomRefCell::new(None), depth: DomRefCell::new(None), stencil: DomRefCell::new(None), @@ -206,7 +206,7 @@ impl WebGLFramebuffer { self.status.set(constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT); } } else { - self.status.set(constants::FRAMEBUFFER_UNSUPPORTED); + self.status.set(constants::FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT); } } diff --git a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini index 06d68f8e7ed..de3a2cf6273 100644 --- a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini +++ b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini @@ -2,20 +2,11 @@ [WebGL test #139: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] expected: fail - [WebGL test #522: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] - expected: FAIL - - [WebGL test #537: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] - expected: FAIL - [WebGL test #559: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexSubImage2D from missing attachment] expected: FAIL [WebGL test #555: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After ReadPixels from missing attachment] expected: FAIL - [WebGL test #519: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT was FRAMEBUFFER_UNSUPPORTED or FRAMEBUFFER_UNSUPPORTED] - expected: FAIL - [WebGL test #557: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexImage2D from missing attachment] expected: FAIL From df8e36aa783b9f6a50a5a16a39f7dcbd65ffde76 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 22 Aug 2018 15:26:32 -0400 Subject: [PATCH 08/13] webgl: Differentiate between missing colour attachments and incomplete attachments. --- components/script/dom/webglframebuffer.rs | 14 ++++++++++---- components/script/dom/webglrenderingcontext.rs | 12 ++++++++---- .../framebuffer-object-attachment.html.ini | 9 --------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index 83814bd2f30..09a1cbea7cc 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -18,6 +18,12 @@ use dom::webgltexture::WebGLTexture; use dom_struct::dom_struct; use std::cell::Cell; +pub enum CompleteForRendering { + Complete, + Incomplete, + MissingColorAttachment, +} + #[must_root] #[derive(Clone, JSTraceable, MallocSizeOf)] enum WebGLFramebufferAttachment { @@ -214,14 +220,14 @@ impl WebGLFramebuffer { return self.status.get(); } - pub fn check_status_for_rendering(&self) -> u32 { + pub fn check_status_for_rendering(&self) -> CompleteForRendering { let result = self.check_status(); if result != constants::FRAMEBUFFER_COMPLETE { - return result; + return CompleteForRendering::Incomplete; } if self.color.borrow().is_none() { - return constants::FRAMEBUFFER_INCOMPLETE_ATTACHMENT; + return CompleteForRendering::MissingColorAttachment; } if !self.is_initialized.get() { @@ -245,7 +251,7 @@ impl WebGLFramebuffer { self.is_initialized.set(true); } - constants::FRAMEBUFFER_COMPLETE + CompleteForRendering::Complete } pub fn renderbuffer(&self, attachment: u32, rb: Option<&WebGLRenderbuffer>) -> WebGLResult<()> { diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index c4ea4667cbd..db0fabf452d 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -39,7 +39,7 @@ use dom::webgl_validations::types::{TexDataType, TexFormat, TexImageTarget}; use dom::webglactiveinfo::WebGLActiveInfo; use dom::webglbuffer::WebGLBuffer; use dom::webglcontextevent::WebGLContextEvent; -use dom::webglframebuffer::{WebGLFramebuffer, WebGLFramebufferAttachmentRoot}; +use dom::webglframebuffer::{WebGLFramebuffer, WebGLFramebufferAttachmentRoot, CompleteForRendering}; use dom::webglobject::WebGLObject; use dom::webglprogram::WebGLProgram; use dom::webglrenderbuffer::WebGLRenderbuffer; @@ -339,10 +339,14 @@ impl WebGLRenderingContext { // this: clear() and getParameter(IMPLEMENTATION_COLOR_READ_*). fn validate_framebuffer(&self) -> WebGLResult<()> { match self.bound_framebuffer.get() { - Some(ref fb) if fb.check_status_for_rendering() != constants::FRAMEBUFFER_COMPLETE => { - Err(InvalidFramebufferOperation) + Some(fb) => match fb.check_status_for_rendering() { + CompleteForRendering::Complete => Ok(()), + CompleteForRendering::Incomplete => + Err(InvalidFramebufferOperation), + CompleteForRendering::MissingColorAttachment => + Err(InvalidOperation), }, - _ => Ok(()), + None => Ok(()), } } diff --git a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini index de3a2cf6273..b0777f21743 100644 --- a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini +++ b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini @@ -1,12 +1,3 @@ [framebuffer-object-attachment.html] [WebGL test #139: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] expected: fail - - [WebGL test #559: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexSubImage2D from missing attachment] - expected: FAIL - - [WebGL test #555: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After ReadPixels from missing attachment] - expected: FAIL - - [WebGL test #557: getError expected: INVALID_OPERATION. Was INVALID_FRAMEBUFFER_OPERATION : After CopyTexImage2D from missing attachment] - expected: FAIL From 1b08dd523274b1fb346b413cb986cd47409f3aa7 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 24 Aug 2018 13:43:32 -0400 Subject: [PATCH 09/13] webgl: Move framebuffer initialization logic to WebGL thread. --- components/canvas/gl_context.rs | 8 +- components/canvas/webgl_thread.rs | 255 +++++++++++++----- components/canvas_traits/webgl.rs | 1 + components/script/dom/webglframebuffer.rs | 22 +- components/script/dom/webglrenderbuffer.rs | 10 + .../script/dom/webglrenderingcontext.rs | 17 +- 6 files changed, 238 insertions(+), 75 deletions(-) diff --git a/components/canvas/gl_context.rs b/components/canvas/gl_context.rs index 7720f8f678a..00e3f3ba4a9 100644 --- a/components/canvas/gl_context.rs +++ b/components/canvas/gl_context.rs @@ -11,7 +11,7 @@ use offscreen_gl_context::{GLLimits, GLVersion}; use offscreen_gl_context::{NativeGLContext, NativeGLContextHandle, NativeGLContextMethods}; use offscreen_gl_context::{OSMesaContext, OSMesaContextHandle}; use std::sync::{Arc, Mutex}; -use super::webgl_thread::WebGLImpl; +use super::webgl_thread::{WebGLImpl, GLState}; /// The GLContextFactory is used to create shared GL contexts with the main thread GL context. /// Currently, shared textures are used to render WebGL textures into the WR compositor. @@ -144,13 +144,13 @@ impl GLContextWrapper { } } - pub fn apply_command(&self, cmd: WebGLCommand) { + pub fn apply_command(&self, cmd: WebGLCommand, state: &mut GLState) { match *self { GLContextWrapper::Native(ref ctx) => { - WebGLImpl::apply(ctx, cmd); + WebGLImpl::apply(ctx, state, cmd); } GLContextWrapper::OSMesa(ref ctx) => { - WebGLImpl::apply(ctx, cmd); + WebGLImpl::apply(ctx, state, cmd); } } } diff --git a/components/canvas/webgl_thread.rs b/components/canvas/webgl_thread.rs index baa7abe2362..b4f63f12f28 100644 --- a/components/canvas/webgl_thread.rs +++ b/components/canvas/webgl_thread.rs @@ -18,6 +18,33 @@ use webrender_api; /// It allows to get a WebGLThread handle for each script pipeline. pub use ::webgl_mode::WebGLThreads; +struct GLContextData { + ctx: GLContextWrapper, + state: GLState, +} + +pub struct GLState { + clear_color: (f32, f32, f32, f32), + scissor_test_enabled: bool, + stencil_write_mask: (u32, u32), + stencil_clear_value: i32, + depth_write_mask: bool, + depth_clear_value: f64, +} + +impl Default for GLState { + fn default() -> GLState { + GLState { + clear_color: (0., 0., 0., 0.), + scissor_test_enabled: false, + stencil_write_mask: (0, 0), + stencil_clear_value: 0, + depth_write_mask: true, + depth_clear_value: 1., + } + } +} + /// A WebGLThread manages the life cycle and message multiplexing of /// a set of WebGLContexts living in the same thread. pub struct WebGLThread { @@ -26,7 +53,7 @@ pub struct WebGLThread, + contexts: FnvHashMap, /// Cached information for WebGLContexts. cached_context_info: FnvHashMap, /// Current bound context. @@ -93,9 +120,9 @@ impl WebGLThread { let result = self.create_webgl_context(version, size, attributes); result_sender.send(result.map(|(id, limits, share_mode)| { - let ctx = Self::make_current_if_needed(id, &self.contexts, &mut self.bound_context_id) + let data = Self::make_current_if_needed(id, &self.contexts, &mut self.bound_context_id) .expect("WebGLContext not found"); - let glsl_version = Self::get_glsl_version(ctx); + let glsl_version = Self::get_glsl_version(&data.ctx); WebGLCreateContextResult { sender: WebGLMsgSender::new(id, webgl_chan.clone()), @@ -139,8 +166,9 @@ impl WebGLThread WebGLThread, usize)>) { - let ctx = Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id) + let data = Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id) .expect("WebGLContext not found in a WebGLMsg::Lock message"); let info = self.cached_context_info.get_mut(&context_id).unwrap(); // Insert a OpenGL Fence sync object that sends a signal when all the WebGL commands are finished. // The related gl().wait_sync call is performed in the WR thread. See WebGLExternalImageApi for mor details. - let gl_sync = ctx.gl().fence_sync(gl::SYNC_GPU_COMMANDS_COMPLETE, 0); + let gl_sync = data.ctx.gl().fence_sync(gl::SYNC_GPU_COMMANDS_COMPLETE, 0); info.gl_sync = Some(gl_sync); // It is important that the fence sync is properly flushed into the GPU's command queue. // Without proper flushing, the sync object may never be signaled. - ctx.gl().flush(); + data.ctx.gl().flush(); sender.send((info.texture_id, info.size, gl_sync as usize)).unwrap(); } /// Handles an unlock external callback received from webrender::ExternalImageHandler fn handle_unlock(&mut self, context_id: WebGLContextId) { - let ctx = Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id) + let data = Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id) .expect("WebGLContext not found in a WebGLMsg::Unlock message"); let info = self.cached_context_info.get_mut(&context_id).unwrap(); if let Some(gl_sync) = info.gl_sync.take() { // Release the GLSync object. - ctx.gl().delete_sync(gl_sync); + data.ctx.gl().delete_sync(gl_sync); } } @@ -207,7 +235,10 @@ impl WebGLThread WebGLThread, sender: WebGLSender>) { - let ctx = Self::make_current_if_needed_mut(context_id, &mut self.contexts, &mut self.bound_context_id); - match ctx.resize(size) { + let data = Self::make_current_if_needed_mut( + context_id, + &mut self.contexts, + &mut self.bound_context_id + ).expect("Missing WebGL context!"); + match data.ctx.resize(size) { Ok(_) => { - let (real_size, texture_id, _) = ctx.get_info(); + let (real_size, texture_id, _) = data.ctx.get_info(); self.observer.on_context_resize(context_id, texture_id, real_size); let info = self.cached_context_info.get_mut(&context_id).unwrap(); @@ -306,7 +341,7 @@ impl WebGLThread { - let pixels = Self::raw_pixels(&self.contexts[&context_id], info.size); + let pixels = Self::raw_pixels(&self.contexts[&context_id].ctx, info.size); match info.image_key.clone() { Some(image_key) => { // ImageKey was already created, but WR Images must @@ -339,18 +374,20 @@ impl WebGLThread { - let ctx = Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id) + let data = Self::make_current_if_needed(context_id, &self.contexts, &mut self.bound_context_id) .expect("WebGLContext not found in a WebGL DOMToTextureCommand::Attach command"); // Initialize the texture that WR will use for frame outputs. - ctx.gl().tex_image_2d(gl::TEXTURE_2D, - 0, - gl::RGBA as gl::GLint, - size.width, - size.height, - 0, - gl::RGBA, - gl::UNSIGNED_BYTE, - None); + data.ctx.gl().tex_image_2d( + gl::TEXTURE_2D, + 0, + gl::RGBA as gl::GLint, + size.width, + size.height, + 0, + gl::RGBA, + gl::UNSIGNED_BYTE, + None + ); self.dom_outputs.insert(pipeline_id, DOMToTextureData { context_id, texture_id, document_id, size }); @@ -361,14 +398,14 @@ impl WebGLThread { let contexts = &self.contexts; let bound_context_id = &mut self.bound_context_id; - let result = self.dom_outputs.get(&pipeline_id).and_then(|data| { - let ctx = Self::make_current_if_needed(data.context_id, contexts, bound_context_id); - ctx.and_then(|ctx| { + let result = self.dom_outputs.get(&pipeline_id).and_then(|dom_data| { + let data = Self::make_current_if_needed(dom_data.context_id, contexts, bound_context_id); + data.and_then(|data| { // The next glWaitSync call is used to synchronize the two flows of // OpenGL commands (WR and WebGL) in order to avoid using semi-ready WR textures. // glWaitSync doesn't block WebGL CPU thread. - ctx.gl().wait_sync(gl_sync as gl::GLsync, 0, gl::TIMEOUT_IGNORED); - Some((data.texture_id.get(), data.size)) + data.ctx.gl().wait_sync(gl_sync as gl::GLsync, 0, gl::TIMEOUT_IGNORED); + Some((dom_data.texture_id.get(), dom_data.size)) }) }); @@ -390,28 +427,37 @@ impl WebGLThread(context_id: WebGLContextId, - contexts: &'a FnvHashMap, - bound_id: &mut Option) -> Option<&'a GLContextWrapper> { - contexts.get(&context_id).and_then(|ctx| { + contexts: &'a FnvHashMap, + bound_id: &mut Option) -> Option<&'a GLContextData> { + let data = contexts.get(&context_id); + + if let Some(data) = data { if Some(context_id) != *bound_id { - ctx.make_current(); + data.ctx.make_current(); *bound_id = Some(context_id); } + } - Some(ctx) - }) + data } /// Gets a mutable reference to a GLContextWrapper for a WebGLContextId and makes it current if required. - fn make_current_if_needed_mut<'a>(context_id: WebGLContextId, - contexts: &'a mut FnvHashMap, - bound_id: &mut Option) -> &'a mut GLContextWrapper { - let ctx = contexts.get_mut(&context_id).expect("WebGLContext not found!"); - if Some(context_id) != *bound_id { - ctx.make_current(); - *bound_id = Some(context_id); + fn make_current_if_needed_mut<'a>( + context_id: WebGLContextId, + contexts: &'a mut FnvHashMap, + bound_id: &mut Option) + -> Option<&'a mut GLContextData> + { + let data = contexts.get_mut(&context_id); + + if let Some(ref data) = data { + if Some(context_id) != *bound_id { + data.ctx.make_current(); + *bound_id = Some(context_id); + } } - ctx + + data } /// Creates a `webrender_api::ImageKey` that uses shared textures. @@ -636,7 +682,11 @@ pub struct WebGLImpl; impl WebGLImpl { #[allow(unsafe_code)] - pub fn apply(ctx: &GLContext, command: WebGLCommand) { + pub fn apply( + ctx: &GLContext, + state: &mut GLState, + command: WebGLCommand + ) { match command { WebGLCommand::GetContextAttributes(ref sender) => sender.send(*ctx.borrow_attributes()).unwrap(), @@ -667,13 +717,19 @@ impl WebGLImpl { }, WebGLCommand::Clear(mask) => ctx.gl().clear(mask), - WebGLCommand::ClearColor(r, g, b, a) => - ctx.gl().clear_color(r, g, b, a), - WebGLCommand::ClearDepth(depth) => { - ctx.gl().clear_depth(depth.max(0.).min(1.) as f64) + WebGLCommand::ClearColor(r, g, b, a) => { + state.clear_color = (r, g, b, a); + ctx.gl().clear_color(r, g, b, a); + } + WebGLCommand::ClearDepth(depth) => { + let value = depth.max(0.).min(1.) as f64; + state.depth_clear_value = value; + ctx.gl().clear_depth(value) + } + WebGLCommand::ClearStencil(stencil) => { + state.stencil_clear_value = stencil; + ctx.gl().clear_stencil(stencil); } - WebGLCommand::ClearStencil(stencil) => - ctx.gl().clear_stencil(stencil), WebGLCommand::ColorMask(r, g, b, a) => ctx.gl().color_mask(r, g, b, a), WebGLCommand::CopyTexImage2D(target, level, internal_format, x, y, width, height, border) => @@ -684,15 +740,25 @@ impl WebGLImpl { ctx.gl().cull_face(mode), WebGLCommand::DepthFunc(func) => ctx.gl().depth_func(func), - WebGLCommand::DepthMask(flag) => - ctx.gl().depth_mask(flag), + WebGLCommand::DepthMask(flag) => { + state.depth_write_mask = flag; + ctx.gl().depth_mask(flag); + } WebGLCommand::DepthRange(near, far) => { ctx.gl().depth_range(near.max(0.).min(1.) as f64, far.max(0.).min(1.) as f64) } - WebGLCommand::Disable(cap) => - ctx.gl().disable(cap), - WebGLCommand::Enable(cap) => - ctx.gl().enable(cap), + WebGLCommand::Disable(cap) => { + if cap == gl::SCISSOR_TEST { + state.scissor_test_enabled = false; + } + ctx.gl().disable(cap); + } + WebGLCommand::Enable(cap) => { + if cap == gl::SCISSOR_TEST { + state.scissor_test_enabled = true; + } + ctx.gl().enable(cap); + } WebGLCommand::FramebufferRenderbuffer(target, attachment, renderbuffertarget, rb) => { let attach = |attachment| { ctx.gl().framebuffer_renderbuffer(target, attachment, @@ -745,10 +811,18 @@ impl WebGLImpl { ctx.gl().stencil_func(func, ref_, mask), WebGLCommand::StencilFuncSeparate(face, func, ref_, mask) => ctx.gl().stencil_func_separate(face, func, ref_, mask), - WebGLCommand::StencilMask(mask) => - ctx.gl().stencil_mask(mask), - WebGLCommand::StencilMaskSeparate(face, mask) => - ctx.gl().stencil_mask_separate(face, mask), + WebGLCommand::StencilMask(mask) => { + state.stencil_write_mask = (mask, mask); + ctx.gl().stencil_mask(mask); + } + WebGLCommand::StencilMaskSeparate(face, mask) => { + if face == gl::FRONT { + state.stencil_write_mask.0 = mask; + } else { + state.stencil_write_mask.1 = mask; + } + ctx.gl().stencil_mask_separate(face, mask); + } WebGLCommand::StencilOp(fail, zfail, zpass) => ctx.gl().stencil_op(fail, zfail, zpass), WebGLCommand::StencilOpSeparate(face, fail, zfail, zpass) => @@ -1124,6 +1198,9 @@ impl WebGLImpl { } sender.send(value).unwrap(); } + WebGLCommand::InitializeFramebuffer { color, depth, stencil } => { + Self::initialize_framebuffer(ctx.gl(), state, color, depth, stencil) + } } // TODO: update test expectations in order to enable debug assertions @@ -1134,6 +1211,62 @@ impl WebGLImpl { assert_eq!(error, gl::NO_ERROR, "Unexpected WebGL error: 0x{:x} ({})", error, error); } + fn initialize_framebuffer( + gl: &gl::Gl, + state: &GLState, + color: bool, + depth: bool, + stencil: bool, + ) { + let bits = [ + (color, gl::COLOR_BUFFER_BIT), + (depth, gl::DEPTH_BUFFER_BIT), + (stencil, gl::STENCIL_BUFFER_BIT), + ].iter().fold(0, |bits, &(enabled, bit)| bits | if enabled { bit } else { 0 }); + + if state.scissor_test_enabled { + gl.disable(gl::SCISSOR_TEST); + } + + if color { + gl.clear_color(0., 0., 0., 0.); + } + + if depth { + gl.depth_mask(true); + gl.clear_depth(1.); + } + + if stencil { + gl.stencil_mask_separate(gl::FRONT, 0xFFFFFFFF); + gl.stencil_mask_separate(gl::BACK, 0xFFFFFFFF); + gl.clear_stencil(0); + } + + gl.clear(bits); + + if state.scissor_test_enabled { + gl.enable(gl::SCISSOR_TEST); + } + + if color { + let (r, g, b, a) = state.clear_color; + gl.clear_color(r, g, b, a); + } + + if depth { + gl.depth_mask(state.depth_write_mask); + gl.clear_depth(state.depth_clear_value); + } + + if stencil { + let (front, back) = state.stencil_write_mask; + gl.stencil_mask_separate(gl::FRONT, front); + gl.stencil_mask_separate(gl::BACK, back); + gl.clear_stencil(state.stencil_clear_value); + } + } + #[allow(unsafe_code)] fn link_program(gl: &gl::Gl, program: WebGLProgramId) -> ProgramLinkInfo { gl.link_program(program.get()); diff --git a/components/canvas_traits/webgl.rs b/components/canvas_traits/webgl.rs index 57647204910..9c74b31fd1e 100644 --- a/components/canvas_traits/webgl.rs +++ b/components/canvas_traits/webgl.rs @@ -296,6 +296,7 @@ pub enum WebGLCommand { GetUniformFloat4(WebGLProgramId, i32, WebGLSender<[f32; 4]>), GetUniformFloat9(WebGLProgramId, i32, WebGLSender<[f32; 9]>), GetUniformFloat16(WebGLProgramId, i32, WebGLSender<[f32; 16]>), + InitializeFramebuffer { color: bool, depth: bool, stencil: bool }, } macro_rules! define_resource_id_struct { diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index 09a1cbea7cc..277b8b2fcba 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -34,10 +34,17 @@ enum WebGLFramebufferAttachment { impl WebGLFramebufferAttachment { fn needs_initialization(&self) -> bool { match *self { - WebGLFramebufferAttachment::Renderbuffer(_) => true, + WebGLFramebufferAttachment::Renderbuffer(ref r) => !r.is_initialized(), WebGLFramebufferAttachment::Texture { .. } => false, } } + + fn mark_initialized(&self) { + match *self { + WebGLFramebufferAttachment::Renderbuffer(ref r) => r.mark_initialized(), + WebGLFramebufferAttachment::Texture { .. } => () + } + } } #[derive(Clone, JSTraceable, MallocSizeOf)] @@ -239,15 +246,14 @@ impl WebGLFramebuffer { ]; let mut clear_bits = 0; for &(attachment, bits) in &attachments { - if attachment.borrow().as_ref().map_or(false, |att| att.needs_initialization()) { - clear_bits |= bits; + if let Some(ref att) = *attachment.borrow() { + if att.needs_initialization() { + att.mark_initialized(); + clear_bits |= bits; + } } } - if clear_bits != 0 { - self.upcast::().context().send_command( - WebGLCommand::Clear(clear_bits) - ); - } + self.upcast::().context().initialize_framebuffer(clear_bits); self.is_initialized.set(true); } diff --git a/components/script/dom/webglrenderbuffer.rs b/components/script/dom/webglrenderbuffer.rs index fa7d7324f94..f346407581f 100644 --- a/components/script/dom/webglrenderbuffer.rs +++ b/components/script/dom/webglrenderbuffer.rs @@ -23,6 +23,7 @@ pub struct WebGLRenderbuffer { is_deleted: Cell, size: Cell>, internal_format: Cell>, + is_initialized: Cell, } impl WebGLRenderbuffer { @@ -34,6 +35,7 @@ impl WebGLRenderbuffer { is_deleted: Cell::new(false), internal_format: Cell::new(None), size: Cell::new(None), + is_initialized: Cell::new(false), } } @@ -66,6 +68,14 @@ impl WebGLRenderbuffer { self.internal_format.get().unwrap_or(constants::RGBA4) } + pub fn mark_initialized(&self) { + self.is_initialized.set(true); + } + + pub fn is_initialized(&self) -> bool { + self.is_initialized.get() + } + pub fn bind(&self, target: u32) { self.ever_bound.set(true); self.upcast::() diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index db0fabf452d..6ca010a0b6e 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -1061,6 +1061,17 @@ impl WebGLRenderingContext { _ => Err(InvalidEnum), } } + + pub fn initialize_framebuffer(&self, clear_bits: u32) { + if clear_bits == 0 { + return; + } + self.send_command(WebGLCommand::InitializeFramebuffer { + color: clear_bits & constants::COLOR_BUFFER_BIT != 0, + depth: clear_bits & constants::DEPTH_BUFFER_BIT != 0, + stencil: clear_bits & constants::STENCIL_BUFFER_BIT != 0, + }); + } } impl Drop for WebGLRenderingContext { @@ -2727,10 +2738,12 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3 fn StencilMaskSeparate(&self, face: u32, mask: u32) { match face { - constants::FRONT | constants::BACK | constants::FRONT_AND_BACK => + constants::FRONT | + constants::BACK | + constants::FRONT_AND_BACK => self.send_command(WebGLCommand::StencilMaskSeparate(face, mask)), _ => return self.webgl_error(InvalidEnum), - } + }; } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3 From bb8d9ba74c024a317fbbd49811541632404ef51c Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 24 Aug 2018 16:10:28 -0400 Subject: [PATCH 10/13] webgl: Ensure that depth and stencil attachments are rebound after messing with DEPTH_STENCIL attachments. --- components/script/dom/webglframebuffer.rs | 192 +++++++++++++----- components/script/dom/webglrenderbuffer.rs | 44 +++- .../script/dom/webglrenderingcontext.rs | 34 +--- components/script/dom/webgltexture.rs | 43 +++- .../framebuffer-object-attachment.html.ini | 3 - 5 files changed, 233 insertions(+), 83 deletions(-) delete mode 100644 tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index 277b8b2fcba..76be81aee87 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -45,6 +45,15 @@ impl WebGLFramebufferAttachment { WebGLFramebufferAttachment::Texture { .. } => () } } + + fn root(&self) -> WebGLFramebufferAttachmentRoot { + match *self { + WebGLFramebufferAttachment::Renderbuffer(ref rb) => + WebGLFramebufferAttachmentRoot::Renderbuffer(DomRoot::from_ref(&rb)), + WebGLFramebufferAttachment::Texture { ref texture, .. } => + WebGLFramebufferAttachmentRoot::Texture(DomRoot::from_ref(&texture)), + } + } } #[derive(Clone, JSTraceable, MallocSizeOf)] @@ -261,24 +270,16 @@ impl WebGLFramebuffer { } pub fn renderbuffer(&self, attachment: u32, rb: Option<&WebGLRenderbuffer>) -> WebGLResult<()> { - let binding = match attachment { - constants::COLOR_ATTACHMENT0 => &self.color, - constants::DEPTH_ATTACHMENT => &self.depth, - constants::STENCIL_ATTACHMENT => &self.stencil, - constants::DEPTH_STENCIL_ATTACHMENT => &self.depthstencil, - _ => return Err(WebGLError::InvalidEnum), - }; + let binding = self.attachment_binding(attachment).ok_or(WebGLError::InvalidEnum)?; let rb_id = match rb { Some(rb) => { + rb.attach(self); *binding.borrow_mut() = Some(WebGLFramebufferAttachment::Renderbuffer(Dom::from_ref(rb))); Some(rb.id()) } - _ => { - *binding.borrow_mut() = None; - None - } + _ => None }; self.upcast::().context().send_command( @@ -290,39 +291,100 @@ impl WebGLFramebuffer { ), ); + if rb.is_none() { + self.detach_binding(binding, attachment); + } + self.update_status(); self.is_initialized.set(false); Ok(()) } - pub fn attachment(&self, attachment: u32) -> Option { - let binding = match attachment { - constants::COLOR_ATTACHMENT0 => &self.color, - constants::DEPTH_ATTACHMENT => &self.depth, - constants::STENCIL_ATTACHMENT => &self.stencil, - constants::DEPTH_STENCIL_ATTACHMENT => &self.depthstencil, - _ => return None, + fn detach_binding( + &self, + binding: &DomRefCell>, + attachment: u32, + ) { + let attachment_obj = binding.borrow().as_ref().map(WebGLFramebufferAttachment::root); + match attachment_obj { + Some(WebGLFramebufferAttachmentRoot::Renderbuffer(ref rb)) => + rb.unattach(self), + Some(WebGLFramebufferAttachmentRoot::Texture(ref texture)) => + texture.unattach(self), + None => (), + } + *binding.borrow_mut() = None; + + if INTERESTING_ATTACHMENT_POINTS.contains(&attachment) { + self.reattach_depth_stencil(); + } + } + + fn attachment_binding( + &self, + attachment: u32 + ) -> Option<&DomRefCell>> + { + match attachment { + constants::COLOR_ATTACHMENT0 => Some(&self.color), + constants::DEPTH_ATTACHMENT => Some(&self.depth), + constants::STENCIL_ATTACHMENT => Some(&self.stencil), + constants::DEPTH_STENCIL_ATTACHMENT => Some(&self.depthstencil), + _ => None + } + } + + fn reattach_depth_stencil(&self) { + let reattach = |attachment: &WebGLFramebufferAttachment, attachment_point| { + let context = self.upcast::().context(); + match *attachment { + WebGLFramebufferAttachment::Renderbuffer(ref rb) => { + context.send_command( + WebGLCommand::FramebufferRenderbuffer( + constants::FRAMEBUFFER, + attachment_point, + constants::RENDERBUFFER, + Some(rb.id()) + ) + ); + } + WebGLFramebufferAttachment::Texture { ref texture, level } => { + context.send_command( + WebGLCommand::FramebufferTexture2D( + constants::FRAMEBUFFER, + attachment_point, + texture.target().expect("missing texture target"), + Some(texture.id()), + level + ) + ); + } + } }; - binding.borrow().as_ref().map(|bin| { - match bin { - &WebGLFramebufferAttachment::Renderbuffer(ref rb) => - WebGLFramebufferAttachmentRoot::Renderbuffer(DomRoot::from_ref(&rb)), - &WebGLFramebufferAttachment::Texture { ref texture, .. } => - WebGLFramebufferAttachmentRoot::Texture(DomRoot::from_ref(&texture)), - } - }) + // Since the DEPTH_STENCIL attachment causes both the DEPTH and STENCIL + // attachments to be overwritten, we need to ensure that we reattach + // the DEPTH and STENCIL attachments when any of those attachments + // is cleared. + if let Some(ref depth) = *self.depth.borrow() { + reattach(depth, constants::DEPTH_ATTACHMENT); + } + if let Some(ref stencil) = *self.stencil.borrow() { + reattach(stencil, constants::STENCIL_ATTACHMENT); + } + if let Some(ref depth_stencil) = *self.depthstencil.borrow() { + reattach(depth_stencil, constants::DEPTH_STENCIL_ATTACHMENT); + } + } + + pub fn attachment(&self, attachment: u32) -> Option { + let binding = self.attachment_binding(attachment)?; + binding.borrow().as_ref().map(WebGLFramebufferAttachment::root) } pub fn texture2d(&self, attachment: u32, textarget: u32, texture: Option<&WebGLTexture>, level: i32) -> WebGLResult<()> { - let binding = match attachment { - constants::COLOR_ATTACHMENT0 => &self.color, - constants::DEPTH_ATTACHMENT => &self.depth, - constants::STENCIL_ATTACHMENT => &self.stencil, - constants::DEPTH_STENCIL_ATTACHMENT => &self.depthstencil, - _ => return Err(WebGLError::InvalidEnum), - }; + let binding = self.attachment_binding(attachment).ok_or(WebGLError::InvalidEnum)?; let tex_id = match texture { // Note, from the GLES 2.0.25 spec, page 113: @@ -367,6 +429,7 @@ impl WebGLFramebuffer { _ => return Err(WebGLError::InvalidOperation), } + texture.attach(self); *binding.borrow_mut() = Some(WebGLFramebufferAttachment::Texture { texture: Dom::from_ref(texture), level: level } @@ -376,7 +439,6 @@ impl WebGLFramebuffer { } _ => { - *binding.borrow_mut() = None; None } }; @@ -391,20 +453,26 @@ impl WebGLFramebuffer { ), ); + if texture.is_none() { + self.detach_binding(binding, attachment); + } + self.update_status(); self.is_initialized.set(false); Ok(()) } fn with_matching_renderbuffers(&self, rb: &WebGLRenderbuffer, mut closure: F) - where F: FnMut(&DomRefCell>) + where F: FnMut(&DomRefCell>, u32) { - let attachments = [&self.color, - &self.depth, - &self.stencil, - &self.depthstencil]; + let attachments = [ + (&self.color, constants::COLOR_ATTACHMENT0), + (&self.depth, constants::DEPTH_ATTACHMENT), + (&self.stencil, constants::STENCIL_ATTACHMENT), + (&self.depthstencil, constants::DEPTH_STENCIL_ATTACHMENT) + ]; - for attachment in &attachments { + for (attachment, name) in &attachments { let matched = { match *attachment.borrow() { Some(WebGLFramebufferAttachment::Renderbuffer(ref att_rb)) @@ -414,20 +482,22 @@ impl WebGLFramebuffer { }; if matched { - closure(attachment); + closure(attachment, *name); } } } fn with_matching_textures(&self, texture: &WebGLTexture, mut closure: F) - where F: FnMut(&DomRefCell>) + where F: FnMut(&DomRefCell>, u32) { - let attachments = [&self.color, - &self.depth, - &self.stencil, - &self.depthstencil]; + let attachments = [ + (&self.color, constants::COLOR_ATTACHMENT0), + (&self.depth, constants::DEPTH_ATTACHMENT), + (&self.stencil, constants::STENCIL_ATTACHMENT), + (&self.depthstencil, constants::DEPTH_STENCIL_ATTACHMENT) + ]; - for attachment in &attachments { + for (attachment, name) in &attachments { let matched = { match *attachment.borrow() { Some(WebGLFramebufferAttachment::Texture { texture: ref att_texture, .. }) @@ -437,33 +507,45 @@ impl WebGLFramebuffer { }; if matched { - closure(attachment); + closure(attachment, *name); } } } pub fn detach_renderbuffer(&self, rb: &WebGLRenderbuffer) { - self.with_matching_renderbuffers(rb, |att| { + let mut depth_or_stencil_updated = false; + self.with_matching_renderbuffers(rb, |att, name| { + depth_or_stencil_updated |= INTERESTING_ATTACHMENT_POINTS.contains(&name); *att.borrow_mut() = None; self.update_status(); }); + + if depth_or_stencil_updated { + self.reattach_depth_stencil(); + } } pub fn detach_texture(&self, texture: &WebGLTexture) { - self.with_matching_textures(texture, |att| { + let mut depth_or_stencil_updated = false; + self.with_matching_textures(texture, |att, name| { + depth_or_stencil_updated |= INTERESTING_ATTACHMENT_POINTS.contains(&name); *att.borrow_mut() = None; self.update_status(); }); + + if depth_or_stencil_updated { + self.reattach_depth_stencil(); + } } pub fn invalidate_renderbuffer(&self, rb: &WebGLRenderbuffer) { - self.with_matching_renderbuffers(rb, |_att| { + self.with_matching_renderbuffers(rb, |_att, _| { self.update_status(); }); } pub fn invalidate_texture(&self, texture: &WebGLTexture) { - self.with_matching_textures(texture, |_att| { + self.with_matching_textures(texture, |_att, _name| { self.update_status(); }); } @@ -478,3 +560,9 @@ impl Drop for WebGLFramebuffer { self.delete(); } } + +static INTERESTING_ATTACHMENT_POINTS: &[u32] = &[ + constants::DEPTH_ATTACHMENT, + constants::STENCIL_ATTACHMENT, + constants::DEPTH_STENCIL_ATTACHMENT, +]; diff --git a/components/script/dom/webglrenderbuffer.rs b/components/script/dom/webglrenderbuffer.rs index f346407581f..7ee108ebdc5 100644 --- a/components/script/dom/webglrenderbuffer.rs +++ b/components/script/dom/webglrenderbuffer.rs @@ -4,12 +4,14 @@ // https://www.khronos.org/registry/webgl/specs/latest/1.0/webgl.idl use canvas_traits::webgl::{webgl_channel, WebGLCommand, WebGLError, WebGLRenderbufferId, WebGLResult}; +use dom::bindings::cell::DomRefCell; use dom::bindings::codegen::Bindings::WebGL2RenderingContextBinding::WebGL2RenderingContextConstants as WebGl2Constants; use dom::bindings::codegen::Bindings::WebGLRenderbufferBinding; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants; use dom::bindings::inheritance::Castable; use dom::bindings::reflector::{DomObject, reflect_dom_object}; -use dom::bindings::root::DomRoot; +use dom::bindings::root::{DomRoot, Dom}; +use dom::webglframebuffer::WebGLFramebuffer; use dom::webglobject::WebGLObject; use dom::webglrenderingcontext::{WebGLRenderingContext, is_gles}; use dom_struct::dom_struct; @@ -24,6 +26,8 @@ pub struct WebGLRenderbuffer { size: Cell>, internal_format: Cell>, is_initialized: Cell, + // Framebuffer that this texture is attached to. + attached_framebuffers: DomRefCell>>, } impl WebGLRenderbuffer { @@ -36,6 +40,7 @@ impl WebGLRenderbuffer { internal_format: Cell::new(None), size: Cell::new(None), is_initialized: Cell::new(false), + attached_framebuffers: Default::default(), } } @@ -86,6 +91,29 @@ impl WebGLRenderbuffer { pub fn delete(&self) { if !self.is_deleted.get() { self.is_deleted.set(true); + + /* + 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() + .map_or(0, |fb| fb.id().get()); + let current_framebuffer = + self.attached_framebuffers + .borrow() + .iter() + .position(|fb| fb.id().get() == currently_bound_framebuffer); + if let Some(fb_index) = current_framebuffer { + self.attached_framebuffers.borrow()[fb_index].detach_renderbuffer(self); + self.attached_framebuffers.borrow_mut().remove(fb_index); + } + self.upcast::() .context() .send_command(WebGLCommand::DeleteRenderbuffer(self.id)); @@ -145,4 +173,18 @@ impl WebGLRenderbuffer { Ok(()) } + + pub fn attach(&self, framebuffer: &WebGLFramebuffer) { + self.attached_framebuffers.borrow_mut().push(Dom::from_ref(framebuffer)); + } + + pub fn unattach(&self, fb: &WebGLFramebuffer) { + let mut attached_framebuffers = self.attached_framebuffers.borrow_mut(); + let idx = attached_framebuffers.iter().position(|attached| { + attached.id() == fb.id() + }); + if let Some(idx) = idx { + attached_framebuffers.remove(idx); + } + } } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 6ca010a0b6e..19b0c20f260 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -1072,6 +1072,10 @@ impl WebGLRenderingContext { stencil: clear_bits & constants::STENCIL_BUFFER_BIT != 0, }); } + + pub fn bound_framebuffer(&self) -> Option> { + self.bound_framebuffer.get() + } } impl Drop for WebGLRenderingContext { @@ -1587,6 +1591,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { renderbuffer.bind(target); } _ => { + if renderbuffer.is_some() { + self.webgl_error(InvalidOperation); + } + self.bound_renderbuffer.set(None); // Unbind the currently bound renderbuffer self.send_command(WebGLCommand::BindRenderbuffer(target, None)); @@ -1986,20 +1994,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { handle_potential_webgl_error!(self, self.validate_ownership(renderbuffer), return); handle_object_deletion!(self, self.bound_renderbuffer, renderbuffer, Some(WebGLCommand::BindRenderbuffer(constants::RENDERBUFFER, None))); - // From the GLES 2.0.25 spec, page 113: - // - // "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." - // - if let Some(fb) = self.bound_framebuffer.get() { - fb.detach_renderbuffer(renderbuffer); - } - renderbuffer.delete() } } @@ -2035,18 +2029,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { )); } - // From the GLES 2.0.25 spec, page 113: - // - // "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." - if let Some(fb) = self.bound_framebuffer.get() { - fb.detach_texture(texture); - } - texture.delete() } } diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index 83378519848..4d085d51442 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -12,8 +12,9 @@ use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderi use dom::bindings::codegen::Bindings::WebGLTextureBinding; use dom::bindings::inheritance::Castable; use dom::bindings::reflector::{DomObject, reflect_dom_object}; -use dom::bindings::root::DomRoot; +use dom::bindings::root::{Dom, DomRoot}; use dom::webgl_validations::types::{TexImageTarget, TexFormat, TexDataType}; +use dom::webglframebuffer::WebGLFramebuffer; use dom::webglobject::WebGLObject; use dom::webglrenderingcontext::WebGLRenderingContext; use dom_struct::dom_struct; @@ -48,6 +49,8 @@ pub struct WebGLTexture { mag_filter: Cell, /// True if this texture is used for the DOMToTexture feature. attached_to_dom: Cell, + // Framebuffer that this texture is attached to. + attached_framebuffers: DomRefCell>>, } impl WebGLTexture { @@ -63,6 +66,7 @@ impl WebGLTexture { mag_filter: Cell::new(constants::LINEAR), image_info_array: DomRefCell::new([ImageInfo::new(); MAX_LEVEL_COUNT * MAX_FACE_COUNT]), attached_to_dom: Cell::new(false), + attached_framebuffers: Default::default(), } } @@ -186,6 +190,29 @@ impl WebGLTexture { DOMToTextureCommand::Detach(self.id), ); } + + /* + 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() + .map_or(0, |fb| fb.id().get()); + let current_framebuffer = + self.attached_framebuffers + .borrow() + .iter() + .position(|fb| fb.id().get() == currently_bound_framebuffer); + if let Some(fb_index) = current_framebuffer { + self.attached_framebuffers.borrow()[fb_index].detach_texture(self); + self.attached_framebuffers.borrow_mut().remove(fb_index); + } + context.send_command(WebGLCommand::DeleteTexture(self.id)); } } @@ -398,6 +425,20 @@ impl WebGLTexture { pub fn set_attached_to_dom(&self) { self.attached_to_dom.set(true); } + + pub fn attach(&self, framebuffer: &WebGLFramebuffer) { + self.attached_framebuffers.borrow_mut().push(Dom::from_ref(framebuffer)); + } + + pub fn unattach(&self, fb: &WebGLFramebuffer) { + let mut attached_framebuffers = self.attached_framebuffers.borrow_mut(); + let idx = attached_framebuffers.iter().position(|attached| { + attached.id() == fb.id() + }); + if let Some(idx) = idx { + attached_framebuffers.remove(idx); + } + } } impl Drop for WebGLTexture { diff --git a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini b/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini deleted file mode 100644 index b0777f21743..00000000000 --- a/tests/wpt/webgl/meta/conformance/renderbuffers/framebuffer-object-attachment.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[framebuffer-object-attachment.html] - [WebGL test #139: at (0, 0) expected: 0,255,0,255 was 255,0,0,255] - expected: fail From 8ede221a83bf751eea8520b3431c1f222ecfbdea Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 31 Aug 2018 10:16:57 -0400 Subject: [PATCH 11/13] webgl: Ensure that renderbuffers have been bound before attaching to a framebuffer. --- components/script/dom/webglframebuffer.rs | 2 +- components/script/dom/webglrenderbuffer.rs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index 76be81aee87..b79393a7863 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -274,7 +274,7 @@ impl WebGLFramebuffer { let rb_id = match rb { Some(rb) => { - rb.attach(self); + rb.attach(self)?; *binding.borrow_mut() = Some(WebGLFramebufferAttachment::Renderbuffer(Dom::from_ref(rb))); Some(rb.id()) } diff --git a/components/script/dom/webglrenderbuffer.rs b/components/script/dom/webglrenderbuffer.rs index 7ee108ebdc5..9637dae02e1 100644 --- a/components/script/dom/webglrenderbuffer.rs +++ b/components/script/dom/webglrenderbuffer.rs @@ -174,8 +174,12 @@ impl WebGLRenderbuffer { Ok(()) } - pub fn attach(&self, framebuffer: &WebGLFramebuffer) { + pub fn attach(&self, framebuffer: &WebGLFramebuffer) -> WebGLResult<()> { + if !self.ever_bound.get() { + return Err(WebGLError::InvalidOperation); + } self.attached_framebuffers.borrow_mut().push(Dom::from_ref(framebuffer)); + Ok(()) } pub fn unattach(&self, fb: &WebGLFramebuffer) { From b8ee62e67bae0ffda558e7e9d819257265f5a16f Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Fri, 31 Aug 2018 12:31:15 -0400 Subject: [PATCH 12/13] webgl: Update test results for newly-exposed missing checks. --- .../webgl-specific-stencil-settings.html.ini | 97 ++++++++++++++++++- .../misc/copy-tex-image-2d-formats.html.ini | 9 ++ .../framebuffer-object-attachment.html.ini | 3 + .../framebuffer-texture-level1.html.ini | 8 +- 4 files changed, 112 insertions(+), 5 deletions(-) diff --git a/tests/wpt/webgl/meta/conformance/misc/webgl-specific-stencil-settings.html.ini b/tests/wpt/webgl/meta/conformance/misc/webgl-specific-stencil-settings.html.ini index 11bb2398b5f..73e988e891a 100644 --- a/tests/wpt/webgl/meta/conformance/misc/webgl-specific-stencil-settings.html.ini +++ b/tests/wpt/webgl/meta/conformance/misc/webgl-specific-stencil-settings.html.ini @@ -1,2 +1,97 @@ [webgl-specific-stencil-settings.html] - expected: CRASH + [WebGL test #623: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #642: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #565: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #611: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #555: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #629: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #513: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #584: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #578: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #620: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #614: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #650: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #488: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #543: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #501: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #590: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #2: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #534: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #632: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #494: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #626: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #537: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #658: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #507: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #571: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #5: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #546: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #549: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #667: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #540: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #552: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + + [WebGL test #617: getError expected: INVALID_OPERATION. Was NO_ERROR : after evaluating: wtu.dummySetProgramAndDrawNothing(gl)] + expected: FAIL + diff --git a/tests/wpt/webgl/meta/conformance/textures/misc/copy-tex-image-2d-formats.html.ini b/tests/wpt/webgl/meta/conformance/textures/misc/copy-tex-image-2d-formats.html.ini index 92f1bc69eed..35a18d4d5da 100644 --- a/tests/wpt/webgl/meta/conformance/textures/misc/copy-tex-image-2d-formats.html.ini +++ b/tests/wpt/webgl/meta/conformance/textures/misc/copy-tex-image-2d-formats.html.ini @@ -26,3 +26,12 @@ [WebGL test #16: Creating framebuffer from ALPHA texture succeeded even though it is not a renderable format] expected: FAIL + [WebGL test #32: getError expected: INVALID_OPERATION. Was NO_ERROR : should not be able to copyTexImage2D ALPHA from RGB] + expected: FAIL + + [WebGL test #40: getError expected: INVALID_OPERATION. Was NO_ERROR : should not be able to copyTexImage2D RGBA from RGB] + expected: FAIL + + [WebGL test #36: getError expected: INVALID_OPERATION. Was NO_ERROR : should not be able to copyTexImage2D LUMINANCE_ALPHA from RGB] + expected: FAIL + diff --git a/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini b/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini index 256dc0faeea..4f295cc0721 100644 --- a/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini +++ b/tests/wpt/webgl/meta/conformance2/renderbuffers/framebuffer-object-attachment.html.ini @@ -29,3 +29,6 @@ [WebGL test #16: gl.getParameter(gl.RED_BITS) + gl.getParameter(gl.GREEN_BITS) + gl.getParameter(gl.BLUE_BITS) + gl.getParameter(gl.ALPHA_BITS) >= 16 should be true. Was false.] expected: FAIL + [WebGL test #59: getError expected: NO_ERROR. Was INVALID_ENUM : Query should not generate error] + expected: FAIL + diff --git a/tests/wpt/webgl/meta/conformance2/rendering/framebuffer-texture-level1.html.ini b/tests/wpt/webgl/meta/conformance2/rendering/framebuffer-texture-level1.html.ini index cb4a718b2b5..134a7641a46 100644 --- a/tests/wpt/webgl/meta/conformance2/rendering/framebuffer-texture-level1.html.ini +++ b/tests/wpt/webgl/meta/conformance2/rendering/framebuffer-texture-level1.html.ini @@ -1,10 +1,10 @@ [framebuffer-texture-level1.html] - [WebGL test #1: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36054. Was 36061.] - expected: FAIL - [WebGL test #3: getError expected: NO_ERROR. Was INVALID_VALUE : Setup framebuffer with texture should succeed.] expected: FAIL - [WebGL test #2: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36061.] + [WebGL test #2: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36055.] + expected: FAIL + + [WebGL test #1: gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36054. Was 36055.] expected: FAIL From 4edb7b194c99a1d394dddaeac2f5064ed8e93f62 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Mon, 10 Sep 2018 16:27:58 -0400 Subject: [PATCH 13/13] webgl: Remove knowledge of attached framebuffers from renderbuffers and textures. --- components/script/dom/webglframebuffer.rs | 14 ++------ components/script/dom/webglrenderbuffer.rs | 38 +++------------------- components/script/dom/webgltexture.rs | 33 +++---------------- 3 files changed, 11 insertions(+), 74 deletions(-) diff --git a/components/script/dom/webglframebuffer.rs b/components/script/dom/webglframebuffer.rs index b79393a7863..d785d0b99fa 100644 --- a/components/script/dom/webglframebuffer.rs +++ b/components/script/dom/webglframebuffer.rs @@ -274,7 +274,9 @@ impl WebGLFramebuffer { let rb_id = match rb { Some(rb) => { - rb.attach(self)?; + if !rb.ever_bound() { + return Err(WebGLError::InvalidOperation); + } *binding.borrow_mut() = Some(WebGLFramebufferAttachment::Renderbuffer(Dom::from_ref(rb))); Some(rb.id()) } @@ -305,16 +307,7 @@ impl WebGLFramebuffer { binding: &DomRefCell>, attachment: u32, ) { - let attachment_obj = binding.borrow().as_ref().map(WebGLFramebufferAttachment::root); - match attachment_obj { - Some(WebGLFramebufferAttachmentRoot::Renderbuffer(ref rb)) => - rb.unattach(self), - Some(WebGLFramebufferAttachmentRoot::Texture(ref texture)) => - texture.unattach(self), - None => (), - } *binding.borrow_mut() = None; - if INTERESTING_ATTACHMENT_POINTS.contains(&attachment) { self.reattach_depth_stencil(); } @@ -429,7 +422,6 @@ impl WebGLFramebuffer { _ => return Err(WebGLError::InvalidOperation), } - texture.attach(self); *binding.borrow_mut() = Some(WebGLFramebufferAttachment::Texture { texture: Dom::from_ref(texture), level: level } diff --git a/components/script/dom/webglrenderbuffer.rs b/components/script/dom/webglrenderbuffer.rs index 9637dae02e1..96ef3bb0d67 100644 --- a/components/script/dom/webglrenderbuffer.rs +++ b/components/script/dom/webglrenderbuffer.rs @@ -4,14 +4,12 @@ // https://www.khronos.org/registry/webgl/specs/latest/1.0/webgl.idl use canvas_traits::webgl::{webgl_channel, WebGLCommand, WebGLError, WebGLRenderbufferId, WebGLResult}; -use dom::bindings::cell::DomRefCell; use dom::bindings::codegen::Bindings::WebGL2RenderingContextBinding::WebGL2RenderingContextConstants as WebGl2Constants; use dom::bindings::codegen::Bindings::WebGLRenderbufferBinding; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants; use dom::bindings::inheritance::Castable; use dom::bindings::reflector::{DomObject, reflect_dom_object}; -use dom::bindings::root::{DomRoot, Dom}; -use dom::webglframebuffer::WebGLFramebuffer; +use dom::bindings::root::DomRoot; use dom::webglobject::WebGLObject; use dom::webglrenderingcontext::{WebGLRenderingContext, is_gles}; use dom_struct::dom_struct; @@ -26,8 +24,6 @@ pub struct WebGLRenderbuffer { size: Cell>, internal_format: Cell>, is_initialized: Cell, - // Framebuffer that this texture is attached to. - attached_framebuffers: DomRefCell>>, } impl WebGLRenderbuffer { @@ -40,7 +36,6 @@ impl WebGLRenderbuffer { internal_format: Cell::new(None), size: Cell::new(None), is_initialized: Cell::new(false), - attached_framebuffers: Default::default(), } } @@ -102,16 +97,9 @@ impl WebGLRenderbuffer { let currently_bound_framebuffer = self.upcast::() .context() - .bound_framebuffer() - .map_or(0, |fb| fb.id().get()); - let current_framebuffer = - self.attached_framebuffers - .borrow() - .iter() - .position(|fb| fb.id().get() == currently_bound_framebuffer); - if let Some(fb_index) = current_framebuffer { - self.attached_framebuffers.borrow()[fb_index].detach_renderbuffer(self); - self.attached_framebuffers.borrow_mut().remove(fb_index); + .bound_framebuffer(); + if let Some(fb) = currently_bound_framebuffer { + fb.detach_renderbuffer(self); } self.upcast::() @@ -173,22 +161,4 @@ impl WebGLRenderbuffer { Ok(()) } - - pub fn attach(&self, framebuffer: &WebGLFramebuffer) -> WebGLResult<()> { - if !self.ever_bound.get() { - return Err(WebGLError::InvalidOperation); - } - self.attached_framebuffers.borrow_mut().push(Dom::from_ref(framebuffer)); - Ok(()) - } - - pub fn unattach(&self, fb: &WebGLFramebuffer) { - let mut attached_framebuffers = self.attached_framebuffers.borrow_mut(); - let idx = attached_framebuffers.iter().position(|attached| { - attached.id() == fb.id() - }); - if let Some(idx) = idx { - attached_framebuffers.remove(idx); - } - } } diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index 4d085d51442..4d8e58380f7 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -12,9 +12,8 @@ use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderi use dom::bindings::codegen::Bindings::WebGLTextureBinding; use dom::bindings::inheritance::Castable; use dom::bindings::reflector::{DomObject, reflect_dom_object}; -use dom::bindings::root::{Dom, DomRoot}; +use dom::bindings::root::DomRoot; use dom::webgl_validations::types::{TexImageTarget, TexFormat, TexDataType}; -use dom::webglframebuffer::WebGLFramebuffer; use dom::webglobject::WebGLObject; use dom::webglrenderingcontext::WebGLRenderingContext; use dom_struct::dom_struct; @@ -49,8 +48,6 @@ pub struct WebGLTexture { mag_filter: Cell, /// True if this texture is used for the DOMToTexture feature. attached_to_dom: Cell, - // Framebuffer that this texture is attached to. - attached_framebuffers: DomRefCell>>, } impl WebGLTexture { @@ -66,7 +63,6 @@ impl WebGLTexture { mag_filter: Cell::new(constants::LINEAR), image_info_array: DomRefCell::new([ImageInfo::new(); MAX_LEVEL_COUNT * MAX_FACE_COUNT]), attached_to_dom: Cell::new(false), - attached_framebuffers: Default::default(), } } @@ -201,16 +197,9 @@ impl WebGLTexture { let currently_bound_framebuffer = self.upcast::() .context() - .bound_framebuffer() - .map_or(0, |fb| fb.id().get()); - let current_framebuffer = - self.attached_framebuffers - .borrow() - .iter() - .position(|fb| fb.id().get() == currently_bound_framebuffer); - if let Some(fb_index) = current_framebuffer { - self.attached_framebuffers.borrow()[fb_index].detach_texture(self); - self.attached_framebuffers.borrow_mut().remove(fb_index); + .bound_framebuffer(); + if let Some(fb) = currently_bound_framebuffer { + fb.detach_texture(self); } context.send_command(WebGLCommand::DeleteTexture(self.id)); @@ -425,20 +414,6 @@ impl WebGLTexture { pub fn set_attached_to_dom(&self) { self.attached_to_dom.set(true); } - - pub fn attach(&self, framebuffer: &WebGLFramebuffer) { - self.attached_framebuffers.borrow_mut().push(Dom::from_ref(framebuffer)); - } - - pub fn unattach(&self, fb: &WebGLFramebuffer) { - let mut attached_framebuffers = self.attached_framebuffers.borrow_mut(); - let idx = attached_framebuffers.iter().position(|attached| { - attached.id() == fb.id() - }); - if let Some(idx) = idx { - attached_framebuffers.remove(idx); - } - } } impl Drop for WebGLTexture {