Skip to content

Commit 5e2b4e7

Browse files
harryterkelsenharryterkelsen
committed
fix(web_ui): move prepareToDraw after raster to improve concurrency and stability
Moving prepareToDraw after the synchronous raster recording phase ensures that the UI state is captured immediately. This prevents potential race conditions in Skwasm where resizing the surface immediately before measuring and painting could lead to uninitialized Skia resources or incorrect rendering states. By deferring physical surface sizing until after the pictures are recorded, we also allow asynchronous surface preparation to better overlap with other work, improving overall frame performance. Fixes #182354
1 parent be0b4bd commit 5e2b4e7

3 files changed

Lines changed: 241 additions & 1 deletion

File tree

‎engine/src/flutter/lib/web_ui/lib/src/engine/compositing/rasterizer.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,13 @@ abstract class ViewRasterizer {
8080
final bitmapSize = BitmapSize.fromSize(frameSize);
8181

8282
currentFrameSize = bitmapSize;
83-
await prepareToDraw();
8483
viewEmbedder.frameSize = currentFrameSize;
8584
final Frame compositorFrame = context.acquireFrame(viewEmbedder);
8685

8786
compositorFrame.raster(layerTree, currentFrameSize, recorder);
8887
_lastRenderedLayerTree = layerTree;
8988

89+
await prepareToDraw();
9090
await viewEmbedder.submitFrame(recorder);
9191
}
9292

‎engine/src/flutter/lib/web_ui/test/ui/rasterizer_order_test.dart‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'package:test/bootstrap/browser.dart';
6+
import 'package:test/test.dart';
7+
import 'package:ui/src/engine.dart';
8+
import 'package:ui/ui.dart' as ui;
9+
10+
import '../common/test_initialization.dart';
11+
12+
void main() {
13+
internalBootstrapBrowserTest(() => testMain);
14+
}
15+
16+
class MockViewEmbedder extends PlatformViewEmbedder {
17+
MockViewEmbedder(super.sceneHost, super.rasterizer);
18+
19+
bool optimizeCompositionCalled = false;
20+
BitmapSize? frameSizeDuringOptimize;
21+
BitmapSize? _capturedFrameSize;
22+
23+
@override
24+
set frameSize(BitmapSize size) {
25+
_capturedFrameSize = size;
26+
super.frameSize = size;
27+
}
28+
29+
@override
30+
void optimizeComposition() {
31+
optimizeCompositionCalled = true;
32+
frameSizeDuringOptimize = _capturedFrameSize;
33+
}
34+
35+
@override
36+
Future<void> submitFrame(FrameTimingRecorder? recorder) async {
37+
await rasterizer.rasterize(<DisplayCanvas>[], <ui.Picture>[], recorder);
38+
}
39+
40+
@override
41+
Iterable<LayerCanvas> getOptimizedCanvases() => <LayerCanvas>[];
42+
}
43+
44+
class OrderVerifyingRasterizer extends ViewRasterizer {
45+
OrderVerifyingRasterizer(super.view);
46+
47+
bool prepareToDrawCalled = false;
48+
bool rasterizeCalled = false;
49+
50+
// We track the state of these flags when each method is called
51+
bool? optimizeCompositionCalledDuringPrepare;
52+
bool? prepareToDrawCalledDuringRasterize;
53+
BitmapSize? sizeDuringPrepare;
54+
BitmapSize? sizeDuringRasterize;
55+
56+
late final MockViewEmbedder _viewEmbedder = MockViewEmbedder(sceneElement, this);
57+
58+
@override
59+
MockViewEmbedder get viewEmbedder => _viewEmbedder;
60+
61+
@override
62+
DisplayCanvasFactory<DisplayCanvas> get displayFactory => throw UnimplementedError();
63+
64+
@override
65+
Future<void> prepareToDraw() async {
66+
prepareToDrawCalled = true;
67+
sizeDuringPrepare = currentFrameSize;
68+
optimizeCompositionCalledDuringPrepare = viewEmbedder.optimizeCompositionCalled;
69+
}
70+
71+
@override
72+
Future<void> rasterize(
73+
List<DisplayCanvas> displayCanvases,
74+
List<ui.Picture> pictures,
75+
FrameTimingRecorder? recorder,
76+
) async {
77+
rasterizeCalled = true;
78+
sizeDuringRasterize = currentFrameSize;
79+
prepareToDrawCalledDuringRasterize = prepareToDrawCalled;
80+
}
81+
}
82+
83+
void testMain() {
84+
group('Rasterizer order', () {
85+
setUpUnitTests();
86+
87+
test('calls prepareToDraw after raster (optimizeComposition) and before rasterize', () async {
88+
final view = EngineFlutterView(
89+
EnginePlatformDispatcher.instance,
90+
domDocument.createElement('div'),
91+
);
92+
final rasterizer = OrderVerifyingRasterizer(view);
93+
94+
final rootLayer = RootLayer();
95+
final layerTree = LayerTree(rootLayer);
96+
97+
// physicalSize must be non-empty for draw() to proceed
98+
view.debugPhysicalSizeOverride = const ui.Size(100, 100);
99+
view.debugForceResize();
100+
101+
await rasterizer.draw(layerTree, null);
102+
103+
final MockViewEmbedder mockEmbedder = rasterizer.viewEmbedder;
104+
105+
expect(
106+
mockEmbedder.optimizeCompositionCalled,
107+
isTrue,
108+
reason: 'optimizeComposition should be called',
109+
);
110+
expect(rasterizer.prepareToDrawCalled, isTrue, reason: 'prepareToDraw should be called');
111+
expect(rasterizer.rasterizeCalled, isTrue, reason: 'rasterize should be called');
112+
113+
expect(
114+
rasterizer.optimizeCompositionCalledDuringPrepare,
115+
isTrue,
116+
reason: 'optimizeComposition (part of raster) should have been called before prepareToDraw',
117+
);
118+
119+
expect(
120+
rasterizer.prepareToDrawCalledDuringRasterize,
121+
isTrue,
122+
reason: 'prepareToDraw should have been called before rasterize (part of submitFrame)',
123+
);
124+
});
125+
126+
test('currentFrameSize is updated before prepareToDraw', () async {
127+
final view = EngineFlutterView(
128+
EnginePlatformDispatcher.instance,
129+
domDocument.createElement('div'),
130+
);
131+
final rasterizer = OrderVerifyingRasterizer(view);
132+
133+
final rootLayer = RootLayer();
134+
final layerTree = LayerTree(rootLayer);
135+
136+
view.debugPhysicalSizeOverride = const ui.Size(123, 456);
137+
view.debugForceResize();
138+
139+
await rasterizer.draw(layerTree, null);
140+
141+
expect(rasterizer.sizeDuringPrepare, const BitmapSize(123, 456));
142+
});
143+
144+
test('renders two frames at different sizes and uses the correct size for each', () async {
145+
final view = EngineFlutterView(
146+
EnginePlatformDispatcher.instance,
147+
domDocument.createElement('div'),
148+
);
149+
final rasterizer = OrderVerifyingRasterizer(view);
150+
final MockViewEmbedder mockEmbedder = rasterizer.viewEmbedder;
151+
152+
final rootLayer = RootLayer();
153+
final layerTree = LayerTree(rootLayer);
154+
155+
// Frame 1: 100x200
156+
view.debugPhysicalSizeOverride = const ui.Size(100, 200);
157+
view.debugForceResize();
158+
await rasterizer.draw(layerTree, null);
159+
160+
expect(mockEmbedder.frameSizeDuringOptimize, const BitmapSize(100, 200));
161+
expect(rasterizer.sizeDuringPrepare, const BitmapSize(100, 200));
162+
expect(rasterizer.sizeDuringRasterize, const BitmapSize(100, 200));
163+
164+
// Frame 2: 300x400
165+
view.debugPhysicalSizeOverride = const ui.Size(300, 400);
166+
view.debugForceResize();
167+
await rasterizer.draw(layerTree, null);
168+
169+
expect(mockEmbedder.frameSizeDuringOptimize, const BitmapSize(300, 400));
170+
expect(rasterizer.sizeDuringPrepare, const BitmapSize(300, 400));
171+
expect(rasterizer.sizeDuringRasterize, const BitmapSize(300, 400));
172+
});
173+
});
174+
}

