Skip to content

Commit

Permalink
[web] Move scene DOM management to DomManager (flutter#47460)
Browse files Browse the repository at this point in the history
- Move scene insertion logic to `DomManager`.
- Add TODOs in `Renderer` subclasses (cc @harryterkelsen).

Part of flutter/flutter#134443
  • Loading branch information
mdebbar authored Nov 17, 2023
1 parent eb29ab0 commit 24efcdd
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 66 deletions.
4 changes: 3 additions & 1 deletion lib/web_ui/lib/src/engine/canvaskit/renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ class CanvasKitRenderer implements Renderer {
// added eagerly during initialization here and never touched, unless the
// system is reset due to hot restart or in a test.
_sceneHost = createDomElement('flt-scene');
embedder.addSceneToSceneHost(_sceneHost);
// TODO(harryterkelsen): Do this operation on the appropriate Flutter View.
final EngineFlutterView implicitView = EnginePlatformDispatcher.instance.implicitView!;
implicitView.dom.setScene(_sceneHost!);
}

@override
Expand Down
18 changes: 0 additions & 18 deletions lib/web_ui/lib/src/engine/embedder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,30 +39,12 @@ class FlutterViewEmbedder {
reset();
}

DomElement get _sceneHostElement => window.dom.sceneHost;

/// A child element of body outside the shadowroot that hosts
/// global resources such svg filters and clip paths when using webkit.
DomElement? _resourcesHost;

DomElement get _semanticsHostElement => window.dom.semanticsHost;

/// The last scene element rendered by the [render] method.
DomElement? get sceneElement => _sceneElement;
DomElement? _sceneElement;

/// Don't unnecessarily move DOM nodes around. If a DOM node is
/// already in the right place, skip DOM mutation. This is both faster and
/// more correct, because moving DOM nodes loses internal state, such as
/// text selection.
void addSceneToSceneHost(DomElement? sceneElement) {
if (sceneElement != _sceneElement) {
_sceneElement?.remove();
_sceneElement = sceneElement;
_sceneHostElement.append(sceneElement!);
}
}

DomElement get _flutterViewElement => window.dom.rootElement;
DomShadowRoot get _glassPaneShadow => window.dom.renderingHost;

Expand Down
9 changes: 3 additions & 6 deletions lib/web_ui/lib/src/engine/html/renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ class HtmlRenderer implements Renderer {

late final HtmlFontCollection _fontCollection = HtmlFontCollection();

late FlutterViewEmbedder _viewEmbedder;

@override
HtmlFontCollection get fontCollection => _fontCollection;

Expand All @@ -38,9 +36,7 @@ class HtmlRenderer implements Renderer {
}

@override
void reset(FlutterViewEmbedder embedder) {
_viewEmbedder = embedder;
}
void reset(FlutterViewEmbedder embedder) {}

@override
ui.Paint createPaint() => SurfacePaint();
Expand Down Expand Up @@ -328,7 +324,8 @@ class HtmlRenderer implements Renderer {

@override
void renderScene(ui.Scene scene) {
_viewEmbedder.addSceneToSceneHost((scene as SurfaceScene).webOnlyRootElement);
final EngineFlutterView implicitView = EnginePlatformDispatcher.instance.implicitView!;
implicitView.dom.setScene((scene as SurfaceScene).webOnlyRootElement!);
frameTimingsOnRasterFinish();
}

Expand Down
4 changes: 3 additions & 1 deletion lib/web_ui/lib/src/engine/skwasm/skwasm_impl/renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,9 @@ class SkwasmRenderer implements Renderer {

@override
void reset(FlutterViewEmbedder embedder) {
embedder.addSceneToSceneHost(sceneView.sceneElement);
// TODO(harryterkelsen): Do this operation on the appropriate Flutter View.
final EngineFlutterView implicitView = EnginePlatformDispatcher.instance.implicitView!;
implicitView.dom.setScene(sceneView.sceneElement);
}

static final Map<String, Future<ui.FragmentProgram>> _programs = <String, Future<ui.FragmentProgram>>{};
Expand Down
19 changes: 19 additions & 0 deletions lib/web_ui/lib/src/engine/view_embedder/dom_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,25 @@ class DomManager {

/// This is where accessibility announcements are inserted.
final DomElement announcementsHost;

DomElement? _lastSceneElement;

/// Inserts the [sceneElement] into the DOM and removes the existing scene (if
/// any).
///
/// The [sceneElement] is inserted as a child of the <flt-scene-host> element
/// inside the [renderingHost].
///
/// If the [sceneElement] has already been inserted, this method does nothing
/// to avoid unnecessary DOM mutations. This is both faster and more correct,
/// because moving DOM nodes loses internal state, such as text selection.
void setScene(DomElement sceneElement) {
if (sceneElement != _lastSceneElement) {
_lastSceneElement?.remove();
_lastSceneElement = sceneElement;
sceneHost.append(sceneElement);
}
}
}

DomShadowRoot _attachShadowRoot(DomElement element) {
Expand Down
61 changes: 21 additions & 40 deletions lib/web_ui/test/canvaskit/embedded_views_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ import 'test_data.dart';
EngineFlutterWindow get implicitView =>
EnginePlatformDispatcher.instance.implicitView!;

DomElement get platformViewsHost {
return EnginePlatformDispatcher.instance.implicitView!.dom.platformViewsHost;
}
DomElement get platformViewsHost => implicitView.dom.platformViewsHost;
DomElement get sceneElement => implicitView.dom.sceneHost.querySelector('flt-scene')!;

void main() {
internalBootstrapBrowserTest(() => testMain);
Expand Down Expand Up @@ -49,8 +48,7 @@ void testMain() {
// as a child of the glassPane, and the slot lives in the glassPane
// shadow root. The slot is the one that has pointer events auto.
final DomElement contents = platformViewsHost.querySelector('#view-0')!;
final DomElement slot =
flutterViewEmbedder.sceneElement!.querySelector('slot')!;
final DomElement slot = sceneElement.querySelector('slot')!;
final DomElement contentsHost = contents.parent!;
final DomElement slotHost = slot.parent!;

Expand Down Expand Up @@ -82,41 +80,27 @@ void testMain() {
rasterizer.draw(sb.build().layerTree);

expect(
flutterViewEmbedder.sceneElement!
.querySelectorAll('#sk_path_defs')
.single,
sceneElement.querySelectorAll('#sk_path_defs').single,
isNotNull,
);
expect(
flutterViewEmbedder.sceneElement!
sceneElement
.querySelectorAll('#sk_path_defs')
.single
.querySelectorAll('clipPath')
.single,
isNotNull,
);
expect(
flutterViewEmbedder.sceneElement!
.querySelectorAll('flt-clip')
.single
.style
.clipPath,
sceneElement.querySelectorAll('flt-clip').single.style.clipPath,
'url("#svgClip1")',
);
expect(
flutterViewEmbedder.sceneElement!
.querySelectorAll('flt-clip')
.single
.style
.width,
sceneElement.querySelectorAll('flt-clip').single.style.width,
'100%',
);
expect(
flutterViewEmbedder.sceneElement!
.querySelectorAll('flt-clip')
.single
.style
.height,
sceneElement.querySelectorAll('flt-clip').single.style.height,
'100%',
);
});
Expand All @@ -140,8 +124,8 @@ void testMain() {
rasterizer.draw(sb.build().layerTree);

// Transformations happen on the slot element.
final DomElement slotHost = flutterViewEmbedder.sceneElement!
.querySelector('flt-platform-view-slot')!;
final DomElement slotHost =
sceneElement.querySelector('flt-platform-view-slot')!;

expect(
slotHost.style.transform,
Expand All @@ -163,8 +147,8 @@ void testMain() {
sb.addPlatformView(0, offset: const ui.Offset(3, 4), width: 5, height: 6);
CanvasKitRenderer.instance.rasterizer.draw(sb.build().layerTree);

final DomElement slotHost = flutterViewEmbedder.sceneElement!
.querySelector('flt-platform-view-slot')!;
final DomElement slotHost =
sceneElement.querySelector('flt-platform-view-slot')!;
final DomCSSStyleDeclaration style = slotHost.style;

expect(style.transform, 'matrix(1, 0, 0, 1, 3, 4)');
Expand Down Expand Up @@ -206,8 +190,8 @@ void testMain() {
CanvasKitRenderer.instance.rasterizer.draw(sb.build().layerTree);

// Transformations happen on the slot element.
DomElement slotHost = flutterViewEmbedder.sceneElement!
.querySelector('flt-platform-view-slot')!;
DomElement slotHost =
sceneElement.querySelector('flt-platform-view-slot')!;

expect(
getTransformChain(slotHost),
Expand All @@ -227,8 +211,7 @@ void testMain() {
CanvasKitRenderer.instance.rasterizer.draw(sb.build().layerTree);

// Transformations happen on the slot element.
slotHost = flutterViewEmbedder.sceneElement!
.querySelector('flt-platform-view-slot')!;
slotHost = sceneElement.querySelector('flt-platform-view-slot')!;

expect(
getTransformChain(slotHost),
Expand Down Expand Up @@ -256,8 +239,8 @@ void testMain() {
CanvasKitRenderer.instance.rasterizer.draw(sb.build().layerTree);

// Transformations happen on the slot element.
final DomElement slotHost = flutterViewEmbedder.sceneElement!
.querySelector('flt-platform-view-slot')!;
final DomElement slotHost =
sceneElement.querySelector('flt-platform-view-slot')!;

expect(
getTransformChain(slotHost),
Expand All @@ -283,8 +266,8 @@ void testMain() {
CanvasKitRenderer.instance.rasterizer.draw(sb.build().layerTree);

// Transformations happen on the slot element.
final DomElement slotHost = flutterViewEmbedder.sceneElement!
.querySelector('flt-platform-view-slot')!;
final DomElement slotHost =
sceneElement.querySelector('flt-platform-view-slot')!;

expect(
getTransformChain(slotHost),
Expand Down Expand Up @@ -736,8 +719,7 @@ void testMain() {
rasterizer.draw(sb.build().layerTree);
}

final DomNode skPathDefs =
flutterViewEmbedder.sceneElement!.querySelector('#sk_path_defs')!;
final DomNode skPathDefs = sceneElement.querySelector('#sk_path_defs')!;

expect(skPathDefs.childNodes, hasLength(0));

Expand Down Expand Up @@ -1000,8 +982,7 @@ void _expectSceneMatches(
String? reason,
}) {
// Convert the scene elements to its corresponding array of _EmbeddedViewMarker
final List<_EmbeddedViewMarker> sceneElements = flutterViewEmbedder
.sceneElement!.children
final List<_EmbeddedViewMarker> sceneElements = sceneElement.children
.where((DomElement element) => element.tagName != 'svg')
.map((DomElement element) =>
_tagToViewMarker[element.tagName.toLowerCase()]!)
Expand Down
21 changes: 21 additions & 0 deletions lib/web_ui/test/engine/view_embedder/dom_manager_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,27 @@ void doTests() {
expect(style!.tagName, equalsIgnoringCase('style'));
expect(style.parentNode, domManager.renderingHost);
});

test('setScene', () {
final DomManager domManager = DomManager(devicePixelRatio: 3.0);

final DomElement sceneHost =
domManager.renderingHost.querySelector('flt-scene-host')!;

final DomElement scene1 = createDomElement('flt-scene');
domManager.setScene(scene1);
expect(sceneHost.children, <DomElement>[scene1]);

// Insert the same scene again.
domManager.setScene(scene1);
expect(sceneHost.children, <DomElement>[scene1]);

// Insert a different scene.
final DomElement scene2 = createDomElement('flt-scene');
domManager.setScene(scene2);
expect(sceneHost.children, <DomElement>[scene2]);
expect(scene1.parent, isNull);
});
});
}

Expand Down

0 comments on commit 24efcdd

Please sign in to comment.