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

[web] Enforce onDrawFrame/onBeginFrame render rule #49214

Merged
merged 6 commits into from
Dec 19, 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
2 changes: 1 addition & 1 deletion lib/web_ui/lib/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ abstract class PlatformDispatcher {

void scheduleFrame();

void render(Scene scene, [FlutterView view]);
Future<void> render(Scene scene, [FlutterView view]);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the mobile version no longer has this method.

Copy link
Member

Choose a reason for hiding this comment

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

Agh shoot, forgot to remove it from our interface 🤦


AccessibilityFeatures get accessibilityFeatures;

Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ class HtmlViewEmbedder {
sceneHost.append(_svgPathDefs!);
}

void submitFrame() {
Future<void> submitFrame() async {
final ViewListDiffResult? diffResult =
(_activeCompositionOrder.isEmpty || _compositionOrder.isEmpty)
? null
Expand All @@ -385,7 +385,7 @@ class HtmlViewEmbedder {
_context.pictureRecorders[pictureRecorderIndex].endRecording());
pictureRecorderIndex++;
}
rasterizer.rasterizeToCanvas(overlay, pictures);
await rasterizer.rasterizeToCanvas(overlay, pictures);
}
for (final CkPictureRecorder recorder
in _context.pictureRecordersCreatedDuringPreroll) {
Expand Down
6 changes: 3 additions & 3 deletions lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class Rasterizer {

/// Creates a new frame from this rasterizer's surface, draws the given
/// [LayerTree] into it, and then submits the frame.
void draw(LayerTree layerTree) {
Future<void> draw(LayerTree layerTree) async {
final ui.Size frameSize = view.physicalSize;
if (frameSize.isEmpty) {
// Available drawing area is empty. Skip drawing.
Expand All @@ -49,10 +49,10 @@ class Rasterizer {
compositorFrame.raster(layerTree, ignoreRasterCache: true);

sceneHost.prepend(renderCanvasFactory.baseCanvas.htmlElement);
rasterizeToCanvas(renderCanvasFactory.baseCanvas,
await rasterizeToCanvas(renderCanvasFactory.baseCanvas,
<CkPicture>[pictureRecorder.endRecording()]);

viewEmbedder.submitFrame();
await viewEmbedder.submitFrame();
}

/// Disposes of this rasterizer.
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/canvaskit/renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ class CanvasKitRenderer implements Renderer {
CkParagraphBuilder(style);

@override
void renderScene(ui.Scene scene, ui.FlutterView view) {
Future<void> renderScene(ui.Scene scene, ui.FlutterView view) async {
// "Build finish" and "raster start" happen back-to-back because we
// render on the same thread, so there's no overhead from hopping to
// another thread.
Expand All @@ -417,7 +417,7 @@ class CanvasKitRenderer implements Renderer {
"Unable to render to a view which hasn't been registered");
final Rasterizer rasterizer = _rasterizers[view.viewId]!;

rasterizer.draw((scene as LayerScene).layerTree);
await rasterizer.draw((scene as LayerScene).layerTree);
frameTimingsOnRasterFinish();
}

Expand Down
22 changes: 9 additions & 13 deletions lib/web_ui/lib/src/engine/canvaskit/surface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -115,22 +115,18 @@ class Surface {
_surface!.flush();

if (browserSupportsCreateImageBitmap) {
DomImageBitmap bitmap;
JSObject bitmapSource;
if (Surface.offscreenCanvasSupported) {
bitmap = (await createImageBitmap(_offscreenCanvas! as JSObject, (
x: 0,
y: _pixelHeight - frameSize.height.toInt(),
width: frameSize.width.toInt(),
height: frameSize.height.toInt(),
)).toDart)! as DomImageBitmap;
bitmapSource = _offscreenCanvas! as JSObject;
} else {
bitmap = (await createImageBitmap(_canvasElement! as JSObject, (
x: 0,
y: _pixelHeight - frameSize.height.toInt(),
width: frameSize.width.toInt(),
height: frameSize.height.toInt()
)).toDart)! as DomImageBitmap;
bitmapSource = _canvasElement! as JSObject;
}
final DomImageBitmap bitmap = await createImageBitmap(bitmapSource, (
x: 0,
y: _pixelHeight - frameSize.height.toInt(),
width: frameSize.width.toInt(),
height: frameSize.height.toInt(),
));
canvas.render(bitmap);
} else {
// If the browser doesn't support `createImageBitmap` (e.g. Safari 14)
Expand Down
8 changes: 5 additions & 3 deletions lib/web_ui/lib/src/engine/dom.dart
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,16 @@ external JSPromise<JSAny?> _createImageBitmap2(
JSNumber width,
JSNumber height,
);
JSPromise<JSAny?> createImageBitmap(JSAny source,
Future<DomImageBitmap> createImageBitmap(JSAny source,
[({int x, int y, int width, int height})? bounds]) {
JSPromise<JSAny?> jsPromise;
if (bounds != null) {
return _createImageBitmap2(source, bounds.x.toJS, bounds.y.toJS,
jsPromise = _createImageBitmap2(source, bounds.x.toJS, bounds.y.toJS,
bounds.width.toJS, bounds.height.toJS);
} else {
return _createImageBitmap1(source);
jsPromise = _createImageBitmap1(source);
}
return js_util.promiseToFuture<DomImageBitmap>(jsPromise);
}

@JS()
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/html/renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ class HtmlRenderer implements Renderer {
CanvasParagraphBuilder(style as EngineParagraphStyle);

@override
void renderScene(ui.Scene scene, ui.FlutterView view) {
Future<void> renderScene(ui.Scene scene, ui.FlutterView view) async {
final EngineFlutterView implicitView = EnginePlatformDispatcher.instance.implicitView!;
implicitView.dom.setScene((scene as SurfaceScene).webOnlyRootElement!);
frameTimingsOnRasterFinish();
Expand Down
24 changes: 21 additions & 3 deletions lib/web_ui/lib/src/engine/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
}
}

/// A set of views which have rendered in the current `onBeginFrame` or
/// `onDrawFrame` scope.
Set<ui.FlutterView>? _viewsRenderedInCurrentFrame;

/// A callback invoked when any window begins a frame.
///
/// A callback that is invoked to notify the application that it is an
Expand All @@ -229,7 +233,9 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
/// Engine code should use this method instead of the callback directly.
/// Otherwise zones won't work properly.
void invokeOnBeginFrame(Duration duration) {
_viewsRenderedInCurrentFrame = <ui.FlutterView>{};
invoke1<Duration>(_onBeginFrame, _onBeginFrameZone, duration);
_viewsRenderedInCurrentFrame = null;
}

/// A callback that is invoked for each frame after [onBeginFrame] has
Expand All @@ -250,7 +256,9 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
/// Engine code should use this method instead of the callback directly.
/// Otherwise zones won't work properly.
void invokeOnDrawFrame() {
_viewsRenderedInCurrentFrame = <ui.FlutterView>{};
invoke(_onDrawFrame, _onDrawFrameZone);
_viewsRenderedInCurrentFrame = null;
}

/// A callback that is invoked when pointer data is available.
Expand Down Expand Up @@ -753,14 +761,23 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
/// * [RendererBinding], the Flutter framework class which manages layout and
/// painting.
@override
void render(ui.Scene scene, [ui.FlutterView? view]) {
Future<void> render(ui.Scene scene, [ui.FlutterView? view]) async {
assert(view != null || implicitView != null,
'Calling render without a FlutterView');
if (view == null && implicitView == null) {
// If there is no view to render into, then this is a no-op.
return;
}
renderer.renderScene(scene, view ?? implicitView!);
final ui.FlutterView viewToRender = view ?? implicitView!;

// Only render in an `onDrawFrame` or `onBeginFrame` scope. This is checked
// by checking if the `_viewsRenderedInCurrentFrame` is non-null and this
// view hasn't been rendered already in this scope.
final bool shouldRender =
_viewsRenderedInCurrentFrame?.add(viewToRender) ?? false;
if (shouldRender) {
await renderer.renderScene(scene, viewToRender);
}
}

/// Additional accessibility features that may be enabled by the platform.
Expand Down Expand Up @@ -1275,7 +1292,8 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
String get defaultRouteName {
// TODO(mdebbar): What should we do in multi-view mode?
// https://github.com/flutter/flutter/issues/139174
return _defaultRouteName ??= implicitView?.browserHistory.currentPath ?? '/';
return _defaultRouteName ??=
implicitView?.browserHistory.currentPath ?? '/';
}

/// Lazily initialized when the `defaultRouteName` getter is invoked.
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,5 @@ abstract class Renderer {

ui.ParagraphBuilder createParagraphBuilder(ui.ParagraphStyle style);

FutureOr<void> renderScene(ui.Scene scene, ui.FlutterView view);
Future<void> renderScene(ui.Scene scene, ui.FlutterView view);
}
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/skwasm/skwasm_stub/renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class SkwasmRenderer implements Renderer {
}

@override
void renderScene(ui.Scene scene, ui.FlutterView view) {
Future<void> renderScene(ui.Scene scene, ui.FlutterView view) {
throw UnimplementedError('Skwasm not implemented on this platform.');
}

Expand Down
15 changes: 7 additions & 8 deletions lib/web_ui/test/canvaskit/backdrop_filter_golden_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import 'package:test/test.dart';
import 'package:ui/src/engine.dart';
import 'package:ui/ui.dart' as ui;

import 'package:web_engine_tester/golden_tester.dart';

import 'common.dart';

void main() {
Expand Down Expand Up @@ -49,8 +47,8 @@ void testMain() {
builder.pushOffset(0, 0);
builder.addPicture(ui.Offset.zero, checkerboard);
builder.pushBackdropFilter(ui.ImageFilter.blur(sigmaX: 10, sigmaY: 10));
CanvasKitRenderer.instance.renderScene(builder.build(), implicitView);
await matchGoldenFile('canvaskit_backdropfilter_blur_edges.png',
await matchSceneGolden(
'canvaskit_backdropfilter_blur_edges.png', builder.build(),
region: region);
});
test('ImageFilter with ColorFilter as child', () async {
Expand All @@ -62,9 +60,7 @@ void testMain() {
final CkPictureRecorder recorder = CkPictureRecorder();
final CkCanvas canvas = recorder.beginRecording(region);
final ui.ColorFilter colorFilter = ui.ColorFilter.mode(
const ui.Color(0XFF00FF00).withOpacity(0.55),
ui.BlendMode.darken
);
const ui.Color(0XFF00FF00).withOpacity(0.55), ui.BlendMode.darken);

// using a colorFilter as an imageFilter for backDrop filter
builder.pushBackdropFilter(colorFilter);
Expand All @@ -75,7 +71,10 @@ void testMain() {
);
final CkPicture redCircle1 = recorder.endRecording();
builder.addPicture(ui.Offset.zero, redCircle1);
await matchSceneGolden('canvaskit_red_circle_green_backdrop_colorFilter.png', builder.build(), region: region);
await matchSceneGolden(
'canvaskit_red_circle_green_backdrop_colorFilter.png',
builder.build(),
region: region);
});
// TODO(hterkelsen): https://github.com/flutter/flutter/issues/71520
}, skip: isSafari || isFirefox);
Expand Down
13 changes: 4 additions & 9 deletions lib/web_ui/test/canvaskit/canvas_golden_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import 'package:ui/src/engine.dart';
import 'package:ui/ui.dart' as ui;
import 'package:ui/ui_web/src/ui_web.dart' as ui_web;

import 'package:web_engine_tester/golden_tester.dart';

import 'common.dart';

void main() {
Expand Down Expand Up @@ -147,16 +145,14 @@ void testMain() {
builder.pushOffset(0, 0);
builder.addPicture(ui.Offset.zero, picture);
final LayerScene scene = builder.build();
CanvasKitRenderer.instance.renderScene(scene, implicitView);
await renderScene(scene);

// Now draw an empty layer tree and confirm that the red rectangle is
// no longer drawn.
final LayerSceneBuilder emptySceneBuilder = LayerSceneBuilder();
emptySceneBuilder.pushOffset(0, 0);
final LayerScene emptyScene = emptySceneBuilder.build();
CanvasKitRenderer.instance.renderScene(emptyScene, implicitView);

await matchGoldenFile('canvaskit_empty_scene.png',
await matchSceneGolden('canvaskit_empty_scene.png', emptyScene,
region: const ui.Rect.fromLTRB(0, 0, 100, 100));
});

Expand Down Expand Up @@ -211,9 +207,8 @@ void testMain() {
sb.pop();

// The below line should not throw an error.
CanvasKitRenderer.instance.renderScene(sb.build(), implicitView);

await matchGoldenFile('cross_overlay_resources.png', region: const ui.Rect.fromLTRB(0, 0, 100, 100));
await matchSceneGolden('cross_overlay_resources.png', sb.build(),
region: const ui.Rect.fromLTRB(0, 0, 100, 100));
});
});
}
Expand Down
12 changes: 6 additions & 6 deletions lib/web_ui/test/canvaskit/common.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ import 'package:ui/src/engine.dart';
import 'package:ui/ui.dart' as ui;
import 'package:web_engine_tester/golden_tester.dart';

import '../common/rendering.dart';
import '../common/test_initialization.dart';

export '../common/rendering.dart' show renderScene;

const MethodCodec codec = StandardMethodCodec();

/// Common test setup for all CanvasKit unit-tests.
Expand Down Expand Up @@ -44,13 +47,10 @@ CkPicture paintPicture(

Future<void> matchSceneGolden(
String goldenFile,
LayerScene scene, {
ui.Scene scene, {
required ui.Rect region,
}) async {
// TODO(harryterkelsen): Enforce the render rule. Render can only be called in
// the scope of `onBeginFrame` or `onDrawFrame`,
// https://github.com/flutter/flutter/issues/137073.
CanvasKitRenderer.instance.renderScene(scene, implicitView);
await renderScene(scene);
await matchGoldenFile(goldenFile, region: region);
}

Expand All @@ -63,7 +63,7 @@ Future<void> matchPictureGolden(String goldenFile, CkPicture picture,
final LayerSceneBuilder sb = LayerSceneBuilder();
sb.pushOffset(0, 0);
sb.addPicture(ui.Offset.zero, picture);
CanvasKitRenderer.instance.renderScene(sb.build(), implicitView);
await renderScene(sb.build());
await matchGoldenFile(goldenFile, region: region);
}

Expand Down
Loading