[go: nahoru, domu]
Skip to content

Commit

Permalink
Move ViewConfiguration ownership to FlutterView (flutter#43701)
Browse files Browse the repository at this point in the history 
This makes the FlutterView object a little bit less brittle by not depending on the PlatformDispatcher so much.
  • Loading branch information
@goderbauer
goderbauer committed Jul 15, 2023
1 parent 85af5ce commit 23a7c5b
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 84 deletions.
28 changes: 1 addition & 27 deletions lib/ui/fixtures/ui_test.dart
View file
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
View file
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
View file
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
View file
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
View file
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)');
});
}

0 comments on commit 23a7c5b

Please sign in to comment.