Auto merge of #9107 - ecoal95:webgl-uniforms-and-nits, r=simartin

webgl: Track the current program, implement some uniform functions, and nits

Was done while implementing sequence arguments.

Depends on #9056.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9107)
<!-- Reviewable:end -->

components/canvas/webgl_paint_thread.rs

@@ -46,6 +46,8 @@ impl WebGLPaintThread {
     /// NB: Not gl-related validations (names, lengths, accepted parameters...) are
     /// done in the corresponding DOM interfaces
     pub fn handle_webgl_message(&self, message: CanvasWebGLMsg) {
+        debug!("WebGL message: {:?}", message);
+
         match message {
             CanvasWebGLMsg::GetContextAttributes(sender) =>
                 self.context_attributes(sender),
@@ -157,8 +159,10 @@ impl WebGLPaintThread {
                 gl::bind_texture(target, id),
             CanvasWebGLMsg::LinkProgram(program_id) =>
                 gl::link_program(program_id),
-            CanvasWebGLMsg::Uniform4fv(uniform_id, data) =>
+            CanvasWebGLMsg::Uniform1f(uniform_id, x) =>
-                gl::uniform_4f(uniform_id, data[0], data[1], data[2], data[3]),
+                gl::uniform_1f(uniform_id, x),
+            CanvasWebGLMsg::Uniform4f(uniform_id, x, y, z, w) =>
+                gl::uniform_4f(uniform_id, x, y, z, w),
             CanvasWebGLMsg::UseProgram(program_id) =>
                 gl::use_program(program_id),
             CanvasWebGLMsg::VertexAttrib(attrib_id, x, y, z, w) =>
@@ -179,8 +183,9 @@ impl WebGLPaintThread {
                 self.send_drawing_buffer_height(sender),
         }
 
-        // FIXME: Convert to `debug_assert!` once tests are run with debug assertions
+        // FIXME: Use debug_assertions once tests are run with them
-        assert!(gl::get_error() == gl::NO_ERROR);
+        let error = gl::get_error();
+        assert!(error == gl::NO_ERROR, "Unexpected WebGL error: 0x{:x} ({})", error, error);
     }
 
     /// Creates a new `WebGLPaintThread` and returns the out-of-process sender and the in-process
@@ -297,6 +302,7 @@ impl WebGLPaintThread {
         } else {
             Some(unsafe { NonZero::new(program) })
         };
+
         chan.send(program).unwrap();
     }
 

components/canvas_traits/lib.rs

@@ -37,6 +37,7 @@ use layers::platform::surface::NativeSurface;
 use offscreen_gl_context::GLContextAttributes;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use std::default::Default;
+use std::fmt;
 use std::str::FromStr;
 use std::sync::mpsc::Sender;
 use util::mem::HeapSizeOf;
@@ -180,7 +181,8 @@ pub enum CanvasWebGLMsg {
     LineWidth(f32),
     PixelStorei(u32, i32),
     LinkProgram(u32),
-    Uniform4fv(i32, Vec<f32>),
+    Uniform1f(i32, f32),
+    Uniform4f(i32, f32, f32, f32, f32),
     UseProgram(u32),
     VertexAttrib(u32, f32, f32, f32, f32),
     VertexAttribPointer2f(u32, i32, bool, i32, u32),
@@ -192,6 +194,82 @@ pub enum CanvasWebGLMsg {
     DrawingBufferHeight(IpcSender<i32>),
 }
 
+impl fmt::Debug for CanvasWebGLMsg {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use CanvasWebGLMsg::*;
+        let name = match *self {
+            GetContextAttributes(..) => "GetContextAttributes",
+            ActiveTexture(..) => "ActiveTexture",
+            BlendColor(..) => "BlendColor",
+            BlendEquation(..) => "BlendEquation",
+            BlendEquationSeparate(..) => "BlendEquationSeparate",
+            BlendFunc(..) => "BlendFunc",
+            BlendFuncSeparate(..) => "BlendFuncSeparate",
+            AttachShader(..) => "AttachShader",
+            BindAttribLocation(..) => "BindAttribLocation",
+            BufferData(..) => "BufferData",
+            BufferSubData(..) => "BufferSubData",
+            Clear(..) => "Clear",
+            ClearColor(..) => "ClearColor",
+            ClearDepth(..) => "ClearDepth",
+            ClearStencil(..) => "ClearStencil",
+            ColorMask(..) => "ColorMask",
+            CullFace(..) => "CullFace",
+            FrontFace(..) => "FrontFace",
+            DepthFunc(..) => "DepthFunc",
+            DepthMask(..) => "DepthMask",
+            DepthRange(..) => "DepthRange",
+            Enable(..) => "Enable",
+            Disable(..) => "Disable",
+            CompileShader(..) => "CompileShader",
+            CreateBuffer(..) => "CreateBuffer",
+            CreateFramebuffer(..) => "CreateFramebuffer",
+            CreateRenderbuffer(..) => "CreateRenderbuffer",
+            CreateTexture(..) => "CreateTexture",
+            CreateProgram(..) => "CreateProgram",
+            CreateShader(..) => "CreateShader",
+            DeleteBuffer(..) => "DeleteBuffer",
+            DeleteFramebuffer(..) => "DeleteFramebuffer",
+            DeleteRenderbuffer(..) => "DeleteRenderBuffer",
+            DeleteTexture(..) => "DeleteTexture",
+            DeleteProgram(..) => "DeleteProgram",
+            DeleteShader(..) => "DeleteShader",
+            BindBuffer(..) => "BindBuffer",
+            BindFramebuffer(..) => "BindFramebuffer",
+            BindRenderbuffer(..) => "BindRenderbuffer",
+            BindTexture(..) => "BindTexture",
+            DrawArrays(..) => "DrawArrays",
+            DrawElements(..) => "DrawElements",
+            EnableVertexAttribArray(..) => "EnableVertexAttribArray",
+            GetBufferParameter(..) => "GetBufferParameter",
+            GetParameter(..) => "GetParameter",
+            GetProgramParameter(..) => "GetProgramParameter",
+            GetShaderParameter(..) => "GetShaderParameter",
+            GetAttribLocation(..) => "GetAttribLocation",
+            GetUniformLocation(..) => "GetUniformLocation",
+            PolygonOffset(..) => "PolygonOffset",
+            Scissor(..) => "Scissor",
+            Hint(..) => "Hint",
+            LineWidth(..) => "LineWidth",
+            PixelStorei(..) => "PixelStorei",
+            LinkProgram(..) => "LinkProgram",
+            Uniform4f(..) => "Uniform4f",
+            Uniform1f(..) => "Uniform1f",
+            UseProgram(..) => "UseProgram",
+            VertexAttrib(..) => "VertexAttrib",
+            VertexAttribPointer2f(..) => "VertexAttribPointer2f",
+            Viewport(..) => "Viewport",
+            TexImage2D(..) => "TexImage2D",
+            TexParameteri(..) => "TexParameteri",
+            TexParameterf(..) => "TexParameterf",
+            DrawingBufferWidth(..) => "DrawingBufferWidth",
+            DrawingBufferHeight(..) => "DrawingBufferHeight",
+        };
+
+        write!(f, "CanvasWebGLMsg::{}(..)", name)
+    }
+}
+
 #[derive(Clone, Copy, PartialEq, Deserialize, Serialize, HeapSizeOf)]
 pub enum WebGLError {
     InvalidEnum,

components/script/dom/webglprogram.rs

@@ -55,6 +55,10 @@ impl WebGLProgram {
 
 
 impl WebGLProgram {
+    pub fn id(&self) -> u32 {
+        self.id
+    }
+
     /// glDeleteProgram
     pub fn delete(&self) {
         if !self.is_deleted.get() {
@@ -69,8 +73,19 @@ impl WebGLProgram {
     }
 
     /// glUseProgram
-    pub fn use_program(&self) {
+    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),
+        }
+
         self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::UseProgram(self.id))).unwrap();
+        Ok(())
     }
 
     /// glAttachShader

components/script/dom/webglrenderingcontext.rs

@@ -79,6 +79,7 @@ pub struct WebGLRenderingContext {
     bound_texture_cube_map: MutNullableHeap<JS<WebGLTexture>>,
     bound_buffer_array: MutNullableHeap<JS<WebGLBuffer>>,
     bound_buffer_element_array: MutNullableHeap<JS<WebGLBuffer>>,
+    current_program: MutNullableHeap<JS<WebGLProgram>>,
 }
 
 impl WebGLRenderingContext {
@@ -106,6 +107,7 @@ impl WebGLRenderingContext {
                 bound_texture_cube_map: MutNullableHeap::new(None),
                 bound_buffer_array: MutNullableHeap::new(None),
                 bound_buffer_element_array: MutNullableHeap::new(None),
+                current_program: MutNullableHeap::new(None),
             }
         })
     }
@@ -284,8 +286,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.14
     fn GetExtension(&self, _cx: *mut JSContext, _name: DOMString) -> *mut JSObject {
-        // TODO(ecoal95) we actually do not support extensions.
-        // `getSupportedExtensions` cannot be implemented as of right now (see #544)
         0 as *mut JSObject
     }
 
@@ -638,8 +638,6 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
         WebGLProgram::maybe_new(self.global().r(), self.ipc_renderer.clone())
     }
 
