chore(canvaskit): remove SurfaceFrame from Surface (#162825)
SurfaceFrame is no longer used. This is a cleanup in preparation for a larger refactor of `Surface` to improve performance. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
Harry Terkelsen
GitHub
parent
a6344bf2b3
commit
db7e82bdfb
@ -23,27 +23,6 @@ import 'util.dart';
|
||||
// removes the ability for disabling AA on Paint objects.
|
||||
const bool _kUsingMSAA = bool.fromEnvironment('flutter.canvaskit.msaa');
|
||||
|
||||
typedef SubmitCallback = bool Function(SurfaceFrame, CkCanvas);
|
||||
|
||||
/// A frame which contains a canvas to be drawn into.
|
||||
class SurfaceFrame {
|
||||
SurfaceFrame(this.skiaSurface, this.submitCallback) : _submitted = false;
|
||||
|
||||
final CkSurface skiaSurface;
|
||||
final SubmitCallback submitCallback;
|
||||
final bool _submitted;
|
||||
|
||||
/// Submit this frame to be drawn.
|
||||
bool submit() {
|
||||
if (_submitted) {
|
||||
return false;
|
||||
}
|
||||
return submitCallback(this, skiaCanvas);
|
||||
}
|
||||
|
||||
CkCanvas get skiaCanvas => skiaSurface.getCanvas();
|
||||
}
|
||||
|
||||
/// A surface which can be drawn into by the compositor.
|
||||
///
|
||||
/// The underlying representation is a [CkSurface], which can be reused by
|
||||
@ -55,6 +34,19 @@ class Surface extends DisplayCanvas {
|
||||
|
||||
CkSurface? _surface;
|
||||
|
||||
/// Returns the underlying CanvasKit Surface. Should only be used in tests.
|
||||
CkSurface? debugGetCkSurface() {
|
||||
bool assertsEnabled = false;
|
||||
assert(() {
|
||||
assertsEnabled = true;
|
||||
return true;
|
||||
}());
|
||||
if (!assertsEnabled) {
|
||||
throw StateError('debugGetCkSurface() can only be used in tests');
|
||||
}
|
||||
return _surface;
|
||||
}
|
||||
|
||||
/// Whether or not to use an `OffscreenCanvas` to back this [Surface].
|
||||
final bool useOffscreenCanvas;
|
||||
|
||||
@ -97,7 +89,17 @@ class Surface extends DisplayCanvas {
|
||||
DomOffscreenCanvas? _offscreenCanvas;
|
||||
|
||||
/// Returns the underlying OffscreenCanvas. Should only be used in tests.
|
||||
DomOffscreenCanvas? get debugOffscreenCanvas => _offscreenCanvas;
|
||||
DomOffscreenCanvas? debugGetOffscreenCanvas() {
|
||||
bool assertsEnabled = false;
|
||||
assert(() {
|
||||
assertsEnabled = true;
|
||||
return true;
|
||||
}());
|
||||
if (!assertsEnabled) {
|
||||
throw StateError('debugGetOffscreenCanvas() can only be used in tests');
|
||||
}
|
||||
return _offscreenCanvas;
|
||||
}
|
||||
|
||||
/// The <canvas> backing this Surface in the case that OffscreenCanvas isn't
|
||||
/// supported.
|
||||
@ -172,20 +174,6 @@ class Surface extends DisplayCanvas {
|
||||
}
|
||||
}
|
||||
|
||||
/// Acquire a frame of the given [size] containing a drawable canvas.
|
||||
///
|
||||
/// The given [size] is in physical pixels.
|
||||
SurfaceFrame acquireFrame(ui.Size size) {
|
||||
final CkSurface surface = createOrUpdateSurface(BitmapSize.fromSize(size));
|
||||
|
||||
// ignore: prefer_function_declarations_over_variables
|
||||
final SubmitCallback submitCallback = (SurfaceFrame surfaceFrame, CkCanvas canvas) {
|
||||
return _presentSurface();
|
||||
};
|
||||
|
||||
return SurfaceFrame(surface, submitCallback);
|
||||
}
|
||||
|
||||
BitmapSize? _currentCanvasPhysicalSize;
|
||||
|
||||
/// Sets the CSS size of the canvas so that canvas pixels are 1:1 with device
|
||||
@ -283,9 +271,9 @@ class Surface extends DisplayCanvas {
|
||||
}
|
||||
}
|
||||
|
||||
// If we reached here, then either we are forcing a new context, or
|
||||
// the size of the surface has changed so we need to make a new one.
|
||||
|
||||
// If we reached here, then this is the first frame and we haven't made a
|
||||
// surface yet, we are forcing a new context, or the size of the surface
|
||||
// has changed and we need to make a new one.
|
||||
_surface?.dispose();
|
||||
_surface = null;
|
||||
|
||||
@ -489,11 +477,6 @@ class Surface extends DisplayCanvas {
|
||||
}
|
||||
}
|
||||
|
||||
bool _presentSurface() {
|
||||
_surface!.flush();
|
||||
return true;
|
||||
}
|
||||
|
||||
@override
|
||||
bool get isConnected => _canvasElement!.isConnected!;
|
||||
|
||||
|
||||
@ -25,8 +25,9 @@ void testMain() {
|
||||
|
||||
test('Surface allocates canvases efficiently', () {
|
||||
final Surface surface = Surface();
|
||||
final CkSurface originalSurface = surface.acquireFrame(const ui.Size(9, 19)).skiaSurface;
|
||||
final DomOffscreenCanvas original = surface.debugOffscreenCanvas!;
|
||||
surface.createOrUpdateSurface(const BitmapSize(9, 19));
|
||||
final CkSurface originalSurface = surface.debugGetCkSurface()!;
|
||||
final DomOffscreenCanvas original = surface.debugGetOffscreenCanvas()!;
|
||||
|
||||
// Expect exact requested dimensions.
|
||||
expect(original.width, 9);
|
||||
@ -36,8 +37,9 @@ void testMain() {
|
||||
|
||||
// Shrinking causes the surface to create a new canvas with the exact
|
||||
// size requested.
|
||||
final CkSurface shrunkSurface = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface;
|
||||
final DomOffscreenCanvas shrunk = surface.debugOffscreenCanvas!;
|
||||
surface.createOrUpdateSurface(const BitmapSize(5, 15));
|
||||
final CkSurface shrunkSurface = surface.debugGetCkSurface()!;
|
||||
final DomOffscreenCanvas shrunk = surface.debugGetOffscreenCanvas()!;
|
||||
expect(shrunk, same(original));
|
||||
expect(shrunkSurface, isNot(same(originalSurface)));
|
||||
expect(shrunkSurface.width(), 5);
|
||||
@ -45,9 +47,9 @@ void testMain() {
|
||||
|
||||
// The first increase will allocate a new surface to exactly the
|
||||
// requested size.
|
||||
final CkSurface firstIncreaseSurface =
|
||||
surface.acquireFrame(const ui.Size(10, 20)).skiaSurface;
|
||||
final DomOffscreenCanvas firstIncrease = surface.debugOffscreenCanvas!;
|
||||
surface.createOrUpdateSurface(const BitmapSize(10, 20));
|
||||
final CkSurface firstIncreaseSurface = surface.debugGetCkSurface()!;
|
||||
final DomOffscreenCanvas firstIncrease = surface.debugGetOffscreenCanvas()!;
|
||||
expect(firstIncrease, same(original));
|
||||
expect(firstIncreaseSurface, isNot(same(shrunkSurface)));
|
||||
|
||||
@ -58,17 +60,18 @@ void testMain() {
|
||||
expect(firstIncreaseSurface.height(), 20);
|
||||
|
||||
// Subsequent increases within 40% will still allocate a new canvas.
|
||||
final CkSurface secondIncreaseSurface =
|
||||
surface.acquireFrame(const ui.Size(11, 22)).skiaSurface;
|
||||
final DomOffscreenCanvas secondIncrease = surface.debugOffscreenCanvas!;
|
||||
surface.createOrUpdateSurface(const BitmapSize(11, 22));
|
||||
final CkSurface secondIncreaseSurface = surface.debugGetCkSurface()!;
|
||||
final DomOffscreenCanvas secondIncrease = surface.debugGetOffscreenCanvas()!;
|
||||
expect(secondIncrease, same(firstIncrease));
|
||||
expect(secondIncreaseSurface, isNot(same(firstIncreaseSurface)));
|
||||
expect(secondIncreaseSurface.width(), 11);
|
||||
expect(secondIncreaseSurface.height(), 22);
|
||||
|
||||
// Increases beyond the 40% limit will cause a new allocation.
|
||||
final CkSurface hugeSurface = surface.acquireFrame(const ui.Size(20, 40)).skiaSurface;
|
||||
final DomOffscreenCanvas huge = surface.debugOffscreenCanvas!;
|
||||
surface.createOrUpdateSurface(const BitmapSize(20, 40));
|
||||
final CkSurface hugeSurface = surface.debugGetCkSurface()!;
|
||||
final DomOffscreenCanvas huge = surface.debugGetOffscreenCanvas()!;
|
||||
expect(huge, same(secondIncrease));
|
||||
expect(hugeSurface, isNot(same(secondIncreaseSurface)));
|
||||
|
||||
@ -79,8 +82,9 @@ void testMain() {
|
||||
expect(hugeSurface.height(), 40);
|
||||
|
||||
// Shrink again. Create a new surface.
|
||||
final CkSurface shrunkSurface2 = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface;
|
||||
final DomOffscreenCanvas shrunk2 = surface.debugOffscreenCanvas!;
|
||||
surface.createOrUpdateSurface(const BitmapSize(5, 15));
|
||||
final CkSurface shrunkSurface2 = surface.debugGetCkSurface()!;
|
||||
final DomOffscreenCanvas shrunk2 = surface.debugGetOffscreenCanvas()!;
|
||||
expect(shrunk2, same(huge));
|
||||
expect(shrunkSurface2, isNot(same(hugeSurface)));
|
||||
expect(shrunkSurface2.width(), 5);
|
||||
@ -89,12 +93,13 @@ void testMain() {
|
||||
// Doubling the DPR should halve the CSS width, height, and translation of the canvas.
|
||||
// This tests https://github.com/flutter/flutter/issues/77084
|
||||
EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(2.0);
|
||||
final CkSurface dpr2Surface2 = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface;
|
||||
final DomOffscreenCanvas dpr2Canvas = surface.debugOffscreenCanvas!;
|
||||
surface.createOrUpdateSurface(const BitmapSize(5, 15));
|
||||
final CkSurface dpr2Surface = surface.debugGetCkSurface()!;
|
||||
final DomOffscreenCanvas dpr2Canvas = surface.debugGetOffscreenCanvas()!;
|
||||
expect(dpr2Canvas, same(huge));
|
||||
expect(dpr2Surface2, isNot(same(hugeSurface)));
|
||||
expect(dpr2Surface2.width(), 5);
|
||||
expect(dpr2Surface2.height(), 15);
|
||||
expect(dpr2Surface, isNot(same(hugeSurface)));
|
||||
expect(dpr2Surface.width(), 5);
|
||||
expect(dpr2Surface.height(), 15);
|
||||
|
||||
// Skipping on Firefox for now since Firefox headless doesn't support WebGL
|
||||
// This causes issues in the test since we create a Canvas-backed surface,
|
||||
@ -195,17 +200,19 @@ void testMain() {
|
||||
() async {
|
||||
final Surface surface = Surface();
|
||||
expect(surface.debugForceNewContext, isTrue);
|
||||
final CkSurface before = surface.acquireFrame(const ui.Size(9, 19)).skiaSurface;
|
||||
surface.createOrUpdateSurface(const BitmapSize(9, 19));
|
||||
final CkSurface before = surface.debugGetCkSurface()!;
|
||||
expect(surface.debugForceNewContext, isFalse);
|
||||
|
||||
// Pump a timer to flush any microtasks.
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
final CkSurface afterAcquireFrame = surface.acquireFrame(const ui.Size(9, 19)).skiaSurface;
|
||||
surface.createOrUpdateSurface(const BitmapSize(9, 19));
|
||||
final CkSurface afterAcquireFrame = surface.debugGetCkSurface()!;
|
||||
// Existing context is reused.
|
||||
expect(afterAcquireFrame, same(before));
|
||||
|
||||
// Emulate WebGL context loss.
|
||||
final DomOffscreenCanvas canvas = surface.debugOffscreenCanvas!;
|
||||
final DomOffscreenCanvas canvas = surface.debugGetOffscreenCanvas()!;
|
||||
final Object ctx = canvas.getContext('webgl2')!;
|
||||
final Object loseContextExtension = js_util.callMethod(ctx, 'getExtension', <String>[
|
||||
'WEBGL_lose_context',
|
||||
@ -226,7 +233,8 @@ void testMain() {
|
||||
await Future<void>.delayed(Duration.zero);
|
||||
expect(surface.debugForceNewContext, isTrue);
|
||||
|
||||
final CkSurface afterContextLost = surface.acquireFrame(const ui.Size(9, 19)).skiaSurface;
|
||||
surface.createOrUpdateSurface(const BitmapSize(9, 19));
|
||||
final CkSurface afterContextLost = surface.debugGetCkSurface()!;
|
||||
// A new context is created.
|
||||
expect(afterContextLost, isNot(same(before)));
|
||||
},
|
||||
@ -239,39 +247,42 @@ void testMain() {
|
||||
'updates canvas logical size when device-pixel ratio changes',
|
||||
() {
|
||||
final Surface surface = Surface();
|
||||
final CkSurface original = surface.acquireFrame(const ui.Size(10, 16)).skiaSurface;
|
||||
surface.createOrUpdateSurface(const BitmapSize(10, 16));
|
||||
final CkSurface original = surface.debugGetCkSurface()!;
|
||||
|
||||
expect(original.width(), 10);
|
||||
expect(original.height(), 16);
|
||||
expect(surface.debugOffscreenCanvas!.width, 10);
|
||||
expect(surface.debugOffscreenCanvas!.height, 16);
|
||||
expect(surface.debugGetOffscreenCanvas()!.width, 10);
|
||||
expect(surface.debugGetOffscreenCanvas()!.height, 16);
|
||||
|
||||
// Increase device-pixel ratio: this makes CSS pixels bigger, so we need
|
||||
// fewer of them to cover the browser window.
|
||||
EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(2.0);
|
||||
final CkSurface highDpr = surface.acquireFrame(const ui.Size(10, 16)).skiaSurface;
|
||||
surface.createOrUpdateSurface(const BitmapSize(10, 16));
|
||||
final CkSurface highDpr = surface.debugGetCkSurface()!;
|
||||
expect(highDpr.width(), 10);
|
||||
expect(highDpr.height(), 16);
|
||||
expect(surface.debugOffscreenCanvas!.width, 10);
|
||||
expect(surface.debugOffscreenCanvas!.height, 16);
|
||||
expect(surface.debugGetOffscreenCanvas()!.width, 10);
|
||||
expect(surface.debugGetOffscreenCanvas()!.height, 16);
|
||||
|
||||
// Decrease device-pixel ratio: this makes CSS pixels smaller, so we need
|
||||
// more of them to cover the browser window.
|
||||
EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(0.5);
|
||||
final CkSurface lowDpr = surface.acquireFrame(const ui.Size(10, 16)).skiaSurface;
|
||||
surface.createOrUpdateSurface(const BitmapSize(10, 16));
|
||||
final CkSurface lowDpr = surface.debugGetCkSurface()!;
|
||||
expect(lowDpr.width(), 10);
|
||||
expect(lowDpr.height(), 16);
|
||||
expect(surface.debugOffscreenCanvas!.width, 10);
|
||||
expect(surface.debugOffscreenCanvas!.height, 16);
|
||||
expect(surface.debugGetOffscreenCanvas()!.width, 10);
|
||||
expect(surface.debugGetOffscreenCanvas()!.height, 16);
|
||||
|
||||
// See https://github.com/flutter/flutter/issues/77084#issuecomment-1120151172
|
||||
EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(2.0);
|
||||
final CkSurface changeRatioAndSize =
|
||||
surface.acquireFrame(const ui.Size(9.9, 15.9)).skiaSurface;
|
||||
surface.createOrUpdateSurface(BitmapSize.fromSize(const ui.Size(9.9, 15.9)));
|
||||
final CkSurface changeRatioAndSize = surface.debugGetCkSurface()!;
|
||||
expect(changeRatioAndSize.width(), 10);
|
||||
expect(changeRatioAndSize.height(), 16);
|
||||
expect(surface.debugOffscreenCanvas!.width, 10);
|
||||
expect(surface.debugOffscreenCanvas!.height, 16);
|
||||
expect(surface.debugGetOffscreenCanvas()!.width, 10);
|
||||
expect(surface.debugGetOffscreenCanvas()!.height, 16);
|
||||
},
|
||||
skip: !Surface.offscreenCanvasSupported,
|
||||
);
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user