Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Fixes invisible platform view compositing edge cases. #39506

Merged
auto-submit merged 5 commits into from
Feb 10, 2023
Merged

[web] Fixes invisible platform view compositing edge cases. #39506

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ce4648a
Somewhat works... Remove first overlay though.
ditman Feb 8, 2023
f6fecbb
needNewOverlay is equivalent to isVisible in the algorithm.
ditman Feb 9, 2023
5cb315d
Only consider groups with a visible view to need an overlay.
ditman Feb 9, 2023
030ac47
Update tests with new logic. Make test output easier to understand an…
ditman Feb 9, 2023
ac3fe45
Introduce OverlayGroup helper class.
ditman Feb 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 16 additions & 23 deletions lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,7 @@ class HtmlViewEmbedder {
// We need an overlay for the first platform view no matter what. The first
// visible platform view doesn't need to create a new one if we already
// created one.
final bool needNewOverlay = (platformViewManager.isVisible(viewId) &&
Comment thread
ditman marked this conversation as resolved.
_context.seenFirstVisibleViewInPreroll) ||
_context.pictureRecordersCreatedDuringPreroll.isEmpty;
if (platformViewManager.isVisible(viewId)) {
_context.seenFirstVisibleViewInPreroll = true;
}
final bool needNewOverlay = platformViewManager.isVisible(viewId);
if (needNewOverlay && hasAvailableOverlay) {
final CkPictureRecorder pictureRecorder = CkPictureRecorder();
pictureRecorder.beginRecording(ui.Offset.zero & _frameSize);
Expand Down Expand Up @@ -169,17 +164,11 @@ class HtmlViewEmbedder {
CkCanvas? compositeEmbeddedView(int viewId) {
final int overlayIndex = _context.visibleViewCount;
_compositionOrder.add(viewId);
if (platformViewManager.isVisible(viewId) ||
_context.pictureRecorders.isEmpty) {
if (platformViewManager.isVisible(viewId)) {
_context.visibleViewCount++;
}
// We need a new overlay if this is a visible view or if we don't have one yet.
final bool needNewOverlay = (platformViewManager.isVisible(viewId) &&
_context.seenFirstVisibleView) ||
_context.pictureRecorders.isEmpty;
if (platformViewManager.isVisible(viewId)) {
_context.seenFirstVisibleView = true;
}
final bool needNewOverlay = platformViewManager.isVisible(viewId);
CkPictureRecorder? recorderToUseForRendering;
if (needNewOverlay) {
if (overlayIndex < _context.pictureRecordersCreatedDuringPreroll.length) {
Expand Down Expand Up @@ -424,7 +413,11 @@ class HtmlViewEmbedder {
? null
: diffViewList(_activeCompositionOrder, _compositionOrder);
_updateOverlays(diffResult);
assert(_context.pictureRecorders.length == _overlays.length);
assert(
_context.pictureRecorders.length == _overlays.length,
'There should be the same picture recorders (${_context.pictureRecorders.length}) '
'than overlays (${_overlays.length}).',
Comment thread
ditman marked this conversation as resolved.
Outdated
);
int pictureRecorderIndex = 0;

for (int i = 0; i < _compositionOrder.length; i++) {
Expand Down Expand Up @@ -601,8 +594,14 @@ class HtmlViewEmbedder {
return;
}
final List<List<int>> overlayGroups = getOverlayGroups(_compositionOrder);
final List<int> viewsNeedingOverlays =
overlayGroups.map((List<int> group) => group.last).toList();
final List<int> viewsNeedingOverlays = overlayGroups
.where(
(List<int> group) => group.any(
(int viewId) => platformViewManager.isVisible(viewId)
)
)
.map((List<int> group) => group.last)
.toList();
Comment thread
ditman marked this conversation as resolved.
Outdated
// If there were more visible views than overlays, then the last group
// doesn't have an overlay.
if (viewsNeedingOverlays.length > SurfaceFactory.instance.maximumOverlays) {
Expand Down Expand Up @@ -905,12 +904,6 @@ class MutatorsStack extends Iterable<Mutator> {

/// The state for the current frame.
class EmbedderFrameContext {
/// Whether or not we have seen a visible platform view in this frame yet.
bool seenFirstVisibleViewInPreroll = false;

/// Whether or not we have seen a visible platform view in this frame yet.
bool seenFirstVisibleView = false;

/// Picture recorders which were created during the preroll phase.
///
/// These picture recorders will be "claimed" in the paint phase by platform
Expand Down
38 changes: 18 additions & 20 deletions lib/web_ui/test/canvaskit/embedded_views_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -926,8 +926,7 @@ void testMain() {
_expectSceneMatches(<_EmbeddedViewMarker>[
_overlay,
_platformView,
_overlay,
]);
], reason: 'Invisible view alone renders on top of base overlay.');

sb = LayerSceneBuilder();
sb.pushOffset(0, 0);
Expand All @@ -940,7 +939,7 @@ void testMain() {
_platformView,
_platformView,
_overlay,
]);
], reason: 'Overlay created after a group containing a visible view.');

sb = LayerSceneBuilder();
sb.pushOffset(0, 0);
Expand All @@ -956,7 +955,7 @@ void testMain() {
_overlay,
_platformView,
_overlay,
]);
], reason: 'Overlays created after each group containing a visible view.');

sb = LayerSceneBuilder();
sb.pushOffset(0, 0);
Expand All @@ -974,7 +973,7 @@ void testMain() {
_platformView,
_platformView,
_overlay,
]);
], reason: 'Invisible views grouped in with visible views.');

sb = LayerSceneBuilder();
sb.pushOffset(0, 0);
Expand Down Expand Up @@ -1058,8 +1057,7 @@ void testMain() {
_platformView,
_platformView,
_platformView,
_overlay,
]);
], reason: 'Many invisible views can be rendered on top of the base overlay.');

sb = LayerSceneBuilder();
sb.pushOffset(0, 0);
Expand Down Expand Up @@ -1108,20 +1106,20 @@ enum _EmbeddedViewMarker {
_EmbeddedViewMarker get _overlay => _EmbeddedViewMarker.overlay;
_EmbeddedViewMarker get _platformView => _EmbeddedViewMarker.platformView;

void _expectSceneMatches(List<_EmbeddedViewMarker> markers) {
final List<DomElement> sceneElements = flutterViewEmbedder
const Map<String, _EmbeddedViewMarker> _tagToViewMarker = <String, _EmbeddedViewMarker>{
'flt-canvas-container': _EmbeddedViewMarker.overlay,
'flt-platform-view-slot': _EmbeddedViewMarker.platformView,
};

void _expectSceneMatches(List<_EmbeddedViewMarker> expectedMarkers, {
String? reason,
}) {
// Convert the scene elements to its corresponding array of _EmbeddedViewMarker
final List<_EmbeddedViewMarker> sceneElements = flutterViewEmbedder
.sceneElement!.children
.where((DomElement element) => element.tagName != 'svg')
.map((DomElement element) => _tagToViewMarker[element.tagName.toLowerCase()]!)
.toList();
expect(markers, hasLength(sceneElements.length));
for (int i = 0; i < markers.length; i++) {
switch (markers[i]) {
case _EmbeddedViewMarker.overlay:
expect(sceneElements[i].tagName, 'FLT-CANVAS-CONTAINER');
break;
case _EmbeddedViewMarker.platformView:
expect(sceneElements[i].tagName, 'FLT-PLATFORM-VIEW-SLOT');
break;
}
}

expect(sceneElements, expectedMarkers, reason: reason);
}