Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
13 changes: 12 additions & 1 deletion shell/common/vsync_waiter_fallback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -37,4 +44,8 @@ void VsyncWaiterFallback::AwaitVSync() {
FireCallback(next, next + kSingleFrameInterval);
}

float VsyncWaiterFallback::GetDisplayRefreshRate() const {
return display_refresh_rate_;
}

} // namespace flutter
11 changes: 9 additions & 2 deletions shell/common/vsync_waiter_fallback.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the class doc comment to cover this ctor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


~VsyncWaiterFallback() override;

float GetDisplayRefreshRate() const override;

private:
fml::TimePoint phase_;
const fml::TimePoint phase_;
const float display_refresh_rate_;

// |VsyncWaiter|
void AwaitVSync() override;
Expand Down
28 changes: 28 additions & 0 deletions shell/platform/embedder/embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -993,13 +994,40 @@ 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.");
}

std::vector<FlutterDisplay> 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;
}

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, //
main_display_refresh_rate, //
};

auto on_create_platform_view = InferPlatformViewCreationCallback(
Expand Down
46 changes: 46 additions & 0 deletions shell/platform/embedder/embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would an embedding enforce this? NSScreen doesn't have an ID, for instance, AFAICT.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use FlutterEngine as the prefix for new types; I've already had to add a macro to solve one collision problem with macOS, and I'd rather we not introduce more potential collision points.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.


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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/actual number/number/

s/returned// (since this isn't part of a function, so nothing is being returned)

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this seems to be describing a function, but it's the comment for a struct element.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, agreed, I will go over the docs and make sure they align with structs and functions.

/// active displays.
FlutterDisplay* displays;

} FlutterDisplaySettings;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same note about prefix.


typedef void (*FlutterDisplaySettingsCallback)(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document.

void* user_data,
FlutterDisplaySettings* display_settings);

typedef int64_t FlutterEngineDartPort;

typedef enum {
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused as to how this API would work; how would the engine know when the display list has changed and it needs to call this again? Since the embedder, not the engine, knows when the display list changes, I would expect a method called by the embedder to push information into the engine.

Even if for some reason that's not necessary now, it certainly will be when we're actually exposing display information in Dart, and we shouldn't have two different ways of providing the same information. /cc @gspencergoog

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, currently this change isn't accounting for cases where the display configuration changes while the app is running. I'm thinking of modifying the API to notify the engine on display reconfiguration, with 4 modes:

  1. Startup (when the app is launched)
  2. Adding new displays
  3. Removing existing displays
  4. Reconfigure an existing display (modify any display settings)

We might be able to use CGDisplayReconfigurationCallBack on macOS.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we will need those events, and they will have to come from the embedder and notify the engine.

One other thing to note is that some platforms won't be able to provide all of the information. For instance, on UWP and web it's not possible to find out what screens are connected, or their resolutions, if the current view isn't on that screen. And it doesn't provide the information for more than one screen if you span screens.

The screen configuration doesn't even have to change, though. You can just drag the window to another screen (or even just adjust it's position so more than half of it is on another screen on some platforms) and that will change these values for that app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other thing to note is that some platforms won't be able to provide all of the information. For instance, on UWP and web it's not possible to find out what screens are connected, or their resolutions, if the current view isn't on that screen. And it doesn't provide the information for more than one screen if you span screens.

I will document that this is best effort, with the only guarantee that embedder MUST provide at least one display setting which will correspond to the main screen (where the app window is active).

The screen configuration doesn't even have to change, though. You can just drag the window to another screen (or even just adjust it's position so more than half of it is on another screen on some platforms) and that will change these values for that app.

I am envisioning that this will result in 2 events being invoked by the embedder:

  1. Add Display: display-2
  2. Remove Display: display-1

Though there is some additional checks that are needed to be done by the embedders to ensure that the engine is aware of at least one display at any given time (i.e, no remove before add), I think it's ok to have this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will document that this is best effort, with the only guarantee that embedder MUST provide at least one display setting which will correspond to the main screen (where the app window is active).

