From 19486289298ee1d57eff1649f86e54bad75d6419 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 16 Feb 2023 11:32:02 -0500 Subject: [PATCH] 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 | 44 +++++++++++++-- examples/lighting-app/linux/ui.cpp | 56 +++++++++---------- examples/lighting-app/linux/ui.h | 11 +++- examples/platform/linux/AppMain.cpp | 26 ++++++++- examples/platform/linux/AppMain.h | 47 +++++++++++++++- examples/platform/nxp/se05x/linux/AppMain.cpp | 11 +++- .../platform/tizen/TizenServiceAppMain.cpp | 7 ++- third_party/imgui/BUILD.gn | 22 ++++++-- 8 files changed, 174 insertions(+), 50 deletions(-) diff --git a/examples/lighting-app/linux/main.cpp b/examples/lighting-app/linux/main.cpp index a74aa5df4c3059..9f04fdfb040cad 100644 --- a/examples/lighting-app/linux/main.cpp +++ b/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; diff --git a/examples/lighting-app/linux/ui.cpp b/examples/lighting-app/linux/ui.cpp index 8ad8e38156a04b..a5c03f917e4eb9 100644 --- a/examples/lighting-app/linux/ui.cpp +++ b/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 diff --git a/examples/lighting-app/linux/ui.h b/examples/lighting-app/linux/ui.h index 85bf2aadf6ba30..d1344201a453a8 100644 --- a/examples/lighting-app/linux/ui.h +++ b/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 diff --git a/examples/platform/linux/AppMain.cpp b/examples/platform/linux/AppMain.cpp index 3c34f2d6aefbee..f5bcce727d6489 100644 --- a/examples/platform/linux/AppMain.cpp +++ b/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(); diff --git a/examples/platform/linux/AppMain.h b/examples/platform/linux/AppMain.h index 37a71fd745e15e..368486b6767594 100644 --- a/examples/platform/linux/AppMain.h +++ b/examples/platform/linux/AppMain.h @@ -18,6 +18,7 @@ #pragma once +#include #include #include #include @@ -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 diff --git a/examples/platform/nxp/se05x/linux/AppMain.cpp b/examples/platform/nxp/se05x/linux/AppMain.cpp index b4652f5a491a6b..6129298cbf3853 100644 --- a/examples/platform/nxp/se05x/linux/AppMain.cpp +++ b/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(); diff --git a/examples/platform/tizen/TizenServiceAppMain.cpp b/examples/platform/tizen/TizenServiceAppMain.cpp index b174abcfd78e2a..778c36960c608b 100644 --- a/examples/platform/tizen/TizenServiceAppMain.cpp +++ b/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; } } diff --git a/third_party/imgui/BUILD.gn b/third_party/imgui/BUILD.gn index 362e9641fc0091..befa8321aa21ef 100644 --- a/third_party/imgui/BUILD.gn +++ b/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" ] }