From d9220ad536441db1071289e7d3ddd918e5c32fb5 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Tue, 14 Aug 2018 20:28:25 +0100 Subject: [PATCH] Fixed bad deallocation in Texture Manager and interface to create textures from raw data: - Fixed rather embarrassing overlook in deallocation of data array passed from to CreateTexture from other function. - Changed CreateTexture to use cleanup function passed with data (optional and only needed if data need to be released). Keeping data allocation and deallocation code together should make bugs like this easier to spot and avoid. Additionally this allows for more generic usage. --- Source/ImGui/Private/ImGuiModuleManager.cpp | 2 +- Source/ImGui/Private/TextureManager.cpp | 14 ++++++-------- Source/ImGui/Private/TextureManager.h | 4 ++-- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Source/ImGui/Private/ImGuiModuleManager.cpp b/Source/ImGui/Private/ImGuiModuleManager.cpp index e3038e7..8ffa94c 100644 --- a/Source/ImGui/Private/ImGuiModuleManager.cpp +++ b/Source/ImGui/Private/ImGuiModuleManager.cpp @@ -72,7 +72,7 @@ void FImGuiModuleManager::LoadTextures() int Width, Height, Bpp; Fonts.GetTexDataAsRGBA32(&Pixels, &Width, &Height, &Bpp); - TextureIndex FontsTexureIndex = TextureManager.CreateTexture(FName{ "ImGuiModule_FontAtlas" }, Width, Height, Bpp, Pixels, false); + TextureIndex FontsTexureIndex = TextureManager.CreateTexture(FName{ "ImGuiModule_FontAtlas" }, Width, Height, Bpp, Pixels); // Set font texture index in ImGui. Fonts.TexID = ImGuiInterops::ToImTextureID(FontsTexureIndex); diff --git a/Source/ImGui/Private/TextureManager.cpp b/Source/ImGui/Private/TextureManager.cpp index 83674f9..7a96433 100644 --- a/Source/ImGui/Private/TextureManager.cpp +++ b/Source/ImGui/Private/TextureManager.cpp @@ -7,7 +7,7 @@ #include -TextureIndex FTextureManager::CreateTexture(const FName& Name, int32 Width, int32 Height, uint32 SrcBpp, uint8* SrcData, bool bDeleteSrcData) +TextureIndex FTextureManager::CreateTexture(const FName& Name, int32 Width, int32 Height, uint32 SrcBpp, uint8* SrcData, TFunction SrcDataCleanup) { checkf(FindTextureIndex(Name) == INDEX_NONE, TEXT("Trying to create texture using resource name '%s' that is already registered."), *Name.ToString()); @@ -19,12 +19,9 @@ TextureIndex FTextureManager::CreateTexture(const FName& Name, int32 Width, int3 // Update texture data. FUpdateTextureRegion2D* TextureRegion = new FUpdateTextureRegion2D(0, 0, 0, 0, Width, Height); - auto DataCleanup = [bDeleteSrcData](uint8* Data, const FUpdateTextureRegion2D* UpdateRegion) + auto DataCleanup = [SrcDataCleanup](uint8* Data, const FUpdateTextureRegion2D* UpdateRegion) { - if (bDeleteSrcData) - { - delete Data; - } + SrcDataCleanup(Data); delete UpdateRegion; }; Texture->UpdateTextureRegions(0, 1u, TextureRegion, SrcBpp * Width, SrcBpp, SrcData, DataCleanup); @@ -42,9 +39,10 @@ TextureIndex FTextureManager::CreatePlainTexture(const FName& Name, int32 Width, const uint32 SizeInBytes = SizeInPixels * Bpp; uint8* SrcData = new uint8[SizeInBytes]; std::fill(reinterpret_cast(SrcData), reinterpret_cast(SrcData) + SizeInPixels, ColorPacked); + auto SrcDataCleanup = [](uint8* Data) { delete[] Data; }; - // Create new texture from raw data (we created the buffer, so mark it for delete). - return CreateTexture(Name, Width, Height, Bpp, SrcData, true); + // Create new texture from raw data. + return CreateTexture(Name, Width, Height, Bpp, SrcData, SrcDataCleanup); } FTextureManager::FTextureEntry::FTextureEntry(const FName& InName, UTexture2D* InTexture) diff --git a/Source/ImGui/Private/TextureManager.h b/Source/ImGui/Private/TextureManager.h index f90d3be..c8df7a5 100644 --- a/Source/ImGui/Private/TextureManager.h +++ b/Source/ImGui/Private/TextureManager.h @@ -57,9 +57,9 @@ public: // @param Height - The texture height // @param SrcBpp - The size in bytes of one pixel // @param SrcData - The source data - // @param bDeleteSrcData - If true, we should delete source data after creating a texture + // @param SrcDataCleanup - Optional function called to release source data after texture is created (only needed, if data need to be released) // @returns The index of a texture that was created - TextureIndex CreateTexture(const FName& Name, int32 Width, int32 Height, uint32 SrcBpp, uint8* SrcData, bool bDeleteSrc = false); + TextureIndex CreateTexture(const FName& Name, int32 Width, int32 Height, uint32 SrcBpp, uint8* SrcData, TFunction SrcDataCleanup = [](uint8*) {}); // Create a plain texture. Throws exception if there is already a texture with that name. // @param Name - The texture name