From c08832a49e741d10c2b56858f0307eda41e6b21b Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 25 Oct 2023 13:40:58 -0700 Subject: [PATCH 01/28] WIP overlay optimizer --- .../src/engine/canvaskit/embedded_views.dart | 8 ++- .../canvaskit/overlay_scene_optimizer.dart | 49 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart 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 a5e9425c0027c..10c5b46dfd187 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -56,6 +56,10 @@ class HtmlViewEmbedder { /// Canvases used to draw on top of platform views, keyed by platform view ID. final Map _overlays = {}; + /// Maps platform view IDs to the bounds of where they will be rendered in + /// screen coordinates. + final Map _platformViewBounds = {}; + /// The views that need to be recomposited into the scene on the next frame. final Set _viewsToRecomposite = {}; @@ -243,6 +247,8 @@ class HtmlViewEmbedder { } } + // TODO(hterkelsen): Compute the platform view bounds while applying + // mutators. We need the bounds to be able to optimize the overlays. void _applyMutators( EmbeddedViewParams params, DomElement embeddedView, int viewId) { final MutatorsStack mutators = params.mutators; @@ -270,7 +276,7 @@ class HtmlViewEmbedder { clipView.style.transform = ''; // We need to set width and height for the clipView to cover the // bounds of the path since Safari seem to incorrectly intersect - // the element bounding rect with the clip path. + // the element bounding rect with the clip path. clipView.style.width = '100%'; clipView.style.height = '100%'; if (mutator.rect != null) { diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart new file mode 100644 index 0000000000000..df124422def61 --- /dev/null +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -0,0 +1,49 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:ui/ui.dart' as ui; + +import 'embedded_views.dart'; +import 'picture.dart'; + +/// A [Rendering] is a concrete description of how a Flutter scene will be +/// rendered in a web browser. +/// +/// A [Rendering] is a sequence containing two types of entities: +/// * Render canvases: which contain rasterized CkPictures, and +/// * Platform views: being HTML content that is to be composited along with +/// the Flutter content. +class Rendering { + final List entities = []; +} + +sealed class RenderingEntity {} + +class RenderingRenderCanvas extends RenderingEntity { + RenderingRenderCanvas(); + final List pictures = []; + + void add(CkPicture picture) { + pictures.add(picture); + } +} + +class RenderingPlatformView extends RenderingEntity { + RenderingPlatformView(this.viewId); + final int viewId; +} + +ui.Rect _computePlatformViewBounds(EmbeddedViewParams params) { + return ui.Rect.largest; +} + +Rendering createOptimizedRendering( + List pictures, List platformViews) { + assert(pictures.length == platformViews.length + 1); + final Rendering result = Rendering(); + RenderingRenderCanvas currentRenderCanvas = RenderingRenderCanvas(); + currentRenderCanvas.add(pictures[0]); + for (int i = 0; i < platformViews.length; i++) {} + return result; +} From 6de705b51aee321e14482e3bb848f7b57697a434 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 26 Oct 2023 15:18:21 -0700 Subject: [PATCH 02/28] More work in progress --- .../src/engine/canvaskit/embedded_views.dart | 4 - .../canvaskit/overlay_scene_optimizer.dart | 96 +++++++++++++++++-- 2 files changed, 90 insertions(+), 10 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 10c5b46dfd187..81bba90cf2788 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -56,10 +56,6 @@ class HtmlViewEmbedder { /// Canvases used to draw on top of platform views, keyed by platform view ID. final Map _overlays = {}; - /// Maps platform view IDs to the bounds of where they will be rendered in - /// screen coordinates. - final Map _platformViewBounds = {}; - /// The views that need to be recomposited into the scene on the next frame. final Set _viewsToRecomposite = {}; diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index df124422def61..e13fe156d8296 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -2,8 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:meta/meta.dart'; +import 'package:ui/src/engine/util.dart'; import 'package:ui/ui.dart' as ui; +import '../../engine.dart' show PlatformViewManager; +import '../vector_math.dart'; import 'embedded_views.dart'; import 'picture.dart'; @@ -16,14 +20,22 @@ import 'picture.dart'; /// the Flutter content. class Rendering { final List entities = []; + + void add(RenderingEntity entity) { + entities.add(entity); + } } sealed class RenderingEntity {} class RenderingRenderCanvas extends RenderingEntity { - RenderingRenderCanvas(); + RenderingRenderCanvas({required this.requiredDueTo}); final List pictures = []; + /// The index of the platform view that caused this render canvas to be + /// required. + final int requiredDueTo; + void add(CkPicture picture) { pictures.add(picture); } @@ -34,16 +46,88 @@ class RenderingPlatformView extends RenderingEntity { final int viewId; } -ui.Rect _computePlatformViewBounds(EmbeddedViewParams params) { - return ui.Rect.largest; +// Computes the bounds of the platform view from its associated parameters. +@visibleForTesting +ui.Rect computePlatformViewBounds(EmbeddedViewParams params) { + ui.Rect currentClipBounds = ui.Rect.largest; + Matrix4 currentTransform = Matrix4.identity(); + for (final Mutator mutator in params.mutators) { + switch (mutator.type) { + case MutatorType.clipRect: + final ui.Rect transformedClipBounds = + transformRectWithMatrix(currentTransform, mutator.rect!); + currentClipBounds = currentClipBounds.intersect(transformedClipBounds); + case MutatorType.clipRRect: + final ui.Rect transformedClipBounds = + transformRectWithMatrix(currentTransform, mutator.rrect!.outerRect); + currentClipBounds = currentClipBounds.intersect(transformedClipBounds); + case MutatorType.clipPath: + final ui.Rect transformedClipBounds = transformRectWithMatrix( + currentTransform, mutator.path!.getBounds()); + currentClipBounds.intersect(transformedClipBounds); + case MutatorType.transform: + currentTransform = currentTransform.multiplied(mutator.matrix!); + case MutatorType.opacity: + // Doesn't effect bounds. + continue; + } + } + final ui.Rect rawBounds = ui.Rect.fromLTWH( + params.offset.dx, + params.offset.dy, + params.size.width, + params.size.height, + ); + final ui.Rect transformedBounds = + transformRectWithMatrix(currentTransform, rawBounds); + return transformedBounds.intersect(currentClipBounds); } Rendering createOptimizedRendering( - List pictures, List platformViews) { + List pictures, + List platformViews, + Map paramsForViews, +) { assert(pictures.length == platformViews.length + 1); + final Map overlayRequirement = {}; final Rendering result = Rendering(); - RenderingRenderCanvas currentRenderCanvas = RenderingRenderCanvas(); + // The first render canvas is required due to the pseudo-platform view "V_0" + // which is defined as a platform view that comes before all Flutter drawing + // commands and intersects with everything. + RenderingRenderCanvas currentRenderCanvas = + RenderingRenderCanvas(requiredDueTo: 0); + + // This line essentially unwinds the first iteration of the following loop. + // Since "V_0" intersects with all subsequent pictures, then the first picture + // it intersects with is "P_0", so we create a new render canvas and add "P_0" + // to it. currentRenderCanvas.add(pictures[0]); - for (int i = 0; i < platformViews.length; i++) {} + result.add(currentRenderCanvas); + for (int i = 0; i < platformViews.length; i++) { + result.add(RenderingPlatformView(platformViews[i])); + // Find the first picture after this platform view that intersects with this + // platform view. + if (PlatformViewManager.instance.isVisible(platformViews[i])) { + final ui.Rect platformViewBounds = + computePlatformViewBounds(paramsForViews[platformViews[i]]!); + for (final int j = i + 1; i < pictures.length; i++) { + final ui.Rect pictureBounds = pictures[j].cullRect; + if (platformViewBounds.overlaps(pictureBounds)) { + overlayRequirement[pictures[j]] = i; + break; + } + } + } + if (overlayRequirement.containsKey(pictures[i + 1]) && + overlayRequirement[pictures[i + 1]]! >= + currentRenderCanvas.requiredDueTo) { + currentRenderCanvas = RenderingRenderCanvas( + requiredDueTo: overlayRequirement[pictures[i + 1]]!); + currentRenderCanvas.add(pictures[i + 1]); + result.add(currentRenderCanvas); + } else { + currentRenderCanvas.add(pictures[i + 1]); + } + } return result; } From 30dcbd649a2a6d5c3777cb29915217c5a3a27da1 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 26 Oct 2023 15:22:34 -0700 Subject: [PATCH 03/28] Remove comment --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 2 -- 1 file changed, 2 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 81bba90cf2788..c71b4118b7bde 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -243,8 +243,6 @@ class HtmlViewEmbedder { } } - // TODO(hterkelsen): Compute the platform view bounds while applying - // mutators. We need the bounds to be able to optimize the overlays. void _applyMutators( EmbeddedViewParams params, DomElement embeddedView, int viewId) { final MutatorsStack mutators = params.mutators; From b421d5281740a1ef607fa8dea8b41f565fdc0a5c Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 17 Jan 2024 16:02:58 -0800 Subject: [PATCH 04/28] fix licenses --- ci/licenses_golden/licenses_flutter | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 8ba62e4b59898..ceb3d8d43d9bd 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -5929,6 +5929,7 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_tree.dart + . ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart + ../../../flutter/LICENSE @@ -8770,6 +8771,7 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/layer_tree.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart +FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart From d02cbf5e34608be07373baca641a41fbe5202f25 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 18 Jan 2024 14:41:53 -0800 Subject: [PATCH 05/28] fix bad license merge --- ci/licenses_golden/licenses_flutter | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 0e8d50a569dc2..e768f5ad5a03d 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -5931,11 +5931,8 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart + ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/multi_surface_rasterizer.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart + ../../../flutter/LICENSE -<<<<<<< HEAD ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart + ../../../flutter/LICENSE -======= ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/offscreen_canvas_rasterizer.dart + ../../../flutter/LICENSE ->>>>>>> main ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart + ../../../flutter/LICENSE @@ -8778,11 +8775,8 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/multi_surface_rasterizer.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart -<<<<<<< HEAD FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart -======= FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/offscreen_canvas_rasterizer.dart ->>>>>>> main FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart From 052b935ce7870b0ab75d3b2afc0c9361fa87a507 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 7 Feb 2024 14:04:26 -0800 Subject: [PATCH 06/28] WIP overlay --- lib/web_ui/lib/src/engine.dart | 1 + .../src/engine/canvaskit/embedded_views.dart | 82 +++++++++---------- .../canvaskit/overlay_scene_optimizer.dart | 53 +++++++++++- .../lib/src/engine/canvaskit/rasterizer.dart | 5 +- 4 files changed, 95 insertions(+), 46 deletions(-) diff --git a/lib/web_ui/lib/src/engine.dart b/lib/web_ui/lib/src/engine.dart index 57c7b35fafb8a..8e3af605c5b27 100644 --- a/lib/web_ui/lib/src/engine.dart +++ b/lib/web_ui/lib/src/engine.dart @@ -39,6 +39,7 @@ export 'engine/canvaskit/multi_surface_rasterizer.dart'; export 'engine/canvaskit/n_way_canvas.dart'; export 'engine/canvaskit/native_memory.dart'; export 'engine/canvaskit/offscreen_canvas_rasterizer.dart'; +export 'engine/canvaskit/overlay_scene_optimizer.dart'; export 'engine/canvaskit/painting.dart'; export 'engine/canvaskit/path.dart'; export 'engine/canvaskit/path_metrics.dart'; 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 9797e2f1fd29a..f75ed19c5dcc3 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -14,6 +14,7 @@ import '../util.dart'; import '../vector_math.dart'; import 'canvas.dart'; import 'embedded_views_diff.dart'; +import 'overlay_scene_optimizer.dart'; import 'path.dart'; import 'picture.dart'; import 'picture_recorder.dart'; @@ -144,7 +145,8 @@ class HtmlViewEmbedder { void _compositeWithParams(int platformViewId, EmbeddedViewParams params) { // If we haven't seen this viewId yet, cache it for clips/transforms. - final ViewClipChain clipChain = _viewClipChains.putIfAbsent(platformViewId, () { + final ViewClipChain clipChain = + _viewClipChains.putIfAbsent(platformViewId, () { return ViewClipChain(view: createPlatformViewSlot(platformViewId)); }); @@ -361,12 +363,20 @@ class HtmlViewEmbedder { sceneHost.append(_svgPathDefs!); } - Future submitFrame() async { + Future submitFrame( + DisplayCanvas baseCanvas, CkPicture basePicture) async { + final List pictures = [basePicture]; + for (final CkPictureRecorder recorder in _context.pictureRecorders) { + pictures.add(recorder.endRecording()); + } + final Rendering rendering = createOptimizedRendering( + pictures, _compositionOrder, _currentCompositionParams); final ViewListDiffResult? diffResult = (_activeCompositionOrder.isEmpty || _compositionOrder.isEmpty) ? null : diffViewList(_activeCompositionOrder, _compositionOrder); - final List? overlayGroups = _updateOverlays(diffResult); + final List? overlayGroups = + _updateOverlays(rendering, diffResult); if (overlayGroups != null) { _activeOverlayGroups = overlayGroups; } @@ -376,15 +386,19 @@ class HtmlViewEmbedder { '(${_context.pictureRecorders.length}) as overlays (${_overlays.length}).', ); - int pictureRecorderIndex = 0; + final List renderCanvases = rendering.canvases; + assert(renderCanvases.length == _activeOverlayGroups.length + 1); + + await rasterizer.rasterizeToCanvas(baseCanvas, renderCanvases[0].pictures); + + // The first render canvas is the base canvas, not an overlay, so skip it + // since we just rasterized to it above. + int renderCanvasIndex = 1; for (final OverlayGroup overlayGroup in _activeOverlayGroups) { final DisplayCanvas overlay = _overlays[overlayGroup.last]!; - final List pictures = []; - for (int i = 0; i < overlayGroup.visibleCount; i++) { - pictures.add( - _context.pictureRecorders[pictureRecorderIndex].endRecording()); - pictureRecorderIndex++; - } + final List pictures = + renderCanvases[renderCanvasIndex].pictures; + renderCanvasIndex++; await rasterizer.rasterizeToCanvas(overlay, pictures); } for (final CkPictureRecorder recorder @@ -439,8 +453,7 @@ class HtmlViewEmbedder { sceneHost.insertBefore(platformViewRoot, elementToInsertBefore); final DisplayCanvas? overlay = _overlays[viewId]; if (overlay != null) { - sceneHost.insertBefore( - overlay.hostElement, elementToInsertBefore); + sceneHost.insertBefore(overlay.hostElement, elementToInsertBefore); } } else { final DomElement platformViewRoot = _viewClipChains[viewId]!.root; @@ -547,7 +560,8 @@ class HtmlViewEmbedder { // composition order of the current and previous frame, respectively. // // TODO(hterkelsen): Test this more thoroughly. - List? _updateOverlays(ViewListDiffResult? diffResult) { + List? _updateOverlays( + Rendering rendering, ViewListDiffResult? diffResult) { if (diffResult != null && diffResult.viewsToAdd.isEmpty && diffResult.viewsToRemove.isEmpty) { @@ -558,8 +572,7 @@ class HtmlViewEmbedder { // Group platform views from their composition order. // Each group contains one visible view, and any number of invisible views // before or after that visible view. - final List overlayGroups = - getOverlayGroups(_compositionOrder); + final List overlayGroups = getOverlayGroups(rendering); final List viewsNeedingOverlays = overlayGroups.map((OverlayGroup group) => group.last).toList(); if (diffResult == null) { @@ -595,39 +608,23 @@ class HtmlViewEmbedder { // If there are more visible views than overlays, then the views which cannot // be assigned an overlay are grouped together and will be rendered on top of // the rest of the scene. - List getOverlayGroups(List views) { + List getOverlayGroups(Rendering rendering) { final List result = []; OverlayGroup currentGroup = OverlayGroup(); - for (int i = 0; i < views.length; i++) { - final int view = views[i]; - if (PlatformViewManager.instance.isInvisible(view)) { - // We add as many invisible views as we find to the current group. - currentGroup.add(view); - } else { - // `view` is visible. - if (!currentGroup.hasVisibleView || - result.length + 1 >= HtmlViewEmbedder.maximumOverlays) { - // If `view` is the first visible one of the group or we've reached - // the maximum number of overlays, add it. - currentGroup.add(view, visible: true); - } else { - // There's already a visible `view` in `currentGroup`, so a new - // OverlayGroup will be needed. - // Let's decide what to do with the `currentGroup` first: - if (currentGroup.hasVisibleView) { - // We only care about groups that have one visible view. - result.add(currentGroup); - } + for (int i = 0; i < rendering.entities.length; i++) { + final RenderingEntity entity = rendering.entities[i]; + + /// We are at an overlay, end the current group and start the next group. + if (entity is RenderingRenderCanvas) { + if (!currentGroup.isEmpty) { + result.add(currentGroup); currentGroup = OverlayGroup(); - currentGroup.add(view, visible: true); } + } else if (entity is RenderingPlatformView) { + currentGroup.add(entity.viewId); } } - // Handle the last group to be (maybe) returned. - if (currentGroup.hasVisibleView) { - result.add(currentGroup); - } return result; } @@ -703,6 +700,9 @@ class OverlayGroup { /// Returns the number of visible views in this overlay group. int get visibleCount => _visibleCount; + + /// Returns [true] if this overlay group is empty. + bool get isEmpty => _group.isEmpty; } /// Represents a Clip Chain (for a view). diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index e13fe156d8296..c448886528a3d 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -24,26 +24,54 @@ class Rendering { void add(RenderingEntity entity) { entities.add(entity); } + + List get canvases => + entities.whereType().toList(); } -sealed class RenderingEntity {} +/// An element of a [Rendering]. Either a render canvas or a platform view. +sealed class RenderingEntity { + /// Returns [true] if this entity is equal to [other] for use in a rendering. + /// + /// For example, all [RenderingRenderCanvas] objects are equal to each other + /// for purposes of rendering since any canvas in that place in the rendering + /// will be equivalent. Platform views are only equal if they are for the same + /// view id. + bool equalForRendering(RenderingEntity other); +} class RenderingRenderCanvas extends RenderingEntity { RenderingRenderCanvas({required this.requiredDueTo}); + + /// The [pictures] which should be rendered in this canvas. final List pictures = []; /// The index of the platform view that caused this render canvas to be /// required. final int requiredDueTo; + /// Adds the [picture] to the pictures that should be rendered in this canvas. void add(CkPicture picture) { pictures.add(picture); } + + @override + bool equalForRendering(RenderingEntity other) { + return other is RenderingRenderCanvas; + } } +/// A platform view to be rendered. class RenderingPlatformView extends RenderingEntity { RenderingPlatformView(this.viewId); + + /// The [viewId] of the platform view to render. final int viewId; + + @override + bool equalForRendering(RenderingEntity other) { + return other is RenderingPlatformView && other.viewId == viewId; + } } // Computes the bounds of the platform view from its associated parameters. @@ -83,12 +111,29 @@ ui.Rect computePlatformViewBounds(EmbeddedViewParams params) { return transformedBounds.intersect(currentClipBounds); } +/// Returns the optimized [Rendering] for a sequence of [pictures] and +/// [platformViews]. +/// +/// [paramsForViews] is required to compute the bounds of the platform views. Rendering createOptimizedRendering( List pictures, List platformViews, Map paramsForViews, ) { assert(pictures.length == platformViews.length + 1); + + final List pictureBounds = []; + for (final CkPicture picture in pictures) { + pictureBounds.add(picture.cullRect); + } + print(pictureBounds); + + final List platformViewBounds = []; + for (final int viewId in platformViews) { + platformViewBounds.add(computePlatformViewBounds(paramsForViews[viewId]!)); + } + print(platformViewBounds); + final Map overlayRequirement = {}; final Rendering result = Rendering(); // The first render canvas is required due to the pseudo-platform view "V_0" @@ -110,7 +155,7 @@ Rendering createOptimizedRendering( if (PlatformViewManager.instance.isVisible(platformViews[i])) { final ui.Rect platformViewBounds = computePlatformViewBounds(paramsForViews[platformViews[i]]!); - for (final int j = i + 1; i < pictures.length; i++) { + for (int j = i + 1; j < pictures.length; j++) { final ui.Rect pictureBounds = pictures[j].cullRect; if (platformViewBounds.overlaps(pictureBounds)) { overlayRequirement[pictures[j]] = i; @@ -131,3 +176,7 @@ Rendering createOptimizedRendering( } return result; } + +/// Updates the DOM to display the [next] rendering by using the fewest +/// DOM operations to rearrange the [current] rendering. +void updateRendering(Rendering current, Rendering next) {} diff --git a/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart b/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart index 58a23e871cc92..3402703ba382a 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart @@ -62,10 +62,9 @@ abstract class ViewRasterizer { compositorFrame.raster(layerTree, ignoreRasterCache: true); sceneHost.prepend(displayFactory.baseCanvas.hostElement); - await rasterizeToCanvas( - displayFactory.baseCanvas, [pictureRecorder.endRecording()]); - await viewEmbedder.submitFrame(); + await viewEmbedder.submitFrame( + displayFactory.baseCanvas, pictureRecorder.endRecording()); } /// Do some initialization to prepare to draw a frame. From 6fdcd4918e71ea28da4fcb851f101420820e5f10 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 8 Feb 2024 10:43:20 -0800 Subject: [PATCH 07/28] debug rects --- .../src/engine/canvaskit/embedded_views.dart | 31 +++++++++++++++++++ .../canvaskit/overlay_scene_optimizer.dart | 12 ++++--- 2 files changed, 39 insertions(+), 4 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 f75ed19c5dcc3..fbabdb3ce4169 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -15,6 +15,7 @@ import '../vector_math.dart'; import 'canvas.dart'; import 'embedded_views_diff.dart'; import 'overlay_scene_optimizer.dart'; +import 'painting.dart'; import 'path.dart'; import 'picture.dart'; import 'picture_recorder.dart'; @@ -65,6 +66,8 @@ class HtmlViewEmbedder { /// The most recent overlay groups. List _activeOverlayGroups = []; + DisplayCanvas? debugBoundsCanvas; + /// The size of the frame, in physical pixels. late ui.Size _frameSize; @@ -515,6 +518,34 @@ class HtmlViewEmbedder { disposeViews(unusedViews); + if (rendering.debugPictureBounds != null && + rendering.debugPlatformViewBounds != null) { + // Draw the rects for the computed bounds. + final CkPictureRecorder recorder = CkPictureRecorder(); + final CkCanvas canvas = recorder.beginRecording(ui.Rect.largest); + + final CkPaint pictureBoundPaint = CkPaint() + ..color = const ui.Color.fromARGB(64, 0, 255, 0); + final CkPaint platformViewBoundPaint = CkPaint() + ..color = const ui.Color.fromARGB(64, 0, 0, 255); + + for (final ui.Rect pictureBound in rendering.debugPictureBounds!) { + canvas.drawRect(pictureBound, pictureBoundPaint); + } + + for (final ui.Rect platformViewBound + in rendering.debugPlatformViewBounds!) { + canvas.drawRect(platformViewBound, platformViewBoundPaint); + } + + final CkPicture boundsPicture = recorder.endRecording(); + + debugBoundsCanvas ??= rasterizer.getOverlay(); + await rasterizer + .rasterizeToCanvas(debugBoundsCanvas!, [boundsPicture]); + sceneHost.append(debugBoundsCanvas!.hostElement); + } + assert( debugInvalidViewIds == null || debugInvalidViewIds!.isEmpty, 'Cannot render platform views: ${debugInvalidViewIds!.join(', ')}. ' diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index c448886528a3d..71a47289ed484 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -27,6 +27,9 @@ class Rendering { List get canvases => entities.whereType().toList(); + + List? debugPlatformViewBounds; + List? debugPictureBounds; } /// An element of a [Rendering]. Either a render canvas or a platform view. @@ -122,20 +125,21 @@ Rendering createOptimizedRendering( ) { assert(pictures.length == platformViews.length + 1); + final Map overlayRequirement = {}; + final Rendering result = Rendering(); + final List pictureBounds = []; for (final CkPicture picture in pictures) { pictureBounds.add(picture.cullRect); } - print(pictureBounds); + result.debugPictureBounds = pictureBounds; final List platformViewBounds = []; for (final int viewId in platformViews) { platformViewBounds.add(computePlatformViewBounds(paramsForViews[viewId]!)); } - print(platformViewBounds); + result.debugPlatformViewBounds = platformViewBounds; - final Map overlayRequirement = {}; - final Rendering result = Rendering(); // The first render canvas is required due to the pseudo-platform view "V_0" // which is defined as a platform view that comes before all Flutter drawing // commands and intersects with everything. From ab2f7d5aba1ea2140f175cdcac7f60b91454c74a Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 8 Feb 2024 16:53:50 -0800 Subject: [PATCH 08/28] it's working! --- .../src/engine/canvaskit/embedded_views.dart | 49 +++------ .../canvaskit/overlay_scene_optimizer.dart | 100 ++++++++---------- 2 files changed, 60 insertions(+), 89 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 fbabdb3ce4169..7bebdae442dba 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -66,6 +66,9 @@ class HtmlViewEmbedder { /// The most recent overlay groups. List _activeOverlayGroups = []; + /// The most recent rendering. + Rendering _activeRendering = Rendering(); + DisplayCanvas? debugBoundsCanvas; /// The size of the frame, in physical pixels. @@ -390,6 +393,12 @@ class HtmlViewEmbedder { ); final List renderCanvases = rendering.canvases; + if (renderCanvases.length != _activeOverlayGroups.length + 1) { + print('render canvas count = ${renderCanvases.length}'); + print( + 'overlay groups = ${_activeOverlayGroups.map((group) => group._group)}'); + print('overlayGroups was null? ${overlayGroups == null}'); + } assert(renderCanvases.length == _activeOverlayGroups.length + 1); await rasterizer.rasterizeToCanvas(baseCanvas, renderCanvases[0].pictures); @@ -518,34 +527,6 @@ class HtmlViewEmbedder { disposeViews(unusedViews); - if (rendering.debugPictureBounds != null && - rendering.debugPlatformViewBounds != null) { - // Draw the rects for the computed bounds. - final CkPictureRecorder recorder = CkPictureRecorder(); - final CkCanvas canvas = recorder.beginRecording(ui.Rect.largest); - - final CkPaint pictureBoundPaint = CkPaint() - ..color = const ui.Color.fromARGB(64, 0, 255, 0); - final CkPaint platformViewBoundPaint = CkPaint() - ..color = const ui.Color.fromARGB(64, 0, 0, 255); - - for (final ui.Rect pictureBound in rendering.debugPictureBounds!) { - canvas.drawRect(pictureBound, pictureBoundPaint); - } - - for (final ui.Rect platformViewBound - in rendering.debugPlatformViewBounds!) { - canvas.drawRect(platformViewBound, platformViewBoundPaint); - } - - final CkPicture boundsPicture = recorder.endRecording(); - - debugBoundsCanvas ??= rasterizer.getOverlay(); - await rasterizer - .rasterizeToCanvas(debugBoundsCanvas!, [boundsPicture]); - sceneHost.append(debugBoundsCanvas!.hostElement); - } - assert( debugInvalidViewIds == null || debugInvalidViewIds!.isEmpty, 'Cannot render platform views: ${debugInvalidViewIds!.join(', ')}. ' @@ -593,11 +574,8 @@ class HtmlViewEmbedder { // TODO(hterkelsen): Test this more thoroughly. List? _updateOverlays( Rendering rendering, ViewListDiffResult? diffResult) { - if (diffResult != null && - diffResult.viewsToAdd.isEmpty && - diffResult.viewsToRemove.isEmpty) { - // The composition order has not changed, continue using the assigned - // overlays. + if (rendering.equalsForRendering(_activeRendering)) { + // The rendering has not changed, continue using the assigned overlays. return null; } // Group platform views from their composition order. @@ -643,9 +621,7 @@ class HtmlViewEmbedder { final List result = []; OverlayGroup currentGroup = OverlayGroup(); - for (int i = 0; i < rendering.entities.length; i++) { - final RenderingEntity entity = rendering.entities[i]; - + for (final RenderingEntity entity in rendering.entities) { /// We are at an overlay, end the current group and start the next group. if (entity is RenderingRenderCanvas) { if (!currentGroup.isEmpty) { @@ -656,6 +632,7 @@ class HtmlViewEmbedder { currentGroup.add(entity.viewId); } } + assert(rendering.canvases.length == result.length + 1); return result; } diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index 71a47289ed484..3b516f63d3b13 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -7,6 +7,7 @@ import 'package:ui/src/engine/util.dart'; import 'package:ui/ui.dart' as ui; import '../../engine.dart' show PlatformViewManager; +import '../display.dart'; import '../vector_math.dart'; import 'embedded_views.dart'; import 'picture.dart'; @@ -25,11 +26,21 @@ class Rendering { entities.add(entity); } + /// Returns [true] if this is equibalent to [other] for use in rendering. + bool equalsForRendering(Rendering other) { + if (other.entities.length != entities.length) { + return false; + } + for (int i = 0; i < entities.length; i++) { + if (!entities[i].equalsForRendering(other.entities[i])) { + return false; + } + } + return true; + } + List get canvases => entities.whereType().toList(); - - List? debugPlatformViewBounds; - List? debugPictureBounds; } /// An element of a [Rendering]. Either a render canvas or a platform view. @@ -40,26 +51,22 @@ sealed class RenderingEntity { /// for purposes of rendering since any canvas in that place in the rendering /// will be equivalent. Platform views are only equal if they are for the same /// view id. - bool equalForRendering(RenderingEntity other); + bool equalsForRendering(RenderingEntity other); } class RenderingRenderCanvas extends RenderingEntity { - RenderingRenderCanvas({required this.requiredDueTo}); + RenderingRenderCanvas(); /// The [pictures] which should be rendered in this canvas. final List pictures = []; - /// The index of the platform view that caused this render canvas to be - /// required. - final int requiredDueTo; - /// Adds the [picture] to the pictures that should be rendered in this canvas. void add(CkPicture picture) { pictures.add(picture); } @override - bool equalForRendering(RenderingEntity other) { + bool equalsForRendering(RenderingEntity other) { return other is RenderingRenderCanvas; } } @@ -72,7 +79,7 @@ class RenderingPlatformView extends RenderingEntity { final int viewId; @override - bool equalForRendering(RenderingEntity other) { + bool equalsForRendering(RenderingEntity other) { return other is RenderingPlatformView && other.viewId == viewId; } } @@ -81,7 +88,14 @@ class RenderingPlatformView extends RenderingEntity { @visibleForTesting ui.Rect computePlatformViewBounds(EmbeddedViewParams params) { ui.Rect currentClipBounds = ui.Rect.largest; - Matrix4 currentTransform = Matrix4.identity(); + + // The transforms are in logical pixels, but we want to compute physical + // pixel bounds, so transform by the device pixel ratio. + final double scale = EngineFlutterDisplay.instance.devicePixelRatio; + Matrix4 currentTransform = Matrix4.diagonal3Values(scale, scale, 1); + currentTransform = currentTransform.multiplied(params.offset == ui.Offset.zero + ? Matrix4.identity() + : Matrix4.translationValues(params.offset.dx, params.offset.dy, 0)); for (final Mutator mutator in params.mutators) { switch (mutator.type) { case MutatorType.clipRect: @@ -103,11 +117,14 @@ ui.Rect computePlatformViewBounds(EmbeddedViewParams params) { continue; } } + + // The width and height are in physical pixels already, so apply the inverse + // scale since the transform already applied the scaling. final ui.Rect rawBounds = ui.Rect.fromLTWH( - params.offset.dx, - params.offset.dy, - params.size.width, - params.size.height, + 0, + 0, + params.size.width / scale, + params.size.height / scale, ); final ui.Rect transformedBounds = transformRectWithMatrix(currentTransform, rawBounds); @@ -125,62 +142,39 @@ Rendering createOptimizedRendering( ) { assert(pictures.length == platformViews.length + 1); - final Map overlayRequirement = {}; final Rendering result = Rendering(); - final List pictureBounds = []; - for (final CkPicture picture in pictures) { - pictureBounds.add(picture.cullRect); - } - result.debugPictureBounds = pictureBounds; - - final List platformViewBounds = []; - for (final int viewId in platformViews) { - platformViewBounds.add(computePlatformViewBounds(paramsForViews[viewId]!)); - } - result.debugPlatformViewBounds = platformViewBounds; - // The first render canvas is required due to the pseudo-platform view "V_0" // which is defined as a platform view that comes before all Flutter drawing // commands and intersects with everything. - RenderingRenderCanvas currentRenderCanvas = - RenderingRenderCanvas(requiredDueTo: 0); + RenderingRenderCanvas currentRenderCanvas = RenderingRenderCanvas(); // This line essentially unwinds the first iteration of the following loop. // Since "V_0" intersects with all subsequent pictures, then the first picture // it intersects with is "P_0", so we create a new render canvas and add "P_0" // to it. currentRenderCanvas.add(pictures[0]); - result.add(currentRenderCanvas); for (int i = 0; i < platformViews.length; i++) { - result.add(RenderingPlatformView(platformViews[i])); - // Find the first picture after this platform view that intersects with this - // platform view. if (PlatformViewManager.instance.isVisible(platformViews[i])) { final ui.Rect platformViewBounds = computePlatformViewBounds(paramsForViews[platformViews[i]]!); - for (int j = i + 1; j < pictures.length; j++) { - final ui.Rect pictureBounds = pictures[j].cullRect; - if (platformViewBounds.overlaps(pictureBounds)) { - overlayRequirement[pictures[j]] = i; + bool intersectsWithCurrentPictures = false; + for (final CkPicture picture in currentRenderCanvas.pictures) { + if (picture.cullRect.overlaps(platformViewBounds)) { + intersectsWithCurrentPictures = true; break; } } + if (intersectsWithCurrentPictures) { + result.add(currentRenderCanvas); + currentRenderCanvas = RenderingRenderCanvas(); + } } - if (overlayRequirement.containsKey(pictures[i + 1]) && - overlayRequirement[pictures[i + 1]]! >= - currentRenderCanvas.requiredDueTo) { - currentRenderCanvas = RenderingRenderCanvas( - requiredDueTo: overlayRequirement[pictures[i + 1]]!); - currentRenderCanvas.add(pictures[i + 1]); - result.add(currentRenderCanvas); - } else { - currentRenderCanvas.add(pictures[i + 1]); - } + result.add(RenderingPlatformView(platformViews[i])); + currentRenderCanvas.add(pictures[i + 1]); + } + if (currentRenderCanvas.pictures.isNotEmpty) { + result.add(currentRenderCanvas); } return result; } - -/// Updates the DOM to display the [next] rendering by using the fewest -/// DOM operations to rearrange the [current] rendering. -void updateRendering(Rendering current, Rendering next) {} From e5ce39d829d1211686c608471dc922637c0e8806 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Tue, 13 Feb 2024 15:00:29 -0800 Subject: [PATCH 09/28] fix ups --- .../src/engine/canvaskit/embedded_views.dart | 188 ++++-------------- .../test/canvaskit/embedded_views_test.dart | 4 + 2 files changed, 46 insertions(+), 146 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 25097e1e53553..3c8ad39db1b05 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -15,7 +15,6 @@ import '../vector_math.dart'; import 'canvas.dart'; import 'embedded_views_diff.dart'; import 'overlay_scene_optimizer.dart'; -import 'painting.dart'; import 'path.dart'; import 'picture.dart'; import 'picture_recorder.dart'; @@ -95,14 +94,9 @@ class HtmlViewEmbedder { } void prerollCompositeEmbeddedView(int viewId, EmbeddedViewParams params) { - // We need an overlay for each visible platform view. Invisible platform - // views will be grouped with (at most) one visible platform view later. - final bool needNewOverlay = PlatformViewManager.instance.isVisible(viewId); - if (needNewOverlay) { - final CkPictureRecorder pictureRecorder = CkPictureRecorder(); - pictureRecorder.beginRecording(ui.Offset.zero & _frameSize); - _context.pictureRecordersCreatedDuringPreroll.add(pictureRecorder); - } + final CkPictureRecorder pictureRecorder = CkPictureRecorder(); + pictureRecorder.beginRecording(ui.Offset.zero & _frameSize); + _context.pictureRecordersCreatedDuringPreroll.add(pictureRecorder); // Do nothing if the params didn't change. if (_currentCompositionParams[viewId] == params) { @@ -125,21 +119,15 @@ class HtmlViewEmbedder { // Ensure platform view with `viewId` is injected into the `rasterizer.view`. rasterizer.view.dom.injectPlatformView(viewId); - final int overlayIndex = _context.visibleViewCount; + final int overlayIndex = _context.viewCount; _compositionOrder.add(viewId); - // Keep track of the number of visible platform views. - if (PlatformViewManager.instance.isVisible(viewId)) { - _context.visibleViewCount++; - } - // We need a new overlay if this is a visible view. - final bool needNewOverlay = PlatformViewManager.instance.isVisible(viewId); + _context.viewCount++; + CkPictureRecorder? recorderToUseForRendering; - if (needNewOverlay) { - if (overlayIndex < _context.pictureRecordersCreatedDuringPreroll.length) { - recorderToUseForRendering = - _context.pictureRecordersCreatedDuringPreroll[overlayIndex]; - _context.pictureRecorders.add(recorderToUseForRendering); - } + if (overlayIndex < _context.pictureRecordersCreatedDuringPreroll.length) { + recorderToUseForRendering = + _context.pictureRecordersCreatedDuringPreroll[overlayIndex]; + _context.pictureRecorders.add(recorderToUseForRendering); } if (_viewsToRecomposite.contains(viewId)) { @@ -377,12 +365,8 @@ class HtmlViewEmbedder { } final Rendering rendering = createOptimizedRendering( pictures, _compositionOrder, _currentCompositionParams); - final ViewListDiffResult? diffResult = - (_activeCompositionOrder.isEmpty || _compositionOrder.isEmpty) - ? null - : diffViewList(_activeCompositionOrder, _compositionOrder); final List? overlayGroups = - _updateOverlays(rendering, diffResult); + _updateOverlays(rendering); if (overlayGroups != null) { _activeOverlayGroups = overlayGroups; } @@ -393,14 +377,6 @@ class HtmlViewEmbedder { ); final List renderCanvases = rendering.canvases; - if (renderCanvases.length != _activeOverlayGroups.length + 1) { - print('render canvas count = ${renderCanvases.length}'); - print( - 'overlay groups = ${_activeOverlayGroups.map((group) => group._group)}'); - print('overlayGroups was null? ${overlayGroups == null}'); - } - assert(renderCanvases.length == _activeOverlayGroups.length + 1); - await rasterizer.rasterizeToCanvas(baseCanvas, renderCanvases[0].pictures); // The first render canvas is the base canvas, not an overlay, so skip it @@ -431,96 +407,31 @@ class HtmlViewEmbedder { List? debugInvalidViewIds; - if (diffResult != null) { - // Dispose of the views that should be removed, except for the ones which - // are going to be added back. Moving rather than removing and re-adding - // the view helps it maintain state. - disposeViews(diffResult.viewsToRemove - .where((int view) => !diffResult.viewsToAdd.contains(view))); - _activeCompositionOrder.addAll(_compositionOrder); - unusedViews.removeAll(_compositionOrder); - - DomElement? elementToInsertBefore; - if (diffResult.addToBeginning) { - elementToInsertBefore = - _viewClipChains[diffResult.viewToInsertBefore!]!.root; - } + rasterizer.removeOverlaysFromDom(); + for (int i = 0; i < _compositionOrder.length; i++) { + final int viewId = _compositionOrder[i]; - for (final int viewId in diffResult.viewsToAdd) { - bool isViewInvalid = false; - assert(() { - isViewInvalid = !PlatformViewManager.instance.knowsViewId(viewId); - if (isViewInvalid) { - debugInvalidViewIds ??= []; - debugInvalidViewIds!.add(viewId); - } - return true; - }()); + bool isViewInvalid = false; + assert(() { + isViewInvalid = !PlatformViewManager.instance.knowsViewId(viewId); if (isViewInvalid) { - continue; - } - - if (diffResult.addToBeginning) { - final DomElement platformViewRoot = _viewClipChains[viewId]!.root; - sceneHost.insertBefore(platformViewRoot, elementToInsertBefore); - final DisplayCanvas? overlay = _overlays[viewId]; - if (overlay != null) { - sceneHost.insertBefore(overlay.hostElement, elementToInsertBefore); - } - } else { - final DomElement platformViewRoot = _viewClipChains[viewId]!.root; - sceneHost.append(platformViewRoot); - final DisplayCanvas? overlay = _overlays[viewId]; - if (overlay != null) { - sceneHost.append(overlay.hostElement); - } + debugInvalidViewIds ??= []; + debugInvalidViewIds!.add(viewId); } + return true; + }()); + if (isViewInvalid) { + continue; } - // It's possible that some platform views which were in the unchanged - // section have newly assigned overlays. If so, add them to the DOM. - for (int i = 0; i < _compositionOrder.length; i++) { - final int view = _compositionOrder[i]; - if (_overlays[view] != null) { - final DomElement overlayElement = _overlays[view]!.hostElement; - if (!overlayElement.isConnected!) { - // This overlay wasn't added to the DOM. - if (i == _compositionOrder.length - 1) { - sceneHost.append(overlayElement); - } else { - final int nextView = _compositionOrder[i + 1]; - final DomElement nextElement = _viewClipChains[nextView]!.root; - sceneHost.insertBefore(overlayElement, nextElement); - } - } - } - } - } else { - rasterizer.removeOverlaysFromDom(); - for (int i = 0; i < _compositionOrder.length; i++) { - final int viewId = _compositionOrder[i]; - - bool isViewInvalid = false; - assert(() { - isViewInvalid = !PlatformViewManager.instance.knowsViewId(viewId); - if (isViewInvalid) { - debugInvalidViewIds ??= []; - debugInvalidViewIds!.add(viewId); - } - return true; - }()); - if (isViewInvalid) { - continue; - } - final DomElement platformViewRoot = _viewClipChains[viewId]!.root; - final DisplayCanvas? overlay = _overlays[viewId]; - sceneHost.append(platformViewRoot); - if (overlay != null) { - sceneHost.append(overlay.hostElement); - } - _activeCompositionOrder.add(viewId); - unusedViews.remove(viewId); + final DomElement platformViewRoot = _viewClipChains[viewId]!.root; + final DisplayCanvas? overlay = _overlays[viewId]; + sceneHost.append(platformViewRoot); + if (overlay != null) { + sceneHost.append(overlay.hostElement); } + _activeCompositionOrder.add(viewId); + unusedViews.remove(viewId); } _compositionOrder.clear(); @@ -573,7 +484,7 @@ class HtmlViewEmbedder { // // TODO(hterkelsen): Test this more thoroughly. List? _updateOverlays( - Rendering rendering, ViewListDiffResult? diffResult) { + Rendering rendering) { if (rendering.equalsForRendering(_activeRendering)) { // The rendering has not changed, continue using the assigned overlays. return null; @@ -581,30 +492,18 @@ class HtmlViewEmbedder { // Group platform views from their composition order. // Each group contains one visible view, and any number of invisible views // before or after that visible view. + // TODO(harryterkelsen): Fix case where RenderCanvas isn't the first + // element in the rendering. final List overlayGroups = getOverlayGroups(rendering); final List viewsNeedingOverlays = overlayGroups.map((OverlayGroup group) => group.last).toList(); - if (diffResult == null) { - // Everything is going to be explicitly recomposited anyway. Release all - // the surfaces and assign an overlay to all the surfaces needing one. - rasterizer.releaseOverlays(); - _overlays.clear(); - viewsNeedingOverlays.forEach(_initializeOverlay); - } else { - // We want to preserve the overlays in the "unchanged" section of the - // diff result as much as possible. Iterate over all the views needing - // overlays and assign them an overlay if they don't have one already. - - // Use `toList` here since we will modify `_overlays` in the for-loop - // below. - final Iterable viewsWithOverlays = _overlays.keys.toList(); - viewsWithOverlays - .where((int view) => !viewsNeedingOverlays.contains(view)) - .forEach(_releaseOverlay); - viewsNeedingOverlays - .where((int view) => !_overlays.containsKey(view)) - .forEach(_initializeOverlay); - } + + // Everything is going to be explicitly recomposited anyway. Release all + // the surfaces and assign an overlay to all the surfaces needing one. + rasterizer.releaseOverlays(); + _overlays.clear(); + + viewsNeedingOverlays.forEach(_initializeOverlay); assert(_overlays.length == viewsNeedingOverlays.length); return overlayGroups; } @@ -632,7 +531,6 @@ class HtmlViewEmbedder { currentGroup.add(entity.viewId); } } - assert(rendering.canvases.length == result.length + 1); return result; } @@ -903,8 +801,6 @@ class EmbedderFrameContext { /// This is a subset of [_pictureRecordersCreatedDuringPreroll]. final List pictureRecorders = []; - /// The number of platform views in this frame which are visible. - /// - /// These platform views will require overlays. - int visibleViewCount = 0; + /// The number of platform views in this frame. + int viewCount = 0; } diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index 2c6fc81684389..0a6a5f66476da 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -31,6 +31,10 @@ void testMain() { EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(1); }); + tearDown(() { + PlatformViewManager.instance.debugClear(); + }); + test('embeds interactive platform views', () async { ui_web.platformViewRegistry.registerViewFactory( 'test-platform-view', From dadd6d768410a46ab5755e7fd238efa5d3ea01d8 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Tue, 20 Feb 2024 13:18:11 -0800 Subject: [PATCH 10/28] wip --- lib/web_ui/lib/src/engine.dart | 1 - .../src/engine/canvaskit/embedded_views.dart | 57 ++++++- .../engine/canvaskit/embedded_views_diff.dart | 161 ------------------ 3 files changed, 52 insertions(+), 167 deletions(-) delete mode 100644 lib/web_ui/lib/src/engine/canvaskit/embedded_views_diff.dart diff --git a/lib/web_ui/lib/src/engine.dart b/lib/web_ui/lib/src/engine.dart index e59035b833530..25b2c1cb604c4 100644 --- a/lib/web_ui/lib/src/engine.dart +++ b/lib/web_ui/lib/src/engine.dart @@ -25,7 +25,6 @@ export 'engine/canvaskit/canvaskit_canvas.dart'; export 'engine/canvaskit/color_filter.dart'; export 'engine/canvaskit/display_canvas_factory.dart'; export 'engine/canvaskit/embedded_views.dart'; -export 'engine/canvaskit/embedded_views_diff.dart'; export 'engine/canvaskit/fonts.dart'; export 'engine/canvaskit/image.dart'; export 'engine/canvaskit/image_filter.dart'; 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 3c8ad39db1b05..122b2a28fd19c 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -1,6 +1,7 @@ // Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:math' as math; import 'package:ui/ui.dart' as ui; @@ -13,7 +14,6 @@ import '../svg.dart'; import '../util.dart'; import '../vector_math.dart'; import 'canvas.dart'; -import 'embedded_views_diff.dart'; import 'overlay_scene_optimizer.dart'; import 'path.dart'; import 'picture.dart'; @@ -365,8 +365,8 @@ class HtmlViewEmbedder { } final Rendering rendering = createOptimizedRendering( pictures, _compositionOrder, _currentCompositionParams); - final List? overlayGroups = - _updateOverlays(rendering); + final List? overlayGroups = _updateOverlays(rendering); + _activeRendering = rendering; if (overlayGroups != null) { _activeOverlayGroups = overlayGroups; } @@ -483,12 +483,14 @@ class HtmlViewEmbedder { // composition order of the current and previous frame, respectively. // // TODO(hterkelsen): Test this more thoroughly. - List? _updateOverlays( - Rendering rendering) { + List? _updateOverlays(Rendering rendering) { if (rendering.equalsForRendering(_activeRendering)) { // The rendering has not changed, continue using the assigned overlays. return null; } + final List indexMap = + _getIndexMapFromPreviousRendering(_activeRendering, rendering); + print('index map: $indexMap'); // Group platform views from their composition order. // Each group contains one visible view, and any number of invisible views // before or after that visible view. @@ -508,6 +510,51 @@ class HtmlViewEmbedder { return overlayGroups; } + List _getIndexMapFromPreviousRendering( + Rendering previous, Rendering next) { + assert(!previous.equalsForRendering(next), + 'Should not be in this method if the Renderings are equal'); + final List result = []; + int index = 0; + + final int maxLength = + math.min(previous.entities.length, next.entities.length); + + // A canvas in the previous rendering can only be used once in the next + // rendering. So if it is matched with one in the next rendering, mark it + // here so it is only matched once. + final Set alreadyClaimedCanvases = {}; + + // Add the unchanged elements from the beginning of the list. + while (index < maxLength && + previous.entities[index].equalsForRendering(next.entities[index])) { + result.add(index); + if (previous.entities[index] is RenderingRenderCanvas) { + alreadyClaimedCanvases.add(index); + } + index += 1; + } + + while (index < next.entities.length) { + for (int oldIndex = 0; + oldIndex < previous.entities.length; + oldIndex += 1) { + if (previous.entities[oldIndex] + .equalsForRendering(next.entities[index]) && + !alreadyClaimedCanvases.contains(oldIndex)) { + result.add(oldIndex); + if (previous.entities[oldIndex] is RenderingRenderCanvas) { + alreadyClaimedCanvases.add(oldIndex); + } + break; + } + } + index += 1; + } + + return result; + } + // Group the platform views into "overlay groups". These are sublists // of the composition order which can share the same overlay. Every overlay // group is a list containing a visible view followed by zero or more diff --git a/lib/web_ui/lib/src/engine/canvaskit/embedded_views_diff.dart b/lib/web_ui/lib/src/engine/canvaskit/embedded_views_diff.dart deleted file mode 100644 index b07cc97711782..0000000000000 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views_diff.dart +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -/// The results of diffing the current composition order with the active -/// composition order. -class ViewListDiffResult { - const ViewListDiffResult( - this.viewsToRemove, this.viewsToAdd, this.addToBeginning, - {this.viewToInsertBefore}); - - /// Views which should be removed from the scene. - final List viewsToRemove; - - /// Views to add to the scene. - final List viewsToAdd; - - /// If `true`, [viewsToAdd] should be added at the beginning of the scene. - /// Otherwise, they should be added at the end of the scene. - final bool addToBeginning; - - /// If [addToBeginning] is `true`, then this is the id of the platform view - /// to insert [viewsToAdd] before. - /// - /// `null` if [addToBeginning] is `false`. - final int? viewToInsertBefore; -} - -/// Diff the composition order with the active composition order. It is -/// common for the composition order and active composition order to differ -/// only slightly. -/// -/// Consider a scrolling list of platform views; from frame -/// to frame the composition order will change in one of two ways, depending -/// on which direction the list is scrolling. One or more views will be added -/// to the beginning of the list, and one or more views will be removed from -/// the end of the list, with the order of the unchanged middle views -/// remaining the same. -// TODO(hterkelsen): Refactor to use [longestIncreasingSubsequence] and logic -// 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; - } - - // This is tried if the first element of the next list is contained in the - // active list at `index`. If the active and next lists are in the expected - // form, then we should be able to iterate from `index` to the end of the - // active list where every element matches in the next list. - ViewListDiffResult? lookForwards(int index) { - for (int i = 0; i + index < active.length; i++) { - if (active[i + index] != next[i]) { - // An element in the next list didn't match. This isn't in the expected - // form we can optimize. - return null; - } - if (i == next.length - 1) { - // The entire next list was contained in the active list. - if (index == 0) { - // If the first index of the next list is also the first index in the - // active list, then the next list is the same as the active list with - // views removed from the end. - return ViewListDiffResult( - active.sublist(i + 1), const [], false); - } else if (i + index == active.length - 1) { - // If this is also the end of the active list, then the next list is - // the same as the active list with some views removed from the - // beginning. - return ViewListDiffResult( - active.sublist(0, index), const [], false); - } else { - return null; - } - } - } - // We reached the end of the active list but have not reached the end of the - // next list. The lists are in the expected form. We should remove the - // elements from `0` to `index` in the active list from the DOM and add the - // elements from `active.length - index` (the entire active list minus the - // number of new elements at the beginning) to the end of the next list to - // the DOM at the end of the list of platform views. - final List viewsToRemove = active.sublist(0, index); - final List viewsToAdd = next.sublist(active.length - index); - - return ViewListDiffResult( - viewsToRemove, - viewsToAdd, - false, - ); - } - - // This is tried if the last element of the next list is contained in the - // active list at `index`. If the lists are in the expected form, we should be - // able to iterate backwards from index to the beginning of the active list - // and have every element match the corresponding element from the next list. - ViewListDiffResult? lookBackwards(int index) { - for (int i = 0; index - i >= 0; i++) { - if (active[index - i] != next[next.length - 1 - i]) { - // An element from the next list didn't match the coresponding element - // from the active list. These lists aren't in the expected form. - return null; - } - if (i == next.length - 1) { - // The entire next list was contained in the active list. - if (index == active.length - 1) { - // If the last element of the next list is also the last element of - // the active list, then the next list is just the active list with - // some elements removed from the beginning. - return ViewListDiffResult( - active.sublist(0, active.length - i - 1), const [], false); - } else if (index == i) { - // If we also reached the beginning of the active list, then the next - // list is the same as the active list with some views removed from - // the end. - return ViewListDiffResult( - active.sublist(index + 1), const [], false); - } else { - return null; - } - } - } - - // We reached the beginning of the active list but have not exhausted the - // entire next list. The lists are in the expected form. We should remove - // the elements from the end of the active list which come after the element - // which matches the last index of the next list (everything after `index`). - // We should add the elements from the next list which we didn't reach while - // iterating above (the first `next.length - index` views). - final List viewsToRemove = active.sublist(index + 1); - final List viewsToAdd = next.sublist(0, next.length - 1 - index); - - return ViewListDiffResult( - viewsToRemove, - viewsToAdd, - true, - viewToInsertBefore: active.first, - ); - } - - // 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]. - final int firstIndex = active.indexOf(next.first); - final int lastIndex = active.lastIndexOf(next.last); - if (firstIndex != -1 && lastIndex != -1) { - // Both the first element and the last element of the next list are in the - // active list. Search in the direction that will result in the least - // amount of deletions. - if (firstIndex <= (active.length - lastIndex)) { - return lookForwards(firstIndex); - } else { - return lookBackwards(lastIndex); - } - } else if (firstIndex != -1) { - return lookForwards(firstIndex); - } else if (lastIndex != -1) { - return lookBackwards(lastIndex); - } else { - return null; - } -} From 6a004c35a60f615b83843ec72e2e79e5f0a3027a Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 21 Feb 2024 13:10:32 -0800 Subject: [PATCH 11/28] wip --- .../src/engine/canvaskit/embedded_views.dart | 1 + .../canvaskit/overlay_scene_optimizer.dart | 10 ++ .../canvaskit/embedded_views_diff_test.dart | 130 ------------------ 3 files changed, 11 insertions(+), 130 deletions(-) delete mode 100644 lib/web_ui/test/canvaskit/embedded_views_diff_test.dart 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 122b2a28fd19c..b375b35092d26 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -370,6 +370,7 @@ class HtmlViewEmbedder { if (overlayGroups != null) { _activeOverlayGroups = overlayGroups; } + print('RENDERING: ${rendering.entities}'); assert( _context.pictureRecorders.length >= _overlays.length, 'There should be at least as many picture recorders ' diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index 3b516f63d3b13..093c8ce692ddb 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -69,6 +69,11 @@ class RenderingRenderCanvas extends RenderingEntity { bool equalsForRendering(RenderingEntity other) { return other is RenderingRenderCanvas; } + + @override + String toString() { + return '$RenderingRenderCanvas(${pictures.length} pictures)'; + } } /// A platform view to be rendered. @@ -82,6 +87,11 @@ class RenderingPlatformView extends RenderingEntity { bool equalsForRendering(RenderingEntity other) { return other is RenderingPlatformView && other.viewId == viewId; } + + @override + String toString() { + return '$RenderingPlatformView($viewId)'; + } } // Computes the bounds of the platform view from its associated parameters. diff --git a/lib/web_ui/test/canvaskit/embedded_views_diff_test.dart b/lib/web_ui/test/canvaskit/embedded_views_diff_test.dart deleted file mode 100644 index cb5edda0d15d9..0000000000000 --- a/lib/web_ui/test/canvaskit/embedded_views_diff_test.dart +++ /dev/null @@ -1,130 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:test/bootstrap/browser.dart'; -import 'package:test/test.dart'; -import 'package:ui/src/engine/canvaskit/embedded_views_diff.dart'; - -import 'common.dart'; - -void main() { - internalBootstrapBrowserTest(() => testMain); -} - -void testMain() { - group('diffViewList', () { - setUpCanvasKitTest(); - - test('works in the expected case', () { - ViewListDiffResult? result = diffViewList( - [1, 2, 3, 4, 5], - [3, 4, 5, 6, 7], - ); - expect(result, isNotNull); - expect(result!.viewsToAdd, [6, 7]); - expect(result.viewsToRemove, [1, 2]); - expect(result.addToBeginning, isFalse); - - result = diffViewList( - [3, 4, 5, 6, 7], - [1, 2, 3, 4, 5], - ); - expect(result, isNotNull); - expect(result!.viewsToAdd, [1, 2]); - expect(result.viewsToRemove, [6, 7]); - expect(result.addToBeginning, isTrue); - expect(result.viewToInsertBefore, 3); - - result = diffViewList([3, 4, 5], [2, 3, 4, 5]); - expect(result, isNotNull); - expect(result!.viewsToAdd, [2]); - expect(result.viewsToRemove, []); - expect(result.addToBeginning, isTrue); - expect(result.viewToInsertBefore, 3); - - result = diffViewList([3, 4, 5], [3, 4, 5, 6]); - expect(result, isNotNull); - expect(result!.viewsToAdd, [6]); - expect(result.viewsToRemove, []); - expect(result.addToBeginning, isFalse); - - result = diffViewList([3, 4, 5, 6], [3, 4, 5]); - expect(result, isNotNull); - expect(result!.viewsToAdd, []); - expect(result.viewsToRemove, [6]); - - result = diffViewList([3, 4, 5, 6], [4, 5, 6]); - expect(result, isNotNull); - expect(result!.viewsToAdd, []); - expect(result.viewsToRemove, [3]); - expect(result.addToBeginning, isFalse); - - result = diffViewList([3, 4, 5, 6, 7, 8], [3, 4, 5]); - expect(result, isNotNull); - expect(result!.viewsToAdd, []); - expect(result.viewsToRemove, [6, 7, 8]); - - result = diffViewList([1, 2, 3, 4, 5, 6], [4, 5, 6]); - expect(result, isNotNull); - expect(result!.viewsToAdd, []); - expect(result.viewsToRemove, [1, 2, 3]); - expect(result.addToBeginning, isFalse); - - result = diffViewList([3, 4, 5, 6, 7, 8], [2, 3, 4, 5]); - expect(result, isNotNull); - expect(result!.viewsToAdd, [2]); - expect(result.viewsToRemove, [6, 7, 8]); - expect(result.addToBeginning, isTrue); - expect(result.viewToInsertBefore, 3); - - result = diffViewList([1, 2, 3, 4, 5, 6], [4, 5, 6, 7]); - expect(result, isNotNull); - expect(result!.viewsToAdd, [7]); - expect(result.viewsToRemove, [1, 2, 3]); - expect(result.addToBeginning, isFalse); - - result = diffViewList([1, 2, 3], [4, 5]); - expect(result, isNull); - - result = diffViewList([1, 2, 3, 4], [2, 3, 5, 4]); - expect(result, isNull); - - result = diffViewList([3, 4], [1, 2, 3, 4, 5, 6]); - expect(result, isNull); - - result = diffViewList([1, 2, 3, 4, 5], [2, 3, 4]); - expect(result, isNull); - }); - - test('works for flutter/flutter#101580', () { - ViewListDiffResult? result; - - // Reverse the list - result = diffViewList([1, 2, 3, 4], [4, 3, 2, 1]); - expect(result, isNotNull); - expect(result!.viewsToAdd, [3, 2, 1]); - expect(result.viewsToRemove, [1, 2, 3]); - expect(result.addToBeginning, isFalse); - - // Sort the list - result = diffViewList([3, 4, 1, 2], [1, 2, 3, 4]); - expect(result, isNotNull); - expect(result!.viewsToAdd, [3, 4]); - expect(result.viewsToRemove, [3, 4]); - expect(result.addToBeginning, isFalse); - - // Move last view to the beginning - result = diffViewList([2, 3, 4, 1], [1, 2, 3, 4]); - expect(result, isNotNull); - expect(result!.viewsToAdd, [1]); - expect(result.viewsToRemove, [1]); - expect(result.addToBeginning, isTrue); - expect(result.viewToInsertBefore, 2); - - // Shuffle the list - result = diffViewList([1, 2, 3, 4], [2, 4, 1, 3]); - expect(result, isNull); - }); - }); -} From 5a2a337eabf757ec553ac92395f1b0cf56a6264e Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Mon, 26 Feb 2024 09:51:09 -0800 Subject: [PATCH 12/28] wip --- .../src/engine/canvaskit/embedded_views.dart | 7 + .../lib/src/engine/canvaskit/rasterizer.dart | 5 + .../lib/src/engine/canvaskit/renderer.dart | 449 +++++++++--------- .../test/canvaskit/embedded_views_test.dart | 10 +- 4 files changed, 230 insertions(+), 241 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 b375b35092d26..ece1acac3ecee 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -619,6 +619,13 @@ class HtmlViewEmbedder { _viewsToRecomposite.clear(); _activeCompositionOrder.clear(); _compositionOrder.clear(); + _activeRendering = Rendering(); + } + + /// Clears the state. Used in tests. + void debugClear() { + dispose(); + rasterizer.removeOverlaysFromDom(); } } diff --git a/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart b/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart index 3402703ba382a..908db983b0886 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart @@ -102,6 +102,11 @@ abstract class ViewRasterizer { viewEmbedder.dispose(); displayFactory.dispose(); } + + /// Clears the state. Used in tests. + void debugClear() { + viewEmbedder.debugClear(); + } } /// A [DisplayCanvas] is an abstraction for a canvas element which displays diff --git a/lib/web_ui/lib/src/engine/canvaskit/renderer.dart b/lib/web_ui/lib/src/engine/canvaskit/renderer.dart index f4fdaef67c0f9..5ff15bf371694 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/renderer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/renderer.dart @@ -77,7 +77,8 @@ class CanvasKitRenderer implements Renderer { canvasKit = windowFlutterCanvasKit!; } else if (windowFlutterCanvasKitLoaded != null) { // CanvasKit is being preloaded by flutter.js. Wait for it to complete. - canvasKit = await promiseToFuture(windowFlutterCanvasKitLoaded!); + canvasKit = + await promiseToFuture(windowFlutterCanvasKitLoaded!); } else { canvasKit = await downloadCanvasKit(); windowFlutterCanvasKit = canvasKit; @@ -111,12 +112,11 @@ class CanvasKitRenderer implements Renderer { List? textureCoordinates, List? colors, List? indices, - }) => CkVertices( - mode, - positions, - textureCoordinates: textureCoordinates, - colors: colors, - indices: indices); + }) => + CkVertices(mode, positions, + textureCoordinates: textureCoordinates, + colors: colors, + indices: indices); @override ui.Vertices createVerticesRaw( @@ -125,26 +125,23 @@ class CanvasKitRenderer implements Renderer { Float32List? textureCoordinates, Int32List? colors, Uint16List? indices, - }) => CkVertices.raw( - mode, - positions, - textureCoordinates: textureCoordinates, - colors: colors, - indices: indices); + }) => + CkVertices.raw(mode, positions, + textureCoordinates: textureCoordinates, + colors: colors, + indices: indices); @override ui.Canvas createCanvas(ui.PictureRecorder recorder, [ui.Rect? cullRect]) => - CanvasKitCanvas(recorder, cullRect); + CanvasKitCanvas(recorder, cullRect); @override ui.Gradient createLinearGradient( - ui.Offset from, - ui.Offset to, - List colors, [ - List? colorStops, - ui.TileMode tileMode = ui.TileMode.clamp, - Float32List? matrix4 - ]) => CkGradientLinear(from, to, colors, colorStops, tileMode, matrix4); + ui.Offset from, ui.Offset to, List colors, + [List? colorStops, + ui.TileMode tileMode = ui.TileMode.clamp, + Float32List? matrix4]) => + CkGradientLinear(from, to, colors, colorStops, tileMode, matrix4); @override ui.Gradient createRadialGradient( @@ -154,38 +151,27 @@ class CanvasKitRenderer implements Renderer { List? colorStops, ui.TileMode tileMode = ui.TileMode.clamp, Float32List? matrix4, - ]) => CkGradientRadial(center, radius, colors, colorStops, tileMode, matrix4); + ]) => + CkGradientRadial(center, radius, colors, colorStops, tileMode, matrix4); @override - ui.Gradient createConicalGradient( - ui.Offset focal, - double focalRadius, - ui.Offset center, - double radius, - List colors, - [List? colorStops, - ui.TileMode tileMode = ui.TileMode.clamp, - Float32List? matrix] - ) => CkGradientConical( - focal, - focalRadius, - center, - radius, - colors, - colorStops, - tileMode, - matrix); - - @override - ui.Gradient createSweepGradient( - ui.Offset center, - List colors, [ - List? colorStops, - ui.TileMode tileMode = ui.TileMode.clamp, - double startAngle = 0.0, - double endAngle = math.pi * 2, - Float32List? matrix4 - ]) => CkGradientSweep(center, colors, colorStops, tileMode, startAngle, endAngle, matrix4); + ui.Gradient createConicalGradient(ui.Offset focal, double focalRadius, + ui.Offset center, double radius, List colors, + [List? colorStops, + ui.TileMode tileMode = ui.TileMode.clamp, + Float32List? matrix]) => + CkGradientConical(focal, focalRadius, center, radius, colors, colorStops, + tileMode, matrix); + + @override + ui.Gradient createSweepGradient(ui.Offset center, List colors, + [List? colorStops, + ui.TileMode tileMode = ui.TileMode.clamp, + double startAngle = 0.0, + double endAngle = math.pi * 2, + Float32List? matrix4]) => + CkGradientSweep( + center, colors, colorStops, tileMode, startAngle, endAngle, matrix4); @override ui.PictureRecorder createPictureRecorder() => CkPictureRecorder(); @@ -194,29 +180,32 @@ class CanvasKitRenderer implements Renderer { ui.SceneBuilder createSceneBuilder() => LayerSceneBuilder(); @override - ui.ImageFilter createBlurImageFilter({ - double sigmaX = 0.0, - double sigmaY = 0.0, - ui.TileMode tileMode = ui.TileMode.clamp - }) => CkImageFilter.blur(sigmaX: sigmaX, sigmaY: sigmaY, tileMode: tileMode); + ui.ImageFilter createBlurImageFilter( + {double sigmaX = 0.0, + double sigmaY = 0.0, + ui.TileMode tileMode = ui.TileMode.clamp}) => + CkImageFilter.blur(sigmaX: sigmaX, sigmaY: sigmaY, tileMode: tileMode); @override - ui.ImageFilter createDilateImageFilter({double radiusX = 0.0, double radiusY = 0.0}) { + ui.ImageFilter createDilateImageFilter( + {double radiusX = 0.0, double radiusY = 0.0}) { // TODO(fzyzcjy): implement dilate. https://github.com/flutter/flutter/issues/101085 - throw UnimplementedError('ImageFilter.dilate not implemented for CanvasKit.'); + throw UnimplementedError( + 'ImageFilter.dilate not implemented for CanvasKit.'); } @override - ui.ImageFilter createErodeImageFilter({double radiusX = 0.0, double radiusY = 0.0}) { + ui.ImageFilter createErodeImageFilter( + {double radiusX = 0.0, double radiusY = 0.0}) { // TODO(fzyzcjy): implement erode. https://github.com/flutter/flutter/issues/101085 - throw UnimplementedError('ImageFilter.erode not implemented for CanvasKit.'); + throw UnimplementedError( + 'ImageFilter.erode not implemented for CanvasKit.'); } @override - ui.ImageFilter createMatrixImageFilter( - Float64List matrix4, { - ui.FilterQuality filterQuality = ui.FilterQuality.low - }) => CkImageFilter.matrix(matrix: matrix4, filterQuality: filterQuality); + ui.ImageFilter createMatrixImageFilter(Float64List matrix4, + {ui.FilterQuality filterQuality = ui.FilterQuality.low}) => + CkImageFilter.matrix(matrix: matrix4, filterQuality: filterQuality); @override ui.ImageFilter composeImageFilters( @@ -230,33 +219,25 @@ class CanvasKitRenderer implements Renderer { inner = CkColorFilterImageFilter(colorFilter: colorFilter); } return CkImageFilter.compose( - outer: outer as CkImageFilter, inner: inner as CkImageFilter); + outer: outer as CkImageFilter, inner: inner as CkImageFilter); } @override - Future instantiateImageCodec( - Uint8List list, { - int? targetWidth, - int? targetHeight, - bool allowUpscaling = true - }) async => skiaInstantiateImageCodec( - list, - targetWidth, - targetHeight - ); + Future instantiateImageCodec(Uint8List list, + {int? targetWidth, + int? targetHeight, + bool allowUpscaling = true}) async => + skiaInstantiateImageCodec(list, targetWidth, targetHeight); @override - Future instantiateImageCodecFromUrl( - Uri uri, { - ui_web.ImageCodecChunkCallback? chunkCallback - }) => skiaInstantiateWebImageCodec(uri.toString(), chunkCallback); + Future instantiateImageCodecFromUrl(Uri uri, + {ui_web.ImageCodecChunkCallback? chunkCallback}) => + skiaInstantiateWebImageCodec(uri.toString(), chunkCallback); @override ui.Image createImageFromImageBitmap(DomImageBitmap imageBitmap) { - final SkImage? skImage = canvasKit.MakeLazyImageFromImageBitmap( - imageBitmap, - true - ); + final SkImage? skImage = + canvasKit.MakeLazyImageFromImageBitmap(imageBitmap, true); if (skImage == null) { throw Exception('Failed to convert image bitmap to an SkImage.'); } @@ -264,36 +245,26 @@ class CanvasKitRenderer implements Renderer { } @override - void decodeImageFromPixels( - Uint8List pixels, - int width, - int height, - ui.PixelFormat format, - ui.ImageDecoderCallback callback, { - int? rowBytes, - int? targetWidth, - int? targetHeight, - bool allowUpscaling = true - }) => skiaDecodeImageFromPixels( - pixels, - width, - height, - format, - callback, - rowBytes: rowBytes, - targetWidth: targetWidth, - targetHeight: targetHeight, - allowUpscaling: allowUpscaling - ); + void decodeImageFromPixels(Uint8List pixels, int width, int height, + ui.PixelFormat format, ui.ImageDecoderCallback callback, + {int? rowBytes, + int? targetWidth, + int? targetHeight, + bool allowUpscaling = true}) => + skiaDecodeImageFromPixels(pixels, width, height, format, callback, + rowBytes: rowBytes, + targetWidth: targetWidth, + targetHeight: targetHeight, + allowUpscaling: allowUpscaling); @override ui.ImageShader createImageShader( - ui.Image image, - ui.TileMode tmx, - ui.TileMode tmy, - Float64List matrix4, - ui.FilterQuality? filterQuality - ) => CkImageShader(image, tmx, tmy, matrix4, filterQuality); + ui.Image image, + ui.TileMode tmx, + ui.TileMode tmy, + Float64List matrix4, + ui.FilterQuality? filterQuality) => + CkImageShader(image, tmx, tmy, matrix4, filterQuality); @override ui.Path createPath() => CkPath(); @@ -303,111 +274,111 @@ class CanvasKitRenderer implements Renderer { @override ui.Path combinePaths(ui.PathOperation op, ui.Path path1, ui.Path path2) => - CkPath.combine(op, path1, path2); - - @override - ui.TextStyle createTextStyle({ - ui.Color? color, - ui.TextDecoration? decoration, - ui.Color? decorationColor, - ui.TextDecorationStyle? decorationStyle, - double? decorationThickness, - ui.FontWeight? fontWeight, - ui.FontStyle? fontStyle, - ui.TextBaseline? textBaseline, - String? fontFamily, - List? fontFamilyFallback, - double? fontSize, - double? letterSpacing, - double? wordSpacing, - double? height, - ui.TextLeadingDistribution? leadingDistribution, - ui.Locale? locale, - ui.Paint? background, - ui.Paint? foreground, - List? shadows, - List? fontFeatures, - List? fontVariations - }) => CkTextStyle( - color: color, - decoration: decoration, - decorationColor: decorationColor, - decorationStyle: decorationStyle, - decorationThickness: decorationThickness, - fontWeight: fontWeight, - fontStyle: fontStyle, - textBaseline: textBaseline, - fontFamily: fontFamily, - fontFamilyFallback: fontFamilyFallback, - fontSize: fontSize, - letterSpacing: letterSpacing, - wordSpacing: wordSpacing, - height: height, - leadingDistribution: leadingDistribution, - locale: locale, - background: background as CkPaint?, - foreground: foreground as CkPaint?, - shadows: shadows, - fontFeatures: fontFeatures, - fontVariations: fontVariations, - ); - - @override - ui.ParagraphStyle createParagraphStyle({ - ui.TextAlign? textAlign, - ui.TextDirection? textDirection, - int? maxLines, - String? fontFamily, - double? fontSize, - double? height, - ui.TextHeightBehavior? textHeightBehavior, - ui.FontWeight? fontWeight, - ui.FontStyle? fontStyle, - ui.StrutStyle? strutStyle, - String? ellipsis, - ui.Locale? locale - }) => CkParagraphStyle( - textAlign: textAlign, - textDirection: textDirection, - maxLines: maxLines, - fontFamily: fontFamily, - fontSize: fontSize, - height: height, - textHeightBehavior: textHeightBehavior, - fontWeight: fontWeight, - fontStyle: fontStyle, - strutStyle: strutStyle, - ellipsis: ellipsis, - locale: locale, - applyRoundingHack: !ui.ParagraphBuilder.shouldDisableRoundingHack, - ); - - @override - ui.StrutStyle createStrutStyle({ - String? fontFamily, - List? fontFamilyFallback, - double? fontSize, - double? height, - ui.TextLeadingDistribution? leadingDistribution, - double? leading, - ui.FontWeight? fontWeight, - ui.FontStyle? fontStyle, - bool? forceStrutHeight - }) => CkStrutStyle( - fontFamily: fontFamily, - fontFamilyFallback: fontFamilyFallback, - fontSize: fontSize, - height: height, - leadingDistribution: leadingDistribution, - leading: leading, - fontWeight: fontWeight, - fontStyle: fontStyle, - forceStrutHeight: forceStrutHeight, - ); + CkPath.combine(op, path1, path2); + + @override + ui.TextStyle createTextStyle( + {ui.Color? color, + ui.TextDecoration? decoration, + ui.Color? decorationColor, + ui.TextDecorationStyle? decorationStyle, + double? decorationThickness, + ui.FontWeight? fontWeight, + ui.FontStyle? fontStyle, + ui.TextBaseline? textBaseline, + String? fontFamily, + List? fontFamilyFallback, + double? fontSize, + double? letterSpacing, + double? wordSpacing, + double? height, + ui.TextLeadingDistribution? leadingDistribution, + ui.Locale? locale, + ui.Paint? background, + ui.Paint? foreground, + List? shadows, + List? fontFeatures, + List? fontVariations}) => + CkTextStyle( + color: color, + decoration: decoration, + decorationColor: decorationColor, + decorationStyle: decorationStyle, + decorationThickness: decorationThickness, + fontWeight: fontWeight, + fontStyle: fontStyle, + textBaseline: textBaseline, + fontFamily: fontFamily, + fontFamilyFallback: fontFamilyFallback, + fontSize: fontSize, + letterSpacing: letterSpacing, + wordSpacing: wordSpacing, + height: height, + leadingDistribution: leadingDistribution, + locale: locale, + background: background as CkPaint?, + foreground: foreground as CkPaint?, + shadows: shadows, + fontFeatures: fontFeatures, + fontVariations: fontVariations, + ); + + @override + ui.ParagraphStyle createParagraphStyle( + {ui.TextAlign? textAlign, + ui.TextDirection? textDirection, + int? maxLines, + String? fontFamily, + double? fontSize, + double? height, + ui.TextHeightBehavior? textHeightBehavior, + ui.FontWeight? fontWeight, + ui.FontStyle? fontStyle, + ui.StrutStyle? strutStyle, + String? ellipsis, + ui.Locale? locale}) => + CkParagraphStyle( + textAlign: textAlign, + textDirection: textDirection, + maxLines: maxLines, + fontFamily: fontFamily, + fontSize: fontSize, + height: height, + textHeightBehavior: textHeightBehavior, + fontWeight: fontWeight, + fontStyle: fontStyle, + strutStyle: strutStyle, + ellipsis: ellipsis, + locale: locale, + applyRoundingHack: !ui.ParagraphBuilder.shouldDisableRoundingHack, + ); + + @override + ui.StrutStyle createStrutStyle( + {String? fontFamily, + List? fontFamilyFallback, + double? fontSize, + double? height, + ui.TextLeadingDistribution? leadingDistribution, + double? leading, + ui.FontWeight? fontWeight, + ui.FontStyle? fontStyle, + bool? forceStrutHeight}) => + CkStrutStyle( + fontFamily: fontFamily, + fontFamilyFallback: fontFamilyFallback, + fontSize: fontSize, + height: height, + leadingDistribution: leadingDistribution, + leading: leading, + fontWeight: fontWeight, + fontStyle: fontStyle, + forceStrutHeight: forceStrutHeight, + ); @override ui.ParagraphBuilder createParagraphBuilder(ui.ParagraphStyle style) => - CkParagraphBuilder(style); + CkParagraphBuilder(style); // TODO(harryterkelsen): Merge this logic with the async logic in // [EngineScene], https://github.com/flutter/flutter/issues/142072. @@ -496,43 +467,51 @@ class CanvasKitRenderer implements Renderer { _rasterizers.clear(); } + /// Clears the state of this renderer. Used in tests. + void debugClear() { + for (final ViewRasterizer rasterizer in _rasterizers.values) { + rasterizer.debugClear(); + } + } + @override void clearFragmentProgramCache() { _programs.clear(); } - static final Map> _programs = >{}; + static final Map> _programs = + >{}; @override Future createFragmentProgram(String assetKey) { if (_programs.containsKey(assetKey)) { return _programs[assetKey]!; } - return _programs[assetKey] = ui_web.assetManager.load(assetKey).then((ByteData data) { + return _programs[assetKey] = + ui_web.assetManager.load(assetKey).then((ByteData data) { return CkFragmentProgram.fromBytes(assetKey, data.buffer.asUint8List()); }); } @override - ui.LineMetrics createLineMetrics({ - required bool hardBreak, - required double ascent, - required double descent, - required double unscaledAscent, - required double height, - required double width, - required double left, - required double baseline, - required int lineNumber - }) => EngineLineMetrics( - hardBreak: hardBreak, - ascent: ascent, - descent: descent, - unscaledAscent: unscaledAscent, - height: height, - width: width, - left: left, - baseline: baseline, - lineNumber: lineNumber - ); + ui.LineMetrics createLineMetrics( + {required bool hardBreak, + required double ascent, + required double descent, + required double unscaledAscent, + required double height, + required double width, + required double left, + required double baseline, + required int lineNumber}) => + EngineLineMetrics( + hardBreak: hardBreak, + ascent: ascent, + descent: descent, + unscaledAscent: unscaledAscent, + height: height, + width: width, + left: left, + baseline: baseline, + lineNumber: lineNumber); } diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index 0a6a5f66476da..0f23dedb6ef55 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -33,6 +33,7 @@ void testMain() { tearDown(() { PlatformViewManager.instance.debugClear(); + CanvasKitRenderer.instance.debugClear(); }); test('embeds interactive platform views', () async { @@ -980,10 +981,8 @@ void testMain() { }); test('can dispose without crashing', () async { - ui_web.platformViewRegistry.registerViewFactory( - 'test-view', - (int viewId) => - createDomHTMLDivElement()..className = 'platform-view', + ui_web.platformViewRegistry.registerViewFactory('test-view', + (int viewId) => createDomHTMLDivElement()..className = 'platform-view', isVisible: false); await createPlatformView(0, 'test-view'); @@ -1007,8 +1006,7 @@ void testMain() { ]); expect(() { - final HtmlViewEmbedder embedder = - (renderer as CanvasKitRenderer) + final HtmlViewEmbedder embedder = (renderer as CanvasKitRenderer) .debugGetRasterizerForView(implicitView)! .viewEmbedder; // The following line used to cause a "Concurrent modification during iteration" From 28994752ee5d62c92febf248836133f8eb354a63 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Tue, 27 Feb 2024 11:07:36 -0800 Subject: [PATCH 13/28] wip --- .../src/engine/canvaskit/embedded_views.dart | 97 +++++++++++++++---- .../lib/src/engine/canvaskit/rasterizer.dart | 5 +- 2 files changed, 80 insertions(+), 22 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 ece1acac3ecee..ec492d7b4a09c 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -46,9 +46,9 @@ class HtmlViewEmbedder { /// * The number of clipping elements used last time the view was composited. final Map _viewClipChains = {}; - /// The maximum number of overlays to create. Too many overlays can cause a - /// performance burden. - static const int maximumOverlays = 7; + /// The maximum number of render canvases to create. Too many canvases can + /// cause a performance burden. + static const int maximumCanvases = 8; /// Canvases used to draw on top of platform views, keyed by platform view ID. final Map _overlays = {}; @@ -357,14 +357,14 @@ class HtmlViewEmbedder { sceneHost.append(_svgPathDefs!); } - Future submitFrame( - DisplayCanvas baseCanvas, CkPicture basePicture) async { + Future submitFrame(CkPicture basePicture) async { final List pictures = [basePicture]; for (final CkPictureRecorder recorder in _context.pictureRecorders) { pictures.add(recorder.endRecording()); } - final Rendering rendering = createOptimizedRendering( + Rendering rendering = createOptimizedRendering( pictures, _compositionOrder, _currentCompositionParams); + rendering = _modifyRenderingForMaxCanvases(rendering); final List? overlayGroups = _updateOverlays(rendering); _activeRendering = rendering; if (overlayGroups != null) { @@ -378,12 +378,19 @@ class HtmlViewEmbedder { ); final List renderCanvases = rendering.canvases; - await rasterizer.rasterizeToCanvas(baseCanvas, renderCanvases[0].pictures); + int renderCanvasIndex = 0; - // The first render canvas is the base canvas, not an overlay, so skip it - // since we just rasterized to it above. - int renderCanvasIndex = 1; for (final OverlayGroup overlayGroup in _activeOverlayGroups) { + if (overlayGroup.isEmpty) { + // This is a canvas that comes before any platform views. + final DisplayCanvas baseCanvas = rasterizer.displayFactory.baseCanvas; + sceneHost.prepend(baseCanvas.hostElement); + final List pictures = + renderCanvases[renderCanvasIndex].pictures; + renderCanvasIndex++; + await rasterizer.rasterizeToCanvas(baseCanvas, pictures); + continue; + } final DisplayCanvas overlay = _overlays[overlayGroup.last]!; final List pictures = renderCanvases[renderCanvasIndex].pictures; @@ -467,6 +474,49 @@ class HtmlViewEmbedder { } } + /// Modify the given rendering by removing canvases until the number of + /// canvases is less than or equal to the maximum number of canvases. + Rendering _modifyRenderingForMaxCanvases(Rendering rendering) { + final Rendering result = Rendering(); + final int numCanvases = rendering.canvases.length; + if (numCanvases <= maximumCanvases) { + return rendering; + } + int numCanvasesToDelete = numCanvases - maximumCanvases; + final List picturesForLastCanvas = []; + final List modifiedEntities = + List.from(rendering.entities); + bool sawLastCanvas = false; + for (int i = rendering.entities.length - 1; i > 0; i--) { + final RenderingEntity entity = modifiedEntities[i]; + if (entity is RenderingRenderCanvas) { + if (!sawLastCanvas) { + sawLastCanvas = true; + picturesForLastCanvas.insertAll(0, entity.pictures); + continue; + } + modifiedEntities.removeAt(i); + picturesForLastCanvas.insertAll(0, entity.pictures); + numCanvasesToDelete--; + if (numCanvasesToDelete == 0) { + break; + } + } + } + // Replace the pictures in the last canvas with all the pictures from the + // deleted canvases. + for (int i = modifiedEntities.length - 1; i > 0; i--) { + final RenderingEntity entity = modifiedEntities[i]; + if (entity is RenderingRenderCanvas) { + entity.pictures.clear(); + entity.pictures.addAll(picturesForLastCanvas); + } + } + + result.entities.addAll(modifiedEntities); + return result; + } + // Assigns overlays to the embedded views in the scene. // // This method attempts to be efficient by taking advantage of the @@ -495,11 +545,11 @@ class HtmlViewEmbedder { // Group platform views from their composition order. // Each group contains one visible view, and any number of invisible views // before or after that visible view. - // TODO(harryterkelsen): Fix case where RenderCanvas isn't the first - // element in the rendering. final List overlayGroups = getOverlayGroups(rendering); - final List viewsNeedingOverlays = - overlayGroups.map((OverlayGroup group) => group.last).toList(); + final List viewsNeedingOverlays = overlayGroups + .where((OverlayGroup group) => !group.isEmpty) + .map((OverlayGroup group) => group.last) + .toList(); // Everything is going to be explicitly recomposited anyway. Release all // the surfaces and assign an overlay to all the surfaces needing one. @@ -571,17 +621,28 @@ class HtmlViewEmbedder { for (final RenderingEntity entity in rendering.entities) { /// We are at an overlay, end the current group and start the next group. if (entity is RenderingRenderCanvas) { - if (!currentGroup.isEmpty) { - result.add(currentGroup); - currentGroup = OverlayGroup(); - } + result.add(currentGroup); + currentGroup = OverlayGroup(); } else if (entity is RenderingPlatformView) { currentGroup.add(entity.viewId); } } + assert(_overlayGroupsAreValid(result)); return result; } + bool _overlayGroupsAreValid(List overlayGroups) { + // Overlay groups are valid if they are all non-empty except for (possibly) + // the first one. + for (int i = 0; i < overlayGroups.length; i++) { + final OverlayGroup overlayGroup = overlayGroups[i]; + if (overlayGroup.isEmpty && i != 0) { + return false; + } + } + return true; + } + void _initializeOverlay(int viewId) { assert(!_overlays.containsKey(viewId)); diff --git a/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart b/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart index 908db983b0886..6cd4989e937ef 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart @@ -61,10 +61,7 @@ abstract class ViewRasterizer { compositorFrame.raster(layerTree, ignoreRasterCache: true); - sceneHost.prepend(displayFactory.baseCanvas.hostElement); - - await viewEmbedder.submitFrame( - displayFactory.baseCanvas, pictureRecorder.endRecording()); + await viewEmbedder.submitFrame(pictureRecorder.endRecording()); } /// Do some initialization to prepare to draw a frame. From dcc36517fdb31cd4bdfee7fe2d6215ab0d984b94 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 28 Feb 2024 10:55:43 -0800 Subject: [PATCH 14/28] wip --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 1 + 1 file changed, 1 insertion(+) 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 ec492d7b4a09c..4e8c57c8c3d03 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -510,6 +510,7 @@ class HtmlViewEmbedder { if (entity is RenderingRenderCanvas) { entity.pictures.clear(); entity.pictures.addAll(picturesForLastCanvas); + break; } } From 251eb75578a7a2641a43af1aed0b90bd6c629bbe Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 28 Feb 2024 17:01:22 -0800 Subject: [PATCH 15/28] Tests working --- .../src/engine/canvaskit/embedded_views.dart | 208 ++++++++++++------ .../canvaskit/overlay_scene_optimizer.dart | 14 +- .../test/canvaskit/embedded_views_test.dart | 76 +++++-- 3 files changed, 215 insertions(+), 83 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 4e8c57c8c3d03..a490f2b2ae02c 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -5,7 +5,8 @@ import 'dart:math' as math; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' show PlatformViewManager; +import '../../engine.dart' + show PlatformViewManager, longestIncreasingSubsequence; import '../display.dart'; import '../dom.dart'; import '../html/path_to_svg_clip.dart'; @@ -365,38 +366,20 @@ class HtmlViewEmbedder { Rendering rendering = createOptimizedRendering( pictures, _compositionOrder, _currentCompositionParams); rendering = _modifyRenderingForMaxCanvases(rendering); - final List? overlayGroups = _updateOverlays(rendering); - _activeRendering = rendering; - if (overlayGroups != null) { - _activeOverlayGroups = overlayGroups; + _updateDomForNewRendering(rendering); + if (rendering.equalsForRendering(_activeRendering)) { + // Just set the rendering to be the active rendering so the display + // canvases carry over as well. + rendering = _activeRendering; } - print('RENDERING: ${rendering.entities}'); - assert( - _context.pictureRecorders.length >= _overlays.length, - 'There should be at least as many picture recorders ' - '(${_context.pictureRecorders.length}) as overlays (${_overlays.length}).', - ); + _activeRendering = rendering; final List renderCanvases = rendering.canvases; - int renderCanvasIndex = 0; - - for (final OverlayGroup overlayGroup in _activeOverlayGroups) { - if (overlayGroup.isEmpty) { - // This is a canvas that comes before any platform views. - final DisplayCanvas baseCanvas = rasterizer.displayFactory.baseCanvas; - sceneHost.prepend(baseCanvas.hostElement); - final List pictures = - renderCanvases[renderCanvasIndex].pictures; - renderCanvasIndex++; - await rasterizer.rasterizeToCanvas(baseCanvas, pictures); - continue; - } - final DisplayCanvas overlay = _overlays[overlayGroup.last]!; - final List pictures = - renderCanvases[renderCanvasIndex].pictures; - renderCanvasIndex++; - await rasterizer.rasterizeToCanvas(overlay, pictures); + for (final RenderingRenderCanvas renderCanvas in renderCanvases) { + await rasterizer.rasterizeToCanvas( + renderCanvas.displayCanvas!, renderCanvas.pictures); } + for (final CkPictureRecorder recorder in _context.pictureRecordersCreatedDuringPreroll) { if (recorder.isRecording) { @@ -415,7 +398,6 @@ class HtmlViewEmbedder { List? debugInvalidViewIds; - rasterizer.removeOverlaysFromDom(); for (int i = 0; i < _compositionOrder.length; i++) { final int viewId = _compositionOrder[i]; @@ -432,19 +414,13 @@ class HtmlViewEmbedder { continue; } - final DomElement platformViewRoot = _viewClipChains[viewId]!.root; - final DisplayCanvas? overlay = _overlays[viewId]; - sceneHost.append(platformViewRoot); - if (overlay != null) { - sceneHost.append(overlay.hostElement); - } _activeCompositionOrder.add(viewId); unusedViews.remove(viewId); } _compositionOrder.clear(); - disposeViews(unusedViews); + unusedViews.forEach(disposeView); assert( debugInvalidViewIds == null || debugInvalidViewIds!.isEmpty, @@ -453,25 +429,14 @@ class HtmlViewEmbedder { ); } - void disposeViews(Iterable viewsToDispose) { - for (final int viewId in viewsToDispose) { - // Remove viewId from the _viewClipChains Map, and then from the DOM. - final ViewClipChain? clipChain = _viewClipChains.remove(viewId); - clipChain?.root.remove(); - // More cleanup - _currentCompositionParams.remove(viewId); - _viewsToRecomposite.remove(viewId); - _cleanUpClipDefs(viewId); - _svgClipDefs.remove(viewId); - } - } - - void _releaseOverlay(int viewId) { - if (_overlays[viewId] != null) { - final DisplayCanvas overlay = _overlays[viewId]!; - rasterizer.releaseOverlay(overlay); - _overlays.remove(viewId); - } + void disposeView(int viewId) { + final ViewClipChain? clipChain = _viewClipChains.remove(viewId); + clipChain?.root.remove(); + // More cleanup + _currentCompositionParams.remove(viewId); + _viewsToRecomposite.remove(viewId); + _cleanUpClipDefs(viewId); + _svgClipDefs.remove(viewId); } /// Modify the given rendering by removing canvases until the number of @@ -518,6 +483,119 @@ class HtmlViewEmbedder { return result; } + void _updateDomForNewRendering(Rendering rendering) { + if (rendering.equalsForRendering(_activeRendering)) { + // The rendering has not changed, so no DOM manipulation is needed. + return; + } + final List indexMap = + _getIndexMapFromPreviousRendering(_activeRendering, rendering); + final List existingIndexMap = + indexMap.where((int index) => index != -1).toList(); + + final List staticElements = + longestIncreasingSubsequence(existingIndexMap); + // Convert longest increasing subsequence from subsequence of indices of + // `existingIndexMap` to a subsequence of indices in previous rendering. + for (int i = 0; i < staticElements.length; i++) { + staticElements[i] = existingIndexMap[staticElements[i]]; + } + + // Remove elements which are in the active rendering, but not in the new + // rendering. + for (int i = 0; i < _activeRendering.entities.length; i++) { + if (indexMap.contains(i)) { + continue; + } + final RenderingEntity entity = _activeRendering.entities[i]; + if (entity is RenderingPlatformView) { + disposeView(entity.viewId); + } else if (entity is RenderingRenderCanvas) { + assert( + entity.displayCanvas != null, + 'RenderCanvas in previous rendering was ' + 'not assigned a DisplayCanvas'); + rasterizer.releaseOverlay(entity.displayCanvas!); + entity.displayCanvas = null; + } + } + + // Updates [renderCanvas] (located in [index] in the next rendering) to have + // a display canvas, either taken from the associated render canvas in the + // previous rendering, or newly created. + void updateRenderCanvasWithDisplay( + RenderingRenderCanvas renderCanvas, int index) { + // Does [nextEntity] correspond with a render canvas in the previous + // rendering? If so, then the render canvas in the previous rendering + // had an associated display canvas. Use this display canvas for + // [nextEntity]. + if (indexMap[index] != -1) { + final RenderingEntity previousEntity = + _activeRendering.entities[indexMap[index]]; + assert(previousEntity is RenderingRenderCanvas && + previousEntity.displayCanvas != null); + renderCanvas.displayCanvas = + (previousEntity as RenderingRenderCanvas).displayCanvas; + previousEntity.displayCanvas = null; + } else { + // There is no corresponding render canvas in the previous + // rendering. So this render canvas needs a display canvas. + renderCanvas.displayCanvas = rasterizer.getOverlay(); + } + } + + // At this point, the DOM contains the static elements and the elements from + // the previous rendering which need to move. We iterate over the static + // elements and insert the elements which come before them into the DOM. + int staticElementIndex = 0; + int nextRenderingIndex = 0; + while (staticElementIndex < staticElements.length) { + final int staticElementIndexInActiveRendering = + staticElements[staticElementIndex]; + final DomElement staticDomElement = _getElement( + _activeRendering.entities[staticElementIndexInActiveRendering]); + // Go through next rendering elements until we reach the static element. + while ( + indexMap[nextRenderingIndex] != staticElementIndexInActiveRendering) { + final RenderingEntity nextEntity = + rendering.entities[nextRenderingIndex]; + if (nextEntity is RenderingRenderCanvas) { + updateRenderCanvasWithDisplay(nextEntity, nextRenderingIndex); + } + sceneHost.insertBefore(_getElement(nextEntity), staticDomElement); + nextRenderingIndex++; + } + if (rendering.entities[nextRenderingIndex] is RenderingRenderCanvas) { + updateRenderCanvasWithDisplay( + rendering.entities[nextRenderingIndex] as RenderingRenderCanvas, + nextRenderingIndex); + } + // Also increment the next rendering index because this is the static + // element. + nextRenderingIndex++; + staticElementIndex++; + } + + // Add the leftover entities. + while (nextRenderingIndex < rendering.entities.length) { + final RenderingEntity nextEntity = rendering.entities[nextRenderingIndex]; + if (nextEntity is RenderingRenderCanvas) { + updateRenderCanvasWithDisplay(nextEntity, nextRenderingIndex); + } + sceneHost.append(_getElement(nextEntity)); + nextRenderingIndex++; + } + } + + DomElement _getElement(RenderingEntity entity) { + switch (entity) { + case RenderingRenderCanvas(): + return entity.displayCanvas!.hostElement; + case RenderingPlatformView(): + return _viewClipChains[entity.viewId]!.root; + } + } + // Assigns overlays to the embedded views in the scene. // // This method attempts to be efficient by taking advantage of the @@ -540,9 +618,6 @@ class HtmlViewEmbedder { // The rendering has not changed, continue using the assigned overlays. return null; } - final List indexMap = - _getIndexMapFromPreviousRendering(_activeRendering, rendering); - print('index map: $indexMap'); // Group platform views from their composition order. // Each group contains one visible view, and any number of invisible views // before or after that visible view. @@ -562,6 +637,9 @@ class HtmlViewEmbedder { return overlayGroups; } + /// Returns a [List] of ints mapping elements from the [next] rendering to + /// elements of the [previous] rendering. If there is no matching element in + /// the previous rendering, then the index map for that element is `-1`. List _getIndexMapFromPreviousRendering( Rendering previous, Rendering next) { assert(!previous.equalsForRendering(next), @@ -569,7 +647,7 @@ class HtmlViewEmbedder { final List result = []; int index = 0; - final int maxLength = + final int maxUnchangedLength = math.min(previous.entities.length, next.entities.length); // A canvas in the previous rendering can only be used once in the next @@ -578,7 +656,7 @@ class HtmlViewEmbedder { final Set alreadyClaimedCanvases = {}; // Add the unchanged elements from the beginning of the list. - while (index < maxLength && + while (index < maxUnchangedLength && previous.entities[index].equalsForRendering(next.entities[index])) { result.add(index); if (previous.entities[index] is RenderingRenderCanvas) { @@ -588,6 +666,7 @@ class HtmlViewEmbedder { } while (index < next.entities.length) { + bool foundForIndex = false; for (int oldIndex = 0; oldIndex < previous.entities.length; oldIndex += 1) { @@ -598,12 +677,17 @@ class HtmlViewEmbedder { if (previous.entities[oldIndex] is RenderingRenderCanvas) { alreadyClaimedCanvases.add(oldIndex); } + foundForIndex = true; break; } } + if (!foundForIndex) { + result.add(-1); + } index += 1; } + assert(result.length == next.entities.length); return result; } @@ -671,7 +755,7 @@ class HtmlViewEmbedder { /// Disposes the state of this view embedder. void dispose() { - disposeViews(_viewClipChains.keys.toList()); + _viewClipChains.keys.toList().forEach(disposeView); _context = EmbedderFrameContext(); _currentCompositionParams.clear(); debugCleanupSvgClipPaths(); diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index 093c8ce692ddb..11838f7a8a9a7 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -11,6 +11,7 @@ import '../display.dart'; import '../vector_math.dart'; import 'embedded_views.dart'; import 'picture.dart'; +import 'rasterizer.dart'; /// A [Rendering] is a concrete description of how a Flutter scene will be /// rendered in a web browser. @@ -60,6 +61,11 @@ class RenderingRenderCanvas extends RenderingEntity { /// The [pictures] which should be rendered in this canvas. final List pictures = []; + /// The [DisplayCanvas] that will be used to display [pictures]. + /// + /// This is set by the view embedder. + DisplayCanvas? displayCanvas; + /// Adds the [picture] to the pictures that should be rendered in this canvas. void add(CkPicture picture) { pictures.add(picture); @@ -163,7 +169,9 @@ Rendering createOptimizedRendering( // Since "V_0" intersects with all subsequent pictures, then the first picture // it intersects with is "P_0", so we create a new render canvas and add "P_0" // to it. - currentRenderCanvas.add(pictures[0]); + if (!pictures[0].cullRect.isEmpty) { + currentRenderCanvas.add(pictures[0]); + } for (int i = 0; i < platformViews.length; i++) { if (PlatformViewManager.instance.isVisible(platformViews[i])) { final ui.Rect platformViewBounds = @@ -181,7 +189,9 @@ Rendering createOptimizedRendering( } } result.add(RenderingPlatformView(platformViews[i])); - currentRenderCanvas.add(pictures[i + 1]); + if (!pictures[i + 1].cullRect.isEmpty) { + currentRenderCanvas.add(pictures[i + 1]); + } } if (currentRenderCanvas.pictures.isNotEmpty) { result.add(currentRenderCanvas); diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index 0f23dedb6ef55..c95787e744775 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -342,8 +342,8 @@ void testMain() { _platformView, _overlay, _platformView, - _platformView, _overlay, + _platformView, ]); // Frame 2: @@ -369,7 +369,6 @@ void testMain() { _platformView, _overlay, _platformView, - _overlay, ]); // Frame 4: @@ -399,8 +398,8 @@ void testMain() { _platformView, _platformView, _platformView, - _platformView, _overlay, + _platformView, ]); // Frame 5: @@ -496,8 +495,8 @@ void testMain() { _platformView, _platformView, _platformView, - _platformView, _overlay, + _platformView, ]); // Frame 2: @@ -521,8 +520,8 @@ void testMain() { _platformView, _platformView, _platformView, - _platformView, _overlay, + _platformView, ]); // Frame 3: @@ -546,8 +545,8 @@ void testMain() { _platformView, _platformView, _platformView, - _platformView, _overlay, + _platformView, ]); // Frame 4: @@ -571,8 +570,8 @@ void testMain() { _platformView, _platformView, _platformView, - _platformView, _overlay, + _platformView, ]); for (int i = 0; i < 20; i++) { @@ -594,7 +593,6 @@ void testMain() { _expectSceneMatches(<_EmbeddedViewMarker>[ _overlay, _platformView, - _overlay, ]); expect(platformViewsHost.querySelector('flt-platform-view'), isNotNull); @@ -643,7 +641,6 @@ void testMain() { _expectSceneMatches(<_EmbeddedViewMarker>[ _overlay, _platformView, - _overlay, ]); implicitView.debugPhysicalSizeOverride = const ui.Size(200, 200); @@ -652,7 +649,6 @@ void testMain() { _expectSceneMatches(<_EmbeddedViewMarker>[ _overlay, _platformView, - _overlay, ]); implicitView.debugPhysicalSizeOverride = null; @@ -676,7 +672,6 @@ void testMain() { _expectSceneMatches(<_EmbeddedViewMarker>[ _overlay, _platformView, - _overlay, ]); expect(platformViewsHost.querySelector('flt-platform-view'), isNotNull); @@ -690,7 +685,6 @@ void testMain() { _expectSceneMatches(<_EmbeddedViewMarker>[ _overlay, _platformView, - _overlay, ]); expect( @@ -775,6 +769,10 @@ void testMain() { }); test('does not create overlays for invisible platform views', () async { + final CkPicture testPicture = + paintPicture(const ui.Rect.fromLTRB(0, 0, 10, 10), (CkCanvas canvas) { + canvas.drawCircle(const ui.Offset(5, 5), 5, CkPaint()); + }); ui_web.platformViewRegistry.registerViewFactory( 'test-visible-view', (int viewId) => @@ -798,17 +796,20 @@ void testMain() { LayerSceneBuilder sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, _platformView, - ], reason: 'Invisible view alone renders on top of base overlay.'); + _overlay, + ], reason: 'Invisible view renders, followed by an overlay.'); sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(0, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); @@ -821,8 +822,11 @@ void testMain() { sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(0, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(2, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); @@ -832,16 +836,19 @@ void testMain() { _platformView, _overlay, _platformView, - _overlay, ], reason: 'Overlays created after each group containing a visible view.'); sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(0, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(2, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(3, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); @@ -857,10 +864,15 @@ void testMain() { sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(0, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(2, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(3, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(4, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); @@ -877,11 +889,17 @@ void testMain() { sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(0, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(2, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(3, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(4, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(5, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); @@ -899,12 +917,19 @@ void testMain() { sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(0, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(2, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(3, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(4, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(5, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(6, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); @@ -923,35 +948,44 @@ void testMain() { sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(3, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(4, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(5, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(6, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, _platformView, _platformView, _platformView, _platformView, _platformView, + _overlay, ], reason: 'Many invisible views can be rendered on top of the base overlay.'); sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(2, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(3, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(4, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, _platformView, + _overlay, _platformView, _platformView, _platformView, @@ -960,16 +994,20 @@ void testMain() { sb = LayerSceneBuilder(); sb.pushOffset(0, 0); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(4, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(3, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(2, width: 10, height: 10); + sb.addPicture(ui.Offset.zero, testPicture); sb.addPlatformView(1, width: 10, height: 10); sb.pop(); await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, _platformView, _platformView, + _overlay, _platformView, _platformView, _overlay, @@ -999,10 +1037,10 @@ void testMain() { await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, _platformView, _platformView, _platformView, + _overlay, ]); expect(() { From bbc8cf54eace46876f7f7587da52c29bb46ff148 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 28 Feb 2024 21:11:30 -0800 Subject: [PATCH 16/28] Fix build --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 3 +-- 1 file changed, 1 insertion(+), 2 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 a490f2b2ae02c..c150775b46014 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -5,8 +5,7 @@ import 'dart:math' as math; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' - show PlatformViewManager, longestIncreasingSubsequence; +import '../../engine.dart' show PlatformViewManager, longestIncreasingSubsequence; import '../display.dart'; import '../dom.dart'; import '../html/path_to_svg_clip.dart'; From 7d6c3a821e35fcced6f080be5065665e529a904a Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 28 Feb 2024 22:00:21 -0800 Subject: [PATCH 17/28] Fix licenses and unused members --- ci/licenses_golden/licenses_flutter | 6 +-- .../src/engine/canvaskit/embedded_views.dart | 44 ------------------- 2 files changed, 2 insertions(+), 48 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 9a49f408d70d2..c128fd1dd703a 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -35729,7 +35729,6 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.da ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/color_filter.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/display_canvas_factory.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/embedded_views_diff.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/fonts.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/image.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/image_filter.dart + ../../../flutter/LICENSE @@ -35742,8 +35741,8 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart + ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/multi_surface_rasterizer.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/offscreen_canvas_rasterizer.dart + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart + ../../../flutter/LICENSE @@ -38575,7 +38574,6 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/color_filter.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/display_canvas_factory.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart -FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/embedded_views_diff.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/fonts.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/image.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/image_filter.dart @@ -38588,8 +38586,8 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/mask_filter.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/multi_surface_rasterizer.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/n_way_canvas.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/native_memory.dart -FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/offscreen_canvas_rasterizer.dart +FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/painting.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/canvaskit/path_metrics.dart 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 c150775b46014..3bbd44be1256d 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -62,9 +62,6 @@ class HtmlViewEmbedder { /// The most recent composition order. final List _activeCompositionOrder = []; - /// The most recent overlay groups. - List _activeOverlayGroups = []; - /// The most recent rendering. Rendering _activeRendering = Rendering(); @@ -595,47 +592,6 @@ class HtmlViewEmbedder { } } - // Assigns overlays to the embedded views in the scene. - // - // This method attempts to be efficient by taking advantage of the - // [diffResult] and trying to re-use overlays which have already been - // assigned. - // - // This method accounts for invisible platform views by grouping them - // with the last visible platform view which precedes it. All invisible - // platform views that come after a visible view share the same overlay - // as the preceding visible view. - // - // This is called right before compositing the scene. - // - // [_compositionOrder] and [_activeComposition] order should contain the - // composition order of the current and previous frame, respectively. - // - // TODO(hterkelsen): Test this more thoroughly. - List? _updateOverlays(Rendering rendering) { - if (rendering.equalsForRendering(_activeRendering)) { - // The rendering has not changed, continue using the assigned overlays. - return null; - } - // Group platform views from their composition order. - // Each group contains one visible view, and any number of invisible views - // before or after that visible view. - final List overlayGroups = getOverlayGroups(rendering); - final List viewsNeedingOverlays = overlayGroups - .where((OverlayGroup group) => !group.isEmpty) - .map((OverlayGroup group) => group.last) - .toList(); - - // Everything is going to be explicitly recomposited anyway. Release all - // the surfaces and assign an overlay to all the surfaces needing one. - rasterizer.releaseOverlays(); - _overlays.clear(); - - viewsNeedingOverlays.forEach(_initializeOverlay); - assert(_overlays.length == viewsNeedingOverlays.length); - return overlayGroups; - } - /// Returns a [List] of ints mapping elements from the [next] rendering to /// elements of the [previous] rendering. If there is no matching element in /// the previous rendering, then the index map for that element is `-1`. From 5441d5c354c40027c17209eb6215430944da9190 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Wed, 28 Feb 2024 22:35:54 -0800 Subject: [PATCH 18/28] Remove unused method --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 8 -------- 1 file changed, 8 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 3bbd44be1256d..091cbdc3b7bba 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -683,14 +683,6 @@ class HtmlViewEmbedder { return true; } - void _initializeOverlay(int viewId) { - assert(!_overlays.containsKey(viewId)); - - // Try reusing a cached overlay created for another platform view. - final DisplayCanvas overlay = rasterizer.getOverlay(); - _overlays[viewId] = overlay; - } - /// Deletes SVG clip paths, useful for tests. void debugCleanupSvgClipPaths() { final DomElement? parent = _svgPathDefs?.children.single; From 271f699c4de33f100561c9aff0fee70e490c8368 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 29 Feb 2024 10:08:40 -0800 Subject: [PATCH 19/28] Remove more unused methods --- .../src/engine/canvaskit/embedded_views.dart | 78 ------------------- 1 file changed, 78 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 091cbdc3b7bba..a31778d8688f3 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -50,9 +50,6 @@ class HtmlViewEmbedder { /// cause a performance burden. static const int maximumCanvases = 8; - /// Canvases used to draw on top of platform views, keyed by platform view ID. - final Map _overlays = {}; - /// The views that need to be recomposited into the scene on the next frame. final Set _viewsToRecomposite = {}; @@ -646,43 +643,6 @@ class HtmlViewEmbedder { return result; } - // Group the platform views into "overlay groups". These are sublists - // of the composition order which can share the same overlay. Every overlay - // group is a list containing a visible view followed by zero or more - // invisible views. - // - // If there are more visible views than overlays, then the views which cannot - // be assigned an overlay are grouped together and will be rendered on top of - // the rest of the scene. - List getOverlayGroups(Rendering rendering) { - final List result = []; - OverlayGroup currentGroup = OverlayGroup(); - - for (final RenderingEntity entity in rendering.entities) { - /// We are at an overlay, end the current group and start the next group. - if (entity is RenderingRenderCanvas) { - result.add(currentGroup); - currentGroup = OverlayGroup(); - } else if (entity is RenderingPlatformView) { - currentGroup.add(entity.viewId); - } - } - assert(_overlayGroupsAreValid(result)); - return result; - } - - bool _overlayGroupsAreValid(List overlayGroups) { - // Overlay groups are valid if they are all non-empty except for (possibly) - // the first one. - for (int i = 0; i < overlayGroups.length; i++) { - final OverlayGroup overlayGroup = overlayGroups[i]; - if (overlayGroup.isEmpty && i != 0) { - return false; - } - } - return true; - } - /// Deletes SVG clip paths, useful for tests. void debugCleanupSvgClipPaths() { final DomElement? parent = _svgPathDefs?.children.single; @@ -708,7 +668,6 @@ class HtmlViewEmbedder { debugCleanupSvgClipPaths(); _currentCompositionParams.clear(); _viewClipChains.clear(); - _overlays.clear(); _viewsToRecomposite.clear(); _activeCompositionOrder.clear(); _compositionOrder.clear(); @@ -722,43 +681,6 @@ class HtmlViewEmbedder { } } -/// A group of views that will be composited together within the same overlay. -/// -/// Each OverlayGroup is a sublist of the composition order which can share the -/// same overlay. -/// -/// Every overlay group is a list containing a visible view preceded or followed -/// by zero or more invisible views. -class OverlayGroup { - OverlayGroup() : _group = []; - - // The internal list of ints. - final List _group; - - /// The number of visible views in this group. - int _visibleCount = 0; - - /// Add a [view] (maybe [visible]) to this group. - void add(int view, {bool visible = false}) { - _group.add(view); - if (visible) { - _visibleCount++; - } - } - - /// Get the "last" view added to this group. - int get last => _group.last; - - /// Returns true if this group contains any visible view. - bool get hasVisibleView => _visibleCount > 0; - - /// Returns the number of visible views in this overlay group. - int get visibleCount => _visibleCount; - - /// Returns [true] if this overlay group is empty. - bool get isEmpty => _group.isEmpty; -} - /// Represents a Clip Chain (for a view). /// /// Objects of this class contain: From 034a218532852825e04591a513dc39fbb4d83168 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 29 Feb 2024 10:40:47 -0800 Subject: [PATCH 20/28] fix case where rendering is the same but pictures are different --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 9 ++++++--- 1 file changed, 6 insertions(+), 3 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 a31778d8688f3..bc0270b658dbc 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -361,9 +361,12 @@ class HtmlViewEmbedder { rendering = _modifyRenderingForMaxCanvases(rendering); _updateDomForNewRendering(rendering); if (rendering.equalsForRendering(_activeRendering)) { - // Just set the rendering to be the active rendering so the display - // canvases carry over as well. - rendering = _activeRendering; + // Copy the display canvases to the new rendering. + for (int i = 0; i < rendering.canvases.length; i++) { + rendering.canvases[i].displayCanvas = + _activeRendering.canvases[i].displayCanvas; + _activeRendering.canvases[i].displayCanvas = null; + } } _activeRendering = rendering; From 4fa72f0f8ec9ca4cfc801193c61c50d6d38bec0e Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 29 Feb 2024 16:36:46 -0800 Subject: [PATCH 21/28] Add test for optimization. Don't always clear with transparent black --- .../src/engine/canvaskit/embedded_views.dart | 3 +- .../lib/src/engine/canvaskit/rasterizer.dart | 1 - .../test/canvaskit/embedded_views_test.dart | 69 ++++++++++++++----- 3 files changed, 54 insertions(+), 19 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 bc0270b658dbc..e459b953e73f1 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -5,7 +5,8 @@ import 'dart:math' as math; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' show PlatformViewManager, longestIncreasingSubsequence; +import '../../engine.dart' + show PlatformViewManager, longestIncreasingSubsequence; import '../display.dart'; import '../dom.dart'; import '../html/path_to_svg_clip.dart'; diff --git a/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart b/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart index 703bae3a3ccbb..92dd83e89e1de 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart @@ -55,7 +55,6 @@ abstract class ViewRasterizer { viewEmbedder.frameSize = currentFrameSize; final CkPictureRecorder pictureRecorder = CkPictureRecorder(); pictureRecorder.beginRecording(ui.Offset.zero & currentFrameSize); - pictureRecorder.recordingCanvas!.clear(const ui.Color(0x00000000)); final Frame compositorFrame = context.acquireFrame(pictureRecorder.recordingCanvas!, viewEmbedder); diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index c95787e744775..0af14433f975a 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -350,7 +350,7 @@ void testMain() { // Render: zero platform views. // Expect: main canvas, no overlays. await renderTestScene(viewCount: 0); - _expectSceneMatches(<_EmbeddedViewMarker>[_overlay]); + _expectSceneMatches(<_EmbeddedViewMarker>[]); // Frame 3: // Render: less than cache size platform views. @@ -406,7 +406,7 @@ void testMain() { // Render: zero platform views. // Expect: main canvas, no overlays. await renderTestScene(viewCount: 0); - _expectSceneMatches(<_EmbeddedViewMarker>[_overlay]); + _expectSceneMatches(<_EmbeddedViewMarker>[]); // Frame 6: // Render: deleted platform views. @@ -440,7 +440,7 @@ void testMain() { // Expect: success. Just checking the system is not left in a corrupted state. await createPlatformView(0, 'test-platform-view'); await renderTestScene(viewCount: 0); - _expectSceneMatches(<_EmbeddedViewMarker>[_overlay]); + _expectSceneMatches(<_EmbeddedViewMarker>[]); for (int i = 0; i < 16; i++) { await disposePlatformView(i); @@ -591,7 +591,6 @@ void testMain() { sb.addPlatformView(0, width: 10, height: 10); await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, _platformView, ]); @@ -603,9 +602,7 @@ void testMain() { sb.pushOffset(0, 0); await renderScene(sb.build()); - _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, - ]); + _expectSceneMatches(<_EmbeddedViewMarker>[]); expect(platformViewsHost.querySelector('flt-platform-view'), isNull); }); @@ -658,7 +655,7 @@ void testMain() { // ImageDecoder is not supported in Safari or Firefox. }, skip: isSafari || isFirefox); - test('removed the DOM node of an unrendered platform view', () async { + test('removes the DOM node of an unrendered platform view', () async { ui_web.platformViewRegistry.registerViewFactory( 'test-platform-view', (int viewId) => createDomHTMLDivElement()..id = 'view-0', @@ -670,7 +667,6 @@ void testMain() { sb.addPlatformView(0, width: 10, height: 10); await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, _platformView, ]); @@ -683,7 +679,6 @@ void testMain() { sb.addPlatformView(1, width: 10, height: 10); await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, _platformView, ]); @@ -697,9 +692,7 @@ void testMain() { sb = LayerSceneBuilder(); sb.pushOffset(0, 0); await renderScene(sb.build()); - _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, - ]); + _expectSceneMatches(<_EmbeddedViewMarker>[]); // The actual contents of the platform view are kept in the dom, until // it's actually disposed of! @@ -761,9 +754,7 @@ void testMain() { sb.pop(); // The below line should not throw an error. await renderScene(sb.build()); - _expectSceneMatches(<_EmbeddedViewMarker>[ - _overlay, - ]); + _expectSceneMatches(<_EmbeddedViewMarker>[]); await disposePlatformView(0); }); @@ -1040,7 +1031,6 @@ void testMain() { _platformView, _platformView, _platformView, - _overlay, ]); expect(() { @@ -1051,6 +1041,51 @@ void testMain() { embedder.dispose(); }, returnsNormally); }); + + test('optimizes out overlays when pictures and platform views do not overlap', + () async { + ui_web.platformViewRegistry.registerViewFactory( + 'test-view', + (int viewId) => createDomHTMLDivElement()..className = 'platform-view', + ); + + CkPicture rectPicture(ui.Rect rect) { + return paintPicture(rect, (CkCanvas canvas) { + canvas.drawRect( + rect, CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0)); + }); + } + + await createPlatformView(0, 'test-view'); + await createPlatformView(1, 'test-view'); + await createPlatformView(2, 'test-view'); + + // Scene 1: Pictures just overlap with the most recently painted platform + // view. Analogous to third-party images with subtitles overlaid. Should + // only need one overlay at the end of the scene. + final LayerSceneBuilder sb1 = LayerSceneBuilder(); + sb1.pushOffset(0, 0); + sb1.addPlatformView(0, + offset: const ui.Offset(10, 10), width: 50, height: 50); + sb1.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(12, 12, 10, 10))); + sb1.addPlatformView(1, + offset: const ui.Offset(70, 10), width: 50, height: 50); + sb1.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(72, 12, 10, 10))); + sb1.addPlatformView(2, + offset: const ui.Offset(130, 10), width: 50, height: 50); + sb1.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(132, 12, 10, 10))); + final LayerScene scene1 = sb1.build(); + await renderScene(scene1); + _expectSceneMatches(<_EmbeddedViewMarker>[ + _platformView, + _platformView, + _platformView, + _overlay, + ]); + }); } // Used to test that the platform views and overlays are in the correct order in From 65efb64560bd241faca1709d29b23db4c0e05bc5 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 29 Feb 2024 16:38:58 -0800 Subject: [PATCH 22/28] fix formatting --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 3 +-- 1 file changed, 1 insertion(+), 2 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 e459b953e73f1..bc0270b658dbc 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -5,8 +5,7 @@ import 'dart:math' as math; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' - show PlatformViewManager, longestIncreasingSubsequence; +import '../../engine.dart' show PlatformViewManager, longestIncreasingSubsequence; import '../display.dart'; import '../dom.dart'; import '../html/path_to_svg_clip.dart'; From b7c975b4cacf86e0d6051023ad2aca9185688d27 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Tue, 5 Mar 2024 14:20:34 -0800 Subject: [PATCH 23/28] add and fix tests --- .../src/engine/canvaskit/embedded_views.dart | 3 +- .../canvaskit/overlay_scene_optimizer.dart | 4 + .../test/canvaskit/embedded_views_test.dart | 217 +++++++++--------- 3 files changed, 116 insertions(+), 108 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 bc0270b658dbc..e459b953e73f1 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -5,7 +5,8 @@ import 'dart:math' as math; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' show PlatformViewManager, longestIncreasingSubsequence; +import '../../engine.dart' + show PlatformViewManager, longestIncreasingSubsequence; import '../display.dart'; import '../dom.dart'; import '../html/path_to_svg_clip.dart'; diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index 11838f7a8a9a7..b7cb7a2b99da2 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -40,8 +40,12 @@ class Rendering { return true; } + /// A list of just the canvases in the rendering. List get canvases => entities.whereType().toList(); + + @override + String toString() => entities.toString(); } /// An element of a [Rendering]. Either a render canvas or a platform view. diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index 0af14433f975a..a93c2d5036fb9 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -66,8 +66,6 @@ void testMain() { reason: 'The slot reenables pointer events.'); expect(contentsHost.getAttribute('slot'), slot.getAttribute('name'), reason: 'The contents and slot are correctly related.'); - - await disposePlatformView(0); }); test('clips platform views with RRects', () async { @@ -108,8 +106,6 @@ void testMain() { sceneHost.querySelectorAll('flt-clip').single.style.height, '100%', ); - - await disposePlatformView(0); }); test('correctly transforms platform views', () async { @@ -140,8 +136,6 @@ void testMain() { // 503 (5 * 100 + 3). 'matrix3d(5, 0, 0, 0, 0, 5, 0, 0, 0, 0, 5, 0, 515, 515, 0, 1)', ); - - await disposePlatformView(0); }); test('correctly offsets platform views', () async { @@ -168,8 +162,6 @@ void testMain() { expect(slotRect.top, 4); expect(slotRect.right, 8); expect(slotRect.bottom, 10); - - await disposePlatformView(0); }); // Returns the list of CSS transforms applied to the ancestor chain of @@ -232,8 +224,6 @@ void testMain() { 'matrix(1, 0, 0, 1, 3, 3)', ], ); - - await disposePlatformView(0); }); test('converts device pixels to logical pixels (no clips)', () async { @@ -259,8 +249,6 @@ void testMain() { getTransformChain(slotHost), ['matrix(0.25, 0, 0, 0.25, 1.5, 1.5)'], ); - - await disposePlatformView(0); }); test('converts device pixels to logical pixels (with clips)', () async { @@ -292,8 +280,6 @@ void testMain() { 'matrix(0.25, 0, 0, 0.25, 0.75, 0.75)', ], ); - - await disposePlatformView(0); }); test('renders overlays on top of platform views', () async { @@ -441,10 +427,6 @@ void testMain() { await createPlatformView(0, 'test-platform-view'); await renderTestScene(viewCount: 0); _expectSceneMatches(<_EmbeddedViewMarker>[]); - - for (int i = 0; i < 16; i++) { - await disposePlatformView(i); - } }); test('correctly reuses overlays', () async { @@ -573,10 +555,6 @@ void testMain() { _overlay, _platformView, ]); - - for (int i = 0; i < 20; i++) { - await disposePlatformView(i); - } }); test('embeds and disposes of a platform view', () async { @@ -651,7 +629,6 @@ void testMain() { implicitView.debugPhysicalSizeOverride = null; implicitView.debugForceResize(); - await disposePlatformView(0); // ImageDecoder is not supported in Safari or Firefox. }, skip: isSafari || isFirefox); @@ -700,9 +677,6 @@ void testMain() { platformViewsHost.querySelectorAll('flt-platform-view'), hasLength(2), ); - - await disposePlatformView(0); - await disposePlatformView(1); }); test( @@ -735,8 +709,6 @@ void testMain() { await renderTestScene(); expect(skPathDefs.childNodes, hasLength(1)); - - await disposePlatformView(0); }); test('does not crash when a prerolled platform view is not composited', @@ -755,8 +727,6 @@ void testMain() { // The below line should not throw an error. await renderScene(sb.build()); _expectSceneMatches(<_EmbeddedViewMarker>[]); - - await disposePlatformView(0); }); test('does not create overlays for invisible platform views', () async { @@ -1003,88 +973,121 @@ void testMain() { _platformView, _overlay, ]); - for (int i = 0; i < 7; i++) { - await disposePlatformView(i); - } }); - }); - test('can dispose without crashing', () async { - ui_web.platformViewRegistry.registerViewFactory('test-view', + test('can dispose without crashing', () async { + ui_web.platformViewRegistry.registerViewFactory( + 'test-view', + (int viewId) => + createDomHTMLDivElement()..className = 'platform-view', + isVisible: false); + + await createPlatformView(0, 'test-view'); + await createPlatformView(1, 'test-view'); + await createPlatformView(2, 'test-view'); + + final LayerSceneBuilder sb = LayerSceneBuilder() + ..pushOffset(0, 0) + ..addPlatformView(0, width: 10, height: 10) + ..addPlatformView(1, width: 10, height: 10) + ..addPlatformView(2, width: 10, height: 10) + ..pop(); + + await renderScene(sb.build()); + + _expectSceneMatches(<_EmbeddedViewMarker>[ + _platformView, + _platformView, + _platformView, + ]); + + expect(() { + final HtmlViewEmbedder embedder = (renderer as CanvasKitRenderer) + .debugGetRasterizerForView(implicitView)! + .viewEmbedder; + // The following line used to cause a "Concurrent modification during iteration" + embedder.dispose(); + }, returnsNormally); + }); + + test( + 'optimizes out overlays when pictures and platform views do not overlap', + () async { + ui_web.platformViewRegistry.registerViewFactory( + 'test-view', (int viewId) => createDomHTMLDivElement()..className = 'platform-view', - isVisible: false); - - await createPlatformView(0, 'test-view'); - await createPlatformView(1, 'test-view'); - await createPlatformView(2, 'test-view'); - - final LayerSceneBuilder sb = LayerSceneBuilder() - ..pushOffset(0, 0) - ..addPlatformView(0, width: 10, height: 10) - ..addPlatformView(1, width: 10, height: 10) - ..addPlatformView(2, width: 10, height: 10) - ..pop(); - - await renderScene(sb.build()); - - _expectSceneMatches(<_EmbeddedViewMarker>[ - _platformView, - _platformView, - _platformView, - ]); - - expect(() { - final HtmlViewEmbedder embedder = (renderer as CanvasKitRenderer) - .debugGetRasterizerForView(implicitView)! - .viewEmbedder; - // The following line used to cause a "Concurrent modification during iteration" - embedder.dispose(); - }, returnsNormally); - }); + ); - test('optimizes out overlays when pictures and platform views do not overlap', - () async { - ui_web.platformViewRegistry.registerViewFactory( - 'test-view', - (int viewId) => createDomHTMLDivElement()..className = 'platform-view', - ); - - CkPicture rectPicture(ui.Rect rect) { - return paintPicture(rect, (CkCanvas canvas) { - canvas.drawRect( - rect, CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0)); - }); - } + CkPicture rectPicture(ui.Rect rect) { + return paintPicture(rect, (CkCanvas canvas) { + canvas.drawRect( + rect, CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0)); + }); + } + + await createPlatformView(0, 'test-view'); + await createPlatformView(1, 'test-view'); + await createPlatformView(2, 'test-view'); + + expect(PlatformViewManager.instance.isVisible(0), isTrue); + expect(PlatformViewManager.instance.isVisible(1), isTrue); + expect(PlatformViewManager.instance.isVisible(2), isTrue); + + // Scene 1: Pictures just overlap with the most recently painted platform + // view. Analogous to third-party images with subtitles overlaid. Should + // only need one overlay at the end of the scene. + final LayerSceneBuilder sb1 = LayerSceneBuilder(); + sb1.pushOffset(0, 0); + sb1.addPlatformView(0, + offset: const ui.Offset(10, 10), width: 50, height: 50); + sb1.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(12, 12, 10, 10))); + sb1.addPlatformView(1, + offset: const ui.Offset(70, 10), width: 50, height: 50); + sb1.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(72, 12, 10, 10))); + sb1.addPlatformView(2, + offset: const ui.Offset(130, 10), width: 50, height: 50); + sb1.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(132, 12, 10, 10))); + final LayerScene scene1 = sb1.build(); + await renderScene(scene1); + _expectSceneMatches(<_EmbeddedViewMarker>[ + _platformView, + _platformView, + _platformView, + _overlay, + ]); - await createPlatformView(0, 'test-view'); - await createPlatformView(1, 'test-view'); - await createPlatformView(2, 'test-view'); - - // Scene 1: Pictures just overlap with the most recently painted platform - // view. Analogous to third-party images with subtitles overlaid. Should - // only need one overlay at the end of the scene. - final LayerSceneBuilder sb1 = LayerSceneBuilder(); - sb1.pushOffset(0, 0); - sb1.addPlatformView(0, - offset: const ui.Offset(10, 10), width: 50, height: 50); - sb1.addPicture( - ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(12, 12, 10, 10))); - sb1.addPlatformView(1, - offset: const ui.Offset(70, 10), width: 50, height: 50); - sb1.addPicture( - ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(72, 12, 10, 10))); - sb1.addPlatformView(2, - offset: const ui.Offset(130, 10), width: 50, height: 50); - sb1.addPicture( - ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(132, 12, 10, 10))); - final LayerScene scene1 = sb1.build(); - await renderScene(scene1); - _expectSceneMatches(<_EmbeddedViewMarker>[ - _platformView, - _platformView, - _platformView, - _overlay, - ]); + // Scene 2: Same as scene 1 but with a background painted first. Should only + // need a canvas for the background and one more for the rest of the + // pictures. + final LayerSceneBuilder sb2 = LayerSceneBuilder(); + sb2.pushOffset(0, 0); + sb2.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(0, 0, 300, 300))); + sb2.addPlatformView(0, + offset: const ui.Offset(10, 10), width: 50, height: 50); + sb2.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(12, 12, 10, 10))); + sb2.addPlatformView(1, + offset: const ui.Offset(70, 10), width: 50, height: 50); + sb2.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(72, 12, 10, 10))); + sb2.addPlatformView(2, + offset: const ui.Offset(130, 10), width: 50, height: 50); + sb2.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(132, 12, 10, 10))); + final LayerScene scene2 = sb2.build(); + await renderScene(scene2); + _expectSceneMatches(<_EmbeddedViewMarker>[ + _overlay, + _platformView, + _platformView, + _platformView, + _overlay, + ]); + }); }); } From 1b4ec4f154849951d976e42b7268e9049bb882ed Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Tue, 5 Mar 2024 16:33:20 -0800 Subject: [PATCH 24/28] Add test --- .../src/engine/canvaskit/embedded_views.dart | 3 +- .../test/canvaskit/embedded_views_test.dart | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 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 e459b953e73f1..bc0270b658dbc 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -5,8 +5,7 @@ import 'dart:math' as math; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' - show PlatformViewManager, longestIncreasingSubsequence; +import '../../engine.dart' show PlatformViewManager, longestIncreasingSubsequence; import '../display.dart'; import '../dom.dart'; import '../html/path_to_svg_clip.dart'; diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index a93c2d5036fb9..0f8d449f53880 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -1087,6 +1087,37 @@ void testMain() { _platformView, _overlay, ]); + + // Scene 3: Paints a full-screen picture between each platform view. This + // is the worst case scenario. There should be an overlay between each + // platform view. + final LayerSceneBuilder sb3 = LayerSceneBuilder(); + sb3.pushOffset(0, 0); + sb3.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(0, 0, 300, 300))); + sb3.addPlatformView(0, + offset: const ui.Offset(10, 10), width: 50, height: 50); + sb3.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(0, 0, 300, 300))); + sb3.addPlatformView(1, + offset: const ui.Offset(70, 10), width: 50, height: 50); + sb3.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(0, 0, 300, 300))); + sb3.addPlatformView(2, + offset: const ui.Offset(130, 10), width: 50, height: 50); + sb3.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(0, 0, 300, 300))); + final LayerScene scene3 = sb3.build(); + await renderScene(scene3); + _expectSceneMatches(<_EmbeddedViewMarker>[ + _overlay, + _platformView, + _overlay, + _platformView, + _overlay, + _platformView, + _overlay, + ]); }); }); } From aa7947556cfd7b237382a5a8d50e001efb8c2876 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 7 Mar 2024 13:03:55 -0800 Subject: [PATCH 25/28] Fix bounds calculation --- .../src/engine/canvaskit/embedded_views.dart | 28 ++++++++++++++++ .../canvaskit/overlay_scene_optimizer.dart | 32 +++++++++--------- lib/web_ui/lib/src/engine/util.dart | 33 ++++++++++++++----- 3 files changed, 70 insertions(+), 23 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 bc0270b658dbc..922eb4e98aea3 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -15,6 +15,7 @@ import '../util.dart'; import '../vector_math.dart'; import 'canvas.dart'; import 'overlay_scene_optimizer.dart'; +import 'painting.dart'; import 'path.dart'; import 'picture.dart'; import 'picture_recorder.dart'; @@ -382,6 +383,30 @@ class HtmlViewEmbedder { recorder.endRecording(); } } + + debugBoundsCanvas ??= rasterizer.displayFactory.getCanvas(); + CkPictureRecorder boundsRecorder = CkPictureRecorder(); + CkCanvas boundsCanvas = boundsRecorder.beginRecording( + ui.Rect.fromLTWH(0, 0, _frameSize.width, _frameSize.height)); + final CkPaint platformViewBoundsPaint = CkPaint() + ..color = const ui.Color.fromARGB(100, 0, 255, 0); + final CkPaint pictureBoundsPaint = CkPaint() + ..color = const ui.Color.fromARGB(100, 0, 0, 255); + for (final RenderingEntity entity in _activeRendering.entities) { + if (entity is RenderingPlatformView) { + if (entity.debugComputedBounds != null) { + boundsCanvas.drawRect( + entity.debugComputedBounds!, platformViewBoundsPaint); + } + } else if (entity is RenderingRenderCanvas) { + for (CkPicture picture in entity.pictures) { + boundsCanvas.drawRect(picture.cullRect, pictureBoundsPaint); + } + } + } + await rasterizer.rasterizeToCanvas( + debugBoundsCanvas!, [boundsRecorder.endRecording()]); + sceneHost.append(debugBoundsCanvas!.hostElement); // Reset the context. _context = EmbedderFrameContext(); if (listEquals(_compositionOrder, _activeCompositionOrder)) { @@ -858,6 +883,9 @@ class MutatorsStack extends Iterable { @override Iterator get iterator => _mutators.reversed.iterator; + + /// Iterate over the mutators in reverse. + Iterable get reversed => _mutators; } /// The state for the current frame. diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index b7cb7a2b99da2..d4549abe27602 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -6,8 +6,7 @@ import 'package:meta/meta.dart'; import 'package:ui/src/engine/util.dart'; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' show PlatformViewManager; -import '../display.dart'; +import '../../engine.dart' show PlatformViewManager, kDebugMode; import '../vector_math.dart'; import 'embedded_views.dart'; import 'picture.dart'; @@ -102,6 +101,10 @@ class RenderingPlatformView extends RenderingEntity { String toString() { return '$RenderingPlatformView($viewId)'; } + + /// The bounds that were computed for this platform view when creating the + /// optimized rendering. This is only set in debug mode. + ui.Rect? debugComputedBounds; } // Computes the bounds of the platform view from its associated parameters. @@ -109,14 +112,8 @@ class RenderingPlatformView extends RenderingEntity { ui.Rect computePlatformViewBounds(EmbeddedViewParams params) { ui.Rect currentClipBounds = ui.Rect.largest; - // The transforms are in logical pixels, but we want to compute physical - // pixel bounds, so transform by the device pixel ratio. - final double scale = EngineFlutterDisplay.instance.devicePixelRatio; - Matrix4 currentTransform = Matrix4.diagonal3Values(scale, scale, 1); - currentTransform = currentTransform.multiplied(params.offset == ui.Offset.zero - ? Matrix4.identity() - : Matrix4.translationValues(params.offset.dx, params.offset.dy, 0)); - for (final Mutator mutator in params.mutators) { + Matrix4 currentTransform = Matrix4.identity(); + for (final Mutator mutator in params.mutators.reversed) { switch (mutator.type) { case MutatorType.clipRect: final ui.Rect transformedClipBounds = @@ -141,10 +138,10 @@ ui.Rect computePlatformViewBounds(EmbeddedViewParams params) { // The width and height are in physical pixels already, so apply the inverse // scale since the transform already applied the scaling. final ui.Rect rawBounds = ui.Rect.fromLTWH( - 0, - 0, - params.size.width / scale, - params.size.height / scale, + params.offset.dx, + params.offset.dy, + params.size.width, + params.size.height, ); final ui.Rect transformedBounds = transformRectWithMatrix(currentTransform, rawBounds); @@ -177,9 +174,14 @@ Rendering createOptimizedRendering( currentRenderCanvas.add(pictures[0]); } for (int i = 0; i < platformViews.length; i++) { + final RenderingPlatformView platformView = + RenderingPlatformView(platformViews[i]); if (PlatformViewManager.instance.isVisible(platformViews[i])) { final ui.Rect platformViewBounds = computePlatformViewBounds(paramsForViews[platformViews[i]]!); + if (kDebugMode) { + platformView.debugComputedBounds = platformViewBounds; + } bool intersectsWithCurrentPictures = false; for (final CkPicture picture in currentRenderCanvas.pictures) { if (picture.cullRect.overlaps(platformViewBounds)) { @@ -192,7 +194,7 @@ Rendering createOptimizedRendering( currentRenderCanvas = RenderingRenderCanvas(); } } - result.add(RenderingPlatformView(platformViews[i])); + result.add(platformView); if (!pictures[i + 1].cullRect.isEmpty) { currentRenderCanvas.add(pictures[i + 1]); } diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 5f54070c155f8..1889e41274763 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. - import 'dart:async'; import 'dart:collection'; import 'dart:math' as math; @@ -527,6 +526,20 @@ int clampInt(int value, int min, int max) { /// to verify that warnings are printed under certain circumstances. void Function(String) printWarning = domWindow.console.warn; +/// Converts a 4x4 matrix into a human-readable String. +String matrixString(List matrix) { + final StringBuffer sb = StringBuffer(); + for (int i = 0; i < 16; i++) { + sb.write(matrix[i]); + if ((i + 1) % 4 == 0) { + sb.write('\n'); + } else { + sb.write(' '); + } + } + return sb.toString(); +} + /// Determines if lists [a] and [b] are deep equivalent. /// /// Returns true if the lists are both null, or if they are both non-null, have @@ -659,15 +672,16 @@ int? tryViewId(Object? arguments) { /// Input: [0, 1, 2, 3] /// Output: 0x00 0x01 0x02 0x03 String bytesToHexString(List data) { - return data.map((int byte) => '0x${byte.toRadixString(16).padLeft(2, '0')}').join(' '); + return data + .map((int byte) => '0x${byte.toRadixString(16).padLeft(2, '0')}') + .join(' '); } /// Sets a style property on [element]. /// /// [name] is the name of the property. [value] is the value of the property. /// If [value] is null, removes the style property. -void setElementStyle( - DomElement element, String name, String? value) { +void setElementStyle(DomElement element, String name, String? value) { if (value == null) { element.style.removeProperty(name); } else { @@ -720,7 +734,8 @@ void drawEllipse( double startAngle, double endAngle, bool antiClockwise) { - _ellipseFeatureDetected ??= getJsProperty(context, 'ellipse') != null; + _ellipseFeatureDetected ??= + getJsProperty(context, 'ellipse') != null; if (_ellipseFeatureDetected!) { context.ellipse(centerX, centerY, radiusX, radiusY, rotation, startAngle, endAngle, antiClockwise); @@ -772,7 +787,8 @@ class LruCache { /// A doubly linked list of the objects in the cache. /// /// This makes it fast to move a recently used object to the front. - final DoubleLinkedQueue<_LruCacheEntry> _itemQueue = DoubleLinkedQueue<_LruCacheEntry>(); + final DoubleLinkedQueue<_LruCacheEntry> _itemQueue = + DoubleLinkedQueue<_LruCacheEntry>(); @visibleForTesting DoubleLinkedQueue<_LruCacheEntry> get debugItemQueue => _itemQueue; @@ -781,7 +797,8 @@ class LruCache { /// /// This makes it fast to find the node in the queue when we need to /// move the object to the front of the queue. - final Map>> _itemMap = >>{}; + final Map>> _itemMap = + >>{}; @visibleForTesting Map>> get itemMap => _itemMap; @@ -805,7 +822,7 @@ class LruCache { /// Returns the cached value associated with the [key]. /// /// If the value is not in the cache, returns null. - V? operator[](K key) { + V? operator [](K key) { return _itemMap[key]?.element.value; } From be7feeb652f44618c91971bb2881606c12cffa8e Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 7 Mar 2024 16:49:18 -0800 Subject: [PATCH 26/28] Add more debugging tools and tests --- .../src/engine/canvaskit/embedded_views.dart | 48 +++++++++++-------- .../canvaskit/overlay_scene_optimizer.dart | 8 +++- .../test/canvaskit/embedded_views_test.dart | 38 ++++++++++++++- 3 files changed, 69 insertions(+), 25 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 922eb4e98aea3..315d07e976ee1 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -5,7 +5,8 @@ import 'dart:math' as math; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' show PlatformViewManager, longestIncreasingSubsequence; +import '../../engine.dart' + show PlatformViewManager, longestIncreasingSubsequence; import '../display.dart'; import '../dom.dart'; import '../html/path_to_svg_clip.dart'; @@ -384,29 +385,34 @@ class HtmlViewEmbedder { } } - debugBoundsCanvas ??= rasterizer.displayFactory.getCanvas(); - CkPictureRecorder boundsRecorder = CkPictureRecorder(); - CkCanvas boundsCanvas = boundsRecorder.beginRecording( - ui.Rect.fromLTWH(0, 0, _frameSize.width, _frameSize.height)); - final CkPaint platformViewBoundsPaint = CkPaint() - ..color = const ui.Color.fromARGB(100, 0, 255, 0); - final CkPaint pictureBoundsPaint = CkPaint() - ..color = const ui.Color.fromARGB(100, 0, 0, 255); - for (final RenderingEntity entity in _activeRendering.entities) { - if (entity is RenderingPlatformView) { - if (entity.debugComputedBounds != null) { - boundsCanvas.drawRect( - entity.debugComputedBounds!, platformViewBoundsPaint); - } - } else if (entity is RenderingRenderCanvas) { - for (CkPicture picture in entity.pictures) { - boundsCanvas.drawRect(picture.cullRect, pictureBoundsPaint); + // Draw the computed bounds for pictures and platform views if overlay + // optimization debugging is enabled. + if (debugOverlayOptimizationBounds) { + debugBoundsCanvas ??= rasterizer.displayFactory.getCanvas(); + final CkPictureRecorder boundsRecorder = CkPictureRecorder(); + final CkCanvas boundsCanvas = boundsRecorder.beginRecording( + ui.Rect.fromLTWH(0, 0, _frameSize.width, _frameSize.height)); + final CkPaint platformViewBoundsPaint = CkPaint() + ..color = const ui.Color.fromARGB(100, 0, 255, 0); + final CkPaint pictureBoundsPaint = CkPaint() + ..color = const ui.Color.fromARGB(100, 0, 0, 255); + for (final RenderingEntity entity in _activeRendering.entities) { + if (entity is RenderingPlatformView) { + if (entity.debugComputedBounds != null) { + boundsCanvas.drawRect( + entity.debugComputedBounds!, platformViewBoundsPaint); + } + } else if (entity is RenderingRenderCanvas) { + for (CkPicture picture in entity.pictures) { + boundsCanvas.drawRect(picture.cullRect, pictureBoundsPaint); + } } } + await rasterizer.rasterizeToCanvas( + debugBoundsCanvas!, [boundsRecorder.endRecording()]); + sceneHost.append(debugBoundsCanvas!.hostElement); } - await rasterizer.rasterizeToCanvas( - debugBoundsCanvas!, [boundsRecorder.endRecording()]); - sceneHost.append(debugBoundsCanvas!.hostElement); + // Reset the context. _context = EmbedderFrameContext(); if (listEquals(_compositionOrder, _activeCompositionOrder)) { diff --git a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart index d4549abe27602..15c010c34cd9e 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/overlay_scene_optimizer.dart @@ -6,12 +6,16 @@ import 'package:meta/meta.dart'; import 'package:ui/src/engine/util.dart'; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' show PlatformViewManager, kDebugMode; +import '../../engine.dart' show PlatformViewManager; import '../vector_math.dart'; import 'embedded_views.dart'; import 'picture.dart'; import 'rasterizer.dart'; +/// If `true`, draws the computed bounds for platform views and pictures to +/// help debug issues with the overlay optimization. +bool debugOverlayOptimizationBounds = false; + /// A [Rendering] is a concrete description of how a Flutter scene will be /// rendered in a web browser. /// @@ -179,7 +183,7 @@ Rendering createOptimizedRendering( if (PlatformViewManager.instance.isVisible(platformViews[i])) { final ui.Rect platformViewBounds = computePlatformViewBounds(paramsForViews[platformViews[i]]!); - if (kDebugMode) { + if (debugOverlayOptimizationBounds) { platformView.debugComputedBounds = platformViewBounds; } bool intersectsWithCurrentPictures = false; diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index 0f8d449f53880..1b0d76249d393 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -1010,8 +1010,7 @@ void testMain() { }, returnsNormally); }); - test( - 'optimizes out overlays when pictures and platform views do not overlap', + test('optimizes overlays when pictures and platform views do not overlap', () async { ui_web.platformViewRegistry.registerViewFactory( 'test-view', @@ -1119,6 +1118,40 @@ void testMain() { _overlay, ]); }); + + test('optimized overlays correctly with transforms and clips', () async { + ui_web.platformViewRegistry.registerViewFactory( + 'test-view', + (int viewId) => createDomHTMLDivElement()..className = 'platform-view', + ); + + CkPicture rectPicture(ui.Rect rect) { + return paintPicture(rect, (CkCanvas canvas) { + canvas.drawRect( + rect, CkPaint()..color = const ui.Color.fromARGB(255, 255, 0, 0)); + }); + } + + await createPlatformView(0, 'test-view'); + + expect(PlatformViewManager.instance.isVisible(0), isTrue); + + // Test optimization correctly computes bounds with transforms and clips. + final LayerSceneBuilder sb = LayerSceneBuilder(); + sb.pushOffset(0, 0); + final Matrix4 scaleMatrix = Matrix4.identity()..scale(3, 3, 1); + sb.pushTransform(scaleMatrix.toFloat64()); + sb.pushClipRect(const ui.Rect.fromLTWH(10, 10, 10, 10)); + sb.addPicture( + ui.Offset.zero, rectPicture(const ui.Rect.fromLTWH(0, 0, 20, 20))); + sb.addPlatformView(0, width: 20, height: 20); + final LayerScene scene = sb.build(); + await renderScene(scene); + _expectSceneMatches(<_EmbeddedViewMarker>[ + _overlay, + _platformView, + ]); + }); }); } @@ -1136,6 +1169,7 @@ const Map _tagToViewMarker = { 'flt-canvas-container': _EmbeddedViewMarker.overlay, 'flt-platform-view-slot': _EmbeddedViewMarker.platformView, + 'flt-clip': _EmbeddedViewMarker.platformView, }; void _expectSceneMatches( From cf4c6a6f0038bda59ad1b858e83b8ff4aabf5028 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 7 Mar 2024 16:57:11 -0800 Subject: [PATCH 27/28] Fix formatting --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 3 +-- 1 file changed, 1 insertion(+), 2 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 315d07e976ee1..b9ab7a0193498 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -5,8 +5,7 @@ import 'dart:math' as math; import 'package:ui/ui.dart' as ui; -import '../../engine.dart' - show PlatformViewManager, longestIncreasingSubsequence; +import '../../engine.dart' show PlatformViewManager, longestIncreasingSubsequence; import '../display.dart'; import '../dom.dart'; import '../html/path_to_svg_clip.dart'; From 65164efd665397fea8f38853b5ee99098a083db9 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Fri, 8 Mar 2024 13:25:29 -0800 Subject: [PATCH 28/28] fix analysis error --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b9ab7a0193498..846107d4ad038 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -402,7 +402,7 @@ class HtmlViewEmbedder { entity.debugComputedBounds!, platformViewBoundsPaint); } } else if (entity is RenderingRenderCanvas) { - for (CkPicture picture in entity.pictures) { + for (final CkPicture picture in entity.pictures) { boundsCanvas.drawRect(picture.cullRect, pictureBoundsPaint); } }