Flix01 / imgui

Dear ImGui Addons Branch = plain unmodified dear imgui plus some extra addon.
https://github.com/Flix01/imgui/wiki/ImGui-Addons-Branch-Home
MIT License
396 stars 34 forks source link

unordered static allocation #38

Closed carsonbrownlee closed 6 years ago

carsonbrownlee commented 6 years ago

I made the following fix, some users were reporting issues with statically allocated variables used by imgui as initialization order is not guaranteed. Wrapping the context in a function fixes this.

diff --git a/apps/exampleViewer/common/imgui/imguifilesystem/imguifilesystem.cpp b/apps/exampleViewer/common/imgui/imguifilesystem/imguifilesystem.cpp
index 4e773818..938c1747 100644
--- a/apps/exampleViewer/common/imgui/imguifilesystem/imguifilesystem.cpp
+++ b/apps/exampleViewer/common/imgui/imguifilesystem/imguifilesystem.cpp
@@ -1483,13 +1483,17 @@ struct ImGuiFsDrawIconStruct {
     return drawIcon(extType,pOptionalColorOverride);
     }
 };
-static ImGuiFsDrawIconStruct MyImGuiFsDrawIconStruct;
+ImGuiFsDrawIconStruct& MyImGuiFsDrawIconStruct()
+{
+  static ImGuiFsDrawIconStruct singleton;
+  return singleton;
+}
 #ifndef IMGUIFS_NO_EXTRA_METHODS
 int FileGetExtensionType(const char* path) {
-    return MyImGuiFsDrawIconStruct.getExtensionType(strrchr(path,'.'));
+    return MyImGuiFsDrawIconStruct().getExtensionType(strrchr(path,'.'));
 }
 void FileGetExtensionTypesFromFilenames(ImVector<int>& fileExtensionTypesOut,const FilenameStringVector& fileNames)  {
-    MyImGuiFsDrawIconStruct.fillExtensionTypesFromFilenames(fileExtensionTypesOut,fileNames);
+    MyImGuiFsDrawIconStruct().fillExtensionTypesFromFilenames(fileExtensionTypesOut,fileNames);
 }
 #if (defined(__EMSCRIPTEN__) && defined(EMSCRIPTEN_SAVE_SHELL))
 bool FileDownload(const char* path,const char* optionalSaveFileName)    {
@@ -2053,7 +2057,7 @@ const char* ChooseFileMainMethod(Dialog& ist,const char* directory,const bool _i
         if (!isSelectFolderDialog)  {
             if (!fileFilterExtensionString || strlen(fileFilterExtensionString)==0) Directory::GetFiles(I.currentFolder,I.files,&I.fileNames,(Sorting)I.sortingMode);
             else                                        Directory::GetFiles(I.currentFolder,I.files,fileFilterExtensionString,NULL,&I.fileNames,(Sorting)I.sortingMode);
-            if (Dialog::DrawFileIconCallback) MyImGuiFsDrawIconStruct.fillExtensionTypesFromFilenames(I.fileExtensionTypes,I.fileNames);
+            if (Dialog::DrawFileIconCallback) MyImGuiFsDrawIconStruct().fillExtensionTypesFromFilenames(I.fileExtensionTypes,I.fileNames);
         }
         else {
             I.files.clear();I.fileNames.clear();I.fileExtensionTypes.clear();
Flix01 commented 6 years ago

Thanks for the fix!

some users were reporting issues with statically allocated variables used by imgui as initialization order is not guaranteed.

It should be interesting to understand when and why this is happening... it reminds me this old post here: https://github.com/Flix01/imgui/issues/21, but I'm not sure it's related to it.

... in any case I've manually patched imguifilesystem.cpp, keeping the static method inside the ImGuiFsDrawIconStruct struct, and calling it as ImGuiFsDrawIconStruct::Get().

Hope it works now :smile:

carsonbrownlee commented 6 years ago

Great thanks for the quick update! No one on our team have seen this before so it must be rare, but a user ran into it on a modern clang compiler. Apparently it's due to a statically declared variable in imguifilesystem calling a constructor which calls ImGUI calls that utilize the GImGUI variable in imgui.cpp, and GImGUI is initialized to a statically declared variable. Those functions were being called before the static variable was initialized in imgui.cpp, which then caused the crash.

Flix01 commented 6 years ago

OK, thanks for the detailed clarification! It can be useful to fix similar issues in the future.