Always use a WebGLVertexArrayObject to handle vertex attribs

This lets us clean up how buffers are reference-counted.

3 files changed, 12 insertions, 151 deletions

diff --git a/components/script/dom/webgl_extensions/ext/oesvertexarrayobject.rs b/components/script/dom/webgl_extensions/ext/oesvertexarrayobject.rs
index 6ae83c401ba..aae50c1effe 100644
--- a/components/script/dom/webgl_extensions/ext/oesvertexarrayobject.rs
+++ b/components/script/dom/webgl_extensions/ext/oesvertexarrayobject.rs
@@ -2,24 +2,20 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use canvas_traits::webgl::{webgl_channel, WebGLCommand, WebGLError, WebGLVersion};
+use canvas_traits::webgl::WebGLVersion;
 use dom::bindings::codegen::Bindings::OESVertexArrayObjectBinding::{self, OESVertexArrayObjectMethods};
 use dom::bindings::codegen::Bindings::OESVertexArrayObjectBinding::OESVertexArrayObjectConstants;
 use dom::bindings::reflector::{DomObject, Reflector, reflect_dom_object};
-use dom::bindings::root::{Dom, DomRoot, MutNullableDom};
+use dom::bindings::root::{Dom, DomRoot};
 use dom::webglrenderingcontext::WebGLRenderingContext;
 use dom::webglvertexarrayobjectoes::WebGLVertexArrayObjectOES;
 use dom_struct::dom_struct;
-use js::conversions::ToJSValConvertible;
-use js::jsapi::JSContext;
-use js::jsval::{JSVal, NullValue};
 use super::{WebGLExtension, WebGLExtensions, WebGLExtensionSpec};
 
 #[dom_struct]
 pub struct OESVertexArrayObject {
     reflector_: Reflector,
     ctx: Dom<WebGLRenderingContext>,
-    bound_vao: MutNullableDom<WebGLVertexArrayObjectOES>,
 }
 
 impl OESVertexArrayObject {
@@ -27,104 +23,29 @@ impl OESVertexArrayObject {
         Self {
             reflector_: Reflector::new(),
             ctx: Dom::from_ref(ctx),
-            bound_vao: MutNullableDom::new(None)
         }
     }
-
-    #[allow(unsafe_code)]
-    fn get_current_binding(&self, cx:*mut JSContext) -> JSVal {
-        rooted!(in(cx) let mut rval = NullValue());
-        if let Some(bound_vao) = self.bound_vao.get() {
-            unsafe {
-                bound_vao.to_jsval(cx, rval.handle_mut());
-            }
-        }
-        rval.get()
-    }
 }
 
 impl OESVertexArrayObjectMethods for OESVertexArrayObject {
     // https://www.khronos.org/registry/webgl/extensions/OES_vertex_array_object/
     fn CreateVertexArrayOES(&self) -> Option<DomRoot<WebGLVertexArrayObjectOES>> {
-        let (sender, receiver) = webgl_channel().unwrap();
-        self.ctx.send_command(WebGLCommand::CreateVertexArray(sender));
-        receiver.recv().unwrap().map(|id| WebGLVertexArrayObjectOES::new(&self.ctx, id))
+        self.ctx.create_vertex_array()
     }
 
     // https://www.khronos.org/registry/webgl/extensions/OES_vertex_array_object/
     fn DeleteVertexArrayOES(&self, vao: Option<&WebGLVertexArrayObjectOES>) {
-        if let Some(vao) = vao {
-            if vao.is_deleted() {
-                return;
-            }
-
-            // Unbind deleted VAO if currently bound
-            if let Some(bound_vao) = self.bound_vao.get() {
-                if bound_vao.id() == vao.id() {
-                    self.bound_vao.set(None);
-                    self.ctx.send_command(WebGLCommand::BindVertexArray(None));
-                }
-            }
-
-            // Remove VAO references from buffers
-            for attrib_data in &*vao.vertex_attribs().borrow() {
-                if let Some(buffer) = attrib_data.buffer() {
-                    buffer.remove_vao_reference(vao.id());
-                }
-            }
-            if let Some(buffer) = vao.bound_buffer_element_array() {
-                buffer.remove_vao_reference(vao.id());
-            }
-
-            // Delete the vao
-            self.ctx.send_command(WebGLCommand::DeleteVertexArray(vao.id()));
-            vao.set_deleted();
-        }
+        self.ctx.delete_vertex_array(vao);
     }
 
     // https://www.khronos.org/registry/webgl/extensions/OES_vertex_array_object/
     fn IsVertexArrayOES(&self, vao: Option<&WebGLVertexArrayObjectOES>) -> bool {
-        // Conformance tests expect false if vao never bound
-        vao.map_or(false, |vao| !vao.is_deleted() && vao.ever_bound())
+        self.ctx.is_vertex_array(vao)
     }
 
     // https://www.khronos.org/registry/webgl/extensions/OES_vertex_array_object/
     fn BindVertexArrayOES(&self, vao: Option<&WebGLVertexArrayObjectOES>) {
-        if let Some(bound_vao) = self.bound_vao.get() {
-            // Store buffers attached to attrib pointers
-            bound_vao.vertex_attribs().clone_from(&self.ctx.vertex_attribs());
-            for attrib_data in &*bound_vao.vertex_attribs().borrow() {
-                if let Some(buffer) = attrib_data.buffer() {
-                    buffer.add_vao_reference(bound_vao.id());
-                }
-            }
-            // Store element array buffer
-            let element_array = self.ctx.bound_buffer_element_array();
-            bound_vao.set_bound_buffer_element_array(element_array.as_ref().map(|buffer| {
-                buffer.add_vao_reference(bound_vao.id());
-                &**buffer
-            }));
-        }
-
-        if let Some(vao) = vao {
-            if vao.is_deleted() {
-                self.ctx.webgl_error(WebGLError::InvalidOperation);
-                return;
-            }
-
-            self.ctx.send_command(WebGLCommand::BindVertexArray(Some(vao.id())));
-            vao.set_ever_bound();
-            self.bound_vao.set(Some(&vao));
-
-            // Restore WebGLRenderingContext current bindings
-            self.ctx.vertex_attribs().clone_from(&vao.vertex_attribs());
-            let element_array = vao.bound_buffer_element_array();
-            self.ctx.set_bound_buffer_element_array(element_array.as_ref().map(|buffer| &**buffer));
-        } else {
-            self.ctx.send_command(WebGLCommand::BindVertexArray(None));
-            self.bound_vao.set(None);
-            self.ctx.vertex_attribs().clear();
-        }
+        self.ctx.bind_vertex_array(vao);
     }
 }
 
