From 87e112dea79e0fbc87f423c285160e48621e3480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 6 Apr 2016 23:37:02 +0200 Subject: [PATCH 01/12] webgl: Implement the pending texImage2D overload, and add more validation This is a large-ish refactor of the Texture2D code, but it should be easier to read and of course more correct. I tried to annotate every error condition with a spec paragraph. --- .../script/dom/webglrenderingcontext.rs | 313 ++++++++++++++++-- .../dom/webidls/WebGLRenderingContext.webidl | 4 + .../more/functions/texImage2D.html.ini | 5 - 3 files changed, 288 insertions(+), 34 deletions(-) delete mode 100644 tests/wpt/metadata/webgl/conformance-1.0.3/conformance/more/functions/texImage2D.html.ini diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 197dcce55f5..710b61fc162 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -8,7 +8,8 @@ use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderi use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{WebGLRenderingContextMethods}; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{self, WebGLContextAttributes}; use dom::bindings::codegen::UnionTypes::ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement; -use dom::bindings::conversions::{ToJSValConvertible, array_buffer_view_to_vec_checked, array_buffer_view_to_vec}; +use dom::bindings::conversions::{ToJSValConvertible, array_buffer_view_data_checked}; +use dom::bindings::conversions::{array_buffer_view_to_vec_checked, array_buffer_view_to_vec}; use dom::bindings::global::GlobalRef; use dom::bindings::inheritance::Castable; use dom::bindings::js::{JS, LayoutJS, MutNullableHeap, Root}; @@ -271,6 +272,181 @@ impl WebGLRenderingContext { return true; } + + fn validate_tex_image_parameters(&self, + target: u32, + level: i32, + internal_format: u32, + width: i32, + height: i32, + border: i32, + format: u32, + data_type: u32) -> bool { + // GL_INVALID_ENUM is generated if target is not GL_TEXTURE_2D, + // GL_TEXTURE_CUBE_MAP_POSITIVE_X, GL_TEXTURE_CUBE_MAP_NEGATIVE_X, + // GL_TEXTURE_CUBE_MAP_POSITIVE_Y, GL_TEXTURE_CUBE_MAP_NEGATIVE_Y, + // GL_TEXTURE_CUBE_MAP_POSITIVE_Z, or GL_TEXTURE_CUBE_MAP_NEGATIVE_Z. + let texture = match target { + constants::TEXTURE_2D + => self.bound_texture_2d.get(), + constants::TEXTURE_CUBE_MAP_POSITIVE_X | + constants::TEXTURE_CUBE_MAP_NEGATIVE_X | + constants::TEXTURE_CUBE_MAP_POSITIVE_Y | + constants::TEXTURE_CUBE_MAP_NEGATIVE_Y | + constants::TEXTURE_CUBE_MAP_POSITIVE_Z | + constants::TEXTURE_CUBE_MAP_NEGATIVE_Z + => self.bound_texture_cube_map.get(), + _ => { + self.webgl_error(InvalidEnum); + return false; + }, + }; + + // If an attempt is made to call this function with no + // WebGLTexture bound, an INVALID_OPERATION error is generated. + if texture.is_none() { + self.webgl_error(InvalidOperation); + return false; + } + + // GL_INVALID_ENUM is generated if data_type is not an accepted value. + match data_type { + constants::UNSIGNED_BYTE | + constants::UNSIGNED_SHORT_4_4_4_4 | + constants::UNSIGNED_SHORT_5_5_5_1 | + constants::UNSIGNED_SHORT_5_6_5 => {}, + _ => { + self.webgl_error(InvalidEnum); + return false; + }, + } + + + // TODO(emilio): GL_INVALID_VALUE may be generated if + // level is greater than log_2(max), where max is + // the returned value of GL_MAX_TEXTURE_SIZE when + // target is GL_TEXTURE_2D or GL_MAX_CUBE_MAP_TEXTURE_SIZE + // when target is not GL_TEXTURE_2D. + let is_cubic = target != constants::TEXTURE_2D; + + // GL_INVALID_VALUE is generated if target is one of the + // six cube map 2D image targets and the width and height + // parameters are not equal. + if is_cubic && width != height { + self.webgl_error(InvalidValue); + return false; + } + + // GL_INVALID_VALUE is generated if internal_format is not an + // accepted format. + match internal_format { + constants::DEPTH_COMPONENT | + constants::ALPHA | + constants::RGB | + constants::RGBA | + constants::LUMINANCE | + constants::LUMINANCE_ALPHA => {}, + + _ => { + self.webgl_error(InvalidValue); + return false; + }, + } + + // GL_INVALID_OPERATION is generated if format does not + // match internal_format. + if format != internal_format { + self.webgl_error(InvalidOperation); + return false; + } + + // GL_INVALID_VALUE is generated if level is less than 0. + // + // GL_INVALID_VALUE is generated if width or height is less than 0 + // or greater than GL_MAX_TEXTURE_SIZE when target is GL_TEXTURE_2D or + // GL_MAX_CUBE_MAP_TEXTURE_SIZE when target is not GL_TEXTURE_2D. + // + // TODO(emilio): Check limits + if width < 0 || height < 0 || level < 0 { + self.webgl_error(InvalidValue); + return false; + } + + // GL_INVALID_VALUE is generated if border is not 0. + if border != 0 { + self.webgl_error(InvalidValue); + return false; + } + + // GL_INVALID_OPERATION is generated if type is GL_UNSIGNED_SHORT_4_4_4_4 or + // GL_UNSIGNED_SHORT_5_5_5_1 and format is not GL_RGBA. + // + // GL_INVALID_OPERATION is generated if type is + // GL_UNSIGNED_SHORT_5_6_5 and format is not GL_RGB. + match data_type { + constants::UNSIGNED_SHORT_4_4_4_4 | + constants::UNSIGNED_SHORT_5_5_5_1 if format != constants::RGBA => { + self.webgl_error(InvalidOperation); + return false; + }, + constants::UNSIGNED_SHORT_5_6_5 if format != constants::RGB => { + self.webgl_error(InvalidOperation); + return false; + }, + _ => {}, + } + + true + } + + fn tex_image_2d(&self, + target: u32, + level: i32, + internal_format: u32, + width: i32, + height: i32, + border: i32, + format: u32, + data_type: u32, + pixels: Vec) { // NB: pixels should NOT be premultipied + // This should be validated before reaching this function + debug_assert!(self.validate_tex_image_parameters(target, level, + internal_format, + width, height, + border, format, + data_type)); + + let slot = match target { + constants::TEXTURE_2D + => self.bound_texture_2d.get(), + _ => self.bound_texture_cube_map.get(), + }; + + let texture = slot.as_ref().expect("No bound texture found after validation"); + + if format == constants::RGBA && + data_type == constants::UNSIGNED_BYTE && + self.texture_unpacking_settings.get().contains(PREMULTIPLY_ALPHA) { + // TODO(emilio): premultiply here. + } + + // TODO(emilio): Flip Y axis if necessary here + + // TexImage2D depth is always equal to 1 + handle_potential_webgl_error!(self, texture.initialize(width as u32, + height as u32, 1, + internal_format, + level as u32)); + + + // TODO(emilio): Invert axis, convert colorspace, premultiply alpha if requested + let msg = WebGLCommand::TexImage2D(target, level, internal_format as i32, + width, height, format, data_type, pixels); + + self.ipc_renderer + .send(CanvasMsg::WebGL(msg)) + .unwrap() + } } impl Drop for WebGLRenderingContext { @@ -526,7 +702,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { let slot = match target { constants::TEXTURE_2D => &self.bound_texture_2d, constants::TEXTURE_CUBE_MAP => &self.bound_texture_cube_map, - _ => return self.webgl_error(InvalidEnum), }; @@ -1402,28 +1577,112 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { .unwrap() } + #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 fn TexImage2D(&self, + _cx: *mut JSContext, target: u32, level: i32, internal_format: u32, + width: i32, + height: i32, + border: i32, format: u32, data_type: u32, - source: Option) { - let texture = match target { - constants::TEXTURE_2D => self.bound_texture_2d.get(), - constants::TEXTURE_CUBE_MAP => self.bound_texture_cube_map.get(), - _ => return self.webgl_error(InvalidEnum), + data: Option<*mut JSObject>) { + if !self.validate_tex_image_parameters(target, + level, + internal_format, + width, height, + border, + format, + data_type) { + return; // Error handled in validate() + } + + // TODO(emilio, #10693): Add type-safe wrappers to validations + let (element_size, components_per_element) = match data_type { + constants::UNSIGNED_BYTE => (1, 1), + constants::UNSIGNED_SHORT_5_6_5 => (2, 3), + constants::UNSIGNED_SHORT_5_5_5_1 | + constants::UNSIGNED_SHORT_4_4_4_4 => (2, 4), + _ => unreachable!(), // previously validated }; - if texture.is_none() { + + let components = match format { + constants::DEPTH_COMPONENT => 1, + constants::ALPHA => 1, + constants::LUMINANCE => 1, + constants::LUMINANCE_ALPHA => 2, + constants::RGB => 3, + constants::RGBA => 4, + _ => unreachable!(), // previously validated + }; + + // If data is non-null, the type of pixels must match the type of the + // data to be read. + // If it is UNSIGNED_BYTE, a Uint8Array must be supplied; + // if it is UNSIGNED_SHORT_5_6_5, UNSIGNED_SHORT_4_4_4_4, + // or UNSIGNED_SHORT_5_5_5_1, a Uint16Array must be supplied. + // If the types do not match, an INVALID_OPERATION error is generated. + let received_size = if let Some(data) = data { + if unsafe { array_buffer_view_data_checked::(data).is_some() } { + 2 + } else if unsafe { array_buffer_view_data_checked::(data).is_some() } { + 1 + } else { + return self.webgl_error(InvalidOperation); + } + } else { + element_size + }; + + if received_size != element_size { return self.webgl_error(InvalidOperation); } - // TODO(emilio): Validate more parameters + + // NOTE: width and height are positive or zero due to validate() + let expected_byte_length = width * height * element_size * components / components_per_element; + + + // If data is null, a buffer of sufficient size + // initialized to 0 is passed. + let buff = if let Some(data) = data { + array_buffer_view_to_vec::(data) + .expect("Can't reach here without being an ArrayBufferView!") + } else { + vec![0u8; expected_byte_length as usize] + }; + + if buff.len() != expected_byte_length as usize { + return self.webgl_error(InvalidOperation); + } + + self.tex_image_2d(target, level, + internal_format, + width, height, border, + format, data_type, buff) + } + + // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 + fn TexImage2D_(&self, + target: u32, + level: i32, + internal_format: u32, + format: u32, + data_type: u32, + source: Option) { let source = match source { Some(s) => s, None => return, }; + + // NOTE: Getting the pixels probably can be short-circuited if some + // parameter is invalid. + // + // Nontheless, since it's the error case, I'm not totally sure the + // complexity is worth it. let (pixels, size) = match source { ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement::ImageData(image_data) => { let global = self.global(); @@ -1445,7 +1704,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { }; let size = Size2D::new(img.width as i32, img.height as i32); - // TODO(emilio): Validate that the format argument is coherent with the image. + + // TODO(emilio): Validate that the format argument + // is coherent with the image. + // // RGB8 should be easy to support too let mut data = match img.format { PixelFormat::RGBA8 => img.bytes.to_vec(), @@ -1456,8 +1718,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { (data, size) }, - // TODO(emilio): Getting canvas data is implemented in CanvasRenderingContext2D, but - // we need to refactor it moving it to `HTMLCanvasElement` and supporting WebGLContext + // TODO(emilio): Getting canvas data is implemented in CanvasRenderingContext2D, + // but we need to refactor it moving it to `HTMLCanvasElement` and support + // WebGLContext (probably via GetPixels()). ImageDataOrHTMLImageElementOrHTMLCanvasElementOrHTMLVideoElement::HTMLCanvasElement(canvas) => { let canvas = canvas.r(); if let Some((mut data, size)) = canvas.fetch_all_data() { @@ -1471,25 +1734,17 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { => unimplemented!(), }; - if size.width < 0 || size.height < 0 || level < 0 { - self.webgl_error(WebGLError::InvalidOperation); + // NB: Border must be zero + if !self.validate_tex_image_parameters(target, level, internal_format, + size.width, size.height, 0, + format, data_type) { + return; // Error handled in validate() } - // TODO(emilio): Invert axis, convert colorspace, premultiply alpha if requested - let msg = WebGLCommand::TexImage2D(target, level, internal_format as i32, - size.width, size.height, - format, data_type, pixels); - - // depth is always 1 when coming from html elements - handle_potential_webgl_error!(self, texture.unwrap().initialize(size.width as u32, - size.height as u32, - 1, - internal_format, - level as u32)); - - self.ipc_renderer - .send(CanvasMsg::WebGL(msg)) - .unwrap() + self.tex_image_2d(target, level, + internal_format, + size.width, size.height, 0, + format, data_type, pixels) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8 diff --git a/components/script/dom/webidls/WebGLRenderingContext.webidl b/components/script/dom/webidls/WebGLRenderingContext.webidl index 6c36844f313..97cd853417f 100644 --- a/components/script/dom/webidls/WebGLRenderingContext.webidl +++ b/components/script/dom/webidls/WebGLRenderingContext.webidl @@ -632,6 +632,10 @@ interface WebGLRenderingContextBase //void texImage2D(GLenum target, GLint level, GLenum internalformat, // GLsizei width, GLsizei height, GLint border, GLenum format, // GLenum type, ArrayBufferView? pixels); + // FIXME: SM interface arguments + void texImage2D(GLenum target, GLint level, GLenum internalformat, + GLsizei width, GLsizei height, GLint border, GLenum format, + GLenum type, optional object data); void texImage2D(GLenum target, GLint level, GLenum internalformat, GLenum format, GLenum type, TexImageSource? source); // May throw DOMException diff --git a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/more/functions/texImage2D.html.ini b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/more/functions/texImage2D.html.ini deleted file mode 100644 index 8f198e94924..00000000000 --- a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/more/functions/texImage2D.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[texImage2D.html] - type: testharness - [WebGL test #0: testTexImage2D] - expected: FAIL - From 5eb59935e3dcd1be1ebb41c8fadf449473bc28c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 7 Apr 2016 00:19:19 +0200 Subject: [PATCH 02/12] webgl: Add test for Texture2d's ArrayBufferView overload --- tests/wpt/mozilla/meta/MANIFEST.json | 24 ++++ .../tests/mozilla/webgl/tex_image_2d_abv.html | 106 ++++++++++++++++++ .../mozilla/webgl/tex_image_2d_abv_ref.html | 13 +++ 3 files changed, 143 insertions(+) create mode 100644 tests/wpt/mozilla/tests/mozilla/webgl/tex_image_2d_abv.html create mode 100644 tests/wpt/mozilla/tests/mozilla/webgl/tex_image_2d_abv_ref.html diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 1cadcfe1765..4076bb8652a 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -5803,6 +5803,18 @@ "url": "/_mozilla/mozilla/webgl/draw_arrays_simple.html" } ], + "mozilla/webgl/tex_image_2d_abv.html": [ + { + "path": "mozilla/webgl/tex_image_2d_abv.html", + "references": [ + [ + "/_mozilla/mozilla/webgl/tex_image_2d_abv_ref.html", + "==" + ] + ], + "url": "/_mozilla/mozilla/webgl/tex_image_2d_abv.html" + } + ], "mozilla/webgl/tex_image_2d_canvas.html": [ { "path": "mozilla/webgl/tex_image_2d_canvas.html", @@ -12465,6 +12477,18 @@ "url": "/_mozilla/mozilla/webgl/draw_arrays_simple.html" } ], + "mozilla/webgl/tex_image_2d_abv.html": [ + { + "path": "mozilla/webgl/tex_image_2d_abv.html", + "references": [ + [ + "/_mozilla/mozilla/webgl/tex_image_2d_abv_ref.html", + "==" + ] + ], + "url": "/_mozilla/mozilla/webgl/tex_image_2d_abv.html" + } + ], "mozilla/webgl/tex_image_2d_canvas.html": [ { "path": "mozilla/webgl/tex_image_2d_canvas.html", diff --git a/tests/wpt/mozilla/tests/mozilla/webgl/tex_image_2d_abv.html b/tests/wpt/mozilla/tests/mozilla/webgl/tex_image_2d_abv.html new file mode 100644 index 00000000000..bfa6019a218 --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/webgl/tex_image_2d_abv.html @@ -0,0 +1,106 @@ + + +WebGL: texImage2D with Array Buffer View + + + + + + + + diff --git a/tests/wpt/mozilla/tests/mozilla/webgl/tex_image_2d_abv_ref.html b/tests/wpt/mozilla/tests/mozilla/webgl/tex_image_2d_abv_ref.html new file mode 100644 index 00000000000..1e84eb7846c --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/webgl/tex_image_2d_abv_ref.html @@ -0,0 +1,13 @@ + + +ref: WebGL: texImage2D with Array Buffer View + + +
From c807cab3001b1676a8f9c47811629f621027e8b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Apr 2016 13:09:06 +0200 Subject: [PATCH 03/12] webgl: Validate that the texture should be power of two if the level is greater than 1 --- components/script/dom/webglrenderingcontext.rs | 18 ++++++++++++++---- components/script/dom/webgltexture.rs | 4 ++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 710b61fc162..408558d3031 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -304,10 +304,13 @@ impl WebGLRenderingContext { // If an attempt is made to call this function with no // WebGLTexture bound, an INVALID_OPERATION error is generated. - if texture.is_none() { - self.webgl_error(InvalidOperation); - return false; - } + let texture = match texture { + Some(texture) => texture, + None => { + self.webgl_error(InvalidOperation); + return false; + } + }; // GL_INVALID_ENUM is generated if data_type is not an accepted value. match data_type { @@ -372,6 +375,13 @@ impl WebGLRenderingContext { return false; } + // GL_INVALID_VALUE is generated if level is greater than zero and the + // texture and the texture is not power of two. + if level > 0 && !texture.is_power_of_two() { + self.webgl_error(InvalidValue); + return false; + } + // GL_INVALID_VALUE is generated if border is not 0. if border != 0 { self.webgl_error(InvalidValue); diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index 5bb7d5ff734..f39fa9b5789 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -227,6 +227,10 @@ impl WebGLTexture { } } + pub fn is_power_of_two(&self) -> bool { + self.image_info_at_face(0, 0).is_power_of_two() + } + pub fn populate_mip_chain(&self, first_level: u32, last_level: u32) -> WebGLResult<()> { let base_image_info = self.image_info_at_face(0, first_level); if !base_image_info.is_initialized() { From f470ad0d884f50247f4846a8271e67616ffc7354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Apr 2016 13:10:09 +0200 Subject: [PATCH 04/12] webgl: Refactor WebGLProgram::link This makes it more correct, since we don't blindly send the Link command. It's not observable though. --- components/script/dom/webglprogram.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/components/script/dom/webglprogram.rs b/components/script/dom/webglprogram.rs index d288e9422c4..28cac1ed11b 100644 --- a/components/script/dom/webglprogram.rs +++ b/components/script/dom/webglprogram.rs @@ -23,6 +23,7 @@ pub struct WebGLProgram { webgl_object: WebGLObject, id: u32, is_deleted: Cell, + linked: Cell, fragment_shader: MutNullableHeap>, vertex_shader: MutNullableHeap>, #[ignore_heap_size_of = "Defined in ipc-channel"] @@ -35,6 +36,7 @@ impl WebGLProgram { webgl_object: WebGLObject::new_inherited(), id: id, is_deleted: Cell::new(false), + linked: Cell::new(false), fragment_shader: Default::default(), vertex_shader: Default::default(), renderer: renderer, @@ -71,19 +73,27 @@ impl WebGLProgram { /// glLinkProgram pub fn link(&self) { + self.linked.set(false); + + match self.fragment_shader.get() { + Some(ref shader) if shader.successfully_compiled() => {}, + _ => return, + } + + match self.vertex_shader.get() { + Some(ref shader) if shader.successfully_compiled() => {}, + _ => return, + } + + self.linked.set(true); + self.renderer.send(CanvasMsg::WebGL(WebGLCommand::LinkProgram(self.id))).unwrap(); } /// glUseProgram pub fn use_program(&self) -> WebGLResult<()> { - match self.fragment_shader.get() { - Some(ref shader) if shader.successfully_compiled() => {}, - _ => return Err(WebGLError::InvalidOperation), - } - - match self.vertex_shader.get() { - Some(ref shader) if shader.successfully_compiled() => {}, - _ => return Err(WebGLError::InvalidOperation), + if !self.linked.get() { + return Err(WebGLError::InvalidOperation); } self.renderer.send(CanvasMsg::WebGL(WebGLCommand::UseProgram(self.id))).unwrap(); From 6a15c2f245a32677edd76dd0e1dedbd404132bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Apr 2016 13:11:28 +0200 Subject: [PATCH 05/12] webgl: Remove active_uniform related validation. It's broken for uniform arrays, since uniform.id() stops being the index then. We need to add a more complex integration with angle for this to ever be correct. Unfortunately the ANGLE part that we should touch is C++, and it has destructors, so we need to hook destructors there, and I can't do it right now. --- .../script/dom/webglrenderingcontext.rs | 50 +++++++++++-------- components/script/dom/webglshader.rs | 5 ++ 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 408558d3031..8257f8536a1 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use canvas_traits::{CanvasCommonMsg, CanvasMsg}; -use dom::bindings::codegen::Bindings::WebGLActiveInfoBinding::WebGLActiveInfoMethods; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{WebGLRenderingContextMethods}; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{self, WebGLContextAttributes}; @@ -68,6 +67,7 @@ bitflags! { } } +#[derive(Debug, PartialEq)] pub enum UniformType { Int, IntVec2, @@ -93,6 +93,25 @@ impl UniformType { } } + fn is_compatible_with(&self, gl_type: u32) -> bool { + gl_type == self.as_gl_constant() || match *self { + // Sampler uniform variables have an index value (the index of the + // texture), and as such they have to be set as ints + UniformType::Int => gl_type == constants::SAMPLER_2D || + gl_type == constants::SAMPLER_CUBE, + // Don't ask me why, but it seems we must allow setting bool + // uniforms with uniform1f. + // + // See the WebGL conformance test + // conformance/uniforms/gl-uniform-bool.html + UniformType::Float => gl_type == constants::BOOL, + UniformType::FloatVec2 => gl_type == constants::BOOL_VEC2, + UniformType::FloatVec3 => gl_type == constants::BOOL_VEC3, + UniformType::FloatVec4 => gl_type == constants::BOOL_VEC4, + _ => false, + } + } + fn as_gl_constant(&self) -> u32 { match *self { UniformType::Int => constants::INT, @@ -225,7 +244,7 @@ impl WebGLRenderingContext { // https://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf#nameddest=section-2.10.4 fn validate_uniform_parameters(&self, uniform: Option<&WebGLUniformLocation>, - type_: UniformType, + uniform_type: UniformType, data: Option<&[T]>) -> bool { let uniform = match uniform { Some(uniform) => uniform, @@ -233,8 +252,8 @@ impl WebGLRenderingContext { }; let program = self.current_program.get(); - let program = match program { - Some(ref program) if program.id() == uniform.program_id() => program, + match program { + Some(ref program) if program.id() == uniform.program_id() => {}, _ => { self.webgl_error(InvalidOperation); return false; @@ -249,28 +268,15 @@ impl WebGLRenderingContext { }, }; - // TODO(autrilla): Don't request this every time, cache it - let active_uniform = match program.get_active_uniform( - uniform.id() as u32) { - Ok(active_uniform) => active_uniform, - Err(_) => { - self.webgl_error(InvalidOperation); - return false; - }, - }; - - if data.len() % type_.element_count() != 0 || - (data.len() / type_.element_count() > active_uniform.Size() as usize) { + // TODO(emilio): Get more complex uniform info from ANGLE, and use it to + // properly validate that the uniform type is compatible with the + // uniform type, and that the uniform size matches. + if data.len() % uniform_type.element_count() != 0 { self.webgl_error(InvalidOperation); return false; } - if type_.as_gl_constant() != active_uniform.Type() { - self.webgl_error(InvalidOperation); - return false; - } - - return true; + true } fn validate_tex_image_parameters(&self, diff --git a/components/script/dom/webglshader.rs b/components/script/dom/webglshader.rs index 495417065d7..eb930a69f13 100644 --- a/components/script/dom/webglshader.rs +++ b/components/script/dom/webglshader.rs @@ -116,6 +116,11 @@ impl WebGLShader { } *self.info_log.borrow_mut() = Some(validator.info_log()); + // TODO(emilio): More data (like uniform data) should be collected + // here to properly validate uniforms. + // + // This requires a more complex interface with ANGLE, using C++ + // bindings and being extremely cautious about destructing things. } } From bc0876241406891690e6bbfef4bd8b58493a17f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Apr 2016 13:22:05 +0200 Subject: [PATCH 06/12] webgl: Rename UniformType to UniformSetterType and hoist to the bottom --- .../script/dom/webglrenderingcontext.rs | 174 ++++++++++-------- components/script/dom/webgluniformlocation.rs | 3 - 2 files changed, 98 insertions(+), 79 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 8257f8536a1..67f7377a1d5 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -67,65 +67,6 @@ bitflags! { } } -#[derive(Debug, PartialEq)] -pub enum UniformType { - Int, - IntVec2, - IntVec3, - IntVec4, - Float, - FloatVec2, - FloatVec3, - FloatVec4, -} - -impl UniformType { - fn element_count(&self) -> usize { - match *self { - UniformType::Int => 1, - UniformType::IntVec2 => 2, - UniformType::IntVec3 => 3, - UniformType::IntVec4 => 4, - UniformType::Float => 1, - UniformType::FloatVec2 => 2, - UniformType::FloatVec3 => 3, - UniformType::FloatVec4 => 4, - } - } - - fn is_compatible_with(&self, gl_type: u32) -> bool { - gl_type == self.as_gl_constant() || match *self { - // Sampler uniform variables have an index value (the index of the - // texture), and as such they have to be set as ints - UniformType::Int => gl_type == constants::SAMPLER_2D || - gl_type == constants::SAMPLER_CUBE, - // Don't ask me why, but it seems we must allow setting bool - // uniforms with uniform1f. - // - // See the WebGL conformance test - // conformance/uniforms/gl-uniform-bool.html - UniformType::Float => gl_type == constants::BOOL, - UniformType::FloatVec2 => gl_type == constants::BOOL_VEC2, - UniformType::FloatVec3 => gl_type == constants::BOOL_VEC3, - UniformType::FloatVec4 => gl_type == constants::BOOL_VEC4, - _ => false, - } - } - - fn as_gl_constant(&self) -> u32 { - match *self { - UniformType::Int => constants::INT, - UniformType::IntVec2 => constants::INT_VEC2, - UniformType::IntVec3 => constants::INT_VEC3, - UniformType::IntVec4 => constants::INT_VEC4, - UniformType::Float => constants::FLOAT, - UniformType::FloatVec2 => constants::FLOAT_VEC2, - UniformType::FloatVec3 => constants::FLOAT_VEC3, - UniformType::FloatVec4 => constants::FLOAT_VEC4, - } - } -} - #[dom_struct] pub struct WebGLRenderingContext { reflector_: Reflector, @@ -244,7 +185,7 @@ impl WebGLRenderingContext { // https://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf#nameddest=section-2.10.4 fn validate_uniform_parameters(&self, uniform: Option<&WebGLUniformLocation>, - uniform_type: UniformType, + uniform_type: UniformSetterType, data: Option<&[T]>) -> bool { let uniform = match uniform { Some(uniform) => uniform, @@ -1302,7 +1243,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { fn Uniform1f(&self, uniform: Option<&WebGLUniformLocation>, val: f32) { - if self.validate_uniform_parameters(uniform, UniformType::Float, Some(&[val])) { + if self.validate_uniform_parameters(uniform, UniformSetterType::Float, Some(&[val])) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform1f(uniform.unwrap().id(), val))) .unwrap() @@ -1313,7 +1254,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { fn Uniform1i(&self, uniform: Option<&WebGLUniformLocation>, val: i32) { - if self.validate_uniform_parameters(uniform, UniformType::Int, Some(&[val])) { + if self.validate_uniform_parameters(uniform, UniformSetterType::Int, Some(&[val])) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform1i(uniform.unwrap().id(), val))) .unwrap() @@ -1326,7 +1267,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let data_vec = data.and_then(|d| array_buffer_view_to_vec::(d)); - if self.validate_uniform_parameters(uniform, UniformType::Int, data_vec.as_ref().map(Vec::as_slice)) { + if self.validate_uniform_parameters(uniform, UniformSetterType::Int, data_vec.as_ref().map(Vec::as_slice)) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform1iv(uniform.unwrap().id(), data_vec.unwrap()))) .unwrap() @@ -1339,7 +1280,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let data_vec = data.and_then(|d| array_buffer_view_to_vec::(d)); - if self.validate_uniform_parameters(uniform, UniformType::Float, data_vec.as_ref().map(Vec::as_slice)) { + if self.validate_uniform_parameters(uniform, UniformSetterType::Float, data_vec.as_ref().map(Vec::as_slice)) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform1fv(uniform.unwrap().id(), data_vec.unwrap()))) .unwrap() @@ -1350,7 +1291,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { fn Uniform2f(&self, uniform: Option<&WebGLUniformLocation>, x: f32, y: f32) { - if self.validate_uniform_parameters(uniform, UniformType::FloatVec2, Some(&[x, y])) { + if self.validate_uniform_parameters(uniform, UniformSetterType::FloatVec2, Some(&[x, y])) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform2f(uniform.unwrap().id(), x, y))) .unwrap() @@ -1363,7 +1304,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let data_vec = data.and_then(|d| array_buffer_view_to_vec::(d)); - if self.validate_uniform_parameters(uniform, UniformType::FloatVec2, data_vec.as_ref().map(Vec::as_slice)) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::FloatVec2, + data_vec.as_ref().map(Vec::as_slice)) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform2fv(uniform.unwrap().id(), data_vec.unwrap()))) .unwrap() @@ -1374,7 +1317,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { fn Uniform2i(&self, uniform: Option<&WebGLUniformLocation>, x: i32, y: i32) { - if self.validate_uniform_parameters(uniform, UniformType::IntVec2, Some(&[x, y])) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::IntVec2, + Some(&[x, y])) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform2i(uniform.unwrap().id(), x, y))) .unwrap() @@ -1387,7 +1332,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let data_vec = data.and_then(|d| array_buffer_view_to_vec::(d)); - if self.validate_uniform_parameters(uniform, UniformType::IntVec2, data_vec.as_ref().map(Vec::as_slice)) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::IntVec2, + data_vec.as_ref().map(Vec::as_slice)) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform2iv(uniform.unwrap().id(), data_vec.unwrap()))) .unwrap() @@ -1398,7 +1345,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { fn Uniform3f(&self, uniform: Option<&WebGLUniformLocation>, x: f32, y: f32, z: f32) { - if self.validate_uniform_parameters(uniform, UniformType::FloatVec3, Some(&[x, y, z])) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::FloatVec3, + Some(&[x, y, z])) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform3f(uniform.unwrap().id(), x, y, z))) .unwrap() @@ -1411,7 +1360,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let data_vec = data.and_then(|d| array_buffer_view_to_vec::(d)); - if self.validate_uniform_parameters(uniform, UniformType::FloatVec3, data_vec.as_ref().map(Vec::as_slice)) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::FloatVec3, + data_vec.as_ref().map(Vec::as_slice)) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform3fv(uniform.unwrap().id(), data_vec.unwrap()))) .unwrap() @@ -1422,7 +1373,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { fn Uniform3i(&self, uniform: Option<&WebGLUniformLocation>, x: i32, y: i32, z: i32) { - if self.validate_uniform_parameters(uniform, UniformType::IntVec3, Some(&[x, y, z])) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::IntVec3, + Some(&[x, y, z])) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform3i(uniform.unwrap().id(), x, y, z))) .unwrap() @@ -1435,7 +1388,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let data_vec = data.and_then(|d| array_buffer_view_to_vec::(d)); - if self.validate_uniform_parameters(uniform, UniformType::IntVec3, data_vec.as_ref().map(Vec::as_slice)) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::IntVec3, + data_vec.as_ref().map(Vec::as_slice)) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform3iv(uniform.unwrap().id(), data_vec.unwrap()))) .unwrap() @@ -1446,7 +1401,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { fn Uniform4i(&self, uniform: Option<&WebGLUniformLocation>, x: i32, y: i32, z: i32, w: i32) { - if self.validate_uniform_parameters(uniform, UniformType::IntVec4, Some(&[x, y, z, w])) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::IntVec4, + Some(&[x, y, z, w])) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform4i(uniform.unwrap().id(), x, y, z, w))) .unwrap() @@ -1460,7 +1417,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let data_vec = data.and_then(|d| array_buffer_view_to_vec::(d)); - if self.validate_uniform_parameters(uniform, UniformType::IntVec4, data_vec.as_ref().map(Vec::as_slice)) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::IntVec4, + data_vec.as_ref().map(Vec::as_slice)) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform4iv(uniform.unwrap().id(), data_vec.unwrap()))) .unwrap() @@ -1471,7 +1430,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { fn Uniform4f(&self, uniform: Option<&WebGLUniformLocation>, x: f32, y: f32, z: f32, w: f32) { - if self.validate_uniform_parameters(uniform, UniformType::FloatVec4, Some(&[x, y, z, w])) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::FloatVec4, + Some(&[x, y, z, w])) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform4f(uniform.unwrap().id(), x, y, z, w))) .unwrap() @@ -1484,7 +1445,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { uniform: Option<&WebGLUniformLocation>, data: Option<*mut JSObject>) { let data_vec = data.and_then(|d| array_buffer_view_to_vec::(d)); - if self.validate_uniform_parameters(uniform, UniformType::FloatVec4, data_vec.as_ref().map(Vec::as_slice)) { + if self.validate_uniform_parameters(uniform, + UniformSetterType::FloatVec4, + data_vec.as_ref().map(Vec::as_slice)) { self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::Uniform4fv(uniform.unwrap().id(), data_vec.unwrap()))) .unwrap() @@ -1785,3 +1748,62 @@ impl LayoutCanvasWebGLRenderingContextHelpers for LayoutJS usize { + match *self { + UniformSetterType::Int => 1, + UniformSetterType::IntVec2 => 2, + UniformSetterType::IntVec3 => 3, + UniformSetterType::IntVec4 => 4, + UniformSetterType::Float => 1, + UniformSetterType::FloatVec2 => 2, + UniformSetterType::FloatVec3 => 3, + UniformSetterType::FloatVec4 => 4, + } + } + + pub fn is_compatible_with(&self, gl_type: u32) -> bool { + gl_type == self.as_gl_constant() || match *self { + // Sampler uniform variables have an index value (the index of the + // texture), and as such they have to be set as ints + UniformSetterType::Int => gl_type == constants::SAMPLER_2D || + gl_type == constants::SAMPLER_CUBE, + // Don't ask me why, but it seems we must allow setting bool + // uniforms with uniform1f. + // + // See the WebGL conformance test + // conformance/uniforms/gl-uniform-bool.html + UniformSetterType::Float => gl_type == constants::BOOL, + UniformSetterType::FloatVec2 => gl_type == constants::BOOL_VEC2, + UniformSetterType::FloatVec3 => gl_type == constants::BOOL_VEC3, + UniformSetterType::FloatVec4 => gl_type == constants::BOOL_VEC4, + _ => false, + } + } + + fn as_gl_constant(&self) -> u32 { + match *self { + UniformSetterType::Int => constants::INT, + UniformSetterType::IntVec2 => constants::INT_VEC2, + UniformSetterType::IntVec3 => constants::INT_VEC3, + UniformSetterType::IntVec4 => constants::INT_VEC4, + UniformSetterType::Float => constants::FLOAT, + UniformSetterType::FloatVec2 => constants::FLOAT_VEC2, + UniformSetterType::FloatVec3 => constants::FLOAT_VEC3, + UniformSetterType::FloatVec4 => constants::FLOAT_VEC4, + } + } +} diff --git a/components/script/dom/webgluniformlocation.rs b/components/script/dom/webgluniformlocation.rs index 43244ec4a33..2f8e3331cd2 100644 --- a/components/script/dom/webgluniformlocation.rs +++ b/components/script/dom/webgluniformlocation.rs @@ -28,10 +28,7 @@ impl WebGLUniformLocation { reflect_dom_object( box WebGLUniformLocation::new_inherited(id, program_id), global, WebGLUniformLocationBinding::Wrap) } -} - -impl WebGLUniformLocation { pub fn id(&self) -> i32 { self.id } From 1ad7f73caf3965621238cceebb19fe3a8203ed8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 20 Apr 2016 19:21:47 +0200 Subject: [PATCH 07/12] webgl: Reset bound buffer when appropiate in BufferData This makes the test bufferDataBadArgs pass appropiately. --- components/script/dom/webglrenderingcontext.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 67f7377a1d5..551a1874b0c 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -213,8 +213,8 @@ impl WebGLRenderingContext { // properly validate that the uniform type is compatible with the // uniform type, and that the uniform size matches. if data.len() % uniform_type.element_count() != 0 { - self.webgl_error(InvalidOperation); - return false; + self.webgl_error(InvalidOperation); + return false; } true @@ -616,6 +616,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { Err(e) => return self.webgl_error(e), } } else { + slot.set(None); // Unbind the current buffer self.ipc_renderer .send(CanvasMsg::WebGL(WebGLCommand::BindBuffer(target, 0))) @@ -698,20 +699,24 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { constants::ELEMENT_ARRAY_BUFFER => self.bound_buffer_element_array.get(), _ => return self.webgl_error(InvalidEnum), }; + let bound_buffer = match bound_buffer { Some(bound_buffer) => bound_buffer, None => return self.webgl_error(InvalidValue), }; + match usage { constants::STREAM_DRAW | constants::STATIC_DRAW | constants::DYNAMIC_DRAW => (), _ => return self.webgl_error(InvalidEnum), } + let data = match data { Some(data) => data, None => return self.webgl_error(InvalidValue), }; + if let Some(data_vec) = array_buffer_view_to_vec::(data) { handle_potential_webgl_error!(self, bound_buffer.buffer_data(target, &data_vec, usage)); } else { From 21e2f97cfaf805c9e8f53e515c717ad263d3b891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 20 Apr 2016 20:24:56 +0200 Subject: [PATCH 08/12] webgl: Fix filling a non-zero level You can fill a level > 0 as long as the width and height values are power of two, so the previous test was bogus. --- components/script/dom/webglrenderingcontext.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 551a1874b0c..51a53cb3435 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -251,13 +251,10 @@ impl WebGLRenderingContext { // If an attempt is made to call this function with no // WebGLTexture bound, an INVALID_OPERATION error is generated. - let texture = match texture { - Some(texture) => texture, - None => { - self.webgl_error(InvalidOperation); - return false; - } - }; + if texture.is_none() { + self.webgl_error(InvalidOperation); + return false; + } // GL_INVALID_ENUM is generated if data_type is not an accepted value. match data_type { @@ -324,7 +321,9 @@ impl WebGLRenderingContext { // GL_INVALID_VALUE is generated if level is greater than zero and the // texture and the texture is not power of two. - if level > 0 && !texture.is_power_of_two() { + if level > 0 && + (!(width as u32).is_power_of_two() || + !(height as u32).is_power_of_two()) { self.webgl_error(InvalidValue); return false; } From d152c7fe8848bb2e75384f59b42516a1c9e3b41d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 20 Apr 2016 20:27:37 +0200 Subject: [PATCH 09/12] webgl: texture: Make initialize only mark as initialized the current face Also refactor a bit the code, and remove the unused `is_initialized` flag. --- .../script/dom/webglrenderingcontext.rs | 3 +- components/script/dom/webgltexture.rs | 43 +++++++++++++------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 51a53cb3435..9e6a0d806dd 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -389,7 +389,8 @@ impl WebGLRenderingContext { // TODO(emilio): Flip Y axis if necessary here // TexImage2D depth is always equal to 1 - handle_potential_webgl_error!(self, texture.initialize(width as u32, + handle_potential_webgl_error!(self, texture.initialize(target, + width as u32, height as u32, 1, internal_format, level as u32)); diff --git a/components/script/dom/webgltexture.rs b/components/script/dom/webgltexture.rs index f39fa9b5789..e92435ce304 100644 --- a/components/script/dom/webgltexture.rs +++ b/components/script/dom/webgltexture.rs @@ -33,7 +33,6 @@ pub struct WebGLTexture { /// The target to which this texture was bound the first time target: Cell>, is_deleted: Cell, - is_initialized: Cell, /// Stores information about mipmap levels and cubemap faces. #[ignore_heap_size_of = "Arrays are cumbersome"] image_info_array: DOMRefCell<[ImageInfo; MAX_LEVEL_COUNT * MAX_FACE_COUNT]>, @@ -51,7 +50,6 @@ impl WebGLTexture { id: id, target: Cell::new(None), is_deleted: Cell::new(false), - is_initialized: Cell::new(false), face_count: Cell::new(0), base_mipmap_level: 0, image_info_array: DOMRefCell::new([ImageInfo::new(); MAX_LEVEL_COUNT * MAX_FACE_COUNT]), @@ -105,7 +103,13 @@ impl WebGLTexture { Ok(()) } - pub fn initialize(&self, width: u32, height: u32, depth: u32, internal_format: u32, level: u32) -> WebGLResult<()> { + pub fn initialize(&self, + target: u32, + width: u32, + height: u32, + depth: u32, + internal_format: u32, + level: u32) -> WebGLResult<()> { let image_info = ImageInfo { width: width, height: height, @@ -113,10 +117,18 @@ impl WebGLTexture { internal_format: Some(internal_format), is_initialized: true, }; - self.set_image_infos_at_level(level, image_info); - self.is_initialized.set(true); + let face = match target { + constants::TEXTURE_2D | constants::TEXTURE_CUBE_MAP_POSITIVE_X => 0, + constants::TEXTURE_CUBE_MAP_NEGATIVE_X => 1, + constants::TEXTURE_CUBE_MAP_POSITIVE_Y => 2, + constants::TEXTURE_CUBE_MAP_NEGATIVE_Y => 3, + constants::TEXTURE_CUBE_MAP_POSITIVE_Z => 4, + constants::TEXTURE_CUBE_MAP_NEGATIVE_Z => 5, + _ => unreachable!(), + }; + self.set_image_infos_at_level_and_face(level, face, image_info); Ok(()) } @@ -130,12 +142,12 @@ impl WebGLTexture { }; let base_image_info = self.base_image_info().unwrap(); - if !base_image_info.is_initialized() { return Err(WebGLError::InvalidOperation); } - if target == constants::TEXTURE_CUBE_MAP && !self.is_cube_complete() { + let is_cubic = target == constants::TEXTURE_CUBE_MAP; + if is_cubic && !self.is_cube_complete() { return Err(WebGLError::InvalidOperation); } @@ -227,10 +239,6 @@ impl WebGLTexture { } } - pub fn is_power_of_two(&self) -> bool { - self.image_info_at_face(0, 0).is_power_of_two() - } - pub fn populate_mip_chain(&self, first_level: u32, last_level: u32) -> WebGLResult<()> { let base_image_info = self.image_info_at_face(0, first_level); if !base_image_info.is_initialized() { @@ -266,6 +274,8 @@ impl WebGLTexture { } fn is_cube_complete(&self) -> bool { + debug_assert!(self.face_count.get() == 6); + let image_info = self.base_image_info().unwrap(); if !image_info.is_defined() { return false; @@ -298,11 +308,16 @@ impl WebGLTexture { fn set_image_infos_at_level(&self, level: u32, image_info: ImageInfo) { for face in 0..self.face_count.get() { - let pos = (level * self.face_count.get() as u32) + face as u32; - self.image_info_array.borrow_mut()[pos as usize] = image_info; + self.set_image_infos_at_level_and_face(level, face, image_info); } } + fn set_image_infos_at_level_and_face(&self, level: u32, face: u8, image_info: ImageInfo) { + debug_assert!(face < self.face_count.get()); + let pos = (level * self.face_count.get() as u32) + face as u32; + self.image_info_array.borrow_mut()[pos as usize] = image_info; + } + fn base_image_info(&self) -> Option { assert!((self.base_mipmap_level as usize) < MAX_LEVEL_COUNT); @@ -345,7 +360,7 @@ impl ImageInfo { } fn is_defined(&self) -> bool { - !self.internal_format.is_none() + self.internal_format.is_some() } fn get_max_mimap_levels(&self) -> u32 { From f6f1e37b298d1c84489b7cdfe9c79e40b9a22d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 20 Apr 2016 20:30:05 +0200 Subject: [PATCH 10/12] webgl: Fix a few typos in comments --- components/script/dom/webglrenderingcontext.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 9e6a0d806dd..55f34cabe90 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -210,7 +210,7 @@ impl WebGLRenderingContext { }; // TODO(emilio): Get more complex uniform info from ANGLE, and use it to - // properly validate that the uniform type is compatible with the + // properly validate that the uniform setter type is compatible with the // uniform type, and that the uniform size matches. if data.len() % uniform_type.element_count() != 0 { self.webgl_error(InvalidOperation); @@ -320,7 +320,7 @@ impl WebGLRenderingContext { } // GL_INVALID_VALUE is generated if level is greater than zero and the - // texture and the texture is not power of two. + // texture is not power of two. if level > 0 && (!(width as u32).is_power_of_two() || !(height as u32).is_power_of_two()) { From 764fac781eb055b307910c4d97a702c299aee216 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 20 Apr 2016 21:42:58 +0200 Subject: [PATCH 11/12] webgl: tests: Mark the bufferDataBadArgs test as passing It now passes consistently. --- .../conformance/more/functions/bufferDataBadArgs.html.ini | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 tests/wpt/metadata/webgl/conformance-1.0.3/conformance/more/functions/bufferDataBadArgs.html.ini diff --git a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/more/functions/bufferDataBadArgs.html.ini b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/more/functions/bufferDataBadArgs.html.ini deleted file mode 100644 index aa52146e07f..00000000000 --- a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/more/functions/bufferDataBadArgs.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[bufferDataBadArgs.html] - type: testharness - [WebGL test #0: testBufferData] - expected: FAIL - From 9fff9d45d1108fcb2a594feb091e0f64d2cab7e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 20 Apr 2016 22:12:39 +0200 Subject: [PATCH 12/12] webgl: tests: Update expectations of tests This tests previously "passed", because they used the texImage2D overload we're adding now. Since that overload previously caused a TypeError, these tests were never run in the first place. --- .../conformance/limits/gl-min-textures.html.ini | 3 +++ .../conformance/renderbuffers/feedback-loop.html.ini | 3 +++ .../conformance/textures/texture-mips.html.ini | 3 +++ .../conformance/textures/texture-npot.html.ini | 3 +++ 4 files changed, 12 insertions(+) diff --git a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/limits/gl-min-textures.html.ini b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/limits/gl-min-textures.html.ini index cd454029296..625e08bbc08 100644 --- a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/limits/gl-min-textures.html.ini +++ b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/limits/gl-min-textures.html.ini @@ -3,3 +3,6 @@ [WebGL test #0: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] expected: FAIL + [WebGL test #1: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] + expected: FAIL + diff --git a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/renderbuffers/feedback-loop.html.ini b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/renderbuffers/feedback-loop.html.ini index a3be8b1b8b1..80b7a714709 100644 --- a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/renderbuffers/feedback-loop.html.ini +++ b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/renderbuffers/feedback-loop.html.ini @@ -3,3 +3,6 @@ [WebGL test #0: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] expected: FAIL + [WebGL test #1: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] + expected: FAIL + diff --git a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/textures/texture-mips.html.ini b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/textures/texture-mips.html.ini index 12cce1a5131..f5b63a2a77d 100644 --- a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/textures/texture-mips.html.ini +++ b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/textures/texture-mips.html.ini @@ -6,3 +6,6 @@ [WebGL test #3: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] expected: FAIL + [WebGL test #11: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] + expected: FAIL + diff --git a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/textures/texture-npot.html.ini b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/textures/texture-npot.html.ini index d03379487c8..82aee04e698 100644 --- a/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/textures/texture-npot.html.ini +++ b/tests/wpt/metadata/webgl/conformance-1.0.3/conformance/textures/texture-npot.html.ini @@ -3,3 +3,6 @@ [WebGL test #1: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] expected: FAIL + [WebGL test #4: successfullyParsed should be true (of type boolean). Was undefined (of type undefined).] + expected: FAIL +