From 3e3bc79adf002a4d539a96446c853ba9aeace44c Mon Sep 17 00:00:00 2001 From: Kawe Mazidjatari <48657826+Mauler125@users.noreply.github.com> Date: Tue, 27 Feb 2024 17:43:18 +0100 Subject: [PATCH] ImguiSystem: properly implement ImGui and fix several bugs Moved implementation to dedicated class 'CImguiSystem' with the following changes: * Added mutex for ImGui_ImplWin32_WndProcHandler(), the window proc is ran from a different thread than CImguiSystem::SampleFrame() is being called from. Needs a mutex as both functions interact with the input queue list. * Added init stages for debugging and preventing the system from running certain parts of the code if a certain stage hasn't been reached yet. * NULL out ImGuiContext::ConfigNavWindowingKeyNext , we don't need any nav window shortcuts as users don't need then in-game, and it also interferes with certain key binds some users have. * CImguiSystem::SampleFrame() (previously ImguiSystem_SampleFrame()), is now being called from CMaterialSystem::SwapBuffers(), as that function is called from the main thread while also getting updated during level loads/etc, so instead of switching between 2 static buffers for as long as the levels are being loaded, we imgui updates properly too and directly swap afterwards. * Removed hack in CGame::WindowProc() where we would only run windowproc if we aren't shutting the game down. This was needed as there was a chance of a crash at some rare cases, but this was simply due to a missing mutex which has been addressed and fixed as of this commit. * Lock mutex during init and shutdown. --- src/engine/host_state.cpp | 7 +- src/engine/sys_mainwind.cpp | 10 +- src/gameui/imgui_system.cpp | 126 ++++++++++++++++----- src/gameui/imgui_system.h | 65 ++++++++++- src/materialsystem/cmaterialsystem.cpp | 6 +- src/thirdparty/imgui/misc/imgui_snapshot.h | 1 + src/windows/id3dx.cpp | 7 +- src/windows/id3dx.h | 1 - 8 files changed, 169 insertions(+), 54 deletions(-) diff --git a/src/engine/host_state.cpp b/src/engine/host_state.cpp index 000d7db1..12f580c5 100644 --- a/src/engine/host_state.cpp +++ b/src/engine/host_state.cpp @@ -125,8 +125,6 @@ void CHostState::FrameUpdate(CHostState* pHostState, double flCurrentTime, float #endif // !CLIENT_DLL #ifndef DEDICATED RCONClient()->RunFrame(); - - ImguiSystem_SampleFrame(); #endif // !DEDICATED // Disable "warning C4611: interaction between '_setjmp' and C++ object destruction is non-portable" @@ -147,8 +145,7 @@ void CHostState::FrameUpdate(CHostState* pHostState, double flCurrentTime, float { Cbuf_Execute(); - HostStates_t oldState = g_pHostState->m_iCurrentState; - + const HostStates_t oldState = g_pHostState->m_iCurrentState; switch (g_pHostState->m_iCurrentState) { case HostStates_t::HS_NEW_GAME: @@ -452,7 +449,7 @@ void CHostState::State_NewGame(void) #ifndef CLIENT_DLL const bool bSplitScreenConnect = m_bSplitScreenConnect; - m_bSplitScreenConnect = 0; + m_bSplitScreenConnect = false; if (!g_pServerGameClients) // Init Game if it ain't valid. { diff --git a/src/engine/sys_mainwind.cpp b/src/engine/sys_mainwind.cpp index ba8a8d78..0aeb6f90 100644 --- a/src/engine/sys_mainwind.cpp +++ b/src/engine/sys_mainwind.cpp @@ -30,16 +30,10 @@ void CGame::PlayStartupVideos(void) //----------------------------------------------------------------------------- int CGame::WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { - if (!ImguiSystem_IsInitialized()) + if (!ImguiSystem()->IsInitialized()) return CGame__WindowProc(hWnd, uMsg, wParam, lParam); - const IEngine::EngineState_t state = g_pEngine->GetState(); - - if (state == IEngine::DLL_CLOSE || - state == IEngine::DLL_RESTART) - return CGame__WindowProc(hWnd, uMsg, wParam, lParam); - - ImGui_ImplWin32_WndProcHandler(hWnd, uMsg, wParam, lParam); + ImguiSystem()->MessageHandler(hWnd, uMsg, wParam, lParam); if (uMsg == WM_KEYDOWN || uMsg == WM_SYSKEYDOWN) { diff --git a/src/gameui/imgui_system.cpp b/src/gameui/imgui_system.cpp index 06598238..7506b590 100644 --- a/src/gameui/imgui_system.cpp +++ b/src/gameui/imgui_system.cpp @@ -1,4 +1,8 @@ //=============================================================================// +// +// Purpose: Dear ImGui engine implementation +// +//=============================================================================// #include "imgui/misc/imgui_snapshot.h" #include "engine/sys_mainwind.h" @@ -9,16 +13,23 @@ #include "imgui_system.h" -static ImDrawDataSnapshot s_imguiSnapshotData; -static bool s_imguiSystemInitialized = false; - -bool ImguiSystem_IsInitialized() +//----------------------------------------------------------------------------- +// Constructors/Destructors. +//----------------------------------------------------------------------------- +CImguiSystem::CImguiSystem() { - return s_imguiSystemInitialized; + m_systemInitState = ImguiSystemInitStage_e::IM_PENDING_INIT; } -bool ImguiSystem_Init() +//----------------------------------------------------------------------------- +// Initializes the imgui system. If this fails, false would be returned and the +// implementation won't run. +//----------------------------------------------------------------------------- +bool CImguiSystem::Init() { + Assert(ThreadInMainThread(), "CImguiSystem::Init() should only be called from the main thread!"); + Assert(m_systemInitState == ImguiSystemInitStage_e::IM_PENDING_INIT, "CImguiSystem::Init() called recursively?"); + /////////////////////////////////////////////////////////////////////////// IMGUI_CHECKVERSION(); ImGuiContext* const context = ImGui::CreateContext(); @@ -26,9 +37,12 @@ bool ImguiSystem_Init() if (!context) return false; + AUTO_LOCK(m_snapshotBufferMutex); + AUTO_LOCK(m_inputEventQueueMutex); + // This is required to disable the ctrl+tab menu as some users use this // shortcut for other things in-game. See: https://github.com/ocornut/imgui/issues/4828 - context->ConfigNavWindowingKeyNext = ImGuiMod_Shift | ImGuiKey_Space; + context->ConfigNavWindowingKeyNext = 0; context->ConfigNavWindowingKeyPrev = 0; ImGuiViewport* const vp = ImGui::GetMainViewport(); @@ -37,50 +51,81 @@ bool ImguiSystem_Init() ImGuiIO& io = ImGui::GetIO(); io.ConfigFlags |= ImGuiConfigFlags_IsSRGB; - const bool win32ImplInit = ImGui_ImplWin32_Init(g_pGame->GetWindow()); + if (!ImGui_ImplWin32_Init(g_pGame->GetWindow()) || + !ImGui_ImplDX11_Init(D3D11Device(), D3D11DeviceContext())) + { + Assert(0); - if (!win32ImplInit) + m_systemInitState = ImguiSystemInitStage_e::IM_INIT_FAILURE; return false; + } - const bool dx11ImplInit = ImGui_ImplDX11_Init(D3D11Device(), D3D11DeviceContext()); - - if (!dx11ImplInit) - return false; - - s_imguiSystemInitialized = true; + m_systemInitState = ImguiSystemInitStage_e::IM_SYSTEM_INIT; return true; } -void ImguiSystem_Shutdown() +//----------------------------------------------------------------------------- +// Shuts the imgui system down, frees all allocated buffers. +//----------------------------------------------------------------------------- +void CImguiSystem::Shutdown() { - if (!s_imguiSystemInitialized) + Assert(ThreadInMainThread(), "CImguiSystem::Shutdown() should only be called from the main thread!"); + Assert(m_systemInitState != ImguiSystemInitStage_e::IM_PENDING_INIT, "CImguiSystem::Shutdown() called recursively?"); + + // Nothing to shutdown. + if (m_systemInitState == ImguiSystemInitStage_e::IM_PENDING_INIT) return; - s_imguiSystemInitialized = false; + AUTO_LOCK(m_snapshotBufferMutex); + AUTO_LOCK(m_inputEventQueueMutex); + + m_systemInitState = ImguiSystemInitStage_e::IM_PENDING_INIT; ImGui_ImplDX11_Shutdown(); ImGui_ImplWin32_Shutdown(); + ImGui::DestroyContext(); - s_imguiSnapshotData.Clear(); + m_snapshotData.Clear(); } -void ImguiSystem_SwapBuffers() +//----------------------------------------------------------------------------- +// Copies currently drawn data into the snapshot buffer which is queued to be +// rendered in the render thread. This should only be called from the same +// thread SampleFrame() is being called from. +//----------------------------------------------------------------------------- +void CImguiSystem::SwapBuffers() { - if (!s_imguiSystemInitialized) + Assert(ThreadInMainThread(), "CImguiSystem::SwapBuffers() should only be called from the main thread!"); + + if (m_systemInitState < ImguiSystemInitStage_e::IM_FRAME_SAMPLED) return; + AUTO_LOCK(m_snapshotBufferMutex); ImDrawData* const drawData = ImGui::GetDrawData(); - if (drawData) - s_imguiSnapshotData.SnapUsingSwap(drawData, ImGui::GetTime()); + // Nothing has been drawn, nothing to swap + if (!drawData) + return; + + m_snapshotData.SnapUsingSwap(drawData, ImGui::GetTime()); + + if (m_systemInitState == ImguiSystemInitStage_e::IM_FRAME_SAMPLED) + m_systemInitState = ImguiSystemInitStage_e::IM_FRAME_SWAPPED; } -void ImguiSystem_SampleFrame() +//----------------------------------------------------------------------------- +// Draws the ImGui panels and applies all queued input events. +//----------------------------------------------------------------------------- +void CImguiSystem::SampleFrame() { - if (!s_imguiSystemInitialized) + Assert(ThreadInMainThread(), "CImguiSystem::SampleFrame() should only be called from the main thread!"); + + if (m_systemInitState == ImguiSystemInitStage_e::IM_PENDING_INIT) return; + AUTO_LOCK(m_inputEventQueueMutex); + ImGui_ImplDX11_NewFrame(); ImGui_ImplWin32_NewFrame(); @@ -94,12 +139,37 @@ void ImguiSystem_SampleFrame() ImGui::EndFrame(); ImGui::Render(); + + if (m_systemInitState == ImguiSystemInitStage_e::IM_SYSTEM_INIT) + m_systemInitState = ImguiSystemInitStage_e::IM_FRAME_SAMPLED; } -void ImguiSystem_RenderFrame() +//----------------------------------------------------------------------------- +// Renders the drawn frame out which has been swapped to the snapshot buffer. +//----------------------------------------------------------------------------- +void CImguiSystem::RenderFrame() { - if (!s_imguiSystemInitialized) + if (m_systemInitState < ImguiSystemInitStage_e::IM_FRAME_SWAPPED) return; - ImGui_ImplDX11_RenderDrawData(&s_imguiSnapshotData.DrawData); + AUTO_LOCK(m_snapshotBufferMutex); + ImGui_ImplDX11_RenderDrawData(&m_snapshotData.DrawData); +} + +//----------------------------------------------------------------------------- +// Window procedure handler. +//----------------------------------------------------------------------------- +LRESULT CImguiSystem::MessageHandler(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) +{ + AUTO_LOCK(ImguiSystem()->m_inputEventQueueMutex); + + extern IMGUI_IMPL_API LRESULT ImGui_ImplWin32_WndProcHandler(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam); + return ImGui_ImplWin32_WndProcHandler(hwnd, msg, wParam, lParam); +} + +static CImguiSystem s_imguiSystem; + +CImguiSystem* ImguiSystem() +{ + return &s_imguiSystem; } diff --git a/src/gameui/imgui_system.h b/src/gameui/imgui_system.h index b79a311f..4c6e701e 100644 --- a/src/gameui/imgui_system.h +++ b/src/gameui/imgui_system.h @@ -1,14 +1,67 @@ +//=============================================================================// +// +// Purpose: Dear ImGui engine implementation +// +//=============================================================================// #ifndef IMGUI_SYSTEM_H #define IMGUI_SYSTEM_H +#include "imgui/misc/imgui_snapshot.h" -extern bool ImguiSystem_IsInitialized(); +class CImguiSystem +{ +public: + CImguiSystem(); -extern bool ImguiSystem_Init(); -extern void ImguiSystem_Shutdown(); + bool Init(); + void Shutdown(); -extern void ImguiSystem_SwapBuffers(); + void SwapBuffers(); -extern void ImguiSystem_SampleFrame(); -extern void ImguiSystem_RenderFrame(); + void SampleFrame(); + void RenderFrame(); + + // statics: + static LRESULT MessageHandler(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam); + + // inlines: + inline bool IsInitialized() const + { + return m_systemInitState >= ImguiSystemInitStage_e::IM_SYSTEM_INIT; + }; + +private: + enum class ImguiSystemInitStage_e + { + // When the system failed to initialize, the stage would be set to + // this. + IM_INIT_FAILURE = -1, + + IM_PENDING_INIT, + IM_SYSTEM_INIT, + + // State gets set to this when the first frame has been sampled. + IM_FRAME_SAMPLED, + + // State gets set to this then buffers have been swapped for the first + // time. + IM_FRAME_SWAPPED + }; + + ImguiSystemInitStage_e m_systemInitState; + ImDrawDataSnapshot m_snapshotData; + + // Mutex used during swapping and rendering, we draw the windows in the + // main thread, and render it in the render thread. The only place this + // mutex is used is during snapshot swapping and during rendering + mutable CThreadFastMutex m_snapshotBufferMutex; + + // Mutex used between ImGui window procedure handling and drawing, see + // https://github.com/ocornut/imgui/issues/6895. In this engine the window + // is ran in thread separate from the main thread, therefore it needs a + // lock to control access as main calls SampleFrame(). + mutable CThreadFastMutex m_inputEventQueueMutex; +}; + +CImguiSystem* ImguiSystem(); #endif // IMGUI_SYSTEM_H diff --git a/src/materialsystem/cmaterialsystem.cpp b/src/materialsystem/cmaterialsystem.cpp index db64393f..de2ef6aa 100644 --- a/src/materialsystem/cmaterialsystem.cpp +++ b/src/materialsystem/cmaterialsystem.cpp @@ -122,7 +122,7 @@ void* __fastcall DispatchDrawCall(int64_t a1, uint64_t a2, int a3, int a4, int64 //--------------------------------------------------------------------------------- ssize_t SpinPresent(void) { - ImguiSystem_RenderFrame(); + ImguiSystem()->RenderFrame(); const ssize_t val = v_SpinPresent(); return val; @@ -130,7 +130,9 @@ ssize_t SpinPresent(void) void* CMaterialSystem::SwapBuffers(CMaterialSystem* pMatSys) { - ImguiSystem_SwapBuffers(); + ImguiSystem()->SampleFrame(); + ImguiSystem()->SwapBuffers(); + return CMaterialSystem__SwapBuffers(pMatSys); } diff --git a/src/thirdparty/imgui/misc/imgui_snapshot.h b/src/thirdparty/imgui/misc/imgui_snapshot.h index c709e302..052fd75f 100644 --- a/src/thirdparty/imgui/misc/imgui_snapshot.h +++ b/src/thirdparty/imgui/misc/imgui_snapshot.h @@ -3,6 +3,7 @@ // snapshot.SnapUsingSwap(ImGui::GetDrawData(), ImGui::GetTime()); // [...] // ImGui_ImplDX11_RenderDrawData(&snapshot.DrawData); +#pragma once // FIXME: Could store an ID in ImDrawList to make this easier for user. #include "imgui_internal.h" // ImPool<>, ImHashData diff --git a/src/windows/id3dx.cpp b/src/windows/id3dx.cpp index f6727648..89d54acc 100644 --- a/src/windows/id3dx.cpp +++ b/src/windows/id3dx.cpp @@ -350,8 +350,8 @@ void DirectX_Init() Error(eDLL_T::COMMON, 0xBAD0C0DE, "Failed to detour process: error code = %08x\n", hr); } - if (!ImguiSystem_Init()) - Error(eDLL_T::MS, 0, "ImGui system initialization failed!\n"); + if (!ImguiSystem()->Init()) + Error(eDLL_T::COMMON, 0, "ImguiSystem()->Init() failed!\n"); } void DirectX_Shutdown() @@ -371,8 +371,7 @@ void DirectX_Shutdown() // Commit the transaction DetourTransactionCommit(); - if (ImguiSystem_IsInitialized()) - ImguiSystem_Shutdown(); + ImguiSystem()->Shutdown(); } void VDXGI::GetAdr(void) const diff --git a/src/windows/id3dx.h b/src/windows/id3dx.h index c531c89c..6031b84b 100644 --- a/src/windows/id3dx.h +++ b/src/windows/id3dx.h @@ -8,7 +8,6 @@ void DirectX_Init(); void DirectX_Shutdown(); -extern LRESULT ImGui_ImplWin32_WndProcHandler(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam); extern HRESULT __stdcall Present(IDXGISwapChain* pSwapChain, UINT nSyncInterval, UINT nFlags); extern bool LoadTextureBuffer(unsigned char* buffer, int len, ID3D11ShaderResourceView** out_srv, int* out_width, int* out_height);