From e720b6a6bd10a9180482662b921f10e3185ff6f9 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 30 Nov 2021 22:11:56 -0800 Subject: [PATCH] Revert "Non painting platform views (#30003)" This reverts commit d280475791dbfd8a47255ef9d88b46e8b7e29648. --- .../src/engine/canvaskit/embedded_views.dart | 64 +++------- .../platform_views/content_manager.dart | 22 +--- lib/web_ui/lib/ui.dart | 6 +- .../test/canvaskit/embedded_views_test.dart | 111 ------------------ 4 files changed, 22 insertions(+), 181 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 adda18f9477c2..a75f08e771632 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -92,11 +92,6 @@ class HtmlViewEmbedder { /// The list of view ids that should be composited, in order. List _compositionOrder = []; - /// The number of platform views in this frame which are visible. - /// - /// These platform views will require overlays. - int _visibleViewCount = 0; - /// The most recent composition order. List _activeCompositionOrder = []; @@ -132,7 +127,7 @@ class HtmlViewEmbedder { } void prerollCompositeEmbeddedView(int viewId, EmbeddedViewParams params) { - if (!disableOverlays && platformViewManager.isVisible(viewId)) { + if (!disableOverlays) { // We must decide in the preroll phase if a platform view will use the // backup overlay, so that draw commands after the platform view will // correctly paint to the backup surface. @@ -175,17 +170,12 @@ class HtmlViewEmbedder { /// If this returns a [CkCanvas], then that canvas should be the new leaf /// node. Otherwise, keep the same leaf node. CkCanvas? compositeEmbeddedView(int viewId) { - final int overlayIndex = _visibleViewCount; + final int compositedViewCount = _compositionOrder.length; _compositionOrder.add(viewId); - if (platformViewManager.isVisible(viewId)) { - _visibleViewCount++; - } - final bool needOverlay = - !disableOverlays && platformViewManager.isVisible(viewId); - if (needOverlay) { - if (overlayIndex < _pictureRecordersCreatedDuringPreroll.length) { + if (!disableOverlays) { + if (compositedViewCount < _pictureRecordersCreatedDuringPreroll.length) { _pictureRecorders[viewId] = - _pictureRecordersCreatedDuringPreroll[overlayIndex]; + _pictureRecordersCreatedDuringPreroll[compositedViewCount]; } else { _viewsUsingBackupSurface.add(viewId); _pictureRecorders[viewId] = _backupPictureRecorder!; @@ -194,7 +184,7 @@ class HtmlViewEmbedder { // Do nothing if this view doesn't need to be composited. if (!_viewsToRecomposite.contains(viewId)) { - if (needOverlay) { + if (!disableOverlays) { return _pictureRecorders[viewId]!.recordingCanvas; } else { return null; @@ -202,7 +192,7 @@ class HtmlViewEmbedder { } _compositeWithParams(viewId, _currentCompositionParams[viewId]!); _viewsToRecomposite.remove(viewId); - if (needOverlay) { + if (!disableOverlays) { return _pictureRecorders[viewId]!.recordingCanvas; } else { return null; @@ -346,8 +336,9 @@ class HtmlViewEmbedder { final svg.ClipPathElement newClipPath = svg.ClipPathElement(); newClipPath.id = clipId; newClipPath.append( - svg.PathElement() - ..setAttribute('d', path.toSvgString()!)); + svg.PathElement() + ..setAttribute('d', path.toSvgString()!) + ); pathDefs.append(newClipPath); // Store the id of the node instead of [newClipPath] directly. For @@ -365,8 +356,9 @@ class HtmlViewEmbedder { final svg.ClipPathElement newClipPath = svg.ClipPathElement(); newClipPath.id = clipId; newClipPath.append( - svg.PathElement() - ..setAttribute('d', path.toSvgString()!)); + svg.PathElement() + ..setAttribute('d', path.toSvgString()!) + ); pathDefs.append(newClipPath); // Store the id of the node instead of [newClipPath] directly. For // some reason, calling `newClipPath.remove()` doesn't remove it @@ -429,22 +421,13 @@ class HtmlViewEmbedder { _compositionOrder.isEmpty || disableOverlays) ? null - : diffViewList( - _activeCompositionOrder - .where((int viewId) => platformViewManager.isVisible(viewId)) - .toList(), - _compositionOrder - .where((int viewId) => platformViewManager.isVisible(viewId)) - .toList()); + : diffViewList(_activeCompositionOrder, _compositionOrder); final Map? insertBeforeMap = _updateOverlays(diffResult); bool _didPaintBackupSurface = false; if (!disableOverlays) { for (int i = 0; i < _compositionOrder.length; i++) { final int viewId = _compositionOrder[i]; - if (platformViewManager.isInvisible(viewId)) { - continue; - } if (_viewsUsingBackupSurface.contains(viewId)) { // Only draw the picture to the backup surface once. if (!_didPaintBackupSurface) { @@ -472,7 +455,6 @@ class HtmlViewEmbedder { _viewsUsingBackupSurface.clear(); if (listEquals(_compositionOrder, _activeCompositionOrder)) { _compositionOrder.clear(); - _visibleViewCount = 0; return; } @@ -560,7 +542,6 @@ class HtmlViewEmbedder { } _compositionOrder.clear(); - _visibleViewCount = 0; disposeViews(unusedViews); @@ -620,15 +601,12 @@ class HtmlViewEmbedder { // to the backup surface. SurfaceFactory.instance.releaseSurfaces(); _overlays.clear(); - final List viewsNeedingOverlays = _compositionOrder - .where((int viewId) => platformViewManager.isVisible(viewId)) - .toList(); final int numOverlays = math.min( SurfaceFactory.instance.maximumOverlays, - viewsNeedingOverlays.length, + _compositionOrder.length, ); for (int i = 0; i < numOverlays; i++) { - final int viewId = viewsNeedingOverlays[i]; + final int viewId = _compositionOrder[i]; assert(!_viewsUsingBackupSurface.contains(viewId)); _initializeOverlay(viewId); } @@ -684,8 +662,7 @@ class HtmlViewEmbedder { while (overlaysToAssign > 0 && index < _compositionOrder.length) { final bool activeView = index < lastOriginalIndex; final int viewId = _compositionOrder[index]; - if (!_overlays.containsKey(viewId) && - platformViewManager.isVisible(viewId)) { + if (!_overlays.containsKey(viewId)) { _initializeOverlay(viewId); overlaysToAssign--; if (activeView) { @@ -709,7 +686,6 @@ class HtmlViewEmbedder { for (int i = 0; i < _compositionOrder.length; i++) { final int viewId = _compositionOrder[i]; assert(_viewsUsingBackupSurface.contains(viewId) || - platformViewManager.isInvisible(viewId) || _overlays[viewId] != null); } } @@ -752,7 +728,6 @@ class HtmlViewEmbedder { _viewsToRecomposite.clear(); _activeCompositionOrder.clear(); _compositionOrder.clear(); - _visibleViewCount = 0; } } @@ -970,9 +945,8 @@ class ViewListDiffResult { // similar to `Surface._insertChildDomNodes` to efficiently handle more cases, // https://github.com/flutter/flutter/issues/89611. ViewListDiffResult? diffViewList(List active, List next) { - if (active.isEmpty || next.isEmpty) { - return null; - } + assert(active.isNotEmpty && next.isNotEmpty, + 'diffViewList called with empty view list'); // If the [active] and [next] lists are in the expected form described above, // then either the first or last element of [next] will be in [active]. int index = active.indexOf(next.first); diff --git a/lib/web_ui/lib/src/engine/platform_views/content_manager.dart b/lib/web_ui/lib/src/engine/platform_views/content_manager.dart index a6268a1c58094..58bf6a37c2895 100644 --- a/lib/web_ui/lib/src/engine/platform_views/content_manager.dart +++ b/lib/web_ui/lib/src/engine/platform_views/content_manager.dart @@ -43,9 +43,6 @@ class PlatformViewManager { // The references to content tags, indexed by their framework-given ID. final Map _contents = {}; - final Set _invisibleViews = {}; - final Map _viewIdToType = {}; - /// Returns `true` if the passed in `viewType` has been registered before. /// /// See [registerViewFactory] to understand how factories are registered. @@ -67,8 +64,7 @@ class PlatformViewManager { /// it's been set. /// /// `factoryFunction` needs to be a [PlatformViewFactory]. - bool registerFactory(String viewType, Function factoryFunction, - {bool isVisible = true}) { + bool registerFactory(String viewType, Function factoryFunction) { assert(factoryFunction is PlatformViewFactory || factoryFunction is ParameterizedPlatformViewFactory); @@ -76,9 +72,6 @@ class PlatformViewManager { return false; } _factories[viewType] = factoryFunction; - if (!isVisible) { - _invisibleViews.add(viewType); - } return true; } @@ -112,7 +105,6 @@ class PlatformViewManager { 'Attempted to render contents of unregistered viewType: $viewType'); final String slotName = getPlatformViewSlotName(viewId); - _viewIdToType[viewId] = viewType; return _contents.putIfAbsent(viewId, () { final html.Element wrapper = html.document @@ -194,16 +186,6 @@ class PlatformViewManager { } } - /// Returns `true` if the given [viewId] is for an invisible platform view. - bool isInvisible(int viewId) { - final String? viewType = _viewIdToType[viewId]; - return viewType != null && _invisibleViews.contains(viewType); - } - - /// Returns `true` if the given [viewId] is a platform view with a visible - /// component. - bool isVisible(int viewId) => !isInvisible(viewId); - /// Clears the state. Used in tests. /// /// Returns the set of know view ids, so they can be cleaned up. @@ -212,8 +194,6 @@ class PlatformViewManager { result.forEach(clearPlatformView); _factories.clear(); _contents.clear(); - _invisibleViews.clear(); - _viewIdToType.clear(); return result; } } diff --git a/lib/web_ui/lib/ui.dart b/lib/web_ui/lib/ui.dart index 5cfb3081508cd..cd032c270f3fd 100644 --- a/lib/web_ui/lib/ui.dart +++ b/lib/web_ui/lib/ui.dart @@ -63,11 +63,9 @@ typedef PlatformViewFactory = html.Element Function(int viewId); /// A registry for factories that create platform views. class PlatformViewRegistry { /// Register [viewTypeId] as being creating by the given [factory]. - bool registerViewFactory(String viewTypeId, PlatformViewFactory viewFactory, - {bool isVisible = true}) { + bool registerViewFactory(String viewTypeId, PlatformViewFactory viewFactory) { // TODO(web): Deprecate this once there's another way of calling `registerFactory` (js interop?) - return engine.platformViewManager - .registerFactory(viewTypeId, viewFactory, isVisible: isVisible); + return engine.platformViewManager.registerFactory(viewTypeId, viewFactory); } } diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index 296f29101ebb3..af02c950c68ac 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -641,117 +641,6 @@ void testMain() { HtmlViewEmbedder.debugDisableOverlays = false; }); - - test('does not create overlays for invisible platform views', () async { - ui.platformViewRegistry.registerViewFactory( - 'test-visible-view', - (int viewId) => - html.DivElement()..className = 'visible-platform-view'); - ui.platformViewRegistry.registerViewFactory( - 'test-invisible-view', - (int viewId) => - html.DivElement()..className = 'invisible-platform-view', - isVisible: false, - ); - await createPlatformView(0, 'test-visible-view'); - await createPlatformView(1, 'test-invisible-view'); - await createPlatformView(2, 'test-visible-view'); - await createPlatformView(3, 'test-invisible-view'); - await createPlatformView(4, 'test-invisible-view'); - await createPlatformView(5, 'test-invisible-view'); - await createPlatformView(6, 'test-invisible-view'); - - final EnginePlatformDispatcher dispatcher = - ui.window.platformDispatcher as EnginePlatformDispatcher; - - int countCanvases() { - return domRenderer.sceneElement!.querySelectorAll('canvas').length; - } - - expect(platformViewManager.isInvisible(0), isFalse); - expect(platformViewManager.isInvisible(1), isTrue); - - LayerSceneBuilder sb = LayerSceneBuilder(); - sb.pushOffset(0, 0); - sb.addPlatformView(1, width: 10, height: 10); - sb.pop(); - dispatcher.rasterizer!.draw(sb.build().layerTree); - expect(countCanvases(), 1); - - sb = LayerSceneBuilder(); - sb.pushOffset(0, 0); - sb.addPlatformView(0, width: 10, height: 10); - sb.addPlatformView(1, width: 10, height: 10); - sb.pop(); - dispatcher.rasterizer!.draw(sb.build().layerTree); - expect(countCanvases(), 2); - - sb = LayerSceneBuilder(); - sb.pushOffset(0, 0); - sb.addPlatformView(0, width: 10, height: 10); - sb.addPlatformView(1, width: 10, height: 10); - sb.addPlatformView(2, width: 10, height: 10); - sb.pop(); - dispatcher.rasterizer!.draw(sb.build().layerTree); - expect(countCanvases(), 3); - - sb = LayerSceneBuilder(); - sb.pushOffset(0, 0); - sb.addPlatformView(0, width: 10, height: 10); - sb.addPlatformView(1, width: 10, height: 10); - sb.addPlatformView(2, width: 10, height: 10); - sb.addPlatformView(3, width: 10, height: 10); - sb.pop(); - dispatcher.rasterizer!.draw(sb.build().layerTree); - expect(countCanvases(), 3); - - sb = LayerSceneBuilder(); - sb.pushOffset(0, 0); - sb.addPlatformView(0, width: 10, height: 10); - sb.addPlatformView(1, width: 10, height: 10); - sb.addPlatformView(2, width: 10, height: 10); - sb.addPlatformView(3, width: 10, height: 10); - sb.addPlatformView(4, width: 10, height: 10); - sb.pop(); - dispatcher.rasterizer!.draw(sb.build().layerTree); - expect(countCanvases(), 3); - - sb = LayerSceneBuilder(); - sb.pushOffset(0, 0); - sb.addPlatformView(0, width: 10, height: 10); - sb.addPlatformView(1, width: 10, height: 10); - sb.addPlatformView(2, width: 10, height: 10); - sb.addPlatformView(3, width: 10, height: 10); - sb.addPlatformView(4, width: 10, height: 10); - sb.addPlatformView(5, width: 10, height: 10); - sb.pop(); - dispatcher.rasterizer!.draw(sb.build().layerTree); - expect(countCanvases(), 3); - - sb = LayerSceneBuilder(); - sb.pushOffset(0, 0); - sb.addPlatformView(0, width: 10, height: 10); - sb.addPlatformView(1, width: 10, height: 10); - sb.addPlatformView(2, width: 10, height: 10); - sb.addPlatformView(3, width: 10, height: 10); - sb.addPlatformView(4, width: 10, height: 10); - sb.addPlatformView(5, width: 10, height: 10); - sb.addPlatformView(6, width: 10, height: 10); - sb.pop(); - dispatcher.rasterizer!.draw(sb.build().layerTree); - expect(countCanvases(), 3); - - sb = LayerSceneBuilder(); - sb.pushOffset(0, 0); - sb.addPlatformView(1, width: 10, height: 10); - sb.addPlatformView(3, width: 10, height: 10); - sb.addPlatformView(4, width: 10, height: 10); - sb.addPlatformView(5, width: 10, height: 10); - sb.addPlatformView(6, width: 10, height: 10); - sb.pop(); - dispatcher.rasterizer!.draw(sb.build().layerTree); - expect(countCanvases(), 1); - }); // TODO(dit): https://github.com/flutter/flutter/issues/60040 }, skip: isIosSafari); }