diff --git a/components/canvas/webgl_thread.rs b/components/canvas/webgl_thread.rs index 046290b7dd8..754b073328d 100644 --- a/components/canvas/webgl_thread.rs +++ b/components/canvas/webgl_thread.rs @@ -740,8 +740,6 @@ impl WebGLImpl { Self::active_attrib(ctx.gl(), program_id, index, chan), WebGLCommand::GetActiveUniform(program_id, index, ref chan) => Self::active_uniform(ctx.gl(), program_id, index, chan), - WebGLCommand::GetAttribLocation(program_id, ref name, ref chan) => - Self::attrib_location(ctx.gl(), program_id, name, chan), WebGLCommand::GetRenderbufferParameter(target, pname, ref chan) => Self::get_renderbuffer_parameter(ctx.gl(), target, pname, chan), WebGLCommand::GetFramebufferAttachmentParameter(target, attachment, pname, ref chan) => @@ -790,8 +788,6 @@ impl WebGLImpl { ctx.gl().bind_renderbuffer(target, id.map_or(0, WebGLRenderbufferId::get)), WebGLCommand::BindTexture(target, id) => ctx.gl().bind_texture(target, id.map_or(0, WebGLTextureId::get)), - WebGLCommand::LinkProgram(program_id) => - ctx.gl().link_program(program_id.get()), WebGLCommand::Uniform1f(uniform_id, v) => ctx.gl().uniform_1f(uniform_id, v), WebGLCommand::Uniform1fv(uniform_id, ref v) => @@ -919,17 +915,17 @@ impl WebGLImpl { } sender.send(value).unwrap() } - WebGLCommand::GetProgramParameterBool(program, param, ref sender) => { + WebGLCommand::GetProgramValidateStatus(program, ref sender) => { let mut value = [0]; unsafe { - ctx.gl().get_program_iv(program.get(), param as u32, &mut value); + ctx.gl().get_program_iv(program.get(), gl::VALIDATE_STATUS, &mut value); } sender.send(value[0] != 0).unwrap() } - WebGLCommand::GetProgramParameterInt(program, param, ref sender) => { + WebGLCommand::GetProgramActiveUniforms(program, ref sender) => { let mut value = [0]; unsafe { - ctx.gl().get_program_iv(program.get(), param as u32, &mut value); + ctx.gl().get_program_iv(program.get(), gl::ACTIVE_UNIFORMS, &mut value); } sender.send(value[0]).unwrap() } @@ -966,6 +962,9 @@ impl WebGLImpl { WebGLCommand::TexParameterf(target, param, value) => { ctx.gl().tex_parameter_f(target, param as u32, value) } + WebGLCommand::LinkProgram(program_id, ref sender) => { + return sender.send(Self::link_program(ctx.gl(), program_id)).unwrap(); + } } // TODO: update test expectations in order to enable debug assertions @@ -976,6 +975,45 @@ impl WebGLImpl { assert_eq!(error, gl::NO_ERROR, "Unexpected WebGL error: 0x{:x} ({})", error, error); } + #[allow(unsafe_code)] + fn link_program(gl: &gl::Gl, program: WebGLProgramId) -> ProgramLinkInfo { + gl.link_program(program.get()); + let mut linked = [0]; + unsafe { + gl.get_program_iv(program.get(), gl::LINK_STATUS, &mut linked); + } + if linked[0] == 0 { + return ProgramLinkInfo { + linked: false, + active_attribs: vec![].into(), + } + } + + let mut num_active_attribs = [0]; + unsafe { + gl.get_program_iv(program.get(), gl::ACTIVE_ATTRIBUTES, &mut num_active_attribs); + } + let active_attribs = (0..num_active_attribs[0] as u32).map(|i| { + let (size, type_, name) = gl.get_active_attrib(program.get(), i); + let location = if name.starts_with("gl_") { + -1 + } else { + gl.get_attrib_location(program.get(), &name) + }; + ActiveAttribInfo { + name: from_name_in_compiled_shader(&name), + size, + type_, + location, + } + }).collect::>().into(); + + ProgramLinkInfo { + linked: true, + active_attribs, + } + } + fn read_pixels( gl: &gl::Gl, x: i32, @@ -1026,21 +1064,6 @@ impl WebGLImpl { chan.send(result).unwrap(); } - fn attrib_location(gl: &gl::Gl, - program_id: WebGLProgramId, - name: &str, - chan: &WebGLSender> ) { - let attrib_location = gl.get_attrib_location(program_id.get(), name); - - let attrib_location = if attrib_location == -1 { - None - } else { - Some(attrib_location) - }; - - chan.send(attrib_location).unwrap(); - } - fn finish(gl: &gl::Gl, chan: &WebGLSender<()>) { gl.finish(); chan.send(()).unwrap(); diff --git a/components/canvas_traits/webgl.rs b/components/canvas_traits/webgl.rs index e86935f6f2a..f39ceb2c7fb 100644 --- a/components/canvas_traits/webgl.rs +++ b/components/canvas_traits/webgl.rs @@ -209,7 +209,6 @@ pub enum WebGLCommand { GetShaderPrecisionFormat(u32, u32, WebGLSender<(i32, i32, i32)>), GetActiveAttrib(WebGLProgramId, u32, WebGLSender>), GetActiveUniform(WebGLProgramId, u32, WebGLSender>), - GetAttribLocation(WebGLProgramId, String, WebGLSender>), GetUniformLocation(WebGLProgramId, String, WebGLSender>), GetShaderInfoLog(WebGLShaderId, WebGLSender), GetProgramInfoLog(WebGLProgramId, WebGLSender), @@ -230,7 +229,7 @@ pub enum WebGLCommand { IsEnabled(u32, WebGLSender), LineWidth(f32), PixelStorei(u32, i32), - LinkProgram(WebGLProgramId), + LinkProgram(WebGLProgramId, WebGLSender), Uniform1f(i32, f32), Uniform1fv(i32, Vec), Uniform1i(i32, i32), @@ -273,8 +272,8 @@ pub enum WebGLCommand { GetParameterFloat(ParameterFloat, WebGLSender), GetParameterFloat2(ParameterFloat2, WebGLSender<[f32; 2]>), GetParameterFloat4(ParameterFloat4, WebGLSender<[f32; 4]>), - GetProgramParameterBool(WebGLProgramId, ProgramParameterBool, WebGLSender), - GetProgramParameterInt(WebGLProgramId, ProgramParameterInt, WebGLSender), + GetProgramValidateStatus(WebGLProgramId, WebGLSender), + GetProgramActiveUniforms(WebGLProgramId, WebGLSender), GetShaderParameterBool(WebGLShaderId, ShaderParameterBool, WebGLSender), GetShaderParameterInt(WebGLShaderId, ShaderParameterInt, WebGLSender), GetCurrentVertexAttrib(u32, WebGLSender<[f32; 4]>), @@ -415,6 +414,28 @@ pub enum DOMToTextureCommand { Lock(PipelineId, usize, WebGLSender)>>), } +/// Information about a WebGL program linking operation. +#[derive(Clone, Deserialize, Serialize)] +pub struct ProgramLinkInfo { + /// Whether the program was linked successfully. + pub linked: bool, + /// The list of active attributes. + pub active_attribs: Box<[ActiveAttribInfo]>, +} + +/// Description of a single active attribute. +#[derive(Clone, Deserialize, MallocSizeOf, Serialize)] +pub struct ActiveAttribInfo { + /// The name of the attribute. + pub name: String, + /// The size of the attribute. + pub size: i32, + /// The type of the attribute. + pub type_: u32, + /// The location of the attribute. + pub location: i32, +} + macro_rules! parameters { ($name:ident { $( $variant:ident($kind:ident { $( @@ -525,21 +546,6 @@ parameters! { } } -parameters! { - ProgramParameter { - Bool(ProgramParameterBool { - DeleteStatus = gl::DELETE_STATUS, - LinkStatus = gl::LINK_STATUS, - ValidateStatus = gl::VALIDATE_STATUS, - }), - Int(ProgramParameterInt { - AttachedShaders = gl::ATTACHED_SHADERS, - ActiveAttributes = gl::ACTIVE_ATTRIBUTES, - ActiveUniforms = gl::ACTIVE_UNIFORMS, - }), - } -} - parameters! { ShaderParameter { Bool(ShaderParameterBool { @@ -565,3 +571,39 @@ parameters! { }), } } + +/// ANGLE adds a `_u` prefix to variable names: +/// +/// https://chromium.googlesource.com/angle/angle/+/855d964bd0d05f6b2cb303f625506cf53d37e94f +/// +/// To avoid hard-coding this we would need to use the `sh::GetAttributes` and `sh::GetUniforms` +/// API to look up the `x.name` and `x.mappedName` members. +const ANGLE_NAME_PREFIX: &'static str = "_u"; + +pub fn to_name_in_compiled_shader(s: &str) -> String { + map_dot_separated(s, |s, mapped| { + mapped.push_str(ANGLE_NAME_PREFIX); + mapped.push_str(s); + }) +} + +pub fn from_name_in_compiled_shader(s: &str) -> String { + map_dot_separated(s, |s, mapped| { + mapped.push_str(if s.starts_with(ANGLE_NAME_PREFIX) { + &s[ANGLE_NAME_PREFIX.len()..] + } else { + s + }) + }) +} + +fn map_dot_separated(s: &str, f: F) -> String { + let mut iter = s.split('.'); + let mut mapped = String::new(); + f(iter.next().unwrap(), &mut mapped); + for s in iter { + mapped.push('.'); + f(s, &mut mapped); + } + mapped +} diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 794a3618820..06892ec8478 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -32,7 +32,7 @@ use app_units::Au; use canvas_traits::canvas::{CanvasGradientStop, CanvasId, LinearGradientStyle, RadialGradientStyle}; use canvas_traits::canvas::{CompositionOrBlending, LineCapStyle, LineJoinStyle, RepetitionStyle}; -use canvas_traits::webgl::{WebGLBufferId, WebGLFramebufferId, WebGLProgramId, WebGLRenderbufferId}; +use canvas_traits::webgl::{ActiveAttribInfo, WebGLBufferId, WebGLFramebufferId, WebGLProgramId, WebGLRenderbufferId}; use canvas_traits::webgl::{WebGLChan, WebGLContextShareMode, WebGLError, WebGLPipeline, WebGLMsgSender}; use canvas_traits::webgl::{WebGLReceiver, WebGLSender, WebGLShaderId, WebGLTextureId, WebGLVertexArrayId}; use canvas_traits::webgl::{WebGLSLVersion, WebGLVersion}; @@ -342,6 +342,7 @@ unsafe impl JSTraceable for (A, } } +unsafe_no_jsmanaged_fields!(ActiveAttribInfo); unsafe_no_jsmanaged_fields!(bool, f32, f64, String, AtomicBool, AtomicUsize, Uuid, char); unsafe_no_jsmanaged_fields!(usize, u8, u16, u32, u64); unsafe_no_jsmanaged_fields!(isize, i8, i16, i32, i64); diff --git a/components/script/dom/webglprogram.rs b/components/script/dom/webglprogram.rs index d710568d1ca..97ebb92d70f 100644 --- a/components/script/dom/webglprogram.rs +++ b/components/script/dom/webglprogram.rs @@ -3,8 +3,10 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // https://www.khronos.org/registry/webgl/specs/latest/1.0/webgl.idl -use canvas_traits::webgl::{WebGLCommand, WebGLError, WebGLMsgSender, WebGLProgramId, WebGLResult}; -use canvas_traits::webgl::webgl_channel; +use canvas_traits::webgl::{ActiveAttribInfo, WebGLCommand, WebGLError, WebGLMsgSender}; +use canvas_traits::webgl::{WebGLProgramId, WebGLResult, from_name_in_compiled_shader}; +use canvas_traits::webgl::{to_name_in_compiled_shader, webgl_channel}; +use dom::bindings::cell::DomRefCell; use dom::bindings::codegen::Bindings::WebGLProgramBinding; use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants; use dom::bindings::reflector::{DomObject, reflect_dom_object}; @@ -16,7 +18,7 @@ use dom::webglrenderingcontext::MAX_UNIFORM_AND_ATTRIBUTE_LEN; use dom::webglshader::WebGLShader; use dom::window::Window; use dom_struct::dom_struct; -use std::cell::Cell; +use std::cell::{Cell, Ref}; #[dom_struct] pub struct WebGLProgram { @@ -29,46 +31,7 @@ pub struct WebGLProgram { vertex_shader: MutNullableDom, #[ignore_malloc_size_of = "Defined in ipc-channel"] renderer: WebGLMsgSender, -} - -/// ANGLE adds a `_u` prefix to variable names: -/// -/// https://chromium.googlesource.com/angle/angle/+/855d964bd0d05f6b2cb303f625506cf53d37e94f -/// -/// To avoid hard-coding this we would need to use the `sh::GetAttributes` and `sh::GetUniforms` -/// API to look up the `x.name` and `x.mappedName` members, -/// then build a data structure for bi-directional lookup (so either linear scan or two hashmaps). -/// Even then, this would probably only support plain variable names like "foo". -/// Strings passed to e.g. `GetUniformLocation` can be expressions like "foo[0].bar", -/// with the mapping for that "bar" name in yet another part of ANGLE’s API. -const ANGLE_NAME_PREFIX: &'static str = "_u"; - -fn to_name_in_compiled_shader(s: &str) -> String { - map_dot_separated(s, |s, mapped| { - mapped.push_str(ANGLE_NAME_PREFIX); - mapped.push_str(s); - }) -} - -fn from_name_in_compiled_shader(s: &str) -> String { - map_dot_separated(s, |s, mapped| { - mapped.push_str(if s.starts_with(ANGLE_NAME_PREFIX) { - &s[ANGLE_NAME_PREFIX.len()..] - } else { - s - }) - }) -} - -fn map_dot_separated(s: &str, f: F) -> String { - let mut iter = s.split('.'); - let mut mapped = String::new(); - f(iter.next().unwrap(), &mut mapped); - for s in iter { - mapped.push('.'); - f(s, &mut mapped); - } - mapped + active_attribs: DomRefCell>, } impl WebGLProgram { @@ -84,6 +47,7 @@ impl WebGLProgram { fragment_shader: Default::default(), vertex_shader: Default::default(), renderer: renderer, + active_attribs: DomRefCell::new(vec![].into()), } } @@ -142,7 +106,7 @@ impl WebGLProgram { return Err(WebGLError::InvalidOperation); } self.linked.set(false); - self.link_called.set(true); + *self.active_attribs.borrow_mut() = vec![].into(); match self.fragment_shader.get() { Some(ref shader) if shader.successfully_compiled() => {}, @@ -154,11 +118,18 @@ impl WebGLProgram { _ => return Ok(()), // callers use gl.LINK_STATUS to check link errors } - self.linked.set(true); - self.renderer.send(WebGLCommand::LinkProgram(self.id)).unwrap(); + let (sender, receiver) = webgl_channel().unwrap(); + self.renderer.send(WebGLCommand::LinkProgram(self.id, sender)).unwrap(); + let link_info = receiver.recv().unwrap(); + self.linked.set(link_info.linked); + *self.active_attribs.borrow_mut() = link_info.active_attribs; Ok(()) } + pub fn active_attribs(&self) -> Ref<[ActiveAttribInfo]> { + Ref::map(self.active_attribs.borrow(), |attribs| &**attribs) + } + /// glUseProgram pub fn use_program(&self) -> WebGLResult<()> { if self.is_deleted() { @@ -281,19 +252,18 @@ impl WebGLProgram { if self.is_deleted() { return Err(WebGLError::InvalidValue); } - let (sender, receiver) = webgl_channel().unwrap(); - self.renderer - .send(WebGLCommand::GetActiveAttrib(self.id, index, sender)) - .unwrap(); - - receiver.recv().unwrap().map(|(size, ty, name)| { - let name = DOMString::from(from_name_in_compiled_shader(&name)); - WebGLActiveInfo::new(self.global().as_window(), size, ty, name) - }) + let attribs = self.active_attribs.borrow(); + let data = attribs.get(index as usize).ok_or(WebGLError::InvalidValue)?; + Ok(WebGLActiveInfo::new( + self.global().as_window(), + data.size, + data.type_, + data.name.clone().into(), + )) } /// glGetAttribLocation - pub fn get_attrib_location(&self, name: DOMString) -> WebGLResult> { + pub fn get_attrib_location(&self, name: DOMString) -> WebGLResult { if !self.is_linked() || self.is_deleted() { return Err(WebGLError::InvalidOperation); } @@ -303,21 +273,20 @@ impl WebGLProgram { // Check if the name is reserved if name.starts_with("gl_") { - return Ok(None); + return Ok(-1); } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#GLSL_CONSTRUCTS if name.starts_with("webgl_") || name.starts_with("_webgl_") { - return Ok(None); + return Ok(-1); } - let name = to_name_in_compiled_shader(&name); - - let (sender, receiver) = webgl_channel().unwrap(); - self.renderer - .send(WebGLCommand::GetAttribLocation(self.id, name, sender)) - .unwrap(); - Ok(receiver.recv().unwrap()) + let location = self.active_attribs + .borrow() + .iter() + .find(|attrib| attrib.name == &*name) + .map_or(-1, |attrib| attrib.location); + Ok(location) } /// glGetUniformLocation diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index 2c34f8f32bb..0750d5f5e5d 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -4,7 +4,7 @@ use byteorder::{NativeEndian, ReadBytesExt, WriteBytesExt}; use canvas_traits::canvas::{byte_swap, multiply_u8_pixel}; -use canvas_traits::webgl::{DOMToTextureCommand, Parameter, ProgramParameter}; +use canvas_traits::webgl::{DOMToTextureCommand, Parameter}; use canvas_traits::webgl::{ShaderParameter, TexParameter, WebGLCommand}; use canvas_traits::webgl::{WebGLContextShareMode, WebGLError}; use canvas_traits::webgl::{WebGLFramebufferBindingRequest, WebGLMsg, WebGLMsgSender}; @@ -2314,18 +2314,12 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn GetActiveAttrib(&self, program: &WebGLProgram, index: u32) -> Option> { - match program.get_active_attrib(index) { - Ok(ret) => Some(ret), - Err(e) => { - self.webgl_error(e); - return None; - } - } + handle_potential_webgl_error!(self, program.get_active_attrib(index), None) } // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10 fn GetAttribLocation(&self, program: &WebGLProgram, name: DOMString) -> i32 { - handle_potential_webgl_error!(self, program.get_attrib_location(name), None).unwrap_or(-1) + handle_potential_webgl_error!(self, program.get_attrib_location(name), -1) } #[allow(unsafe_code)] @@ -2497,17 +2491,32 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { #[allow(unsafe_code)] // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9 unsafe fn GetProgramParameter(&self, _: *mut JSContext, program: &WebGLProgram, param: u32) -> JSVal { - match handle_potential_webgl_error!(self, ProgramParameter::from_u32(param), return NullValue()) { - ProgramParameter::Bool(param) => { + // FIXME(nox): INVALID_OPERATION if program comes from a different context. + match param { + constants::DELETE_STATUS => BooleanValue(program.is_deleted()), + constants::LINK_STATUS => BooleanValue(program.is_linked()), + constants::VALIDATE_STATUS => { + // FIXME(nox): This could be cached on the DOM side when we call validateProgram + // but I'm not sure when the value should be reset. let (sender, receiver) = webgl_channel().unwrap(); - self.send_command(WebGLCommand::GetProgramParameterBool(program.id(), param, sender)); + self.send_command(WebGLCommand::GetProgramValidateStatus(program.id(), sender)); BooleanValue(receiver.recv().unwrap()) } - ProgramParameter::Int(param) => { + constants::ATTACHED_SHADERS => { + // FIXME(nox): This allocates a vector and roots a couple of shaders for nothing. + Int32Value(program.attached_shaders().map(|shaders| shaders.len() as i32).unwrap_or(0)) + } + constants::ACTIVE_ATTRIBUTES => Int32Value(program.active_attribs().len() as i32), + constants::ACTIVE_UNIFORMS => { + // FIXME(nox): We'll need to cache that on the DOM side at some point. let (sender, receiver) = webgl_channel().unwrap(); - self.send_command(WebGLCommand::GetProgramParameterInt(program.id(), param, sender)); + self.send_command(WebGLCommand::GetProgramActiveUniforms(program.id(), sender)); Int32Value(receiver.recv().unwrap()) } + _ => { + self.webgl_error(InvalidEnum); + NullValue() + } } } diff --git a/tests/wpt/mozilla/meta/webgl/conformance-1.0.3/conformance/programs/program-test.html.ini b/tests/wpt/mozilla/meta/webgl/conformance-1.0.3/conformance/programs/program-test.html.ini index a5f39d42b57..947742cb1e9 100644 --- a/tests/wpt/mozilla/meta/webgl/conformance-1.0.3/conformance/programs/program-test.html.ini +++ b/tests/wpt/mozilla/meta/webgl/conformance-1.0.3/conformance/programs/program-test.html.ini @@ -2,9 +2,6 @@ [WebGL test #53: getError expected: INVALID_OPERATION. Was NO_ERROR : drawing with a null program should generate INVALID_OPERATION] expected: FAIL - [WebGL test #58: linking should fail with in-use formerly good program, with new bad shader attached] - expected: FAIL - [WebGL test #64: getError expected: NO_ERROR. Was INVALID_OPERATION : delete the current program shouldn't change the current rendering state] expected: FAIL