Auto merge of #6380 - ecoal95:webgl-refactoring, r=jdm

Refactor WebGL implementation to move logic inside the DOM interfaces

This improves the encapsulation and consistency in our WebGL
implementation.

Also allows to implement new methods such as `getShaderSource()`.

It will also allow us to use `delete()` in the destructors of them (note
that we will probably want to keep track of them from the context before).

Some concerns:

**Trait method repetition**:
I'm aware that the traits `WebGL{Buffer,Renderbuffer,Framebuffer,Texture}Helpers` are basically the same, but `delete()` and `id()` methods are everywhere. I've thought something like:

```rust
pub trait WebGLIdentifiable {
    type WebGLId; // id is sometimes i32 (see WebGLUniformLocation)
    fn id(&self) -> Self::WebGLId;
}

pub trait WebGLBindable {
    fn bind(&self);
}

pub trait WebGLDeletable {
    fn delete(&self);
}
```

But I'd want to know your opinion first.

**`renderer` repetition**:
Thought of moving the field: `renderer: Sender<CanvasMsg>` to `WebGLObject`, but I think it makes it way more complicated to read, and also a bit unnecessary, at least IMO (`WebGLObject` will never interact with the field directly). It would also mean that all `WebGLObject`s should have one, which is true at this moment, but maybe not with WebGL 2, for example.

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

1 files changed, 129 insertions, 92 deletions

diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs
index 766c61fa57c..123c1e200c3 100644
--- a/components/script/dom/webglrenderingcontext.rs
+++ b/components/script/dom/webglrenderingcontext.rs
@@ -3,10 +3,13 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use canvas::webgl_paint_task::WebGLPaintTask;
-use canvas_traits::{CanvasMsg, CanvasWebGLMsg, CanvasCommonMsg};
-use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding;
-use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::{
-    WebGLContextAttributes, WebGLRenderingContextMethods, WebGLRenderingContextConstants};
+use canvas_traits::
+            {CanvasMsg, CanvasWebGLMsg, CanvasCommonMsg, WebGLError,
+             WebGLShaderParameter, WebGLFramebufferBindingRequest};
+use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::
+            {self, WebGLContextAttributes, WebGLRenderingContextMethods};
+use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants;
+
 use dom::bindings::global::{GlobalRef, GlobalField};
 use dom::bindings::js::{JS, LayoutJS, Root};
 use dom::bindings::utils::{Reflector, reflect_dom_object};
@@ -22,7 +25,8 @@ use dom::webgluniformlocation::{WebGLUniformLocation, WebGLUniformLocationHelper
 use euclid::size::Size2D;
 use js::jsapi::{JSContext, JSObject, RootedValue};
 use js::jsapi::{JS_GetFloat32ArrayData, JS_GetObjectAsArrayBufferView};
-use js::jsval::{JSVal, UndefinedValue, NullValue, Int32Value};
+use js::jsval::{JSVal, UndefinedValue, NullValue, Int32Value, BooleanValue};
+use std::cell::Cell;
 use std::mem;
 use std::ptr;
 use std::slice;
@@ -30,12 +34,27 @@ use std::sync::mpsc::{channel, Sender};
 use util::str::DOMString;
 use offscreen_gl_context::GLContextAttributes;
 
+pub const MAX_UNIFORM_AND_ATTRIBUTE_LEN: usize = 256;
+
+macro_rules! handle_potential_webgl_error {
+    ($context:ident, $call:expr, $return_on_error:expr) => {
+        match $call {
+            Ok(ret) => ret,
+            Err(error) => {
+                $context.handle_webgl_error(error);
+                $return_on_error
+            }
+        }
+    }
+}
+
 #[dom_struct]
 pub struct WebGLRenderingContext {
     reflector_: Reflector,
     global: GlobalField,
     renderer: Sender<CanvasMsg>,
     canvas: JS<HTMLCanvasElement>,
+    last_error: Cell<Option<WebGLError>>,
 }
 
 impl WebGLRenderingContext {
@@ -50,6 +69,7 @@ impl WebGLRenderingContext {
             reflector_: Reflector::new(),
             global: GlobalField::from_rooted(&global),
             renderer: chan,
+            last_error: Cell::new(None),
             canvas: JS::from_ref(canvas),
         })
     }
