Repository: gfx-rs/wgpu (language: rust)

Issue:
Conflict between type of `HashMap` in `PipelineCompilationOptions` and `HashMap` exposed by the surface API
**Description**

Currently, the type `PipelineCompilationOptions` has a field `constants` of type that is directly defined as `hashbrown::HashMap`.

However, that type is not exposed to the surface API, making it impossible for the user to declare an object of the same type and then pass to `constants` (unless I am missing how this type is re-exported from the WGPU crate).

Using `[FashHashMap](wgpu::naga::FastHashMap)` (which is an alias of `hashbrown::HashMap`) is also an error, as it uses a hasher different than the one that `constants` expects. Bellow is the case of using `wgpu::naga::FastHashMap<>`:

```rust
let mut vertex_hash_map_constant: wgpu::naga::FastHashMap<String, f64> = Default::default();
for /* ... */ {
    wgpu::naga::FastHashMap::insert(
        // ...
    );
}
let vertex_wgpu_pipeline_compilation_options = wgpu::PipelineCompilationOptions {
    constants: &vertex_hash_map_constant,
    // ...
};
```
Error (minus personal information in the output):
```sh
error[E0308]: mismatched types
    |
126 |                 constants: &vertex_hash_map_constant,
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&HashMap<String, f64>`, found `&HashMap<String, f64, BuildHasherDefault<FxHasher>>`
    |
    = note: expected reference `&hashbrown::map::HashMap<_, _, foldhash::seed::fast::RandomState>`
               found reference `&hashbrown::map::HashMap<_, _, BuildHasherDefault<rustc_hash::FxHasher>>`
```

**Expected vs observed behavior**
Either the hashmap type should be exposed in a way that will not cause confusion to the user, or `PipelineCompilationOptions` should use a generic type that then WGPU converts internally in whatever it uses.

**Platform**

WGPU version: https://github.com/gfx-rs/wgpu/commit/65d499f302fe3482207147d2c70892c12030b442



Discussion / hints:
We really should just use a list hear, as much as I am not thrilled with it.
Update

Using `let mut options = wgpu::PipelineCompilationOptions::default()` to then modify the hashmap of the field `constants` is not a circumvention to the problem, as the field `constants` is an [immutable reference](https://github.com/gfx-rs/wgpu/blob/2a456f5c7b80d41452de7175fe8179ffcc3766b4/wgpu/src/api/common_pipeline.rs#L9-L38).

The solution for now is using the latest commit [fcbadc9529d52f4684924fcaf5f1ec020c343b81] before the expanded use of hashbrown [df54acc05dcaedf51be903604b40f888ee9b9b2a] in the crate.
If you're up to it (or anyone else) this would be a pretty low lift issue to switch to lists.
What list type should be used?
`wgpu::naga::FastHashMap<>`?
Just a `&[(Key, Value)]` type deal
I think `&[(String, f64)]` ended up being a difficult option. That field `.constants` is passed in multiple places wrapped in a `Cow::Borrowed()`, which I assume it is for performance reasons. And those places also use a hashmap, so it would either altering a lot of code in the codebase to use `Cow<[(String, f64)]>` or converting the `[(String, f64)]` into a hashmap as soon as `.constants` is used. I think that you guys do not want that, because that would would add unnecessary overhead and defeat the performance reasons for the use of `Cow<>`. 

## Looking for another solution

So, I investigated a little the hashmaps used in the codebase:

1. An alias `FastHashMap`
https://github.com/gfx-rs/wgpu/blob/4e7d892317ea84e2d781106de6387506a6e28d84/wgpu-core/src/hash_utils.rs#L6-L7

2. The `hashbrown::HashMap` is imported in many places, which is the one used by the `PipelineCompilationOptions`, but not declared as public, which led to the problem in the first place.

3. Also, there is an alias `PipelineConstants`
https://github.com/gfx-rs/wgpu/blob/4e7d892317ea84e2d781106de6387506a6e28d84/naga/src/back/mod.rs#L58

The alias `PipelineConstants` to the private `hashbrown::HashMap` is public, so I used that for the `PipelineCompilationOptions.constants` and it worked.

Solution:

```rust
pub struct PipelineCompilationOptions<'a> {
    pub constants: &'a naga::back::PipelineConstants,
    ...
}
```

## Final questions

1. Currently, we have to use the full path `wgpu::naga::back::PipelineConstants` in the end-user codebase to use that alias. Should we re-export that alias with a shorter path?

2. Would not be simpler to have only one hashmap type for the whole crate?
Those two types of `hashbrown::HashMap` with different hashers make them incompatible with one another.

Sorry about the wall of text.
I'll respond to the whole wall of text tomorrow, but I would note that I would not treat the boilerplate of pipeline creation as hot code. Some small waste there is acceptable. I'm not worried about allocating a hashmap here interally. 

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 21e129ec6d..892dc6e987 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/deno_webgpu/device.rs b/deno_webgpu/device.rs
index d4772e7cb7..01f86c4343 100644
--- a/deno_webgpu/device.rs
+++ b/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(
diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs
index f56356ba93..a15b4ea3ff 100644
--- a/wgpu-core/src/device/resource.rs
+++ b/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,
diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs
index dcd9cf4922..12cfc4cc07 100644
--- a/wgpu-core/src/pipeline.rs
+++ b/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
diff --git a/wgpu/src/api/common_pipeline.rs b/wgpu/src/api/common_pipeline.rs
index 7f07231f9d..79ac7121ec 100644
--- a/wgpu/src/api/common_pipeline.rs
+++ b/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,
         }
     }
diff --git a/wgpu/src/api/shader_module.rs b/wgpu/src/api/shader_module.rs
index 2f3e39fc9b..14eadfd692 100644
--- a/wgpu/src/api/shader_module.rs
+++ b/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")]
diff --git a/wgpu/src/backend/webgpu.rs b/wgpu/src/backend/webgpu.rs
index deaf45dee7..898372efef 100644
--- a/wgpu/src/backend/webgpu.rs
+++ b/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");
     }
 
diff --git a/wgpu/src/backend/wgpu_core.rs b/wgpu/src/backend/wgpu_core.rs
index 0108829063..539b6da3e3 100644
--- a/wgpu/src/backend/wgpu_core.rs
+++ b/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,6 +1267,14 @@ 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),
@@ -1268,7 +1282,7 @@ impl dispatch::DeviceInterface for CoreDevice {
                 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,


Test patch:
diff --git a/tests/tests/shader/array_size_overrides.rs b/tests/tests/shader/array_size_overrides.rs
index 7f1d324254..f3c49005bc 100644
--- a/tests/tests/shader/array_size_overrides.rs
+++ b/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(
diff --git a/tests/tests/shader/workgroup_size_overrides.rs b/tests/tests/shader/workgroup_size_overrides.rs
index 458011079a..2624640f64 100644
--- a/tests/tests/shader/workgroup_size_overrides.rs
+++ b/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(