Remove HashMaps from wgpu API (#7133)

* Remove HashMaps from Surface API

* Fix Comments and Errors

CHANGELOG.md

@@ -40,6 +40,17 @@ Bottom level categories:
 
 ## Unreleased
 
+### Major Changes
+
+#### Hashmaps Removed from APIs
+
+Both `PipelineCompilationOptions::constants` and `ShaderSource::Glsl::defines` now take
+slices of key-value pairs instead of `hashmap`s. This is to prepare for `no_std`
+support and allow us to keep which `hashmap` hasher and such as implementation details. It
+also allows more easily creating these structures inline.
+
+By @cwfitzgerald in [#7133](https://github.com/gfx-rs/wgpu/pull/7133)
+
 ### New Features
 
 #### General

deno_webgpu/device.rs

@@ -632,7 +632,7 @@ impl GPUDevice {
             stage: ProgrammableStageDescriptor {
                 module: descriptor.compute.module.id,
                 entry_point: descriptor.compute.entry_point.map(Into::into),
-                constants: Cow::Owned(descriptor.compute.constants.into_iter().collect()),
+                constants: descriptor.compute.constants.into_iter().collect(),
                 zero_initialize_workgroup_memory: true,
             },
             cache: None,
@@ -660,7 +660,7 @@ impl GPUDevice {
             stage: ProgrammableStageDescriptor {
                 module: descriptor.vertex.module.id,
                 entry_point: descriptor.vertex.entry_point.map(Into::into),
-                constants: Cow::Owned(descriptor.vertex.constants.into_iter().collect()),
+                constants: descriptor.vertex.constants.into_iter().collect(),
                 zero_initialize_workgroup_memory: true,
             },
             buffers: Cow::Owned(
@@ -753,7 +753,7 @@ impl GPUDevice {
                     stage: ProgrammableStageDescriptor {
                         module: fragment.module.id,
                         entry_point: fragment.entry_point.map(Into::into),
-                        constants: Cow::Owned(fragment.constants.into_iter().collect()),
+                        constants: fragment.constants.into_iter().collect(),
                         zero_initialize_workgroup_memory: true,
                     },
                     targets: Cow::Owned(

tests/tests/shader/array_size_overrides.rs

@@ -52,7 +52,7 @@ async fn array_size_overrides(
             source: wgpu::ShaderSource::Wgsl(std::borrow::Cow::Borrowed(SHADER)),
         });
     let pipeline_options = wgpu::PipelineCompilationOptions {
-        constants: &[("n".to_owned(), n.unwrap_or(0).into())].into(),
+        constants: &[("n", f64::from(n.unwrap_or(0)))],
         ..Default::default()
     };
     let compute_pipeline = fail_if(

tests/tests/shader/workgroup_size_overrides.rs

@@ -37,7 +37,7 @@ async fn workgroup_size_overrides(
             source: wgpu::ShaderSource::Wgsl(std::borrow::Cow::Borrowed(SHADER)),
         });
     let pipeline_options = wgpu::PipelineCompilationOptions {
-        constants: &[("n".to_owned(), n.unwrap_or(0).into())].into(),
+        constants: &[("n", f64::from(n.unwrap_or(0)))],
         ..Default::default()
     };
     let compute_pipeline = fail_if(

wgpu-core/src/device/resource.rs

@@ -2828,7 +2828,7 @@ impl Device {
             stage: hal::ProgrammableStage {
                 module: shader_module.raw(),
                 entry_point: final_entry_point_name.as_ref(),
-                constants: desc.stage.constants.as_ref(),
+                constants: &desc.stage.constants,
                 zero_initialize_workgroup_memory: desc.stage.zero_initialize_workgroup_memory,
             },
             cache: cache.as_ref().map(|it| it.raw()),
@@ -3287,7 +3287,7 @@ impl Device {
             hal::ProgrammableStage {
                 module: vertex_shader_module.raw(),
                 entry_point: &vertex_entry_point_name,
-                constants: stage_desc.constants.as_ref(),
+                constants: &stage_desc.constants,
                 zero_initialize_workgroup_memory: stage_desc.zero_initialize_workgroup_memory,
             }
         };
@@ -3341,7 +3341,7 @@ impl Device {
                 Some(hal::ProgrammableStage {
                     module: shader_module.raw(),
                     entry_point: &fragment_entry_point_name,
-                    constants: fragment_state.stage.constants.as_ref(),
+                    constants: &fragment_state.stage.constants,
                     zero_initialize_workgroup_memory: fragment_state
                         .stage
                         .zero_initialize_workgroup_memory,

wgpu-core/src/pipeline.rs

@@ -139,7 +139,7 @@ pub struct ProgrammableStageDescriptor<'a, SM = ShaderModuleId> {
     /// the key must be the constant's identifier name.
     ///
     /// The value may represent any of WGSL's concrete scalar types.
-    pub constants: Cow<'a, naga::back::PipelineConstants>,
+    pub constants: naga::back::PipelineConstants,
     /// Whether workgroup scoped memory will be initialized with zero values for this stage.
     ///
     /// This is required by the WebGPU spec, but may have overhead which can be avoided

wgpu/src/api/common_pipeline.rs

@@ -1,5 +1,3 @@
-use hashbrown::HashMap;
-
 use crate::*;
 
 #[derive(Clone, Debug)]
@@ -13,8 +11,10 @@ pub struct PipelineCompilationOptions<'a> {
     /// the key must be the pipeline constant ID as a decimal ASCII number; if not,
     /// the key must be the constant's identifier name.
     ///
+    /// If the given constant is specified more than once, the last value specified is used.
+    ///
     /// The value may represent any of WGSL's concrete scalar types.
-    pub constants: &'a HashMap<String, f64>,
+    pub constants: &'a [(&'a str, f64)],
     /// Whether workgroup scoped memory will be initialized with zero values for this stage.
     ///
     /// This is required by the WebGPU spec, but may have overhead which can be avoided
@@ -24,14 +24,8 @@ pub struct PipelineCompilationOptions<'a> {
 
 impl Default for PipelineCompilationOptions<'_> {
     fn default() -> Self {
-        // HashMap doesn't have a const constructor, due to the use of RandomState
-        // This does introduce some synchronisation costs, but these should be minor,
-        // and might be cheaper than the alternative of getting new random state
-        static DEFAULT_CONSTANTS: std::sync::OnceLock<HashMap<String, f64>> =
-            std::sync::OnceLock::new();
-        let constants = DEFAULT_CONSTANTS.get_or_init(Default::default);
         Self {
-            constants,
+            constants: Default::default(),
             zero_initialize_workgroup_memory: true,
         }
     }

wgpu/src/api/shader_module.rs

@@ -186,8 +186,10 @@ pub enum ShaderSource<'a> {
         shader: Cow<'a, str>,
         /// The shader stage that the shader targets. For example, `naga::ShaderStage::Vertex`
         stage: naga::ShaderStage,
-        /// Defines to unlock configured shader features.
-        defines: naga::FastHashMap<String, String>,
+        /// Key-value pairs to represent defines sent to the glsl preprocessor.
+        ///
+        /// If the same name is defined multiple times, the last value is used.
+        defines: &'a [(&'a str, &'a str)],
     },
     /// WGSL module as a string slice.
     #[cfg(feature = "wgsl")]

wgpu/src/backend/webgpu.rs

@@ -5,7 +5,6 @@ mod ext_bindings;
 #[allow(clippy::allow_attributes)]
 mod webgpu_sys;
 
-use hashbrown::HashMap;
 use js_sys::Promise;
 use std::{
     cell::RefCell,
@@ -1756,14 +1755,17 @@ impl dispatch::DeviceInterface for WebDevice {
             crate::ShaderSource::Glsl {
                 ref shader,
                 stage,
-                ref defines,
+                defines,
             } => {
                 use naga::front;
 
                 // Parse the given shader code and store its representation.
                 let options = front::glsl::Options {
                     stage,
-                    defines: defines.clone(),
+                    defines: defines
+                        .iter()
+                        .map(|&(key, value)| (String::from(key), String::from(value)))
+                        .collect(),
                 };
                 let mut parser = front::glsl::Frontend::default();
                 parser
@@ -3835,19 +3837,23 @@ impl Drop for WebQueueWriteBuffer {
 /// exposed by `wasm-bindgen`. See the following issues for details:
 /// - [gfx-rs/wgpu#5688](https://github.com/gfx-rs/wgpu/pull/5688)
 /// - [rustwasm/wasm-bindgen#3587](https://github.com/rustwasm/wasm-bindgen/issues/3587)
-fn insert_constants_map(target: &JsValue, map: &HashMap<String, f64>) {
+fn insert_constants_map(target: &JsValue, map: &[(&str, f64)]) {
     if !map.is_empty() {
-        js_sys::Reflect::set(target, &"constants".into(), &hashmap_to_jsvalue(map))
-            .expect("Setting the values in a Javascript pipeline descriptor should never fail");
+        js_sys::Reflect::set(
+            target,
+            &JsValue::from_str("constants"),
+            &hashmap_to_jsvalue(map),
+        )
+        .expect("Setting the values in a Javascript pipeline descriptor should never fail");
     }
 }
 
 /// Converts a hashmap to a Javascript object.
-fn hashmap_to_jsvalue(map: &HashMap<String, f64>) -> JsValue {
+fn hashmap_to_jsvalue(map: &[(&str, f64)]) -> JsValue {
     let obj = js_sys::Object::new();
 
-    for (k, v) in map.iter() {
-        js_sys::Reflect::set(&obj, &k.into(), &(*v).into())
+    for &(key, v) in map.iter() {
+        js_sys::Reflect::set(&obj, &JsValue::from_str(key), &JsValue::from_f64(v))
             .expect("Setting the values in a Javascript map should never fail");
     }
 

wgpu/src/backend/wgpu_core.rs

@@ -994,7 +994,13 @@ impl dispatch::DeviceInterface for CoreDevice {
                 stage,
                 defines,
             } => {
-                let options = naga::front::glsl::Options { stage, defines };
+                let options = naga::front::glsl::Options {
+                    stage,
+                    defines: defines
+                        .iter()
+                        .map(|&(key, value)| (String::from(key), String::from(value)))
+                        .collect(),
+                };
                 wgc::pipeline::ShaderModuleSource::Glsl(Borrowed(shader), options)
             }
             #[cfg(feature = "wgsl")]
@@ -1261,14 +1267,22 @@ impl dispatch::DeviceInterface for CoreDevice {
             })
             .collect();
 
+        let vert_constants = desc
+            .vertex
+            .compilation_options
+            .constants
+            .iter()
+            .map(|&(key, value)| (String::from(key), value))
+            .collect();
+
         let descriptor = pipe::RenderPipelineDescriptor {
             label: desc.label.map(Borrowed),
             layout: desc.layout.map(|layout| layout.inner.as_core().id),
             vertex: pipe::VertexState {
                 stage: pipe::ProgrammableStageDescriptor {
                     module: desc.vertex.module.inner.as_core().id,
                     entry_point: desc.vertex.entry_point.map(Borrowed),
-                    constants: Borrowed(desc.vertex.compilation_options.constants),
+                    constants: vert_constants,
                     zero_initialize_workgroup_memory: desc
                         .vertex
                         .compilation_options
@@ -1279,16 +1293,24 @@ impl dispatch::DeviceInterface for CoreDevice {
             primitive: desc.primitive,
             depth_stencil: desc.depth_stencil.clone(),
             multisample: desc.multisample,
-            fragment: desc.fragment.as_ref().map(|frag| pipe::FragmentState {
-                stage: pipe::ProgrammableStageDescriptor {
-                    module: frag.module.inner.as_core().id,
-                    entry_point: frag.entry_point.map(Borrowed),
-                    constants: Borrowed(frag.compilation_options.constants),
-                    zero_initialize_workgroup_memory: frag
-                        .compilation_options
-                        .zero_initialize_workgroup_memory,
-                },
-                targets: Borrowed(frag.targets),
+            fragment: desc.fragment.as_ref().map(|frag| {
+                let frag_constants = frag
+                    .compilation_options
+                    .constants
+                    .iter()
+                    .map(|&(key, value)| (String::from(key), value))
+                    .collect();
+                pipe::FragmentState {
+                    stage: pipe::ProgrammableStageDescriptor {
+                        module: frag.module.inner.as_core().id,
+                        entry_point: frag.entry_point.map(Borrowed),
+                        constants: frag_constants,
+                        zero_initialize_workgroup_memory: frag
+                            .compilation_options
+                            .zero_initialize_workgroup_memory,
+                    },
+                    targets: Borrowed(frag.targets),
+                }
             }),
             multiview: desc.multiview,
             cache: desc.cache.map(|cache| cache.inner.as_core().id),
@@ -1324,13 +1346,20 @@ impl dispatch::DeviceInterface for CoreDevice {
     ) -> dispatch::DispatchComputePipeline {
         use wgc::pipeline as pipe;
 
+        let constants = desc
+            .compilation_options
+            .constants
+            .iter()
+            .map(|&(key, value)| (String::from(key), value))
+            .collect();
+
         let descriptor = pipe::ComputePipelineDescriptor {
             label: desc.label.map(Borrowed),
             layout: desc.layout.map(|pll| pll.inner.as_core().id),
             stage: pipe::ProgrammableStageDescriptor {
                 module: desc.module.inner.as_core().id,
                 entry_point: desc.entry_point.map(Borrowed),
-                constants: Borrowed(desc.compilation_options.constants),
+                constants,
                 zero_initialize_workgroup_memory: desc
                     .compilation_options
                     .zero_initialize_workgroup_memory,