Merge pull request #2852 from erikaharrison-adsk/feature-hgi-vulkan-general-stability

Autodesk: General HgiVulkan stability

(Internal change: 2313559)

diff --cc pxr/imaging/hdSt/renderBuffer.cpp
index a7c70dd2bbe9d071ab6a839c950535308ebb3c38,1e6490200c3e69309ed279e2432e83c6ca86dc47..3e0d496bc9a03c669b2366879a163daec3cf8719
--- 1/pxr/imaging/hdSt/renderBuffer.cpp
--- 2/pxr/imaging/hdSt/renderBuffer.cpp
+++ b/pxr/imaging/hdSt/renderBuffer.cpp
@@@ -44,7 -44,11 +44,10 @@@ HgiTextureUsage _GetTextureUsage(HdForm
                 HgiTextureUsageBitsStencilTarget;
      }
  
-     return HgiTextureUsageBitsColorTarget;
 -    // We are assuming at some point in a render buffer's life time it
 -    // could be used to read from. So just providing that ability to
 -    // the render buffer. This is specially useful when using hgiVulkan
 -    // back-end
++    // We are assuming at some point in a render buffer's lifetime it could be
++    // used to read from, so provide that ability to the render buffer. This is 
++    // especially useful when for the HgiVulkan back-end.
+     return HgiTextureUsageBitsColorTarget | HgiTextureUsageBitsShaderRead;
  }
  
  HdStRenderBuffer::HdStRenderBuffer(

diff --cc pxr/imaging/hdx/colorCorrectionTask.cpp
index af50746d73e107a2129d2a480e4eee3a8f20ef07,4e94fffb05f6da780770fa227c3fb04d4634e289..9940047f87e2c2175a45a04da184b8148b85e6eb
--- 1/pxr/imaging/hdx/colorCorrectionTask.cpp
--- 2/pxr/imaging/hdx/colorCorrectionTask.cpp
+++ b/pxr/imaging/hdx/colorCorrectionTask.cpp
@@@ -1146,6 -1139,15 +1146,15 @@@ HdxColorCorrectionTask::Execute(HdTaskC
      _GetTaskContextData(
          ctx, HdxAovTokens->colorIntermediate, &aovTextureIntermediate);
  
 -    // since we are going to be sampling from this buffer and writing to a color
 -    // corrected - intermediate buffer.
 -    // However, the intermediate color corrected buffer is in the correct layout
+     // We need to ensure the incoming color buffer is set to the correct layout
+     // ie: Shader Read Only Optimal
++    // since we are going to be sampling from this buffer and writing to a 
++    // color-corrected intermediate buffer.
++    // However, the intermediate color-corrected buffer is in the correct layout
+     // ie: Color Attachment Optimal.
+     // So, no layout transition there.
+     aovTexture->SubmitLayoutChange(HgiTextureUsageBitsShaderRead);
+ 
      if (!TF_VERIFY(_CreateBufferResources())) {
          return;
      }
@@@ -1164,6 -1166,12 +1173,12 @@@
  
      _ApplyColorCorrection(aovTextureIntermediate);
  
 -    // Before we ping-pong the buffers, we are going to ensure -
 -    // The color buffer is converted to a Color Attachment Optimal layout.
++    // Before we ping-pong the buffers, we are going to ensure the color buffer 
++    // is converted to a Color Attachment Optimal layout.
+     // Hence, preserving the state atomicity of this pass.
 -    // Otherwise, its business as usual.
++    // Otherwise, it's business as usual.
+     aovTexture->SubmitLayoutChange(HgiTextureUsageBitsColorTarget);
+ 
      // Toggle color and colorIntermediate
      _ToggleRenderTarget(ctx);
  }

diff --cc pxr/imaging/hgi/texture.h
index 8b214520fdc7667e6929a37645169efd09efada0,8d0aacde33e76f991d0556a1553c1959faad75e4..883b16b18a5105f6f5fbd8c4f882231a46940c51
--- 1/pxr/imaging/hgi/texture.h
--- 2/pxr/imaging/hgi/texture.h
+++ b/pxr/imaging/hgi/texture.h
@@@ -186,6 -186,13 +186,12 @@@ public
      HGI_API
      virtual uint64_t GetRawResource() const = 0;
  
 -    /// This function initiates a layout change process on this texture resource.
 -    /// This feature is at the moment required explicitly by explicit APIs like
 -    /// Vulkan
++    /// This function initiates a layout change process on this texture 
++    /// resource. This feature is at the moment required explicitly by explicit 
++    /// APIs like Vulkan.
+     HGI_API
 -    virtual
 -    void SubmitLayoutChange(HgiTextureUsage newLayout) = 0;
++    virtual void SubmitLayoutChange(HgiTextureUsage newLayout) = 0;
+ 
  protected:
      HGI_API
      static

diff --cc pxr/imaging/hgiGL/texture.cpp
index 40f8c217eb81cde65c528c61aa1efef37449d6ed,547a3484af4751b324458cc050f67e3ab4720014..297a7e5e3eb8acb365c997481a5ca0b411709455
--- 1/pxr/imaging/hgiGL/texture.cpp
--- 2/pxr/imaging/hgiGL/texture.cpp
+++ b/pxr/imaging/hgiGL/texture.cpp
@@@ -436,5 -436,11 +436,10 @@@ HgiGLTexture::GetBindlessHandle(
      return _bindlessHandle;
  }
  
+ void 
+ HgiGLTexture::SubmitLayoutChange(HgiTextureUsage newLayout)
+ {
+     return;
+ }
  
 -
  PXR_NAMESPACE_CLOSE_SCOPE

diff --cc pxr/imaging/hgiGL/texture.h
index a8655c22cdb06ff2936158d47785a521bc3e1064,c841196a4effee1b84a545052809401cf34c7269..cdf36f0797896296f931f1b6d4a83665177f3306
--- 1/pxr/imaging/hgiGL/texture.h
--- 2/pxr/imaging/hgiGL/texture.h
+++ b/pxr/imaging/hgiGL/texture.h
@@@ -63,6 -63,13 +63,12 @@@ public
      HGIGL_API
      uint64_t GetBindlessHandle();
  
 -    /// layout transition in non-explicit APIs like OpenGL. Hence
 -    /// this function simply returns void
+     /// This function does not do anything. There is no support for explicit 
 -    virtual
 -    void SubmitLayoutChange(HgiTextureUsage newLayout);
++    /// layout transition in non-explicit APIs like OpenGL. Hence this function
++    /// simply returns void.
+     HGIGL_API
++    void SubmitLayoutChange(HgiTextureUsage newLayout) override;
+ 
  protected:
      friend class HgiGL;
  

diff --cc pxr/imaging/hgiMetal/texture.h
index f56a87fce6ad99d0436dc1b40d877cdbec854e3c,038cc0657197910805c9f07fd878302c2ab82858..623fdf3cf1384f6d9f1fc14af77a49125b2ae749
--- 1/pxr/imaging/hgiMetal/texture.h
--- 2/pxr/imaging/hgiMetal/texture.h
+++ b/pxr/imaging/hgiMetal/texture.h
@@@ -55,6 -55,11 +55,12 @@@ public
      /// Returns the handle to the Metal texture.
      HGIMETAL_API
      id<MTLTexture> GetTextureId() const;
 -    /// At the moment there is no need for explicit layout transition
 -    /// for this render API backend. Hence this funciton returns void. 
+     
 -    virtual void SubmitLayoutChange(HgiTextureUsage newLayout);
++    /// This function does not do anything. At the moment there is no need for 
++    /// explicit layout transitions for the Metal backend. Hence this function 
++    /// simply returns void. 
+     HGIMETAL_API
++    void SubmitLayoutChange(HgiTextureUsage newLayout) override;
  
  protected:
      friend class HgiMetal;

diff --cc pxr/imaging/hgiMetal/texture.mm
Simple merge

diff --cc pxr/imaging/hgiVulkan/texture.cpp
index b08ec88ba13af61d815b879de12eb5727d6f905e,c806bf848abf136c70053c91779ffc118d04dc6e..bba17f87e1f5773e76a190e2931a6833d474a68d
--- 1/pxr/imaging/hgiVulkan/texture.cpp
--- 2/pxr/imaging/hgiVulkan/texture.cpp
+++ b/pxr/imaging/hgiVulkan/texture.cpp
@@@ -101,10 -101,18 +101,6 @@@ HgiVulkanTexture::HgiVulkanTexture
  
      // XXX STORAGE_IMAGE requires VK_IMAGE_USAGE_STORAGE_BIT, but Hgi
      // doesn't tell us if a texture will be used as image load/store.
--    if ((desc.usage & HgiTextureUsageBitsShaderRead) ||
-             (desc.usage & HgiTextureUsageBitsShaderWrite)) {
-         imageCreateInfo.usage |= VK_IMAGE_USAGE_STORAGE_BIT;
 -            (desc.usage & HgiTextureUsageBitsShaderWrite)){
 -        // Vulkan does not allow image views of this format to be of type storage.
 -        // They are used for sampling or presentation
 -        if (imageCreateInfo.format == VK_FORMAT_R8G8B8A8_SRGB){
 -            imageCreateInfo.usage |= VK_IMAGE_USAGE_SAMPLED_BIT;
 -        }
 -        else{ 
 -            imageCreateInfo.usage |= VK_IMAGE_USAGE_STORAGE_BIT;
--        }
 -        
 -    }
  
      VkFormatFeatureFlags formatValidationFlags =
          HgiVulkanConversions::GetFormatFeature(desc.usage);
@@@ -514,6 -522,80 +510,77 @@@ HgiVulkanTexture::CopyBufferToTexture
          VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT); // Consumer stage
  }
  
 -    VkAccessFlags srcAccessMask, dstAccessMask;
