-
Notifications
You must be signed in to change notification settings - Fork 28.5k
use immutable buffer for loading asset images #103496
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
use immutable buffer for loading asset images #103496
Conversation
dev/automated_tests/pubspec.yaml
Outdated
@@ -70,6 +70,6 @@ dependencies: | |||
flutter: | |||
uses-material-design: true | |||
assets: | |||
- icon/ | |||
- icon/test.png |
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.
This was indented wrong and never loaded the asset
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.
Why didn't the associated test above fail, then? It seems to claim that it tests whether the asset loads....
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.
The previously used asset loader workaround does not throw errors in the same way as the regular asset loader: https://github.com/flutter/flutter/blob/master/packages/flutter_test/lib/src/_binding_io.dart#L34
…er into use_immutable_buffer
TBD whether or not we should leave the old methods implemented. I intended to do further testing to see if there are any places in google that uses the load method directly |
Loading asset images via the immutable buffer should both improve loading performance by removing a copy and reduce GC pressure by avoiding a round-trip of the compressed image bytes through the dart heap.
This is the start of a breaking change, this PR is intended to be non-breaking (though it currently isn't, for testing purposes)
To make this change less-breaking, a default implementation of the loadBuffer is added to imageProvider, that returns a const instance of a no-op image stream. We detect this case and fallback to the old load method conditionally, allowing image providers time to migrate.