Cleanup and fix for reflections

libraries/display-plugins/src/display-plugins/VulkanDisplayPlugin.cpp

@@ -351,8 +351,8 @@ void VulkanDisplayPlugin::deactivate() {
         _container->currentDisplayActions().clear();
     }
 
-    // VKTODO: Should this be here?
-    uncustomizeContext();
+    // VKTODO: This shouldn't be here, because it's wrong thread, but then where?
+    //uncustomizeContext();
 
     Parent::deactivate();
 }

libraries/gpu-vk/src/gpu/vk/VKBackend.cpp

@@ -64,6 +64,8 @@ BackendPointer VKBackend::createBackend() {
     // FIXME provide a mechanism to override the backend for testing
     // Where the gpuContext is initialized and where the TRUE Backend is created and assigned
     std::shared_ptr<VKBackend> result = std::make_shared<VKBackend>();
+    result->initTransform();
+    result->initDefaultTexture();
     INSTANCE = result.get();
     void* voidInstance = &(*result);
     qApp->setProperty(VK_BACKEND_PROPERTY_NAME, QVariant::fromValue(voidInstance));
@@ -111,8 +113,6 @@ VKBackend::VKBackend() {
         _framePool.push_back(std::make_shared<FrameData>(this));
         _framesToReuse.push_back(_framePool.back());
     }
-    initTransform();
-    initDefaultTexture();
 }
 
 VKBackend::~VKBackend() {
@@ -832,9 +832,8 @@ void VKBackend::setDrawCommandBuffer(VkCommandBuffer commandBuffer) {
     _currentCommandBuffer = commandBuffer;
 }
 
-void VKBackend::trash(VKBuffer& buffer) {
-    // VKTODO: thread safety for this and similar calls
-    buffer.destroy();
+VkDescriptorImageInfo VKBackend::getDefaultTextureDescriptorInfo() {
+    return _defaultTextureVk->getDescriptorImageInfo();
 }
 
 void VKBackend::TransformStageState::preUpdate(size_t commandIndex, const StereoState& stereo, Vec2u framebufferSize) {
@@ -1224,7 +1223,7 @@ void VKBackend::updateVkDescriptorWriteSetsTexture(VkDescriptorSet target) {
                     qDebug() << "Texture is null during descriptor " << i
                              << " write: " << _resource._textures[i].texture->source();
                 }
-                imageInfo = _defaultTexture.descriptor;
+                imageInfo = _defaultTextureImageInfo;
             }
             //imageInfo.imageLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
             //imageInfo.imageView = texture->;
@@ -1251,7 +1250,7 @@ void VKBackend::updateVkDescriptorWriteSetsTexture(VkDescriptorSet target) {
                 descriptorWriteSet.dstArrayElement = 0;
                 descriptorWriteSet.descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER;
                 descriptorWriteSet.descriptorCount = 1;
-                descriptorWriteSet.pImageInfo = &_defaultTexture.descriptor;
+                descriptorWriteSet.pImageInfo = &_defaultTextureImageInfo;
                 sets.push_back(descriptorWriteSet);
             }
         }
@@ -1993,7 +1992,13 @@ void VKBackend::initDefaultTexture() {
             buffer[x + y * width + 3] = 255;
         }
     }
