[Android] Add display corner radii support.#179219
Conversation
There was a problem hiding this comment.
Code Review
This PR adds support for display corner radii on Android. The changes span across the engine and framework to expose this new information. The implementation looks solid, with new APIs in dart:ui and MediaQuery to access the corner radii. Tests have been added to cover the new functionality. I've found a critical issue in the copyWith method in MediaQueryData that will cause a compilation error, and another suggestion to improve API consistency. Please see my detailed comments.
mdebbar
left a comment
There was a problem hiding this comment.
The web stubs look good to me!
mboetger
left a comment
There was a problem hiding this comment.
Android-side plumbing looks good. One question below. Also we'll need someone from the framework to look at this, I believe.
mboetger
left a comment
There was a problem hiding this comment.
LGTM from the Android side - please wait to submit until you get LGTMs from the web and framework side though.
|
I know why the Google testing is failing - I can fix it. I had to make a similar change in #176063 in the JNI (will need to rebase it if that one lands first). |
Thanks a lot for taking a look at the Google Testing failure! |
Co-authored-by: Matt Boetger <[email protected]>
426843b to
d194b89
Compare
|
@mboetger, could you please take a look at why Google Testing failed? |
Working on it. |
|
Hi @justinmc, thanks a lot for your review!
As was discussed in #97349, the iOS API for getting display corner radii is private. The only safe way to get the radii values is to generate a lookup table ahead and use its values at runtime. I would be happy to work on the iOS implementation in a separate PR after this lands.
I don't have a physical foldable device, but I tried to run a demo on the Code sampleimport 'package:flutter/material.dart';
void main() {
runApp(const App());
}
class App extends StatelessWidget {
const App({super.key});
@override
Widget build(BuildContext context) {
return const MaterialApp(
debugShowCheckedModeBanner: false,
home: Screen(),
);
}
}
class Screen extends StatefulWidget {
const Screen({super.key});
@override
State<Screen> createState() => _ScreenState();
}
class _ScreenState extends State<Screen> with SingleTickerProviderStateMixin {
@override
Widget build(BuildContext context) {
print('dpr: ${MediaQuery.devicePixelRatioOf(context)}');
return Scaffold(
body: Padding(
padding: const EdgeInsets.all(20),
child: Center(
child: DisplayCornerRadiiReactor(),
),
),
);
}
}
class DisplayCornerRadiiReactor extends StatefulWidget {
const DisplayCornerRadiiReactor({super.key});
@override
State<DisplayCornerRadiiReactor> createState() =>
_DisplayCornerRadiiReactorState();
}
class _DisplayCornerRadiiReactorState extends State<DisplayCornerRadiiReactor> {
@override
Widget build(BuildContext context) {
return Column(
mainAxisSize: MainAxisSize.min,
spacing: 20,
children: [
Text(
'displayCornerRadii: ${View.of(context).displayCornerRadii}',
textAlign: TextAlign.center,
style: TextStyle(fontSize: 20),
),
Text(
'displayCornerRadii: ${MediaQuery.displayCornerRadiiOf(context)}',
textAlign: TextAlign.center,
style: TextStyle(fontSize: 20),
),
],
);
}
}
foldable.mp4 |
justinmc
left a comment
There was a problem hiding this comment.
LGTM 👍 . Thanks for the followup. Let's wait for a web review and then this is good to go.
| 0.0, // maxWidth | ||
| 0.0, // minHeight | ||
| 0.0, // maxHeight | ||
| -1.0, // display corner radius top left |
There was a problem hiding this comment.
Got it, thanks for reassuring me 👍
| @override | ||
| String toString() { |
| int get hashCode => Object.hash(topLeft, topRight, bottomRight, bottomLeft); | ||
|
|
||
| @override | ||
| String toString() { |
There was a problem hiding this comment.
This is not possible as discussed elsewhere.
|
@justinmc thanks for your review! |
|
Ah I missed that, sorry 😅 . I reran one test that appeared to be a flake, otherwise this is ready to land. |
|
Thanks a lot, everyone, for your reviews and feedback! |
Closes flutter#97349 Unblocks flutter#178463 ### Description This PR focuses only on the Android side. As was discussed in flutter#97349, the iOS API for getting display corner radii is private. The only safe way to get the radii values is to generate a lookup table ahead and use its values at runtime. I would be happy to work on the iOS implementation in a separate PR after this lands. - Adds `displayCornerRadii` in the physical pixels to the engine's window metrics - Adds `displayCornerRadii` support on the Android API 31+ - Adds `displayCornerRadii` in the logical pixels to the `MediaQuery` - Adds and updates tests ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Matt Boetger <[email protected]>
refer to commit:
697572a("[Android] Add display corner radii support. (flutter#179219)")
Signed-off-by: Yongqin Liu <[email protected]>
Closes #97349
Unblocks #178463
Description
This PR focuses only on the Android side.
As was discussed in #97349, the iOS API for getting display corner radii is private. The only safe way to get the radii values is to generate a lookup table ahead and use its values at runtime. I would be happy to work on the iOS implementation in a separate PR after this lands.
displayCornerRadiiin the physical pixels to the engine's window metricsdisplayCornerRadiisupport on the Android API 31+displayCornerRadiiin the logical pixels to theMediaQueryPre-launch Checklist
///).