From ac65ff9e914a9e0abe65bfa67bbcac6300384133 Mon Sep 17 00:00:00 2001 From: Qining Date: Thu, 13 Sep 2018 10:08:43 -0400 Subject: [PATCH] Vulkan: Fix wrong descriptor set indices when binding descriptor sets (#2191) * Vulkan: Fix wrong descriptor set indices when binding descriptor sets This fixes the wrong calculation of descriptor set indices when binding descriptor sets. Also, for multi-layer maps (e.g. `map!(u32, map!(u32...`), we need to explicitly assign the child map back to the parent map. This is because the generated go/c++ code will not assign the child map back to the parent map automactically (go: `Get()`, c++: `findOrZero`) when write api file like: `parent[parent_key][child_key] = child_content`. TODO: Similar bugs for other mutlti-layer maps --- gapil/analysis/reference_value.go | 20 +++++++++++++++--- gapis/api/vulkan/api/descriptor.api | 32 ++++++----------------------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/gapil/analysis/reference_value.go b/gapil/analysis/reference_value.go index 59b37ab689..f273cc57f0 100644 --- a/gapil/analysis/reference_value.go +++ b/gapil/analysis/reference_value.go @@ -171,8 +171,15 @@ func (v *ReferenceValue) Clone() Value { func (v *ReferenceValue) field(s *scope, name string) Value { candidates := make([]Value, 0, len(v.Assignments)) for a := range v.Assignments { - field := s.getInstance(a).(fieldHolder).field(s, name) - candidates = append(candidates, field) + // Check the assignment has an instance. No instance can happen when the + // assignment took place in a sibling block which has not been merged + // into the common scope yet. + // In this particular case, the instance is inaccessible to this block, + // so skipping it is the correct thing to do. + if fh, ok := s.getInstance(a).(fieldHolder); ok { + field := fh.field(s, name) + candidates = append(candidates, field) + } } if len(candidates) == 0 { return v.Unknown.(fieldHolder).field(s, name) @@ -184,7 +191,14 @@ func (v *ReferenceValue) field(s *scope, name string) Value { // in the scope's instances map. func (v *ReferenceValue) setField(s *scope, name string, val Value) Value { for a := range v.Assignments { - s.instances[a] = s.getInstance(a).(fieldHolder).setField(s, name, val) + // Check the assignment has an instance. No instance can happen when the + // assignment took place in a sibling block which has not been merged + // into the common scope yet. + // In this particular case, the instance is inaccessible to this block, + // so skipping it is the correct thing to do. + if fh, ok := s.getInstance(a).(fieldHolder); ok { + s.instances[a] = fh.setField(s, name, val) + } } return v } diff --git a/gapis/api/vulkan/api/descriptor.api b/gapis/api/vulkan/api/descriptor.api index b8e7fc5a5d..2db1479e3f 100644 --- a/gapis/api/vulkan/api/descriptor.api +++ b/gapis/api/vulkan/api/descriptor.api @@ -671,7 +671,7 @@ vkCmdBindDescriptorSetsArgs { } sub void dovkCmdBindDescriptorSets(ref!vkCmdBindDescriptorSetsArgs args) { - _ = PipelineLayouts[args.Layout] + _ = PipelineLayouts[args.Layout] dynamic_offset_index := MutableU32(0) computeInfo := lastComputeInfo() @@ -696,12 +696,14 @@ sub void dovkCmdBindDescriptorSets(ref!vkCmdBindDescriptorSetsArgs args) { set := args.DescriptorSets[as!u32(i)] setObj := DescriptorSets[set] currentDescriptorSets[args.FirstSet + as!u32(i)] = DescriptorSets[set] + desc_set_buf_offsets := bufferBindingOffsets[as!u32(i)+args.FirstSet] // Since the pDynamicOffsets point into the bindings in order of // binding index, and then array index, we have to loop over all // of the Bindings in order of the binding number. for j in (0 .. setObj.Layout.MaximumBinding + 1) { if j in setObj.Bindings { + desc_binding_buf_offsets := desc_set_buf_offsets[as!u32(j)] binding := setObj.Bindings[j] for k in (0 .. len(binding.BufferBinding)) { bufferBinding := binding.BufferBinding[as!u32(k)] @@ -718,34 +720,12 @@ sub void dovkCmdBindDescriptorSets(ref!vkCmdBindDescriptorSetsArgs args) { default: dynamic_offset_index.Val } - bufferBindingOffsets[as!u32(i)][as!u32(j)][as!u32(k)] = binding_offset - if (bufferBinding.Buffer in Buffers) { - bufferObject := Buffers[bufferBinding.Buffer] - readMemoryInBuffer(bufferObject, binding_offset, bufferBinding.Range) - } - } - for k in (0 .. len(binding.BufferViewBindings)) { - buffer_view := binding.BufferViewBindings[as!u32(k)] - if (buffer_view != as!VkBufferView(0)) { - buffer_view_object := BufferViews[buffer_view] - readMemoryInBuffer(buffer_view_object.Buffer, buffer_view_object.Offset, buffer_view_object.Range) - } - } - for k in (0 .. len(binding.ImageBinding)) { - imageBinding := binding.ImageBinding[as!u32(k)] - _ = Samplers[imageBinding.Sampler] - if (imageBinding.ImageView != as!VkImageView(0)) { - if imageBinding.ImageView in ImageViews { - imageViewObj := ImageViews[imageBinding.ImageView] - imageObj := imageViewObj.Image - rng := imageViewObj.SubresourceRange - readImageSubresource(imageObj, rng) - updateImageQueue(imageObj, rng) - } - } + desc_binding_buf_offsets[as!u32(k)] = binding_offset } + desc_set_buf_offsets[as!u32(j)] = desc_binding_buf_offsets } } + bufferBindingOffsets[as!u32(i)+args.FirstSet] = desc_set_buf_offsets } } }