Reland "Speed up first asset load by using the binary-formatted asset manifest for image resolution#121322
Merged
auto-submit[bot] merged 14 commits intoflutter:masterfrom Mar 6, 2023
Conversation
5244790 to
e00f17a
Compare
…-asset-manifest-for-asset-resolution
This reverts commit e69cee3.
…-asset-manifest-for-asset-resolution
jonahwilliams
approved these changes
Mar 6, 2023
Contributor
jonahwilliams
left a comment
There was a problem hiding this comment.
The changes to the asset loading API sound reasonable.
LGTM!
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 8, 2023
…ed asset manifest for image resolution (flutter/flutter#121322)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 8, 2023
…ed asset manifest for image resolution (flutter/flutter#121322)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Mar 8, 2023
…ed asset manifest for image resolution (flutter/flutter#121322)
11 tasks
8 tasks
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 10, 2023
…ed asset manifest for image resolution (flutter/flutter#121322)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 10, 2023
…ed asset manifest for image resolution (flutter/flutter#121322)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relands #118782 with some changes.
The original PR added a behavior change--when trying to load an asset and we don't find its key in the asset manifest file, we would throw an error (code). The idea was that this would provide a more useful error message than the one that would get shown when the image inevitably fails to load (code). However, this caused the
errorBuilderfunctionality ofImageto not work in the case of a missing asset (this was revealed by an internal test breakage). Fixing this appears non-trivial, so I think it's better to scope this out of this PR and perhaps look at it again later.This also makes a small update to the recently-added
AssetManifest.getAssetVariantsAPI (still in beta). When given a key that is not found within the asset manifest file, the method will returnnullinstead of throwing an error. Throwing an error is a bit too opinionated for an API that's just meant to provide read-access to the asset manifest file. The tradeoff is that callers will have to do null checking/assertions.Let me know if you feel strongly that this should be its own PR.
Pre-launch Checklist
///).