@@ -102,16 +122,33 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
         // TODO(ecoal95): Implement the missing parameters from the spec
         let mut rval = RootedValue::new(cx, UndefinedValue());
         match parameter {
-            WebGLRenderingContextConstants::VERSION =>
+            constants::VERSION =>
                 "WebGL 1.0".to_jsval(cx, rval.handle_mut()),
-            WebGLRenderingContextConstants::RENDERER |
-            WebGLRenderingContextConstants::VENDOR =>
+            constants::RENDERER |
+            constants::VENDOR =>
                 "Mozilla/Servo".to_jsval(cx, rval.handle_mut()),
             _ => rval.ptr = NullValue(),
         }
         rval.ptr
     }
 
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.3
+    fn GetError(self) -> u32 {
+        let error_code = if let Some(error) = self.last_error.get() {
+            match error {
+                WebGLError::InvalidEnum => constants::INVALID_ENUM,
+                WebGLError::InvalidValue => constants::INVALID_VALUE,
+                WebGLError::InvalidOperation => constants::INVALID_OPERATION,
+                WebGLError::OutOfMemory => constants::OUT_OF_MEMORY,
+                WebGLError::ContextLost => constants::CONTEXT_LOST_WEBGL,
+            }
+        } else {
+            constants::NO_ERROR
+        };
+        self.last_error.set(None);
+        error_code
+    }
+
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.2
     fn GetContextAttributes(self) -> Option<WebGLContextAttributes> {
         let (sender, receiver) = channel();
@@ -174,43 +211,49 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn AttachShader(self, program: Option<&WebGLProgram>, shader: Option<&WebGLShader>) {
-        let program_id = match program {
-            Some(program) => program.get_id(),
-            None => return,
-        };
-        let shader_id = match shader {
-            Some(shader) => shader.get_id(),
-            None => return,
-        };
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::AttachShader(program_id, shader_id))).unwrap()
+        if let Some(program) = program {
+            if let Some(shader) = shader {
+                handle_potential_webgl_error!(self, program.attach_shader(shader), ());
+            }
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5
     fn BindBuffer(self, target: u32, buffer: Option<&WebGLBuffer>) {
-        let id = buffer.map(|buf| buf.get_id()).unwrap_or(0);
-        self.renderer.send(
-            CanvasMsg::WebGL(CanvasWebGLMsg::BindBuffer(target, id))).unwrap()
+        if let Some(buffer) = buffer {
+            buffer.bind(target)
+        } else {
+            // Unbind the current buffer
+            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindBuffer(target, 0))).unwrap()
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6
     fn BindFramebuffer(self, target: u32, framebuffer: Option<&WebGLFramebuffer>) {
-        let id = framebuffer.map(|fb| fb.get_id()).unwrap_or(0);
-        self.renderer.send(
-            CanvasMsg::WebGL(CanvasWebGLMsg::BindFramebuffer(target, id))).unwrap()
+        if let Some(framebuffer) = framebuffer {
+            framebuffer.bind(target)
+        } else {
+            // Bind the default framebuffer
+            let cmd = CanvasWebGLMsg::BindFramebuffer(target, WebGLFramebufferBindingRequest::Default);
+            self.renderer.send(CanvasMsg::WebGL(cmd)).unwrap();
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7
     fn BindRenderbuffer(self, target: u32, renderbuffer: Option<&WebGLRenderbuffer>) {
-        let id = renderbuffer.map(|rb| rb.get_id()).unwrap_or(0);
-        self.renderer.send(
-            CanvasMsg::WebGL(CanvasWebGLMsg::BindRenderbuffer(target, id))).unwrap()
+        if let Some(renderbuffer) = renderbuffer {
+            renderbuffer.bind(target)
+        } else {
+            // Unbind the currently bound renderbuffer
+            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::BindRenderbuffer(target, 0))).unwrap()
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8
     fn BindTexture(self, target: u32, texture: Option<&WebGLTexture>) {
-        let id = texture.map(|tex| tex.get_id()).unwrap_or(0);
-        self.renderer.send(
-            CanvasMsg::WebGL(CanvasWebGLMsg::BindTexture(target, id))).unwrap()
+        if let Some(texture) = texture {
+            texture.bind(target)
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5
@@ -246,104 +289,84 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn CompileShader(self, shader: Option<&WebGLShader>) {
-        let shader_id = match shader {
-            Some(shader) => shader.get_id(),
-            None => return,
-        };
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CompileShader(shader_id))).unwrap()
+        if let Some(shader) = shader {
+            shader.compile()
+        }
     }
 
     // TODO(ecoal95): Probably in the future we should keep track of the
     // generated objects, either here or in the webgl task
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5
     fn CreateBuffer(self) -> Option<Root<WebGLBuffer>> {
-        let (sender, receiver) = channel();
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateBuffer(sender))).unwrap();
-        receiver.recv().unwrap()
-            .map(|buffer_id| WebGLBuffer::new(self.global.root().r(), *buffer_id))
+        WebGLBuffer::maybe_new(self.global.root().r(), self.renderer.clone())
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6
     fn CreateFramebuffer(self) -> Option<Root<WebGLFramebuffer>> {
-        let (sender, receiver) = channel();
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateFramebuffer(sender))).unwrap();
-        receiver.recv().unwrap()
-            .map(|fb_id| WebGLFramebuffer::new(self.global.root().r(), *fb_id))
+        WebGLFramebuffer::maybe_new(self.global.root().r(), self.renderer.clone())
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7
     fn CreateRenderbuffer(self) -> Option<Root<WebGLRenderbuffer>> {
-        let (sender, receiver) = channel();
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateRenderbuffer(sender))).unwrap();
-        receiver.recv().unwrap()
-            .map(|program_id| WebGLRenderbuffer::new(self.global.root().r(), *program_id))
+        WebGLRenderbuffer::maybe_new(self.global.root().r(), self.renderer.clone())
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8
     fn CreateTexture(self) -> Option<Root<WebGLTexture>> {
-        let (sender, receiver) = channel();
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateTexture(sender))).unwrap();
-        receiver.recv().unwrap()
-            .map(|texture_id| WebGLTexture::new(self.global.root().r(), *texture_id))
+        WebGLTexture::maybe_new(self.global.root().r(), self.renderer.clone())
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn CreateProgram(self) -> Option<Root<WebGLProgram>> {
-        let (sender, receiver) = channel();
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateProgram(sender))).unwrap();
-        receiver.recv().unwrap()
-            .map(|program_id| WebGLProgram::new(self.global.root().r(), *program_id))
+        WebGLProgram::maybe_new(self.global.root().r(), self.renderer.clone())
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     // TODO(ecoal95): Check if constants are cross-platform or if we must make a translation
     // between WebGL constants and native ones.
     fn CreateShader(self, shader_type: u32) -> Option<Root<WebGLShader>> {
-        let (sender, receiver) = channel();
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::CreateShader(shader_type, sender))).unwrap();
-        receiver.recv().unwrap()
-            .map(|shader_id| WebGLShader::new(self.global.root().r(), *shader_id))
+        WebGLShader::maybe_new(self.global.root().r(), self.renderer.clone(), shader_type)
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.5
     fn DeleteBuffer(self, buffer: Option<&WebGLBuffer>) {
         if let Some(buffer) = buffer {
-            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteBuffer(buffer.get_id()))).unwrap();
+            buffer.delete()
         }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.6
     fn DeleteFramebuffer(self, framebuffer: Option<&WebGLFramebuffer>) {
         if let Some(framebuffer) = framebuffer {
-            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteFramebuffer(framebuffer.get_id()))).unwrap();
+            framebuffer.delete()
         }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.7
     fn DeleteRenderbuffer(self, renderbuffer: Option<&WebGLRenderbuffer>) {
         if let Some(renderbuffer) = renderbuffer {
-            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteRenderbuffer(renderbuffer.get_id()))).unwrap();
+            renderbuffer.delete()
         }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.8
     fn DeleteTexture(self, texture: Option<&WebGLTexture>) {
         if let Some(texture) = texture {
-            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteTexture(texture.get_id()))).unwrap();
+            texture.delete()
         }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn DeleteProgram(self, program: Option<&WebGLProgram>) {
         if let Some(program) = program {
-            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteProgram(program.get_id()))).unwrap();
+            program.delete()
         }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn DeleteShader(self, shader: Option<&WebGLShader>) {
         if let Some(shader) = shader {
-            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::DeleteShader(shader.get_id()))).unwrap();
+            shader.delete()
         }
     }
 
