[defunct] MediaQuery as InheritedModel #97928

moffatman · 2022-02-07T04:51:26Z

When some aspect of the flutter MediaQuery changes, every widget which depends on MediaQuery.of(context) is rebuilt. Most of these rebuilds are unnecessary, since most changes to MediaQuery do not change more than 1 or 2 fields at a time (window resizing, system brightness changing, system font scale changing).

MediaQuery does not change often on mobile, but resizing desktop windows happens often and flutter should be able to handle being resized without dropping frames. There is one place that MediaQuery is rapidly updated on mobile: on iOS, as the keyboard opens, MediaQuery is updated every frame with the only change being the keyboard insets.

Here is a demo program, resizing the window causes MyHomePage to rebuild needlessly. The issue here could be alleviated by better Widget encapsulation, but many framework widgets are rebuilt on every change in MediaQuery when they depend on rarely-changing properties such as platformBrightness or textScaleFactor.

import 'package:flutter/cupertino.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return const CupertinoApp(
      title: 'Flutter Demo',
      home: MyHomePage()
    );
  }
}

class MyHomePage extends StatelessWidget {
  const MyHomePage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Container(
      color: CupertinoDynamicColor.resolve(CupertinoColors.systemFill, context),
      child: ListView(
        children: <Widget>[
          // Some expensive build function
          for (int i = 0; i < 100000; i++) Text('Item $i', textAlign: TextAlign.center)
        ]
      )
    );
  }
}

This change makes MediaQuery an InheritedModel rather than an InheritedWidget, so any widget which knows it only depends on a specific property of MediaQuery the ability to declare that when reading the MediaQuery from the context.

For example,

MediaQuery.of(context).platformBrightness

could become

MediaQuery.of(context, MediaQueryAspect.platformBrightness).platformBrightness

as a performance optimization.

MediaQuery.of(context) will continue to work as it always has.

Fixes #102060

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
All existing and new tests are passing.

skia-gold · 2022-03-11T23:33:05Z

Gold has detected about 1 new digest(s) on patchset 2.
View them at https://flutter-gold.skia.org/cl/github/97928

b3nni97 · 2022-04-18T00:55:51Z

Is this still being worked on?

moffatman · 2022-04-18T04:43:01Z

I think I need to make an issue so that it will be reviewed. Will do that.

nikitadol · 2022-04-20T07:18:15Z

I propose a more universal solution:
Add a function select (which will be caught by InheritedWidget or InheritedModel), as was done in the package provider

https://github.com/rrousselGit/provider/blob/master/lib/src/inherited_provider.dart#L246

Since it will be useful not only for MediaQuery

b3nni97 · 2022-04-28T21:30:16Z

Can anyone here review the PR? This improves the performance of many apps.

FelixMittermeier · 2022-05-01T10:49:31Z

This looks very promising 👍🏻 Thanks for your work.
These kind of performance improvements are always great

aurangzaibsiddiqui · 2022-06-11T21:30:23Z

Hi,

This can be very useful for my application as I am facing a lot of jank on soft keyboard pop-up. However, I am relatively new to Flutter programming. Can someone please guide me how I can use @moffatman's solution?

As in how do I pull his github into my Flutter code?

b3nni97 · 2022-06-21T00:25:54Z

Can anyone here review this? This is not a breaking change and for all who modify their code a lot of unnecessary rebuilds will be avoided... @Piinks

marctan · 2022-08-18T02:15:21Z

any update on this? still not in stable?

moffatman · 2022-10-28T22:47:48Z

I thought of some default values here. But the more I think about it, I would rather not add new default values, and just keep the minimum compatibility the old behaviour. I don't see the value of hiding this likely accidental state where there is no MediaQuery ancestor.

Property	Possible default value	Notes
size	WidgetsBinding.instance.window.physicalSize
orientation	Could be imputed from size
devicePixelRatio	1.0
textScaleFactor	1.0	🟢 Existing behaviour
platformBrightness	Brightness.light	🟢 Existing behaviour
padding	EdgeInsets.zero
viewInsets	EdgeInsets.zero
systemGestureInsets	EdgeInsets.zero
viewPadding	EdgeInsets.zero
alwaysUse24HourFormat	false
accessibleNavigation	false
invertColors	false
highContrast	false	🟢 Existing behaviour
disableAnimations	false
boldTextOverride	false	🟠 There is already `MediaQuery.boldTextOverride` (notice no `of` in the name), which has a default return value of false. Would like to deprecate that one and rename to match as `MediaQuery.boldTextOverrideOf`. Not sure if it's accepted to deprecated and rename just to match other names?
navigationMode	NavigationMode.traditional
gestureSettings	const DeviceGestureSettings()
displayFeatures	const []

packages/flutter/lib/src/cupertino/desktop_text_selection.dart

packages/flutter/lib/src/material/desktop_text_selection.dart

packages/flutter/test/cupertino/text_selection_toolbar_test.dart

packages/flutter/lib/src/widgets/media_query.dart

goderbauer · 2022-10-28T23:21:20Z

I would rather not add new default values, and just keep the minimum compatibility the old behaviour.

I agree with this.

goderbauer

LGTM

@gspencergoog FYI since you're currently working on unifying the ofs and maybeOfs.

packages/flutter/lib/src/widgets/media_query.dart

packages/flutter/lib/src/material/desktop_text_selection.dart

packages/flutter/lib/src/material/drawer.dart

packages/flutter/lib/src/material/scaffold.dart

