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 1 commit
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
1 change: 1 addition & 0 deletions lib/ui/dart_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ typedef CanvasPath Path;
V(NativeStringAttribute::initSpellOutStringAttribute) \
V(PlatformConfigurationNativeApi::DefaultRouteName) \
V(PlatformConfigurationNativeApi::ScheduleFrame) \
V(PlatformConfigurationNativeApi::ImposeSyncFrame) \
V(PlatformConfigurationNativeApi::Render) \
V(PlatformConfigurationNativeApi::UpdateSemantics) \
V(PlatformConfigurationNativeApi::SetNeedsReportTimings) \
Expand Down
30 changes: 30 additions & 0 deletions lib/ui/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -801,11 +801,41 @@ class PlatformDispatcher {
///
/// * [SchedulerBinding], the Flutter framework class which manages the
/// scheduling of frames.
/// * [imposeSyncFrame], a similar method that is used in rare cases that
/// a frame must be rendered immediately.
void scheduleFrame() => _scheduleFrame();

@Native<Void Function()>(symbol: 'PlatformConfigurationNativeApi::ScheduleFrame')
external static void _scheduleFrame();

/// Immediately render a frame by invoking the [onBeginFrame] and
/// [onDrawFrame] callbacks synchronously.
///
/// This method performs the same computation for a frame as [scheduleFrame]
/// does, but instead of doing so at an appropriate opportunity, the render is
/// completed synchronously within this call.
///
/// This method should not be used in usual cases. It is designed for
/// situations that require a frame is rendered as soon as possible, even at
/// the cost of rendering quality.
Copy link
Member

Choose a reason for hiding this comment

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

Can you give the warmup frame as an explicit example here? Some of the verbiage from https://main-api.flutter.dev/flutter/scheduler/SchedulerBinding/scheduleWarmUpFrame.html would fit here as well to motivate that use case.

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, can you take a look?

///
/// See also:
///
/// * [SchedulerBinding], the Flutter framework class which manages the
/// scheduling of frames.
/// * [scheduleFrame].
///
// TODO(dkwingsmt): This method is not used for now, and is not implemented by
// the engine yet. This is because we have to land the Dart FFI method first
// before being able to run the performance test for the full changes due to
// the restriction of the performance test. Eventually, we should either land
// the full changes, or remove this method.
// https://github.com/flutter/flutter/issues/142851
void imposeSyncFrame() => _imposeSyncFrame();
Copy link
Member

Choose a reason for hiding this comment

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

In terms of naming, I favor forceSyncFrame. I do like the Sync in the name, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just call it warmUpFrame? or something like that.

Copy link
Contributor Author

@dkwingsmt dkwingsmt Feb 9, 2024

Choose a reason for hiding this comment

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

Why not just call it warmUpFrame? or something like that.

I think "warm up frame" is a more application-level concept and belongs to the scheduler binding, and for PlatformDispatcher it's better to name it with what the developer makes the engine do.

Copy link
Contributor

Choose a reason for hiding this comment

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

but its only used for a warm up frame, and probably isn't usefulfor anything besides a warm up frame.

Copy link
Contributor Author

@dkwingsmt dkwingsmt Feb 9, 2024

Choose a reason for hiding this comment

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

We're expecting some developers calling the dart:ui package directly instead of going through the framework, who will have their own scheduling scheme and terms. I think it's better to leave room for them on how they use this method. (I've added scheduleWarmUpFrame to the doc as an example, do you think it helps easing your concern?)

Copy link
Contributor

Choose a reason for hiding this comment

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

But if you don't know what the usages are then its unlikely that you'll be able to design it in such a way as to accomidate them.

Copy link
Contributor Author

@dkwingsmt dkwingsmt Feb 11, 2024

Choose a reason for hiding this comment

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

I've added "used in scheduleWarmUpFrame" to document. Does it feel better?

(Although it's not in the framework yet, this is the PR to add it.)


@Native<Void Function()>(symbol: 'PlatformConfigurationNativeApi::ImposeSyncFrame')
external static void _imposeSyncFrame();

/// Additional accessibility features that may be enabled by the platform.
AccessibilityFeatures get accessibilityFeatures => _configuration.accessibilityFeatures;

Expand Down
10 changes: 10 additions & 0 deletions lib/ui/window/platform_configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,16 @@ void PlatformConfigurationNativeApi::ScheduleFrame() {
UIDartState::Current()->platform_configuration()->client()->ScheduleFrame();
}

void PlatformConfigurationNativeApi::ImposeSyncFrame() {
// TODO(dkwingsmt): This method is not implemented and is not used for now.
// This is because we have to land the Dart FFI method first before being able
// to run the performance test for the full changes due to the restriction of
// the performance test. Eventually, we should either land the full changes,
// or remove this method.
// https://github.com/flutter/flutter/issues/142851
FML_UNREACHABLE();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is in fact quite reachable. You could error log if you want to discourage calling this method but leaving a process exit accessible from dart:ui does not seem reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR has been merged to #49950 because I realized that this PR does not improve the situation alone. Now this method is implemented.

}

void PlatformConfigurationNativeApi::UpdateSemantics(SemanticsUpdate* update) {
UIDartState::ThrowIfUIOperationsProhibited();
UIDartState::Current()->platform_configuration()->client()->UpdateSemantics(
Expand Down
2 changes: 2 additions & 0 deletions lib/ui/window/platform_configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,8 @@ class PlatformConfigurationNativeApi {

static void ScheduleFrame();

static void ImposeSyncFrame();

static void Render(int64_t view_id,
Scene* scene,
double width,
Expand Down