From 9ab5ff6d4ef8e3f5400b821e04417f41958a7f3f Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Wed, 2 Sep 2026 22:01:59 +0300 Subject: [PATCH] fix: dynamic uniform/storage buffer offsets (Vulkan + Software) (#343) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vulkan: bufferBindingTypeToVk() now returns VK_DESCRIPTOR_TYPE_*_DYNAMIC when HasDynamicOffset is true. DescriptorCounts tracks dynamic counts separately. Descriptor pool includes dynamic pool sizes. Matches Rust wgpu-hal conv.rs:834-858. Software: buildExecutionContext() checks HasDynamicOffset per-binding before consuming from dynamic offsets array. BindGroupLayout stores entries for HasDynamicOffset lookup. DX12: documented as TODO — requires root CBV descriptor architecture (Rust wgpu-hal dx12/command.rs:1175-1236). GLES + Metal already correct (unchanged). Fixes #343 --- hal/dx12/command.go | 14 ++++++++-- hal/software/command.go | 3 ++- hal/software/device.go | 39 +++++++++++++++++++++++----- hal/software/draw.go | 5 ++-- hal/software/resource.go | 13 +++++----- hal/vulkan/compute_test.go | 17 ++++++++----- hal/vulkan/convert.go | 15 ++++++++--- hal/vulkan/convert_test.go | 20 +++++++++------ hal/vulkan/descriptor.go | 52 ++++++++++++++++++++++++++------------ hal/vulkan/device.go | 20 +++++++++++---- 10 files changed, 140 insertions(+), 58 deletions(-) diff --git a/hal/dx12/command.go b/hal/dx12/command.go index f76d1716..c221ada7 100644 --- a/hal/dx12/command.go +++ b/hal/dx12/command.go @@ -656,7 +656,14 @@ func (e *RenderPassEncoder) SetBindGroup(index uint32, group hal.BindGroup, offs // Bind the group using graphics root descriptor tables. e.encoder.bindGroupToRootTables(index, bg, false, mappings) e.encoder.trackBindGroupState(bg) - _ = offsets // Dynamic offsets handled via root constants (simplified for now) + + // TODO(#343): DX12 dynamic uniform buffer offsets require root CBV descriptors + // (not descriptor tables) so the GPU virtual address can be adjusted per-draw. + // Rust wgpu-hal uses DynamicBuffer::Uniform(gpu_base) with root descriptor slots + // and DynamicBuffer::Storage with root constants for storage buffer offsets. + // This requires restructuring CreatePipelineLayout to allocate root parameter + // slots for each dynamic buffer binding. See Rust wgpu-hal dx12/command.rs:1175-1236. + _ = offsets } // SetVertexBuffer sets a vertex buffer. @@ -931,7 +938,10 @@ func (e *ComputePassEncoder) SetBindGroup(index uint32, group hal.BindGroup, off // entry types (BindingTypeStorageBuffer) with buffer binding handles. e.boundStorageBuffers = append(e.boundStorageBuffers, bg.storageBuffers...) - _ = offsets // Dynamic offsets handled via root constants (simplified for now) + // TODO(#343): DX12 dynamic buffer offsets — same as RenderPassEncoder.SetBindGroup. + // Requires root CBV slots for dynamic uniform buffers and root constants for + // dynamic storage buffer offsets. See Rust wgpu-hal dx12/command.rs:1175-1236. + _ = offsets } // Dispatch dispatches compute work. diff --git a/hal/software/command.go b/hal/software/command.go index 03593ea6..4bbf9a92 100644 --- a/hal/software/command.go +++ b/hal/software/command.go @@ -733,6 +733,7 @@ func (c *ComputePassEncoder) Dispatch(x, y, z uint32) { if bg == nil { continue } + // Dynamic offsets are only consumed for bindings with HasDynamicOffset. dynIdx := 0 for bindingIdx, bs := range bg.bufferBindings { if bs.buf == nil { @@ -741,7 +742,7 @@ func (c *ComputePassEncoder) Dispatch(x, y, z uint32) { bs.buf.mu.Lock() data := bs.buf.data off := bs.offset - if dynIdx < len(bg.dynamicOffsets) { + if bg.hasDynamicOffset[bindingIdx] && dynIdx < len(bg.dynamicOffsets) { off += uint64(bg.dynamicOffsets[dynIdx]) dynIdx++ } diff --git a/hal/software/device.go b/hal/software/device.go index 0c6fbad5..9d7bcf78 100644 --- a/hal/software/device.go +++ b/hal/software/device.go @@ -142,9 +142,21 @@ func (d *Device) CreateSampler(desc *hal.SamplerDescriptor) (hal.Sampler, error) // DestroySampler is a no-op. func (d *Device) DestroySampler(_ hal.Sampler) {} +// BindGroupLayout stores layout entries for the software backend. +// Entries are needed at draw time to determine which buffer bindings +// have HasDynamicOffset and should consume dynamic offset values. +type BindGroupLayout struct { + Resource + entries []gputypes.BindGroupLayoutEntry +} + // CreateBindGroupLayout creates a software bind group layout. -func (d *Device) CreateBindGroupLayout(_ *hal.BindGroupLayoutDescriptor) (hal.BindGroupLayout, error) { - return &Resource{}, nil +func (d *Device) CreateBindGroupLayout(desc *hal.BindGroupLayoutDescriptor) (hal.BindGroupLayout, error) { + bgl := &BindGroupLayout{} + if desc != nil { + bgl.entries = desc.Entries + } + return bgl, nil } // DestroyBindGroupLayout is a no-op. @@ -152,15 +164,28 @@ func (d *Device) DestroyBindGroupLayout(_ hal.BindGroupLayout) {} // CreateBindGroup creates a software bind group. // It resolves handle-based entries to typed software resources using the device registry. +// Layout entries are inspected to build the hasDynamicOffset map so that dynamic +// offsets are only applied to bindings explicitly marked with HasDynamicOffset. func (d *Device) CreateBindGroup(desc *hal.BindGroupDescriptor) (hal.BindGroup, error) { bg := &BindGroup{ - desc: desc, - textureViews: make(map[uint32]*TextureView), - buffers: make(map[uint32]*Buffer), - bufferBindings: make(map[uint32]bufferSlice), - samplers: make(map[uint32]*SamplerResource), + desc: desc, + textureViews: make(map[uint32]*TextureView), + buffers: make(map[uint32]*Buffer), + bufferBindings: make(map[uint32]bufferSlice), + samplers: make(map[uint32]*SamplerResource), + hasDynamicOffset: make(map[uint32]bool), } + + // Extract HasDynamicOffset from layout entries. if desc != nil { + if bgl, ok := desc.Layout.(*BindGroupLayout); ok && bgl != nil { + for _, le := range bgl.entries { + if le.Buffer != nil && le.Buffer.HasDynamicOffset { + bg.hasDynamicOffset[le.Binding] = true + } + } + } + for _, entry := range desc.Entries { switch res := entry.Resource.(type) { case gputypes.TextureViewBinding: diff --git a/hal/software/draw.go b/hal/software/draw.go index 4bf30d76..75993e2c 100644 --- a/hal/software/draw.go +++ b/hal/software/draw.go @@ -808,7 +808,8 @@ func (r *RenderPassEncoder) buildExecutionContext() *shader.ExecutionContext { if bg == nil { continue } - // Buffers (uniform/storage) — apply offset/size from BufferBinding + dynamic offsets. + // Buffers (uniform/storage) — apply offset/size from BufferBinding. + // Dynamic offsets are only consumed for bindings with HasDynamicOffset. dynIdx := 0 for bindingIdx, bs := range bg.bufferBindings { if bs.buf == nil { @@ -817,7 +818,7 @@ func (r *RenderPassEncoder) buildExecutionContext() *shader.ExecutionContext { bs.buf.mu.RLock() data := bs.buf.data off := bs.offset - if dynIdx < len(bg.dynamicOffsets) { + if bg.hasDynamicOffset[bindingIdx] && dynIdx < len(bg.dynamicOffsets) { off += uint64(bg.dynamicOffsets[dynIdx]) dynIdx++ } diff --git a/hal/software/resource.go b/hal/software/resource.go index 5d03160e..9bb416b7 100644 --- a/hal/software/resource.go +++ b/hal/software/resource.go @@ -424,12 +424,13 @@ type bufferSlice struct { type BindGroup struct { Resource - desc *hal.BindGroupDescriptor - textureViews map[uint32]*TextureView // binding index -> resolved texture view - buffers map[uint32]*Buffer // binding index -> resolved buffer (legacy, offset=0) - bufferBindings map[uint32]bufferSlice // binding index -> buffer + offset/size - samplers map[uint32]*SamplerResource // binding index -> resolved sampler - dynamicOffsets []uint32 // applied via SetBindGroup + desc *hal.BindGroupDescriptor + textureViews map[uint32]*TextureView // binding index -> resolved texture view + buffers map[uint32]*Buffer // binding index -> resolved buffer (legacy, offset=0) + bufferBindings map[uint32]bufferSlice // binding index -> buffer + offset/size + samplers map[uint32]*SamplerResource // binding index -> resolved sampler + dynamicOffsets []uint32 // applied via SetBindGroup + hasDynamicOffset map[uint32]bool // binding index -> true if HasDynamicOffset } // ComputePipeline stores compute pipeline configuration for the software backend. diff --git a/hal/vulkan/compute_test.go b/hal/vulkan/compute_test.go index 851d0bd4..fcaba747 100644 --- a/hal/vulkan/compute_test.go +++ b/hal/vulkan/compute_test.go @@ -145,16 +145,19 @@ func TestVulkanComputeStorageBuffer(t *testing.T) { t.Run("binding type conversion", func(t *testing.T) { tests := []struct { - bindingType gputypes.BufferBindingType - expect vk.DescriptorType + bindingType gputypes.BufferBindingType + hasDynamicOffset bool + expect vk.DescriptorType }{ - {gputypes.BufferBindingTypeStorage, vk.DescriptorTypeStorageBuffer}, - {gputypes.BufferBindingTypeReadOnlyStorage, vk.DescriptorTypeStorageBuffer}, - {gputypes.BufferBindingTypeUniform, vk.DescriptorTypeUniformBuffer}, + {gputypes.BufferBindingTypeStorage, false, vk.DescriptorTypeStorageBuffer}, + {gputypes.BufferBindingTypeStorage, true, vk.DescriptorTypeStorageBufferDynamic}, + {gputypes.BufferBindingTypeReadOnlyStorage, false, vk.DescriptorTypeStorageBuffer}, + {gputypes.BufferBindingTypeUniform, false, vk.DescriptorTypeUniformBuffer}, + {gputypes.BufferBindingTypeUniform, true, vk.DescriptorTypeUniformBufferDynamic}, } for _, tt := range tests { - if got := bufferBindingTypeToVk(tt.bindingType); got != tt.expect { - t.Errorf("bufferBindingTypeToVk(%v) = %v, want %v", tt.bindingType, got, tt.expect) + if got := bufferBindingTypeToVk(tt.bindingType, tt.hasDynamicOffset); got != tt.expect { + t.Errorf("bufferBindingTypeToVk(%v, dynamic=%v) = %v, want %v", tt.bindingType, tt.hasDynamicOffset, got, tt.expect) } } }) diff --git a/hal/vulkan/convert.go b/hal/vulkan/convert.go index 15fe3467..e35d5567 100644 --- a/hal/vulkan/convert.go +++ b/hal/vulkan/convert.go @@ -284,13 +284,20 @@ func shaderStagesToVk(stages gputypes.ShaderStages) vk.ShaderStageFlags { } // bufferBindingTypeToVk converts WebGPU buffer binding type to Vulkan descriptor type. -func bufferBindingTypeToVk(bindingType gputypes.BufferBindingType) vk.DescriptorType { +// When hasDynamicOffset is true, returns the corresponding dynamic descriptor type +// (VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC) +// which tells Vulkan to apply dynamic offsets at vkCmdBindDescriptorSets time. +func bufferBindingTypeToVk(bindingType gputypes.BufferBindingType, hasDynamicOffset bool) vk.DescriptorType { switch bindingType { case gputypes.BufferBindingTypeUniform: + if hasDynamicOffset { + return vk.DescriptorTypeUniformBufferDynamic + } return vk.DescriptorTypeUniformBuffer - case gputypes.BufferBindingTypeStorage: - return vk.DescriptorTypeStorageBuffer - case gputypes.BufferBindingTypeReadOnlyStorage: + case gputypes.BufferBindingTypeStorage, gputypes.BufferBindingTypeReadOnlyStorage: + if hasDynamicOffset { + return vk.DescriptorTypeStorageBufferDynamic + } return vk.DescriptorTypeStorageBuffer default: return vk.DescriptorTypeUniformBuffer diff --git a/hal/vulkan/convert_test.go b/hal/vulkan/convert_test.go index fa79878a..b189c6b6 100644 --- a/hal/vulkan/convert_test.go +++ b/hal/vulkan/convert_test.go @@ -358,19 +358,23 @@ func TestShaderStagesToVk(t *testing.T) { // TestBufferBindingTypeToVk tests buffer binding type conversions. func TestBufferBindingTypeToVk(t *testing.T) { tests := []struct { - name string - bindingType gputypes.BufferBindingType - expect vk.DescriptorType + name string + bindingType gputypes.BufferBindingType + hasDynamicOffset bool + expect vk.DescriptorType }{ - {"Uniform", gputypes.BufferBindingTypeUniform, vk.DescriptorTypeUniformBuffer}, - {"Storage", gputypes.BufferBindingTypeStorage, vk.DescriptorTypeStorageBuffer}, - {"ReadOnlyStorage", gputypes.BufferBindingTypeReadOnlyStorage, vk.DescriptorTypeStorageBuffer}, - {"Unknown defaults to Uniform", gputypes.BufferBindingType(99), vk.DescriptorTypeUniformBuffer}, + {"Uniform", gputypes.BufferBindingTypeUniform, false, vk.DescriptorTypeUniformBuffer}, + {"UniformDynamic", gputypes.BufferBindingTypeUniform, true, vk.DescriptorTypeUniformBufferDynamic}, + {"Storage", gputypes.BufferBindingTypeStorage, false, vk.DescriptorTypeStorageBuffer}, + {"StorageDynamic", gputypes.BufferBindingTypeStorage, true, vk.DescriptorTypeStorageBufferDynamic}, + {"ReadOnlyStorage", gputypes.BufferBindingTypeReadOnlyStorage, false, vk.DescriptorTypeStorageBuffer}, + {"ReadOnlyStorageDynamic", gputypes.BufferBindingTypeReadOnlyStorage, true, vk.DescriptorTypeStorageBufferDynamic}, + {"Unknown defaults to Uniform", gputypes.BufferBindingType(99), false, vk.DescriptorTypeUniformBuffer}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := bufferBindingTypeToVk(tt.bindingType) + got := bufferBindingTypeToVk(tt.bindingType, tt.hasDynamicOffset) if got != tt.expect { t.Errorf("bufferBindingTypeToVk() = %v, want %v", got, tt.expect) } diff --git a/hal/vulkan/descriptor.go b/hal/vulkan/descriptor.go index 2d76d2d6..f2edf903 100644 --- a/hal/vulkan/descriptor.go +++ b/hal/vulkan/descriptor.go @@ -16,21 +16,26 @@ import ( // DescriptorCounts tracks the number of descriptors by type. // Used to determine pool sizes for allocation. +// Dynamic buffer counts are tracked separately because Vulkan requires +// separate pool size entries for VK_DESCRIPTOR_TYPE_*_BUFFER_DYNAMIC. type DescriptorCounts struct { - Samplers uint32 - SampledImages uint32 - StorageImages uint32 - UniformBuffers uint32 - StorageBuffers uint32 - UniformTexelBuffer uint32 - StorageTexelBuffer uint32 - InputAttachments uint32 + Samplers uint32 + SampledImages uint32 + StorageImages uint32 + UniformBuffers uint32 + StorageBuffers uint32 + UniformBuffersDynamic uint32 + StorageBuffersDynamic uint32 + UniformTexelBuffer uint32 + StorageTexelBuffer uint32 + InputAttachments uint32 } // Total returns the total number of descriptors. func (c DescriptorCounts) Total() uint32 { return c.Samplers + c.SampledImages + c.StorageImages + c.UniformBuffers + c.StorageBuffers + + c.UniformBuffersDynamic + c.StorageBuffersDynamic + c.UniformTexelBuffer + c.StorageTexelBuffer + c.InputAttachments } @@ -42,14 +47,16 @@ func (c DescriptorCounts) IsEmpty() bool { // Multiply multiplies all counts by a factor. func (c DescriptorCounts) Multiply(factor uint32) DescriptorCounts { return DescriptorCounts{ - Samplers: c.Samplers * factor, - SampledImages: c.SampledImages * factor, - StorageImages: c.StorageImages * factor, - UniformBuffers: c.UniformBuffers * factor, - StorageBuffers: c.StorageBuffers * factor, - UniformTexelBuffer: c.UniformTexelBuffer * factor, - StorageTexelBuffer: c.StorageTexelBuffer * factor, - InputAttachments: c.InputAttachments * factor, + Samplers: c.Samplers * factor, + SampledImages: c.SampledImages * factor, + StorageImages: c.StorageImages * factor, + UniformBuffers: c.UniformBuffers * factor, + StorageBuffers: c.StorageBuffers * factor, + UniformBuffersDynamic: c.UniformBuffersDynamic * factor, + StorageBuffersDynamic: c.StorageBuffersDynamic * factor, + UniformTexelBuffer: c.UniformTexelBuffer * factor, + StorageTexelBuffer: c.StorageTexelBuffer * factor, + InputAttachments: c.InputAttachments * factor, } } @@ -217,6 +224,7 @@ func (a *DescriptorAllocator) createPool(counts DescriptorCounts) (*DescriptorPo // different bind group layouts (e.g., uniform-only vs sampler+texture) // share the same pool. Requested counts scale the primary types; // all other types get a reasonable baseline allocation. + // Dynamic buffer types are included when any dynamic offsets are used. poolSizes := []vk.DescriptorPoolSize{ {Type: vk.DescriptorTypeSampler, DescriptorCount: max(counts.Samplers, 1) * poolSize}, {Type: vk.DescriptorTypeSampledImage, DescriptorCount: max(counts.SampledImages, 1) * poolSize}, @@ -225,6 +233,18 @@ func (a *DescriptorAllocator) createPool(counts DescriptorCounts) (*DescriptorPo {Type: vk.DescriptorTypeStorageBuffer, DescriptorCount: max(counts.StorageBuffers*poolSize, poolSize/2)}, {Type: vk.DescriptorTypeCombinedImageSampler, DescriptorCount: max(counts.Samplers, 1) * poolSize}, } + if counts.UniformBuffersDynamic > 0 { + poolSizes = append(poolSizes, vk.DescriptorPoolSize{ + Type: vk.DescriptorTypeUniformBufferDynamic, + DescriptorCount: counts.UniformBuffersDynamic * poolSize, + }) + } + if counts.StorageBuffersDynamic > 0 { + poolSizes = append(poolSizes, vk.DescriptorPoolSize{ + Type: vk.DescriptorTypeStorageBufferDynamic, + DescriptorCount: counts.StorageBuffersDynamic * poolSize, + }) + } createInfo := vk.DescriptorPoolCreateInfo{ SType: vk.StructureTypeDescriptorPoolCreateInfo, diff --git a/hal/vulkan/device.go b/hal/vulkan/device.go index f093d819..319265ae 100644 --- a/hal/vulkan/device.go +++ b/hal/vulkan/device.go @@ -949,14 +949,24 @@ func (d *Device) CreateBindGroupLayout(desc *hal.BindGroupLayoutDescriptor) (hal StageFlags: shaderStagesToVk(entry.Visibility), } - // Determine descriptor type based on which binding is set + // Determine descriptor type based on which binding is set. + // For buffers, pass HasDynamicOffset so Vulkan gets the correct + // dynamic descriptor type (VK_DESCRIPTOR_TYPE_*_BUFFER_DYNAMIC). switch { case entry.Buffer != nil: - binding.DescriptorType = bufferBindingTypeToVk(entry.Buffer.Type) - if entry.Buffer.Type == gputypes.BufferBindingTypeUniform { - counts.UniformBuffers++ + binding.DescriptorType = bufferBindingTypeToVk(entry.Buffer.Type, entry.Buffer.HasDynamicOffset) + if entry.Buffer.HasDynamicOffset { + if entry.Buffer.Type == gputypes.BufferBindingTypeUniform { + counts.UniformBuffersDynamic++ + } else { + counts.StorageBuffersDynamic++ + } } else { - counts.StorageBuffers++ + if entry.Buffer.Type == gputypes.BufferBindingTypeUniform { + counts.UniformBuffers++ + } else { + counts.StorageBuffers++ + } } case entry.Sampler != nil: binding.DescriptorType = vk.DescriptorTypeSampler