From c928b2fdf00e504f4b1de94da677a8290a4cfb43 Mon Sep 17 00:00:00 2001 From: Qining Date: Thu, 22 Nov 2018 10:40:52 -0500 Subject: [PATCH] Vulkan: Lazy create descriptor bindings and array elements in state (#2384) * Vulkan: Lazy create descriptor bindings and array elements in state Some applications allocate a lot of descriptors a head but only update a few of them a few times. Allocating those bindings and elements only when the descriptors are updated can reduce the state tracking overhead for such cases. --- gapii/cc/vulkan_inlines.inc | 6 ++- gapis/api/vulkan/api/descriptor.api | 67 ++++++++++++----------------- gapis/api/vulkan/errors.api | 1 + gapis/api/vulkan/externs.go | 8 ++++ 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/gapii/cc/vulkan_inlines.inc b/gapii/cc/vulkan_inlines.inc index 97532cc532..d9e54487a2 100644 --- a/gapii/cc/vulkan_inlines.inc +++ b/gapii/cc/vulkan_inlines.inc @@ -54,4 +54,8 @@ inline void VulkanSpy::vkErrInvalidImageLayout(CallObserver*, VkImage img, uint3 inline void VulkanSpy::vkErrInvalidImageSubresource(CallObserver*, VkImage img, std::string subresourceType, uint32_t value) { GAPID_WARNING("Error: Accessing invalid image subresource at Image: %" PRIu64 ", %s: %" PRIu32, img, subresourceType.c_str(), value); -} \ No newline at end of file +} + +inline void VulkanSpy::vkErrInvalidDescriptorBindingType(CallObserver*, VkDescriptorSet set, uint32_t binding, uint32_t layout_type, uint32_t update_type) { + GAPID_WARNING("Error: Updating descriptor binding at: %" PRIu64 ": %" PRIu32 " with type: %" PRIu32 ", but the type defined in descriptor set layout is: %" PRIu32 "", set, binding, layout_type, update_type); +} diff --git a/gapis/api/vulkan/api/descriptor.api b/gapis/api/vulkan/api/descriptor.api index 48c6beec68..279506fde5 100644 --- a/gapis/api/vulkan/api/descriptor.api +++ b/gapis/api/vulkan/api/descriptor.api @@ -284,38 +284,6 @@ cmd VkResult vkAllocateDescriptorSets( pool := DescriptorPools[info.descriptorPool] pool.DescriptorSets[handle] = object object.Layout = DescriptorSetLayouts[layouts[i]] - for j in (0 .. object.Layout.MaximumBinding + 1) { - if j in object.Layout.Bindings { - binding := object.Layout.Bindings[j] - descriptorBinding := new!DescriptorBinding( - BindingType: binding.Type) - imageInfos := descriptorBinding.ImageBinding - bufferInfos := descriptorBinding.BufferBinding - bufferViews := descriptorBinding.BufferViewBindings - for k in (0 .. binding.Count) { - switch binding.Type { - case VK_DESCRIPTOR_TYPE_SAMPLER, - VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, - VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, - VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, - VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: - imageInfos[k] = new!VkDescriptorImageInfo() - case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER, - VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: - bufferViews[k] = as!VkBufferView(0) - case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, - VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, - VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, - VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: - bufferInfos[k] = new!VkDescriptorBufferInfo() - } - } - descriptorBinding.ImageBinding = imageInfos - descriptorBinding.BufferBinding = bufferInfos - descriptorBinding.BufferViewBindings = bufferViews - object.Bindings[j] = descriptorBinding - } - } DescriptorSets[handle] = object } } @@ -575,7 +543,7 @@ cmd void vkUpdateDescriptorSets( } } } - + cs := pDescriptorCopies[0:descriptorCopyCount] for i in (0 .. descriptorCopyCount) { c := cs[i] @@ -600,7 +568,16 @@ cmd void vkUpdateDescriptorSets( set := DescriptorSets[w.DstSet] binding := w.Binding arrayIndex := w.BindingArrayIndex - setBinding := set.Bindings[binding] + setBinding := switch set.Bindings[binding] == null { + case false: + set.Bindings[binding] + case true: + new!DescriptorBinding(BindingType: set.Layout.Bindings[binding].Type) + } + if set.Layout.Bindings[binding].Type != setBinding.BindingType { + vkErrInvalidDescriptorBindingType(set.VulkanHandle, binding, set.Layout.Bindings[binding].Type, setBinding.BindingType) + } + switch w.Type { case VK_DESCRIPTOR_TYPE_SAMPLER, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, @@ -639,7 +616,7 @@ cmd void vkUpdateDescriptorSets( } } } - + case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, @@ -658,8 +635,18 @@ cmd void vkUpdateDescriptorSets( for _ , _ , c in copies { srcSet := DescriptorSets[c.SrcSet] dstSet := DescriptorSets[c.DstSet] - srcBinding := srcSet.Bindings[c.SrcBinding] - dstBinding := dstSet.Bindings[c.DstBinding] + srcBinding := switch (c.SrcBinding in srcSet.Bindings) { + case false: + srcSet.Bindings[c.SrcBinding] + case true: + new!DescriptorBinding(BindingType: srcSet.Layout.Bindings[c.SrcBinding].Type) + } + dstBinding := switch (c.DstBinding in dstSet.Bindings) { + case false: + dstSet.Bindings[c.DstBinding] + case true: + new!DescriptorBinding(BindingType: dstSet.Layout.Bindings[c.DstBinding].Type) + } switch (srcBinding.BindingType) { case VK_DESCRIPTOR_TYPE_SAMPLER, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, @@ -733,7 +720,7 @@ vkCmdBindDescriptorSetsArgs { map!(u32, u32) DynamicOffsets } -@internal class +@internal class emptyBufferOffsets { map!(u32, map!(u32, map!(VkDeviceSize, bool))) BufferBindingOffsets } @@ -778,7 +765,7 @@ sub void dovkCmdBindDescriptorSets(ref!vkCmdBindDescriptorSetsArgs args) { case false: emptyBufferOffsets().BufferBindingOffsets } - + // 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. @@ -815,7 +802,7 @@ sub void dovkCmdBindDescriptorSets(ref!vkCmdBindDescriptorSetsArgs args) { if !hasBeenRead.b { oldSets := ProcessedDescriptorSets.val[set].bufferOffsets - ProcessedDescriptorSets.val[set] = + ProcessedDescriptorSets.val[set] = ProcessedDescriptorSet(oldSets, true) } diff --git a/gapis/api/vulkan/errors.api b/gapis/api/vulkan/errors.api index 623aa20f11..b04a3848cb 100644 --- a/gapis/api/vulkan/errors.api +++ b/gapis/api/vulkan/errors.api @@ -44,6 +44,7 @@ extern void vkErrNotNullPointer(string pointerType) extern void vkErrUnrecognizedExtension(string name) extern void vkErrExpectNVDedicatedlyAllocatedHandle(string handleType, u64 handle) extern void vkErrInvalidDescriptorArrayElement(u64 set, u32 binding, u32 arrayIndex) +extern void vkErrInvalidDescriptorBindingType(VkDescriptorSet set, u32 binding, VkDescriptorType layoutType, VkDescriptorType updateType) extern void vkErrCommandBufferIncomplete(VkCommandBuffer cmdbuf) extern void vkErrInvalidImageLayout(VkImage img, u32 aspect, u32 layer, u32 level, VkImageLayout layout, VkImageLayout expectedLayout) extern void vkErrInvalidImageSubresource(VkImage img, string subresourceType, u32 value) diff --git a/gapis/api/vulkan/externs.go b/gapis/api/vulkan/externs.go index b6621a39a2..b34dd10a2c 100644 --- a/gapis/api/vulkan/externs.go +++ b/gapis/api/vulkan/externs.go @@ -482,6 +482,14 @@ func (e externs) vkErrInvalidImageSubresource(img VkImage, subresourceType strin e.onVkError(issue) } +func (e externs) vkErrInvalidDescriptorBindingType(set VkDescriptorSet, binding uint32, layoutType, updateType VkDescriptorType) { + var issue replay.Issue + issue.Command = e.cmdID + issue.Severity = service.Severity_ErrorLevel + issue.Error = fmt.Errorf("Updating descriptor binding at: %v: %d with type: %v, but the type defined in descriptor set layout is: %v", set, binding, layoutType, updateType) + e.onVkError(issue) +} + type fenceSignal uint64 func (e externs) recordFenceSignal(fence VkFence) {