WebGPU backend leaks when using ImGui::Image

[Jairard]

Version/Branch of Dear ImGui:

Version 1.90.4, Branch: docking

Back-ends:

imgui_impl_wgpu.cpp + imgui_impl_glfw.cpp

Compiler, OS:

Windows 10 + clang-cl 18.1.8

Full config/build information:

Dear ImGui 1.90.4 (19040)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=202400
define: _WIN32
define: _WIN64
define: _MSC_VER=1939
define: _MSVC_LANG=202004
define: __clang_version__=17.0.6 
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_glfw
io.BackendRendererName: imgui_impl_webgpu
io.ConfigFlags: 0x00000000
io.ConfigViewportsNoDecoration
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00000C0E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 640.00,480.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

Whenever an ImGui::Image is used with WebGPU backend, memory leaks are reported

Here is the output I get when running the minimal code sample and closing the window:

D3D12 WARNING: Process is terminating. Using simple reporting. Please call ReportLiveObjects() at runtime for standard reporting. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: Live Producer at 0x0000027177755518, Refcount: 6. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x0000027177896250, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x00000271777E6870, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x000002717781AF00, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x0000027174F25F30, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x000002717774BBA0, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x00000271777E88B0, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x000002717774C320, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x0000027174C7C700, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x000002717774DD50, Refcount: 1. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x0000027174F25310, Refcount: 1. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x0000027177953050, Refcount: 0. [ STATE_CREATION WARNING #0: UNKNOWN]
D3D12 WARNING: 	Live Object at 0x00000271779537D0, Refcount: 1. [ STATE_CREATION WARNING #0: UNKNOWN]

As soon as I comment the ImGui::Image line, this log disappears.

Investigation

I investigated the imgui_impl_wgpu.cpp file and it appears that the WGPUBindGroups contained in ImageBindGroups (of the RenderResources structure) are not released in static void SafeRelease(RenderResources& res), that is called by ImGui_ImplWGPU_Shutdown().

Initially, I stumbled upon this while having the issue mentioned reported in https://github.com/ocornut/imgui/issues/7765. It appears that clearing the WGPUBindGroups each frame could also solve the issue, if it is done at the end of the call to ImGui_ImplWGPU_RenderDrawData, but I think the groups need to survive longer than this. Anyway, the SafeRelease functions still do not do their job properly.

That's why I chose to open a distinct issue.

Local hack to fix the issue

Locally, I hacked my way through the issue but in a very ugly way. I expose the code here in order to show my way of thinking about this issue:

// imgui_mock is a namespace where I copy-pasted structs that are only defined in imgui_impl_wgpu.cpp
static auto clear_image_bind_groups() -> void {
    imgui_mock::ImGui_ImplWGPU_Data & bd =
        *reinterpret_cast<imgui_mock::ImGui_ImplWGPU_Data *>(ImGui::GetIO().BackendRendererUserData);
    auto & storage = bd.renderResources.ImageBindGroups;
    // This group is already released but stored in the same place
    auto const font_bind_group = bd.renderResources.ImageBindGroup;

    for (auto const & pair : storage.Data) {
        if (pair.val_p != nullptr && pair.val_p != font_bind_group)
            wgpuBindGroupRelease(reinterpret_cast<WGPUBindGroup>(pair.val_p));
    }

    storage.Clear();
}

void render_frame()
{
    // Configure everything
    WGPURenderPassEncoder render_pass = /* ... */;

    // Draw ImGui data
    ImGui::Render();
    ImGui_ImplWGPU_RenderDrawData(ImGui::GetDrawData(), render_pass);
    
    // Other rendering stuff
    wgpuQueueSubmit(/* ... */);
    wgpuSurfacePresent(/* ... */);

    // When everything is done, clean bind groups
    clear_image_bind_groups();
}

Fix proposal

I do believe that adding some cleaning code in SafeRelease would fix the issue in a clean way, something that would look like that:

static void SafeRelease(RenderResources& res)
{
    SafeRelease(res.FontTexture);
    SafeRelease(res.FontTextureView);
    SafeRelease(res.Sampler);
    SafeRelease(res.Uniforms);
    SafeRelease(res.CommonBindGroup);

    // Add this loop
    for (int i = 0; i < res.ImageBindGroups.Data.Size; i++)
    {
        if (res.ImageBindGroups.Data[i].val_p != res.ImageBindGroup)
            SafeRelease((WGPUBindGroup)res.ImageBindGroups.Data[i].val_p);
    }

    SafeRelease(res.ImageBindGroup);
    SafeRelease(res.ImageBindGroupLayout);
};

Note that we avoid releasing twice the WGPUBindGroup that is both stored in ImageBindGroup and ImageBindGroups. This bind group is used for the default font atlas and is correctly released. When calling ImGui::ShowDemoWindow(), there is no leak because the only call to ImGui::Image is to display the default font atlas.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

// At init time, create or load a texture:
WGPUTextureView my_texture_view = /* ... */;

// In main loop, call ImGui::Image
ImGui::Begin("Example Bug");
ImGui::Image((void *)my_texture_view, ImVec2(my_image_width, my_image_height));
ImGui::End();

[Jairard]

Should I propose a PR ?

[ocornut]

Yes that would be good. One that possibly tackle this and #7765 (if meaningful you can split in multiple commits). I don't know WGPU very well so any explanation is useful as we tend to return back to those issues later.

Note that it is legal for other backends call RenderDrawData() multiple time so I wouldn't perform any clear there. If anything they can be in backend NewFrame() and Shutdown() function if needed.

[Jairard]

Ok, so I could tackle both issues.

I tried implementing the solution you proposed in https://github.com/ocornut/imgui/issues/7765#issuecomment-2206436309, in order to remove the permanent storage of WGPUBindGroups. Unfortunately, it's not feasible because we would have to release the WGPUBindGroups in RenderDrawData(), but they need to stay alive longer.

So the solution I went with keeps the permanent storage but clears it in NewFrame() as it was also suggested. An additional cleaning is added in Shutdown() via the SafeRelease() functions to get rid of the WGPUBindGroups allocated in the last frame. As a side effect, I could remove the ImageBindGroup member of RenderResources to make things more clear.

As for a global explanation, I think it is pretty clear in https://github.com/ocornut/imgui/issues/7765 but I can try to sum it up here. In the backend, we provide a way to draw a texture via its WGPUTextureView. But we can't keep this view in memory because the user can release it at any point and we have no way to notify this to the backend (and it's no wishable to introduce one as it would be error-prone). Moreover, the user can release a WGPUTextureView and create a new one, which could have the exact same value since on WebGPU's side, the previous one doesn't exist anymore so the new one can be allocated with the same id. Hence we need to clear the backend cache before each new frame. This cache is an ImGuiStorage that contains WGPUBindGroups referenced by the hash of the corresponding WGPUTextureView. So in order to clear it we need to release each WGPUBindGroup and call the Clear() method. This way, we recreate the bind groups as needed by each frame.

N.B: PR incoming, I just wanted to post this before

[ocornut]

Hello, I have merged https://github.com/ocornut/imgui/commit/aaacb01b8d4441453928116240b5041625d43fbf, however your past post said:

Unfortunately, it's not feasible because we would have to release the WGPUBindGroups in RenderDrawData(), but they need to stay alive longer.

Can you clarify why? I don't know how the API operate. @pplux solution did clear them at the end of RenderDrawData(), and I am assuming it worked for them, and it has the advantage it can be done in a single spot. But if you think this would create issue in some case please let me know and I can alter the solution to be closer to what you have suggested in #8046

Thanks a lot for your explanation, and, I apologize I didn't catch on this PR earlier.

[Jairard]

Hello,

Thanks for coming back on this issue. I must confess that I didn't practice WebGPU much since I posted this. For what I remember, the WGPUBindGroups need to stay alive when you call wgpuQueueSubmit(), which is done by the user outside of RenderDrawData(). It is possible that it causes no issue for @pplux simply by coincidence. I might test the merged solution if I get some time to do so (but I can't tell when it's going to be).

Another argument against clearing the bind groups there was yours:

Note that it is legal for other backends call RenderDrawData() multiple time so I wouldn't perform any clear there.

So I'm not really convinced that it's a good idea.

I hope this was kind of clear.

[Jairard]

I got some time (not as mush as I'd like) to check on this, here are the news.

EDIT: don't mind this comment, I'm not up-to-date with latest webgpu so it might not be relevant. I'll try that again with the code on master and the latest webgpu as soon as I can and I'll keep you up-to-date. I let my comments as a spoiler for now.

Investigational thoughts

I couldn't get my minimal testing code to work with the newly merged code, but that's an issue on my side. I just didn't have enough time to solve compilation issues. However, I could get back on the code I was working on, and when I release the bind groups in RenderDrawData() and only there, I got the following error on the first frame of running the test, meaning that the binding group is released too soon:

thread '<unnamed>' panicked at C:\Users\runneradmin\.cargo\git\checkouts\wgpu-53e70f8674b08dd4\87576b7\wgpu-core\src\storage.rs:113:39:
BindGroup[Id(1,1,vk)] does not exist

I don't know how @pplux structures their code, but here is an overview of mine:

  while (!glfwWindowShouldClose(ctx.window)) {
    ImGui_ImplWGPU_NewFrame();
    ImGui_ImplGlfw_NewFrame();
    ImGui::NewFrame();

    ImGui::Begin("WebGPU Texture Test");
    ImGui::Image((void *)my_texture_view,
                 ImVec2(my_image_width, my_image_height));
    ImGui::End();

    // Create render pass
    // ...
    ImGui::Render();
    ImGui_ImplWGPU_RenderDrawData(ImGui::GetDrawData(), render_pass);
    wgpuRenderPassEncoderEnd(render_pass);

    // Create command buffer
    // ...
    wgpuQueueSubmit(queue, 1, &command_buffer);
    wgpuSurfacePresent(surface);
  }

Maybe there is an other way to do this that allows clearing in RenderDrawData(); maybe it's some other part of the merged MR that avoids the crash; or maybe it doesn't crash on every hardware.

[pplux]

Hi @Jairard, Since we merged the "fix" releasing resources on every frame, to me is been working great. I'm even creating/destroying textures and rendering into them dynamically on the fly with absolutely zero issues. With the code working fine on windows, mac, and wasm concurrently.

This the code I use to render with imgui,

https://github.com/pplux/minimal-wgpu-native/blob/dev_test/src/DemoImgui.cpp#L402-L434

And this is the code I use where I render each sub-wbgpu renderer, creating a texture as render target on the fly, for example on every frame when resizing the imgui windows

https://github.com/pplux/minimal-wgpu-native/blob/dev_test/src/DemoImgui.cpp#L324-L366

The end result is that I can render to texture, resize those textures every frame, and compose everything with imgui at the end with no issue at all.

Hope it helps :)

[Jairard]

Thank you, that's really helpful ! I'll have a look once I upgrade to latest webgpu.