Skip to content

Conversation

@andrewkolos
Copy link
Contributor

Cherry-picks #128143 onto stable

…ts exist (flutter#128143)

Fixes flutter#127090.

flutter#122505 did a few things to speed up the first asset load that a flutter app performs. One of those things was to not include the main asset in its own list of variants in the asset manifest. The idea was that we know that the main asset always exists, so including it in its list of variants is a waste of storage space and loading time (even if the cost was tiny).

However, the assumption that the main asset always exists is wrong. From [Declaring resolution-aware image assets](https://docs.flutter.dev/ui/assets-and-images#resolution-aware), which predates flutter#122505:

> Each entry in the asset section of the pubspec.yaml should correspond to a real file, with the exception of the main asset entry. If the main asset entry doesn�t correspond to a real file, then the asset with the lowest resolution is used as the fallback for devices with device pixel ratios below that resolution. The entry should still be included in the pubspec.yaml manifest, however.

For example, it's valid to declare `assets/image.png` as an asset even if only `assets/3x/image.png` exists on disk.

This fix restores older behavior of including a main asset as a variant of itself in the manifest if it exists.

This fix also includes a non-user-visible behavior change:
* `"dpr"` is no longer a required field in the asset manifest's underlying structure. For the main asset entry, we do not include `"dpr"`. It makes less sense for the tool to decide what the default target dpr for an image should be. This should be left to the framework.
@flutter-dashboard flutter-dashboard bot added framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels. labels Jun 7, 2023
@andrewkolos andrewkolos marked this pull request as ready for review June 7, 2023 18:28
@andrewkolos andrewkolos requested a review from jonahwilliams June 7, 2023 18:30
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

@jonahwilliams
Copy link
Contributor

Please file a separate CP request too

@andrewkolos
Copy link
Contributor Author

Please file a separate CP request too

#128450

@andrewkolos andrewkolos added the cp: review Cherry-picks in the review queue label Jun 8, 2023
@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 9, 2023
@auto-submit auto-submit bot merged commit e05d997 into flutter:flutter-3.10-candidate.1 Jun 9, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 14, 2023
@andrewkolos andrewkolos deleted the cp-fix-nonexistant-main-asset-case branch June 15, 2023 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App cp: review Cherry-picks in the review queue framework flutter/packages/flutter repository. See also f: labels. tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants