[canvaskit] Makes access to CkSurface null-safer (flutter/engine#54895)

Handles exceptions coming from `MakeSWCanvasSurface` and `MakeOffscreenSWCanvasSurface`.

Also makes surface size a member of `CkSurface` to make sure that it's always in sync with `surface`.

Fixes https://github.com/flutter/flutter/issues/154402

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
K. P. Krasiński-Sroka 2024-10-23 00:27:00 +02:00 committed by GitHub
parent 7ebba40ad4
commit 10f2a2ccd1
2 changed files with 61 additions and 22 deletions

View File

@ -71,6 +71,10 @@ class Surface extends DisplayCanvas {
bool _contextLost = false; bool _contextLost = false;
bool get debugContextLost => _contextLost; bool get debugContextLost => _contextLost;
/// Forces AssertionError when attempting to create a CPU-based surface.
/// Only for tests.
bool debugThrowOnSoftwareSurfaceCreation = false;
/// A cached copy of the most recently created `webglcontextlost` listener. /// A cached copy of the most recently created `webglcontextlost` listener.
/// ///
/// We must cache this function because each time we access the tear-off it /// We must cache this function because each time we access the tear-off it
@ -182,7 +186,6 @@ class Surface extends DisplayCanvas {
} }
BitmapSize? _currentCanvasPhysicalSize; BitmapSize? _currentCanvasPhysicalSize;
BitmapSize? _currentSurfaceSize;
/// Sets the CSS size of the canvas so that canvas pixels are 1:1 with device /// Sets the CSS size of the canvas so that canvas pixels are 1:1 with device
/// pixels. /// pixels.
@ -250,7 +253,7 @@ class Surface extends DisplayCanvas {
if (!_forceNewContext) { if (!_forceNewContext) {
// Check if the window is the same size as before, and if so, don't allocate // Check if the window is the same size as before, and if so, don't allocate
// a new canvas as the previous canvas is big enough to fit everything. // a new canvas as the previous canvas is big enough to fit everything.
final BitmapSize? previousSurfaceSize = _currentSurfaceSize; final BitmapSize? previousSurfaceSize = _surface?._size;
if (previousSurfaceSize != null && if (previousSurfaceSize != null &&
size.width == previousSurfaceSize.width && size.width == previousSurfaceSize.width &&
size.height == previousSurfaceSize.height) { size.height == previousSurfaceSize.height) {
@ -299,10 +302,8 @@ class Surface extends DisplayCanvas {
_currentCanvasPhysicalSize = size; _currentCanvasPhysicalSize = size;
} }
_currentSurfaceSize = size;
_surface?.dispose(); _surface?.dispose();
_surface = _createNewSurface(size); return _surface = _createNewSurface(size);
return _surface!;
} }
JSVoid _contextRestoredListener(DomEvent event) { JSVoid _contextRestoredListener(DomEvent event) {
@ -462,11 +463,17 @@ class Surface extends DisplayCanvas {
CkSurface _createNewSurface(BitmapSize size) { CkSurface _createNewSurface(BitmapSize size) {
assert(_offscreenCanvas != null || _canvasElement != null); assert(_offscreenCanvas != null || _canvasElement != null);
if (webGLVersion == -1) { if (webGLVersion == -1) {
return _makeSoftwareCanvasSurface('WebGL support not detected'); return _makeSoftwareCanvasSurface('WebGL support not detected', size);
} else if (configuration.canvasKitForceCpuOnly) { } else if (configuration.canvasKitForceCpuOnly) {
return _makeSoftwareCanvasSurface('CPU rendering forced by application'); return _makeSoftwareCanvasSurface(
'CPU rendering forced by application',
size,
);
} else if (_glContext == 0) { } else if (_glContext == 0) {
return _makeSoftwareCanvasSurface('Failed to initialize WebGL context'); return _makeSoftwareCanvasSurface(
'Failed to initialize WebGL context',
size,
);
} else { } else {
final SkSurface? skSurface = canvasKit.MakeOnScreenGLSurface( final SkSurface? skSurface = canvasKit.MakeOnScreenGLSurface(
_grContext!, _grContext!,
@ -477,31 +484,37 @@ class Surface extends DisplayCanvas {
_stencilBits); _stencilBits);
if (skSurface == null) { if (skSurface == null) {
return _makeSoftwareCanvasSurface('Failed to initialize WebGL surface'); return _makeSoftwareCanvasSurface(
'Failed to initialize WebGL surface',
size,
);
} }
return CkSurface(skSurface, _glContext); return CkSurface(skSurface, _glContext, size);
} }
} }
static bool _didWarnAboutWebGlInitializationFailure = false; static bool _didWarnAboutWebGlInitializationFailure = false;
CkSurface _makeSoftwareCanvasSurface(String reason) { CkSurface _makeSoftwareCanvasSurface(String reason, BitmapSize size) {
if (!_didWarnAboutWebGlInitializationFailure) { if (!_didWarnAboutWebGlInitializationFailure) {
printWarning('WARNING: Falling back to CPU-only rendering. $reason.'); printWarning('WARNING: Falling back to CPU-only rendering. $reason.');
_didWarnAboutWebGlInitializationFailure = true; _didWarnAboutWebGlInitializationFailure = true;
} }
try {
assert(!debugThrowOnSoftwareSurfaceCreation);
SkSurface surface; SkSurface surface;
if (useOffscreenCanvas) { if (useOffscreenCanvas) {
surface = canvasKit.MakeOffscreenSWCanvasSurface(_offscreenCanvas!); surface = canvasKit.MakeOffscreenSWCanvasSurface(_offscreenCanvas!);
} else { } else {
surface = canvasKit.MakeSWCanvasSurface(_canvasElement!); surface = canvasKit.MakeSWCanvasSurface(_canvasElement!);
} }
return CkSurface( return CkSurface(surface, null, size);
surface, } catch (error) {
null, throw CanvasKitError('Failed to create CPU-based surface: $error.');
); }
} }
bool _presentSurface() { bool _presentSurface() {
@ -534,9 +547,9 @@ class Surface extends DisplayCanvas {
browserSupportsOffscreenCanvas && !isSafari; browserSupportsOffscreenCanvas && !isSafari;
} }
/// A Dart wrapper around Skia's CkSurface. /// A Dart wrapper around Skia's SkSurface.
class CkSurface { class CkSurface {
CkSurface(this.surface, this._glContext); CkSurface(this.surface, this._glContext, this._size);
CkCanvas getCanvas() { CkCanvas getCanvas() {
assert(!_isDisposed, 'Attempting to use the canvas of a disposed surface'); assert(!_isDisposed, 'Attempting to use the canvas of a disposed surface');
@ -549,6 +562,8 @@ class CkSurface {
/// at any moment. Storing it may lead to dangling pointer bugs. /// at any moment. Storing it may lead to dangling pointer bugs.
final SkSurface surface; final SkSurface surface;
final BitmapSize _size;
final int? _glContext; final int? _glContext;
/// Flushes the graphics to be rendered on screen. /// Flushes the graphics to be rendered on screen.

View File

@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:js_interop';
import 'dart:js_util' as js_util; import 'dart:js_util' as js_util;
import 'package:test/bootstrap/browser.dart'; import 'package:test/bootstrap/browser.dart';
@ -292,6 +293,29 @@ void testMain() {
}, },
skip: !Surface.offscreenCanvasSupported, skip: !Surface.offscreenCanvasSupported,
); );
test('can recover from MakeSWCanvasSurface failure', () async {
debugOverrideJsConfiguration(<String, Object?>{
'canvasKitForceCpuOnly': true,
}.jsify() as JsFlutterConfiguration?);
addTearDown(() => debugOverrideJsConfiguration(null));
final Surface surface = Surface();
surface.debugThrowOnSoftwareSurfaceCreation = true;
expect(
() => surface.createOrUpdateSurface(const BitmapSize(12, 34)),
throwsA(isA<CanvasKitError>()),
);
await Future<void>.delayed(Duration.zero);
expect(surface.debugForceNewContext, isFalse);
surface.debugThrowOnSoftwareSurfaceCreation = false;
final ckSurface = surface.createOrUpdateSurface(const BitmapSize(12, 34));
expect(ckSurface.surface.width(), 12);
expect(ckSurface.surface.height(), 34);
});
}); });
} }