‎engine/src/flutter/lib/web_ui/test/ui/rasterizer_resize_golden_test.dart‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:math' as math;
6+
7+
import 'package:test/bootstrap/browser.dart';
8+
import 'package:test/test.dart';
9+
import 'package:ui/src/engine.dart';
10+
import 'package:ui/ui.dart' as ui;
11+
import 'package:web_engine_tester/golden_tester.dart';
12+
13+
import '../common/test_initialization.dart';
14+
import 'utils.dart';
15+
16+
void main() {
17+
internalBootstrapBrowserTest(() => testMain);
18+
}
19+
20+
void testMain() {
21+
group('Rasterizer resize', () {
22+
setUpUnitTests(withImplicitView: true, setUpTestViewDimensions: false);
23+
24+
test('renders correctly after resizing the view', () async {
25+
final view = implicitView as EngineFlutterView;
26+
27+
// 1. Initial size 200x200
28+
view.debugPhysicalSizeOverride = const ui.Size(200, 200);
29+
view.debugForceResize();
30+
31+
Future<void> drawCenteredCircle(ui.Size size, ui.Color color) async {
32+
final recorder = ui.PictureRecorder();
33+
final canvas = ui.Canvas(recorder);
34+
canvas.drawCircle(
35+
ui.Offset(size.width / 2, size.height / 2),
36+
math.min(size.width, size.height) / 4,
37+
ui.Paint()..color = color,
38+
);
39+
final ui.Picture picture = recorder.endRecording();
40+
final sb = ui.SceneBuilder();
41+
sb.addPicture(ui.Offset.zero, picture);
42+
await renderer.renderScene(sb.build(), view);
43+
}
44+
45+
// Draw first frame at 200x200. This ensures the rasterizer is initialized
46+
// and has a "current" size.
47+
await drawCenteredCircle(const ui.Size(200, 200), const ui.Color(0xFFFF0000));
48+
49+
// 2. Resize to 400x400 and draw again.
50+
// This tests that the second frame correctly uses the new size for
51+
// measurement and painting, even if it happens immediately after the resize.
52+
view.debugPhysicalSizeOverride = const ui.Size(400, 400);
53+
view.debugForceResize();
54+
55+
// Draw second frame at 400x400 with a green circle.
56+
// If the reordering was incorrect, the circle might be centered at (100, 100)
57+
// instead of (200, 200), or it might be clipped.
58+
await drawCenteredCircle(const ui.Size(400, 400), const ui.Color(0xFF00FF00));
59+
60+
await matchGoldenFile(
61+
'ui_resize_centered_circle.png',
62+
region: const ui.Rect.fromLTWH(0, 0, 400, 400),
63+
);
64+
});
65+
});
66+
}

0 commit comments

Comments
 (0)