From f40fc908f514b175a28a697b7b8cdce48c316da2 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" Date: Tue, 6 Feb 2024 17:33:00 +0000 Subject: [PATCH] Revert "[web] Fix Scene clip bounds. Trigger resize on DPR Change. (#50161)" This reverts commit 880bf52e6c42a363684c17be9223ace0761ee6ce. --- ci/licenses_golden/licenses_flutter | 2 - lib/web_ui/lib/src/engine.dart | 1 - lib/web_ui/lib/src/engine/html/scene.dart | 12 +-- .../custom_element_dimensions_provider.dart | 40 +++----- .../dimensions_provider.dart | 25 ++--- .../full_page_dimensions_provider.dart | 5 +- .../view_embedder/display_dpr_stream.dart | 93 ------------------- ...stom_element_dimensions_provider_test.dart | 34 ++----- .../dimensions_provider_test.dart | 14 +++ .../full_page_dimensions_provider_test.dart | 3 + .../display_dpr_stream_test.dart | 45 --------- 11 files changed, 48 insertions(+), 226 deletions(-) delete mode 100644 lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart delete mode 100644 lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 6419812c072a5..26dfb762b3930 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -6134,7 +6134,6 @@ ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/vector_math.dart + ../../../f ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart + ../../../flutter/LICENSE -ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dom_manager.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart + ../../../flutter/LICENSE ORIGIN: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart + ../../../flutter/LICENSE @@ -8998,7 +8997,6 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/vector_math.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart -FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/dom_manager.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart diff --git a/lib/web_ui/lib/src/engine.dart b/lib/web_ui/lib/src/engine.dart index 14bda2390fdcc..57c7b35fafb8a 100644 --- a/lib/web_ui/lib/src/engine.dart +++ b/lib/web_ui/lib/src/engine.dart @@ -190,7 +190,6 @@ export 'engine/vector_math.dart'; export 'engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart'; export 'engine/view_embedder/dimensions_provider/dimensions_provider.dart'; export 'engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart'; -export 'engine/view_embedder/display_dpr_stream.dart'; export 'engine/view_embedder/dom_manager.dart'; export 'engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart'; export 'engine/view_embedder/embedding_strategy/embedding_strategy.dart'; diff --git a/lib/web_ui/lib/src/engine/html/scene.dart b/lib/web_ui/lib/src/engine/html/scene.dart index b9f69996b1097..c9703038b6103 100644 --- a/lib/web_ui/lib/src/engine/html/scene.dart +++ b/lib/web_ui/lib/src/engine/html/scene.dart @@ -45,15 +45,9 @@ class PersistedScene extends PersistedContainerSurface { @override void recomputeTransformAndClip() { - // The scene clip is the size of the entire window **in Logical pixels**. - // - // Even though the majority of the engine uses `physicalSize`, there are some - // bits (like the HTML renderer, or dynamic view sizing) that are implemented - // using CSS, and CSS operates in logical pixels. - // - // See also: [EngineFlutterView.resize]. - final ui.Size bounds = window.physicalSize / window.devicePixelRatio; - localClipBounds = ui.Rect.fromLTRB(0, 0, bounds.width, bounds.height); + // The scene clip is the size of the entire window. + final ui.Size screen = window.physicalSize; + localClipBounds = ui.Rect.fromLTRB(0, 0, screen.width, screen.height); projectedClip = null; } diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart index 25f77afdd5038..7b39ca908e576 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider.dart @@ -4,7 +4,6 @@ import 'dart:async'; -import 'package:ui/src/engine/display.dart'; import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; @@ -13,36 +12,21 @@ import 'dimensions_provider.dart'; /// This class provides observable, real-time dimensions of a host element. /// -/// This class needs a `Stream` of `devicePixelRatio` changes, like the one -/// provided by [DisplayDprStream], because html resize observers do not report -/// DPR changes. -/// /// All the measurements returned from this class are potentially *expensive*, /// and should be cached as needed. Every call to every method on this class /// WILL perform actual DOM measurements. -/// -/// This broadcasts `null` size events, to match the implementation of the -/// FullPageDimensionsProvider, but it could broadcast the size coming from the -/// DomResizeObserverEntry. Further changes in the engine are required for this -/// to be effective. class CustomElementDimensionsProvider extends DimensionsProvider { /// Creates a [CustomElementDimensionsProvider] from a [_hostElement]. - CustomElementDimensionsProvider(this._hostElement, { - Stream? onDprChange, - }) { - // Send a resize event when the page DPR changes. - _dprChangeStreamSubscription = onDprChange?.listen((_) { - _broadcastSize(null); - }); - + CustomElementDimensionsProvider(this._hostElement) { // Hook up a resize observer on the hostElement (if supported!). _hostElementResizeObserver = createDomResizeObserver(( List entries, DomResizeObserver _, ) { - for (final DomResizeObserverEntry _ in entries) { - _broadcastSize(null); - } + entries + .map((DomResizeObserverEntry entry) => + ui.Size(entry.contentRect.width, entry.contentRect.height)) + .forEach(_broadcastSize); }); assert(() { @@ -61,12 +45,11 @@ class CustomElementDimensionsProvider extends DimensionsProvider { // Handle resize events late DomResizeObserver? _hostElementResizeObserver; - late StreamSubscription? _dprChangeStreamSubscription; - final StreamController _onResizeStreamController = - StreamController.broadcast(); + final StreamController _onResizeStreamController = + StreamController.broadcast(); // Broadcasts the last seen `Size`. - void _broadcastSize(ui.Size? size) { + void _broadcastSize(ui.Size size) { _onResizeStreamController.add(size); } @@ -75,17 +58,16 @@ class CustomElementDimensionsProvider extends DimensionsProvider { super.close(); _hostElementResizeObserver?.disconnect(); // ignore:unawaited_futures - _dprChangeStreamSubscription?.cancel(); - // ignore:unawaited_futures _onResizeStreamController.close(); } @override - Stream get onResize => _onResizeStreamController.stream; + Stream get onResize => _onResizeStreamController.stream; @override ui.Size computePhysicalSize() { - final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; + final double devicePixelRatio = getDevicePixelRatio(); + return ui.Size( _hostElement.clientWidth * devicePixelRatio, _hostElement.clientHeight * devicePixelRatio, diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart index 469f3a197d25b..2fcf44834395e 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/dimensions_provider.dart @@ -5,11 +5,11 @@ import 'dart:async'; import 'package:meta/meta.dart'; -import 'package:ui/src/engine/dom.dart'; -import 'package:ui/src/engine/view_embedder/display_dpr_stream.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; +import '../../display.dart'; +import '../../dom.dart'; import 'custom_element_dimensions_provider.dart'; import 'full_page_dimensions_provider.dart'; @@ -32,15 +32,18 @@ abstract class DimensionsProvider { /// Creates the appropriate DimensionsProvider depending on the incoming [hostElement]. factory DimensionsProvider.create({DomElement? hostElement}) { if (hostElement != null) { - return CustomElementDimensionsProvider( - hostElement, - onDprChange: DisplayDprStream.instance.dprChanged, - ); + return CustomElementDimensionsProvider(hostElement); } else { return FullPageDimensionsProvider(); } } + /// Returns the DPI reported by the browser. + double getDevicePixelRatio() { + // This is overridable in tests. + return EngineFlutterDisplay.instance.devicePixelRatio; + } + /// Returns the [ui.Size] of the "viewport". /// /// This function is expensive. It triggers browser layout if there are @@ -54,16 +57,6 @@ abstract class DimensionsProvider { ); /// Returns a Stream with the changes to [ui.Size] (when cheap to get). - /// - /// Currently this Stream always returns `null` measurements because the - /// resize event that we use for [FullPageDimensionsProvider] does not contain - /// the new size, so users of this Stream everywhere immediately retrieve the - /// new `physicalSize` from the window. - /// - /// The [CustomElementDimensionsProvider] *could* broadcast the new size, but - /// to keep both implementations consistent (and their consumers), for now all - /// events from this Stream are going to be `null` (until we find a performant - /// way to retrieve the dimensions in full-page mode). Stream get onResize; /// Whether the [DimensionsProvider] instance has been closed or not. diff --git a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart index bcacba4c869f3..8221d95457aef 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/dimensions_provider/full_page_dimensions_provider.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'package:ui/src/engine/browser_detection.dart'; -import 'package:ui/src/engine/display.dart'; import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/window.dart'; import 'package:ui/ui.dart' as ui show Size; @@ -68,7 +67,7 @@ class FullPageDimensionsProvider extends DimensionsProvider { late double windowInnerWidth; late double windowInnerHeight; final DomVisualViewport? viewport = domWindow.visualViewport; - final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; + final double devicePixelRatio = getDevicePixelRatio(); if (viewport != null) { if (operatingSystem == OperatingSystem.iOs) { @@ -103,7 +102,7 @@ class FullPageDimensionsProvider extends DimensionsProvider { double physicalHeight, bool isEditingOnMobile, ) { - final double devicePixelRatio = EngineFlutterDisplay.instance.devicePixelRatio; + final double devicePixelRatio = getDevicePixelRatio(); final DomVisualViewport? viewport = domWindow.visualViewport; late double windowInnerHeight; diff --git a/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart b/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart deleted file mode 100644 index d73eeb5aec244..0000000000000 --- a/lib/web_ui/lib/src/engine/view_embedder/display_dpr_stream.dart +++ /dev/null @@ -1,93 +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 'dart:async'; -import 'dart:js_interop'; - -import 'package:meta/meta.dart'; -import 'package:ui/src/engine/display.dart'; -import 'package:ui/src/engine/dom.dart'; -import 'package:ui/ui.dart' as ui show Display; - -/// Provides a stream of `devicePixelRatio` changes for the given display. -/// -/// Note that until the Window Management API is generally available, this class -/// only monitors the global `devicePixelRatio` property, provided by the default -/// [EngineFlutterDisplay.instance]. -/// -/// See: https://developer.mozilla.org/en-US/docs/Web/API/Window_Management_API -class DisplayDprStream { - DisplayDprStream( - this._display, { - @visibleForTesting DebugDisplayDprStreamOverrides? overrides, - }) : _currentDpr = _display.devicePixelRatio, - _debugOverrides = overrides { - // Start listening to DPR changes. - _subscribeToMediaQuery(); - } - - /// A singleton instance of DisplayDprStream. - static DisplayDprStream instance = - DisplayDprStream(EngineFlutterDisplay.instance); - - // The display object that will provide the DPR information. - final ui.Display _display; - - // Last reported value of DPR. - double _currentDpr; - - // Controls the [dprChanged] broadcast Stream. - final StreamController _dprStreamController = - StreamController.broadcast(); - - // Object that fires a `change` event for the `_currentDpr`. - late DomEventTarget _dprMediaQuery; - - // Creates the media query for the latest known DPR value, and adds a change listener to it. - void _subscribeToMediaQuery() { - if (_debugOverrides?.getMediaQuery != null) { - _dprMediaQuery = _debugOverrides!.getMediaQuery!(_currentDpr); - } else { - _dprMediaQuery = domWindow.matchMedia('(resolution: ${_currentDpr}dppx)'); - } - _dprMediaQuery.addEventListenerWithOptions( - 'change', - createDomEventListener(_onDprMediaQueryChange), - { - // We only listen `once` because this event only triggers once when the - // DPR changes from `_currentDpr`. Once that happens, we need a new - // `_dprMediaQuery` that is watching the new `_currentDpr`. - // - // By using `once`, we don't need to worry about detaching the event - // listener from the old mediaQuery after we're done with it. - 'once': true, - 'passive': true, - }, - ); - } - - // Handler of the _dprMediaQuery 'change' event. - // - // This calls subscribe again because events are listened to with `once: true`. - JSVoid _onDprMediaQueryChange(DomEvent _) { - _currentDpr = _display.devicePixelRatio; - _dprStreamController.add(_currentDpr); - // Re-subscribe... - _subscribeToMediaQuery(); - } - - /// A stream that emits the latest value of [EngineFlutterDisplay.instance.devicePixelRatio]. - Stream get dprChanged => _dprStreamController.stream; - - // The overrides object that is used for testing. - final DebugDisplayDprStreamOverrides? _debugOverrides; -} - -@visibleForTesting -class DebugDisplayDprStreamOverrides { - DebugDisplayDprStreamOverrides({ - this.getMediaQuery, - }); - final DomEventTarget Function(double currentValue)? getMediaQuery; -} diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart index 4416aa59bc56c..9ee7a95083c05 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/custom_element_dimensions_provider_test.dart @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +@TestOn('browser') +library; + import 'dart:async'; import 'package:test/bootstrap/browser.dart'; @@ -106,48 +109,23 @@ void doTests() { }); test('funnels resize events on sizeSource', () async { - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(2.7); - sizeSource ..style.width = '100px' ..style.height = '100px'; - expect(provider.onResize.first, completes); - expect(provider.computePhysicalSize(), const ui.Size(270, 270)); + expect(await provider.onResize.first, const ui.Size(100, 100)); sizeSource ..style.width = '200px' ..style.height = '200px'; - expect(provider.onResize.first, completes); - expect(provider.computePhysicalSize(), const ui.Size(540, 540)); + expect(await provider.onResize.first, const ui.Size(200, 200)); sizeSource ..style.width = '300px' ..style.height = '300px'; - expect(provider.onResize.first, completes); - expect(provider.computePhysicalSize(), const ui.Size(810, 810)); - }); - - test('funnels DPR change events too', () async { - // Override the source of DPR events... - final StreamController dprController = - StreamController.broadcast(); - - // Inject the dprController stream into the CustomElementDimensionsProvider. - final CustomElementDimensionsProvider provider = - CustomElementDimensionsProvider( - sizeSource, - onDprChange: dprController.stream, - ); - - // Set and broadcast the mock DPR value - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(3.2); - dprController.add(3.2); - - expect(provider.onResize.first, completes); - expect(provider.computePhysicalSize(), const ui.Size(32, 32)); + expect(await provider.onResize.first, const ui.Size(300, 300)); }); test('closed by onHotRestart', () async { diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart index 7c355f94fdd17..ba81aa32f01e3 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/dimensions_provider_test.dart @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +@TestOn('browser') +library; + import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; @@ -28,4 +31,15 @@ void doTests() { expect(provider, isA()); }); }); + + group('getDevicePixelRatio', () { + test('Returns the correct pixelRatio', () async { + // Override the DPI to something known, but weird... + EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(33930); + + final DimensionsProvider provider = DimensionsProvider.create(); + + expect(provider.getDevicePixelRatio(), 33930); + }); + }); } diff --git a/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart b/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart index cd633da01526d..88a037c053e45 100644 --- a/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart +++ b/lib/web_ui/test/engine/view_embedder/dimensions_provider/full_page_dimensions_provider_test.dart @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +@TestOn('browser') +library; + import 'dart:async'; import 'package:test/bootstrap/browser.dart'; diff --git a/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart b/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart deleted file mode 100644 index a32ff78620fcf..0000000000000 --- a/lib/web_ui/test/engine/view_embedder/display_dpr_stream_test.dart +++ /dev/null @@ -1,45 +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 'dart:async'; - -import 'package:test/bootstrap/browser.dart'; -import 'package:test/test.dart'; -import 'package:ui/src/engine.dart'; - -void main() { - internalBootstrapBrowserTest(() => doTests); -} - -void doTests() { - final DomEventTarget eventTarget = createDomElement('div'); - - group('dprChanged Stream', () { - late DisplayDprStream dprStream; - - setUp(() async { - dprStream = DisplayDprStream(EngineFlutterDisplay.instance, - overrides: DebugDisplayDprStreamOverrides( - getMediaQuery: (_) => eventTarget, - )); - }); - - test('funnels display DPR on every mediaQuery "change" event.', () async { - final Future> dprs = dprStream.dprChanged - .take(3) - .timeout(const Duration(seconds: 1)) - .toList(); - - // Simulate the events - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(6.9); - eventTarget.dispatchEvent(createDomEvent('Event', 'change')); - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(4.2); - eventTarget.dispatchEvent(createDomEvent('Event', 'change')); - EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(0.71); - eventTarget.dispatchEvent(createDomEvent('Event', 'change')); - - expect(await dprs, [6.9, 4.2, 0.71]); - }); - }); -}