Auto merge of #20539 - servo:webgl, r=emilio

Moar WebGL improvements, again

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20539)
<!-- Reviewable:end -->

components/canvas/webgl_thread.rs

@@ -678,8 +678,9 @@ impl WebGLImpl {
                 ctx.gl().clear(mask),
             WebGLCommand::ClearColor(r, g, b, a) =>
                 ctx.gl().clear_color(r, g, b, a),
-            WebGLCommand::ClearDepth(depth) =>
+            WebGLCommand::ClearDepth(depth) => {
-                ctx.gl().clear_depth(depth),
+                ctx.gl().clear_depth(depth.max(0.).min(1.) as f64)
+            }
             WebGLCommand::ClearStencil(stencil) =>
                 ctx.gl().clear_stencil(stencil),
             WebGLCommand::ColorMask(r, g, b, a) =>
@@ -694,8 +695,9 @@ impl WebGLImpl {
                 ctx.gl().depth_func(func),
             WebGLCommand::DepthMask(flag) =>
                 ctx.gl().depth_mask(flag),
-            WebGLCommand::DepthRange(near, far) =>
+            WebGLCommand::DepthRange(near, far) => {
-                ctx.gl().depth_range(near, far),
+                ctx.gl().depth_range(near.max(0.).min(1.) as f64, far.max(0.).min(1.) as f64)
+            }
             WebGLCommand::Disable(cap) =>
                 ctx.gl().disable(cap),
             WebGLCommand::Enable(cap) =>
@@ -758,8 +760,6 @@ impl WebGLImpl {
                 Self::vertex_attrib(ctx.gl(), index, pname, chan),
             WebGLCommand::GetVertexAttribOffset(index, pname, chan) =>
                 Self::vertex_attrib_offset(ctx.gl(), index, pname, chan),
-            WebGLCommand::GetBufferParameter(target, param_id, chan) =>
-                Self::buffer_parameter(ctx.gl(), target, param_id, chan),
             WebGLCommand::GetParameter(param_id, chan) =>
                 Self::parameter(ctx.gl(), param_id, chan),
             WebGLCommand::GetTexParameter(target, pname, chan) =>
@@ -1127,15 +1127,6 @@ impl WebGLImpl {
         chan.send(result).unwrap();
     }
 
-    fn buffer_parameter(gl: &gl::Gl,
-                        target: u32,
-                        param_id: u32,
-                        chan: WebGLSender<i32>) {
-        let result = gl.get_buffer_parameter_iv(target, param_id);
-
-        chan.send(result).unwrap();
-    }
-
     fn program_parameter(gl: &gl::Gl,
                          program_id: WebGLProgramId,
                          param_id: u32,

components/canvas_traits/webgl.rs

@@ -170,14 +170,14 @@ pub enum WebGLCommand {
     BufferSubData(u32, isize, ByteBuf),
     Clear(u32),
     ClearColor(f32, f32, f32, f32),
-    ClearDepth(f64),
+    ClearDepth(f32),
     ClearStencil(i32),
     ColorMask(bool, bool, bool, bool),
     CullFace(u32),
     FrontFace(u32),
     DepthFunc(u32),
     DepthMask(bool),
-    DepthRange(f64, f64),
+    DepthRange(f32, f32),
     Enable(u32),
     Disable(u32),
     CompileShader(WebGLShaderId, String),
@@ -205,7 +205,6 @@ pub enum WebGLCommand {
     EnableVertexAttribArray(u32),
     FramebufferRenderbuffer(u32, u32, u32, Option<WebGLRenderbufferId>),
     FramebufferTexture2D(u32, u32, u32, Option<WebGLTextureId>, i32),
-    GetBufferParameter(u32, u32, WebGLSender<i32>),
     GetExtensions(WebGLSender<String>),
     GetParameter(u32, WebGLSender<WebGLResult<WebGLParameter>>),
     GetTexParameter(u32, u32, WebGLSender<i32>),
@@ -479,7 +478,6 @@ impl fmt::Debug for WebGLCommand {
             EnableVertexAttribArray(..) => "EnableVertexAttribArray",
             FramebufferRenderbuffer(..) => "FramebufferRenderbuffer",
             FramebufferTexture2D(..) => "FramebufferTexture2D",
-            GetBufferParameter(..) => "GetBufferParameter",
             GetExtensions(..) => "GetExtensions",
             GetParameter(..) => "GetParameter",
             GetTexParameter(..) => "GetTexParameter",

components/script/dom/webglbuffer.rs

@@ -30,6 +30,8 @@ pub struct WebGLBuffer {
     pending_delete: Cell<bool>,
     #[ignore_malloc_size_of = "Defined in ipc-channel"]
     renderer: WebGLMsgSender,
+    /// https://www.khronos.org/registry/OpenGL-Refpages/es2.0/xhtml/glGetBufferParameteriv.xml
+    usage: Cell<u32>,
 }
 
 impl WebGLBuffer {
@@ -45,6 +47,7 @@ impl WebGLBuffer {
             vao_references: DomRefCell::new(None),
             pending_delete: Cell::new(false),
             renderer: renderer,
+            usage: Cell::new(WebGLRenderingContextConstants::STATIC_DRAW),
         }
     }
 
@@ -105,6 +108,7 @@ impl WebGLBuffer {
         }
         let data = data.into();
         self.capacity.set(data.len());
+        self.usage.set(usage);
         self.renderer.send(WebGLCommand::BufferData(target, data.into(), usage)).unwrap();
 
         Ok(())
@@ -158,6 +162,10 @@ impl WebGLBuffer {
             }
         }
     }
+
+    pub fn usage(&self) -> u32 {
+        self.usage.get()
+    }
 }
 
 impl Drop for WebGLBuffer {

components/script/dom/webglrenderingcontext.rs

@@ -1113,7 +1113,7 @@ impl WebGLRenderingContext {
         match cap {
             constants::BLEND | constants::CULL_FACE | constants::DEPTH_TEST | constants::DITHER |
             constants::POLYGON_OFFSET_FILL | constants::SAMPLE_ALPHA_TO_COVERAGE | constants::SAMPLE_COVERAGE |
-            constants::SAMPLE_COVERAGE_INVERT | constants::SCISSOR_TEST | constants::STENCIL_TEST => true,
+            constants::SCISSOR_TEST | constants::STENCIL_TEST => true,
             _ => {
                 self.webgl_error(InvalidEnum);
                 false
@@ -1250,30 +1250,20 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
         target: u32,
         parameter: u32,
     ) -> JSVal {
-        let buffer = handle_potential_webgl_error!(self, self.bound_buffer(target), return NullValue());
+        let buffer = handle_potential_webgl_error!(
+            self,
+            self.bound_buffer(target).and_then(|buf| buf.ok_or(InvalidOperation)),
+            return NullValue()
+        );
 
         match parameter {
-            constants::BUFFER_SIZE | constants::BUFFER_USAGE => {},
+            constants::BUFFER_SIZE => Int32Value(buffer.capacity() as i32),
+            constants::BUFFER_USAGE => Int32Value(buffer.usage() as i32),
             _ => {
                 self.webgl_error(InvalidEnum);
-                return NullValue();
+                NullValue()
             }
         }
-        let buffer = match buffer {
-            Some(buffer) => buffer,
-            None => {
-                self.webgl_error(InvalidOperation);
-                return NullValue();
-            }
-        };
-
-        if parameter == constants::BUFFER_SIZE {
-            return Int32Value(buffer.capacity() as i32);
-        }
-
-        let (sender, receiver) = webgl_channel().unwrap();
-        self.send_command(WebGLCommand::GetBufferParameter(target, parameter, sender));
-        Int32Value(receiver.recv().unwrap())
     }
 
     #[allow(unsafe_code)]
@@ -1487,19 +1477,30 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
     fn BlendEquation(&self, mode: u32) {
-        if mode != constants::FUNC_ADD {
+        match mode {
-            return self.webgl_error(InvalidEnum);
+            constants::FUNC_ADD |
+            constants::FUNC_SUBTRACT |
+            constants::FUNC_REVERSE_SUBTRACT => {
+                self.send_command(WebGLCommand::BlendEquation(mode))
+            }
+            _ => self.webgl_error(InvalidEnum),
         }
-
-        self.send_command(WebGLCommand::BlendEquation(mode));
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
     fn BlendEquationSeparate(&self, mode_rgb: u32, mode_alpha: u32) {
-        if mode_rgb != constants::FUNC_ADD || mode_alpha != constants::FUNC_ADD {
+        match mode_rgb {
-            return self.webgl_error(InvalidEnum);
+            constants::FUNC_ADD |
+            constants::FUNC_SUBTRACT |
+            constants::FUNC_REVERSE_SUBTRACT => {},
+            _ => return self.webgl_error(InvalidEnum),
+        }
+        match mode_alpha {
+            constants::FUNC_ADD |
+            constants::FUNC_SUBTRACT |
+            constants::FUNC_REVERSE_SUBTRACT => {},
+            _ => return self.webgl_error(InvalidEnum),
         }
-
         self.send_command(WebGLCommand::BlendEquationSeparate(mode_rgb, mode_alpha));
     }
 
@@ -1875,7 +1876,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
     fn ClearDepth(&self, depth: f32) {
-        self.send_command(WebGLCommand::ClearDepth(depth as f64))
+        self.send_command(WebGLCommand::ClearDepth(depth))
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
@@ -1924,19 +1925,15 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
     fn DepthRange(&self, near: f32, far: f32) {
-        // From the WebGL 1.0 spec, 6.12: Viewport Depth Range:
+        // https://www.khronos.org/registry/webgl/specs/latest/1.0/#VIEWPORT_DEPTH_RANGE
-        //
-        //     "A call to depthRange will generate an
-        //      INVALID_OPERATION error if zNear is greater than
-        //      zFar."
         if near > far {
             return self.webgl_error(InvalidOperation);
         }
-
+        self.send_command(WebGLCommand::DepthRange(near, far))
-        self.send_command(WebGLCommand::DepthRange(near as f64, far as f64))
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
+    // FIXME: https://github.com/servo/servo/issues/20534
     fn Enable(&self, cap: u32) {
         if self.validate_feature_enum(cap) {
             self.send_command(WebGLCommand::Enable(cap));
@@ -1944,6 +1941,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
+    // FIXME: https://github.com/servo/servo/issues/20534
     fn Disable(&self, cap: u32) {
         if self.validate_feature_enum(cap) {
             self.send_command(WebGLCommand::Disable(cap));
@@ -2500,8 +2498,8 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext {
         buffer.map_or(false, |buf| buf.target().is_some() && !buf.is_deleted())
     }
 
-    // TODO: We could write this without IPC, recording the calls to `enable` and `disable`.
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
+    // FIXME: https://github.com/servo/servo/issues/20534
     fn IsEnabled(&self, cap: u32) -> bool {
         if self.validate_feature_enum(cap) {
             let (sender, receiver) = webgl_channel().unwrap();