Add ability to run the imgui UI on mac (#25090)

* Update imgui compile flags for mac

* Apple updates for opengl version. Runs if in main, but separate thread still crashes

* Add ability to run separate event loop for linux platforms, update platform manager event loop to detect is running or not, fix darwin double-stop of event loop

* Restyle

* Use a timed wait for the status update of the semaphore

* Undo timed wait - it uses absolute times, so too much of a headache

* Restyle

* Undo platform updates, use a main loop delegate

* Restyle

* Typo fix

* Updated a TODO comment

* Another comment update

* Update shutdown logic: everything UI driven first, main loop handles chip loop. Based on side discussion with Boris

* Restyle

* Slight comment update

* Typo update

* Comment update

examples/lighting-app/linux/main.cpp

@@ -77,6 +77,41 @@ void ApplicationInit()
 #endif
 }
 
+#if defined(CHIP_IMGUI_ENABLED) && CHIP_IMGUI_ENABLED
+
+class UiAppMainLoopImplementation : public AppMainLoopImplementation
+{
+public:
+    void RunMainLoop() override;
+    void SignalSafeStopMainLoop() override;
+};
+
+void UiAppMainLoopImplementation::RunMainLoop()
+{
+    // Guaranteed to be on the main task (no chip event loop started yet)
+    example::Ui::Init();
+
+    // Platform event loop will be on a separate thread,
+    // while the event UI loop will be on the main thread.
+    chip::DeviceLayer::PlatformMgr().StartEventLoopTask();
+
+    // StopEventLoop will stop the loop below. It is called
+    // from within SignalSafeStopMainLoop below and
+    // UI knows how to stop itself if windows are closed.
+    example::Ui::EventLoop();
+
+    // Stop the chip main loop as well. This is expected to
+    // wait for the task to finish.
+    chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
+}
+
+void UiAppMainLoopImplementation::SignalSafeStopMainLoop()
+{
+    example::Ui::StopEventLoop();
+}
+
+#endif
+
 int main(int argc, char * argv[])
 {
     if (ChipLinuxAppInit(argc, argv) != 0)
@@ -87,13 +122,10 @@ int main(int argc, char * argv[])
     LightingMgr().Init();
 
 #if defined(CHIP_IMGUI_ENABLED) && CHIP_IMGUI_ENABLED
-    example::Ui::Start();
-#endif
-
+    UiAppMainLoopImplementation loop;
+    ChipLinuxAppMainLoop(&loop);
+#else
     ChipLinuxAppMainLoop();
-
-#if defined(CHIP_IMGUI_ENABLED) && CHIP_IMGUI_ENABLED
-    example::Ui::Stop();
 #endif
 
     return 0;

examples/lighting-app/linux/ui.cpp

@@ -388,10 +388,18 @@ void UiInit(SDL_GLContext * gl_context, SDL_Window ** window)
         return;
     }
 
+#if defined(__APPLE__)
+    SDL_GL_SetAttribute(SDL_GL_CONTEXT_FLAGS, SDL_GL_CONTEXT_FORWARD_COMPATIBLE_FLAG); // Always required on Mac
+    SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);
+    SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 3);
+    SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 2);
+#else
     SDL_GL_SetAttribute(SDL_GL_CONTEXT_FLAGS, 0);
     SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);
     SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 3);
     SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 0);
+#endif
+
 #ifdef SDL_HINT_IME_SHOW_UI
     SDL_SetHint(SDL_HINT_IME_SHOW_UI, "1");
 #endif
@@ -400,8 +408,7 @@ void UiInit(SDL_GLContext * gl_context, SDL_Window ** window)
     SDL_GL_SetAttribute(SDL_GL_DEPTH_SIZE, 24);
     SDL_GL_SetAttribute(SDL_GL_STENCIL_SIZE, 8);
     SDL_WindowFlags window_flags = (SDL_WindowFlags)(SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE | SDL_WINDOW_ALLOW_HIGHDPI);
-    *window     = SDL_CreateWindow("Dear ImGui SDL2+OpenGL3 example", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 1280, 720,
-                               window_flags);
+    *window     = SDL_CreateWindow("Light UI", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 1280, 720, window_flags);
     *gl_context = SDL_GL_CreateContext(*window);
     SDL_GL_MakeCurrent(*window, *gl_context);
     SDL_GL_SetSwapInterval(1); // Enable vsync
@@ -414,13 +421,11 @@ void UiInit(SDL_GLContext * gl_context, SDL_Window ** window)
     // io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard;     // Enable Keyboard Controls
     // io.ConfigFlags |= ImGuiConfigFlags_NavEnableGamepad;      // Enable Gamepad Controls
 
-    // Setup Dear ImGui style
     ImGui::StyleColorsDark();
-    // ImGui::StyleColorsLight();
 
     // Setup Platform/Renderer backends
     ImGui_ImplSDL2_InitForOpenGL(*window, *gl_context);
