Fix WebGL tests & Implement WebGLRenderingContext::{validateProgram, getProgramInfoLog, disableVertexAttribArray}, r=emilio

3 files changed, 207 insertions, 29 deletions

diff --git a/components/script/dom/webglprogram.rs b/components/script/dom/webglprogram.rs
index 9ac3f4e1ad4..6382897efb3 100644
--- a/components/script/dom/webglprogram.rs
+++ b/components/script/dom/webglprogram.rs
@@ -24,6 +24,7 @@ pub struct WebGLProgram {
     webgl_object: WebGLObject,
     id: WebGLProgramId,
     is_deleted: Cell<bool>,
+    link_called: Cell<bool>,
     linked: Cell<bool>,
     fragment_shader: MutNullableHeap<JS<WebGLShader>>,
     vertex_shader: MutNullableHeap<JS<WebGLShader>>,
@@ -39,6 +40,7 @@ impl WebGLProgram {
             webgl_object: WebGLObject::new_inherited(),
             id: id,
             is_deleted: Cell::new(false),
+            link_called: Cell::new(false),
             linked: Cell::new(false),
             fragment_shader: Default::default(),
             vertex_shader: Default::default(),
@@ -91,27 +93,38 @@ impl WebGLProgram {
         self.is_deleted.get()
     }
 
+    pub fn is_linked(&self) -> bool {
+        self.linked.get()
+    }
+
     /// glLinkProgram
-    pub fn link(&self) {
+    pub fn link(&self) -> WebGLResult<()>  {
+        if self.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
         self.linked.set(false);
+        self.link_called.set(true);
 
         match self.fragment_shader.get() {
             Some(ref shader) if shader.successfully_compiled() => {},
-            _ => return,
+            _ => return Ok(()), // callers use gl.LINK_STATUS to check link errors
         }
 
         match self.vertex_shader.get() {
             Some(ref shader) if shader.successfully_compiled() => {},
-            _ => return,
+            _ => return Ok(()), // callers use gl.LINK_STATUS to check link errors
         }
 
         self.linked.set(true);
-
         self.renderer.send(CanvasMsg::WebGL(WebGLCommand::LinkProgram(self.id))).unwrap();
+        Ok(())
     }
 
     /// glUseProgram
     pub fn use_program(&self) -> WebGLResult<()> {
+        if self.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
         if !self.linked.get() {
             return Err(WebGLError::InvalidOperation);
         }
@@ -120,8 +133,20 @@ impl WebGLProgram {
         Ok(())
     }
 
+    /// glValidateProgram
+    pub fn validate(&self) -> WebGLResult<()> {
+        if self.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
+        self.renderer.send(CanvasMsg::WebGL(WebGLCommand::ValidateProgram(self.id))).unwrap();
+        Ok(())
+    }
+
     /// glAttachShader
     pub fn attach_shader(&self, shader: &WebGLShader) -> WebGLResult<()> {
+        if self.is_deleted() || shader.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
         let shader_slot = match shader.gl_type() {
             constants::FRAGMENT_SHADER => &self.fragment_shader,
             constants::VERTEX_SHADER => &self.vertex_shader,
@@ -147,6 +172,9 @@ impl WebGLProgram {
 
     /// glDetachShader
     pub fn detach_shader(&self, shader: &WebGLShader) -> WebGLResult<()> {
+        if self.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
         let shader_slot = match shader.gl_type() {
             constants::FRAGMENT_SHADER => &self.fragment_shader,
             constants::VERTEX_SHADER => &self.vertex_shader,
@@ -174,6 +202,9 @@ impl WebGLProgram {
 
     /// glBindAttribLocation
     pub fn bind_attrib_location(&self, index: u32, name: DOMString) -> WebGLResult<()> {
+        if self.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
         if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN {
             return Err(WebGLError::InvalidValue);
         }
@@ -190,6 +221,9 @@ impl WebGLProgram {
     }
 
     pub fn get_active_uniform(&self, index: u32) -> WebGLResult<Root<WebGLActiveInfo>> {
+        if self.is_deleted() {
+            return Err(WebGLError::InvalidValue);
+        }
         let (sender, receiver) = ipc::channel().unwrap();
         self.renderer
             .send(CanvasMsg::WebGL(WebGLCommand::GetActiveUniform(self.id, index, sender)))
@@ -201,6 +235,9 @@ impl WebGLProgram {
 
     /// glGetActiveAttrib
     pub fn get_active_attrib(&self, index: u32) -> WebGLResult<Root<WebGLActiveInfo>> {
+        if self.is_deleted() {
+            return Err(WebGLError::InvalidValue);
+        }
         let (sender, receiver) = ipc::channel().unwrap();
         self.renderer
             .send(CanvasMsg::WebGL(WebGLCommand::GetActiveAttrib(self.id, index, sender)))
@@ -212,6 +249,9 @@ impl WebGLProgram {
 
     /// glGetAttribLocation
     pub fn get_attrib_location(&self, name: DOMString) -> WebGLResult<Option<i32>> {
+        if !self.is_linked() || self.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
         if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN {
             return Err(WebGLError::InvalidValue);
         }
@@ -234,6 +274,9 @@ impl WebGLProgram {
 
     /// glGetUniformLocation
     pub fn get_uniform_location(&self, name: DOMString) -> WebGLResult<Option<i32>> {
+        if !self.is_linked() || self.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
         if name.len() > MAX_UNIFORM_AND_ATTRIBUTE_LEN {
             return Err(WebGLError::InvalidValue);
         }
@@ -250,6 +293,25 @@ impl WebGLProgram {
         Ok(receiver.recv().unwrap())
     }
 
+    /// glGetProgramInfoLog
+    pub fn get_info_log(&self) -> WebGLResult<String> {
+        if self.is_deleted() {
+            return Err(WebGLError::InvalidOperation);
+        }
+        if self.link_called.get() {
+            let shaders_compiled = match (self.fragment_shader.get(), self.vertex_shader.get()) {
+                (Some(fs), Some(vs)) => fs.successfully_compiled() && vs.successfully_compiled(),
+                _ => false
+            };
+            if !shaders_compiled {
+                return Ok("One or more shaders failed to compile".to_string());
+            }
+        }
+        let (sender, receiver) = ipc::channel().unwrap();
+        self.renderer.send(CanvasMsg::WebGL(WebGLCommand::GetProgramInfoLog(self.id, sender))).unwrap();
+        Ok(receiver.recv().unwrap())
+    }
+
     /// glGetProgramParameter
     pub fn parameter(&self, param_id: u32) -> WebGLResult<WebGLParameter> {
         let (sender, receiver) = ipc::channel().unwrap();
diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs
index d1342190f73..b69f11146fd 100644
--- a/components/script/dom/webglrenderingcontext.rs
+++ b/components/script/dom/webglrenderingcontext.rs
@@ -1281,15 +1281,36 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             return self.webgl_error(InvalidOperation);
         }
 
-        if count <= 0 {
-            return self.webgl_error(InvalidOperation);
+        if count < 0 {
+            return self.webgl_error(InvalidValue);
         }
 
         if offset < 0 {
             return self.webgl_error(InvalidValue);
         }
 
-        if self.current_program.get().is_none() || self.bound_buffer_element_array.get().is_none() {
+        if self.current_program.get().is_none() {
+            // From the WebGL spec
+            //
+            //     If the CURRENT_PROGRAM is null, an INVALID_OPERATION error will be generated.
+            //     WebGL performs additional error checking beyond that specified
+            //     in OpenGL ES 2.0 during calls to drawArrays and drawElements.
+            //
+            return self.webgl_error(InvalidOperation);
+        }
+
+        if let Some(array_buffer) = self.bound_buffer_element_array.get() {
+            // WebGL Spec: check buffer overflows, must be a valid multiple of the size.
+            let val = offset as u64 + (count as u64 * type_size as u64);
+            if val > array_buffer.capacity() as u64 {
+                return self.webgl_error(InvalidOperation);
+            }
+        } else {
+            // From the WebGL spec
+            //
+            //      a non-null WebGLBuffer must be bound to the ELEMENT_ARRAY_BUFFER binding point
+            //      or an INVALID_OPERATION error will be generated.
+            //
             return self.webgl_error(InvalidOperation);
         }
 
@@ -1323,25 +1344,68 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
+    fn DisableVertexAttribArray(&self, attrib_id: u32) {
+        if attrib_id > self.limits.max_vertex_attribs {
+            return self.webgl_error(InvalidValue);
+        }
+
+        self.ipc_renderer
+            .send(CanvasMsg::WebGL(WebGLCommand::DisableVertexAttribArray(attrib_id)))
+            .unwrap()
+    }
+
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
     fn GetActiveUniform(&self, program: Option<&WebGLProgram>, index: u32) -> Option<Root<WebGLActiveInfo>> {
-        program.and_then(|p| match p.get_active_uniform(index) {
+        let program = match program {
+            Some(program) => program,
+            None => {
+                // Reasons to generate InvalidValue error
+                // From the GLES 2.0 spec
+                //
+                //     "INVALID_VALUE is generated if index is greater than or equal
+                //      to the number of active uniform variables in program"
+                //
+                // A null program has no uniforms so any index is always greater than the active uniforms
+                // WebGl conformance expects error with null programs. Check tests in get-active-test.html
+                self.webgl_error(InvalidValue);
+                return None;
+             }
+        };
+
+        match program.get_active_uniform(index) {
             Ok(ret) => Some(ret),
-            Err(error) => {
-                self.webgl_error(error);
-                None
-            },
-        })
+            Err(e) => {
+                self.webgl_error(e);
+                return None;
+            }
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
     fn GetActiveAttrib(&self, program: Option<&WebGLProgram>, index: u32) -> Option<Root<WebGLActiveInfo>> {
-        program.and_then(|p| match p.get_active_attrib(index) {
+        let program = match program {
+            Some(program) => program,
+            None => {
+                // Reasons to generate InvalidValue error
+                // From the GLES 2.0 spec
+                //
+                //     "INVALID_VALUE is generated if index is greater than or equal
+                //      to the number of active attribute variables in program"
+                //
+                // A null program has no attributes so any index is always greater than the active uniforms
+                // WebGl conformance expects error with null programs. Check tests in get-active-test.html
+                self.webgl_error(InvalidValue);
+                return None;
+             }
+        };
+
+        match program.get_active_attrib(index) {
             Ok(ret) => Some(ret),
-            Err(error) => {
-                self.webgl_error(error);
-                None
-            },
-        })
+            Err(e) => {
+                self.webgl_error(e);
+                return None;
+            }
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
@@ -1354,6 +1418,22 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
+    fn GetProgramInfoLog(&self, program: Option<&WebGLProgram>) -> Option<DOMString> {
+        if let Some(program) = program {
+            match program.get_info_log() {
+                Ok(value) => Some(DOMString::from(value)),
+                Err(e) => {
+                    self.webgl_error(e);
+                    None
+                }
+            }
+        } else {
+            self.webgl_error(WebGLError::InvalidValue);
+            None
+        }
+    }
+
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn GetProgramParameter(&self, _: *mut JSContext, program: Option<&WebGLProgram>, param_id: u32) -> JSVal {
         if let Some(program) = program {
             match handle_potential_webgl_error!(self, program.parameter(param_id), WebGLParameter::Invalid) {
@@ -1699,7 +1779,9 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn LinkProgram(&self, program: Option<&WebGLProgram>) {
         if let Some(program) = program {
-            program.link()
+            if let Err(e) = program.link() {
+                self.webgl_error(e);
+            }
         }
     }
 
@@ -1940,6 +2022,15 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
         }
     }
 
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
+    fn ValidateProgram(&self, program: Option<&WebGLProgram>) {
+        if let Some(program) = program {
+            if let Err(e) = program.validate() {
+                self.webgl_error(e);
+            }
+        }
+    }
+
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
     fn VertexAttrib1f(&self, indx: u32, x: f32) {
         self.vertex_attrib(indx, x, 0f32, 0f32, 1f32)
@@ -2016,13 +2107,38 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             return self.webgl_error(InvalidValue);
         }
 
-        if let constants::FLOAT = data_type {
-           let msg = CanvasMsg::WebGL(
-               WebGLCommand::VertexAttribPointer2f(attrib_id, size, normalized, stride, offset as u32));
-            self.ipc_renderer.send(msg).unwrap()
-        } else {
-            panic!("VertexAttribPointer: Data Type not supported")
+        // GLES spec: If offset or stride  is negative, an INVALID_VALUE error will be generated
+        // WebGL spec: the maximum supported stride is 255
+        if stride < 0 || stride > 255 || offset < 0 {
+            return self.webgl_error(InvalidValue);
+        }
+        if size < 1 || size > 4 {
+            return self.webgl_error(InvalidValue);
+        }
+        if self.bound_buffer_array.get().is_none() {
+            return self.webgl_error(InvalidOperation);
+        }
+
+        // stride and offset must be multiple of data_type
+        match data_type {
+            constants::BYTE | constants::UNSIGNED_BYTE => {},
+            constants::SHORT | constants::UNSIGNED_SHORT => {
+                if offset % 2 > 0 || stride % 2 > 0 {
+                    return self.webgl_error(InvalidOperation);
+                }
+            },
+            constants::FLOAT => {
+                if offset % 4 > 0 || stride % 4 > 0 {
+                    return self.webgl_error(InvalidOperation);
+                }
+            },
+            _ => return self.webgl_error(InvalidEnum),
+
         }
+
+        let msg = CanvasMsg::WebGL(
+                    WebGLCommand::VertexAttribPointer(attrib_id, size, data_type, normalized, stride, offset as u32));
+        self.ipc_renderer.send(msg).unwrap()
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.4
diff --git a/components/script/dom/webidls/WebGLRenderingContext.webidl b/components/script/dom/webidls/WebGLRenderingContext.webidl
index 06727b3a536..19e5f029168 100644
--- a/components/script/dom/webidls/WebGLRenderingContext.webidl
+++ b/components/script/dom/webidls/WebGLRenderingContext.webidl
@@ -552,7 +552,7 @@ interface WebGLRenderingContextBase
     void depthRange(GLclampf zNear, GLclampf zFar);
     void detachShader(WebGLProgram? program, WebGLShader? shader);
     void disable(GLenum cap);
-    //void disableVertexAttribArray(GLuint index);
+    void disableVertexAttribArray(GLuint index);
     void drawArrays(GLenum mode, GLint first, GLsizei count);
     void drawElements(GLenum mode, GLsizei count, GLenum type, GLintptr offset);
 
@@ -583,7 +583,7 @@ interface WebGLRenderingContextBase
     //any getFramebufferAttachmentParameter(GLenum target, GLenum attachment,
     //                                      GLenum pname);
     any getProgramParameter(WebGLProgram? program, GLenum pname);
-    //DOMString? getProgramInfoLog(WebGLProgram? program);
+    DOMString? getProgramInfoLog(WebGLProgram? program);
     //any getRenderbufferParameter(GLenum target, GLenum pname);
     any getShaderParameter(WebGLShader? shader, GLenum pname);
     //WebGLShaderPrecisionFormat? getShaderPrecisionFormat(GLenum shadertype, GLenum precisiontype);
@@ -704,7 +704,7 @@ interface WebGLRenderingContextBase
     //                      sequence<GLfloat> value);
 
     void useProgram(WebGLProgram? program);
-    //void validateProgram(WebGLProgram? program);
+    void validateProgram(WebGLProgram? program);
 
     // FIXME(dmarcos)
     // The code generator doesn't handle Float32Array so we're using 'object'