Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Skip invalid renders in Dart #47323

Merged
merged 4 commits into from
Oct 26, 2023
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
58 changes: 19 additions & 39 deletions lib/ui/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,8 @@ 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.
// 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]
Expand All @@ -320,16 +319,9 @@ class PlatformDispatcher {
// 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;
Set<FlutterView>? _renderedViews;
// The `_renderedViews` value between `_beginFrame` and `_drawFrame`.
Set<FlutterView>? _renderedViewsBetweenCallbacks;

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

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

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

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

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

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

_invoke(onDrawFrame, _onDrawFrameZone);

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

/// A callback that is invoked when pointer data is available.
Expand Down
8 changes: 2 additions & 6 deletions lib/ui/window.dart
Original file line number Diff line number Diff line change
Expand Up @@ -355,14 +355,10 @@ class FlutterView {
/// painting.
void render(Scene scene) {
// Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated
// by _debugRenderedViews being null) are ignored. See _debugRenderedViews.
// by _renderedViews being null) are ignored. See _renderedViews.
// TODO(dkwingsmt): We should change this skip into an assertion.
// https://github.com/flutter/flutter/issues/137073
bool validRender = true;
assert(() {
validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false;
return true;
}());
final bool validRender = platformDispatcher._renderedViews?.add(this) ?? false;
if (validRender) {
_render(viewId, scene as _NativeScene);
}
Expand Down
10 changes: 1 addition & 9 deletions shell/common/animator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,7 @@ void Animator::EndFrame() {
void Animator::Render(int64_t view_id,
std::unique_ptr<flutter::LayerTree> layer_tree,
float device_pixel_ratio) {
// Animator::Render should be called between BeginFrame and EndFrame,
// which is indicated by frame_timings_recorder_ being non-null.
// This might happen on release build, and is guarded by PlatformDispatcher on
// debug build.
// TODO(dkwingsmt): We should change this skip into an assertion.
// https://github.com/flutter/flutter/issues/137073
if (frame_timings_recorder_ == nullptr) {
return;
}
FML_CHECK(frame_timings_recorder_ != nullptr);

has_rendered_ = true;

Expand Down
3 changes: 2 additions & 1 deletion shell/common/animator.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ class Animator final {
///
/// This method must be called during a vsync callback, or
/// technically, between Animator::BeginFrame and Animator::EndFrame
/// (both private methods). Otherwise, this call will be ignored.
/// (both private methods). Otherwise, an assertion will be
/// triggered.
///
void Render(int64_t view_id,
std::unique_ptr<flutter::LayerTree> layer_tree,
Expand Down