-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
Request for feedback: multi-threaded update/render domain with dynamic fonts & texture updates (v1.92) #8597
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
Comments
I think this is a nice and elegant solution, given the guarantee that the TexID field will never be directly accessed from within the ImGui thread (at least while a texture has ImTextureStatus_OK status because thats the only time a race condition could happen?). An edge case I could think of for this approach would be if 2 ImGui::Render happen without a ImGui_ImplDX11_RenderDrawData call (i.e. imgui thread runs every frame on high framerate while render thread is synced to 60fps). Frame 1:
Frame 2:
I dont think this is necessarily the responsibility of ImGui to handle but i do think that in order for the user to be able to nicely handle this, texture updates shouldnt be put into ImDrawData but a separate texture queue list which is then fed as an argument to |
They are only printing out the value so it doesn't really matter.
Yes it should definitively be a queue because in this proposed scheme the main scheme writes to ->Status immediately. It would be tempting to formalize a backend design where an extra function needs to be called: ImGui::Render();
ImGui_ImplDX11_UpdateTextures(ImGui::GetTextures());
ImGui_ImplDX11_RenderDrawData(ImGui::GetDrawData()); Tho it niggles me a bit to add an extra call in the main loop. PS.1: I genuinely don't know if stock backend _RenderDrawData() function have been ever used for threaded rendering. (note than in this wip branch, a majority of the xxx_NewFrame() functions are now emptier than ever, or near empty, so it's becoming more likely). My intuition is that nobody used stock backends for this. Therefore an extra hypothetical PS.2: An alternative would be that backend could optionally set a function pointer e.g. io.Renderer_UpdateTextures(), which would avoid the explicit call. The legacy design from 1.30 to 1.60 was that backend would set a |
I agree, though I do prefer it for the sake of being explicit.
I cant speak for everyone but people I know and myself have used stock ImGui to perform threaded rendering, it doesnt really need any modifications if you use a double buffered ImDrawData instance that you swap out from the main ImGui thread |
I tried a basic implementation, but I haven't formally tested it for threading issues.
If you have a threaded rendering setup I'd be interested if you can try it: // Usage for multi-threaded rendering.
// [Storage]
// - ImTextureQueue tex_queue;
// [Main Thread]
// - after ImGui::EndFrame()/Render()
// - call tex_queue.QueueRequests(ImGui::GetTextures());
// [Render Thread]
// - call tex_queue.ProcessRequests(ImGui_ImplDX11_UpdateTexture, 3); // 3 is number of maximum frames in flight
struct ImTextureRenderQueueRequest
{
ImTextureData TexCopy;
ImTextureData* TexInMainThread;
~ImTextureRenderQueueRequest() { TexCopy.Pixels = NULL; }
};
struct ImTextureRenderQueue
{
ImVector<ImTextureRenderQueueRequest> Requests;
// Main/Update thread queue requests.
void QueueRequests(ImVector<ImTextureData*>* textures);
// Render thread process requests.
template<typename Backend_UpdateTextureFunc>
void ProcessRequests(Backend_UpdateTextureFunc update_texture_func, int in_flight_frames);
};
void ImTextureRenderQueue::QueueRequests(ImVector<ImTextureData*>* textures)
{
for (ImTextureData* tex : *textures)
{
if (tex->Status == ImTextureStatus_OK)
continue;
// Store a copy of the ImTextureData
// (code is a bit fragile/awkward because our helpers are not fulfilling constructor/destructor in a standard way)
Requests.resize(Requests.Size + 1, ImTextureRenderQueueRequest());
ImTextureRenderQueueRequest& new_req = Requests.back();
new_req.TexCopy = *tex;
new_req.TexInMainThread = tex;
// Acknowledge as created from Main Thread POV
if (tex->Status == ImTextureStatus_WantCreate || tex->Status == ImTextureStatus_WantUpdates)
tex->Status = ImTextureStatus_OK;
}
}
template<typename Backend_UpdateTextureFunc>
void ImTextureRenderQueue::ProcessRequests(Backend_UpdateTextureFunc update_texture_func, int in_flight_frames)
{
for (ImTextureRenderQueueRequest& req : Requests)
{
ImTextureData* tex_copy = &req.TexCopy;
if (tex_copy->Status == ImTextureStatus_OK)
continue;
if (tex_copy->Status == ImTextureStatus_WantDestroy && tex_copy->UnusedFrames <= in_flight_frames)
continue;
ImTextureStatus prev_status = tex_copy->Status;
// Call backend function
update_texture_func(tex_copy);
// Backend must honor immediately
if (prev_status == ImTextureStatus_WantCreate || prev_status == ImTextureStatus_WantUpdates)
IM_ASSERT(tex_copy->Status == ImTextureStatus_OK);
if (prev_status == ImTextureStatus_WantDestroy)
IM_ASSERT(tex_copy->Status == ImTextureStatus_Destroyed);
// Write back to main thread
if (prev_status == ImTextureStatus_WantCreate)
{
req.TexInMainThread->SetTexID(tex_copy->TexID);
req.TexInMainThread->BackendUserData = tex_copy->BackendUserData;
}
if (prev_status == ImTextureStatus_WantDestroy)
{
req.TexInMainThread->Status = ImTextureStatus_Destroyed;
req.TexInMainThread->SetTexID(ImTextureID_Invalid);
req.TexInMainThread->BackendUserData = NULL;
}
}
Requests.resize(0);
} |
The reason a name like struct ImTextureRequests
{
ImVector<ImTextureData*> Textures;
}; ImGui_ImplDX11_UpdateTextures(ImGui::GetTextureRequests()); Is because the backend also this needs a list during shutdown. Current code being: // Destroy all textures
for (ImTextureData* tex : ImGui::GetPlatformIO().Textures)
if (tex->RefCount == 1)
{
tex->Status = ImTextureStatus_WantDestroy;
ImGui_ImplDX11_UpdateTexture(tex);
} Because the shutdown order is: ImGui_ImplDX11_Shutdown();
ImGui_ImplWin32_Shutdown();
ImGui::DestroyContext(); We can't mark WantDestroy and check RefCount in If the function name is Alternative struct ImTextureList
{
ImVector<ImTextureData*> Textures;
}; ImGui_ImplDX11_UpdateTextures(ImGui::GetTextureList()); |
Okay so I've been testing it on a barebones multithreaded setup using your ImDrawDataSnapshot class (#1860 (comment)) in the DX11 imgui example ImTextureRenderQueue tex_queue;
ImDrawDataSnapshot active_snapshot;
ImDrawDataSnapshot pending_snapshot;
std::mutex snapshot_mutex;
// moved up here from the main function
bool done = false;
void ImGuiThread()
{
const auto tick = std::chrono::duration<float>( 1.f / 32.f );
while ( !done )
{
const auto start = std::chrono::steady_clock::now();
// Start the Dear ImGui frame
ImGui_ImplDX11_NewFrame();
ImGui_ImplWin32_NewFrame();
ImGui::NewFrame();
ImGui::ShowDemoWindow( &show_demo_window );
// Rendering
ImGui::Render();
tex_queue.QueueRequests( &ImGui::GetPlatformIO().Textures );
snapshot_mutex.lock();
pending_snapshot.SnapUsingSwap( ImGui::GetDrawData(), ImGui::GetTime() );
snapshot_mutex.unlock();
const auto time_elapsed = std::chrono::steady_clock::now() - start;
if ( time_elapsed < tick )
std::this_thread::sleep_for( tick - time_elapsed );
}
}
// Main code
int main( int, char** )
{
// Omitted ImGui setup code
std::thread( ImGuiThread ).detach();
// Main loop
while ( !done )
{
// Omitted loop code
if ( snapshot_mutex.try_lock() )
{
if ( pending_snapshot.DrawData.Valid )
{
// reason we process the requests here is to make sure that "active_snapshot" cant have references to deleted textures
tex_queue.ProcessRequests( ImGui_ImplDX11_UpdateTexture, 3 );
active_snapshot.SnapUsingSwap( &pending_snapshot.DrawData, ImGui::GetTime() );
// mark as invalid so we wait for the next one and dont
// keep swapping back and forth in case the imgui thread is running slow
pending_snapshot.DrawData.Valid = false;
}
snapshot_mutex.unlock();
}
if ( active_snapshot.DrawData.Valid ) {
ImGui_ImplDX11_RenderDrawData( &active_snapshot.DrawData );
}
// Present
HRESULT hr = g_pSwapChain->Present( 1, 0 ); // Present with vsync
//HRESULT hr = g_pSwapChain->Present(0, 0); // Present without vsync
g_SwapChainOccluded = ( hr == DXGI_STATUS_OCCLUDED );
} Turning vsync on/off and increasing or decreasing the To create texture updates i decided to just slide around the The 2 issues ive come across are:
The race condition can be fixed by just adding lock guards to both of the functions, but the other one is a little trickier.
Here is my (dirty) modified version which addresses all these issues void ImTextureRenderQueue::QueueRequests( ImVector<ImTextureData*>* textures )
{
RequestsMutex.lock();
for ( ImTextureData* tex : *textures )
{
if ( tex->Status == ImTextureStatus_OK )
continue;
// Store a copy of the ImTextureData
// (code is a bit fragile/awkward because our helpers are not fulfilling constructor/destructor in a standard way)
Requests.resize( Requests.Size + 1, ImTextureRenderQueueRequest() );
ImTextureRenderQueueRequest& new_req = Requests.back();
new_req.TexCopy = *tex;
new_req.TexInMainThread = tex;
new_req.OriginalStatus = tex->Status;
// Acknowledge as created from Main Thread POV
if ( tex->Status == ImTextureStatus_WantCreate || tex->Status == ImTextureStatus_WantUpdates )
tex->Status = ImTextureStatus_OK;
// Need to set this immediately to prevent ImGui from dispatching more requests
if ( tex->Status == ImTextureStatus_WantDestroy )
{
tex->Status = ImTextureStatus_Destroyed;
tex->SetTexID( ImTextureID_Invalid );
tex->BackendUserData = NULL;
// In addition, we need to also invalidate all prior requests that attempted to modify this texture (excluding this one)
for ( auto i = 0; i < Requests.size() - 1; i++ )
{
auto& req = Requests[ i ];
if ( req.TexCopy.UniqueID == tex->UniqueID )
req.TexCopy.Status = ImTextureStatus_Destroyed;
}
}
}
// FIXME: is there no better way to do this?
// Validate that all requests are still for valid textures, this is to account for ImGui sometimes creating 2 texture in a row without it ever reaching the backend and thus just removing it from the list
for ( auto i = 0; i < Requests.size(); i++ )
{
auto& req = Requests[ i ];
if ( req.TexCopy.Status != ImTextureStatus_WantCreate && req.TexCopy.Status != ImTextureStatus_WantUpdates )
continue;
bool does_texture_exist = false;
for ( ImTextureData* tex : *textures )
{
if ( tex->UniqueID == req.TexCopy.UniqueID )
{
does_texture_exist = true;
break;
}
}
if ( does_texture_exist )
continue;
// Reaching this point means this texture was already removed internally, so its no longer safe to access the pixels
req.TexCopy.Status = ImTextureStatus_Destroyed;
}
RequestsMutex.unlock();
}
template<typename Backend_UpdateTextureFunc>
void ImTextureRenderQueue::ProcessRequests( Backend_UpdateTextureFunc update_texture_func, int in_flight_frames )
{
RequestsMutex.lock();
for ( auto i = 0; i < Requests.size(); i++ )
{
auto& req = Requests[ i ];
ImTextureData* tex_copy = &req.TexCopy;
if ( tex_copy->Status == ImTextureStatus_OK || tex_copy->Status == ImTextureStatus_Destroyed )
continue;
if ( tex_copy->Status == ImTextureStatus_WantDestroy && tex_copy->UnusedFrames <= in_flight_frames )
continue;
ImTextureStatus prev_status = tex_copy->Status;
// Call backend function
update_texture_func( tex_copy );
// Backend must honor immediately
if ( prev_status == ImTextureStatus_WantCreate || prev_status == ImTextureStatus_WantUpdates )
IM_ASSERT( tex_copy->Status == ImTextureStatus_OK );
if ( prev_status == ImTextureStatus_WantDestroy )
IM_ASSERT( tex_copy->Status == ImTextureStatus_Destroyed );
// Write back to main thread
if ( prev_status == ImTextureStatus_WantCreate )
{
// Make sure future requests will access the correct texture
for ( auto j = i + 1; j < Requests.size(); j++ )
{
auto& subsequent_req = Requests[ j ];
if ( subsequent_req.TexCopy.UniqueID == req.TexCopy.UniqueID )
{
subsequent_req.TexCopy.SetTexID( tex_copy->TexID );
subsequent_req.TexCopy.BackendUserData = tex_copy->BackendUserData;
}
}
req.TexInMainThread->SetTexID( tex_copy->TexID );
req.TexInMainThread->BackendUserData = tex_copy->BackendUserData;
}
}
Requests.resize( 0 );
RequestsMutex.unlock();
} Theoretically, since the status is immediately set without waiting for the render thread this would also remove the need for a separate
|
Thank you for your feedback. I'll first try to get myself a proper multi-threaded rendering test (without even considering texture update). I'm struggling to understand your code above. The render loop seems to be swapping continuously regardless of anything being rendering. It seems to be missing part. I'm not sure I understand the pending/active logic. Shouldn't it be modeled with a small array representing a round-robin queue? |
The goal of this setup is to keep both (but especially the rendering thread) going with minimal slowdowns. If the pending & active snapshots were merged into just 1 snapshot variable then the The render thread only ever wants to render the most recent frame so theres no point in storing any past imgui drawdata frames in a queue. Also regarding:
Thats what setting |
But it's not clearing framebuffer and it's always swapping: if ( active_snapshot.DrawData.Valid ) {
ImGui_ImplDX11_RenderDrawData( &active_snapshot.DrawData );
}
// Present
HRESULT hr = g_pSwapChain->Present( 1, 0 ); // Present with vsync
//HRESULT hr = g_pSwapChain->Present(0, 0); // Present without vsync
g_SwapChainOccluded = ( hr == DXGI_STATUS_OCCLUDED ); |
Oh yea sorry that was part of the const float clear_color_with_alpha[ 4 ] = { clear_color.x * clear_color.w, clear_color.y * clear_color.w, clear_color.z * clear_color.w, clear_color.w };
g_pd3dDeviceContext->OMSetRenderTargets( 1, &g_mainRenderTargetView, nullptr );
g_pd3dDeviceContext->ClearRenderTargetView( g_mainRenderTargetView, clear_color_with_alpha );
if ( snapshot_mutex.try_lock() )
{
if ( pending_snapshot.DrawData.Valid )
{
// reason we process the requests here is to make sure that "active_snapshot" cant have references to deleted textures
tex_queue.ProcessRequests( ImGui_ImplDX11_UpdateTexture, 3 );
active_snapshot.SnapUsingSwap( &pending_snapshot.DrawData, ImGui::GetTime() );
// mark as invalid so we wait for the next one and dont keep swapping back and forth in case the imgui thread is running slow
pending_snapshot.DrawData.Valid = false;
}
snapshot_mutex.unlock();
}
if ( active_snapshot.DrawData.Valid ) {
ImGui_ImplDX11_RenderDrawData( &active_snapshot.DrawData );
}
// Present
HRESULT hr = g_pSwapChain->Present( 1, 0 ); // Present with vsync
//HRESULT hr = g_pSwapChain->Present(0, 0); // Present without vsync
g_SwapChainOccluded = ( hr == DXGI_STATUS_OCCLUDED ); But im not sure what you mean by
Are you saying it shouldn't always call into |
Sorry I realize the terminology overlap, by "It's always swapping" I meant
|
Yea calling Present every time doesnt make sense in the scenario of a standalone imgui application. The way i use this code is in my game engine though, the game needs to be rendered every frame so maintaining the framebuffer until the next imgui update is not feasible. For example: for my game i have an imgui overlay which lists all entities and allows you to modify any field (health, speed etc.). In order for that to not cause race conditions imgui needs to run in the games logic thread.
|
I have been quite agonizing over this, as it would be a breaking change for 100% of apps updating backends, and in the name of supporting multi-threaded staged rendering for standard backends, which I haven't really expected would happen. I have instead added a pointer to the texture array in ImDrawData d16f151 tho it still rubs me off a little bit as it means rendering uses ImDrawData->Textures[] while backend shutdown uses GetPlatformIO()->Textures[]. I also had to go through a tangent verifying and making fixes/changes to ensure that both multi-atlas was supported in a same context and that it was possible for end-user/app/third-party extension to submit their own texture. |
Uh oh!
There was an error while loading. Please reload this page.
Version/Branch of Dear ImGui:
Branch: dynamic_fonts
Details:
Regarding the work-in-progress rewrite of textures & fonts system in
dynamic_fonts
.This is a topic dedicated to discussing the issue mentioned by @DucaRii in #8465 (comment)
I don't have a proposal just yet but I'm opening an issue to gather feedback from people using custom engine who use staged rendering, e.g. Update thread vs Render thread.
Outline of the current design as of today (2025/04/26) is described below.
It is likely we would need to change it.
ImGui::EndFrame()
updates a texture lists inplatform_io.Textures[]
.ImGui_ImplDX11_RenderDrawData()
are iterating this list to update dirty textures (see code below).Note: backends are also their exposing the function that "updates" a texture, e.g.
ImGui_ImplDX11_UpdateTexture()
the reasoning being that user application may want to process this update before the _RenderDrawData() function is called.A problem arises for people who aim to perform rendering from another thread.
The general suggestion until now is that we suggest use to keep a copy of the
ImDrawData
instances afterImGui::Render()
, which may be rendered later from another thread, while in the main/ui thread may already be processing another ImGui frame.Helpers such as this
ImDrawDataSnapshot
(#1860 (comment)) allow for easily preserving theImDrawData
contents without needing a do a costly deep-copy. This was based on the premise that all data necessary to rendering was self-contained inside theImDrawData
structure.This isn't the case anymore with that WIP design.
It is also the case that in the current WIP,
ImTextureData
is a input/output structure, as the backend is in charge of updating texture Status to acknowledge e.g. creation/update/destruction, and the backend is in charge of storing TexID and BackendUserData inside the texture.What is guaranteed:
Draft for implementing this
A theoretical solution below.
[ UI / Main Thread ]
io.Textures[]
lists, queue creates/update/destroy textures request into a format suitable for Render Task.ImTextureStatus_WantCreate
orImTextureStatus_WantUpdate
status:ImTextureData
struct somewhere for Render Task (*)ImTextureData*
pointer somewhere for Render Task. This will be used to write back TexID/BackendUserData during rendering. Because we control destroying textures, we now this pointer is going to be valid.tex->Status = ImTextureStatus_OK
immediately, for nextImGui::NewFrame()
to catch on it.ImTextureStatus_WantDestroy
status:tex->UnusedFrames
and your own rendering pipeline). if it is safe:ImTextureData
struct somewhere for Render Task (*)tex->Status = ImTextureStatus_Destroyed
immediately, for nextImGui::NewFrame()
to catch on it.tex->SetTexID(ImTextureID_Invalid)
immediately.(*) would need to deep-copy
ImTextureData::Updates[]
if using multi-dirty rects, but I suspect the singleUpdateRect
is enough.[ Render Task ]
tex->SetTexID()
andtex->BackendUserData = xxx
. This is safe because:ImTextureStatus_WantCreate
requests, and that stored value will be pulled via ImDrawCmd::GetTexID().I think this should work and be safe.
I'm writing this down as a first step, then I will try to implement it, then I will decide if it's worth making changes to main lib to make it more obviously / easy to implement, or investigate helpers.
I don't have a framework for staged rendering but I can partly simulate this by calling NewFrame() for Frame N+1 before running rendering for Frame N.
It may also make sense that we decide to store the
io.Textures[]
list somewhere else. The reason it wasn't in ImDrawData is because:ImDrawData
and their rendering order is user controlled.Screenshots/Video:
No response
Minimal, Complete and Verifiable Example code:
No response
The text was updated successfully, but these errors were encountered: