From 71dd082f6962b0e8131b402b8c85688ce2b9ae7a Mon Sep 17 00:00:00 2001 From: Sebastian Date: Sat, 3 Aug 2019 17:45:53 +0100 Subject: [PATCH] Changed FImGuiContextManager to prevent moving created context proxies and to allow other objects with the same or shorter lifespan safely use cached pointers to those resources. - Disabled move semantics in FImGuiContextProxy and removed code related to moving ImGui context. - Changed FImGuiContextManager to store proxies as unique pointers. --- Source/ImGui/Private/ImGuiContextManager.cpp | 10 +++--- Source/ImGui/Private/ImGuiContextManager.h | 32 +++++-------------- Source/ImGui/Private/ImGuiContextProxy.cpp | 28 ++++++++--------- Source/ImGui/Private/ImGuiContextProxy.h | 33 ++++---------------- 4 files changed, 32 insertions(+), 71 deletions(-) diff --git a/Source/ImGui/Private/ImGuiContextManager.cpp b/Source/ImGui/Private/ImGuiContextManager.cpp index 388e60f..b3aba70 100644 --- a/Source/ImGui/Private/ImGuiContextManager.cpp +++ b/Source/ImGui/Private/ImGuiContextManager.cpp @@ -88,7 +88,7 @@ void FImGuiContextManager::Tick(float DeltaSeconds) auto& ContextData = Pair.Value; if (ContextData.CanTick()) { - ContextData.ContextProxy.Tick(DeltaSeconds); + ContextData.ContextProxy->Tick(DeltaSeconds); } else { @@ -129,7 +129,7 @@ FImGuiContextManager::FContextData& FImGuiContextManager::GetEditorContextData() if (UNLIKELY(!Data)) { Data = &Contexts.Emplace(Utilities::EDITOR_CONTEXT_INDEX, FContextData{ GetEditorContextName(), Utilities::EDITOR_CONTEXT_INDEX, DrawMultiContextEvent, FontAtlas, -1 }); - ContextProxyCreatedEvent.Broadcast(Utilities::EDITOR_CONTEXT_INDEX, Data->ContextProxy); + ContextProxyCreatedEvent.Broadcast(Utilities::EDITOR_CONTEXT_INDEX, *Data->ContextProxy); } return *Data; @@ -144,7 +144,7 @@ FImGuiContextManager::FContextData& FImGuiContextManager::GetStandaloneWorldCont if (UNLIKELY(!Data)) { Data = &Contexts.Emplace(Utilities::STANDALONE_GAME_CONTEXT_INDEX, FContextData{ GetWorldContextName(), Utilities::STANDALONE_GAME_CONTEXT_INDEX, DrawMultiContextEvent, FontAtlas }); - ContextProxyCreatedEvent.Broadcast(Utilities::STANDALONE_GAME_CONTEXT_INDEX, Data->ContextProxy); + ContextProxyCreatedEvent.Broadcast(Utilities::STANDALONE_GAME_CONTEXT_INDEX, *Data->ContextProxy); } return *Data; @@ -185,7 +185,7 @@ FImGuiContextManager::FContextData& FImGuiContextManager::GetWorldContextData(co if (UNLIKELY(!Data)) { Data = &Contexts.Emplace(Index, FContextData{ GetWorldContextName(World), Index, DrawMultiContextEvent, FontAtlas, WorldContext->PIEInstance }); - ContextProxyCreatedEvent.Broadcast(Index, Data->ContextProxy); + ContextProxyCreatedEvent.Broadcast(Index, *Data->ContextProxy); } else { @@ -196,7 +196,7 @@ FImGuiContextManager::FContextData& FImGuiContextManager::GetWorldContextData(co if (UNLIKELY(!Data)) { Data = &Contexts.Emplace(Index, FContextData{ GetWorldContextName(World), Index, DrawMultiContextEvent, FontAtlas }); - ContextProxyCreatedEvent.Broadcast(Index, Data->ContextProxy); + ContextProxyCreatedEvent.Broadcast(Index, *Data->ContextProxy); } #endif diff --git a/Source/ImGui/Private/ImGuiContextManager.h b/Source/ImGui/Private/ImGuiContextManager.h index a2a3208..99f1b6e 100644 --- a/Source/ImGui/Private/ImGuiContextManager.h +++ b/Source/ImGui/Private/ImGuiContextManager.h @@ -34,25 +34,25 @@ public: #if WITH_EDITOR // Get or create editor ImGui context proxy. - FORCEINLINE FImGuiContextProxy& GetEditorContextProxy() { return GetEditorContextData().ContextProxy; } + FORCEINLINE FImGuiContextProxy& GetEditorContextProxy() { return *GetEditorContextData().ContextProxy; } #endif #if !WITH_EDITOR // Get or create standalone game ImGui context proxy. - FORCEINLINE FImGuiContextProxy& GetWorldContextProxy() { return GetStandaloneWorldContextData().ContextProxy; } + FORCEINLINE FImGuiContextProxy& GetWorldContextProxy() { return *GetStandaloneWorldContextData().ContextProxy; } #endif // Get or create ImGui context proxy for given world. - FORCEINLINE FImGuiContextProxy& GetWorldContextProxy(const UWorld& World) { return GetWorldContextData(World).ContextProxy; } + FORCEINLINE FImGuiContextProxy& GetWorldContextProxy(const UWorld& World) { return *GetWorldContextData(World).ContextProxy; } // Get or create ImGui context proxy for given world. Additionally get context index for that proxy. - FORCEINLINE FImGuiContextProxy& GetWorldContextProxy(const UWorld& World, int32& OutContextIndex) { return GetWorldContextData(World, &OutContextIndex).ContextProxy; } + FORCEINLINE FImGuiContextProxy& GetWorldContextProxy(const UWorld& World, int32& OutContextIndex) { return *GetWorldContextData(World, &OutContextIndex).ContextProxy; } // Get context proxy by index, or null if context with that index doesn't exist. FORCEINLINE FImGuiContextProxy* GetContextProxy(int32 ContextIndex) { FContextData* Data = Contexts.Find(ContextIndex); - return Data ? &(Data->ContextProxy) : nullptr; + return Data ? Data->ContextProxy.Get() : nullptr; } // Delegate called for all contexts in manager, right after calling context specific draw event. Allows listeners @@ -66,38 +66,20 @@ public: private: -#if WITH_EDITOR - struct FContextData { FContextData(const FString& ContextName, int32 ContextIndex, FSimpleMulticastDelegate& SharedDrawEvent, ImFontAtlas& FontAtlas, int32 InPIEInstance = -1) : PIEInstance(InPIEInstance) - , ContextProxy(ContextName, ContextIndex, &SharedDrawEvent, &FontAtlas) + , ContextProxy(new FImGuiContextProxy(ContextName, ContextIndex, &SharedDrawEvent, &FontAtlas)) { } FORCEINLINE bool CanTick() const { return PIEInstance < 0 || GEngine->GetWorldContextFromPIEInstance(PIEInstance); } int32 PIEInstance = -1; - FImGuiContextProxy ContextProxy; + TUniquePtr ContextProxy; }; -#else - - struct FContextData - { - FContextData(const FString& ContextName, int32 ContextIndex, FSimpleMulticastDelegate& SharedDrawEvent, ImFontAtlas& FontAtlas) - : ContextProxy(ContextName, ContextIndex, &SharedDrawEvent, &FontAtlas) - { - } - - FORCEINLINE bool CanTick() const { return true; } - - FImGuiContextProxy ContextProxy; - }; - -#endif // WITH_EDITOR - void OnWorldTickStart(ELevelTick TickType, float DeltaSeconds); #if ENGINE_COMPATIBILITY_WITH_WORLD_POST_ACTOR_TICK diff --git a/Source/ImGui/Private/ImGuiContextProxy.cpp b/Source/ImGui/Private/ImGuiContextProxy.cpp index bf21891..ad44234 100644 --- a/Source/ImGui/Private/ImGuiContextProxy.cpp +++ b/Source/ImGui/Private/ImGuiContextProxy.cpp @@ -41,19 +41,6 @@ namespace } } -FImGuiContextProxy::FImGuiContextPtr::~FImGuiContextPtr() -{ - if (Context) - { - // Setting this as a current context. is still required in the current framework version to properly shutdown - // and save data. - ImGui::SetCurrentContext(Context); - - // Save context data and destroy. - ImGui::DestroyContext(Context); - } -} - FImGuiContextProxy::FImGuiContextProxy(const FString& InName, int32 InContextIndex, FSimpleMulticastDelegate* InSharedDrawEvent, ImFontAtlas* InFontAtlas) : Name(InName) , ContextIndex(InContextIndex) @@ -61,7 +48,7 @@ FImGuiContextProxy::FImGuiContextProxy(const FString& InName, int32 InContextInd , IniFilename(TCHAR_TO_ANSI(*GetIniFile(InName))) { // Create context. - Context = FImGuiContextPtr(ImGui::CreateContext(InFontAtlas)); + Context = ImGui::CreateContext(InFontAtlas); // Set this context in ImGui for initialization (any allocations will be tracked in this context). SetAsCurrent(); @@ -84,6 +71,19 @@ FImGuiContextProxy::FImGuiContextProxy(const FString& InName, int32 InContextInd BeginFrame(); } +FImGuiContextProxy::~FImGuiContextProxy() +{ + if (Context) + { + // It seems that to properly shutdown context we need to set it as the current one (at least in this framework + // version), even though we can pass it to the destroy function. + SetAsCurrent(); + + // Save context data and destroy. + ImGui::DestroyContext(Context); + } +} + void FImGuiContextProxy::DrawEarlyDebug() { if (bIsFrameStarted && !bIsDrawEarlyDebugCalled) diff --git a/Source/ImGui/Private/ImGuiContextProxy.h b/Source/ImGui/Private/ImGuiContextProxy.h index 2d3e738..2cc0030 100644 --- a/Source/ImGui/Private/ImGuiContextProxy.h +++ b/Source/ImGui/Private/ImGuiContextProxy.h @@ -17,37 +17,16 @@ // broadcasts draw events to allow listeners draw their controls. After update it stores draw data. class FImGuiContextProxy { - class FImGuiContextPtr - { - public: - - FImGuiContextPtr() = default; - FImGuiContextPtr(ImGuiContext* InContext) : Context(InContext) {} - - FImGuiContextPtr(const FImGuiContextPtr&) = delete; - FImGuiContextPtr& operator=(const FImGuiContextPtr&) = delete; - - FImGuiContextPtr(FImGuiContextPtr&& Other) : Context(Other.Context) { Other.Context = nullptr; } - FImGuiContextPtr& operator=(FImGuiContextPtr&& Other) { std::swap(Context, Other.Context); return *this; } - - ~FImGuiContextPtr(); - - ImGuiContext* Get() const { return Context; } - - private: - - ImGuiContext* Context = nullptr; - }; - public: FImGuiContextProxy(const FString& Name, int32 InContextIndex, FSimpleMulticastDelegate* InSharedDrawEvent, ImFontAtlas* InFontAtlas); + ~FImGuiContextProxy(); FImGuiContextProxy(const FImGuiContextProxy&) = delete; FImGuiContextProxy& operator=(const FImGuiContextProxy&) = delete; - FImGuiContextProxy(FImGuiContextProxy&&) = default; - FImGuiContextProxy& operator=(FImGuiContextProxy&&) = default; + FImGuiContextProxy(FImGuiContextProxy&&) = delete; + FImGuiContextProxy& operator=(FImGuiContextProxy&&) = delete; // Get the name of this context. const FString& GetName() const { return Name; } @@ -60,10 +39,10 @@ public: const FImGuiInputState& GetInputState() const { return InputState; } // Is this context the current ImGui context. - bool IsCurrentContext() const { return ImGui::GetCurrentContext() == Context.Get(); } + bool IsCurrentContext() const { return ImGui::GetCurrentContext() == Context; } // Set this context as current ImGui context. - void SetAsCurrent() { ImGui::SetCurrentContext(Context.Get()); } + void SetAsCurrent() { ImGui::SetCurrentContext(Context); } // Context display size (read once per frame during context update). const FVector2D& GetDisplaySize() const { return DisplaySize; } @@ -102,7 +81,7 @@ private: void BroadcastWorldDebug(); void BroadcastMultiContextDebug(); - FImGuiContextPtr Context; + ImGuiContext* Context; FVector2D DisplaySize = FVector2D::ZeroVector;