Skip to content

Commit

Permalink
Reland: Enforce the rule of calling FlutterView.Render (#45300) (#45555)
Browse files Browse the repository at this point in the history
This PR relands #45300 which was reverted in #45525 due to hanging on a windows startup test. The culprit test still calls `FlutterView.render` in the illegal way, which is ignored, causing no frame being ever produced. This has been fixed in flutter/flutter#134245. I've also searched through the framework repo for `render(` to ensure there are no other cases.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
dkwingsmt authored and harryterkelsen committed Oct 23, 2023
1 parent dc024ab commit 2b16901
Show file tree
Hide file tree
Showing 7 changed files with 370 additions and 23 deletions.
42 changes: 42 additions & 0 deletions lib/ui/fixtures/ui_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1076,3 +1076,45 @@ external void _callHook(
Object? arg20,
Object? arg21,
]);

Scene _createRedBoxScene(Size size) {
final SceneBuilder builder = SceneBuilder();
builder.pushOffset(0.0, 0.0);
final Paint paint = Paint()
..color = Color.fromARGB(255, 255, 0, 0)
..style = PaintingStyle.fill;
final PictureRecorder baseRecorder = PictureRecorder();
final Canvas canvas = Canvas(baseRecorder);
canvas.drawRect(Rect.fromLTRB(0.0, 0.0, size.width, size.height), paint);
final Picture picture = baseRecorder.endRecording();
builder.addPicture(Offset(0.0, 0.0), picture);
builder.pop();
return builder.build();
}

@pragma('vm:entry-point')
void incorrectImmediateRender() {
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(2, 2)));
_finish();
// Don't schedule a frame here. This test only checks if the
// [FlutterView.render] call is propagated to PlatformConfiguration.render
// and thus doesn't need anything from `Animator` or `Engine`, which,
// besides, are not even created in the native side at all.
}

@pragma('vm:entry-point')
void incorrectDoubleRender() {
PlatformDispatcher.instance.onBeginFrame = (Duration value) {
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(2, 2)));
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(3, 3)));
};
PlatformDispatcher.instance.onDrawFrame = () {
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(4, 4)));
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(5, 5)));
};
_finish();
// Don't schedule a frame here. This test only checks if the
// [FlutterView.render] call is propagated to PlatformConfiguration.render
// and thus doesn't need anything from `Animator` or `Engine`, which,
// besides, are not even created in the native side at all.
}
33 changes: 33 additions & 0 deletions lib/ui/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,21 @@ class PlatformDispatcher {
_invoke(onMetricsChanged, _onMetricsChangedZone);
}

// [FlutterView]s for which [FlutterView.render] has already been called
// during the current [onBeginFrame]/[onDrawFrame] callback sequence.
//
// The field 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.
Set<FlutterView>? _renderedViews;
// Temporary storage of the `_renderedViews` value between `_beginFrame` and
// `_drawFrame`.
Set<FlutterView>? _renderedViewsBetweenCallbacks;

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

// Called from the engine, via hooks.dart
void _beginFrame(int microseconds) {
assert(_renderedViews == null);
assert(_renderedViewsBetweenCallbacks == null);

_renderedViews = <FlutterView>{};
_invoke1<Duration>(
onBeginFrame,
_onBeginFrameZone,
Duration(microseconds: microseconds),
);
_renderedViewsBetweenCallbacks = _renderedViews;
_renderedViews = null;

assert(_renderedViews == null);
assert(_renderedViewsBetweenCallbacks != null);
}

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

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

_renderedViews = _renderedViewsBetweenCallbacks;
_renderedViewsBetweenCallbacks = null;
_invoke(onDrawFrame, _onDrawFrameZone);
_renderedViews = null;

assert(_renderedViews == null);
assert(_renderedViewsBetweenCallbacks == null);
}

/// A callback that is invoked when pointer data is available.
Expand Down
28 changes: 21 additions & 7 deletions lib/ui/window.dart
Original file line number Diff line number Diff line change
Expand Up @@ -327,14 +327,21 @@ class FlutterView {

/// Updates the view's rendering on the GPU with the newly provided [Scene].
///
/// This function must be called within the scope of the
/// ## Requirement for calling this method
///
/// This method must be called within the synchronous scope of the
/// [PlatformDispatcher.onBeginFrame] or [PlatformDispatcher.onDrawFrame]
/// callbacks being invoked.
/// callbacks. Calls out of this scope will be ignored. To use this method,
/// create a callback that calls this method instead, and assign it to either
/// of the fields above; then schedule a frame, which is done typically with
/// [PlatformDispatcher.scheduleFrame]. Also, make sure the callback does not
/// have `await` before the `FlutterWindow.render` call.
///
/// Additionally, this method can only be called once for each view during a
/// single [PlatformDispatcher.onBeginFrame]/[PlatformDispatcher.onDrawFrame]
/// callback sequence. Duplicate calls will be ignored in production.
///
/// If this function is called a second time during a single
/// [PlatformDispatcher.onBeginFrame]/[PlatformDispatcher.onDrawFrame]
/// callback sequence or called outside the scope of those callbacks, the call
/// will be ignored.
/// ## How to record a scene
///
/// To record graphical operations, first create a [PictureRecorder], then
/// construct a [Canvas], passing that [PictureRecorder] to its constructor.
Expand All @@ -353,7 +360,14 @@ 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) {
if (platformDispatcher._renderedViews?.add(this) != true) {
// Duplicated calls or calls outside of onBeginFrame/onDrawFrame
// (indicated by _renderedViews being null) are ignored, as documented.
return;
}
_render(scene as _NativeScene);
}

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

0 comments on commit 2b16901

Please sign in to comment.