Work on Vulkan framebuffer attachment layouts

overte-org · Jan 1, 2025 · 0cde5d9 · 0cde5d9
1 parent 7b103e0
commit 0cde5d9
Show file tree

Hide file tree

Showing 9 changed files with 237 additions and 93 deletions.
diff --git a/libraries/gpu-vk/src/gpu/vk/VKBackend.cpp b/libraries/gpu-vk/src/gpu/vk/VKBackend.cpp
diff --git a/libraries/gpu-vk/src/gpu/vk/VKBackend.h b/libraries/gpu-vk/src/gpu/vk/VKBackend.h
@@ -48,6 +48,8 @@
 
 namespace gpu { namespace vk {
 
+class VKAttachmentTexture;
+
 static const int MAX_NUM_UNIFORM_BUFFERS = 14; // There's also camera buffer at slot 15
 
 static const int32_t MIN_REQUIRED_TEXTURE_IMAGE_UNITS = 16;
@@ -310,7 +312,7 @@ class VKBackend : public Backend, public std::enable_shared_from_this<VKBackend>
     VKTexture* syncGPUObject(const Texture& texture);
     VKQuery* syncGPUObject(const Query& query);
 
-    void blitToFramebuffer(gpu::Texture &input, gpu::Texture &output);
+    void blitToFramebuffer(VKAttachmentTexture &input, const Vec4i& srcViewport, VKAttachmentTexture &output, const Vec4i& dstViewport);
 
 public:
     VKBackend();
@@ -446,6 +448,7 @@ class VKBackend : public Backend, public std::enable_shared_from_this<VKBackend>
     // VKTODO: quick hack
     VKFramebuffer *_outputTexture{ nullptr };
 protected:
+    void transitionImageLayouts();
 
     // These are filled by syncGPUObject() calls, and are needed to track backend objects so that they can be destroyed before
     // destroying backend.
@@ -483,6 +486,10 @@ class VKBackend : public Backend, public std::enable_shared_from_this<VKBackend>
     std::shared_ptr<FrameData> _currentFrame;
     // Frame for which command buffer is already generated and it's currently being rendered.
     std::shared_ptr<FrameData> _currentlyRenderedFrame;
+
+    std::vector<VKAttachmentTexture*> _attachmentTexturesToTransitionToRead;
+    std::vector<VKAttachmentTexture*> _attachmentTexturesToTransitionToWrite;
+
     // Safety check to ensure that shutdown was completed before destruction.
     std::atomic<bool> isBackendShutdownComplete{ false };
 

diff --git a/libraries/gpu-vk/src/gpu/vk/VKFramebuffer.cpp b/libraries/gpu-vk/src/gpu/vk/VKFramebuffer.cpp
@@ -14,6 +14,7 @@
 void gpu::vk::VKFramebuffer::update() {
     auto backend = _backend.lock();
     VkDevice device = backend->getContext().device->logicalDevice;
+    // VKTODO: this is wrong, most of framebuffer code will need to be rewritten
     if (vkFramebuffer != VK_NULL_HANDLE) {
         vkDestroyFramebuffer(device, vkFramebuffer, nullptr);
     }
@@ -63,7 +64,7 @@ void gpu::vk::VKFramebuffer::update() {
                         attachmentCI.height = vkTexture->_gpuObject.getHeight();
                         attachmentCI.layerCount = 1;
                         attachmentCI.format = gpu::vk::evalTexelFormatInternal(vkTexture->_gpuObject.getTexelFormat());
-                        attachmentCI.usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT;
+                        attachmentCI.usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT;
                         attachmentCI.imageSampleCount = VK_SAMPLE_COUNT_1_BIT;
                         addAttachment(attachmentCI, vkTexture->_vkImage);
                         //glNamedFramebufferTexture(_id, colorAttachments[unit], gltexture->_texture, 0);
@@ -258,7 +259,7 @@ VkResult gpu::vk::VKFramebuffer::createFramebuffer()
     return VK_SUCCESS;
 }
 
-bool gpu::vk::VKFramebuffer::checkStatus(gpu::vk::VKFramebuffer::FramebufferStatus target) const {
+//bool gpu::vk::VKFramebuffer::checkStatus(gpu::vk::VKFramebuffer::FramebufferStatus target) const {
     // VKTODO
     /*switch (_status) {
         case GL_FRAMEBUFFER_COMPLETE:
@@ -287,7 +288,7 @@ bool gpu::vk::VKFramebuffer::checkStatus(gpu::vk::VKFramebuffer::FramebufferStat
     }
     return false;
 */
-}
+//}
 
 gpu::vk::VKFramebuffer::~VKFramebuffer() {
     auto backend = _backend.lock();
@@ -334,29 +335,9 @@ uint32_t gpu::vk::VKFramebuffer::addAttachment(VKAttachmentCreateInfo createinfo
 
     assert(aspectMask > 0);
 
-    /*VkImageCreateInfo image = vks::initializers::imageCreateInfo();
-    image.imageType = VK_IMAGE_TYPE_2D;
-    image.format = createinfo.format;
-    image.extent.width = createinfo.width;
-    image.extent.height = createinfo.height;
-    image.extent.depth = 1;
-    image.mipLevels = 1;
-    image.arrayLayers = createinfo.layerCount;
-    image.samples = createinfo.imageSampleCount;
-    image.tiling = VK_IMAGE_TILING_OPTIMAL;
-    image.usage = createinfo.usage;*/
-
     VkMemoryAllocateInfo memAlloc = vks::initializers::memoryAllocateInfo();
     VkMemoryRequirements memReqs;
 
-    // Create image for this attachment
-    /*VK_CHECK_RESULT(vkCreateImage(vulkanDevice->logicalDevice, &image, nullptr, &attachment.image));
-    vkGetImageMemoryRequirements(vulkanDevice->logicalDevice, attachment.image, &memReqs);
-    memAlloc.allocationSize = memReqs.size;
-    memAlloc.memoryTypeIndex = vulkanDevice->getMemoryType(memReqs.memoryTypeBits, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
-    // VKTODO: this may need to be changed to VMA
-    VK_CHECK_RESULT(vkAllocateMemory(vulkanDevice->logicalDevice, &memAlloc, nullptr, &attachment.memory));
-    VK_CHECK_RESULT(vkBindImageMemory(vulkanDevice->logicalDevice, attachment.image, attachment.memory, 0));*/
     attachment.image = image;
 
     attachment.subresourceRange = {};
@@ -385,11 +366,13 @@ uint32_t gpu::vk::VKFramebuffer::addAttachment(VKAttachmentCreateInfo createinfo
     // If not, final layout depends on attachment type
     if (attachment.hasDepth() || attachment.hasStencil())
     {
-        attachment.description.finalLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL;
+        //attachment.description.finalLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL;
+        attachment.description.finalLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; // VKTODO: this is tricky, because it depends on what the image will be used for
     }
     else
     {
-        attachment.description.finalLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
+        //attachment.description.finalLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
+        attachment.description.finalLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; // VKTODO: this is tricky, because it depends on what the image will be used for
     }
 
     attachments.push_back(attachment);

diff --git a/libraries/gpu-vk/src/gpu/vk/VKFramebuffer.h b/libraries/gpu-vk/src/gpu/vk/VKFramebuffer.h
@@ -102,12 +102,15 @@ class VKFramebuffer : public vk::VKObject<Framebuffer> {
 
     };
 
+    // VKTODO: this can be removed in the future, it's redundant
     std::vector<FramebufferAttachment> attachments;
 
 protected:
-    enum FramebufferStatus { VK_FRAMEBUFFER_COMPLETE } _status;
+    //VkImageLayout _currentLayout {VK_IMAGE_LAYOUT_UNDEFINED}; // Used by render passes. If it's VK_IMAGE_LAYOUT_UNDEFINED, then image will be cleared in the render pass
+    //enum FramebufferStatus { VK_FRAMEBUFFER_COMPLETE } _status;
+
     virtual void update();
-    bool checkStatus(FramebufferStatus target) const;
+    //bool checkStatus(FramebufferStatus target) const;
     VkResult createFramebuffer();
     struct VKAttachmentCreateInfo
     {

diff --git a/libraries/gpu-vk/src/gpu/vk/VKPipelineCache.cpp b/libraries/gpu-vk/src/gpu/vk/VKPipelineCache.cpp
@@ -100,7 +100,7 @@ Cache::Pipeline::PipelineLayout Cache::Pipeline::getPipelineAndDescriptorLayout(
 
         // Create the descriptor set layouts
         std::vector<VkDescriptorSetLayout> layouts;
-        if (!uniLayout.empty()) {
+        if (!uniLayout.empty() || !texLayout.empty() || !stoLayout.empty()) {
             VkDescriptorSetLayoutCreateInfo descriptorSetLayoutCI =
                 vks::initializers::descriptorSetLayoutCreateInfo(uniLayout.data(), uniLayout.size());
             VkDescriptorSetLayout descriptorSetLayout;
@@ -110,7 +110,8 @@ Cache::Pipeline::PipelineLayout Cache::Pipeline::getPipelineAndDescriptorLayout(
             layout.uniformLayout = descriptorSetLayout;
         }
 #if SEP_DESC
-        if (!texLayout.empty()) {
+        // Descriptor set needs to be created even if it's empty if later descriptor sets are not empty.
+        if (!texLayout.empty() || !stoLayout.empty()) {
             VkDescriptorSetLayoutCreateInfo descriptorSetLayoutCI =
                 vks::initializers::descriptorSetLayoutCreateInfo(texLayout.data(), texLayout.size());
             VkDescriptorSetLayout descriptorSetLayout;

diff --git a/libraries/gpu-vk/src/gpu/vk/VKTexture.cpp b/libraries/gpu-vk/src/gpu/vk/VKTexture.cpp
@@ -204,15 +204,13 @@ void VKAttachmentTexture::createTexture(VKBackend &backend) {
         || _gpuObject.getTexelFormat().getSemantic() == gpu::R11G11B10
         || _gpuObject.getTexelFormat().getSemantic() == gpu::SRGB
         || _gpuObject.getTexelFormat().getSemantic() == gpu::SRGBA) {
-        imageCI.usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT;
+        imageCI.usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT | VK_IMAGE_USAGE_TRANSFER_SRC_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT;
     } else if (_gpuObject.isDepthStencilRenderTarget()) {
         imageCI.usage = VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT;
     } else {
         Q_ASSERT(false);
     }
 
-    auto device = _backend.lock()->getContext().device->logicalDevice;
-
     // Create image for this attachment
     /*VK_CHECK_RESULT(vkCreateImage(device, &imageCI, nullptr, &_texture));
     VkMemoryAllocateInfo memAlloc = vks::initializers::memoryAllocateInfo();
@@ -292,7 +290,7 @@ VkDescriptorImageInfo VKAttachmentTexture::getDescriptorImageInfo() {
 
     VkDescriptorImageInfo result {};
     result.sampler = _vkSampler;
-    result.imageLayout = _vkImageLayout;
+    result.imageLayout = _vkImageLayout;  // VKTODO: this needs to be updated on blits and other image writes
     result.imageView = _vkImageView;
     return result;
 };

diff --git a/libraries/gpu-vk/src/gpu/vk/VKTexture.h b/libraries/gpu-vk/src/gpu/vk/VKTexture.h
@@ -280,7 +280,7 @@ class VKAttachmentTexture : public VKFixedAllocationTexture {
     VkDescriptorImageInfo getDescriptorImageInfo() override; // VKTODO
 
     VkImageView _vkImageView { VK_NULL_HANDLE };
-    VkImageLayout _vkImageLayout {}; // VKTODO
+    VkImageLayout _vkImageLayout {}; // VKTODO: this needs to be updated on blits and other image writes
     VkSampler _vkSampler { VK_NULL_HANDLE };
     //VkImage _vkImage { VK_NULL_HANDLE };
     //VkDeviceMemory _vkDeviceMemory{ VK_NULL_HANDLE };
@@ -306,7 +306,7 @@ class VKStrictResourceTexture: public VKFixedAllocationTexture {
     VkDescriptorImageInfo getDescriptorImageInfo() override;
     //VkImage _vkImage { VK_NULL_HANDLE };
     VkImageView _vkImageView { VK_NULL_HANDLE };
-    VkImageLayout _vkImageLayout {}; // VKTODO
+    VkImageLayout _vkImageLayout {};
     VkSampler _vkSampler { VK_NULL_HANDLE };
     // This need to be moved to VKFixedAllocationTexture and allocated in allocateStorage()
     //VkDeviceMemory _vkDeviceMemory{ VK_NULL_HANDLE };
@@ -343,7 +343,7 @@ class VKExternalTexture: public VKTexture {
     void postTransfer(VKBackend &backend) override;
     VkDescriptorImageInfo getDescriptorImageInfo() override;
     VkImageView _vkImageView { VK_NULL_HANDLE };
-    VkImageLayout _vkImageLayout {}; // VKTODO
+    VkImageLayout _vkImageLayout {};
     VkSampler _vkSampler { VK_NULL_HANDLE };
 
     // Shared texture properties

diff --git a/libraries/gpu/src/gpu/Texture.cpp b/libraries/gpu/src/gpu/Texture.cpp
@@ -315,7 +315,7 @@ Texture::Size Texture::resize(Type type, const Element& texelFormat, uint16 widt
 }
 
 bool Texture::isColorRenderTarget() const {
-    return (_texelFormat.getSemantic() == gpu::RGBA);
+    return (_texelFormat.getSemantic() == gpu::RGBA); // TODO: this is wrong, especially since we use B10G11R11 render targets too
 }
 
 bool Texture::isDepthStencilRenderTarget() const {

diff --git a/libraries/vk/src/vk/VulkanTools.cpp b/libraries/vk/src/vk/VulkanTools.cpp
@@ -290,24 +290,6 @@ namespace vks
                 1, &imageMemoryBarrier);
         }
 
-        // Fixed sub resource on first mip level and layer
-        void setImageLayout(
-            VkCommandBuffer cmdbuffer,
-            VkImage image,
-            VkImageAspectFlags aspectMask,
-            VkImageLayout oldImageLayout,
-            VkImageLayout newImageLayout,
-            VkPipelineStageFlags srcStageMask,
-            VkPipelineStageFlags dstStageMask)
-        {
-            VkImageSubresourceRange subresourceRange = {};
-            subresourceRange.aspectMask = aspectMask;
-            subresourceRange.baseMipLevel = 0;
-            subresourceRange.levelCount = 1;
-            subresourceRange.layerCount = 1;
-            setImageLayout(cmdbuffer, image, oldImageLayout, newImageLayout, subresourceRange, srcStageMask, dstStageMask);
-        }
-
         void insertImageMemoryBarrier(
             VkCommandBuffer cmdbuffer,
             VkImage image,