gspencergoog · 2022-10-31T17:14:46Z

packages/flutter/lib/src/widgets/overscroll_indicator.dart

I wonder if it makes sense for size to be something where you can depend on just the width or just the height changing? I think most properties it doesn't make sense to go down another level like that, but size is something that can have huge redraw implications, and if we can avoid rebuilding a widget when the height changes and the widget only cares about width (like in a horizontal widget during keyboard animation), that could be good.

There was a discussion about this above and we decided against exposing sub-properties for now.

You could possible have like 18 more methods including topPaddingOf, leftViewInsetsOf... I see the value but let's hold on that for now.

Ahh, OK. I missed that discussion.

flutter-dashboard · 2022-11-01T06:11:53Z

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 package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #97928 at sha 455de13

goderbauer · 2022-11-01T22:43:10Z

@moffatman Sorry for the hassle, but could you close this PR and open a new one with the same change? It looks like since this PR is so old our golden testing infra got confused by it and is in a corrupted state. Opening this as a new PR should resolve the issue.

moffatman force-pushed the mediaquery branch 2 times, most recently from dadf6d6 to 1a2f307 Compare February 12, 2022 02:19

moffatman force-pushed the mediaquery branch from 1a2f307 to 2cadd44 Compare February 18, 2022 02:52

moffatman force-pushed the mediaquery branch from 2cadd44 to 33f72d7 Compare February 25, 2022 21:26

moffatman force-pushed the mediaquery branch from 33f72d7 to 5246dd2 Compare March 11, 2022 23:14

moffatman force-pushed the mediaquery branch from 5246dd2 to b48cf73 Compare March 29, 2022 22:42

moffatman force-pushed the mediaquery branch 2 times, most recently from ea50c52 to 66bc46a Compare April 18, 2022 05:34

moffatman mentioned this pull request Apr 18, 2022

Should be able to subscribe to changes on properties of MediaQuery instead of all changes #102060

Closed

moffatman force-pushed the mediaquery branch from 66bc46a to 2ca57c0 Compare May 2, 2022 21:46

moffatman force-pushed the mediaquery branch from 2ca57c0 to d1efccb Compare May 14, 2022 17:06

moffatman force-pushed the mediaquery branch from d1efccb to 1c304c4 Compare June 3, 2022 03:35

ZRunner mentioned this pull request Jun 28, 2022

Provide more control on when queries are executed zino-hofmann/graphql-flutter#1168

Closed

moffatman force-pushed the mediaquery branch from 1c304c4 to 225b44d Compare July 25, 2022 15:11

b3nni97 mentioned this pull request Jul 26, 2022

Multiple Rebuild issue on opening a keyboard #105184

Open

moffatman force-pushed the mediaquery branch from 4ca3742 to 2b52d51 Compare October 28, 2022 21:13

goderbauer reviewed Oct 28, 2022

View reviewed changes

goderbauer approved these changes Oct 31, 2022

View reviewed changes

packages/flutter/lib/src/widgets/media_query.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/widgets/media_query.dart Outdated Show resolved Hide resolved

gspencergoog reviewed Oct 31, 2022

View reviewed changes

moffatman added 12 commits November 1, 2022 01:36

Subscribe to specific properties of MediaQuery

c6e2526

Address feedback

0fb9064

Update for recent mediaquery usages after rebase

bdf5c47

Try making MediaQueryAspect private

cc058b6

Remove trailing spaces

d838ae8

Go back to public MediaQueryAspect

0b72c6d

Fix test failures

952c4b7

Add fix for boldTextOverride deprecation

d655c96

Switch back to _MediaQueryAspect, address feedback

b3824e2

Some broken fix test slipped through CI?

186e3d3

Feedback 2

0e81794

Convert another MediaQuery.of(context)

455de13

moffatman force-pushed the mediaquery branch from f0176c5 to 455de13 Compare November 1, 2022 05:39

flutter-dashboard bot added the will affect goldens Changes to golden files label Nov 1, 2022

moffatman changed the title ~~MediaQuery as InheritedModel~~ [defunct] MediaQuery as InheritedModel Nov 1, 2022

moffatman mentioned this pull request Nov 1, 2022

MediaQuery as InheritedModel #114459

Merged

8 tasks

moffatman closed this Nov 1, 2022

moffatman mentioned this pull request Nov 3, 2022

WidgetController.startGesture trackpad support #114312

Closed

8 tasks

[defunct] MediaQuery as InheritedModel #97928

[defunct] MediaQuery as InheritedModel #97928

Uh oh!

Conversation

moffatman commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

skia-gold commented Mar 11, 2022

Uh oh!

b3nni97 commented Apr 18, 2022

Uh oh!

moffatman commented Apr 18, 2022

Uh oh!

nikitadol commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

b3nni97 commented Apr 28, 2022

Uh oh!

FelixMittermeier commented May 1, 2022

Uh oh!

aurangzaibsiddiqui commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

b3nni97 commented Jun 21, 2022

Uh oh!

marctan commented Aug 18, 2022

Uh oh!

moffatman commented Oct 28, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

goderbauer commented Oct 28, 2022

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gspencergoog Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

goderbauer Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

moffatman Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

gspencergoog Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Nov 1, 2022

Uh oh!

goderbauer commented Nov 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

moffatman commented Feb 7, 2022 •

edited

Loading

nikitadol commented Apr 20, 2022 •

edited

Loading

aurangzaibsiddiqui commented Jun 11, 2022 •

edited

Loading