Speed up first asset load by using the binary-formatted asset manifest for image resolution (flutter#118782)

andrewkolos · web-flow · commit e3db0488adaf · 2023-02-21T16:05:12.000Z
* add asset manifest bin loading and asset manifest api

* use new api for image resolution

* remove upfront smc data casting

* fix typecasting issue

* remove unused import

* fix tests

* lints

* lints

* fix import

* fix outdated type name

* restore AssetManifest docstrings

* update test

* update other test

* make error message for invalid keys more useful
diff --git a/dev/integration_tests/ui/test/asset_test.dart b/dev/integration_tests/ui/test/asset_test.dart
@@ -14,6 +14,6 @@ void main() {
 
     // If this asset couldn't be loaded, the exception message would be
     // "asset failed to load"
-    expect(tester.takeException().toString(), contains('Invalid image data'));
+    expect(tester.takeException().toString(), contains('The key was not found in the asset manifest'));
   });
 }
diff --git a/packages/flutter/lib/src/painting/image_resolution.dart b/packages/flutter/lib/src/painting/image_resolution.dart
@@ -4,15 +4,12 @@
 
 import 'dart:async';
 import 'dart:collection';
-import 'dart:convert';
 
 import 'package:flutter/foundation.dart';
 import 'package:flutter/services.dart';
 
 import 'image_provider.dart';
 
-const String _kAssetManifestFileName = 'AssetManifest.json';
-
 /// A screen with a device-pixel ratio strictly less than this value is
 /// considered a low-resolution screen (typically entry-level to mid-range
 /// laptops, desktop screens up to QHD, low-end tablets such as Kindle Fire).