@@ -359,21 +382,17 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
     fn GetAttribLocation(self, program: Option<&WebGLProgram>, name: DOMString) -> i32 {
-        let program_id = match program {
-            Some(program) => program.get_id(),
-            None => return -1,
-        };
-        let (sender, receiver) = channel();
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::GetAttribLocation(program_id, name, sender))).unwrap();
-        receiver.recv().unwrap()
+        if let Some(program) = program {
+            handle_potential_webgl_error!(self, program.get_attrib_location(name), None).unwrap_or(-1)
+        } else {
+            -1
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn GetShaderInfoLog(self, shader: Option<&WebGLShader>) -> Option<DOMString> {
         if let Some(shader) = shader {
-            let (sender, receiver) = channel();
-            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::GetShaderInfoLog(shader.get_id(), sender))).unwrap();
-            Some(receiver.recv().unwrap())
+            shader.info_log()
         } else {
             None
         }
@@ -382,10 +401,11 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn GetShaderParameter(self, _: *mut JSContext, shader: Option<&WebGLShader>, param_id: u32) -> JSVal {
         if let Some(shader) = shader {
-            let (sender, receiver) = channel();
-            self.renderer.send(
-                CanvasMsg::WebGL(CanvasWebGLMsg::GetShaderParameter(shader.get_id(), param_id, sender))).unwrap();
-            Int32Value(receiver.recv().unwrap())
+            match handle_potential_webgl_error!(self, shader.parameter(param_id), WebGLShaderParameter::Invalid) {
+                WebGLShaderParameter::Int(val) => Int32Value(val),
+                WebGLShaderParameter::Bool(val) => BooleanValue(val),
+                WebGLShaderParameter::Invalid => NullValue(),
+            }
         } else {
             NullValue()
         }
@@ -396,10 +416,7 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
                           program: Option<&WebGLProgram>,
                           name: DOMString) -> Option<Root<WebGLUniformLocation>> {
         if let Some(program) = program {
-            let (sender, receiver) = channel();
-            self.renderer.send(
-                CanvasMsg::WebGL(CanvasWebGLMsg::GetUniformLocation(program.get_id(), name, sender))).unwrap();
-            receiver.recv().unwrap()
+            handle_potential_webgl_error!(self, program.get_uniform_location(name), None)
                 .map(|location| WebGLUniformLocation::new(self.global.root().r(), location))
         } else {
             None
@@ -409,15 +426,23 @@ impl<'a> WebGLRenderingContextMethods for &'a 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 {
-            self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::LinkProgram(program.get_id()))).unwrap()
+            program.link()
         }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn ShaderSource(self, shader: Option<&WebGLShader>, source: DOMString) {
         if let Some(shader) = shader {
-            self.renderer.send(
-                CanvasMsg::WebGL(CanvasWebGLMsg::ShaderSource(shader.get_id(), source))).unwrap();
+            shader.set_source(source)
+        }
+    }
+
+    // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
+    fn GetShaderSource(self, shader: Option<&WebGLShader>) -> Option<DOMString> {
+        if let Some(shader) = shader {
+            shader.source()
+        } else {
+            None
         }
     }
 