+ void 
+ HgiVulkanTexture::SubmitLayoutChange(HgiTextureUsage newLayout)
+ {
+     VkImageLayout newVkLayout = 
+         HgiVulkanTexture::GetDefaultImageLayout(newLayout);
+ 
+     HgiVulkanCommandQueue* queue = _device->GetCommandQueue();
+     HgiVulkanCommandBuffer* cb = queue->AcquireResourceCommandBuffer();
+ 
 -        dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT;
++    VkAccessFlags srcAccessMask, dstAccessMask = VK_ACCESS_NONE;
+ 
+     // The following cases are based on few initial assumptions to provide
+     // an infrastructure for access mask selection based on layouts.
+     // Feel free to update depending on need and use cases.
+     switch (GetImageLayout()) {
+     case VK_IMAGE_LAYOUT_PREINITIALIZED:
+         srcAccessMask =
+             VK_ACCESS_HOST_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT;
+         break;
+     case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL:
+         srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT;
+         break;
+     case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL:
+         srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
+         break;
+     case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL:
+         srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT;
+         break;
+     case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL:
+         srcAccessMask = VK_ACCESS_SHADER_READ_BIT;
+         break;
+     default:
+         srcAccessMask = VK_ACCESS_NONE;
+         break;
+     }
+ 
+     switch (newVkLayout) {
+     case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL:
+         dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT;
+         break;
+     case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL:
+         srcAccessMask |= VK_ACCESS_TRANSFER_READ_BIT;
+         dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT;
+         break;
+     case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL:
 -        dstAccessMask |=
 -            VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
+         srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT;
++        dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT;
+         break;
+     case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL:
 -        srcAccessMask =
 -            VK_ACCESS_SHADER_READ_BIT;
++        dstAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT;
+         break;
+     case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL:
 -        srcAccessMask = VK_ACCESS_NONE;
++        srcAccessMask = VK_ACCESS_SHADER_READ_BIT;
+         dstAccessMask = VK_ACCESS_SHADER_READ_BIT;
+         break;
+     default:
 -        newVkLayout,                            // Transition tex to this layout
 -        srcAccessMask,                          // No pending writes
 -        dstAccessMask,                          // Write access to image
 -        VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT,     // Producer stage
 -        VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT);    // Consumer stage
+         dstAccessMask = VK_ACCESS_NONE;
+         break;
+     }
+ 
+     TransitionImageBarrier(
+         cb,
+         this,
+         GetImageLayout(),
++        newVkLayout,
++        srcAccessMask, 
++        dstAccessMask,
++        VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT,
++        VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT);
+ }
+ 
  void
  HgiVulkanTexture::TransitionImageBarrier(
      HgiVulkanCommandBuffer* cb,
@@@ -570,7 -652,13 +637,13 @@@ HgiVulkanTexture::GetDefaultImageLayout
          // Assume the ShaderWrite means its a storage image.
          return VK_IMAGE_LAYOUT_GENERAL;
      } else if (usage & HgiTextureUsageBitsShaderRead) {
-         return VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
 -        // Also check if image is going to be used as a color 
 -        // attachment as well. if yes, then explicitly give 
 -        // it a color attachment layout
 -        if(usage & HgiTextureUsageBitsColorTarget)
++        // Also check if image is going to be used as a color attachment as 
++        // well. If yes, then explicitly give it a color attachment layout.
++        if (usage & HgiTextureUsageBitsColorTarget) {
+             return VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL;
 -        else
++        } else {
+             return VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
++        }
      } else if (usage & HgiTextureUsageBitsDepthTarget) {
          return VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL;
      } else if (usage & HgiTextureUsageBitsColorTarget) {

diff --cc pxr/imaging/hgiVulkan/texture.h
index 5a41206381dbf68857f1c1d9ee1bbfb5ef9b7150,3f98dc306dc90acc3ad3e7453b2a8d3845306fc8..587b7669d5f138258c775e0fbed6ce3919d8f530
--- 1/pxr/imaging/hgiVulkan/texture.h
--- 2/pxr/imaging/hgiVulkan/texture.h
+++ b/pxr/imaging/hgiVulkan/texture.h
@@@ -100,6 -100,13 +100,12 @@@ public
          GfVec3i const& dstTexelOffset = GfVec3i(0),
          int mipLevel=-1);
  
 -    /// This function issues a layout change barrier. However the layout transition 
 -    /// isn't immediately executed. The command buffer simply records the request
 -    /// and executes when in the next submission cycle
++    /// This function issues a layout change barrier. However, the layout 
++    /// transition isn't immediately executed. The command buffer simply 
++    /// records the request and executes when in the next submission cycle.
+     HGIVULKAN_API
 -    virtual
 -    void SubmitLayoutChange(HgiTextureUsage newLayout);
++    void SubmitLayoutChange(HgiTextureUsage newLayout) override;
+ 
      /// Transition image from oldLayout to newLayout.
      /// `producerAccess` of 0 means:
      ///    Only invalidation barrier, no flush barrier. For read-only resources.