Revert "Reland "Speed up first asset load by using the binary-formatted asset manifest for image resolution (#121322)" (#122449)

Revert "Reland "Speed up first asset load by using the binary-formatted asset manifest for image resolution"

Author: Jonah Williams

packages/flutter/lib/src/painting/image_resolution.dart

@@ -4,12 +4,15 @@
 
 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).
@@ -281,18 +284,18 @@ class AssetImage extends AssetBundleImageProvider {
     Completer<AssetBundleImageKey>? completer;
     Future<AssetBundleImageKey>? result;
 
-    AssetManifest.loadFromAssetBundle(chosenBundle)
-      .then((AssetManifest manifest) {
-        final Iterable<AssetMetadata>? candidateVariants = manifest.getAssetVariants(keyName);
-        final AssetMetadata chosenVariant = _chooseVariant(
+    chosenBundle.loadStructuredData<Map<String, List<String>>?>(_kAssetManifestFileName, manifestParser).then<void>(
+      (Map<String, List<String>>? manifest) {
+        final String chosenName = _chooseVariant(
           keyName,
           configuration,
-          candidateVariants,
-        );
+          manifest == null ? null : manifest[keyName],
+        )!;
+        final double chosenScale = _parseScale(chosenName);
         final AssetBundleImageKey key = AssetBundleImageKey(
           bundle: chosenBundle,
-          name: chosenVariant.key,
-          scale: chosenVariant.targetDevicePixelRatio ?? _naturalResolution,
+          name: chosenName,
+          scale: chosenScale,
         );
         if (completer != null) {
           // We already returned from this function, which means we are in the
@@ -306,15 +309,14 @@ class AssetImage extends AssetBundleImageProvider {
           // ourselves.
           result = SynchronousFuture<AssetBundleImageKey>(key);
         }
-      })
-      .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);
-      });
-
+      },
+    ).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);
+    });
     if (result != null) {
       // The code above ran synchronously, and came up with an answer.
       // Return the SynchronousFuture that we created above.
@@ -326,24 +328,35 @@ class AssetImage extends AssetBundleImageProvider {
     return completer.future;
   }
 
-  AssetMetadata _chooseVariant(String mainAssetKey, ImageConfiguration config, Iterable<AssetMetadata>? candidateVariants) {
-    if (candidateVariants == null) {
-      return AssetMetadata(key: mainAssetKey, targetDevicePixelRatio: null, main: true);
+  /// 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);
     }
+    // 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);
+  }
 
-    if (config.devicePixelRatio == null) {
-      return candidateVariants.firstWhere((AssetMetadata variant) => variant.main);
+  String? _chooseVariant(String main, ImageConfiguration config, List<String>? candidates) {
+    if (config.devicePixelRatio == null || candidates == null || candidates.isEmpty) {
+      return main;
     }
-
-    final SplayTreeMap<double, AssetMetadata> candidatesByDevicePixelRatio =
-      SplayTreeMap<double, AssetMetadata>();
-    for (final AssetMetadata candidate in candidateVariants) {
-      candidatesByDevicePixelRatio[candidate.targetDevicePixelRatio ?? _naturalResolution] = candidate;
+    // 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;
     }
     // TODO(ianh): implement support for config.locale, config.textDirection,
     // config.size, config.platform (then document this over in the Image.asset
     // docs)
-    return _findBestVariant(candidatesByDevicePixelRatio, config.devicePixelRatio!);
+    return _findBestVariant(mapping, config.devicePixelRatio!);
   }
 
   // Returns the "best" asset variant amongst the available `candidates`.
@@ -358,28 +371,48 @@ 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`.
-  AssetMetadata _findBestVariant(SplayTreeMap<double, AssetMetadata> candidatesByDpr, double value) {
-    if (candidatesByDpr.containsKey(value)) {
-      return candidatesByDpr[value]!;
+  String? _findBestVariant(SplayTreeMap<double, String> candidates, double value) {
+    if (candidates.containsKey(value)) {
+      return candidates[value]!;
     }
-    final double? lower = candidatesByDpr.lastKeyBefore(value);
-    final double? upper = candidatesByDpr.firstKeyAfter(value);
+    final double? lower = candidates.lastKeyBefore(value);
+    final double? upper = candidates.firstKeyAfter(value);
     if (lower == null) {
-      return candidatesByDpr[upper]!;
+      return candidates[upper];
     }
     if (upper == null) {
-      return candidatesByDpr[lower]!;
+      return candidates[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 candidatesByDpr[upper]!;
+      return candidates[upper];
     } else {
-      return candidatesByDpr[lower]!;
+      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 _naturalResolution; // i.e. default to 1.0x
   }
 
   @override

packages/flutter/lib/src/services/asset_manifest.dart

@@ -30,12 +30,14 @@ abstract class AssetManifest {
   /// information.
   List<String> listAssets();
 
-  /// Retrieves metadata about an asset and its variants. Returns null if the
-  /// key was not found in the asset manifest.
+  /// Retrieves metadata about an asset and its variants.
   ///
   /// This method considers a main asset to be a variant of itself and
   /// includes it in the returned list.
-  List<AssetMetadata>? getAssetVariants(String key);
+  ///
+  /// Throws an [ArgumentError] if [key] cannot be found within the manifest. To
+  /// avoid this, use a key obtained from the [listAssets] method.
+  List<AssetMetadata> getAssetVariants(String key);
 }
 
 // Lazily parses the binary asset manifest into a data structure that's easier to work
@@ -62,14 +64,14 @@ class _AssetManifestBin implements AssetManifest {
   final Map<String, List<AssetMetadata>> _typeCastedData = <String, List<AssetMetadata>>{};
 
   @override
-  List<AssetMetadata>? getAssetVariants(String key) {
+  List<AssetMetadata> getAssetVariants(String key) {
     // We lazily delay typecasting to prevent a performance hiccup when parsing
     // large asset manifests. This is important to keep an app's first asset
     // load fast.
     if (!_typeCastedData.containsKey(key)) {
       final Object? variantData = _data[key];
       if (variantData == null) {
-        return null;
+        throw ArgumentError('Asset key $key was not found within the asset manifest.');
       }
       _typeCastedData[key] = ((_data[key] ?? <Object?>[]) as Iterable<Object?>)
         .cast<Map<Object?, Object?>>()

packages/flutter/test/painting/image_resolution_test.dart

@@ -2,6 +2,7 @@
 // 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';
@@ -12,14 +13,18 @@ import 'package:flutter_test/flutter_test.dart';
 class TestAssetBundle extends CachingAssetBundle {
   TestAssetBundle(this._assetBundleMap);
 
-  final Map<String, List<Map<Object?, Object?>>> _assetBundleMap;
+  final Map<String, List<String>> _assetBundleMap;
 
   Map<String, int> loadCallCount = <String, int>{};
 
+  String get _assetBundleContents {
+    return json.encode(_assetBundleMap);
+  }
+
   @override
   Future<ByteData> load(String key) async {
-    if (key == 'AssetManifest.bin') {
-      return const StandardMessageCodec().encodeMessage(_assetBundleMap)!;
+    if (key == 'AssetManifest.json') {
+      return ByteData.view(Uint8List.fromList(const Utf8Encoder().convert(_assetBundleContents)).buffer);
     }
 
     loadCallCount[key] = loadCallCount[key] ?? 0 + 1;
@@ -40,10 +45,9 @@ class TestAssetBundle extends CachingAssetBundle {
 void main() {
   group('1.0 scale device tests', () {
     void buildAndTestWithOneAsset(String mainAssetPath) {
-      final Map<String, List<Map<Object?, Object?>>> assetBundleMap =
-        <String, List<Map<Object?, Object?>>>{};
+      final Map<String, List<String>> assetBundleMap = <String, List<String>>{};
 
-      assetBundleMap[mainAssetPath] = <Map<Object?, Object?>>[];
+      assetBundleMap[mainAssetPath] = <String>[];
 
       final AssetImage assetImage = AssetImage(
         mainAssetPath,
@@ -89,13 +93,11 @@ void main() {
       const String mainAssetPath = 'assets/normalFolder/normalFile.png';
       const String variantPath = 'assets/normalFolder/3.0x/normalFile.png';
 
-      final Map<String, List<Map<Object?, Object?>>> assetBundleMap =
-        <String, List<Map<Object?, Object?>>>{};
+      final Map<String, List<String>> assetBundleMap =
+      <String, List<String>>{};
+
+      assetBundleMap[mainAssetPath] = <String>[mainAssetPath, variantPath];
 
-      final Map<Object?, Object?> mainAssetVariantManifestEntry = <Object?, Object?>{};
-      mainAssetVariantManifestEntry['asset'] = variantPath;
-      mainAssetVariantManifestEntry['dpr'] = 3.0;
-      assetBundleMap[mainAssetPath] = <Map<Object?, Object?>>[mainAssetVariantManifestEntry];
       final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap);
 
       final AssetImage assetImage = AssetImage(
@@ -121,10 +123,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<Map<Object?, Object?>>> assetBundleMap =
-        <String, List<Map<Object?, Object?>>>{};
+      final Map<String, List<String>> assetBundleMap =
+      <String, List<String>>{};
 
-      assetBundleMap[mainAssetPath] = <Map<Object?, Object?>>[];
+      assetBundleMap[mainAssetPath] = <String>[mainAssetPath];
 
       final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap);
 
@@ -160,13 +162,10 @@ void main() {
       double chosenAssetRatio,
       String expectedAssetPath,
     ) {
-      final Map<String, List<Map<Object?, Object?>>> assetBundleMap =
-        <String, List<Map<Object?, Object?>>>{};
+      final Map<String, List<String>> assetBundleMap =
+      <String, List<String>>{};
 
-      final Map<Object?, Object?> mainAssetVariantManifestEntry = <Object?, Object?>{};
-      mainAssetVariantManifestEntry['asset'] = variantPath;
-      mainAssetVariantManifestEntry['dpr'] = 3.0;
-      assetBundleMap[mainAssetPath] = <Map<Object?, Object?>>[mainAssetVariantManifestEntry];
+      assetBundleMap[mainAssetPath] = <String>[mainAssetPath, variantPath];
 
       final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap);
 

packages/flutter/test/services/asset_manifest_test.dart

@@ -41,7 +41,7 @@ void main() {
 
     expect(manifest.listAssets(), unorderedEquals(<String>['assets/foo.png', 'assets/bar.png']));
 
-    final List<AssetMetadata> fooVariants = manifest.getAssetVariants('assets/foo.png')!;
+    final List<AssetMetadata> fooVariants = manifest.getAssetVariants('assets/foo.png');
     expect(fooVariants.length, 2);
     final AssetMetadata firstFooVariant = fooVariants[0];
     expect(firstFooVariant.key, 'assets/foo.png');
@@ -52,16 +52,17 @@ void main() {
     expect(secondFooVariant.targetDevicePixelRatio, 2.0);
     expect(secondFooVariant.main, false);
 
-    final List<AssetMetadata> barVariants = manifest.getAssetVariants('assets/bar.png')!;
+    final List<AssetMetadata> barVariants = manifest.getAssetVariants('assets/bar.png');
     expect(barVariants.length, 1);
     final AssetMetadata firstBarVariant = barVariants[0];
     expect(firstBarVariant.key, 'assets/bar.png');
     expect(firstBarVariant.targetDevicePixelRatio, null);
     expect(firstBarVariant.main, true);
   });
 
-  test('getAssetVariants returns null if the key not contained in the asset manifest', () async {
+  test('getAssetVariants throws if given a key not contained in the asset manifest', () async {
     final AssetManifest manifest = await AssetManifest.loadFromAssetBundle(TestAssetBundle());
-    expect(manifest.getAssetVariants('invalid asset key'), isNull);
+
+    expect(() => manifest.getAssetVariants('invalid asset key'), throwsArgumentError);
   });
 }