@@ -428,7 +453,7 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
                   uniform: Option<&WebGLUniformLocation>,
                   data: Option<*mut JSObject>) {
         let uniform_id = match uniform {
-            Some(uniform) => uniform.get_id(),
+            Some(uniform) => uniform.id(),
             None => return,
         };
 
@@ -446,18 +471,16 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
     fn UseProgram(self, program: Option<&WebGLProgram>) {
-        let program_id = match program {
-            Some(program) => program.get_id(),
-            None => return,
-        };
-        self.renderer.send(CanvasMsg::WebGL(CanvasWebGLMsg::UseProgram(program_id as u32))).unwrap()
+        if let Some(program) = program {
+            program.use_program()
+        }
     }
 
     // https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
     fn VertexAttribPointer(self, attrib_id: u32, size: i32, data_type: u32,
                            normalized: bool, stride: i32, offset: i64) {
         match data_type {
-            WebGLRenderingContextConstants::FLOAT => {
+            constants::FLOAT => {
                let msg = CanvasMsg::WebGL(
                    CanvasWebGLMsg::VertexAttribPointer2f(attrib_id, size, normalized, stride, offset));
                 self.renderer.send(msg).unwrap()
@@ -473,6 +496,20 @@ impl<'a> WebGLRenderingContextMethods for &'a WebGLRenderingContext {
     }
 }
 
+pub trait WebGLRenderingContextHelpers {
+    fn handle_webgl_error(&self, err: WebGLError);
+}
+
+impl<'a> WebGLRenderingContextHelpers for &'a WebGLRenderingContext {
+    fn handle_webgl_error(&self, err: WebGLError) {
+        // If an error has been detected no further errors must be
+        // recorded until `getError` has been called
+        if self.last_error.get().is_none() {
+            self.last_error.set(Some(err));
+        }
+    }
+}
+
 pub trait LayoutCanvasWebGLRenderingContextHelpers {
     #[allow(unsafe_code)]
     unsafe fn get_renderer(&self) -> Sender<CanvasMsg>;