Merge pull request #1333 from nstoddard/master

Make the slice argument of texImage2D and related functions immutable
2025-03-18 19:20:51 +00:00 · 2019-03-11 14:14:50 -05:00 · 2019-03-11 14:14:50 -05:00 · 90c3196b7b
commit 90c3196b7b
parent c857a93784 03fa00d201
6 changed files with 109 additions and 33 deletions
--- a/crates/web-sys/tests/wasm/element.js
+++ b/crates/web-sys/tests/wasm/element.js
@ -156,6 +156,11 @@ export function new_webgl_rendering_context() {
  return canvas.getContext('webgl');
 }

+export function new_webgl2_rendering_context() {
+  const canvas = document.createElement('canvas');
+  return canvas.getContext('webgl2');
+}
+
 export function new_xpath_result() {
    let xmlDoc = new DOMParser().parseFromString("<root><value>tomato</value></root>", "application/xml");
    let xpathResult = xmlDoc.evaluate("/root//value", xmlDoc, null, XPathResult.ANY_TYPE, null);
--- a/crates/web-sys/tests/wasm/whitelisted_immutable_slices.rs
+++ b/crates/web-sys/tests/wasm/whitelisted_immutable_slices.rs
@ -12,11 +12,12 @@

 use wasm_bindgen::prelude::*;
 use wasm_bindgen_test::*;
-use web_sys::WebGlRenderingContext;
+use web_sys::{WebGlRenderingContext, WebGl2RenderingContext};

 #[wasm_bindgen(module = "/tests/wasm/element.js")]
 extern "C" {
    fn new_webgl_rendering_context() -> WebGlRenderingContext;
+    fn new_webgl2_rendering_context() -> WebGl2RenderingContext;
 // TODO: Add a function to create another type to test here.
 // These functions come from element.js
 }
@ -49,8 +50,41 @@ extern "C" {
 //    gl.uniform_matrix2fv_with_f32_array(None, false, &[1.]);
 //    gl.uniform_matrix3fv_with_f32_array(None, false, &[1.]);
 //    gl.uniform_matrix4fv_with_f32_array(None, false, &[1.]);
-//}
+//
+//    gl.tex_image_2d_with_i32_and_i32_and_i32_and_format_and_type_and_opt_u8_array(
+//        0,
+//        0,
+//        0,
+//        0,
+//        0,
+//        0,
+//        0,
+//        0,
+//        Some(&[1]),
+//    );
+//    gl.tex_sub_image_2d_with_i32_and_i32_and_u32_and_type_and_opt_u8_array(
+//        0,
+//        0,
+//        0,
+//        0,
+//        0,
+//        0,
+//        0,
+//        0,
+//        Some(&[1]),
+//    );
+//    gl.compressed_tex_image_2d_with_u8_array(0, 0, 0, 0, 0, 0, &[1]);
+// }
+//
+//#[wasm_bindgen_test]
+//fn test_webgl2_rendering_context_immutable_slices() {
+//    let gl = new_webgl2_rendering_context();

+//    gl.tex_image_3d_with_opt_u8_array(0, 0, 0, 0, 0, 0, 0, 0, 0, Some(&[1]));
+//    gl.tex_sub_image_3d_with_opt_u8_array(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, Some(&[1]));
+//    gl.compressed_tex_image_3d_with_u8_array(0, 0, 0, 0, 0, 0, 0, &[1]);
+//}
+//
 // TODO:
 //#[wasm_bindgen_test]
 //fn test_another_types_immutable_slices_here() {
--- a/crates/webidl/src/first_pass.rs
+++ b/crates/webidl/src/first_pass.rs
@ -37,7 +37,7 @@ pub(crate) struct FirstPassRecord<'src> {
    pub(crate) dictionaries: BTreeMap<&'src str, DictionaryData<'src>>,
    pub(crate) callbacks: BTreeSet<&'src str>,
    pub(crate) callback_interfaces: BTreeMap<&'src str, CallbackInterfaceData<'src>>,
-    pub(crate) immutable_f32_whitelist: BTreeSet<&'static str>,
+    pub(crate) immutable_slice_whitelist: BTreeSet<&'static str>,
 }

 /// We need to collect interface data during the first pass, to be used later.
--- a/crates/webidl/src/idl_type.rs
+++ b/crates/webidl/src/idl_type.rs
@ -34,8 +34,10 @@ pub(crate) enum IdlType<'a> {
    ArrayBuffer,
    DataView,
    Int8Array,
-    Uint8Array,
-    Uint8ArrayMut,
+    Uint8Array {
+        /// Whether or not the generated web-sys function should use an immutable slice
+        immutable: bool,
+    },
    Uint8ClampedArray,
    Int16Array,
    Uint16Array,
@ -46,8 +48,14 @@ pub(crate) enum IdlType<'a> {
        immutable: bool,
    },
    Float64Array,
