Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
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
52 changes: 52 additions & 0 deletions lib/ui/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,28 @@ class PlatformDispatcher {
_invoke(onMetricsChanged, _onMetricsChangedZone);
}

// A debug-only variable that stores the [FlutterView]s for which
// [FlutterView.render] has already been called during the current
// [onBeginFrame]/[onDrawFrame] callback sequence.
//
// It is null outside the scope of those callbacks indicating that calls to
// [FlutterView.render] must be ignored. Furthermore, if a given [FlutterView]
// is already present in this set when its [FlutterView.render] is called
// again, that call must be ignored as a duplicate.
//
// Between [onBeginFrame] and [onDrawFrame] the properties value is
// temporarily stored in `_renderedViewsBetweenCallbacks` so that it survives
// the gap between the two callbacks.
//
// In release build, this variable is null, and therefore the calling rule is
// not enforced. This is because the check might hurt cold startup delay;
// see https://github.com/flutter/engine/pull/46919.
Set<FlutterView>? _debugRenderedViews;
// A debug-only variable that temporarily stores the `_renderedViews` value
// between `_beginFrame` and `_drawFrame`.
//
// In release build, this variable is null.
Set<FlutterView>? _debugRenderedViewsBetweenCallbacks;

/// A callback invoked when any view begins a frame.
///
Expand All @@ -329,11 +351,26 @@ class PlatformDispatcher {

// Called from the engine, via hooks.dart
void _beginFrame(int microseconds) {
assert(_debugRenderedViews == null);
assert(_debugRenderedViewsBetweenCallbacks == null);
assert(() {
_debugRenderedViews = <FlutterView>{};
return true;
}());

_invoke1<Duration>(
onBeginFrame,
_onBeginFrameZone,
Duration(microseconds: microseconds),
);

assert(_debugRenderedViews != null);
assert(_debugRenderedViewsBetweenCallbacks == null);
assert(() {
_debugRenderedViewsBetweenCallbacks = _debugRenderedViews;
_debugRenderedViews = null;
return true;
}());
}

/// A callback that is invoked for each frame after [onBeginFrame] has
Expand All @@ -351,7 +388,22 @@ class PlatformDispatcher {

// Called from the engine, via hooks.dart
void _drawFrame() {
assert(_debugRenderedViews == null);
assert(_debugRenderedViewsBetweenCallbacks != null);
assert(() {
_debugRenderedViews = _debugRenderedViewsBetweenCallbacks;
_debugRenderedViewsBetweenCallbacks = null;
return true;
}());

_invoke(onDrawFrame, _onDrawFrameZone);

assert(_debugRenderedViews != null);
assert(_debugRenderedViewsBetweenCallbacks == null);
assert(() {
_debugRenderedViews = null;
return true;
}());
}

/// A callback that is invoked when pointer data is available.
Expand Down
13 changes: 12 additions & 1 deletion lib/ui/window.dart
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,18 @@ class FlutterView {
/// scheduling of frames.
/// * [RendererBinding], the Flutter framework class which manages layout and
/// painting.
void render(Scene scene) => _render(scene as _NativeScene);
void render(Scene scene) {
// Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated
// by _debugRenderedViews being null) are ignored. See _debugRenderedViews.
bool validRender = true;
assert(() {
validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false;
return true;
}());
if (validRender) {
_render(scene as _NativeScene);
}
}
Copy link
Member

@loic-sharma loic-sharma Oct 19, 2023

Choose a reason for hiding this comment

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

Summarizing conversation with @dkwingsmt: we suspect that dropping invalid renders is what caused the performance regression. This reland only drops invalid renders in debug mode, we expect that the performance test will be unaffected now. @dkwingsmt will run a perf test once this lands to verify this.

Ideally our render rule enforcement would be stricter. Ideally invalid renders would trigger assertion errors on debug mode and would be dropped in release mode. However, this causes significant unit tests failures. We should consider doing this after we've confirmed performance is unaffected by this change.

Original code which would dropped invalid render calls on release mode as well: https://github.com/dkwingsmt/engine/blob/e266da1854fc9e3e972e562e9c7d97e73eb738c6/lib/ui/window.dart#L356-L363


@Native<Void Function(Pointer<Void>)>(symbol: 'PlatformConfigurationNativeApi::Render')
external static void _render(_NativeScene scene);
Expand Down