Walnut::Image::SetData causes unresolved memory allocation

[SinouDev]

After using the Diagnostic Tool of Microsoft Visual Studio it seems the memory allocation happen everytime when
m_FinalImage->SetData(m_Renderer.GetImageDataBuffer()->Get<uint8_t*>());
get called and the size of the allocation keep increasing everytime, I don't know if the issue is in my code side or the Walnut::Image library itself. I hope the issue get fixed.

UPDATE:
I noticed that it happens when the window resized or minimized.

Source code

[nathansCodes]

I'm having a problem where my program crashes after some time with VK_ERROR_OUT_OF_POOL_MEMORY.
I think that this crash has to do with that.
Are you having the same issue?

[arialpew]

First, I'm talking about the "dev" branch - I'm not using the "master" branch so everything I say here have to be investigated further in "master" - it should be the same.

---

The leak

I had a crash tonight due to Image::SetData leak that happen on each frame. I'm not a Vulkan expert, but I've enough knowledge with DX12 to transpose the problem.

First obvious reason when you have a dynamic texture leak like that : Vulkan ressource isn't released properly each frame, leaking memory over time.

I didn't found issues inside the Image class, everything seem fine. The descriptors are setup once, the upload buffer is created once, the memory copy operation between CPU and GPU is setup properly, the implementation seem fine but beware that calling Image::SetData many times could lead to memory leak if you don't call vkFreeCommandBuffers later (this is exactly our issues in that case, because Image::SetData use command buffers).

First thing to do is to reproduce the leak. I realized I can't reproduce this leak again and again. It seem to happen only when the application is minimized. Resizing and maximizing doesn't change anything, and at best it can aleviate the problem by recreating the swap chain and releasing the leaked ressource.

Here's a leak trace :

Walnut::Application::Run() - Line 955
ntdll.dll!0x7ffce43acb41()
KernelBase.dll!0x7ffce1965151()
nvoglv64.dll!0x7ffc006c5bd4()
nvoglv64.dll!0x7ffc00b37ade()
nvoglv64.dll!0x7ffc00b376aa()
nvoglv64.dll!0x7ffc00b3a411()
nvoglv64.dll!0x7ffc00b3a592()
nvoglv64.dll!0x7ffc00aa18a5()
VkLayer_khronos_validation.dll!DispatchAllocateCommandBuffers() - Line 1396
VkLayer_khronos_validation.dll!vulkan_layer_chassis::AllocateCommandBuffers() - Line 2643
vulkan-1.dll!0x7ffc1a4b004e()
Walnut::Application::GetCommandBuffer() - Line 1044
Walnut::Image::SetData() - Line 240
RootLayer::OnUIRender() - Line 56

Ok, Application::GetCommandBuffer seem to be the problem.

Remember what I said before about Image::SetData/command buffers and vkFreeCommandBuffers ? The leak doesn't happen when the application is running normally, this happen only when it's minimized. So that mean some command buffers aren't released when the application is minimized. Is this possible, really ? Yes.

// ApplicationGUI.cpp
// Rendering
ImGui::Render();
ImDrawData* main_draw_data      = ImGui::GetDrawData();
const bool  main_is_minimized   = (main_draw_data->DisplaySize.x <= 0.0f || main_draw_data->DisplaySize.y <= 0.0f);
wd->ClearValue.color.float32[0] = clear_color.x * clear_color.w;
wd->ClearValue.color.float32[1] = clear_color.y * clear_color.w;
wd->ClearValue.color.float32[2] = clear_color.z * clear_color.w;
wd->ClearValue.color.float32[3] = clear_color.w;

if (!main_is_minimized) // <--- This condition can create a leak
	FrameRender(wd, main_draw_data);

// Update and Render additional Platform Windows
if (io.ConfigFlags & ImGuiConfigFlags_ViewportsEnable)
{
	ImGui::UpdatePlatformWindows();
	ImGui::RenderPlatformWindowsDefault();
}

// Present Main Platform Window
if (!main_is_minimized) // <--- This condition can create a leak
	FramePresent(wd);
else
	std::this_thread::sleep_for(std::chrono::milliseconds(5));

Walnut Application try to skip rendering and presentation of the frame when the application is minimized, to reduce ressources consumption. This is good in theory, but in practice with the current implementation, if we skip FrameRender and FramePresent when the application is minimized, we skip all of theses steps in order :

• Free ressource in queue.
• Free command buffers (vkFreeCommandBuffers) allocated by Application::GetCommandBuffer (here's the leak related to Image::SetData, bingo).
• Record dear imgui primitives into command buffer.
• Submit command buffer.
• Queue present.

But wait, whenever the application is minimized or not, we walk the layer stack and call OnUIRender virtual member function. If you use a dynamic texture and call Image::SetData every single time OnUIRender is called, the command buffers gonna keep growing untill you unminimize the application to render and present. Hence the leak.

Solution

• ⚠️Do not call layer OnUIRender and/or OnUpdate when the application is minimized. It doesn't make sense to update textures data and call ImGui stuff if the frames will be skipped.

Sadly, immediate mode framework doesn't work well with this approach so it's not possible to skip rendering of ImGui even if the application is minimized.

"Error: Mismatched Begin/BeginChild vs End/EndChild calls: did you forget to call End/EndChild (did you skipped rendering?)"

So in reality, we should let OnUIRender like this.

Should we skip OnUpdate if the application is minimized ? Maybe ... this is up to Cherno decision. But no matter, we can't count solely on that to fix the leak because there's no guarantee that people gonna put their side effect inside OnUpdate.

Also, most people that fire side-effect inside their OnUpdate function don't expect application to "stop the world" when they are minimized because the current behavior doesn't do that.

If we change OnUpdate behavior (doesn't called when minimized), then it's a breaking change. In that case, if this change is made, you simply move your Image::SetData from OnUIRender to OnUpdate and you have no leak. But remember, we can't solely count on that, it's fragile and easy to forgot that OnUiRender should never touch the command buffers.

If you want to know if the window is minimized at any time, you can use glfw :

const bool main_is_minimized = glfwGetWindowAttrib(m_WindowHandle, GLFW_ICONIFIED);

• 🥇 Release the appropriated Vulkan ressources each frame, even if the frame is skipped when the application is minimized.

This is probably the best solution and the good fix. People should be able to interact with command buffers at their own discretion because everyone has different use cases, and ressources should be freed properly each frame no matter the state of the window. If they want to skip work, they can put their work inside OnUpdate - like Image::SetData could be skipped if the window is minimized (assuming the above change about OnUpdate is made).

• ❌ Make Image::SetData aware of if the application is minimized or not.

The idea is if the application is minimized, Image::SetData should be a no-op. This is unrealistic since Image and window state are something unrelated.

• 🥈 Remove the FrameRender and FramePresent skip when the application is minimized (lazy but easy fix).

This cause the application to render and present even when it's minimized, so the command buffers gonna be properly released each frame. This remove the benefit of skipping rendering, but if you have the leak it's probably the easiest fix you can do. Simply remove the 2 conditions about minimization.

My current fix :

• OnUpdate skipped when the application is minimized.
• Image::SetData inside OnUpdate, never call Image::SetData inside OnUiRender or you'll leak because of command buffers not freed when application is minimized - remember OnUiRender is running each frame.