Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
[web] Enforce onDrawFrame/onBeginFrame render rule
Also update tests to conform to the render rule.
  • Loading branch information
harryterkelsen committed Dec 19, 2023
commit 10c390f525888f076922942baab78fa80935ac86
22 changes: 20 additions & 2 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 @@ -760,7 +768,16 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
// 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) {
renderer.renderScene(scene, view ?? implicitView!);
}
}

/// 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
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);
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);
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);
renderScene(sb.build());
await matchGoldenFile(goldenFile, region: region);
}

Expand Down
Loading