From 0cdc5259925200966bd956d30c159bf25ed457a8 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 13:37:04 -0700 Subject: [PATCH 01/10] Move ViewConfiguration ownership to FlutterView --- lib/ui/platform_dispatcher.dart | 84 +++++++++---------------- lib/ui/window.dart | 10 +-- lib/ui/window/platform_configuration.cc | 1 + 3 files changed, 35 insertions(+), 60 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 47eb65cab3363..4826eeaa818b0 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -66,6 +66,11 @@ typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace); // A gesture setting value that indicates it has not been set by the engine. const double _kUnsetGestureSetting = -1.0; +// The view ID of PlatformDispatcher.implicitView. +// +// Keep this in sync with kImplicitViewId in window/platform_configuration.cc. +const int _kImplicitViewId = 0; + // A message channel to receive KeyData from the platform. // // See embedder.cc::kFlutterKeyDataChannel for more information. @@ -185,9 +190,6 @@ class PlatformDispatcher { /// otherwise. FlutterView? view({required int id}) => _views[id]; - // A map of opaque platform view identifiers to view configurations. - final Map _viewConfigurations = {}; - /// The [FlutterView] provided by the engine if the platform is unable to /// create windows, or, for backwards compatibility. /// @@ -213,7 +215,7 @@ class PlatformDispatcher { /// * [View.of], for accessing the current view. /// * [PlatformDispatcher.views] for a list of all [FlutterView]s provided /// by the platform. - FlutterView? get implicitView => _implicitViewEnabled() ? _views[0] : null; + FlutterView? get implicitView => _implicitViewEnabled() ? _views[_kImplicitViewId] : null; @Native(symbol: 'PlatformConfigurationNativeApi::ImplicitViewEnabled') external static bool _implicitViewEnabled(); @@ -281,13 +283,7 @@ class PlatformDispatcher { List displayFeaturesState, int displayId, ) { - final _ViewConfiguration previousConfiguration = - _viewConfigurations[id] ?? const _ViewConfiguration(); - if (!_views.containsKey(id)) { - _views[id] = FlutterView._(id, this); - } - _viewConfigurations[id] = previousConfiguration.copyWith( - view: _views[id], + final _ViewConfiguration _viewConfiguration = _ViewConfiguration( devicePixelRatio: devicePixelRatio, geometry: Rect.fromLTWH(0.0, 0.0, width, height), viewPadding: ViewPadding._( @@ -314,7 +310,6 @@ class PlatformDispatcher { bottom: math.max(0.0, systemGestureInsetBottom), left: math.max(0.0, systemGestureInsetLeft), ), - // -1 is used as a sentinel for an undefined touch slop gestureSettings: GestureSettings( physicalTouchSlop: physicalTouchSlop == _kUnsetGestureSetting ? null : physicalTouchSlop, ), @@ -326,6 +321,17 @@ class PlatformDispatcher { ), displayId: displayId, ); + + final FlutterView? view = _views[id]; + if (id == _kImplicitViewId && view == null) { + // TODO(goderbauer): Remove the implicit creation of the implicit view + // when we have an addView API and the implicit view is added via that. + _views[id] = FlutterView._(id, this, _viewConfiguration); + } else { + assert(view != null); + view!._viewConfiguration = _viewConfiguration; + } + _invoke(onMetricsChanged, _onMetricsChangedZone); } @@ -1362,50 +1368,18 @@ class _PlatformConfiguration { /// An immutable view configuration. class _ViewConfiguration { const _ViewConfiguration({ - this.view, - this.devicePixelRatio = 1.0, - this.geometry = Rect.zero, - this.visible = false, - this.viewInsets = ViewPadding.zero, - this.viewPadding = ViewPadding.zero, - this.systemGestureInsets = ViewPadding.zero, - this.padding = ViewPadding.zero, - this.gestureSettings = const GestureSettings(), - this.displayFeatures = const [], - this.displayId = 0, + required this.devicePixelRatio, + required this.geometry, + required this.visible, + required this.viewInsets, + required this.viewPadding,, + required this.systemGestureInsets,, + required this.padding,, + required this.gestureSettings, + required this.displayFeatures, + required this.displayId, }); - /// Copy this configuration with some fields replaced. - _ViewConfiguration copyWith({ - FlutterView? view, - double? devicePixelRatio, - Rect? geometry, - bool? visible, - ViewPadding? viewInsets, - ViewPadding? viewPadding, - ViewPadding? systemGestureInsets, - ViewPadding? padding, - GestureSettings? gestureSettings, - List? displayFeatures, - int? displayId, - }) { - return _ViewConfiguration( - view: view ?? this.view, - devicePixelRatio: devicePixelRatio ?? this.devicePixelRatio, - geometry: geometry ?? this.geometry, - visible: visible ?? this.visible, - viewInsets: viewInsets ?? this.viewInsets, - viewPadding: viewPadding ?? this.viewPadding, - systemGestureInsets: systemGestureInsets ?? this.systemGestureInsets, - padding: padding ?? this.padding, - gestureSettings: gestureSettings ?? this.gestureSettings, - displayFeatures: displayFeatures ?? this.displayFeatures, - displayId: displayId ?? this.displayId, - ); - } - - final FlutterView? view; - /// The identifier for a display for this view, in /// [PlatformDispatcher._displays]. final int displayId; @@ -1488,7 +1462,7 @@ class _ViewConfiguration { @override String toString() { - return '$runtimeType[view: $view, geometry: $geometry]'; + return '$runtimeType[geometry: $geometry]'; } } diff --git a/lib/ui/window.dart b/lib/ui/window.dart index 321e1a792aa9f..9b320ed8bb6be 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -86,7 +86,7 @@ class Display { /// that in the [padding], which is always safe to use for such /// calculations. class FlutterView { - FlutterView._(this.viewId, this.platformDispatcher); + FlutterView._(this.viewId, this.platformDispatcher, this._viewConfiguration); /// The opaque ID for this view. final int viewId; @@ -96,10 +96,7 @@ class FlutterView { final PlatformDispatcher platformDispatcher; /// The configuration of this view. - _ViewConfiguration get _viewConfiguration { - assert(platformDispatcher._viewConfigurations.containsKey(viewId)); - return platformDispatcher._viewConfigurations[viewId]!; - } + _ViewConfiguration _viewConfiguration; /// The [Display] this view is drawn in. Display get display { @@ -373,6 +370,9 @@ class FlutterView { @Native)>(symbol: 'PlatformConfigurationNativeApi::UpdateSemantics') external static void _updateSemantics(_NativeSemanticsUpdate update); + + @override + String toString() => 'FlutterView(id: $viewId)'; } /// Deprecated. Will be removed in a future version of Flutter. diff --git a/lib/ui/window/platform_configuration.cc b/lib/ui/window/platform_configuration.cc index fe6ff03d62b69..8d11b747fd53b 100644 --- a/lib/ui/window/platform_configuration.cc +++ b/lib/ui/window/platform_configuration.cc @@ -22,6 +22,7 @@ namespace flutter { namespace { +// Keep this in sync with _kImplicitViewId in ../platform_dispatcher.dart. constexpr int kImplicitViewId = 0; Dart_Handle ToByteData(const fml::Mapping& buffer) { From 6419d6f4f44e670f7cf40f922c029f03020c1c2c Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 13:47:17 -0700 Subject: [PATCH 02/10] test --- lib/ui/platform_dispatcher.dart | 22 +++++++++------------- lib/ui/window.dart | 13 ++++++++++--- testing/dart/window_test.dart | 6 ++++++ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 4826eeaa818b0..03de1e1ea9381 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -1368,16 +1368,15 @@ class _PlatformConfiguration { /// An immutable view configuration. class _ViewConfiguration { const _ViewConfiguration({ - required this.devicePixelRatio, - required this.geometry, - required this.visible, - required this.viewInsets, - required this.viewPadding,, - required this.systemGestureInsets,, - required this.padding,, - required this.gestureSettings, - required this.displayFeatures, - required this.displayId, + this.devicePixelRatio = 1.0, + this.geometry = Rect.zero, + this.viewInsets = ViewPadding.zero, + this.viewPadding = ViewPadding.zero, + this.systemGestureInsets = ViewPadding.zero, + this.padding = ViewPadding.zero, + this.gestureSettings = const GestureSettings(), + this.displayFeatures = const [], + this.displayId = 0, }); /// The identifier for a display for this view, in @@ -1391,9 +1390,6 @@ class _ViewConfiguration { /// window, in logical pixels. final Rect geometry; - /// Whether or not the view is currently visible on the screen. - final bool visible; - /// The number of physical pixels on each side of the display rectangle into /// which the view can render, but over which the operating system will likely /// place system UI, such as the keyboard, that fully obscures any content. diff --git a/lib/ui/window.dart b/lib/ui/window.dart index 9b320ed8bb6be..6b74e7ed4b8da 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -405,8 +405,15 @@ class SingletonFlutterWindow extends FlutterView { 'Deprecated to prepare for the upcoming multi-window support. ' 'This feature was deprecated after v3.7.0-32.0.pre.' ) - SingletonFlutterWindow._(super.windowId, super.platformDispatcher) - : super._(); + SingletonFlutterWindow._() : super._( + _kImplicitViewId, + PlatformDispatcher.instance, + const _ViewConfiguration(), + ); + + // Shares a configuration with the FlutterView with the same ID. + @override + _ViewConfiguration get _viewConfiguration => platformDispatcher._views[viewId]?._viewConfiguration ?? super._viewConfiguration; /// A callback that is invoked whenever the [devicePixelRatio], /// [physicalSize], [padding], [viewInsets], [PlatformDispatcher.views], or @@ -1016,7 +1023,7 @@ enum Brightness { 'Deprecated to prepare for the upcoming multi-window support. ' 'This feature was deprecated after v3.7.0-32.0.pre.' ) -final SingletonFlutterWindow window = SingletonFlutterWindow._(0, PlatformDispatcher.instance); +final SingletonFlutterWindow window = SingletonFlutterWindow._(); /// Additional data available on each flutter frame. class FrameData { diff --git a/testing/dart/window_test.dart b/testing/dart/window_test.dart index f51c6501cf29c..8096883304ca0 100644 --- a/testing/dart/window_test.dart +++ b/testing/dart/window_test.dart @@ -89,4 +89,10 @@ void main() { expect(display.refreshRate, 60); expect(display.size, implicitView.physicalSize); }); + + test('FlutterView.toString contains the viewId', () { + final FlutterView flutterView = PlatformDispatcher.instance.implicitView!; + expect(flutterView.viewId, 0) + expect(flutterView.toString(), 'FlutterView(id: $0)'); + }); } From b82fcfc7e9207e282cf8f6ee262e6c538bc9a94f Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 14:11:12 -0700 Subject: [PATCH 03/10] update comment --- lib/ui/window.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/window.dart b/lib/ui/window.dart index 6b74e7ed4b8da..ae80a69dce8b8 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -411,7 +411,7 @@ class SingletonFlutterWindow extends FlutterView { const _ViewConfiguration(), ); - // Shares a configuration with the FlutterView with the same ID. + // Gets its configuration from the FlutterView with the same ID. @override _ViewConfiguration get _viewConfiguration => platformDispatcher._views[viewId]?._viewConfiguration ?? super._viewConfiguration; From 8cf8c0e5436f4bff7f3801379dd73d08159eef26 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 14:18:16 -0700 Subject: [PATCH 04/10] fix test --- testing/dart/window_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/dart/window_test.dart b/testing/dart/window_test.dart index 8096883304ca0..253999821ecf4 100644 --- a/testing/dart/window_test.dart +++ b/testing/dart/window_test.dart @@ -93,6 +93,6 @@ void main() { test('FlutterView.toString contains the viewId', () { final FlutterView flutterView = PlatformDispatcher.instance.implicitView!; expect(flutterView.viewId, 0) - expect(flutterView.toString(), 'FlutterView(id: $0)'); + expect(flutterView.toString(), 'FlutterView(id: 0)'); }); } From 42e3c05c6852fc59ff0f7d3cef7be64242dee333 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 14:24:00 -0700 Subject: [PATCH 05/10] comment update --- lib/ui/window.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/window.dart b/lib/ui/window.dart index ae80a69dce8b8..fb0a65e800963 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -411,7 +411,7 @@ class SingletonFlutterWindow extends FlutterView { const _ViewConfiguration(), ); - // Gets its configuration from the FlutterView with the same ID. + // Gets its configuration from the FlutterView with the same ID if it exists. @override _ViewConfiguration get _viewConfiguration => platformDispatcher._views[viewId]?._viewConfiguration ?? super._viewConfiguration; From f16bea4647a50634b94bb83ced5e8d6ce83f0211 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 14:25:33 -0700 Subject: [PATCH 06/10] test fix --- testing/dart/window_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/dart/window_test.dart b/testing/dart/window_test.dart index 253999821ecf4..a0c98c441ea3e 100644 --- a/testing/dart/window_test.dart +++ b/testing/dart/window_test.dart @@ -92,7 +92,7 @@ void main() { test('FlutterView.toString contains the viewId', () { final FlutterView flutterView = PlatformDispatcher.instance.implicitView!; - expect(flutterView.viewId, 0) + expect(flutterView.viewId, 0); expect(flutterView.toString(), 'FlutterView(id: 0)'); }); } From 6b9d2f8bdf39110bbcdf92ea35e147396bbb7804 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 15:48:56 -0700 Subject: [PATCH 07/10] adjust test to make it pass --- lib/ui/fixtures/ui_test.dart | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 09caf203097dd..0aff1775688b7 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -539,33 +539,7 @@ void hooksTests() async { }); await test('PlatformDispatcher.view getter returns view with provided ID', () { - const int viewId = 123456789; - _callHook( - '_updateWindowMetrics', - 21, - viewId, // window Id - 1.0, // devicePixelRatio - 800.0, // width - 600.0, // height - 50.0, // paddingTop - 0.0, // paddingRight - 40.0, // paddingBottom - 0.0, // paddingLeft - 0.0, // insetTop - 0.0, // insetRight - 0.0, // insetBottom - 0.0, // insetLeft - 0.0, // systemGestureInsetTop - 0.0, // systemGestureInsetRight - 0.0, // systemGestureInsetBottom - 0.0, // systemGestureInsetLeft - 22.0, // physicalTouchSlop - [], // display features bounds - [], // display features types - [], // display features states - 0, // Display ID - ); - + const int viewId = 0; expectEquals(PlatformDispatcher.instance.view(id: viewId)?.viewId, viewId); }); From 9c95f384732c4fdc86ab500fe94828ea0fba5d56 Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 16:17:44 -0700 Subject: [PATCH 08/10] analyzer --- lib/ui/platform_dispatcher.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 03de1e1ea9381..8b31368503d4d 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -283,7 +283,7 @@ class PlatformDispatcher { List displayFeaturesState, int displayId, ) { - final _ViewConfiguration _viewConfiguration = _ViewConfiguration( + final _ViewConfiguration viewConfiguration = _ViewConfiguration( devicePixelRatio: devicePixelRatio, geometry: Rect.fromLTWH(0.0, 0.0, width, height), viewPadding: ViewPadding._( @@ -326,10 +326,10 @@ class PlatformDispatcher { if (id == _kImplicitViewId && view == null) { // TODO(goderbauer): Remove the implicit creation of the implicit view // when we have an addView API and the implicit view is added via that. - _views[id] = FlutterView._(id, this, _viewConfiguration); + _views[id] = FlutterView._(id, this, viewConfiguration); } else { assert(view != null); - view!._viewConfiguration = _viewConfiguration; + view!._viewConfiguration = viewConfiguration; } _invoke(onMetricsChanged, _onMetricsChangedZone); From 8d3b9b819da372e1b2c3649979e56d6f26d99e6f Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 16:27:08 -0700 Subject: [PATCH 09/10] add ignore for linter bug --- lib/ui/window.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ui/window.dart b/lib/ui/window.dart index fb0a65e800963..63b7f74b8b5a7 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -96,7 +96,8 @@ class FlutterView { final PlatformDispatcher platformDispatcher; /// The configuration of this view. - _ViewConfiguration _viewConfiguration; + // TODO(goderbauer): remove ignore when https://github.com/dart-lang/linter/issues/4562 is fixed. + _ViewConfiguration _viewConfiguration; // ignore: prefer_final_fields /// The [Display] this view is drawn in. Display get display { From f231b0b94557719b2931776f80b70aae411107ff Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Fri, 14 Jul 2023 17:30:44 -0700 Subject: [PATCH 10/10] Update lib/ui/platform_dispatcher.dart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com> --- lib/ui/platform_dispatcher.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 8b31368503d4d..d529fac09e1d7 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -66,7 +66,10 @@ typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace); // A gesture setting value that indicates it has not been set by the engine. const double _kUnsetGestureSetting = -1.0; -// The view ID of PlatformDispatcher.implicitView. +// The view ID of PlatformDispatcher.implicitView. This is an +// implementation detail that may change at any time. Apps +// should always use PlatformDispatcher.implicitView to determine +// the current implicit view, if any. // // Keep this in sync with kImplicitViewId in window/platform_configuration.cc. const int _kImplicitViewId = 0;