Added comments from Vulkan spec

7 files changed, 49 insertions, 3 deletions

diff --git a/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h b/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
index 8caa4213..d89d6e71 100644
--- a/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
+++ b/Graphics/GraphicsEngineVulkan/include/RenderDeviceVkImpl.h
@@ -116,6 +116,8 @@ public:
     }
     DescriptorPoolAllocation AllocateDynamicDescriptorSet(VkDescriptorSetLayout SetLayout, Uint32 CtxId)
     {
+        // Descriptor pools are externally synchronized, meaning that the application must not allocate 
+        // and/or free descriptor sets from the same pool in multiple threads simultaneously (13.2.3)
         return m_DescriptorPools[1 + CtxId].Allocate(SetLayout);
     }
 
diff --git a/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp b/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp
index e9b4dd67..bca37fff 100644
--- a/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/DescriptorPoolManager.cpp
@@ -58,6 +58,8 @@ void DescriptorPoolManager::CreateNewPool()
 
 DescriptorPoolAllocation DescriptorPoolManager::Allocate(VkDescriptorSetLayout SetLayout)
 {
+    // Descriptor pools are externally synchronized, meaning that the application must not allocate 
+    // and/or free descriptor sets from the same pool in multiple threads simultaneously (13.2.3)
     std::unique_lock<std::mutex> Lock(m_Mutex, std::defer_lock);
     if (m_IsThreadSafe)
         Lock.lock();
diff --git a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
index f3932b09..c5bac746 100644
--- a/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/PipelineLayout.cpp
@@ -285,6 +285,10 @@ PipelineLayout::DescriptorSetLayoutManager::~DescriptorSetLayoutManager()
 
 bool PipelineLayout::DescriptorSetLayoutManager::operator == (const DescriptorSetLayoutManager& rhs)const
 {
+    // Two pipeline layouts are defined to be “compatible for set N” if they were created with identically 
+    // defined descriptor set layouts for sets zero through N, and if they were created with identical push 
+    // constant ranges (13.2.2)
+
     if(m_ActiveSets != rhs.m_ActiveSets)
         return false;
 
@@ -323,6 +327,7 @@ void PipelineLayout::DescriptorSetLayoutManager::AllocateResourceSlot(const SPIR
     VkBinding.binding = Binding;
     VkBinding.descriptorType = GetVkDescriptorType(ResAttribs);
     VkBinding.descriptorCount = ResAttribs.ArraySize;
+    // There are no limitations on what combinations of stages can use a descriptor binding (13.2.1)
     VkBinding.stageFlags = ShaderTypeToVkShaderStageFlagBit(ShaderType);
     if (ResAttribs.StaticSamplerInd >= 0)
     {
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp
index 5e795134..b5bb57b6 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceCacheVk.cpp
@@ -140,6 +140,12 @@ void ShaderResourceCacheVk::TransitionResources(DeviceContextVkImpl *pCtxVkImpl)
             {
                 auto *pTextureViewVk = Res.pObject.RawPtr<TextureViewVkImpl>();
                 auto *pTextureVk = ValidatedCast<TextureVkImpl>(pTextureViewVk->GetTexture());
+
+                // The image subresources for a storage image must be in the VK_IMAGE_LAYOUT_GENERAL layout in 
+                // order to access its data in a shader (13.1.1)
+                // The image subresources for a sampled image or a combined image sampler must be in the 
+                // VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, 
+                // or VK_IMAGE_LAYOUT_GENERAL layout in order to access its data in a shader (13.1.3, 13.1.4).
                 VkImageLayout RequiredLayout = 
                     Res.Type == SPIRVShaderResourceAttribs::ResourceType::StorageImage ? 
                         VK_IMAGE_LAYOUT_GENERAL : VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
@@ -182,8 +188,19 @@ VkDescriptorBufferInfo ShaderResourceCacheVk::Resource::GetBufferDescriptorWrite
 
     auto* pBuffVk = pObject.RawPtr<const BufferVkImpl>();
 
+    // The buffer must be created with the following flags so that it can be bound to the specified descriptor (13.2.4):
+    //  * VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER or VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC -> VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT
+    //  * VK_DESCRIPTOR_TYPE_STORAGE_BUFFER or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC -> VK_BUFFER_USAGE_STORAGE_BUFFER_BIT
+    //  * VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER -> VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT
+    //  * VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER -> VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT
+
     VkDescriptorBufferInfo DescrBuffInfo;
     DescrBuffInfo.buffer = pBuffVk->GetVkBuffer();
+    // If descriptorType is VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER or VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, the offset 
+    // member of each element of pBufferInfo must be a multiple of VkPhysicalDeviceLimits::minUniformBufferOffsetAlignment
+    // If descriptorType is VK_DESCRIPTOR_TYPE_STORAGE_BUFFER or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC, the offset 
+    // member of each element of pBufferInfo must be a multiple of VkPhysicalDeviceLimits::minStorageBufferOffsetAlignment
+    // (13.2.4)
     DescrBuffInfo.offset = 0;
     DescrBuffInfo.range = pBuffVk->GetDesc().uiSizeInBytes;
     return DescrBuffInfo;
@@ -202,9 +219,15 @@ VkDescriptorImageInfo ShaderResourceCacheVk::Resource::GetImageDescriptorWriteIn
     DescrImgInfo.sampler = VK_NULL_HANDLE;
     if (Type == SPIRVShaderResourceAttribs::ResourceType::SampledImage && !IsImmutableSampler)
     {
+        // Immutable samplers are permanently bound into the set layout; later binding a sampler 
+        // into an immutable sampler slot in a descriptor set is not allowed (13.2.1)
         auto *pSamplerVk = ValidatedCast<const SamplerVkImpl>(pTexViewVk->GetSampler());
         if (pSamplerVk != nullptr)
         {
+            // If descriptorType is VK_DESCRIPTOR_TYPE_SAMPLER or VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 
+            // and dstSet was not allocated with a layout that included immutable samplers for dstBinding with 
+            // descriptorType, the sampler member of each element of pImageInfo must be a valid VkSampler 
+            // object (13.2.4)
             DescrImgInfo.sampler = pSamplerVk->GetVkSampler();
         }
         else
@@ -216,7 +239,7 @@ VkDescriptorImageInfo ShaderResourceCacheVk::Resource::GetImageDescriptorWriteIn
     
     // If descriptorType is VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, for each descriptor that will be accessed 
     // via load or store operations the imageLayout member for corresponding elements of pImageInfo 
-    // must be VK_IMAGE_LAYOUT_GENERAL (13.2.4)
+    // MUST be VK_IMAGE_LAYOUT_GENERAL (13.2.4)
     DescrImgInfo.imageLayout = 
         (Type == SPIRVShaderResourceAttribs::ResourceType::StorageImage) ? 
             VK_IMAGE_LAYOUT_GENERAL : 
@@ -241,6 +264,7 @@ VkDescriptorImageInfo ShaderResourceCacheVk::Resource::GetSamplerDescriptorWrite
 
     auto* pSamplerVk = pObject.RawPtr<const SamplerVkImpl>();
     VkDescriptorImageInfo DescrImgInfo;
+    // For VK_DESCRIPTOR_TYPE_SAMPLER, only the sample member of each element of VkWriteDescriptorSet::pImageInfo is accessed (13.2.4)
     DescrImgInfo.sampler = pSamplerVk->GetVkSampler();
     DescrImgInfo.imageView = VK_NULL_HANDLE;
     DescrImgInfo.imageLayout = VK_IMAGE_LAYOUT_UNDEFINED;
diff --git a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
index 980ef06e..4be1025d 100644
--- a/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/ShaderResourceLayoutVk.cpp
@@ -255,6 +255,8 @@ void ShaderResourceLayoutVk::VkResource::UpdateDescriptorHandle(VkDescriptorSet
     WriteDescrSet.dstBinding = Binding;
     WriteDescrSet.dstArrayElement = ArrayElement;
     WriteDescrSet.descriptorCount = 1;
+    // descriptorType must be the same type as that specified in VkDescriptorSetLayoutBinding for dstSet at dstBinding. 
+    // The type of the descriptor also controls which array the descriptors are taken from. (13.2.4)
     WriteDescrSet.descriptorType = PipelineLayout::GetVkDescriptorType(SpirvAttribs);
     WriteDescrSet.pImageInfo = pImageInfo;
     WriteDescrSet.pBufferInfo = pBufferInfo;
@@ -309,6 +311,11 @@ void ShaderResourceLayoutVk::VkResource::CacheBuffer(IDeviceObject*
     if( UpdateCachedResource(DstRes, ArrayInd, pBuffer, IID_BufferVk, "buffer") )
     {
 #ifdef VERIFY_SHADER_BINDINGS
+        // The buffer must be created with the following flags so that it can be bound to the specified descriptor (13.2.4):
+        //  * VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER or VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC -> VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT
+        //  * VK_DESCRIPTOR_TYPE_STORAGE_BUFFER or VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC -> VK_BUFFER_USAGE_STORAGE_BUFFER_BIT
+        //  * VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER -> VK_BUFFER_USAGE_UNIFORM_TEXEL_BUFFER_BIT
+        //  * VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER -> VK_BUFFER_USAGE_STORAGE_TEXEL_BUFFER_BIT
         auto* pBuffVk = DstRes.pObject.RawPtr<BufferVkImpl>(); // Use final type
         const auto IsUniformBuffer = SpirvAttribs.Type == SPIRVShaderResourceAttribs::ResourceType::UniformBuffer;
         const auto ExpectedBindFlag = IsUniformBuffer ? BIND_UNIFORM_BUFFER : BIND_UNORDERED_ACCESS;
@@ -487,6 +494,8 @@ void ShaderResourceLayoutVk::VkResource::BindResource(IDeviceObject *pObj, Uint3
                 }
                 else
                 {
+                    // Immutable samplers are permanently bound into the set layout; later binding a sampler 
+                    // into an immutable sampler slot in a descriptor set is not allowed (13.2.1)
                     LOG_ERROR_MESSAGE("Attempting to assign a sampler to a static sampler '", SpirvAttribs.Name, '\'');
                 }
             break;
@@ -727,6 +736,8 @@ void ShaderResourceLayoutVk::CommitDynamicResources(const ShaderResourceCacheVk&
                 {
                     if(Res.SpirvAttribs.StaticSamplerInd < 0)
                     {
+                        // Immutable samplers are permanently bound into the set layout; later binding a sampler 
+                        // into an immutable sampler slot in a descriptor set is not allowed (13.2.1)
                         const auto& CachedRes = SetResources.GetResource(Res.CacheOffset + i);
                         VkDescriptorImageInfo DescrImgInfo = CachedRes.GetSamplerDescriptorWriteInfo();
                         Res.UpdateDescriptorHandle(vkSet, i, &DescrImgInfo, nullptr, nullptr);
diff --git a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanCommandBuffer.cpp b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanCommandBuffer.cpp
index f63bd7d6..9969a009 100644
--- a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanCommandBuffer.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanCommandBuffer.cpp
@@ -175,7 +175,7 @@ void VulkanCommandBuffer::TransitionImageLayout(VkCommandBuffer CmdBuffer,
 
         // supports all types of device access
         case VK_IMAGE_LAYOUT_GENERAL:
-            // VK_IMAGE_LAYOUT_GENERAL must be used for image load/store operations (13.2.4)
+            // VK_IMAGE_LAYOUT_GENERAL must be used for image load/store operations (13.1.1, 13.2.4)
             ImgBarrier.srcAccessMask = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT;
         break;
 
@@ -244,7 +244,7 @@ void VulkanCommandBuffer::TransitionImageLayout(VkCommandBuffer CmdBuffer,
         break;
 
         case VK_IMAGE_LAYOUT_GENERAL:
-            // VK_IMAGE_LAYOUT_GENERAL must be used for image load/store operations (13.2.4)
+            // VK_IMAGE_LAYOUT_GENERAL must be used for image load/store operations (13.1.1, 13.2.4)
             ImgBarrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT;
         break;
 
diff --git a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanDescriptorPool.cpp b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanDescriptorPool.cpp
index 31834c47..c31bd96d 100644
--- a/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanDescriptorPool.cpp
+++ b/Graphics/GraphicsEngineVulkan/src/VulkanUtilities/VulkanDescriptorPool.cpp
@@ -52,6 +52,8 @@ namespace VulkanUtilities
         DescrSetAllocInfo.descriptorPool = m_Pool;
         DescrSetAllocInfo.descriptorSetCount = 1;
         DescrSetAllocInfo.pSetLayouts = &SetLayout;
+        // Descriptor pools are externally synchronized, meaning that the application must not allocate 
+        // and/or free descriptor sets from the same pool in multiple threads simultaneously (13.2.3)
         return m_LogicalDevice->AllocateVkDescriptorSet(DescrSetAllocInfo, DebugName);
     }