Merge #201

201: Add uniform buffer offset alignment constant r=kvark a=cloudhead

Fixes #158 

Also needs to be exposed in `wgpu-rs`.

Happy to change the var name to something different or shorter.

Co-authored-by: Alexis Sellier <alexis@monadic.xyz>

wgpu-native/src/command/compute.rs

@@ -8,6 +8,7 @@ use crate::{
     CommandBufferId,
     ComputePassId,
     ComputePipelineId,
+    BIND_BUFFER_ALIGNMENT,
     RawString,
     Stored,
 };
@@ -73,6 +74,18 @@ pub extern "C" fn wgpu_compute_pass_set_bind_group(
         &[]
     };
 
+    if cfg!(debug_assertions) {
+        for off in offsets {
+            assert_eq!(
+                *off % BIND_BUFFER_ALIGNMENT,
+                0,
+                "Misaligned dynamic buffer offset: {} does not align with {}",
+                off,
+                BIND_BUFFER_ALIGNMENT
+            );
+        }
+    }
+
     //Note: currently, WebGPU compute passes have synchronization defined
     // at a dispatch granularity, so we insert the necessary barriers here.
 

wgpu-native/src/command/render.rs

@@ -1,7 +1,7 @@
 use crate::{
     command::bind::{Binder, LayoutChange},
     conv,
-    device::{MAX_VERTEX_BUFFERS, RenderPassContext},
+    device::{RenderPassContext, BIND_BUFFER_ALIGNMENT, MAX_VERTEX_BUFFERS},
     hub::HUB,
     pipeline::{IndexFormat, InputStepMode, PipelineFlags},
     resource::BufferUsage,
@@ -219,6 +219,18 @@ pub extern "C" fn wgpu_render_pass_set_bind_group(
         &[]
     };
 
+    if cfg!(debug_assertions) {
+        for off in offsets {
+            assert_eq!(
+                *off % BIND_BUFFER_ALIGNMENT,
+                0,
+                "Misaligned dynamic buffer offset: {} does not align with {}",
+                off,
+                BIND_BUFFER_ALIGNMENT
+            );
+        }
+    }
+
     pass.trackers.consume_by_extend(&bind_group.used);
 
     if let Some((pipeline_layout_id, follow_up)) =

wgpu-native/src/device.rs

@@ -60,6 +60,9 @@ const CLEANUP_WAIT_MS: u64 = 5000;
 pub const MAX_COLOR_TARGETS: usize = 4;
 pub const MAX_VERTEX_BUFFERS: usize = 8;
 
+/// Bound uniform/storage buffer offsets must be aligned to this number.
+pub const BIND_BUFFER_ALIGNMENT: hal::buffer::Offset = 256;
+
 pub fn all_buffer_stages() -> hal::pso::PipelineStage {
     use hal::pso::PipelineStage as Ps;
     Ps::DRAW_INDIRECT
@@ -406,7 +409,6 @@ fn map_buffer(
 pub struct Device<B: hal::Backend> {
     pub(crate) raw: B::Device,
     adapter_id: AdapterId,
-    limits: hal::Limits,
     pub(crate) queue_group: hal::QueueGroup<B, hal::General>,
     pub(crate) com_allocator: command::CommandAllocator<B>,
     mem_allocator: Mutex<Heaps<B>>,
@@ -424,7 +426,6 @@ impl<B: hal::Backend> Device<B> {
         adapter_id: AdapterId,
         queue_group: hal::QueueGroup<B, hal::General>,
         mem_props: hal::MemoryProperties,
-        limits: hal::Limits,
     ) -> Self {
         // don't start submission index at zero
         let life_guard = LifeGuard::new();
@@ -459,7 +460,6 @@ impl<B: hal::Backend> Device<B> {
         Device {
             raw,
             adapter_id,
-            limits,
             com_allocator: command::CommandAllocator::new(queue_group.family()),
             mem_allocator: Mutex::new(heaps),
             desc_allocator: Mutex::new(DescriptorAllocator::new()),
@@ -1020,15 +1020,13 @@ pub fn device_create_bind_group(
             binding_model::BindingResource::Buffer(ref bb) => {
                 let (alignment, usage) = match decl.ty {
                     binding_model::BindingType::UniformBuffer
-                    | binding_model::BindingType::UniformBufferDynamic => (
-                        device.limits.min_uniform_buffer_offset_alignment,
-                        resource::BufferUsage::UNIFORM,
-                    ),
+                    | binding_model::BindingType::UniformBufferDynamic => {
+                        (BIND_BUFFER_ALIGNMENT, resource::BufferUsage::UNIFORM)
+                    }
                     binding_model::BindingType::StorageBuffer
-                    | binding_model::BindingType::StorageBufferDynamic => (
-                        device.limits.min_storage_buffer_offset_alignment,
-                        resource::BufferUsage::STORAGE,
-                    ),
+                    | binding_model::BindingType::StorageBufferDynamic => {
+                        (BIND_BUFFER_ALIGNMENT, resource::BufferUsage::STORAGE)
+                    }
                     binding_model::BindingType::Sampler
                     | binding_model::BindingType::SampledTexture
                     | binding_model::BindingType::StorageTexture => {

wgpu-native/src/instance.rs

@@ -1,5 +1,6 @@
 use crate::{
     binding_model::MAX_BIND_GROUPS,
+    device::BIND_BUFFER_ALIGNMENT,
     hub::HUB,
     AdapterHandle,
     AdapterId,
@@ -225,7 +226,19 @@ pub extern "C" fn wgpu_instance_get_adapter(
     desc: &AdapterDescriptor,
 ) -> AdapterId {
     let adapter = instance_get_adapter(instance_id, desc);
+    let limits = adapter.physical_device.limits();
+
     info!("Adapter {:?}", adapter.info);
+
+    assert!(
+        BIND_BUFFER_ALIGNMENT % limits.min_storage_buffer_offset_alignment == 0,
+        "Adapter storage buffer offset alignment not compatible with WGPU"
+    );
+    assert!(
+        BIND_BUFFER_ALIGNMENT % limits.min_uniform_buffer_offset_alignment == 0,
+        "Adapter uniform buffer offset alignment not compatible with WGPU"
+    );
+
     HUB.adapters.register_local(adapter)
 }
 
@@ -234,8 +247,8 @@ pub fn adapter_create_device(adapter_id: AdapterId, _desc: &DeviceDescriptor) ->
     let adapter = &adapter_guard[adapter_id];
     let (raw, queue_group) = adapter.open_with::<_, hal::General>(1, |_qf| true).unwrap();
     let mem_props = adapter.physical_device.memory_properties();
-    let limits = adapter.physical_device.limits();
-    DeviceHandle::new(raw, adapter_id, queue_group, mem_props, limits)
+
+    DeviceHandle::new(raw, adapter_id, queue_group, mem_props)
 }
 
 #[cfg(feature = "local")]