From 1d3e7fa92b0bdf9409e6a36aa1dae811fe5494ef Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Mon, 14 Sep 2020 17:02:12 -0700 Subject: [PATCH 1/3] [embedder] Add support for passing display refresh rate The api currently supports mutliple displays, but the implementation only wires in the first display we get as the main display refresh rate. --- shell/common/shell.cc | 2 +- shell/common/vsync_waiter_fallback.cc | 13 +++++- shell/common/vsync_waiter_fallback.h | 11 ++++- shell/platform/embedder/embedder.cc | 40 ++++++++++++++-- shell/platform/embedder/embedder.h | 46 +++++++++++++++++++ .../embedder/platform_view_embedder.cc | 7 +-- .../embedder/platform_view_embedder.h | 2 + .../embedder/tests/embedder_config_builder.cc | 16 +++++++ .../embedder/tests/embedder_config_builder.h | 4 ++ .../embedder/tests/embedder_test_context.cc | 8 ++++ .../embedder/tests/embedder_test_context.h | 9 ++++ .../embedder/tests/embedder_unittests.cc | 43 +++++++++++++++++ .../embedder/vsync_waiter_embedder.cc | 11 ++++- .../platform/embedder/vsync_waiter_embedder.h | 6 ++- 14 files changed, 203 insertions(+), 15 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 6bd27c260e82d..92ce308a90f4e 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1442,7 +1442,7 @@ bool Shell::OnServiceProtocolGetDisplayRefreshRate( FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); response->SetObject(); response->AddMember("type", "DisplayRefreshRate", response->GetAllocator()); - response->AddMember("fps", engine_->GetDisplayRefreshRate(), + response->AddMember("fps", display_refresh_rate_.load(), response->GetAllocator()); return true; } diff --git a/shell/common/vsync_waiter_fallback.cc b/shell/common/vsync_waiter_fallback.cc index ef5cb69390cfe..667c5f2bb1d53 100644 --- a/shell/common/vsync_waiter_fallback.cc +++ b/shell/common/vsync_waiter_fallback.cc @@ -21,8 +21,15 @@ static fml::TimePoint SnapToNextTick(fml::TimePoint value, } // namespace +VsyncWaiterFallback::VsyncWaiterFallback(TaskRunners task_runners, + float display_refresh_rate) + : VsyncWaiter(std::move(task_runners)), + phase_(fml::TimePoint::Now()), + display_refresh_rate_(display_refresh_rate) {} + VsyncWaiterFallback::VsyncWaiterFallback(TaskRunners task_runners) - : VsyncWaiter(std::move(task_runners)), phase_(fml::TimePoint::Now()) {} + : VsyncWaiterFallback(std::move(task_runners), + VsyncWaiter::kUnknownRefreshRateFPS) {} VsyncWaiterFallback::~VsyncWaiterFallback() = default; @@ -37,4 +44,8 @@ void VsyncWaiterFallback::AwaitVSync() { FireCallback(next, next + kSingleFrameInterval); } +float VsyncWaiterFallback::GetDisplayRefreshRate() const { + return display_refresh_rate_; +} + } // namespace flutter diff --git a/shell/common/vsync_waiter_fallback.h b/shell/common/vsync_waiter_fallback.h index 5226dda019504..c124b177e803b 100644 --- a/shell/common/vsync_waiter_fallback.h +++ b/shell/common/vsync_waiter_fallback.h @@ -12,15 +12,22 @@ namespace flutter { -/// A |VsyncWaiter| that will fire at 60 fps irrespective of the vsync. +/// A |VsyncWaiter| that will fire at 60 fps irrespective of the vsync. The +/// refresh rate can also optionally be set, this is currently only used to +/// indicate to devtools, this will still fire at 60fps. class VsyncWaiterFallback final : public VsyncWaiter { public: VsyncWaiterFallback(TaskRunners task_runners); + VsyncWaiterFallback(TaskRunners task_runners, float display_refresh_rate); + ~VsyncWaiterFallback() override; + float GetDisplayRefreshRate() const override; + private: - fml::TimePoint phase_; + const fml::TimePoint phase_; + const float display_refresh_rate_; // |VsyncWaiter| void AwaitVSync() override; diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index bf2c1b9d44831..95af99a280eab 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -47,6 +47,7 @@ extern const intptr_t kPlatformStrongDillSize; #include "flutter/shell/common/persistent_cache.h" #include "flutter/shell/common/rasterizer.h" #include "flutter/shell/common/switches.h" +#include "flutter/shell/common/vsync_waiter_fallback.h" #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/shell/platform/embedder/embedder_engine.h" #include "flutter/shell/platform/embedder/embedder_platform_message_response.h" @@ -993,13 +994,42 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, "Compositor arguments were invalid."); } + auto display_settings_callback = + SAFE_ACCESS(args, display_settings_callback, nullptr); + double main_display_refresh_rate = + flutter::VsyncWaiter::kUnknownRefreshRateFPS; + + if (display_settings_callback) { + FlutterDisplaySettings display_settings; + display_settings.struct_size = sizeof(FlutterDisplaySettings); + display_settings.displays = nullptr; + display_settings.display_count = 0; + display_settings_callback(user_data, &display_settings); + + if (display_settings.display_count == 0) { + return LOG_EMBEDDER_ERROR(kInvalidArguments, + "Invalid display settings provided by the " + "embedder, got 0 active displays."); + } + + display_settings.displays = + new FlutterDisplay[display_settings.display_count]; + display_settings_callback(user_data, &display_settings); + + // TODO(iskakaushik): Add support for multiple displays. + main_display_refresh_rate = display_settings.displays[0].refresh_rate; + + delete display_settings.displays; + } + flutter::PlatformViewEmbedder::PlatformDispatchTable platform_dispatch_table = { - update_semantics_nodes_callback, // - update_semantics_custom_actions_callback, // - platform_message_response_callback, // - vsync_callback, // - compute_platform_resolved_locale_callback, // + update_semantics_nodes_callback, // + update_semantics_custom_actions_callback, // + platform_message_response_callback, // + vsync_callback, // + compute_platform_resolved_locale_callback, // + static_cast(main_display_refresh_rate), // }; auto on_create_platform_view = InferPlatformViewCreationCallback( diff --git a/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 97df3d2468308..b5951d0916a32 100644 --- a/shell/platform/embedder/embedder.h +++ b/shell/platform/embedder/embedder.h @@ -963,6 +963,43 @@ typedef const FlutterLocale* (*FlutterComputePlatformResolvedLocaleCallback)( const FlutterLocale** /* supported_locales*/, size_t /* Number of locales*/); +/// Display refers to a graphics hardware system consisting of a framebuffer, +/// typically a monitor or a screen. This ID is unique per display and is +/// stable until the Flutter application restarts. +typedef uint64_t FlutterDisplayId; + +typedef struct { + /// This size of this struct. Must be sizeof(FlutterDisplay). + size_t struct_size; + + FlutterDisplayId display_id; + + /// A double-precision floating-point value representing the actual refresh + /// period in seconds. This value may be zero if the device is not running or + /// unavaliable or unknown. + double refresh_rate; + +} FlutterDisplay; + +typedef struct { + /// This size of this struct. Must be sizeof(FlutterDisplaySettings). + size_t struct_size; + + /// Contains the actual number of displays returned in the displays array. + size_t display_count; + + /// A pointer to storage provided by the caller for an array of + /// FlutterDisplays. On return, the array contains a list of active displays. + /// If you pass NULL, on return the display_count contains the total number of + /// active displays. + FlutterDisplay* displays; + +} FlutterDisplaySettings; + +typedef void (*FlutterDisplaySettingsCallback)( + void* user_data, + FlutterDisplaySettings* display_settings); + typedef int64_t FlutterEngineDartPort; typedef enum { @@ -1318,6 +1355,15 @@ typedef struct { /// matches what the platform would natively resolve to as possible. FlutterComputePlatformResolvedLocaleCallback compute_platform_resolved_locale_callback; + + /// A callback to provide the settings for all the displays that are active + /// i.e, not mirrored or sleeping. + /// + /// A display is considered active if: + /// 1. The frame buffer hardware is connected. + /// 2. The display is drawable, e.g. it isn't being mirrored from another + /// connected display or sleeping. + FlutterDisplaySettingsCallback display_settings_callback; } FlutterProjectArgs; //------------------------------------------------------------------------------ diff --git a/shell/platform/embedder/platform_view_embedder.cc b/shell/platform/embedder/platform_view_embedder.cc index 67d0d3557460b..13aa0b23c8781 100644 --- a/shell/platform/embedder/platform_view_embedder.cc +++ b/shell/platform/embedder/platform_view_embedder.cc @@ -85,12 +85,13 @@ sk_sp PlatformViewEmbedder::CreateResourceContext() const { // |PlatformView| std::unique_ptr PlatformViewEmbedder::CreateVSyncWaiter() { if (!platform_dispatch_table_.vsync_callback) { - // Superclass implementation creates a timer based fallback. - return PlatformView::CreateVSyncWaiter(); + return std::make_unique( + task_runners_, platform_dispatch_table_.display_refresh_rate); } return std::make_unique( - platform_dispatch_table_.vsync_callback, task_runners_); + platform_dispatch_table_.vsync_callback, task_runners_, + platform_dispatch_table_.display_refresh_rate); } // |PlatformView| diff --git a/shell/platform/embedder/platform_view_embedder.h b/shell/platform/embedder/platform_view_embedder.h index 47c03d41a4596..301cb1a669c7e 100644 --- a/shell/platform/embedder/platform_view_embedder.h +++ b/shell/platform/embedder/platform_view_embedder.h @@ -9,6 +9,7 @@ #include "flutter/fml/macros.h" #include "flutter/shell/common/platform_view.h" +#include "flutter/shell/common/vsync_waiter_fallback.h" #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/shell/platform/embedder/embedder_surface.h" #include "flutter/shell/platform/embedder/embedder_surface_gl.h" @@ -38,6 +39,7 @@ class PlatformViewEmbedder final : public PlatformView { VsyncWaiterEmbedder::VsyncCallback vsync_callback; // optional ComputePlatformResolvedLocaleCallback compute_platform_resolved_locale_callback; + float display_refresh_rate; }; // Creates a platform view that sets up an OpenGL rasterizer. diff --git a/shell/platform/embedder/tests/embedder_config_builder.cc b/shell/platform/embedder/tests/embedder_config_builder.cc index 4870681460a73..d2675bb4944be 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.cc +++ b/shell/platform/embedder/tests/embedder_config_builder.cc @@ -232,6 +232,22 @@ void EmbedderConfigBuilder::SetPlatformMessageCallback( context_.SetPlatformMessageCallback(callback); } +void EmbedderConfigBuilder::SetDisplayRefreshRate(double refresh_rate) { + context_.SetDisplayRefreshRate(refresh_rate); + project_args_.display_settings_callback = + [](void* context, FlutterDisplaySettings* display_settings) { + if (!display_settings->displays) { + display_settings->display_count = 1; + } else { + auto& display = display_settings->displays[0]; + display.struct_size = sizeof(FlutterDisplay); + display.display_id = 1; + display.refresh_rate = reinterpret_cast(context) + ->GetDisplayRefreshRate(); + } + }; +} + void EmbedderConfigBuilder::SetCompositor() { context_.SetupCompositor(); auto& compositor = context_.GetCompositor(); diff --git a/shell/platform/embedder/tests/embedder_config_builder.h b/shell/platform/embedder/tests/embedder_config_builder.h index 84063b7245713..7b6bf7e296160 100644 --- a/shell/platform/embedder/tests/embedder_config_builder.h +++ b/shell/platform/embedder/tests/embedder_config_builder.h @@ -87,6 +87,10 @@ class EmbedderConfigBuilder { void SetCompositor(); + /// Sets the `FlutterProjectArgs`'s display refresh rate to + /// `display_refresh_rate`. This is in frames per second. + void SetDisplayRefreshRate(double display_refresh_rate); + FlutterCompositor& GetCompositor(); void SetRenderTargetType( diff --git a/shell/platform/embedder/tests/embedder_test_context.cc b/shell/platform/embedder/tests/embedder_test_context.cc index f4e35bbfb9764..c1927aa94aee3 100644 --- a/shell/platform/embedder/tests/embedder_test_context.cc +++ b/shell/platform/embedder/tests/embedder_test_context.cc @@ -209,5 +209,13 @@ void EmbedderTestContext::FireRootSurfacePresentCallbackIfPresent( callback(image_callback()); } +void EmbedderTestContext::SetDisplayRefreshRate(double refresh_rate) { + display_refresh_rate_ = refresh_rate; +} + +double EmbedderTestContext::GetDisplayRefreshRate() const { + return display_refresh_rate_; +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/embedder/tests/embedder_test_context.h b/shell/platform/embedder/tests/embedder_test_context.h index d37411b7d4a01..4f6bd473a40e7 100644 --- a/shell/platform/embedder/tests/embedder_test_context.h +++ b/shell/platform/embedder/tests/embedder_test_context.h @@ -78,6 +78,14 @@ class EmbedderTestContext { virtual size_t GetSurfacePresentCount() const = 0; + /// Sets the refresh rate of the display. + void SetDisplayRefreshRate(double refresh_rate); + + /// Returns the last set refresh rate of the display. Returns zero otherwise. + /// + /// See: `SetDisplayRefreshRate`. + double GetDisplayRefreshRate() const; + // TODO(gw280): encapsulate these properly for subclasses to use protected: // This allows the builder to access the hooks. @@ -100,6 +108,7 @@ class EmbedderTestContext { std::unique_ptr compositor_; NextSceneCallback next_scene_callback_; SkMatrix root_surface_transformation_; + double display_refresh_rate_ = 0; static VoidCallback GetIsolateCreateCallbackHook(); diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 93c897623d6c8..37240d84eaf09 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -4483,5 +4483,48 @@ TEST_F(EmbedderTest, PresentInfoContainsValidFBOId) { frame_latch.Wait(); } +TEST_F(EmbedderTest, DisplayRefreshRateIsSet) { + auto& context = GetEmbedderContext(); + fml::AutoResetWaitableEvent latch; + context.AddIsolateCreateCallback([&latch]() { latch.Signal(); }); + EmbedderConfigBuilder builder(context); + builder.SetSoftwareRendererConfig(); + const double refresh_rate = 60; + builder.SetDisplayRefreshRate(refresh_rate); + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + // Wait for the root isolate to launch. + latch.Wait(); + + flutter::Shell& shell = ToEmbedderEngine(engine.get())->GetShell(); + auto vsync_waiter = shell.GetPlatformView()->CreateVSyncWaiter(); + + const float embedder_refresh_rate = vsync_waiter->GetDisplayRefreshRate(); + ASSERT_FLOAT_EQ(embedder_refresh_rate, refresh_rate); + + engine.reset(); +} + +TEST_F(EmbedderTest, DefaultDisplayRefreshRateIsUnknown) { + auto& context = GetEmbedderContext(); + fml::AutoResetWaitableEvent latch; + context.AddIsolateCreateCallback([&latch]() { latch.Signal(); }); + EmbedderConfigBuilder builder(context); + builder.SetSoftwareRendererConfig(); + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + // Wait for the root isolate to launch. + latch.Wait(); + + flutter::Shell& shell = ToEmbedderEngine(engine.get())->GetShell(); + auto vsync_waiter = shell.GetPlatformView()->CreateVSyncWaiter(); + + const float embedder_refresh_rate = vsync_waiter->GetDisplayRefreshRate(); + ASSERT_FLOAT_EQ(embedder_refresh_rate, + flutter::VsyncWaiter::kUnknownRefreshRateFPS); + + engine.reset(); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/embedder/vsync_waiter_embedder.cc b/shell/platform/embedder/vsync_waiter_embedder.cc index a3d483f51b41f..979842193be09 100644 --- a/shell/platform/embedder/vsync_waiter_embedder.cc +++ b/shell/platform/embedder/vsync_waiter_embedder.cc @@ -7,8 +7,11 @@ namespace flutter { VsyncWaiterEmbedder::VsyncWaiterEmbedder(const VsyncCallback& vsync_callback, - flutter::TaskRunners task_runners) - : VsyncWaiter(std::move(task_runners)), vsync_callback_(vsync_callback) { + flutter::TaskRunners task_runners, + float display_refresh_rate) + : VsyncWaiter(std::move(task_runners)), + vsync_callback_(vsync_callback), + display_refresh_rate_(display_refresh_rate) { FML_DCHECK(vsync_callback_); } @@ -40,4 +43,8 @@ bool VsyncWaiterEmbedder::OnEmbedderVsync(intptr_t baton, return true; } +float VsyncWaiterEmbedder::GetDisplayRefreshRate() const { + return display_refresh_rate_; +} + } // namespace flutter diff --git a/shell/platform/embedder/vsync_waiter_embedder.h b/shell/platform/embedder/vsync_waiter_embedder.h index c1b06327c8fc3..c043fc3c5fea0 100644 --- a/shell/platform/embedder/vsync_waiter_embedder.h +++ b/shell/platform/embedder/vsync_waiter_embedder.h @@ -15,7 +15,8 @@ class VsyncWaiterEmbedder final : public VsyncWaiter { using VsyncCallback = std::function; VsyncWaiterEmbedder(const VsyncCallback& callback, - flutter::TaskRunners task_runners); + flutter::TaskRunners task_runners, + float display_refresh_rate); ~VsyncWaiterEmbedder() override; @@ -23,8 +24,11 @@ class VsyncWaiterEmbedder final : public VsyncWaiter { fml::TimePoint frame_start_time, fml::TimePoint frame_target_time); + float GetDisplayRefreshRate() const override; + private: const VsyncCallback vsync_callback_; + const float display_refresh_rate_; // |VsyncWaiter| void AwaitVSync() override; From eadc0052efec884a124410fc169f6746a068071a Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Fri, 18 Sep 2020 11:54:48 -0700 Subject: [PATCH 2/3] Fix CR comments --- shell/platform/embedder/embedder.cc | 18 ++++++++---------- .../platform/embedder/platform_view_embedder.h | 2 +- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index 95af99a280eab..0dbda3fffd85c 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1012,24 +1012,22 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, "embedder, got 0 active displays."); } - display_settings.displays = - new FlutterDisplay[display_settings.display_count]; + std::vector displays(display_settings.display_count); + display_settings.displays = displays.data(); display_settings_callback(user_data, &display_settings); // TODO(iskakaushik): Add support for multiple displays. main_display_refresh_rate = display_settings.displays[0].refresh_rate; - - delete display_settings.displays; } flutter::PlatformViewEmbedder::PlatformDispatchTable platform_dispatch_table = { - update_semantics_nodes_callback, // - update_semantics_custom_actions_callback, // - platform_message_response_callback, // - vsync_callback, // - compute_platform_resolved_locale_callback, // - static_cast(main_display_refresh_rate), // + update_semantics_nodes_callback, // + update_semantics_custom_actions_callback, // + platform_message_response_callback, // + vsync_callback, // + compute_platform_resolved_locale_callback, // + main_display_refresh_rate, // }; auto on_create_platform_view = InferPlatformViewCreationCallback( diff --git a/shell/platform/embedder/platform_view_embedder.h b/shell/platform/embedder/platform_view_embedder.h index 301cb1a669c7e..cc5da4e737ef3 100644 --- a/shell/platform/embedder/platform_view_embedder.h +++ b/shell/platform/embedder/platform_view_embedder.h @@ -39,7 +39,7 @@ class PlatformViewEmbedder final : public PlatformView { VsyncWaiterEmbedder::VsyncCallback vsync_callback; // optional ComputePlatformResolvedLocaleCallback compute_platform_resolved_locale_callback; - float display_refresh_rate; + double display_refresh_rate; }; // Creates a platform view that sets up an OpenGL rasterizer. From 627dde9491a7a0ac04b327306aa348f9ef02ebca Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Mon, 21 Sep 2020 09:37:49 -0700 Subject: [PATCH 3/3] fix unittest --- shell/platform/embedder/tests/embedder_unittests.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 37240d84eaf09..b52cfb9853452 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -4484,7 +4484,7 @@ TEST_F(EmbedderTest, PresentInfoContainsValidFBOId) { } TEST_F(EmbedderTest, DisplayRefreshRateIsSet) { - auto& context = GetEmbedderContext(); + auto& context = GetEmbedderContext(ContextType::kOpenGLContext); fml::AutoResetWaitableEvent latch; context.AddIsolateCreateCallback([&latch]() { latch.Signal(); }); EmbedderConfigBuilder builder(context); @@ -4506,7 +4506,7 @@ TEST_F(EmbedderTest, DisplayRefreshRateIsSet) { } TEST_F(EmbedderTest, DefaultDisplayRefreshRateIsUnknown) { - auto& context = GetEmbedderContext(); + auto& context = GetEmbedderContext(ContextType::kOpenGLContext); fml::AutoResetWaitableEvent latch; context.AddIsolateCreateCallback([&latch]() { latch.Signal(); }); EmbedderConfigBuilder builder(context);