Well, but when we have multiple windows, there will be multiple display settings that apply (one or more for each window). Not that you need to solve multi-window here, but maybe the requirement should be that it provide screen settings corresponding to each window displayed.

} FlutterProjectArgs;

//------------------------------------------------------------------------------
Expand Down
7 changes: 4 additions & 3 deletions shell/platform/embedder/platform_view_embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,13 @@ sk_sp<GrDirectContext> PlatformViewEmbedder::CreateResourceContext() const {
// |PlatformView|
std::unique_ptr<VsyncWaiter> PlatformViewEmbedder::CreateVSyncWaiter() {
if (!platform_dispatch_table_.vsync_callback) {
// Superclass implementation creates a timer based fallback.
return PlatformView::CreateVSyncWaiter();
return std::make_unique<VsyncWaiterFallback>(
task_runners_, platform_dispatch_table_.display_refresh_rate);
}

return std::make_unique<VsyncWaiterEmbedder>(
platform_dispatch_table_.vsync_callback, task_runners_);
platform_dispatch_table_.vsync_callback, task_runners_,
platform_dispatch_table_.display_refresh_rate);
}

// |PlatformView|
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/embedder/platform_view_embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -38,6 +39,7 @@ class PlatformViewEmbedder final : public PlatformView {
VsyncWaiterEmbedder::VsyncCallback vsync_callback; // optional
ComputePlatformResolvedLocaleCallback
compute_platform_resolved_locale_callback;
double display_refresh_rate;
};

// Creates a platform view that sets up an OpenGL rasterizer.
Expand Down
16 changes: 16 additions & 0 deletions shell/platform/embedder/tests/embedder_config_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<EmbedderTestContext*>(context)
->GetDisplayRefreshRate();
}
};
}

void EmbedderConfigBuilder::SetCompositor() {
context_.SetupCompositor();
auto& compositor = context_.GetCompositor();
Expand Down
4 changes: 4 additions & 0 deletions shell/platform/embedder/tests/embedder_config_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 8 additions & 0 deletions shell/platform/embedder/tests/embedder_test_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions shell/platform/embedder/tests/embedder_test_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -100,6 +108,7 @@ class EmbedderTestContext {
std::unique_ptr<EmbedderTestCompositor> compositor_;
NextSceneCallback next_scene_callback_;
SkMatrix root_surface_transformation_;
double display_refresh_rate_ = 0;

static VoidCallback GetIsolateCreateCallbackHook();

Expand Down
43 changes: 43 additions & 0 deletions shell/platform/embedder/tests/embedder_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4483,5 +4483,48 @@ TEST_F(EmbedderTest, PresentInfoContainsValidFBOId) {
frame_latch.Wait();
}

TEST_F(EmbedderTest, DisplayRefreshRateIsSet) {
auto& context = GetEmbedderContext(ContextType::kOpenGLContext);
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(ContextType::kOpenGLContext);
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
11 changes: 9 additions & 2 deletions shell/platform/embedder/vsync_waiter_embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
}

Expand Down Expand Up @@ -40,4 +43,8 @@ bool VsyncWaiterEmbedder::OnEmbedderVsync(intptr_t baton,
return true;
}

float VsyncWaiterEmbedder::GetDisplayRefreshRate() const {
return display_refresh_rate_;
}

} // namespace flutter
6 changes: 5 additions & 1 deletion shell/platform/embedder/vsync_waiter_embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,20 @@ class VsyncWaiterEmbedder final : public VsyncWaiter {
using VsyncCallback = std::function<void(intptr_t)>;

VsyncWaiterEmbedder(const VsyncCallback& callback,
flutter::TaskRunners task_runners);
flutter::TaskRunners task_runners,
float display_refresh_rate);

~VsyncWaiterEmbedder() override;

static bool OnEmbedderVsync(intptr_t baton,
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;
Expand Down