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

Commit bedc49e

Browse files
authored
Skip invalid renders in Dart (#47323)
With this PR, invalid renders are skipped in Dart in both release build and debug build. Part of flutter/flutter#137073 and part of relanding #45555. Unit tests will be relanded in a following PR once this PR is confirmed performance-safe. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 87f384c commit bedc49e

4 files changed

Lines changed: 24 additions & 55 deletions

File tree

lib/ui/platform_dispatcher.dart

Lines changed: 19 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,8 @@ class PlatformDispatcher {
308308
_invoke(onMetricsChanged, _onMetricsChangedZone);
309309
}
310310

311-
// A debug-only variable that stores the [FlutterView]s for which
312-
// [FlutterView.render] has already been called during the current
313-
// [onBeginFrame]/[onDrawFrame] callback sequence.
311+
// The [FlutterView]s for which [FlutterView.render] has already been called
312+
// during the current [onBeginFrame]/[onDrawFrame] callback sequence.
314313
//
315314
// It is null outside the scope of those callbacks indicating that calls to
316315
// [FlutterView.render] must be ignored. Furthermore, if a given [FlutterView]
@@ -320,16 +319,9 @@ class PlatformDispatcher {
320319
// Between [onBeginFrame] and [onDrawFrame] the properties value is
321320
// temporarily stored in `_renderedViewsBetweenCallbacks` so that it survives
322321
// the gap between the two callbacks.
323-
//
324-
// In release build, this variable is null, and therefore the calling rule is
325-
// not enforced. This is because the check might hurt cold startup delay;
326-
// see https://github.com/flutter/engine/pull/46919.
327-
Set<FlutterView>? _debugRenderedViews;
328-
// A debug-only variable that temporarily stores the `_renderedViews` value
329-
// between `_beginFrame` and `_drawFrame`.
330-
//
331-
// In release build, this variable is null.
332-
Set<FlutterView>? _debugRenderedViewsBetweenCallbacks;
322+
Set<FlutterView>? _renderedViews;
323+
// The `_renderedViews` value between `_beginFrame` and `_drawFrame`.
324+
Set<FlutterView>? _renderedViewsBetweenCallbacks;
333325

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

352344
// Called from the engine, via hooks.dart
353345
void _beginFrame(int microseconds) {
354-
assert(_debugRenderedViews == null);
355-
assert(_debugRenderedViewsBetweenCallbacks == null);
356-
assert(() {
357-
_debugRenderedViews = <FlutterView>{};
358-
return true;
359-
}());
346+
assert(_renderedViews == null);
347+
assert(_renderedViewsBetweenCallbacks == null);
348+
_renderedViews = <FlutterView>{};
360349

361350
_invoke1<Duration>(
362351
onBeginFrame,
363352
_onBeginFrameZone,
364353
Duration(microseconds: microseconds),
365354
);
366355

367-
assert(_debugRenderedViews != null);
368-
assert(_debugRenderedViewsBetweenCallbacks == null);
369-
assert(() {
370-
_debugRenderedViewsBetweenCallbacks = _debugRenderedViews;
371-
_debugRenderedViews = null;
372-
return true;
373-
}());
356+
assert(_renderedViews != null);
357+
assert(_renderedViewsBetweenCallbacks == null);
358+
_renderedViewsBetweenCallbacks = _renderedViews;
359+
_renderedViews = null;
374360
}
375361

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

389375
// Called from the engine, via hooks.dart
390376
void _drawFrame() {
391-
assert(_debugRenderedViews == null);
392-
assert(_debugRenderedViewsBetweenCallbacks != null);
393-
assert(() {
394-
_debugRenderedViews = _debugRenderedViewsBetweenCallbacks;
395-
_debugRenderedViewsBetweenCallbacks = null;
396-
return true;
397-
}());
377+
assert(_renderedViews == null);
378+
assert(_renderedViewsBetweenCallbacks != null);
379+
_renderedViews = _renderedViewsBetweenCallbacks;
380+
_renderedViewsBetweenCallbacks = null;
398381

399382
_invoke(onDrawFrame, _onDrawFrameZone);
400383

401-
assert(_debugRenderedViews != null);
402-
assert(_debugRenderedViewsBetweenCallbacks == null);
403-
assert(() {
404-
_debugRenderedViews = null;
405-
return true;
406-
}());
384+
assert(_renderedViews != null);
385+
assert(_renderedViewsBetweenCallbacks == null);
386+
_renderedViews = null;
407387
}
408388

409389
/// A callback that is invoked when pointer data is available.

lib/ui/window.dart

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -355,14 +355,10 @@ class FlutterView {
355355
/// painting.
356356
void render(Scene scene) {
357357
// Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated
358-
// by _debugRenderedViews being null) are ignored. See _debugRenderedViews.
358+
// by _renderedViews being null) are ignored. See _renderedViews.
359359
// TODO(dkwingsmt): We should change this skip into an assertion.
360360
// https://github.com/flutter/flutter/issues/137073
361-
bool validRender = true;
362-
assert(() {
363-
validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false;
364-
return true;
365-
}());
361+
final bool validRender = platformDispatcher._renderedViews?.add(this) ?? false;
366362
if (validRender) {
367363
_render(viewId, scene as _NativeScene);
368364
}

shell/common/animator.cc

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -179,15 +179,7 @@ void Animator::EndFrame() {
179179
void Animator::Render(int64_t view_id,
180180
std::unique_ptr<flutter::LayerTree> layer_tree,
181181
float device_pixel_ratio) {
182-
// Animator::Render should be called between BeginFrame and EndFrame,
183-
// which is indicated by frame_timings_recorder_ being non-null.
184-
// This might happen on release build, and is guarded by PlatformDispatcher on
185-
// debug build.
186-
// TODO(dkwingsmt): We should change this skip into an assertion.
187-
// https://github.com/flutter/flutter/issues/137073
188-
if (frame_timings_recorder_ == nullptr) {
189-
return;
190-
}
182+
FML_CHECK(frame_timings_recorder_ != nullptr);
191183

192184
has_rendered_ = true;
193185

shell/common/animator.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ class Animator final {
5858
///
5959
/// This method must be called during a vsync callback, or
6060
/// technically, between Animator::BeginFrame and Animator::EndFrame
61-
/// (both private methods). Otherwise, this call will be ignored.
61+
/// (both private methods). Otherwise, an assertion will be
62+
/// triggered.
6263
///
6364
void Render(int64_t view_id,
6465
std::unique_ptr<flutter::LayerTree> layer_tree,

0 commit comments

Comments
 (0)