@@ -147,18 +68,7 @@ impl WebGLExtension for OESVertexArrayObject {
     }
 
     fn enable(ext: &WebGLExtensions) {
-        let query = OESVertexArrayObjectConstants::VERTEX_ARRAY_BINDING_OES;
-        ext.add_query_parameter_handler(query, Box::new(|cx, webgl_ctx| {
-            match webgl_ctx.get_extension_manager().get_dom_object::<OESVertexArrayObject>() {
-                Some(dom_object) => {
-                    Ok(dom_object.get_current_binding(cx))
-                },
-                None => {
-                    // Extension instance not found!
-                    Err(WebGLError::InvalidOperation)
-                }
-            }
-        }));
+        ext.enable_get_parameter_name(OESVertexArrayObjectConstants::VERTEX_ARRAY_BINDING_OES);
     }
 
     fn name() -> &'static str {
diff --git a/components/script/dom/webgl_extensions/extensions.rs b/components/script/dom/webgl_extensions/extensions.rs
index b3812f51cf5..51d8de44efc 100644
--- a/components/script/dom/webgl_extensions/extensions.rs
+++ b/components/script/dom/webgl_extensions/extensions.rs
@@ -2,24 +2,20 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use canvas_traits::webgl::{WebGLError, WebGLVersion};
+use canvas_traits::webgl::WebGLVersion;
 use dom::bindings::cell::DomRefCell;
 use dom::bindings::codegen::Bindings::ANGLEInstancedArraysBinding::ANGLEInstancedArraysConstants;
 use dom::bindings::codegen::Bindings::EXTTextureFilterAnisotropicBinding::EXTTextureFilterAnisotropicConstants;
 use dom::bindings::codegen::Bindings::OESStandardDerivativesBinding::OESStandardDerivativesConstants;
 use dom::bindings::codegen::Bindings::OESTextureHalfFloatBinding::OESTextureHalfFloatConstants;
+use dom::bindings::codegen::Bindings::OESVertexArrayObjectBinding::OESVertexArrayObjectConstants;
 use dom::bindings::codegen::Bindings::WebGLRenderingContextBinding::WebGLRenderingContextConstants as constants;
-use dom::bindings::root::DomRoot;
 use dom::bindings::trace::JSTraceable;
 use dom::webglrenderingcontext::WebGLRenderingContext;
 use fnv::{FnvHashMap, FnvHashSet};
 use gleam::gl::GLenum;
-use js::jsapi::JSContext;
 use js::jsapi::JSObject;
-use js::jsval::JSVal;
 use malloc_size_of::MallocSizeOf;
-use ref_filter_map::ref_filter_map;
-use std::cell::Ref;
 use std::collections::HashMap;
 use std::iter::FromIterator;
 use std::ptr::NonNull;