-    ImGui_ImplOpenGL3_Init("#version 130");
+    ImGui_ImplOpenGL3_Init();
 }
 
 void UiShutdown(SDL_GLContext * gl_context, SDL_Window ** window)
@@ -434,8 +439,18 @@ void UiShutdown(SDL_GLContext * gl_context, SDL_Window ** window)
     SDL_Quit();
 }
 
-void UiLoop()
+} // namespace
+
+void Init()
+{
+    // Init inside the "main" thread, so that it can access globals
+    // properly (for QR code and such)
+    gDeviceState.Init();
+}
+
+void EventLoop()
 {
+    gUiRunning = true;
     SDL_GLContext gl_context;
     SDL_Window * window = nullptr;
 
@@ -449,14 +464,11 @@ void UiLoop()
         while (SDL_PollEvent(&event))
         {
             ImGui_ImplSDL2_ProcessEvent(&event);
-            if (event.type == SDL_QUIT)
+            if ((event.type == SDL_QUIT) ||
+                (event.type == SDL_WINDOWEVENT && event.window.event == SDL_WINDOWEVENT_CLOSE &&
+                 event.window.windowID == SDL_GetWindowID(window)))
             {
-                chip::Server::GetInstance().DispatchShutDownAndStopEventLoop();
-            }
-            if (event.type == SDL_WINDOWEVENT && event.window.event == SDL_WINDOWEVENT_CLOSE &&
-                event.window.windowID == SDL_GetWindowID(window))
-            {
-                chip::Server::GetInstance().DispatchShutDownAndStopEventLoop();
+                StopEventLoop();
             }
         }
 
@@ -481,25 +493,9 @@ void UiLoop()
     ChipLogProgress(AppServer, "UI thread Stopped...");
 }
 
-static std::thread gUiThread;
-
-} // namespace
-
-void Start()
-{
-    // Init inside the "main" thread, so that it can access globals
-    // proparly (for QR code and such)
-    gDeviceState.Init();
-
-    gUiRunning = true;
-    std::thread uiThread(&UiLoop);
-    gUiThread.swap(uiThread);
-}
-
-void Stop()
+void StopEventLoop()
 {
     gUiRunning = false;
-    gUiThread.join();
 }
 
 } // namespace Ui

examples/lighting-app/linux/ui.h

@@ -20,9 +20,16 @@
 namespace example {
 namespace Ui {
 
-void Start();
+// MUST be called from within the main thread, to access CHIP global
+// data
+void Init();
 
-void Stop();
+// MAC requires this to be run from the main event loop. Linux supports
+// running this from a thread
+void EventLoop();
+
+// Flags the UI Event loop to stop
+void StopEventLoop();
 
 } // namespace Ui
 } // namespace example

examples/platform/linux/AppMain.cpp

@@ -101,6 +101,8 @@ static constexpr uint8_t kWiFiStartCheckAttempts    = 5;
 #endif
 
 namespace {
+AppMainLoopImplementation * gMainLoopImplementation = nullptr;
+
 // To hold SPAKE2+ verifier, discriminator, passcode
 LinuxCommissionableDataProvider gCommissionableDataProvider;
 
@@ -133,7 +135,14 @@ void Cleanup()
 #if !defined(ENABLE_CHIP_SHELL)
 void StopSignalHandler(int signal)
 {
-    Server::GetInstance().DispatchShutDownAndStopEventLoop();
+    if (gMainLoopImplementation != nullptr)
+    {
+        gMainLoopImplementation->SignalSafeStopMainLoop();
+    }
+    else
+    {
+        Server::GetInstance().DispatchShutDownAndStopEventLoop();
+    }
 }
 #endif // !defined(ENABLE_CHIP_SHELL)
 
@@ -331,8 +340,10 @@ int ChipLinuxAppInit(int argc, char * const argv[], OptionSet * customOptions)
     return 0;
 }
 
