diff --git a/CHANGELOG.md b/CHANGELOG.md index 21e129ec6d0..892dc6e9870 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 d4772e7cb73..01f86c43434 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/tests/tests/shader/array_size_overrides.rs b/tests/tests/shader/array_size_overrides.rs index 7f1d324254c..f3c49005bc6 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 458011079a2..2624640f648 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( diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index f56356ba93b..a15b4ea3ff3 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 dcd9cf49220..12cfc4cc07b 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 7f07231f9d4..79ac7121eca 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, + 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> = - 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 2f3e39fc9b0..14eadfd692f 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, + /// 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 deaf45dee71..898372efeff 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) { +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) -> 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 01088290632..539b6da3e3c 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,