@@ -43,9 +39,10 @@ const DEFAULT_NOT_FILTERABLE_TEX_TYPES: [GLenum; 2] = [
 // Param names that are implemented for glGetParameter in a WebGL 1.0 context
 // but must trigger a InvalidEnum error until the related WebGL Extensions are enabled.
 // Example: https://www.khronos.org/registry/webgl/extensions/OES_standard_derivatives/
-const DEFAULT_DISABLED_GET_PARAMETER_NAMES_WEBGL1: [GLenum; 2] = [
+const DEFAULT_DISABLED_GET_PARAMETER_NAMES_WEBGL1: [GLenum; 3] = [
     EXTTextureFilterAnisotropicConstants::MAX_TEXTURE_MAX_ANISOTROPY_EXT,
     OESStandardDerivativesConstants::FRAGMENT_SHADER_DERIVATIVE_HINT_OES,
+    OESVertexArrayObjectConstants::VERTEX_ARRAY_BINDING_OES,
 ];
 
 // Param names that are implemented for glGetTexParameter in a WebGL 1.0 context
@@ -69,7 +66,6 @@ struct WebGLExtensionFeatures {
     disabled_tex_types: FnvHashSet<GLenum>,
     not_filterable_tex_types: FnvHashSet<GLenum>,
     effective_tex_internal_formats: FnvHashMap<TexFormatType, u32>,
-    query_parameter_handlers: FnvHashMap<GLenum, WebGLQueryParameterHandler>,
     /// WebGL Hint() targets enabled by extensions.
     hint_targets: FnvHashSet<GLenum>,
     /// WebGL GetParameter() names enabled by extensions.
@@ -120,7 +116,6 @@ impl WebGLExtensionFeatures {
             disabled_tex_types,
             not_filterable_tex_types: DEFAULT_NOT_FILTERABLE_TEX_TYPES.iter().cloned().collect(),
             effective_tex_internal_formats: Default::default(),
-            query_parameter_handlers: Default::default(),
             hint_targets: Default::default(),
             disabled_get_parameter_names,
             disabled_get_tex_parameter_names,
@@ -196,18 +191,6 @@ impl WebGLExtensions {
         self.extensions.borrow().get(&name).map_or(false, |ext| { ext.is_enabled() })
     }
 
-    pub fn get_dom_object<T>(&self) -> Option<DomRoot<T::Extension>>
-    where
-        T: 'static + WebGLExtension + JSTraceable + MallocSizeOf
-    {
-        let name = T::name().to_uppercase();
-        self.extensions.borrow().get(&name).and_then(|extension| {
-            extension.as_any().downcast_ref::<TypedWebGLExtensionWrapper<T>>().and_then(|extension| {
-                extension.dom_object()
-            })
-        })
-    }
-
     pub fn supports_gl_extension(&self, name: &str) -> bool {
         self.features.borrow().gl_extensions.contains(name)
     }
@@ -252,19 +235,6 @@ impl WebGLExtensions {
         self.features.borrow().not_filterable_tex_types.get(&text_data_type).is_none()
     }
 
-    pub fn add_query_parameter_handler(&self, name: GLenum, f: Box<WebGLQueryParameterFunc>) {
-        let handler = WebGLQueryParameterHandler {
-            func: f
-        };
-        self.features.borrow_mut().query_parameter_handlers.insert(name, handler);
-    }
-
-    pub fn get_query_parameter_handler(&self, name: GLenum) -> Option<Ref<Box<WebGLQueryParameterFunc>>> {
-        ref_filter_map(self.features.borrow(), |features| {
-            features.query_parameter_handlers.get(&name).map(|item| &item.func)
-        })
-    }
-
     pub fn enable_hint_target(&self, name: GLenum) {
         self.features.borrow_mut().hint_targets.insert(name);
     }
@@ -331,14 +301,3 @@ impl WebGLExtensions {
 // Helper structs
 #[derive(Eq, Hash, JSTraceable, MallocSizeOf, PartialEq)]
 struct TexFormatType(u32, u32);
-
-type WebGLQueryParameterFunc = Fn(*mut JSContext, &WebGLRenderingContext)
-                               -> Result<JSVal, WebGLError>;
-
-#[derive(MallocSizeOf)]
-struct WebGLQueryParameterHandler {
-    #[ignore_malloc_size_of = "Closures are hard"]
-    func: Box<WebGLQueryParameterFunc>
-}
-
-unsafe_no_jsmanaged_fields!(WebGLQueryParameterHandler);
diff --git a/components/script/dom/webgl_extensions/wrapper.rs b/components/script/dom/webgl_extensions/wrapper.rs
index 05fb375ffbd..b5166327381 100644
--- a/components/script/dom/webgl_extensions/wrapper.rs
+++ b/components/script/dom/webgl_extensions/wrapper.rs
@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::bindings::reflector::DomObject;
-use dom::bindings::root::{DomRoot, MutNullableDom};
+use dom::bindings::root::MutNullableDom;
 use dom::bindings::trace::JSTraceable;
 use dom::webglrenderingcontext::WebGLRenderingContext;
 use js::jsapi::JSObject;
@@ -87,11 +87,3 @@ impl<T> WebGLExtensionWrapper for TypedWebGLExtensionWrapper<T>
         self
     }
 }
-
-impl<T> TypedWebGLExtensionWrapper<T>
-    where T: WebGLExtension + JSTraceable + MallocSizeOf + 'static
-{
-    pub fn dom_object(&self) -> Option<DomRoot<T::Extension>> {
-        self.extension.get()
-    }
-}