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

Pass Android Q insets.systemGestureInsets to Window #10413

Merged
merged 31 commits into from
Aug 16, 2019
Merged

Pass Android Q insets.systemGestureInsets to Window #10413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0c1252d
(WIP) Wired inset values to framework
shihaohong Jul 31, 2019
32a6ee7
Retrieve systemGestureInsetsTop using reflection
shihaohong Aug 1, 2019
59c39b5
Implement systemGestureInsets
shihaohong Aug 1, 2019
e908f71
Update missing documentation
shihaohong Aug 1, 2019
0577e2e
Fix arguments on a failing test
shihaohong Aug 1, 2019
f8c3715
Remove reflection getting system gesture insets
shihaohong Aug 6, 2019
a26d365
Update variable names to be consistent with rest of file
shihaohong Aug 6, 2019
db78d67
Merge with upstream/master
shihaohong Aug 6, 2019
eef5685
Missing updates to merge request
shihaohong Aug 6, 2019
ebe1348
Incorporate API doc updates from code review
shihaohong Aug 6, 2019
ab80439
Remove unnecessary _updateWindowMetrics() call, since tearDown is now…
shihaohong Aug 6, 2019
0ce87cc
Update build-tools and platform-tools versions in DEPS file
shihaohong Aug 6, 2019
6a37a63
Update to latest references of android SDK
shihaohong Aug 6, 2019
d420f27
Merge branch 'master' of github.com:flutter/engine into android-q-ges…
shihaohong Aug 6, 2019
8d78d2a
Update shell unit tests, update buildroot commit hash
shihaohong Aug 6, 2019
01282b6
Add FlutterView changes to new embedding
shihaohong Aug 6, 2019
d32f014
Fix .cc formatting
shihaohong Aug 7, 2019
ab83eac
Revert from VERSION_CODES to int values for version
shihaohong Aug 7, 2019
33b6ea0
Add systemGestureInsets to stub_ui
shihaohong Aug 9, 2019
e560333
(WIP) - Mocked engine for FlutterView unit tests
shihaohong Aug 9, 2019
0a465b7
Merge with upstream/master
shihaohong Aug 12, 2019
ff27613
Comment tests back into run_tests.py
shihaohong Aug 12, 2019
bc72f98
Fix ordering of android/BUILD.gn sources
shihaohong Aug 12, 2019
98cd1f2
Remove FlutterView tests
shihaohong Aug 15, 2019
a1f6cbf
Merge branch 'master' into android-q-gestures
shihaohong Aug 15, 2019
7dac454
Remove changes to FlutterTestSuite.java
shihaohong Aug 15, 2019
3f4af17
Update to use VERSION_CODES instead of int value
shihaohong Aug 15, 2019
a8b17b4
Merge branch 'android-q-gestures' of github.com:shihaohong/engine int…
shihaohong Aug 15, 2019
8309143
Add lint suppression for guarded Android Q API calls
shihaohong Aug 15, 2019
7f50a08
Fix SuppressLint imports and syntax
shihaohong Aug 15, 2019
ab8f418
Move SuppressLint comment out of Javadoc block to above @SuppressLint…
shihaohong Aug 16, 2019
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
37 changes: 24 additions & 13 deletions lib/ui/hooks.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,24 @@ dynamic _decodeJSON(String message) {

@pragma('vm:entry-point')
// ignore: unused_element
void _updateWindowMetrics(double devicePixelRatio,
double width,
double height,
double depth,
double viewPaddingTop,
double viewPaddingRight,
double viewPaddingBottom,
double viewPaddingLeft,
double viewInsetTop,
double viewInsetRight,
double viewInsetBottom,
double viewInsetLeft) {
void _updateWindowMetrics(
double devicePixelRatio,
double width,
double height,
double depth,
double viewPaddingTop,
double viewPaddingRight,
double viewPaddingBottom,
double viewPaddingLeft,
double viewInsetTop,
double viewInsetRight,
double viewInsetBottom,
double viewInsetLeft,
double systemGestureInsetTop,
double systemGestureInsetRight,
double systemGestureInsetBottom,
double systemGestureInsetLeft,
) {
window
.._devicePixelRatio = devicePixelRatio
.._physicalSize = Size(width, height)
Expand All @@ -48,7 +54,12 @@ void _updateWindowMetrics(double devicePixelRatio,
top: math.max(0.0, viewPaddingTop - viewInsetTop),
right: math.max(0.0, viewPaddingRight - viewInsetRight),
bottom: math.max(0.0, viewPaddingBottom - viewInsetBottom),
left: math.max(0.0, viewPaddingLeft - viewInsetLeft));
left: math.max(0.0, viewPaddingLeft - viewInsetLeft))
.._systemGestureInsets = WindowPadding._(
top: math.max(0.0, systemGestureInsetTop),
right: math.max(0.0, systemGestureInsetRight),
bottom: math.max(0.0, systemGestureInsetBottom),
left: math.max(0.0, systemGestureInsetLeft));
_invoke(window.onMetricsChanged, window._onMetricsChangedZone);
}

Expand Down
29 changes: 24 additions & 5 deletions lib/ui/window.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ class Window {
/// will likely place system UI, such as the keyboard, that fully obscures
/// any content.
///
/// When this changes, [onMetricsChanged] is called.
/// When this property changes, [onMetricsChanged] is called.
///
/// The relationship between this [Window.viewInsets], [Window.viewPadding],
/// and [Window.padding] are described in more detail in the documentation for
Expand All @@ -664,7 +664,7 @@ class Window {
/// response to the soft keyboard being visible or hidden, whereas
/// [Window.padding] will.
///
/// When this changes, [onMetricsChanged] is called.
/// When this property changes, [onMetricsChanged] is called.
///
/// The relationship between this [Window.viewInsets], [Window.viewPadding],
/// and [Window.padding] are described in more detail in the documentation for
Expand All @@ -680,6 +680,24 @@ class Window {
WindowPadding get viewPadding => _viewPadding;
WindowPadding _viewPadding = WindowPadding.zero;

/// The number of physical pixels on each side of the display rectangle into
Copy link
Contributor

@matthew-carroll matthew-carroll Aug 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary for this PR, but can we follow up with a diagram of the various paddings and insets and put it on the wiki, or on the website, and then maybe link to it from here?

Copy link
Author

@shihaohong shihaohong Aug 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an issue (flutter/flutter#38649). some work already exists in the MediaQueryData API docs, we could elaborate on the new insets there and add links to this particular resource once everything has been solidified.

/// which the application can render, but where the operating system will
/// consume input gestures for the sake of system navigation.
///
/// For example, an operating system might use the vertical edges of the
/// screen, where swiping inwards from the edges takes users backward
/// through the history of screens they previously visited.
///
/// When this property changes, [onMetricsChanged] is called.
///
/// See also:
///
/// * [WidgetsBindingObserver], for a mechanism at the widgets layer to
/// observe when this value changes.
/// * [MediaQuery.of], a simpler mechanism for the same.
WindowPadding get systemGestureInsets => _systemGestureInsets;
WindowPadding _systemGestureInsets = WindowPadding.zero;

/// The number of physical pixels on each side of the display rectangle into
/// which the application can render, but which may be partially obscured by
/// system UI (such as the system notification area), or or physical
Expand Down Expand Up @@ -711,9 +729,10 @@ class Window {
WindowPadding _padding = WindowPadding.zero;

/// A callback that is invoked whenever the [devicePixelRatio],
/// [physicalSize], [padding], or [viewInsets] values change, for example
/// when the device is rotated or when the application is resized (e.g. when
/// showing applications side-by-side on Android).
/// [physicalSize], [padding], [viewInsets], or [systemGestureInsets]
/// values change, for example when the device is rotated or when the
/// application is resized (e.g. when showing applications side-by-side
/// on Android).
///
/// The engine invokes this callback in the same zone in which the callback
/// was set.
Expand Down
15 changes: 13 additions & 2 deletions lib/ui/window/viewport_metrics.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio,
double p_physical_view_inset_top,
double p_physical_view_inset_right,
double p_physical_view_inset_bottom,
double p_physical_view_inset_left)
double p_physical_view_inset_left,
double p_physical_system_gesture_inset_top,
double p_physical_system_gesture_inset_right,
double p_physical_system_gesture_inset_bottom,
double p_physical_system_gesture_inset_left)
: device_pixel_ratio(p_device_pixel_ratio),
physical_width(p_physical_width),
physical_height(p_physical_height),
Expand All @@ -28,7 +32,14 @@ ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio,
physical_view_inset_top(p_physical_view_inset_top),
physical_view_inset_right(p_physical_view_inset_right),
physical_view_inset_bottom(p_physical_view_inset_bottom),
physical_view_inset_left(p_physical_view_inset_left) {}
physical_view_inset_left(p_physical_view_inset_left),
physical_system_gesture_inset_top(p_physical_system_gesture_inset_top),
physical_system_gesture_inset_right(
p_physical_system_gesture_inset_right),
physical_system_gesture_inset_bottom(
p_physical_system_gesture_inset_bottom),
physical_system_gesture_inset_left(p_physical_system_gesture_inset_left) {
}

ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio,
double p_physical_width,
Expand Down
10 changes: 9 additions & 1 deletion lib/ui/window/viewport_metrics.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ struct ViewportMetrics {
double p_physical_view_inset_top,
double p_physical_view_inset_right,
double p_physical_view_inset_bottom,
double p_physical_view_inset_left);
double p_physical_view_inset_left,
double p_physical_system_gesture_inset_top,
double p_physical_system_gesture_inset_right,
double p_physical_system_gesture_inset_bottom,
double p_physical_system_gesture_inset_left);

// Create a ViewportMetrics instance that contains z information.
ViewportMetrics(double p_device_pixel_ratio,
Expand Down Expand Up @@ -63,6 +67,10 @@ struct ViewportMetrics {
double physical_view_inset_left = 0;
double physical_view_inset_front = kUnsetDepth;
double physical_view_inset_back = kUnsetDepth;
double physical_system_gesture_inset_top = 0;
Copy link
Contributor

@matthew-carroll matthew-carroll Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any implications of defining gesture insets in such a central location? I'm guessing that this definition of viewport metrics is used for all platforms, right? If so, would gesture insets ever make sense on Windows, Linux, Mac, and other non-touch surfaces?

Copy link
Author

@shihaohong shihaohong Aug 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason I decided to define gesture insets this way is that I could foresee other platforms using system gesture navigations as well. I'll need to see how iOS currently handles this, but I know that swiping up from the bottom of the screen on later iPhones is analogous to pressing the home button on older models of the phone.

double physical_system_gesture_inset_right = 0;
double physical_system_gesture_inset_bottom = 0;
double physical_system_gesture_inset_left = 0;
};

struct LogicalSize {
Expand Down
4 changes: 4 additions & 0 deletions lib/ui/window/window.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ void Window::UpdateWindowMetrics(const ViewportMetrics& metrics) {
tonic::ToDart(metrics.physical_view_inset_right),
tonic::ToDart(metrics.physical_view_inset_bottom),
tonic::ToDart(metrics.physical_view_inset_left),
tonic::ToDart(metrics.physical_system_gesture_inset_top),
tonic::ToDart(metrics.physical_system_gesture_inset_right),
tonic::ToDart(metrics.physical_system_gesture_inset_bottom),
tonic::ToDart(metrics.physical_system_gesture_inset_left),
}));
}

Expand Down
2 changes: 2 additions & 0 deletions lib/web_ui/lib/src/ui/window.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,8 @@ abstract class Window {

WindowPadding get viewPadding => WindowPadding.zero;

WindowPadding get systemGestureInsets => WindowPadding.zero;

/// The number of physical pixels on each side of the display rectangle into
/// which the application can render, but which may be partially obscured by
/// system UI (such as the system notification area), or or physical
Expand Down
3 changes: 2 additions & 1 deletion shell/common/shell_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ void ShellTest::PumpOneFrame(Shell* shell) {
fml::AutoResetWaitableEvent latch;
shell->GetTaskRunners().GetUITaskRunner()->PostTask(
[&latch, engine = shell->weak_engine_]() {
engine->SetViewportMetrics({1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0});
engine->SetViewportMetrics(
{1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0});
engine->animator_->BeginFrame(fml::TimePoint::Now(),
fml::TimePoint::Now());
latch.Signal();
Expand Down
8 changes: 4 additions & 4 deletions shell/common/shell_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ TEST_F(ShellTest, SetResourceCacheSize) {
fml::TaskRunner::RunNowOrPostTask(
shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell]() {
shell->GetPlatformView()->SetViewportMetrics(
{1.0, 400, 200, 0, 0, 0, 0, 0, 0, 0, 0});
{1.0, 400, 200, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0});
});
PumpOneFrame(shell.get());

Expand All @@ -658,7 +658,7 @@ TEST_F(ShellTest, SetResourceCacheSize) {
fml::TaskRunner::RunNowOrPostTask(
shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell]() {
shell->GetPlatformView()->SetViewportMetrics(
{1.0, 800, 400, 0, 0, 0, 0, 0, 0, 0, 0});
{1.0, 800, 400, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0});
});
PumpOneFrame(shell.get());

Expand All @@ -676,7 +676,7 @@ TEST_F(ShellTest, SetResourceCacheSizeEarly) {
fml::TaskRunner::RunNowOrPostTask(
shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell]() {
shell->GetPlatformView()->SetViewportMetrics(
{1.0, 400, 200, 0, 0, 0, 0, 0, 0, 0, 0});
{1.0, 400, 200, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0});
});
PumpOneFrame(shell.get());

Expand Down Expand Up @@ -704,7 +704,7 @@ TEST_F(ShellTest, SetResourceCacheSizeNotifiesDart) {
fml::TaskRunner::RunNowOrPostTask(
shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell]() {
shell->GetPlatformView()->SetViewportMetrics(
{1.0, 400, 200, 0, 0, 0, 0, 0, 0, 0, 0});
{1.0, 400, 200, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0});
});
PumpOneFrame(shell.get());

Expand Down
18 changes: 17 additions & 1 deletion shell/platform/android/io/flutter/embedding/android/FlutterView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
package io.flutter.embedding.android;

import android.annotation.TargetApi;
import android.annotation.SuppressLint;
import android.content.Context;
import android.content.res.Configuration;
import android.graphics.Insets;
import android.graphics.Rect;
import android.os.Build;
import android.os.LocaleList;
Expand Down Expand Up @@ -296,6 +298,10 @@ protected void onSizeChanged(int width, int height, int oldWidth, int oldHeight)
@Override
@TargetApi(20)
@RequiresApi(20)
// The annotations to suppress "InlinedApi" and "NewApi" lints prevent lint warnings
// caused by usage of Android Q APIs. These calls are safe because they are
// guarded.
@SuppressLint({"InlinedApi", "NewApi"})
@NonNull
public final WindowInsets onApplyWindowInsets(@NonNull WindowInsets insets) {
WindowInsets newInsets = super.onApplyWindowInsets(insets);
Expand All @@ -312,11 +318,21 @@ public final WindowInsets onApplyWindowInsets(@NonNull WindowInsets insets) {
viewportMetrics.viewInsetBottom = insets.getSystemWindowInsetBottom();
viewportMetrics.viewInsetLeft = 0;

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
Copy link
Contributor

@mklim mklim Aug 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it be worth it to still add a test for the non Q case in this PR where these are set to zero? It would just basically be verifying the current behavior and making sure that these lines were guarded and didn't cause problems on lower SDK versions. So some but not a huge amount of value from them.

Copy link
Author

@shihaohong shihaohong Aug 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the root cause of the problem, but I believe that because the TargetApi is set to 20 and the tests only use up to SDK 16 at runtime, the onApplyWindowInsets method gets completely ignored by the test framework (I tried putting print statements in the method and they weren't being called). So, even this simple case might be blocked on upgrading Robolectric :\

Copy link
Contributor

@mklim mklim Aug 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. The entire method here requires at least API 20, and our setup only includes API 16 right now.

If you wanted to test just the non-Q parts of this method, you should be able to bring in API 21 without any trouble. Robolectric has supported it since 3.0 (we're on 3.8). It's 29/Q specifically that required upgrading to 4.3 and is currently blocked. But doing this has some extra overhead where it would require bringing in another robolectric android package for the right SDK (like robolectric.android-all:5.0.0_r2-robolectric-0) and adding it to CIPD as described here. So at that point I'm not sure if it's worth the hassle. There may be some unknown unknowns that cause it to take extra time.

Insets systemGestureInsets = insets.getSystemGestureInsets();
viewportMetrics.systemGestureInsetTop = systemGestureInsets.top;
viewportMetrics.systemGestureInsetRight = systemGestureInsets.right;
viewportMetrics.systemGestureInsetBottom = systemGestureInsets.bottom;
viewportMetrics.systemGestureInsetLeft = systemGestureInsets.left;
}

Log.v(TAG, "Updating window insets (onApplyWindowInsets()):\n"
+ "Status bar insets: Top: " + viewportMetrics.paddingTop
+ ", Left: " + viewportMetrics.paddingLeft + ", Right: " + viewportMetrics.paddingRight + "\n"
+ "Keyboard insets: Bottom: " + viewportMetrics.viewInsetBottom
+ ", Left: " + viewportMetrics.viewInsetLeft + ", Right: " + viewportMetrics.viewInsetRight);
+ ", Left: " + viewportMetrics.viewInsetLeft + ", Right: " + viewportMetrics.viewInsetRight
+ "System Gesture Insets - Left: " + viewportMetrics.systemGestureInsetLeft + ", Top: " + viewportMetrics.systemGestureInsetTop
+ ", Right: " + viewportMetrics.systemGestureInsetRight + ", Bottom: " + viewportMetrics.viewInsetBottom);

sendViewportMetricsToFlutter();

Expand Down
18 changes: 15 additions & 3 deletions shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,11 @@ public void setViewportMetrics(
int physicalViewInsetTop,
int physicalViewInsetRight,
int physicalViewInsetBottom,
int physicalViewInsetLeft
int physicalViewInsetLeft,
int systemGestureInsetTop,
int systemGestureInsetRight,
int systemGestureInsetBottom,
int systemGestureInsetLeft
) {
ensureRunningOnMainThread();
ensureAttachedToNative();
Expand All @@ -372,7 +376,11 @@ public void setViewportMetrics(
physicalViewInsetTop,
physicalViewInsetRight,
physicalViewInsetBottom,
physicalViewInsetLeft
physicalViewInsetLeft,
systemGestureInsetTop,
systemGestureInsetRight,
systemGestureInsetBottom,
systemGestureInsetLeft
);
}

Expand All @@ -388,7 +396,11 @@ private native void nativeSetViewportMetrics(
int physicalViewInsetTop,
int physicalViewInsetRight,
int physicalViewInsetBottom,
int physicalViewInsetLeft
int physicalViewInsetLeft,
int systemGestureInsetTop,
int systemGestureInsetRight,
int systemGestureInsetBottom,
int systemGestureInsetLeft
);
//----- End Render Surface Support -----

Expand Down
14 changes: 12 additions & 2 deletions shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,9 @@ public void setViewportMetrics(@NonNull ViewportMetrics viewportMetrics) {
+ "Padding - L: " + viewportMetrics.paddingLeft + ", T: " + viewportMetrics.paddingTop
+ ", R: " + viewportMetrics.paddingRight + ", B: " + viewportMetrics.paddingBottom + "\n"
+ "Insets - L: " + viewportMetrics.viewInsetLeft + ", T: " + viewportMetrics.viewInsetTop
+ ", R: " + viewportMetrics.viewInsetRight + ", B: " + viewportMetrics.viewInsetBottom);
+ ", R: " + viewportMetrics.viewInsetRight + ", B: " + viewportMetrics.viewInsetBottom + "\n"
+ "System Gesture Insets - L: " + viewportMetrics.systemGestureInsetLeft + ", T: " + viewportMetrics.systemGestureInsetTop
+ ", R: " + viewportMetrics.systemGestureInsetRight + ", B: " + viewportMetrics.viewInsetBottom);

flutterJNI.setViewportMetrics(
viewportMetrics.devicePixelRatio,
Expand All @@ -215,7 +217,11 @@ public void setViewportMetrics(@NonNull ViewportMetrics viewportMetrics) {
viewportMetrics.viewInsetTop,
viewportMetrics.viewInsetRight,
viewportMetrics.viewInsetBottom,
viewportMetrics.viewInsetLeft
viewportMetrics.viewInsetLeft,
viewportMetrics.systemGestureInsetTop,
viewportMetrics.systemGestureInsetRight,
viewportMetrics.systemGestureInsetBottom,
viewportMetrics.systemGestureInsetLeft
);
}

Expand Down Expand Up @@ -346,5 +352,9 @@ public static final class ViewportMetrics {
public int viewInsetRight = 0;
public int viewInsetBottom = 0;
public int viewInsetLeft = 0;
public int systemGestureInsetTop = 0;
public int systemGestureInsetRight = 0;
public int systemGestureInsetBottom = 0;
public int systemGestureInsetLeft = 0;
}
}
Loading