-    ArrayBufferView,
-    BufferSource,
+    ArrayBufferView {
+        /// Whether or not the generated web-sys function should use an immutable slice
+        immutable: bool,
+    },
+    BufferSource {
+        /// Whether or not the generated web-sys function should use an immutable slice
+        immutable: bool,
+    },

    Interface(&'a str),
    Dictionary(&'a str),
@ -327,15 +335,6 @@ impl<'a> ToIdlType<'a> for Identifier<'a> {
    }
 }

-// We default to Float32Array's being mutable, but in certain cases where we're certain that
-// slices won't get mutated on the JS side (such as the WebGL APIs) we might, later in the flow,
-// instead use the immutable version.
-impl<'a> ToIdlType<'a> for term::Float32Array {
-    fn to_idl_type(&self, _record: &FirstPassRecord<'a>) -> IdlType<'a> {
-        IdlType::Float32Array { immutable: false }
-    }
-}
-
 macro_rules! terms_to_idl_type {
    ($($t:tt => $r:tt)*) => ($(
        impl<'a> ToIdlType<'a> for term::$t {
@ -346,6 +345,19 @@ macro_rules! terms_to_idl_type {
    )*);
 }

+// We default to arrays being mutable, but in certain cases where we're certain that
+// slices won't get mutated on the JS side (such as the WebGL APIs) we might, later in the flow,
+// instead use the immutable version.
+macro_rules! terms_to_idl_type_maybe_immutable {
+    ($($t:tt => $r:tt)*) => ($(
+        impl<'a> ToIdlType<'a> for term::$t {
+            fn to_idl_type(&self, _record: &FirstPassRecord<'a>) -> IdlType<'a> {
+                IdlType::$r { immutable: false }
+            }
+        }
+    )*);
+}
+
 terms_to_idl_type! {
    Symbol => Symbol
    ByteString => ByteString
@ -366,14 +378,18 @@ terms_to_idl_type! {
    Int8Array => Int8Array
    Int16Array => Int16Array
    Int32Array => Int32Array
-    Uint8Array => Uint8Array
    Uint16Array => Uint16Array
    Uint32Array => Uint32Array
    Uint8ClampedArray => Uint8ClampedArray
    Float64Array => Float64Array
+    Error => Error
+}
+
+terms_to_idl_type_maybe_immutable! {
+    Uint8Array => Uint8Array
+    Float32Array => Float32Array
    ArrayBufferView => ArrayBufferView
    BufferSource => BufferSource
-    Error => Error
 }

 impl<'a> IdlType<'a> {
@ -400,8 +416,7 @@ impl<'a> IdlType<'a> {
            IdlType::ArrayBuffer => dst.push_str("array_buffer"),
            IdlType::DataView => dst.push_str("data_view"),
            IdlType::Int8Array => dst.push_str("i8_array"),
-            IdlType::Uint8Array => dst.push_str("u8_array"),
-            IdlType::Uint8ArrayMut => dst.push_str("u8_array"),
+            IdlType::Uint8Array { .. } => dst.push_str("u8_array"),
            IdlType::Uint8ClampedArray => dst.push_str("u8_clamped_array"),
            IdlType::Int16Array => dst.push_str("i16_array"),
            IdlType::Uint16Array => dst.push_str("u16_array"),
@ -409,8 +424,8 @@ impl<'a> IdlType<'a> {
            IdlType::Uint32Array => dst.push_str("u32_array"),
            IdlType::Float32Array { .. } => dst.push_str("f32_array"),
            IdlType::Float64Array => dst.push_str("f64_array"),
-            IdlType::ArrayBufferView => dst.push_str("array_buffer_view"),
-            IdlType::BufferSource => dst.push_str("buffer_source"),
+            IdlType::ArrayBufferView { .. } => dst.push_str("array_buffer_view"),
+            IdlType::BufferSource { .. } => dst.push_str("buffer_source"),

            IdlType::Interface(name) => dst.push_str(&snake_case_ident(name)),
            IdlType::UnknownInterface(name) => dst.push_str(&snake_case_ident(name)),
@ -513,8 +528,7 @@ impl<'a> IdlType<'a> {
            IdlType::ArrayBuffer => js_sys("ArrayBuffer"),
            IdlType::DataView => None,
            IdlType::Int8Array => Some(array("i8", pos, false)),
-            IdlType::Uint8Array => Some(array("u8", pos, false)),
-            IdlType::Uint8ArrayMut => Some(array("u8", pos, false)),
+            IdlType::Uint8Array { immutable } => Some(array("u8", pos, *immutable)),
            IdlType::Uint8ClampedArray => Some(clamped(array("u8", pos, false))),
            IdlType::Int16Array => Some(array("i16", pos, false)),
            IdlType::Uint16Array => Some(array("u16", pos, false)),
@ -523,7 +537,7 @@ impl<'a> IdlType<'a> {
            IdlType::Float32Array { immutable } => Some(array("f32", pos, *immutable)),
            IdlType::Float64Array => Some(array("f64", pos, false)),

-            IdlType::ArrayBufferView | IdlType::BufferSource => js_sys("Object"),
+            IdlType::ArrayBufferView { .. } | IdlType::BufferSource { .. } => js_sys("Object"),
            IdlType::Interface(name)
            | IdlType::Dictionary(name)
            | IdlType::CallbackInterface { name, .. } => {
@ -654,8 +668,22 @@ impl<'a> IdlType<'a> {
                .iter()
                .flat_map(|idl_type| idl_type.flatten())
                .collect(),
-            IdlType::ArrayBufferView => vec![IdlType::ArrayBufferView, IdlType::Uint8ArrayMut],
-            IdlType::BufferSource => vec![IdlType::BufferSource, IdlType::Uint8ArrayMut],
+            IdlType::ArrayBufferView { immutable } => vec![
+                IdlType::ArrayBufferView {
+                    immutable: *immutable,
+                },
+                IdlType::Uint8Array {
+                    immutable: *immutable,
+                },
+            ],
+            IdlType::BufferSource { immutable } => vec![
+                IdlType::BufferSource {
+                    immutable: *immutable,
+                },
+                IdlType::Uint8Array {
+                    immutable: *immutable,
+                },
+            ],
            IdlType::LongLong => vec![IdlType::Long, IdlType::Double],
            IdlType::UnsignedLongLong => vec![IdlType::UnsignedLong, IdlType::Double],
            IdlType::CallbackInterface {
--- a/crates/webidl/src/lib.rs
+++ b/crates/webidl/src/lib.rs
@ -82,7 +82,7 @@ fn parse(webidl_source: &str, allowed_types: Option<&[&str]>) -> Result<Program>

    let mut first_pass_record: FirstPassRecord = Default::default();
    first_pass_record.builtin_idents = builtin_idents();
-    first_pass_record.immutable_f32_whitelist = immutable_f32_whitelist();
+    first_pass_record.immutable_slice_whitelist = immutable_slice_whitelist();

    definitions.first_pass(&mut first_pass_record, ())?;
    let mut program = Default::default();
@ -181,9 +181,9 @@ fn builtin_idents() -> BTreeSet<Ident> {
    )
 }

-fn immutable_f32_whitelist() -> BTreeSet<&'static str> {
+fn immutable_slice_whitelist() -> BTreeSet<&'static str> {
    BTreeSet::from_iter(vec![
-        // WebGlRenderingContext
+        // WebGlRenderingContext, WebGl2RenderingContext
        "uniform1fv",
        "uniform2fv",
        "uniform3fv",
@ -195,6 +195,14 @@ fn immutable_f32_whitelist() -> BTreeSet<&'static str> {
        "vertexAttrib2fv",
        "vertexAttrib3fv",
        "vertexAttrib4fv",
+        "bufferData",
+        "texImage2D",
+        "texSubImage2D",
+        "compressedTexImage2D",
+        // WebGl2RenderingContext
+        "texImage3D",
+        "texSubImage3D",
+        "compressedTexImage3D",
        // TODO: Add another type's functions here. Leave a comment header with the type name
    ])
 }
--- a/crates/webidl/src/util.rs
+++ b/crates/webidl/src/util.rs
@ -647,12 +647,10 @@ impl<'src> FirstPassRecord<'src> {
            _ => return idl_type,
        };

-        if self.immutable_f32_whitelist.contains(op) {
+        if self.immutable_slice_whitelist.contains(op) {
            flag_slices_immutable(&mut idl_type)
        }

-        // TODO: Add other whitelisted slices here, such as F64 or u8..
-
        idl_type
    }
 }
@ -737,7 +735,10 @@ pub fn public() -> syn::Visibility {

 fn flag_slices_immutable(ty: &mut IdlType) {
    match ty {
+        IdlType::Uint8Array { immutable } => *immutable = true,
        IdlType::Float32Array { immutable } => *immutable = true,
+        IdlType::ArrayBufferView { immutable } => *immutable = true,
+        IdlType::BufferSource { immutable } => *immutable = true,
        IdlType::Nullable(item) => flag_slices_immutable(item),
        IdlType::FrozenArray(item) => flag_slices_immutable(item),
        IdlType::Sequence(item) => flag_slices_immutable(item),