From 57c7aa53c178f722aec19a76a60552e78ad09997 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Mon, 6 Mar 2023 20:37:43 +0000 Subject: [PATCH] Reland "Speed up first asset load by using the binary-formatted asset manifest for image resolution (#121322) Reland "Speed up first asset load by using the binary-formatted asset manifest for image resolution --- .../lib/src/painting/image_resolution.dart | 107 ++++++------------ .../lib/src/services/asset_manifest.dart | 12 +- .../test/painting/image_resolution_test.dart | 41 +++---- .../test/services/asset_manifest_test.dart | 9 +- .../test/widgets/image_resolution_test.dart | 80 +++++-------- 5 files changed, 93 insertions(+), 156 deletions(-) diff --git a/packages/flutter/lib/src/painting/image_resolution.dart b/packages/flutter/lib/src/painting/image_resolution.dart index 3c202777e53b2..58d3acf71af3d 100644 --- 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? completer; Future? result; - chosenBundle.loadStructuredData>?>(_kAssetManifestFileName, manifestParser).then( - (Map>? manifest) { - final String chosenName = _chooseVariant( + AssetManifest.loadFromAssetBundle(chosenBundle) + .then((AssetManifest manifest) { + final Iterable? candidateVariants = manifest.getAssetVariants(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(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,24 @@ class AssetImage extends AssetBundleImageProvider { return completer.future; } - /// Parses the asset manifest string into a strongly-typed map. - @visibleForTesting - static Future>?> manifestParser(String? jsonData) { - if (jsonData == null) { - return SynchronousFuture>?>(null); + AssetMetadata _chooseVariant(String mainAssetKey, ImageConfiguration config, Iterable? candidateVariants) { + if (candidateVariants == null) { + return AssetMetadata(key: mainAssetKey, targetDevicePixelRatio: null, main: true); } - // TODO(ianh): JSON decoding really shouldn't be on the main thread. - final Map parsedJson = json.decode(jsonData) as Map; - final Iterable keys = parsedJson.keys; - final Map> parsedManifest = > { - for (final String key in keys) key: List.from(parsedJson[key] as List), - }; - // TODO(ianh): convert that data structure to the right types. - return SynchronousFuture>?>(parsedManifest); - } - String? _chooseVariant(String main, ImageConfiguration config, List? candidates) { - if (config.devicePixelRatio == null || candidates == null || candidates.isEmpty) { - return main; + if (config.devicePixelRatio == null) { + return candidateVariants.firstWhere((AssetMetadata variant) => variant.main); } - // TODO(ianh): Consider moving this parsing logic into _manifestParser. - final SplayTreeMap mapping = SplayTreeMap(); - for (final String candidate in candidates) { - mapping[_parseScale(candidate)] = candidate; + + final SplayTreeMap candidatesByDevicePixelRatio = + SplayTreeMap(); + 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,17 +358,17 @@ 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 candidates, double value) { - if (candidates.containsKey(value)) { - return candidates[value]!; + AssetMetadata _findBestVariant(SplayTreeMap 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 @@ -389,30 +376,10 @@ class AssetImage extends AssetBundleImageProvider { // 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 index 4de568ad75693..3249909f2249d 100644 --- a/packages/flutter/lib/src/services/asset_manifest.dart +++ b/packages/flutter/lib/src/services/asset_manifest.dart @@ -30,14 +30,12 @@ abstract class AssetManifest { /// information. List listAssets(); - /// Retrieves metadata about an asset and its variants. + /// Retrieves metadata about an asset and its variants. Returns null if the + /// key was not found in the asset manifest. /// /// This method considers a main asset to be a variant of itself and /// includes it in the returned list. - /// - /// Throws an [ArgumentError] if [key] cannot be found within the manifest. To - /// avoid this, use a key obtained from the [listAssets] method. - List getAssetVariants(String key); + List? getAssetVariants(String key); } // Lazily parses the binary asset manifest into a data structure that's easier to work @@ -64,14 +62,14 @@ class _AssetManifestBin implements AssetManifest { final Map> _typeCastedData = >{}; @override - List getAssetVariants(String key) { + List? 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) { - throw ArgumentError('Asset key $key was not found within the asset manifest.'); + return null; } _typeCastedData[key] = ((_data[key] ?? []) as Iterable) .cast>() diff --git a/packages/flutter/test/painting/image_resolution_test.dart b/packages/flutter/test/painting/image_resolution_test.dart index 8e04f2aada4fb..79ea58f074ec9 100644 --- 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> _assetBundleMap; + final Map>> _assetBundleMap; Map loadCallCount = {}; - String get _assetBundleContents { - return json.encode(_assetBundleMap); - } - @override Future 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> assetBundleMap = >{}; + final Map>> assetBundleMap = + >>{}; - assetBundleMap[mainAssetPath] = []; + assetBundleMap[mainAssetPath] = >[]; 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> assetBundleMap = - >{}; - - assetBundleMap[mainAssetPath] = [mainAssetPath, variantPath]; + final Map>> assetBundleMap = + >>{}; + final Map mainAssetVariantManifestEntry = {}; + mainAssetVariantManifestEntry['asset'] = variantPath; + mainAssetVariantManifestEntry['dpr'] = 3.0; + assetBundleMap[mainAssetPath] = >[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> assetBundleMap = - >{}; + final Map>> assetBundleMap = + >>{}; - assetBundleMap[mainAssetPath] = [mainAssetPath]; + assetBundleMap[mainAssetPath] = >[]; final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap); @@ -162,10 +160,13 @@ void main() { double chosenAssetRatio, String expectedAssetPath, ) { - final Map> assetBundleMap = - >{}; + final Map>> assetBundleMap = + >>{}; - assetBundleMap[mainAssetPath] = [mainAssetPath, variantPath]; + final Map mainAssetVariantManifestEntry = {}; + mainAssetVariantManifestEntry['asset'] = variantPath; + mainAssetVariantManifestEntry['dpr'] = 3.0; + assetBundleMap[mainAssetPath] = >[mainAssetVariantManifestEntry]; final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap); diff --git a/packages/flutter/test/services/asset_manifest_test.dart b/packages/flutter/test/services/asset_manifest_test.dart index 4bd9c92223f0f..c37fbd500737a 100644 --- a/packages/flutter/test/services/asset_manifest_test.dart +++ b/packages/flutter/test/services/asset_manifest_test.dart @@ -41,7 +41,7 @@ void main() { expect(manifest.listAssets(), unorderedEquals(['assets/foo.png', 'assets/bar.png'])); - final List fooVariants = manifest.getAssetVariants('assets/foo.png'); + final List fooVariants = manifest.getAssetVariants('assets/foo.png')!; expect(fooVariants.length, 2); final AssetMetadata firstFooVariant = fooVariants[0]; expect(firstFooVariant.key, 'assets/foo.png'); @@ -52,7 +52,7 @@ void main() { expect(secondFooVariant.targetDevicePixelRatio, 2.0); expect(secondFooVariant.main, false); - final List barVariants = manifest.getAssetVariants('assets/bar.png'); + final List barVariants = manifest.getAssetVariants('assets/bar.png')!; expect(barVariants.length, 1); final AssetMetadata firstBarVariant = barVariants[0]; expect(firstBarVariant.key, 'assets/bar.png'); @@ -60,9 +60,8 @@ void main() { expect(firstBarVariant.main, true); }); - test('getAssetVariants throws if given a key not contained in the asset manifest', () async { + test('getAssetVariants returns null if the key not contained in the asset manifest', () async { final AssetManifest manifest = await AssetManifest.loadFromAssetBundle(TestAssetBundle()); - - expect(() => manifest.getAssetVariants('invalid asset key'), throwsArgumentError); + expect(manifest.getAssetVariants('invalid asset key'), isNull); }); } diff --git a/packages/flutter/test/widgets/image_resolution_test.dart b/packages/flutter/test/widgets/image_resolution_test.dart index fe836c1ed4b36..dae47c6c61661 100644 --- a/packages/flutter/test/widgets/image_resolution_test.dart +++ b/packages/flutter/test/widgets/image_resolution_test.dart @@ -5,6 +5,7 @@ @TestOn('!chrome') library; +import 'dart:convert'; import 'dart:ui' as ui show Image; import 'package:flutter/foundation.dart'; @@ -18,27 +19,31 @@ import '../image_data.dart'; ByteData testByteData(double scale) => ByteData(8)..setFloat64(0, scale); double scaleOf(ByteData data) => data.getFloat64(0); -const String testManifest = ''' +final Map testManifest = json.decode(''' { "assets/image.png" : [ - "assets/image.png", - "assets/1.5x/image.png", - "assets/2.0x/image.png", - "assets/3.0x/image.png", - "assets/4.0x/image.png" + {"asset": "assets/1.5x/image.png", "dpr": 1.5}, + {"asset": "assets/2.0x/image.png", "dpr": 2.0}, + {"asset": "assets/3.0x/image.png", "dpr": 3.0}, + {"asset": "assets/4.0x/image.png", "dpr": 4.0} ] } -'''; +''') as Map; class TestAssetBundle extends CachingAssetBundle { - TestAssetBundle({ this.manifest = testManifest }); + TestAssetBundle({ required Map manifest }) { + this.manifest = const StandardMessageCodec().encodeMessage(manifest)!; + } - final String manifest; + late final ByteData manifest; @override Future load(String key) { late ByteData data; switch (key) { + case 'AssetManifest.bin': + data = manifest; + break; case 'assets/image.png': data = testByteData(1.0); break; @@ -61,14 +66,6 @@ class TestAssetBundle extends CachingAssetBundle { return SynchronousFuture(data); } - @override - Future loadString(String key, { bool cache = true }) { - if (key == 'AssetManifest.json') { - return SynchronousFuture(manifest); - } - return SynchronousFuture(''); - } - @override String toString() => '${describeIdentity(this)}()'; } @@ -107,7 +104,7 @@ Widget buildImageAtRatio(String imageName, Key key, double ratio, bool inferSize devicePixelRatio: ratio, ), child: DefaultAssetBundle( - bundle: bundle ?? TestAssetBundle(), + bundle: bundle ?? TestAssetBundle(manifest: testManifest), child: Center( child: inferSize ? Image( @@ -260,46 +257,21 @@ void main() { expect(getRenderImage(tester, key).scale, 4.0); }); - testWidgets('Image for device pixel ratio 1.0, with no main asset', (WidgetTester tester) async { - const String manifest = ''' - { - "assets/image.png" : [ - "assets/1.5x/image.png", - "assets/2.0x/image.png", - "assets/3.0x/image.png", - "assets/4.0x/image.png" - ] - } - '''; - final AssetBundle bundle = TestAssetBundle(manifest: manifest); - - const double ratio = 1.0; - Key key = GlobalKey(); - await pumpTreeToLayout(tester, buildImageAtRatio(image, key, ratio, false, images, bundle)); - expect(getRenderImage(tester, key).size, const Size(200.0, 200.0)); - expect(getRenderImage(tester, key).scale, 1.5); - key = GlobalKey(); - await pumpTreeToLayout(tester, buildImageAtRatio(image, key, ratio, true, images, bundle)); - expect(getRenderImage(tester, key).size, const Size(48.0, 48.0)); - expect(getRenderImage(tester, key).scale, 1.5); - }); - testWidgets('Image for device pixel ratio 1.0, with a main asset and a 1.0x asset', (WidgetTester tester) async { // If both a main asset and a 1.0x asset are specified, then prefer // the 1.0x asset. - const String manifest = ''' + final Map manifest = json.decode(''' { "assets/image.png" : [ - "assets/image.png", - "assets/1.0x/image.png", - "assets/1.5x/image.png", - "assets/2.0x/image.png", - "assets/3.0x/image.png", - "assets/4.0x/image.png" + {"asset": "assets/1.0x/image.png", "dpr": 1.0}, + {"asset": "assets/1.5x/image.png", "dpr": 1.5}, + {"asset": "assets/2.0x/image.png", "dpr": 2.0}, + {"asset": "assets/3.0x/image.png", "dpr": 3.0}, + {"asset": "assets/4.0x/image.png", "dpr": 4.0} ] } - '''; + ''') as Map; final AssetBundle bundle = TestAssetBundle(manifest: manifest); const double ratio = 1.0; @@ -338,14 +310,14 @@ void main() { // if higher resolution assets are not available we will pick the best // available. testWidgets('Low-resolution assets', (WidgetTester tester) async { - final AssetBundle bundle = TestAssetBundle(manifest: ''' + final Map manifest = json.decode(''' { "assets/image.png" : [ - "assets/image.png", - "assets/1.5x/image.png" + {"asset": "assets/1.5x/image.png", "dpr": 1.5} ] } - '''); + ''') as Map; + final AssetBundle bundle = TestAssetBundle(manifest: manifest); Future testRatio({required double ratio, required double expectedScale}) async { Key key = GlobalKey();