LukasBanana / LLGL

Low Level Graphics Library (LLGL) is a thin abstraction layer for the modern graphics APIs OpenGL, Direct3D, Vulkan, and Metal
BSD 3-Clause "New" or "Revised" License
2.1k stars 139 forks source link

Vulkan swapChainImages and swapChainFrameBuffers not being properly deleted when SwapChain is recreated. #76

Closed lodinukal closed 2 years ago

lodinukal commented 2 years ago

I have identified bugs at https://github.com/LukasBanana/LLGL/blob/master/sources/Renderer/Vulkan/VKRenderContext.cpp#L411-L416 and https://github.com/LukasBanana/LLGL/blob/master/sources/Renderer/Vulkan/VKRenderContext.cpp#L465-L470

the outdated VKPtr and VKPtr don't get deleted, instead a new object is made every time the SwapChain is recreated; this causes it not to be deleted on the GPU.

I think it should check if one of the objects exists, if it doesn't, it should create one; then it can recreate it. Like this:

    if (!swapChainImageViews_[i]) {
        swapChainImageViews_[i] = VKPtr<VkImageView>{device_, vkDestroyImageView};
    }

    auto result = vkCreateImageView(device_, &createInfo, nullptr, wapChainImageViews_[i].ReleaseAndGetAddressOf());
    VKThrowIfFailed(result, "failed to create Vulkan swap-chain image view");
    if (!swapChainFramebuffers_[i]) {
        swapChainFramebuffers_[i] = VKPtr<VkFramebuffer>{device_, vkDestroyFramebuffer};
    }

    auto result = vkCreateFramebuffer(device_, &createInfo, nullptr, swapChainFramebuffers_[i].ReleaseAndGetAddressOf());
    VKThrowIfFailed(result, "failed to create Vulkan swap-chain framebuffer");
LukasBanana commented 2 years ago

Thank you. I can take a look next week.

LukasBanana commented 2 years ago

swapChainFramebuffers_[] is an array of VKPtr which should handle the deletion in the move operator, but I guess that operator is wrong in not calling the assigned deleter: https://github.com/LukasBanana/LLGL/blob/master/sources/Renderer/Vulkan/VKPtr.h#L131

Can you change that operator to this and check if the issue persists:

VKPtr<T>& operator = (VKPtr&& rhs)
{
    Release();                    // <-- This was missing
    object_ = rhs.object_;
    rhs.object_ = VK_NULL_HANDLE;
    return *this;
}
lodinukal commented 2 years ago

The issue is fixed after that change.

LukasBanana commented 2 years ago

Thanks for pointing out this issue. Fixed with 9dc06e5.