-    // TODO(ecoal95): Check if constants are cross-platform or if we must make a translation
-    // between WebGL constants and native ones.
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn CreateShader(&self, shader_type: u32) -> Option<Root<WebGLShader>> {
         WebGLShader::maybe_new(self.global().r(), self.ipc_renderer.clone(), shader_type)
@@ -694,8 +692,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             constants::LINE_LOOP | constants::LINES |
             constants::TRIANGLE_STRIP | constants::TRIANGLE_FAN |
             constants::TRIANGLES => {
-                // TODO(ecoal95): Check the CURRENT_PROGRAM when we keep track of it, and if it's
+                if self.current_program.get().is_none() {
-                // null generate an InvalidOperation error
+                    return self.webgl_error(InvalidOperation);
+                }
+
                 if first < 0 || count < 0 {
                     self.webgl_error(InvalidValue);
                 } else {
@@ -730,9 +730,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
             return self.webgl_error(InvalidValue);
         }
 
-        // TODO ensure a non-null WebGLBuffer must be bound to the ELEMENT_ARRAY_BUFFER
+        if self.current_program.get().is_none() || self.bound_buffer_element_array.get().is_none() {
-        // TODO(ecoal95): Check the CURRENT_PROGRAM when we keep track of it, and if it's
+            return self.webgl_error(InvalidOperation);
-        // null generate an InvalidOperation error
+        }
+
         match mode {
             constants::POINTS | constants::LINE_STRIP |
             constants::LINE_LOOP | constants::LINES |
@@ -804,7 +805,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
                           name: DOMString) -> Option<Root<WebGLUniformLocation>> {
         if let Some(program) = program {
             handle_potential_webgl_error!(self, program.get_uniform_location(name), None)
-                .map(|location| WebGLUniformLocation::new(self.global().r(), location))
+                .map(|location| WebGLUniformLocation::new(self.global().r(), location, program.id()))
         } else {
             None
         }
@@ -930,30 +931,71 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
         }
     }
 
-    #[allow(unsafe_code)]
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
+    fn Uniform1f(&self,
+                  uniform: Option<&WebGLUniformLocation>,
+                  val: f32) {
+        let uniform = match uniform {
+            Some(uniform) => uniform,
+            None => return,
+        };
+
+        match self.current_program.get() {
+            Some(ref program) if program.id() == uniform.program_id() => {},
+            _ => return self.webgl_error(InvalidOperation),
+        };
+
+        self.ipc_renderer
+            .send(CanvasMsg::WebGL(CanvasWebGLMsg::Uniform1f(uniform.id(), val)))
+            .unwrap()
+    }
+
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
+    fn Uniform1fv(&self,
+                  uniform: Option<&WebGLUniformLocation>,
+                  data: Vec<f32>) {
+        if data.is_empty() {
+            return self.webgl_error(InvalidValue);
+        }
+
+        self.Uniform1f(uniform, data[0]);
+    }
+
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
+    fn Uniform4f(&self,
+                  uniform: Option<&WebGLUniformLocation>,
+                  x: f32, y: f32, z: f32, w: f32) {
+        let uniform = match uniform {
+            Some(uniform) => uniform,
+            None => return,
+        };
+
+        match self.current_program.get() {
+            Some(ref program) if program.id() == uniform.program_id() => {},
+            _ => return self.webgl_error(InvalidOperation),
+        };
+
+        self.ipc_renderer
+            .send(CanvasMsg::WebGL(CanvasWebGLMsg::Uniform4f(uniform.id(), x, y, z, w)))
+            .unwrap()
+    }
+
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
     fn Uniform4fv(&self,
                   _cx: *mut JSContext,
                   uniform: Option<&WebGLUniformLocation>,
                   data: Option<*mut JSObject>) {
-        let uniform_id = match uniform {
-            Some(uniform) => uniform.id(),
-            None => return,
-        };
-
         let data = match data {
             Some(data) => data,
-            None => return,
+            None => return self.webgl_error(InvalidValue),
         };
 
-        if let Some(data_vec) = array_buffer_view_to_vec_checked::<f32>(data) {
+        if let Some(data) = array_buffer_view_to_vec_checked::<f32>(data) {
-            if data_vec.len() < 4 {
+            if data.len() < 4 {
                 return self.webgl_error(InvalidOperation);
             }
 
-            self.ipc_renderer
+            self.Uniform4f(uniform, data[0], data[1], data[2], data[3]);
-                .send(CanvasMsg::WebGL(CanvasWebGLMsg::Uniform4fv(uniform_id, data_vec)))
-                .unwrap()
         } else {
             self.webgl_error(InvalidValue);
         }
@@ -962,7 +1004,10 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn UseProgram(&self, program: Option<&WebGLProgram>) {
         if let Some(program) = program {
-            program.use_program()
+            match program.use_program() {
+                Ok(()) => self.current_program.set(Some(program)),
+                Err(e) => self.webgl_error(e),
+            }
         }
     }
 

components/script/dom/webglshader.rs

@@ -147,6 +147,10 @@ impl WebGLShader {
     pub fn set_source(&self, source: DOMString) {
         *self.source.borrow_mut() = Some(source);
     }
+
+    pub fn successfully_compiled(&self) -> bool {
+        self.compilation_status.get() == ShaderCompilationStatus::Succeeded
+    }
 }
 
 impl Drop for WebGLShader {

components/script/dom/webgluniformlocation.rs

@@ -12,18 +12,21 @@ use dom::bindings::reflector::{Reflector, reflect_dom_object};
 pub struct WebGLUniformLocation {
     reflector_: Reflector,
     id: i32,
+    program_id: u32,
 }
 
 impl WebGLUniformLocation {
-    fn new_inherited(id: i32) -> WebGLUniformLocation {
+    fn new_inherited(id: i32, program_id: u32) -> WebGLUniformLocation {
         WebGLUniformLocation {
             reflector_: Reflector::new(),
             id: id,
+            program_id: program_id,
         }
     }
 
-    pub fn new(global: GlobalRef, id: i32) -> Root<WebGLUniformLocation> {
+    pub fn new(global: GlobalRef, id: i32, program_id: u32) -> Root<WebGLUniformLocation> {
-        reflect_dom_object(box WebGLUniformLocation::new_inherited(id), global, WebGLUniformLocationBinding::Wrap)
+        reflect_dom_object(
+            box WebGLUniformLocation::new_inherited(id, program_id), global, WebGLUniformLocationBinding::Wrap)
     }
 }
 
@@ -32,4 +35,8 @@ impl WebGLUniformLocation {
     pub fn id(&self) -> i32 {
         self.id
     }
+
+    pub fn program_id(&self) -> u32 {
+        self.program_id
+    }
 }

components/script/dom/webidls/WebGLRenderingContext.webidl

@@ -644,9 +644,9 @@ interface WebGLRenderingContextBase
     //void texSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset,
     //                   GLenum format, GLenum type, TexImageSource? source); // May throw DOMException
 
-    //void uniform1f(WebGLUniformLocation? location, GLfloat x);
+    void uniform1f(WebGLUniformLocation? location, GLfloat x);
     //void uniform1fv(WebGLUniformLocation? location, Float32Array v);
-    //void uniform1fv(WebGLUniformLocation? location, sequence<GLfloat> v);
+    void uniform1fv(WebGLUniformLocation? location, sequence<GLfloat> v);
     //void uniform1i(WebGLUniformLocation? location, GLint x);
     //void uniform1iv(WebGLUniformLocation? location, Int32Array v);
     //void uniform1iv(WebGLUniformLocation? location, sequence<long> v);
@@ -662,9 +662,9 @@ interface WebGLRenderingContextBase
     //void uniform3i(WebGLUniformLocation? location, GLint x, GLint y, GLint z);
     //void uniform3iv(WebGLUniformLocation? location, Int32Array v);
     //void uniform3iv(WebGLUniformLocation? location, sequence<long> v);
-    //void uniform4f(WebGLUniformLocation? location, GLfloat x, GLfloat y, GLfloat z, GLfloat w);
+    void uniform4f(WebGLUniformLocation? location, GLfloat x, GLfloat y, GLfloat z, GLfloat w);
     // FIXME(dmarcos) The function below is the original function in the webIdl:
-    // void uniform4fv(WebGLUniformLocation? location, Float32Array v);
+    //void uniform4fv(WebGLUniformLocation? location, Float32Array v);
     // The Code genearator doesn't handle BufferDataSource so we're using 'optional object'
     // in the meantime
     void uniform4fv(WebGLUniformLocation? location, optional object v);
@@ -718,13 +718,3 @@ interface WebGLRenderingContext
 };
 WebGLRenderingContext implements WebGLRenderingContextBase;
 
-
-//[Constructor(DOMString type, optional WebGLContextEventInit eventInit)]
-//interface WebGLContextEvent : Event {
-//    readonly attribute DOMString statusMessage;
-//};
-
-// EventInit is defined in the DOM4 specification.
-//dictionary WebGLContextEventInit : EventInit {
-//    DOMString statusMessage;
-//};

tests/wpt/mozilla/tests/mozilla/webgl/draw_arrays_simple.html

@@ -11,6 +11,8 @@
 </style>
 <canvas id="c" width="512" height="512"></canvas>
 <script id="vertex_shader" type="x-shader/x-vertex">
+precision mediump float;
+
 attribute vec2 a_position;
 
 void main() {
@@ -19,6 +21,8 @@ void main() {
 </script>
 
 <script id="fragment_shader" type="x-shader/x-fragment">
+precision mediump float;
+
 void main() {
   gl_FragColor = vec4(0, 1, 0, 1);  // green
 }
@@ -67,5 +71,5 @@ void main() {
 
   gl.enableVertexAttribArray(position);
   gl.vertexAttribPointer(position, 2, gl.FLOAT, false, 0, 0);
-  gl.drawArrays(gl.TRIANGLES, 0, square_data.length);
+  gl.drawArrays(gl.TRIANGLES, 0, square_data.length / 2);
 </script>