Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move ViewConfiguration ownership to FlutterView #43701

Merged
merged 10 commits into from
Jul 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 1 addition & 27 deletions lib/ui/fixtures/ui_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
<double>[], // display features bounds
<int>[], // display features types
<int>[], // display features states
0, // Display ID
);

const int viewId = 0;
expectEquals(PlatformDispatcher.instance.view(id: viewId)?.viewId, viewId);
});

Expand Down
71 changes: 22 additions & 49 deletions lib/ui/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ 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. 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;

// A message channel to receive KeyData from the platform.
//
// See embedder.cc::kFlutterKeyDataChannel for more information.
Expand Down Expand Up @@ -185,9 +193,6 @@ class PlatformDispatcher {
/// otherwise.
FlutterView? view({required int id}) => _views[id];

// A map of opaque platform view identifiers to view configurations.
final Map<Object, _ViewConfiguration> _viewConfigurations = <Object, _ViewConfiguration>{};

/// The [FlutterView] provided by the engine if the platform is unable to
/// create windows, or, for backwards compatibility.
///
Expand All @@ -213,7 +218,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<Handle Function()>(symbol: 'PlatformConfigurationNativeApi::ImplicitViewEnabled')
external static bool _implicitViewEnabled();
Expand Down Expand Up @@ -281,13 +286,7 @@ class PlatformDispatcher {
List<int> 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._(
Expand All @@ -314,7 +313,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,
),
Expand All @@ -326,6 +324,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);
}

Expand Down Expand Up @@ -1362,10 +1371,8 @@ 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,
Expand All @@ -1375,37 +1382,6 @@ class _ViewConfiguration {
this.displayId = 0,
});

/// 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<DisplayFeature>? 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;
Expand All @@ -1417,9 +1393,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.
Expand Down Expand Up @@ -1488,7 +1461,7 @@ class _ViewConfiguration {

@override
String toString() {
return '$runtimeType[view: $view, geometry: $geometry]';
return '$runtimeType[geometry: $geometry]';
}
}

Expand Down
24 changes: 16 additions & 8 deletions lib/ui/window.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -96,10 +96,8 @@ class FlutterView {
final PlatformDispatcher platformDispatcher;

/// The configuration of this view.
_ViewConfiguration get _viewConfiguration {
assert(platformDispatcher._viewConfigurations.containsKey(viewId));
return platformDispatcher._viewConfigurations[viewId]!;
}
// 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 {
Expand Down Expand Up @@ -373,6 +371,9 @@ class FlutterView {

@Native<Void Function(Pointer<Void>)>(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.
Expand Down Expand Up @@ -405,8 +406,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(),
);

// Gets its configuration from the FlutterView with the same ID if it exists.
@override
_ViewConfiguration get _viewConfiguration => platformDispatcher._views[viewId]?._viewConfiguration ?? super._viewConfiguration;

/// A callback that is invoked whenever the [devicePixelRatio],
/// [physicalSize], [padding], [viewInsets], [PlatformDispatcher.views], or
Expand Down Expand Up @@ -1016,7 +1024,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 {
Expand Down
1 change: 1 addition & 0 deletions lib/ui/window/platform_configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions testing/dart/window_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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)');
});
}