From 5c9f67a178b071a9838416a513afc4e58dfd455e Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 25 Aug 2021 15:07:52 -0700 Subject: [PATCH 1/2] Add flag to disable overlays --- .../src/engine/canvaskit/embedded_views.dart | 102 ++++++++++++------ .../lib/src/engine/canvaskit/layer.dart | 10 +- 2 files changed, 74 insertions(+), 38 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart index 702ffe01e25f5..b521a2557dacd 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -32,6 +32,16 @@ class HtmlViewEmbedder { defaultValue: 8, ); + /// If `true`, overlay canvases are disabled. + /// + /// This causes all drawing to go to a single canvas, with all of the platform + /// views rendered over top. This may result in incorrect rendering with + /// platform views. + static const bool disableOverlays = bool.fromEnvironment( + 'FLUTTER_WEB_DISABLE_OVERLAYS', + defaultValue: false, + ); + /// The picture recorder shared by all platform views which paint to the /// backup surface. CkPictureRecorder? _backupPictureRecorder; @@ -86,6 +96,9 @@ class HtmlViewEmbedder { } List getCurrentCanvases() { + if (disableOverlays) { + return const []; + } final Set canvases = {}; for (int i = 0; i < _compositionOrder.length; i++) { final int viewId = _compositionOrder[i]; @@ -95,21 +108,23 @@ class HtmlViewEmbedder { } void prerollCompositeEmbeddedView(int viewId, EmbeddedViewParams params) { - _ensureOverlayInitialized(viewId); - if (_viewsUsingBackupSurface.contains(viewId)) { - if (_backupPictureRecorder == null) { - // Only initialize the picture recorder for the backup surface once. + if (!disableOverlays) { + _ensureOverlayInitialized(viewId); + if (_viewsUsingBackupSurface.contains(viewId)) { + if (_backupPictureRecorder == null) { + // Only initialize the picture recorder for the backup surface once. + final CkPictureRecorder pictureRecorder = CkPictureRecorder(); + pictureRecorder.beginRecording(ui.Offset.zero & _frameSize); + pictureRecorder.recordingCanvas!.clear(const ui.Color(0x00000000)); + _backupPictureRecorder = pictureRecorder; + } + _pictureRecorders[viewId] = _backupPictureRecorder!; + } else { final CkPictureRecorder pictureRecorder = CkPictureRecorder(); pictureRecorder.beginRecording(ui.Offset.zero & _frameSize); pictureRecorder.recordingCanvas!.clear(const ui.Color(0x00000000)); - _backupPictureRecorder = pictureRecorder; + _pictureRecorders[viewId] = pictureRecorder; } - _pictureRecorders[viewId] = _backupPictureRecorder!; - } else { - final CkPictureRecorder pictureRecorder = CkPictureRecorder(); - pictureRecorder.beginRecording(ui.Offset.zero & _frameSize); - pictureRecorder.recordingCanvas!.clear(const ui.Color(0x00000000)); - _pictureRecorders[viewId] = pictureRecorder; } _compositionOrder.add(viewId); @@ -121,14 +136,26 @@ class HtmlViewEmbedder { _viewsToRecomposite.add(viewId); } + /// Prepares to composite [viewId]. + /// + /// If this returns a [CkCanvas], then that canvas should be the new leaf + /// node. Otherwise, keep the same leaf node. CkCanvas? compositeEmbeddedView(int viewId) { // Do nothing if this view doesn't need to be composited. if (!_viewsToRecomposite.contains(viewId)) { - return _pictureRecorders[viewId]!.recordingCanvas; + if (!disableOverlays) { + return _pictureRecorders[viewId]!.recordingCanvas; + } else { + return null; + } } _compositeWithParams(viewId, _currentCompositionParams[viewId]!); _viewsToRecomposite.remove(viewId); - return _pictureRecorders[viewId]!.recordingCanvas; + if (!disableOverlays) { + return _pictureRecorders[viewId]!.recordingCanvas; + } else { + return null; + } } void _compositeWithParams(int viewId, EmbeddedViewParams params) { @@ -351,26 +378,29 @@ class HtmlViewEmbedder { void submitFrame() { bool _didPaintBackupSurface = false; - for (int i = 0; i < _compositionOrder.length; i++) { - final int viewId = _compositionOrder[i]; - if (_viewsUsingBackupSurface.contains(viewId)) { - // Only draw the picture to the backup surface once. - if (!_didPaintBackupSurface) { - final SurfaceFrame backupFrame = - SurfaceFactory.instance.backupSurface.acquireFrame(_frameSize); - backupFrame.skiaCanvas - .drawPicture(_backupPictureRecorder!.endRecording()); - _backupPictureRecorder = null; - backupFrame.submit(); - _didPaintBackupSurface = true; + if (!disableOverlays) { + for (int i = 0; i < _compositionOrder.length; i++) { + final int viewId = _compositionOrder[i]; + if (_viewsUsingBackupSurface.contains(viewId)) { + // Only draw the picture to the backup surface once. + if (!_didPaintBackupSurface) { + final SurfaceFrame backupFrame = + SurfaceFactory.instance.backupSurface.acquireFrame(_frameSize); + backupFrame.skiaCanvas + .drawPicture(_backupPictureRecorder!.endRecording()); + _backupPictureRecorder = null; + backupFrame.submit(); + _didPaintBackupSurface = true; + } + } else { + final SurfaceFrame frame = + _overlays[viewId]!.acquireFrame(_frameSize); + final CkCanvas canvas = frame.skiaCanvas; + canvas.drawPicture( + _pictureRecorders[viewId]!.endRecording(), + ); + frame.submit(); } - } else { - final SurfaceFrame frame = _overlays[viewId]!.acquireFrame(_frameSize); - final CkCanvas canvas = frame.skiaCanvas; - canvas.drawPicture( - _pictureRecorders[viewId]!.endRecording(), - ); - frame.submit(); } } if (listEquals(_compositionOrder, _activeCompositionOrder)) { @@ -396,11 +426,13 @@ class HtmlViewEmbedder { unusedViews.remove(viewId); final html.Element platformViewRoot = _viewClipChains[viewId]!.root; - final html.Element overlay = _overlays[viewId]!.htmlElement; platformViewRoot.remove(); skiaSceneHost!.append(platformViewRoot); - overlay.remove(); - skiaSceneHost!.append(overlay); + if (!disableOverlays) { + final html.Element overlay = _overlays[viewId]!.htmlElement; + overlay.remove(); + skiaSceneHost!.append(overlay); + } _activeCompositionOrder.add(viewId); } if (_didPaintBackupSurface) { diff --git a/lib/web_ui/lib/src/engine/canvaskit/layer.dart b/lib/web_ui/lib/src/engine/canvaskit/layer.dart index d061e4ab8897b..74b1b18a00216 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/layer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/layer.dart @@ -363,7 +363,8 @@ class TransformEngineLayer extends ContainerLayer void preroll(PrerollContext prerollContext, Matrix4 matrix) { final Matrix4 childMatrix = matrix.multiplied(_transform); prerollContext.mutatorsStack.pushTransform(_transform); - final ui.Rect childPaintBounds = prerollChildren(prerollContext, childMatrix); + final ui.Rect childPaintBounds = + prerollChildren(prerollContext, childMatrix); paintBounds = transformRect(_transform, childPaintBounds); prerollContext.mutatorsStack.pop(); } @@ -602,7 +603,10 @@ class PlatformViewLayer extends Layer { @override void paint(PaintContext paintContext) { - final CkCanvas? canvas = paintContext.viewEmbedder!.compositeEmbeddedView(viewId); - paintContext.leafNodesCanvas = canvas; + final CkCanvas? canvas = + paintContext.viewEmbedder!.compositeEmbeddedView(viewId); + if (canvas != null) { + paintContext.leafNodesCanvas = canvas; + } } } From a72835f4ee65b354460dbd476fcd1078d841d98b Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 25 Aug 2021 16:46:57 -0700 Subject: [PATCH 2/2] Make disableOverlays = maximumOverlays == 1 --- .../src/engine/canvaskit/embedded_views.dart | 9 +++------ .../src/engine/canvaskit/surface_factory.dart | 6 +++--- .../test/canvaskit/embedded_views_test.dart | 20 +++++++++---------- .../test/canvaskit/surface_factory_test.dart | 4 ++-- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart index b521a2557dacd..9e62d9320e0d0 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -27,8 +27,8 @@ class HtmlViewEmbedder { HtmlViewEmbedder._(); /// The maximum number of overlay surfaces that can be live at once. - static const int maximumOverlaySurfaces = int.fromEnvironment( - 'FLUTTER_WEB_MAXIMUM_OVERLAYS', + static const int maximumSurfaces = int.fromEnvironment( + 'FLUTTER_WEB_MAXIMUM_SURFACES', defaultValue: 8, ); @@ -37,10 +37,7 @@ class HtmlViewEmbedder { /// This causes all drawing to go to a single canvas, with all of the platform /// views rendered over top. This may result in incorrect rendering with /// platform views. - static const bool disableOverlays = bool.fromEnvironment( - 'FLUTTER_WEB_DISABLE_OVERLAYS', - defaultValue: false, - ); + static const bool disableOverlays = maximumSurfaces <= 1; /// The picture recorder shared by all platform views which paint to the /// backup surface. diff --git a/lib/web_ui/lib/src/engine/canvaskit/surface_factory.dart b/lib/web_ui/lib/src/engine/canvaskit/surface_factory.dart index 70cf9bfc68c9d..a6ec4bb814188 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/surface_factory.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/surface_factory.dart @@ -12,11 +12,11 @@ import 'surface.dart'; class SurfaceFactory { /// The cache singleton. static final SurfaceFactory instance = - SurfaceFactory(HtmlViewEmbedder.maximumOverlaySurfaces); + SurfaceFactory(HtmlViewEmbedder.maximumSurfaces); SurfaceFactory(this.maximumSurfaces) - : assert(maximumSurfaces >= 2, - 'The maximum number of surfaces must be at least 2'); + : assert(maximumSurfaces >= 1, + 'The maximum number of surfaces must be at least 1'); /// The base surface to paint on. This is the default surface which will be /// painted to. If there are no platform views, then this surface will receive diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index 8f0669be05a52..e754166b3d4f8 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -213,7 +213,7 @@ void testMain() { // Initialize all platform views to be used in the test. final List platformViewIds = []; - for (int i = 0; i < HtmlViewEmbedder.maximumOverlaySurfaces * 2; i++) { + for (int i = 0; i < HtmlViewEmbedder.maximumSurfaces * 2; i++) { ui.platformViewRegistry.registerViewFactory( 'test-platform-view', (int viewId) => html.DivElement()..id = 'view-$i', @@ -242,8 +242,8 @@ void testMain() { // Frame 1: // Render: up to cache size platform views. // Expect: main canvas plus platform view overlays. - renderTestScene(viewCount: HtmlViewEmbedder.maximumOverlaySurfaces); - expect(countCanvases(), HtmlViewEmbedder.maximumOverlaySurfaces); + renderTestScene(viewCount: HtmlViewEmbedder.maximumSurfaces); + expect(countCanvases(), HtmlViewEmbedder.maximumSurfaces); // Frame 2: // Render: zero platform views. @@ -256,15 +256,15 @@ void testMain() { // Render: less than cache size platform views. // Expect: overlays reused. await Future.delayed(Duration.zero); - renderTestScene(viewCount: HtmlViewEmbedder.maximumOverlaySurfaces - 2); - expect(countCanvases(), HtmlViewEmbedder.maximumOverlaySurfaces - 1); + renderTestScene(viewCount: HtmlViewEmbedder.maximumSurfaces - 2); + expect(countCanvases(), HtmlViewEmbedder.maximumSurfaces - 1); // Frame 4: // Render: more platform views than max cache size. // Expect: main canvas, backup overlay, maximum overlays. await Future.delayed(Duration.zero); - renderTestScene(viewCount: HtmlViewEmbedder.maximumOverlaySurfaces * 2); - expect(countCanvases(), HtmlViewEmbedder.maximumOverlaySurfaces); + renderTestScene(viewCount: HtmlViewEmbedder.maximumSurfaces * 2); + expect(countCanvases(), HtmlViewEmbedder.maximumSurfaces); // Frame 5: // Render: zero platform views. @@ -346,21 +346,21 @@ void testMain() { // Render: Views 1-10 // Expect: main canvas plus platform view overlays; empty cache. renderTestScene([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); - expect(countCanvases(), HtmlViewEmbedder.maximumOverlaySurfaces); + expect(countCanvases(), HtmlViewEmbedder.maximumSurfaces); // Frame 2: // Render: Views 2-11 // Expect: main canvas plus platform view overlays; empty cache. await Future.delayed(Duration.zero); renderTestScene([2, 3, 4, 5, 6, 7, 8, 9, 10, 11]); - expect(countCanvases(), HtmlViewEmbedder.maximumOverlaySurfaces); + expect(countCanvases(), HtmlViewEmbedder.maximumSurfaces); // Frame 3: // Render: Views 3-12 // Expect: main canvas plus platform view overlays; empty cache. await Future.delayed(Duration.zero); renderTestScene([3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); - expect(countCanvases(), HtmlViewEmbedder.maximumOverlaySurfaces); + expect(countCanvases(), HtmlViewEmbedder.maximumSurfaces); // TODO(yjbanov): skipped due to https://github.com/flutter/flutter/issues/73867 }, skip: isSafari); diff --git a/lib/web_ui/test/canvaskit/surface_factory_test.dart b/lib/web_ui/test/canvaskit/surface_factory_test.dart index 77b03269ad03e..f0be0203ffa8d 100644 --- a/lib/web_ui/test/canvaskit/surface_factory_test.dart +++ b/lib/web_ui/test/canvaskit/surface_factory_test.dart @@ -19,10 +19,10 @@ void testMain() { group('$SurfaceFactory', () { setUpCanvasKitTest(); - test('cannot be created with size less than 2', () { + test('cannot be created with size less than 1', () { expect(() => SurfaceFactory(-1), throwsAssertionError); expect(() => SurfaceFactory(0), throwsAssertionError); - expect(() => SurfaceFactory(1), throwsAssertionError); + expect(SurfaceFactory(1), isNotNull); expect(SurfaceFactory(2), isNotNull); });