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

Commit dc05416

Browse files
dkwingsmtharryterkelsen
authored andcommitted
Reland 2 (part 2): Enforce the rule of calling FlutterView.Render (#47095)
This PR relands part of #45300, which was reverted in #46919 due to performance regression. Due to how little and trivial production code the original PR touches, I really couldn't figure out the exact line that caused it except through experimentation, which requires changes to be officially landed on the main branch. After this PR lands, I'll immediately fire a performance test. This PR contains the render rule check performed by `PlatformDispatcher` of the original PR, the remaining changes to production code besides [the part 1](#47062). Since part 1 shows no regression, the changes of this PR is highly likely to be the culprit. Therefore I made some changes: The rule enforcement is no longer performed in release mode, but only in debug mode. This will cause behavior deviation between builds, but since the developer should be able to notice violation in debug mode anyway, I think this design is acceptable. It is intentional to not contain any unit tests or other changes of the original PR. They will be landed shortly after this PR. Part of flutter/flutter#136826. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 947ab1f commit dc05416

File tree

2 files changed

+64
-1
lines changed

2 files changed

+64
-1
lines changed

lib/ui/platform_dispatcher.dart

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,28 @@ 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.
314+
//
315+
// It is null outside the scope of those callbacks indicating that calls to
316+
// [FlutterView.render] must be ignored. Furthermore, if a given [FlutterView]
317+
// is already present in this set when its [FlutterView.render] is called
318+
// again, that call must be ignored as a duplicate.
319+
//
320+
// Between [onBeginFrame] and [onDrawFrame] the properties value is
321+
// temporarily stored in `_renderedViewsBetweenCallbacks` so that it survives
322+
// 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;
311333

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

330352
// Called from the engine, via hooks.dart
331353
void _beginFrame(int microseconds) {
354+
assert(_debugRenderedViews == null);
355+
assert(_debugRenderedViewsBetweenCallbacks == null);
356+
assert(() {
357+
_debugRenderedViews = <FlutterView>{};
358+
return true;
359+
}());
360+
332361
_invoke1<Duration>(
333362
onBeginFrame,
334363
_onBeginFrameZone,
335364
Duration(microseconds: microseconds),
336365
);
366+
367+
assert(_debugRenderedViews != null);
368+
assert(_debugRenderedViewsBetweenCallbacks == null);
369+
assert(() {
370+
_debugRenderedViewsBetweenCallbacks = _debugRenderedViews;
371+
_debugRenderedViews = null;
372+
return true;
373+
}());
337374
}
338375

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

352389
// Called from the engine, via hooks.dart
353390
void _drawFrame() {
391+
assert(_debugRenderedViews == null);
392+
assert(_debugRenderedViewsBetweenCallbacks != null);
393+
assert(() {
394+
_debugRenderedViews = _debugRenderedViewsBetweenCallbacks;
395+
_debugRenderedViewsBetweenCallbacks = null;
396+
return true;
397+
}());
398+
354399
_invoke(onDrawFrame, _onDrawFrameZone);
400+
401+
assert(_debugRenderedViews != null);
402+
assert(_debugRenderedViewsBetweenCallbacks == null);
403+
assert(() {
404+
_debugRenderedViews = null;
405+
return true;
406+
}());
355407
}
356408

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

lib/ui/window.dart

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,18 @@ class FlutterView {
353353
/// scheduling of frames.
354354
/// * [RendererBinding], the Flutter framework class which manages layout and
355355
/// painting.
356-
void render(Scene scene) => _render(scene as _NativeScene);
356+
void render(Scene scene) {
357+
// Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated
358+
// by _debugRenderedViews being null) are ignored. See _debugRenderedViews.
359+
bool validRender = true;
360+
assert(() {
361+
validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false;
362+
return true;
363+
}());
364+
if (validRender) {
365+
_render(scene as _NativeScene);
366+
}
367+
}
357368

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

0 commit comments

Comments
 (0)