Unverified Commit 759ebef6 authored by Andrew Kolos's avatar Andrew Kolos Committed by GitHub

Do not try to load main/default asset image if only higher-res variants exist (#128143)

Fixes https://github.com/flutter/flutter/issues/127090.

https://github.com/flutter/flutter/pull/122505 did a few things to speed up the first asset load that a flutter app performs. One of those things was to not include the main asset in its own list of variants in the asset manifest. The idea was that we know that the main asset always exists, so including it in its list of variants is a waste of storage space and loading time (even if the cost was tiny).

However, the assumption that the main asset always exists is wrong. From [Declaring resolution-aware image assets](https://docs.flutter.dev/ui/assets-and-images#resolution-aware), which predates https://github.com/flutter/flutter/pull/122505:

> Each entry in the asset section of the pubspec.yaml should correspond to a real file, with the exception of the main asset entry. If the main asset entry doesn’t correspond to a real file, then the asset with the lowest resolution is used as the fallback for devices with device pixel ratios below that resolution. The entry should still be included in the pubspec.yaml manifest, however.

For example, it's valid to declare `assets/image.png` as an asset even if only `assets/3x/image.png` exists on disk.

This fix restores older behavior of including a main asset as a variant of itself in the manifest if it exists.

This fix also includes a non-user-visible behavior change:
* `"dpr"` is no longer a required field in the asset manifest's underlying structure. For the main asset entry, we do not include `"dpr"`. It makes less sense for the tool to decide what the default target dpr for an image should be. This should be left to the framework.
parent 19f85935
...@@ -327,14 +327,10 @@ class AssetImage extends AssetBundleImageProvider { ...@@ -327,14 +327,10 @@ class AssetImage extends AssetBundleImageProvider {
} }
AssetMetadata _chooseVariant(String mainAssetKey, ImageConfiguration config, Iterable<AssetMetadata>? candidateVariants) { AssetMetadata _chooseVariant(String mainAssetKey, ImageConfiguration config, Iterable<AssetMetadata>? candidateVariants) {
if (candidateVariants == null) { if (candidateVariants == null || candidateVariants.isEmpty || config.devicePixelRatio == null) {
return AssetMetadata(key: mainAssetKey, targetDevicePixelRatio: null, main: true); return AssetMetadata(key: mainAssetKey, targetDevicePixelRatio: null, main: true);
} }
if (config.devicePixelRatio == null) {
return candidateVariants.firstWhere((AssetMetadata variant) => variant.main);
}
final SplayTreeMap<double, AssetMetadata> candidatesByDevicePixelRatio = final SplayTreeMap<double, AssetMetadata> candidatesByDevicePixelRatio =
SplayTreeMap<double, AssetMetadata>(); SplayTreeMap<double, AssetMetadata>();
for (final AssetMetadata candidate in candidateVariants) { for (final AssetMetadata candidate in candidateVariants) {
......
...@@ -33,8 +33,8 @@ abstract class AssetManifest { ...@@ -33,8 +33,8 @@ abstract class AssetManifest {
/// Retrieves metadata about an asset and its variants. Returns null if the /// Retrieves metadata about an asset and its variants. Returns null if the
/// key was not found in the asset manifest. /// key was not found in the asset manifest.
/// ///
/// This method considers a main asset to be a variant of itself and /// This method considers a main asset to be a variant of itself. The returned
/// includes it in the returned list. /// list will include it if it exists.
List<AssetMetadata>? getAssetVariants(String key); List<AssetMetadata>? getAssetVariants(String key);
} }
...@@ -73,22 +73,21 @@ class _AssetManifestBin implements AssetManifest { ...@@ -73,22 +73,21 @@ class _AssetManifestBin implements AssetManifest {
} }
_typeCastedData[key] = ((_data[key] ?? <Object?>[]) as Iterable<Object?>) _typeCastedData[key] = ((_data[key] ?? <Object?>[]) as Iterable<Object?>)
.cast<Map<Object?, Object?>>() .cast<Map<Object?, Object?>>()
.map((Map<Object?, Object?> data) => AssetMetadata( .map((Map<Object?, Object?> data) {
final String asset = data['asset']! as String;
final Object? dpr = data['dpr'];
return AssetMetadata(
key: data['asset']! as String, key: data['asset']! as String,
targetDevicePixelRatio: data['dpr']! as double, targetDevicePixelRatio: dpr as double?,
main: false, main: key == asset,
)) );
})
.toList(); .toList();
_data.remove(key); _data.remove(key);
} }
final AssetMetadata mainAsset = AssetMetadata(key: key, return _typeCastedData[key]!;
targetDevicePixelRatio: null,
main: true
);
return <AssetMetadata>[mainAsset, ..._typeCastedData[key]!];
} }
@override @override
......
...@@ -43,8 +43,6 @@ void main() { ...@@ -43,8 +43,6 @@ void main() {
final Map<String, List<Map<Object?, Object?>>> assetBundleMap = final Map<String, List<Map<Object?, Object?>>> assetBundleMap =
<String, List<Map<Object?, Object?>>>{}; <String, List<Map<Object?, Object?>>>{};
assetBundleMap[mainAssetPath] = <Map<Object?, Object?>>[];
final AssetImage assetImage = AssetImage( final AssetImage assetImage = AssetImage(
mainAssetPath, mainAssetPath,
bundle: TestAssetBundle(assetBundleMap), bundle: TestAssetBundle(assetBundleMap),
...@@ -160,15 +158,17 @@ void main() { ...@@ -160,15 +158,17 @@ void main() {
double chosenAssetRatio, double chosenAssetRatio,
String expectedAssetPath, String expectedAssetPath,
) { ) {
final Map<String, List<Map<Object?, Object?>>> assetBundleMap = const Map<String, List<Map<Object?, Object?>>> assetManifest =
<String, List<Map<Object?, Object?>>>{}; <String, List<Map<Object?, Object?>>>{
'assets/normalFolder/normalFile.png': <Map<Object?, Object?>>[
final Map<Object?, Object?> mainAssetVariantManifestEntry = <Object?, Object?>{}; <Object?, Object?>{'asset': 'assets/normalFolder/normalFile.png'},
mainAssetVariantManifestEntry['asset'] = variantPath; <Object?, Object?>{
mainAssetVariantManifestEntry['dpr'] = 3.0; 'asset': 'assets/normalFolder/3.0x/normalFile.png',
assetBundleMap[mainAssetPath] = <Map<Object?, Object?>>[mainAssetVariantManifestEntry]; 'dpr': 3.0
},
final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap); ]
};
final TestAssetBundle testAssetBundle = TestAssetBundle(assetManifest);
final AssetImage assetImage = AssetImage( final AssetImage assetImage = AssetImage(
mainAssetPath, mainAssetPath,
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:convert';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
...@@ -11,12 +13,19 @@ class TestAssetBundle extends AssetBundle { ...@@ -11,12 +13,19 @@ class TestAssetBundle extends AssetBundle {
if (key == 'AssetManifest.smcbin') { if (key == 'AssetManifest.smcbin') {
final Map<String, List<Object>> binManifestData = <String, List<Object>>{ final Map<String, List<Object>> binManifestData = <String, List<Object>>{
'assets/foo.png': <Object>[ 'assets/foo.png': <Object>[
<String, Object>{
'asset': 'assets/foo.png',
},
<String, Object>{ <String, Object>{
'asset': 'assets/2x/foo.png', 'asset': 'assets/2x/foo.png',
'dpr': 2.0 'dpr': 2.0
} },
],
'assets/bar.png': <Object>[
<String, Object>{
'asset': 'assets/bar.png',
},
], ],
'assets/bar.png': <Object>[],
}; };
final ByteData data = const StandardMessageCodec().encodeMessage(binManifestData)!; final ByteData data = const StandardMessageCodec().encodeMessage(binManifestData)!;
...@@ -32,7 +41,6 @@ class TestAssetBundle extends AssetBundle { ...@@ -32,7 +41,6 @@ class TestAssetBundle extends AssetBundle {
} }
} }
void main() { void main() {
TestWidgetsFlutterBinding.ensureInitialized(); TestWidgetsFlutterBinding.ensureInitialized();
...@@ -65,3 +73,29 @@ void main() { ...@@ -65,3 +73,29 @@ void main() {
expect(manifest.getAssetVariants('invalid asset key'), isNull); expect(manifest.getAssetVariants('invalid asset key'), isNull);
}); });
} }
String createAssetManifestJson(Map<String, List<AssetMetadata>> manifest) {
final Map<Object, Object> jsonObject = manifest.map(
(String key, List<AssetMetadata> value) {
final List<String> variants = value.map((AssetMetadata e) => e.key).toList();
return MapEntry<String, List<String>>(key, variants);
}
);
return json.encode(jsonObject);
}
ByteData createAssetManifestSmcBin(Map<String, List<AssetMetadata>> manifest) {
final Map<Object, Object> smcObject = manifest.map(
(String key, List<AssetMetadata> value) {
final List<Object> variants = value.map((AssetMetadata variant) => <String, Object?>{
'asset': variant.key,
'dpr': variant.targetDevicePixelRatio,
}).toList();
return MapEntry<String, List<Object>>(key, variants);
}
);
return const StandardMessageCodec().encodeMessage(smcObject)!;
}
...@@ -19,16 +19,15 @@ import '../image_data.dart'; ...@@ -19,16 +19,15 @@ import '../image_data.dart';
ByteData testByteData(double scale) => ByteData(8)..setFloat64(0, scale); ByteData testByteData(double scale) => ByteData(8)..setFloat64(0, scale);
double scaleOf(ByteData data) => data.getFloat64(0); double scaleOf(ByteData data) => data.getFloat64(0);
final Map<Object?, Object?> testManifest = json.decode(''' final Map<Object?, Object?> testManifest = <Object?, Object?>{
{ 'assets/image.png' : <Map<String, Object>>[
"assets/image.png" : [ <String, String>{'asset': 'assets/image.png'},
{"asset": "assets/1.5x/image.png", "dpr": 1.5}, <String, Object>{'asset': 'assets/1.5x/image.png', 'dpr': 1.5},
{"asset": "assets/2.0x/image.png", "dpr": 2.0}, <String, Object>{'asset': 'assets/2.0x/image.png', 'dpr': 2.0},
{"asset": "assets/3.0x/image.png", "dpr": 3.0}, <String, Object>{'asset': 'assets/3.0x/image.png', 'dpr': 3.0},
{"asset": "assets/4.0x/image.png", "dpr": 4.0} <String, Object>{'asset': 'assets/4.0x/image.png', 'dpr': 4.0}
] ],
} };
''') as Map<Object?, Object?>;
class TestAssetBundle extends CachingAssetBundle { class TestAssetBundle extends CachingAssetBundle {
TestAssetBundle({ required Map<Object?, Object?> manifest }) { TestAssetBundle({ required Map<Object?, Object?> manifest }) {
...@@ -303,13 +302,12 @@ void main() { ...@@ -303,13 +302,12 @@ void main() {
// if higher resolution assets are not available we will pick the best // if higher resolution assets are not available we will pick the best
// available. // available.
testWidgets('Low-resolution assets', (WidgetTester tester) async { testWidgets('Low-resolution assets', (WidgetTester tester) async {
final Map<Object?, Object?> manifest = json.decode(''' const Map<Object?, Object?> manifest = <Object?, Object?>{
{ 'assets/image.png': <Map<String, Object>>[
"assets/image.png" : [ <String, Object>{'asset': 'assets/image.png'},
{"asset": "assets/1.5x/image.png", "dpr": 1.5} <String, Object>{'asset': 'assets/1.5x/image.png', 'dpr': 1.5},
] ],
} };
''') as Map<Object?, Object?>;
final AssetBundle bundle = TestAssetBundle(manifest: manifest); final AssetBundle bundle = TestAssetBundle(manifest: manifest);
Future<void> testRatio({required double ratio, required double expectedScale}) async { Future<void> testRatio({required double ratio, required double expectedScale}) async {
......
...@@ -166,7 +166,6 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -166,7 +166,6 @@ class ManifestAssetBundle implements AssetBundle {
DateTime? _lastBuildTimestamp; DateTime? _lastBuildTimestamp;
// We assume the main asset is designed for a device pixel ratio of 1.0. // We assume the main asset is designed for a device pixel ratio of 1.0.
static const double _defaultResolution = 1.0;
static const String _kAssetManifestJsonFilename = 'AssetManifest.json'; static const String _kAssetManifestJsonFilename = 'AssetManifest.json';
static const String _kAssetManifestBinFilename = 'AssetManifest.smcbin'; static const String _kAssetManifestBinFilename = 'AssetManifest.smcbin';
...@@ -688,7 +687,7 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -688,7 +687,7 @@ class ManifestAssetBundle implements AssetBundle {
DevFSByteContent _createAssetManifestBinary( DevFSByteContent _createAssetManifestBinary(
Map<String, List<String>> assetManifest Map<String, List<String>> assetManifest
) { ) {
double parseScale(String key) { double? parseScale(String key) {
final Uri assetUri = Uri.parse(key); final Uri assetUri = Uri.parse(key);
String directoryPath = ''; String directoryPath = '';
if (assetUri.pathSegments.length > 1) { if (assetUri.pathSegments.length > 1) {
...@@ -699,7 +698,8 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -699,7 +698,8 @@ class ManifestAssetBundle implements AssetBundle {
if (match != null && match.groupCount > 0) { if (match != null && match.groupCount > 0) {
return double.parse(match.group(1)!); return double.parse(match.group(1)!);
} }
return _defaultResolution;
return null;
} }
final Map<String, dynamic> result = <String, dynamic>{}; final Map<String, dynamic> result = <String, dynamic>{};
...@@ -708,15 +708,12 @@ class ManifestAssetBundle implements AssetBundle { ...@@ -708,15 +708,12 @@ class ManifestAssetBundle implements AssetBundle {
final List<dynamic> resultVariants = <dynamic>[]; final List<dynamic> resultVariants = <dynamic>[];
final List<String> entries = (manifestEntry.value as List<dynamic>).cast<String>(); final List<String> entries = (manifestEntry.value as List<dynamic>).cast<String>();
for (final String variant in entries) { for (final String variant in entries) {
if (variant == manifestEntry.key) {
// With the newer binary format, don't include the main asset in it's
// list of variants. This reduces parsing time at runtime.
continue;
}
final Map<String, dynamic> resultVariant = <String, dynamic>{}; final Map<String, dynamic> resultVariant = <String, dynamic>{};
final double variantDevicePixelRatio = parseScale(variant); final double? variantDevicePixelRatio = parseScale(variant);
resultVariant['asset'] = variant; resultVariant['asset'] = variant;
if (variantDevicePixelRatio != null) {
resultVariant['dpr'] = variantDevicePixelRatio; resultVariant['dpr'] = variantDevicePixelRatio;
}
resultVariants.add(resultVariant); resultVariants.add(resultVariant);
} }
result[manifestEntry.key] = resultVariants; result[manifestEntry.key] = resultVariants;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment