From fa161274fb5dfc8fbde383fb3db47e5666061a17 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Thu, 22 Aug 2019 11:39:00 -0700 Subject: [PATCH] Switch to an incremental runloop for GLFW (#11368) Rather than running the runloop forever, have the API expose an incremental runloop. This allows clients to do other processing if they need it. This allows for removing the odd construction of having knowledge of GTK event handling built into the library even though nothing in the library uses it; instead runner applications that use GTK plugins (such as FDE's testbed) can do that processing at the application level instead. --- build/install-build-deps-linux-desktop.sh | 2 +- shell/platform/glfw/BUILD.gn | 5 +-- .../flutter_window_controller.cc | 34 ++++++++++++--- .../flutter/flutter_window_controller.h | 14 ++++-- .../testing/stub_flutter_glfw_api.cc | 8 +++- .../testing/stub_flutter_glfw_api.h | 6 ++- shell/platform/glfw/flutter_glfw.cc | 43 ++++--------------- shell/platform/glfw/public/flutter_glfw.h | 21 ++++----- shell/platform/linux/config/BUILD.gn | 4 -- 9 files changed, 71 insertions(+), 66 deletions(-) diff --git a/build/install-build-deps-linux-desktop.sh b/build/install-build-deps-linux-desktop.sh index ef24ad5d95c..1f7ec3a568b 100755 --- a/build/install-build-deps-linux-desktop.sh +++ b/build/install-build-deps-linux-desktop.sh @@ -8,4 +8,4 @@ set -e -sudo apt-get -y install libgtk-3-dev libx11-dev +sudo apt-get -y install libx11-dev diff --git a/shell/platform/glfw/BUILD.gn b/shell/platform/glfw/BUILD.gn index 3a2d254f746..f751aa23ce7 100644 --- a/shell/platform/glfw/BUILD.gn +++ b/shell/platform/glfw/BUILD.gn @@ -63,10 +63,7 @@ source_set("flutter_glfw") { if (is_linux) { libs = [ "GL" ] - configs += [ - "$flutter_root/shell/platform/linux/config:gtk3", - "$flutter_root/shell/platform/linux/config:x11", - ] + configs += [ "$flutter_root/shell/platform/linux/config:x11" ] } else if (is_mac) { libs = [ "CoreVideo.framework", diff --git a/shell/platform/glfw/client_wrapper/flutter_window_controller.cc b/shell/platform/glfw/client_wrapper/flutter_window_controller.cc index 42535974fc5..0d3fc3c4c36 100644 --- a/shell/platform/glfw/client_wrapper/flutter_window_controller.cc +++ b/shell/platform/glfw/client_wrapper/flutter_window_controller.cc @@ -70,12 +70,36 @@ FlutterDesktopPluginRegistrarRef FlutterWindowController::GetRegistrarForPlugin( return FlutterDesktopGetPluginRegistrar(controller_, plugin_name.c_str()); } -void FlutterWindowController::RunEventLoop() { - if (controller_) { - FlutterDesktopRunWindowLoop(controller_); +bool FlutterWindowController::RunEventLoopWithTimeout( + std::chrono::milliseconds timeout) { + if (!controller_) { + std::cerr << "Cannot run event loop without a window window; call " + "CreateWindow first." + << std::endl; + return false; + } + uint32_t timeout_milliseconds; + if (timeout == std::chrono::milliseconds::max()) { + // The C API uses 0 to represent no timeout, so convert |max| to 0. + timeout_milliseconds = 0; + } else if (timeout.count() > UINT32_MAX) { + timeout_milliseconds = UINT32_MAX; + } else { + timeout_milliseconds = static_cast(timeout.count()); + } + bool still_running = FlutterDesktopRunWindowEventLoopWithTimeout( + controller_, timeout_milliseconds); + if (!still_running) { + FlutterDesktopDestroyWindow(controller_); + window_ = nullptr; + controller_ = nullptr; + } + return still_running; +} + +void FlutterWindowController::RunEventLoop() { + while (RunEventLoopWithTimeout()) { } - window_ = nullptr; - controller_ = nullptr; } } // namespace flutter diff --git a/shell/platform/glfw/client_wrapper/include/flutter/flutter_window_controller.h b/shell/platform/glfw/client_wrapper/include/flutter/flutter_window_controller.h index 5c54b42d773..a49fd0ccca4 100644 --- a/shell/platform/glfw/client_wrapper/include/flutter/flutter_window_controller.h +++ b/shell/platform/glfw/client_wrapper/include/flutter/flutter_window_controller.h @@ -5,12 +5,13 @@ #ifndef FLUTTER_SHELL_PLATFORM_GLFW_CLIENT_WRAPPER_INCLUDE_FLUTTER_FLUTTER_WINDOW_CONTROLLER_H_ #define FLUTTER_SHELL_PLATFORM_GLFW_CLIENT_WRAPPER_INCLUDE_FLUTTER_FLUTTER_WINDOW_CONTROLLER_H_ +#include + +#include #include #include #include -#include - #include "flutter_window.h" #include "plugin_registrar.h" @@ -67,7 +68,14 @@ class FlutterWindowController { // before CreateWindow is called, and after RunEventLoop returns; FlutterWindow* window() { return window_.get(); } - // Loops on Flutter window events until the window closes. + // Processes the next event on this window, or returns early if |timeout| is + // reached before the next event. + // + // Returns false if the window was closed as a result of event processing. + bool RunEventLoopWithTimeout( + std::chrono::milliseconds timeout = std::chrono::milliseconds::max()); + + // Deprecated. Use RunEventLoopWithTimeout. void RunEventLoop(); private: diff --git a/shell/platform/glfw/client_wrapper/testing/stub_flutter_glfw_api.cc b/shell/platform/glfw/client_wrapper/testing/stub_flutter_glfw_api.cc index a3390ad07ff..46c691bac78 100644 --- a/shell/platform/glfw/client_wrapper/testing/stub_flutter_glfw_api.cc +++ b/shell/platform/glfw/client_wrapper/testing/stub_flutter_glfw_api.cc @@ -121,10 +121,14 @@ double FlutterDesktopWindowGetScaleFactor( return 1.0; } -void FlutterDesktopRunWindowLoop(FlutterDesktopWindowControllerRef controller) { +bool FlutterDesktopRunWindowEventLoopWithTimeout( + FlutterDesktopWindowControllerRef controller, + uint32_t millisecond_timeout) { if (s_stub_implementation) { - s_stub_implementation->RunWindowLoop(); + return s_stub_implementation->RunWindowEventLoopWithTimeout( + millisecond_timeout); } + return true; } FlutterDesktopEngineRef FlutterDesktopRunEngine(const char* assets_path, diff --git a/shell/platform/glfw/client_wrapper/testing/stub_flutter_glfw_api.h b/shell/platform/glfw/client_wrapper/testing/stub_flutter_glfw_api.h index d974584dfae..52e0682da60 100644 --- a/shell/platform/glfw/client_wrapper/testing/stub_flutter_glfw_api.h +++ b/shell/platform/glfw/client_wrapper/testing/stub_flutter_glfw_api.h @@ -70,8 +70,10 @@ class StubFlutterGlfwApi { // Called for FlutterDesktopWindowGetScaleFactor. virtual double GetWindowScaleFactor() { return 1.0; } - // Called for FlutterDesktopRunWindowLoop. - virtual void RunWindowLoop() {} + // Called for FlutterDesktopRunWindowEventLoopWithTimeout. + virtual bool RunWindowEventLoopWithTimeout(uint32_t millisecond_timeout) { + return true; + } // Called for FlutterDesktopRunEngine. virtual FlutterDesktopEngineRef RunEngine(const char* assets_path, diff --git a/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index e3259a6c93a..ba51eecbfec 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/shell/platform/glfw/flutter_glfw.cc @@ -21,19 +21,6 @@ #include "flutter/shell/platform/glfw/platform_handler.h" #include "flutter/shell/platform/glfw/text_input_plugin.h" -// For compatibility with GTK-based plugins, special message loop setup is -// required (e.g., for GTK modal windows). This can be disabled for cases where -// a GTK dependency is undesirable by defining FLUTTER_DISABLE_GTK. -#if defined(__linux__) && !defined(FLUTTER_DISABLE_GTK) -#define FLUTTER_USE_GTK 1 -#endif - -#ifdef FLUTTER_USE_GTK -// For plugin-compatible event handling (e.g., modal windows). -#include -#include -#endif - // GLFW_TRUE & GLFW_FALSE are introduced since libglfw-3.3, // add definitions here to compile under the old versions. #ifndef GLFW_TRUE @@ -564,10 +551,6 @@ FlutterDesktopWindowControllerRef FlutterDesktopCreateWindow( const char* icu_data_path, const char** arguments, size_t argument_count) { -#ifdef FLUTTER_USE_GTK - gtk_init(0, nullptr); -#endif - auto state = std::make_unique(); // Create the window, and set the state as its user data. @@ -734,25 +717,15 @@ double FlutterDesktopWindowGetScaleFactor( return flutter_window->pixels_per_screen_coordinate; } -void FlutterDesktopRunWindowLoop(FlutterDesktopWindowControllerRef controller) { - GLFWwindow* window = controller->window.get(); -#ifdef FLUTTER_USE_GTK - // Necessary for GTK thread safety. - XInitThreads(); -#endif +bool FlutterDesktopRunWindowEventLoopWithTimeout( + FlutterDesktopWindowControllerRef controller, + uint32_t timeout_milliseconds) { + auto wait_duration = timeout_milliseconds == 0 + ? std::chrono::milliseconds::max() + : std::chrono::milliseconds(timeout_milliseconds); + controller->event_loop->WaitForEvents(wait_duration); - while (!glfwWindowShouldClose(window)) { - auto wait_duration = std::chrono::milliseconds::max(); -#ifdef FLUTTER_USE_GTK - // If we are not using GTK, there is no point in waking up. - wait_duration = std::chrono::milliseconds(10); - if (gtk_events_pending()) { - gtk_main_iteration(); - } -#endif - controller->event_loop->WaitForEvents(wait_duration); - } - FlutterDesktopDestroyWindow(controller); + return !glfwWindowShouldClose(controller->window.get()); } FlutterDesktopWindowRef FlutterDesktopGetWindow( diff --git a/shell/platform/glfw/public/flutter_glfw.h b/shell/platform/glfw/public/flutter_glfw.h index f4c718fe8d2..502722cda6a 100644 --- a/shell/platform/glfw/public/flutter_glfw.h +++ b/shell/platform/glfw/public/flutter_glfw.h @@ -50,10 +50,9 @@ FLUTTER_EXPORT void FlutterDesktopTerminate(); // for details. Not all arguments will apply to desktop. // // Returns a null pointer in the event of an error. Otherwise, the pointer is -// valid until FlutterDesktopRunWindowLoop has been called and returned, or -// FlutterDesktopDestroyWindow is called. +// valid until FlutterDesktopDestroyWindow is called. // Note that calling FlutterDesktopCreateWindow without later calling -// one of those two methods on the returned reference is a memory leak. +// FlutterDesktopDestroyWindow on the returned reference is a memory leak. FLUTTER_EXPORT FlutterDesktopWindowControllerRef FlutterDesktopCreateWindow(int initial_width, int initial_height, @@ -70,15 +69,17 @@ FlutterDesktopCreateWindow(int initial_width, FLUTTER_EXPORT void FlutterDesktopDestroyWindow( FlutterDesktopWindowControllerRef controller); -// Loops on Flutter window events until the window is closed. +// Waits for and processes the next event before |timeout_milliseconds|. // -// Once this function returns, |controller| is no longer valid, and must not be -// be used again, as it calls FlutterDesktopDestroyWindow internally. +// If |timeout_milliseconds| is zero, it will wait for the next event +// indefinitely. A non-zero timeout is needed only if processing unrelated to +// the event loop is necessary (e.g., to handle events from another source). // -// TODO: Replace this with a method that allows running the runloop -// incrementally. -FLUTTER_EXPORT void FlutterDesktopRunWindowLoop( - FlutterDesktopWindowControllerRef controller); +// Returns false if the window should be closed as a result of the last event +// processed. +FLUTTER_EXPORT bool FlutterDesktopRunWindowEventLoopWithTimeout( + FlutterDesktopWindowControllerRef controller, + uint32_t timeout_milliseconds); // Returns the window handle for the window associated with // FlutterDesktopWindowControllerRef. diff --git a/shell/platform/linux/config/BUILD.gn b/shell/platform/linux/config/BUILD.gn index f501bac6109..48cf022355c 100644 --- a/shell/platform/linux/config/BUILD.gn +++ b/shell/platform/linux/config/BUILD.gn @@ -4,10 +4,6 @@ import("//build/config/linux/pkg_config.gni") -pkg_config("gtk3") { - packages = [ "gtk+-3.0" ] -} - pkg_config("x11") { packages = [ "x11" ] }