-    _defaultTexture.fromBuffer(buffer.data(), buffer.size(), VK_FORMAT_R8G8B8A8_SRGB, width, height, _context.device.get(),  _context.transferQueue);
+
+    _defaultTexture = gpu::Texture::create2D(gpu::Element{ gpu::VEC4, gpu::NUINT8, gpu::RGBA }, width, height, 1U,
+                                                gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_LINEAR, gpu::Sampler::WRAP_CLAMP));
+    _defaultTexture->setStoredMipFormat(_defaultTexture->getTexelFormat());
+    _defaultTexture->assignStoredMip(0, width * height * sizeof(uint8_t) * 4, (const gpu::Byte*)buffer.data());
+    _defaultTextureVk = syncGPUObject(*_defaultTexture);
+    _defaultTextureImageInfo = _defaultTextureVk->getDescriptorImageInfo();
 }
 
 void VKBackend::acquireFrameData() {
@@ -2016,6 +2021,149 @@ void VKBackend::waitForGPU() {
     VK_CHECK_RESULT(vkDeviceWaitIdle(_context.device->logicalDevice));
 }
 
+void VKBackend::Recycler::trashVkSampler(VkSampler& sampler) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+    vkSamplers.push_back(sampler);
+}
+
+void VKBackend::Recycler::trashVkFramebuffer(VkFramebuffer& framebuffer) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+    vkFramebuffer.push_back(framebuffer);
+}
+
+void VKBackend::Recycler::trashVkImageView(VkImageView& imageView) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+    vkImageViews.push_back(imageView);
+}
+
+void VKBackend::Recycler::trashVkImage(VkImage& image) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+    vkImages.push_back(image);
+}
+
+void VKBackend::Recycler::trashVkBuffer(VkBuffer& buffer) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+    vkBuffers.push_back(buffer);
+}
+
+void VKBackend::Recycler::trashVkRenderPass(VkRenderPass& renderPass) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+    vkRenderPasses.push_back(renderPass);
+}
+
+void VKBackend::Recycler::trashVkPipeline(VkPipeline& pipeline) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+    vkPipelines.push_back(pipeline);
+}
+
+void VKBackend::Recycler::trashVkShaderModule(VkShaderModule& module) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+    vkShaderModules.push_back(module);
+}
+
+void VKBackend::Recycler::trashVmaAllocation(VmaAllocation& allocation) {
+    std::lock_guard<std::recursive_mutex> lockGuard(recyclerMutex);
+}
+
+void VKBackend::perFrameCleanup() {
+    std::lock_guard<std::recursive_mutex> lockGuard(recycler.recyclerMutex);
+    // Remove pointers to objects that were deleted during the frame.
+    for (auto framebuffer : recycler.deletedFramebuffers) {
+        framebuffers.erase(framebuffer);
+    }
+
+    size_t capacityBeforeClear = recycler.deletedFramebuffers.capacity();
+    recycler.deletedFramebuffers.resize(0);
+    recycler.deletedFramebuffers.reserve(capacityBeforeClear);
+    for (auto buffer : recycler.deletedBuffers) {
+        buffers.erase(buffer);
+    }
+
+    capacityBeforeClear = recycler.deletedBuffers.capacity();
+    recycler.deletedBuffers.resize(0);
+    recycler.deletedBuffers.reserve(capacityBeforeClear);
+    for (auto texture : recycler.deletedTextures) {
+        textures.erase(texture);
+    }
+
+    capacityBeforeClear = recycler.deletedTextures.capacity();
+    recycler.deletedTextures.resize(0);
+    recycler.deletedTextures.reserve(capacityBeforeClear);
+    for (auto query : recycler.deletedQueries) {
+        queries.erase(query);
+    }
+
+    capacityBeforeClear = recycler.deletedQueries.capacity();
+    recycler.deletedQueries.resize(0);
+    recycler.deletedQueries.reserve(capacityBeforeClear);
+
+    auto device = _context.device->logicalDevice;
+    for (auto sampler : recycler.vkSamplers) {
+        vkDestroySampler(device, sampler, nullptr);
+    }
+
+    for (auto imageView : recycler.vkImageViews) {
+        vkDestroyImageView(device, imageView, nullptr);
+    }
+
+    for (auto image : recycler.vkImages) {
+        vkDestroyImage(device, image, nullptr);
+    }
+
+    for (auto buffer : recycler.vkBuffers) {
+        vkDestroyBuffer(device, buffer, nullptr);
+    }
+
+    for (auto renderPass: recycler.vkRenderPasses) {
+        vkDestroyRenderPass(device, renderPass, nullptr);
+    }
+
+    for (auto pipeline: recycler.vkPipelines) {
+        vkDestroyPipeline(device, pipeline, nullptr);
+    }
+
+    for (auto allocation : recycler.vmaAllocations) {
+        vmaFreeMemory(vks::Allocation::getAllocator(), allocation);
+    }
+}
+
+void VKBackend::beforeShutdownCleanup() {
+    // Lock prevents destroying objects while hashes
+    std::lock_guard<std::recursive_mutex> lockGuard(recycler.recyclerMutex);
+    // Remove pointers to objects that were deleted during the frame.
+    // This prevents access-after-delete.
+    perFrameCleanup();
+
+    // Delete remaining backend objects.
+    for (auto framebuffer : framebuffers) {
+        framebuffer->_gpuObject.gpuObject.setGPUObject(nullptr);
+    }
+    framebuffers.clear();
+
+    for (auto buffer : buffers) {
+        buffer->_gpuObject.gpuObject.setGPUObject(nullptr);
+    }
+    buffers.clear();
+
+    for (auto texture : textures) {
+        texture->_gpuObject.gpuObject.setGPUObject(nullptr);
+    }
+    textures.clear();
+
+    for (auto query : queries) {
+        query->_gpuObject.gpuObject.setGPUObject(nullptr);
+    }
+    queries.clear();
+
+    // Deleted objects got added to recycler, so they need to be cleaned since sets are already empty.
+    recycler.deletedFramebuffers.clear();
+    recycler.deletedBuffers.clear();
+    recycler.deletedTextures.clear();
+    recycler.deletedQueries.clear();
+
+    // One more cleanup to destroy Vulkan objects released by backend objects.
+    perFrameCleanup();
+}
 
 void VKBackend::initTransform() {
 

libraries/gpu-vk/src/gpu/vk/VKBackend.h

@@ -263,7 +263,6 @@ class VKBackend : public Backend, public std::enable_shared_from_this<VKBackend>
     void updateRenderPass();
     void resetRenderPass();
 
-    // VKTODO: one instance per each frame
     // Contains objects that are created per frame and need to be deleted after the frame is rendered
     class FrameData {
     public:
@@ -289,6 +288,46 @@ class VKBackend : public Backend, public std::enable_shared_from_this<VKBackend>
         VKBackend *_backend;
     };
 
+    // Contains objects that need to be deleted on Vulkan backend thread after frame is rendered.
+    // It's filled by destructors of objects like gpu::Texture and gpu::Buffer, since these destroy
+    // backend counterpart of their objects.
+public:
+    class Recycler {
+    public:
+        // This means that for every GPU object mutex will be locked and unlocked several times.
+        // VKTODO: It would be good to do profiling and check if it impacts performance or not.
+        void trashVkSampler(VkSampler &sampler);
+        void trashVkFramebuffer(VkFramebuffer &framebuffer);
+        void trashVkImageView(VkImageView &imageView);
+        void trashVkImage(VkImage &image);
+        void trashVkBuffer(VkBuffer &buffer);
+        void trashVkRenderPass(VkRenderPass &renderPass);
+        void trashVkPipeline(VkPipeline &pipeline);
+        void trashVkShaderModule(VkShaderModule &module);
+        void trashVmaAllocation(VmaAllocation &allocation);
+
+    private:
+        std::recursive_mutex recyclerMutex;
+
+        std::vector<VkSampler> vkSamplers;
+        std::vector<VkFramebuffer> vkFramebuffer;
+        std::vector<VkImageView> vkImageViews;
+        std::vector<VkImage> vkImages;
+        std::vector<VkBuffer> vkBuffers;
+        std::vector<VkRenderPass> vkRenderPasses;
+        std::vector<VkPipeline> vkPipelines;
+        std::vector<VkShaderModule> vkShaderModules;
+        std::vector<VmaAllocation> vmaAllocations;
+
+        // List of pointers to objects that were deleted and need to be removed from backend object sets.
+        std::vector<VKFramebuffer*> deletedFramebuffers;
+        std::vector<VKBuffer*> deletedBuffers;
+        std::vector<VKTexture*> deletedTextures;
+        std::vector<VKQuery*> deletedQueries;
+        friend class VKBackend;
+    } recycler;
+
+private:
     void draw(VkPrimitiveTopology mode, uint32 numVertices, uint32 startVertex);
     void renderPassTransfer(const Batch& batch);
     void renderPassDraw(const Batch& batch);
@@ -320,13 +359,10 @@ class VKBackend : public Backend, public std::enable_shared_from_this<VKBackend>
     void downloadFramebuffer(const FramebufferPointer& srcFramebuffer, const Vec4i& region, QImage& destImage) final;
     void setDrawCommandBuffer(VkCommandBuffer commandBuffer);
     size_t getNumInputBuffers() const { return _input._invalidBuffers.size(); }
-    VkDescriptorImageInfo getDefaultTextureDescriptorInfo() { return _defaultTexture.descriptor; };
+    VkDescriptorImageInfo getDefaultTextureDescriptorInfo() ;
     // Used by GPU frame player to move camera around
     void enableContextViewCorrectionForFramePlayer() { _transform._viewCorrectionEnabledForFramePlayer = true; };
 
-
-    void trash(VKBuffer& buffer);
-
     static gpu::Primitive getPrimitiveTopologyFromCommand(Batch::Command command, const Batch& batch, size_t paramOffset);
 
     // Draw Stage
@@ -422,14 +458,27 @@ class VKBackend : public Backend, public std::enable_shared_from_this<VKBackend>
     VKFramebuffer *_outputTexture{ nullptr };
 protected:
 
+    // These are filled by syncGPUObject() calls, and are needed to track backend objects so that they can be destroyed before
+    // destroying backend.
+    // Access to these objects happens only from the backend thread. Destructors don't access them directly, but through a recycler.
+    std::unordered_set<VKFramebuffer*> framebuffers;
+    std::unordered_set<VKBuffer*> buffers;
+    std::unordered_set<VKTexture*> textures;
+    std::unordered_set<VKQuery*> queries;
+    void perFrameCleanup();
+    // Called by the destructor
+    void beforeShutdownCleanup();
+
     // Logical device, application's view of the physical device (GPU)
     // VkPipeline cache object
     VkPipelineCache _pipelineCache;
 
     vks::Context& _context{ vks::Context::get() };
     //VkQueue _graphicsQueue; //TODO: initialize from device
     //VkQueue _transferQueue; //TODO: initialize from device
-    vks::Texture2D _defaultTexture;
+    std::shared_ptr<gpu::Texture> _defaultTexture;
+    VKTexture* _defaultTextureVk{ nullptr };
+    VkDescriptorImageInfo _defaultTextureImageInfo{};
     friend class VKBuffer;
     friend class VKFramebuffer;
     VkCommandBuffer _currentCommandBuffer;

libraries/gpu-vk/src/gpu/vk/VKBuffer.cpp

@@ -92,7 +92,8 @@ VKBuffer::~VKBuffer() {
     Backend::bufferCount.decrement();
     auto backend = _backend.lock();
     if (backend) {
-        backend->trash(*this);
+        backend->recycler.trashVkBuffer(buffer);
+        backend->recycler.trashVmaAllocation(allocation);
     }
 }
 

libraries/gpu-vk/src/gpu/vk/VKTexture.cpp

@@ -241,6 +241,7 @@ void VKAttachmentTexture::createTexture(VKBackend &backend) {
 }
 
 VKAttachmentTexture::~VKAttachmentTexture() {
+    // VKTODO: Redo destructors for cleanup to happen on present thread
     auto backend = _backend.lock();
     auto device = backend->getContext().device->logicalDevice;
     if (_vkImageView) {

libraries/gpu/src/gpu/Forward.h

@@ -95,6 +95,11 @@ namespace gpu {
     class TextureTable;
     using TextureTablePointer = std::shared_ptr<TextureTable>;
 
+    namespace vk {
+        class VKBackend;
+        class VKBuffer;
+    }
+
     struct StereoState {
         StereoState() {}
         bool isStereo() const {
@@ -127,6 +132,7 @@ namespace gpu {
         GPUObject* getGPUObject() const { return _gpuObject.get(); }
 
         friend class Backend;
+        friend class vk::VKBackend;
         friend class Texture;
     };
 
@@ -150,10 +156,6 @@ namespace gpu {
         class GLESBuffer;
     }
 
-    namespace vk {
-        class VKBackend;
-        class VKBuffer;
-    }
 }
 
 #endif

libraries/render-utils/src/LightingModel.cpp

@@ -88,6 +88,7 @@ LightingModel::LightingModel() {
             }
         });
 
+        _ambientFresnelLUT->setStoredMipFormat(_ambientFresnelLUT->getTexelFormat());
         _ambientFresnelLUT->assignStoredMip(0, N_roughness * N_NdotV * sizeof(LUTVector::value_type), (const gpu::Byte*)lut.data());
     }
 #endif