@@ -284,18 +281,18 @@ class AssetImage extends AssetBundleImageProvider {
     Completer<AssetBundleImageKey>? completer;
     Future<AssetBundleImageKey>? result;
 
-    chosenBundle.loadStructuredData<Map<String, List<String>>?>(_kAssetManifestFileName, manifestParser).then<void>(
-      (Map<String, List<String>>? manifest) {
-        final String chosenName = _chooseVariant(
+    AssetManifest.loadFromAssetBundle(chosenBundle)
+      .then((AssetManifest manifest) {
+        final Iterable<AssetMetadata> candidateVariants = _getVariants(manifest, keyName);
+        final AssetMetadata chosenVariant = _chooseVariant(
           keyName,
           configuration,
-          manifest == null ? null : manifest[keyName],
-        )!;
-        final double chosenScale = _parseScale(chosenName);
+          candidateVariants,
+        );
         final AssetBundleImageKey key = AssetBundleImageKey(
           bundle: chosenBundle,
-          name: chosenName,
-          scale: chosenScale,
+          name: chosenVariant.key,
+          scale: chosenVariant.targetDevicePixelRatio ?? _naturalResolution,
         );
         if (completer != null) {
           // We already returned from this function, which means we are in the
@@ -309,14 +306,15 @@ class AssetImage extends AssetBundleImageProvider {
           // ourselves.
           result = SynchronousFuture<AssetBundleImageKey>(key);
         }
-      },
-    ).catchError((Object error, StackTrace stack) {
-      // We had an error. (This guarantees we weren't called synchronously.)
-      // Forward the error to the caller.
-      assert(completer != null);
-      assert(result == null);
-      completer!.completeError(error, stack);
-    });
+      })
+      .onError((Object error, StackTrace stack) {
+        // We had an error. (This guarantees we weren't called synchronously.)
+        // Forward the error to the caller.
+        assert(completer != null);
+        assert(result == null);
+        completer!.completeError(error, stack);
+      });
+
     if (result != null) {
       // The code above ran synchronously, and came up with an answer.
       // Return the SynchronousFuture that we created above.
@@ -328,35 +326,34 @@ class AssetImage extends AssetBundleImageProvider {
     return completer.future;
   }
 
-  /// Parses the asset manifest string into a strongly-typed map.
-  @visibleForTesting
-  static Future<Map<String, List<String>>?> manifestParser(String? jsonData) {
-    if (jsonData == null) {
-      return SynchronousFuture<Map<String, List<String>>?>(null);
+  Iterable<AssetMetadata> _getVariants(AssetManifest manifest, String key) {
+    try {
+      return manifest.getAssetVariants(key);
+    } catch (e) {
+      throw FlutterError.fromParts(<DiagnosticsNode>[
+        ErrorSummary('Unable to load asset with key "$key".'),
+        ErrorDescription(
+'''
+The key was not found in the asset manifest.
+Make sure the key is correct and the appropriate file or folder is specified in pubspec.yaml.
+'''),
+      ]);
     }
-    // TODO(ianh): JSON decoding really shouldn't be on the main thread.
-    final Map<String, dynamic> parsedJson = json.decode(jsonData) as Map<String, dynamic>;
-    final Iterable<String> keys = parsedJson.keys;
-    final Map<String, List<String>> parsedManifest = <String, List<String>> {
-      for (final String key in keys) key: List<String>.from(parsedJson[key] as List<dynamic>),
-    };
-    // TODO(ianh): convert that data structure to the right types.
-    return SynchronousFuture<Map<String, List<String>>?>(parsedManifest);
   }
 
-  String? _chooseVariant(String main, ImageConfiguration config, List<String>? candidates) {
-    if (config.devicePixelRatio == null || candidates == null || candidates.isEmpty) {
-      return main;
+  AssetMetadata _chooseVariant(String mainAssetKey, ImageConfiguration config, Iterable<AssetMetadata> candidateVariants) {
+    if (config.devicePixelRatio == null || candidateVariants.isEmpty) {
+      return candidateVariants.firstWhere((AssetMetadata variant) => variant.main);
     }
-    // TODO(ianh): Consider moving this parsing logic into _manifestParser.
-    final SplayTreeMap<double, String> mapping = SplayTreeMap<double, String>();
-    for (final String candidate in candidates) {
-      mapping[_parseScale(candidate)] = candidate;
+    final SplayTreeMap<double, AssetMetadata> candidatesByDevicePixelRatio =
+      SplayTreeMap<double, AssetMetadata>();
+    for (final AssetMetadata candidate in candidateVariants) {
+      candidatesByDevicePixelRatio[candidate.targetDevicePixelRatio ?? _naturalResolution] = candidate;
     }
     // TODO(ianh): implement support for config.locale, config.textDirection,
     // config.size, config.platform (then document this over in the Image.asset
     // docs)
-    return _findBestVariant(mapping, config.devicePixelRatio!);
+    return _findBestVariant(candidatesByDevicePixelRatio, config.devicePixelRatio!);
   }
 
   // Returns the "best" asset variant amongst the available `candidates`.
@@ -371,48 +368,28 @@ class AssetImage extends AssetBundleImageProvider {
   //   lowest key higher than `value`.
   // - If the screen has high device pixel ratio, choose the variant with the
   //   key nearest to `value`.
-  String? _findBestVariant(SplayTreeMap<double, String> candidates, double value) {
-    if (candidates.containsKey(value)) {
-      return candidates[value]!;
+  AssetMetadata _findBestVariant(SplayTreeMap<double, AssetMetadata> candidatesByDpr, double value) {
+    if (candidatesByDpr.containsKey(value)) {
+      return candidatesByDpr[value]!;
     }
-    final double? lower = candidates.lastKeyBefore(value);
-    final double? upper = candidates.firstKeyAfter(value);
+    final double? lower = candidatesByDpr.lastKeyBefore(value);
+    final double? upper = candidatesByDpr.firstKeyAfter(value);
     if (lower == null) {
-      return candidates[upper];
+      return candidatesByDpr[upper]!;
     }
     if (upper == null) {
-      return candidates[lower];
+      return candidatesByDpr[lower]!;
     }
 
     // On screens with low device-pixel ratios the artifacts from upscaling
     // images are more visible than on screens with a higher device-pixel
     // ratios because the physical pixels are larger. Choose the higher
     // resolution image in that case instead of the nearest one.
     if (value < _kLowDprLimit || value > (lower + upper) / 2) {
-      return candidates[upper];
+      return candidatesByDpr[upper]!;
     } else {
-      return candidates[lower];
-    }
-  }
-
-  static final RegExp _extractRatioRegExp = RegExp(r'/?(\d+(\.\d*)?)x$');
-
-  double _parseScale(String key) {
-    if (key == assetName) {
-      return _naturalResolution;
-    }
-
-    final Uri assetUri = Uri.parse(key);
-    String directoryPath = '';
-    if (assetUri.pathSegments.length > 1) {
-      directoryPath = assetUri.pathSegments[assetUri.pathSegments.length - 2];
-    }
-
-    final Match? match = _extractRatioRegExp.firstMatch(directoryPath);
-    if (match != null && match.groupCount > 0) {
-      return double.parse(match.group(1)!);
+      return candidatesByDpr[lower]!;
     }
-    return _naturalResolution; // i.e. default to 1.0x
   }
 
   @override
diff --git a/packages/flutter/lib/src/services/asset_manifest.dart b/packages/flutter/lib/src/services/asset_manifest.dart
@@ -71,7 +71,7 @@ class _AssetManifestBin implements AssetManifest {
     if (!_typeCastedData.containsKey(key)) {
       final Object? variantData = _data[key];
       if (variantData == null) {
-        throw ArgumentError('Asset key $key was not found within the asset manifest.');
+        throw ArgumentError('Asset key "$key" was not found.');
       }
       _typeCastedData[key] = ((_data[key] ?? <Object?>[]) as Iterable<Object?>)
         .cast<Map<Object?, Object?>>()
diff --git a/packages/flutter/test/painting/image_resolution_test.dart b/packages/flutter/test/painting/image_resolution_test.dart
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:convert';
 import 'dart:ui' as ui;
 
 import 'package:flutter/foundation.dart';
@@ -13,18 +12,14 @@ import 'package:flutter_test/flutter_test.dart';
 class TestAssetBundle extends CachingAssetBundle {
   TestAssetBundle(this._assetBundleMap);
 
-  final Map<String, List<String>> _assetBundleMap;
+  final Map<String, List<Map<Object?, Object?>>> _assetBundleMap;
 
   Map<String, int> loadCallCount = <String, int>{};
 
-  String get _assetBundleContents {
-    return json.encode(_assetBundleMap);
-  }
-
   @override
   Future<ByteData> load(String key) async {
-    if (key == 'AssetManifest.json') {
-      return ByteData.view(Uint8List.fromList(const Utf8Encoder().convert(_assetBundleContents)).buffer);
+    if (key == 'AssetManifest.bin') {
+      return const StandardMessageCodec().encodeMessage(_assetBundleMap)!;
     }
 
     loadCallCount[key] = loadCallCount[key] ?? 0 + 1;
@@ -45,9 +40,10 @@ class TestAssetBundle extends CachingAssetBundle {
 void main() {
   group('1.0 scale device tests', () {
     void buildAndTestWithOneAsset(String mainAssetPath) {
-      final Map<String, List<String>> assetBundleMap = <String, List<String>>{};
+      final Map<String, List<Map<dynamic, dynamic>>> assetBundleMap =
+        <String, List<Map<dynamic, dynamic>>>{};
 
-      assetBundleMap[mainAssetPath] = <String>[];
+      assetBundleMap[mainAssetPath] = <Map<Object?, Object?>>[];
 
       final AssetImage assetImage = AssetImage(
         mainAssetPath,
@@ -93,11 +89,13 @@ void main() {
       const String mainAssetPath = 'assets/normalFolder/normalFile.png';
       const String variantPath = 'assets/normalFolder/3.0x/normalFile.png';
 
-      final Map<String, List<String>> assetBundleMap =
-      <String, List<String>>{};
-
-      assetBundleMap[mainAssetPath] = <String>[mainAssetPath, variantPath];
+      final Map<String, List<Map<dynamic, dynamic>>> assetBundleMap =
+        <String, List<Map<dynamic, dynamic>>>{};
 
+      final Map<dynamic, dynamic> mainAssetVariantManifestEntry = <dynamic, dynamic>{};
+      mainAssetVariantManifestEntry['asset'] = variantPath;
+      mainAssetVariantManifestEntry['dpr'] = 3.0;
+      assetBundleMap[mainAssetPath] = <Map<dynamic, dynamic>>[mainAssetVariantManifestEntry];
       final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap);
 
       final AssetImage assetImage = AssetImage(
@@ -123,10 +121,10 @@ void main() {
     test('When high-res device and high-res asset not present in bundle then return main variant', () {
       const String mainAssetPath = 'assets/normalFolder/normalFile.png';
 
-      final Map<String, List<String>> assetBundleMap =
-      <String, List<String>>{};
+      final Map<String, List<Map<dynamic, dynamic>>> assetBundleMap =
+        <String, List<Map<dynamic, dynamic>>>{};
 
-      assetBundleMap[mainAssetPath] = <String>[mainAssetPath];
+      assetBundleMap[mainAssetPath] = <Map<Object?, Object?>>[];
 
       final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap);
 
@@ -162,10 +160,13 @@ void main() {
       double chosenAssetRatio,
       String expectedAssetPath,
     ) {
-      final Map<String, List<String>> assetBundleMap =
-      <String, List<String>>{};
+      final Map<String, List<Map<dynamic, dynamic>>> assetBundleMap =
+        <String, List<Map<dynamic, dynamic>>>{};
 
-      assetBundleMap[mainAssetPath] = <String>[mainAssetPath, variantPath];
+      final Map<dynamic, dynamic> mainAssetVariantManifestEntry = <dynamic, dynamic>{};
+      mainAssetVariantManifestEntry['asset'] = variantPath;
+      mainAssetVariantManifestEntry['dpr'] = 3.0;
+      assetBundleMap[mainAssetPath] = <Map<dynamic, dynamic>>[mainAssetVariantManifestEntry];
 
       final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap);
 
diff --git a/packages/flutter/test/widgets/image_resolution_test.dart b/packages/flutter/test/widgets/image_resolution_test.dart
diff --git a/packages/flutter/test/widgets/image_test.dart b/packages/flutter/test/widgets/image_test.dart

Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,6 @@ void main() {`
`14`	`14`
`15`	`15`	`// If this asset couldn't be loaded, the exception message would be`
`16`	`16`	`// "asset failed to load"`
`17`		`- expect(tester.takeException().toString(), contains('Invalid image data'));`
	`17`	`+ expect(tester.takeException().toString(), contains('The key was not found in the asset manifest'));`
`18`	`18`	`});`
`19`	`19`	`}`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ class _AssetManifestBin implements AssetManifest {`
`71`	`71`	`if (!_typeCastedData.containsKey(key)) {`
`72`	`72`	`final Object? variantData = _data[key];`
`73`	`73`	`if (variantData == null) {`
`74`		`- throw ArgumentError('Asset key $key was not found within the asset manifest.');`
	`74`	`+ throw ArgumentError('Asset key "$key" was not found.');`
`75`	`75`	`}`
`76`	`76`	`_typeCastedData[key] = ((_data[key] ?? <Object?>[]) as Iterable<Object?>)`
`77`	`77`	`.cast<Map<Object?, Object?>>()`