-void ChipLinuxAppMainLoop()
+void ChipLinuxAppMainLoop(AppMainLoopImplementation * impl)
 {
+    gMainLoopImplementation = impl;
+
     static chip::CommonCaseDeviceServerInitParams initParams;
     VerifyOrDie(initParams.InitializeStaticResourcesBeforeServerInit() == CHIP_NO_ERROR);
 
@@ -406,7 +417,16 @@ void ChipLinuxAppMainLoop()
     signal(SIGINT, StopSignalHandler);
     signal(SIGTERM, StopSignalHandler);
 #endif // !defined(ENABLE_CHIP_SHELL)
-    DeviceLayer::PlatformMgr().RunEventLoop();
+
+    if (impl != nullptr)
+    {
+        impl->RunMainLoop();
+    }
+    else
+    {
+        DeviceLayer::PlatformMgr().RunEventLoop();
+    }
+    gMainLoopImplementation = nullptr;
 
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
     ShutdownCommissioner();

examples/platform/linux/AppMain.h

@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include <app/server/Server.h>
 #include <controller/CHIPDeviceController.h>
 #include <controller/CommissionerDiscoveryController.h>
 #include <lib/core/CHIPError.h>
@@ -28,7 +29,51 @@
 #include "Options.h"
 
 int ChipLinuxAppInit(int argc, char * const argv[], chip::ArgParser::OptionSet * customOptions = nullptr);
-void ChipLinuxAppMainLoop();
+
+/**
+ * A main loop implementation describes how an application main loop is to be
+ * run:
+ *    - how to execute the main loop
+ *    - what to do to stop it (inside a signal handler - CTRL+C is captured
+ *      by the main loop function)
+ */
+class AppMainLoopImplementation
+{
+public:
+    virtual ~AppMainLoopImplementation() = default;
+    /**
+     * Execute main loop. Generally should have at least some
+     * `DeviceLayer::PlatformMgr().RunEventLoop();` or equivalent setup
+     *
+     * This is expected to RUN and BLOCK until SignalSafeStopMainLoop is
+     * called or some internal close logic is run (e.g. a UI may
+     * stop when the window close button is clicked.)
+     */
+    virtual void RunMainLoop() = 0;
+
+    /**
+     * Stop the above `RunMainLoop` function.
+     *
+     * Generally should contain at least a
+     *    Server::GetInstance().DispatchShutDownAndStopEventLoop()
+     */
+    virtual void SignalSafeStopMainLoop() = 0;
+};
+
+class DefaultAppMainLoopImplementation : public AppMainLoopImplementation
+{
+public:
+    void RunMainLoop() override { chip::DeviceLayer::PlatformMgr().RunEventLoop(); }
+    void SignalSafeStopMainLoop() override { chip::Server::GetInstance().DispatchShutDownAndStopEventLoop(); }
+};
+
+/**
+ * Start up the Linux app and use the provided main loop for event processing.
+ *
+ * If no main loop implementation is provided, an equivalent of
+ * DefaultAppMainLoopImplementation will be used.
+ */
+void ChipLinuxAppMainLoop(AppMainLoopImplementation * impl = nullptr);
 
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
 

examples/platform/nxp/se05x/linux/AppMain.cpp

@@ -361,7 +361,7 @@ struct CommonCaseDeviceServerInitParams_Se05x : public CommonCaseDeviceServerIni
 
 #endif
 
-void ChipLinuxAppMainLoop()
+void ChipLinuxAppMainLoop(AppMainLoopImplementation * impl)
 {
 #ifdef ENABLE_HSM_EC_KEY
     static CommonCaseDeviceServerInitParams_Se05x initParams;
@@ -435,7 +435,14 @@ void ChipLinuxAppMainLoop()
 
     ApplicationInit();
 
-    DeviceLayer::PlatformMgr().RunEventLoop();
+    if (impl != nullptr)
+    {
+        impl->RunMainLoop();
+    }
+    else
+    {
+        DeviceLayer::PlatformMgr().RunEventLoop();
+    }
 
 #if CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
     ShutdownCommissioner();

examples/platform/tizen/TizenServiceAppMain.cpp

@@ -79,6 +79,11 @@ void TizenServiceAppMain::AppTerminated()
     ChipLogProgress(NotSpecified, "Tizen app terminated");
 }
 
+static void TizenMainLoopWrapper()
+{
+    ChipLinuxAppMainLoop();
+}
+
 void TizenServiceAppMain::AppControl(app_control_h app_control)
 {
     ChipLogProgress(NotSpecified, "Tizen app control");
@@ -91,7 +96,7 @@ void TizenServiceAppMain::AppControl(app_control_h app_control)
             return;
         }
 
-        mLinuxThread = std::thread(ChipLinuxAppMainLoop);
+        mLinuxThread = std::thread(TizenMainLoopWrapper);
         initialized  = true;
     }
 }

third_party/imgui/BUILD.gn

@@ -23,8 +23,19 @@ config("imgui_config") {
   defines = [ "CHIP_IMGUI_ENABLED=1" ]
 
   # FUTURE: these should be parsed from `sdl2-config --cflags`
-  include_dirs += [ "/usr/include/SDL2" ]
-  defines += [ "_REENTRANT" ]
+  if (current_os == "mac") {
+    # Default path setup by `brew install sdl2`
+    include_dirs += [ "/usr/local/include/SDL2" ]
+    defines += [ "_THREAD_SAFE" ]
+    ldflags = [
+      "-framework",
+      "OpenGL",
+    ]
+  } else {
+    # Linux defaults
+    include_dirs += [ "/usr/include/SDL2" ]
+    defines += [ "_REENTRANT" ]
+  }
 }
 
 source_set("imgui") {
@@ -51,13 +62,14 @@ source_set("imgui") {
   ]
 
   # FUTURE: SDL2 libs should be from `sdl2-config --libs`
-  #         Also different platforms may require different seettings (e.g. on mac this
-  #         seems to need `-framework OpenGl -framework CoreFoundation`
   libs = [
     "SDL2",
-    "GL",
     "dl",
   ]
 
+  if (current_os == "linux") {
+    libs += [ "GL" ]
+  }
+
   public_configs = [ ":imgui_config" ]
 }