Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BGFX_DEBUG_TEXT buffer memory corruption #3418

Closed
synaptor opened this issue Mar 27, 2025 · 5 comments · Fixed by #3419
Closed

BGFX_DEBUG_TEXT buffer memory corruption #3418

synaptor opened this issue Mar 27, 2025 · 5 comments · Fixed by #3419
Labels

Comments

@synaptor
Copy link

synaptor commented Mar 27, 2025

Describe the bug
Under certain conditions, debug text overlay corrupts rendering.

To Reproduce
Steps to reproduce the behavior:

  1. Patch fb7b505
  2. Run 27-terrain
  3. Observe artifacts on the screen (sometimes pink screen, sometimes blinking IMGUI windows)

Expected behavior
Normally working applications.

Screenshots

Image

Additional context

The bug is reproducible on Linux and Windows. Best visible in Vulkan. Direct3D11, Direct3D12, OpenGL are also affected, but on my machine artifacts are much more subtle.

I suspect use-after-free in the text blitter, but not 100% sure.

bgfx/src/bgfx.cpp

Lines 713 to 714 in abe193a

m_vb = s_ctx->createTransientVertexBuffer(numBatchVertices*m_layout.m_stride, &m_layout);
m_ib = s_ctx->createTransientIndexBuffer(numBatchIndices*2);
seems to allocate a transient buffer which (I guess) is auto-disposed at the end of the frame, but keeps being reused.

@bkaradzic bkaradzic added the bug label Mar 27, 2025
@bkaradzic
Copy link
Owner

@synaptor

The bug is reproducible on Linux and Windows. Best visible in Vulkan. Direct3D11, Direct3D12, OpenGL are also affected, but on my machine artifacts are much more subtle.

Are you sure you're seeing anything with D3D11, D3D12, GL?

@bkaradzic
Copy link
Owner

@mcourteaux This issue is related to scratch buffer in VK. I made workaround, but it's not real fix: 72f9b8b

@mcourteaux
Copy link
Contributor

Indeed doesn't seem like this should fix it. I can have a look at this on Monday.

@synaptor
Copy link
Author

Are you sure you're seeing anything with D3D11, D3D12, GL?

Those artifacts were due to an unrelated data race in my own code. I was changing buffers while bgfx was uploading them to GPU. Disregard that part. I can confirm that I only observed the current issue in Vulkan.

Thank you for your quick help!

@mcourteaux
Copy link
Contributor

Debugging this issue revealed the problem:

I added some tracing, and this is the loop causing the issue:

../../../src/renderer_vk.cpp (8435): BGFX Submit
../../../src/renderer_vk.cpp (4851): BGFX BufferVK::update(this=0x8058c4e0): srcOffset=3848, dstOffset=0, size=9342
../../../src/renderer_vk.cpp (4851): BGFX BufferVK::update(this=0x805a4508): srcOffset=13192, dstOffset=0, size=42440
../../../src/renderer_vk.cpp (8513): BGFX reset scratch buffer
../../../src/renderer_vk.cpp (9476): BGFX VideoMemBlitter
../../../src/renderer_vk.cpp (2705): BGFX Update TextVideoMemBlitter index buffer
../../../src/renderer_vk.cpp (4851): BGFX BufferVK::update(this=0x8058c4c8): srcOffset=0, dstOffset=0, size=372
../../../src/renderer_vk.cpp (2707): BGFX Update TextVideoMemBlitter vertex buffer
../../../src/renderer_vk.cpp (4851): BGFX BufferVK::update(this=0x805a44c8): srcOffset=376, dstOffset=0, size=3472
../../../src/renderer_vk.cpp (8223): BGFX kick(): vkSubmitQueue()

In words,

  1. ImGui stores its data in a transient buffer, which gets updated through a (scratch) staging buffer at the beginning of the frame submit code.
  2. Then scratch buffers get reset, meaning resetting their used space to 0.
  3. Then at the very end of the frame, after everything else is put in the command buffers, the TextVideoMemBlitter does its thing, reusing the space used in the scratch buffer and overwriting the data put there earlier by ImGui.
  4. The queue gets submitted for execution.

The fix is simply moving the reset calls for the scratch buffers to after the flush() calls. I'll PR that, while undoing the workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants