Externals: Use Common ASSERT for IM_ASSERT

[Pokechu22]

The first commit calls the IMGUI_CHECKVERSION macro, which is intended to prevent issues like the one I caused after #10188 where the wrong version of imgui was linked. The second commit is also future-proofing.

The third commit switches to Common's ASSERT macro for the IM_ASSERT macro. This does mean imgui now depends on both common and fmt, but since imgui is only used by dolphinqt (and the cmakelists file is added by dolphin in the first place), I don't think this is a problem. In my opinion, it's better to use the same assert window for everything, rather than have the default assert/retry/ignore one appear sometimes.

Examples by removing the call to ImGui::CreateContext() in Renderer::InitializeImGui:

Before:

After:

I did attempt to avoid the direct dependency by declaring a global variable in imconfig.h to be modified, but that failed because imconfig.h is included in multiple locations.

Failed patch

diff --git a/Externals/imgui/imconfig.h b/Externals/imgui/imconfig.h
index f255601704..57c287f798 100644
--- a/Externals/imgui/imconfig.h
+++ b/Externals/imgui/imconfig.h
@@ -14,10 +14,30 @@
 
 #pragma once
 
+#include <assert.h>
+
+//---- Tip: You can add extra functions within the ImGui:: namespace, here or in your own headers files.
+namespace ImGui
+{
+  // condition, file, line, func
+  void (*g_dolphin_assert_handler)(const char*, const char*, int, const char*) = nullptr;
+}
+
 //---- Define assertion handler. Defaults to calling assert().
 // If your macro uses multiple statements, make sure is enclosed in a 'do { .. } while (0)' block so it can be used as a single statement.
-//#define IM_ASSERT(_EXPR)  MyAssert(_EXPR)
-//#define IM_ASSERT(_EXPR)  ((void)(_EXPR))     // Disable asserts
+#define IM_ASSERT(_EXPR)                                                                           \
+  do                                                                                               \
+  {                                                                                                \
+    if (ImGui::g_dolphin_assert_handler != nullptr)                                                \
+    {                                                                                              \
+      if (!(_EXPR))                                                                                  \
+        ImGui::g_dolphin_assert_handler(#_EXPR, __FILE__, __LINE__, __func__);                     \
+    }                                                                                              \
+    else                                                                                           \
+    {                                                                                              \
+      assert(_EXPR && "g_dolphin_assert_handler not set by Dolphin");                              \
+    }                                                                                              \
+  } while (0)
 
 //---- Define attributes of all API symbols declarations, e.g. for DLL under Windows
 // Using Dear ImGui via a shared library is not recommended, because of function call overhead and because we don't guarantee backward nor forward ABI compatibility.
@@ -113,11 +133,3 @@
 
 //---- Debug Tools: Enable slower asserts
 //#define IMGUI_DEBUG_PARANOID
-
-//---- Tip: You can add extra functions within the ImGui:: namespace, here or in your own headers files.
-/*
-namespace ImGui
-{
-    void MyFunction(const char* name, const MyMatrix44& v);
-}
-*/
diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp
index 861c69d985..78b6022482 100644
--- a/Source/Core/VideoCommon/RenderBase.cpp
+++ b/Source/Core/VideoCommon/RenderBase.cpp
@@ -971,6 +971,14 @@ void Renderer::RecordVideoMemory()
 
 bool Renderer::InitializeImGui()
 {
+  ImGui::g_dolphin_assert_handler = [](const char* cond, const char* file, int line,
+                                       const char* func) {
+    if (!PanicYesNoFmt("An ImGui error occurred.\n\n"
+                       "  Condition: {}\n  File: {}\n  Line: {}\n  Function: {}\n\n"
+                       "Ignore and continue?",
+                       cond, file, line, func))
+      Crash();
+  };
   if (!IMGUI_CHECKVERSION())
   {
     PanicAlertFmt("ImGui version check failed");

[JosJuice]

Are you sure "imgui is only used by dolphinqt" is correct? The imgui-rendered OSD messages and so on show up on Android...

Sounds reasonable other than that.

[Pokechu22]

Oops, that's wrong; there's also a dependency in VideoCommon:

dolphin/Source/Core/VideoCommon/CMakeLists.txt

Lines 134 to 142 in 7b8e846

	target_link_libraries(videocommon
	PUBLIC
	core
	PRIVATE
	fmt::fmt
	png
	xxhash
	imgui
	)

Still, VideoCommon depends on common (through core) and fmt, so I don't think adding this dependency is a problem.

There isn't an explicit dependency for android (but it looks like no android code directly references imgui, while some DolphinQt code and a lot of VideoCommon code does directly reference imgui).

[JosJuice]

Still, VideoCommon depends on common (through core) and fmt, so I don't think adding this dependency is a problem.

Yes, I agree. I was just confused by the PR description claiming that.

[AdmiralCurtiss]

You may want to leave a note somewhere visible that these changes need to be re-applied when updating imgui.

[Pokechu22]

Note that imgui itself documents the process:

dolphin/Externals/imgui/imconfig.h

Lines 1 to 13 in e58cf36

	//-----------------------------------------------------------------------------
	// COMPILE-TIME OPTIONS FOR DEAR IMGUI
	// Runtime options (clipboard callbacks, enabling various features, etc.) can generally be set via the ImGuiIO structure.
	// You can use ImGui::SetAllocatorFunctions() before calling ImGui::CreateContext() to rewire memory allocation functions.
	//-----------------------------------------------------------------------------
	// A) You may edit imconfig.h (and not overwrite it when updating Dear ImGui, or maintain a patch/rebased branch with your modifications to it)
	// B) or '#define IMGUI_USER_CONFIG "my_imgui_config.h"' in your project and then add directives in your own file without touching this template.
	//-----------------------------------------------------------------------------
	// You need to make sure that configuration settings are defined consistently _everywhere_ Dear ImGui is used, which include the imgui*.cpp
	// files but also _any_ of your code that uses Dear ImGui. This is because some compile-time options have an affect on data structures.
	// Defining those options in imconfig.h will ensure every compilation unit gets to see the same data structure layouts.
	// Call IMGUI_CHECKVERSION() from your .cpp files to verify that the data structures your files are using are matching the ones imgui.cpp is using.
	//-----------------------------------------------------------------------------

Since imgui doesn't include a CMakeLists.txt by default, we could create a custom imconfig.h and define it in the CMakeLists. (We'd also need to handle that define for MSVC.) Or we can just avoid overwriting it.

[AdmiralCurtiss]

Sure, but are you sure someone's gonna read that when they update? I'd just drop a read_when_updating_imgui.txt in the folder that tells you to not overwrite the imconfig.h, or something along those lines.

[Pokechu22]

I've added a README.txt for it.

[shuffle2]

intended to prevent issues like the one I caused after #10188 where the wrong version of imgui was linked.

ftr, this was my mistake (not really related to #10188 ) and fixed by #10219