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
[framework] re-rasterize when window size or insets changes #113647
[framework] re-rasterize when window size or insets changes #113647
Conversation
| // If the screen size changes during the transition, perhaps due to | ||
| // a keyboard dismissal, then ensure that contents are re-rasterized once. | ||
| final MediaQueryData data = MediaQuery.of(context); | ||
| if (mediaQueryData != null && data.size != mediaQueryData!.size) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might could inline the MediaQuery.of call so in the case where medaQueryData IS null, you avoid the instantiation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its really quite difficult to live without a media query, so we'll almost always want to create one and then cache it. Which I of course forgot to do...
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Golden file changes are available for triage from new commit, Click here to view. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| // If the screen size changes during the transition, perhaps due to | ||
| // a keyboard dismissal, then ensure that contents are re-rasterized once. | ||
| final MediaQueryData? data = MediaQuery.maybeOf(context); | ||
| if (mediaQueryData?.size != data?.size) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if we only care about size, just cache the size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I should probably know this because I worked on it at one point, but isn't the IME "just" changing the insets/padding values and not the size?
I tried to come up with an IME based reproduction for the internal bug that could be used in open source and failed, but I hadn't spent very long on it. If you've confirmed this fixes the internal bug that's cool, but if it hasn't been confirmed internally it may be worth looking into and figuring out if we're being affected by insets/padding rather than size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://api.flutter.dev/flutter/widgets/MediaQueryData-class.html - I'm pretty sure it's not the size that'll get impacted here, but the viewInsets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, converting to draft for now. I think we should probably handle any of size and/or view insets. But particularly we care about the render object size, so maybe I should just check there instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, we could just clear whenever anything on the media query changes, which runs a small risk of changing for things not visually impacting the page size.
|
Yeah, thanks @dnfield it was view insets Repro app: // Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart';
void main() {
timeDilation = 10;
runApp(MaterialApp(
home: Example(),
routes: {
'/b': (context) => B(),
},
));
}
class Example extends StatefulWidget {
const Example({super.key});
@override
State<Example> createState() => _ExampleState();
}
class _ExampleState extends State<Example> {
@override
Widget build(BuildContext context) {
return Scaffold(
body: Container(
color: Colors.red,
child: Center(
child: TextButton(
child: Text('Forward'),
onPressed: () {
Navigator.of(context).pushNamed('/b');
},
),
),
),
);
}
}
class B extends StatelessWidget {
const B({super.key});
@override
Widget build(BuildContext context) {
return Scaffold(
body: Container(
color: Colors.blue,
child: Column(
children: [
TextField(),
TextButton(
child: Text('BACK', style: TextStyle(color: Colors.black),),
onPressed: () {
Navigator.of(context).pop();
},
)
],
),
),
);
}
}
|


When navigating to a new page, if the keyboard is dismissed partially through the navigation, the page view will be too small. Track the widow size and re-rasterize if it changes during the transition.
b/250815292