Unverified Commit 8f7ccd4c authored by Casey Hillers's avatar Casey Hillers Committed by GitHub

Revert "Speed up first asset load by using the binary-formatted asset manifest...

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

This reverts commit e3db0488.
parent 1168c2c8
......@@ -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('The key was not found in the asset manifest'));
expect(tester.takeException().toString(), contains('Invalid image data'));
});
}
......@@ -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 = _getVariants(manifest, 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,34 +328,35 @@ class AssetImage extends AssetBundleImageProvider {
return completer.future;
}
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.
'''),
]);
/// 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);
}
AssetMetadata _chooseVariant(String mainAssetKey, ImageConfiguration config, Iterable<AssetMetadata> candidateVariants) {
if (config.devicePixelRatio == null || candidateVariants.isEmpty) {
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`.
......@@ -368,17 +371,17 @@ Make sure the key is correct and the appropriate file or folder is specified in
// 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
......@@ -386,10 +389,30 @@ Make sure the key is correct and the appropriate file or folder is specified in
// 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
......
......@@ -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.');
throw ArgumentError('Asset key $key was not found within the asset manifest.');
}
_typeCastedData[key] = ((_data[key] ?? <Object?>[]) as Iterable<Object?>)
.cast<Map<Object?, Object?>>()
......
......@@ -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<dynamic, dynamic>>> assetBundleMap =
<String, List<Map<dynamic, dynamic>>>{};
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<dynamic, dynamic>>> assetBundleMap =
<String, List<Map<dynamic, dynamic>>>{};
final Map<String, List<String>> assetBundleMap =
<String, List<String>>{};
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);
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<dynamic, dynamic>>> assetBundleMap =
<String, List<Map<dynamic, dynamic>>>{};
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<dynamic, dynamic>>> assetBundleMap =
<String, List<Map<dynamic, dynamic>>>{};
final Map<String, List<String>> assetBundleMap =
<String, List<String>>{};
final Map<dynamic, dynamic> mainAssetVariantManifestEntry = <dynamic, dynamic>{};
mainAssetVariantManifestEntry['asset'] = variantPath;
mainAssetVariantManifestEntry['dpr'] = 3.0;
assetBundleMap[mainAssetPath] = <Map<dynamic, dynamic>>[mainAssetVariantManifestEntry];
assetBundleMap[mainAssetPath] = <String>[mainAssetPath, variantPath];
final TestAssetBundle testAssetBundle = TestAssetBundle(assetBundleMap);
......
......@@ -5,7 +5,6 @@
@TestOn('!chrome')
library;
import 'dart:convert';
import 'dart:ui' as ui show Image;
import 'package:flutter/foundation.dart';
......@@ -19,31 +18,27 @@ import '../image_data.dart';
ByteData testByteData(double scale) => ByteData(8)..setFloat64(0, scale);
double scaleOf(ByteData data) => data.getFloat64(0);
final Map<dynamic, dynamic> testManifest = json.decode('''
const String testManifest = '''
{
"assets/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}
"assets/image.png",
"assets/1.5x/image.png",
"assets/2.0x/image.png",
"assets/3.0x/image.png",
"assets/4.0x/image.png"
]
}
''') as Map<Object?, Object?>;
''';
class TestAssetBundle extends CachingAssetBundle {
TestAssetBundle({ required Map<dynamic, dynamic> manifest }) {
this.manifest = const StandardMessageCodec().encodeMessage(manifest)!;
}
TestAssetBundle({ this.manifest = testManifest });
late final ByteData manifest;
final String manifest;
@override
Future<ByteData> load(String key) {
late ByteData data;
switch (key) {
case 'AssetManifest.bin':
data = manifest;
break;
case 'assets/image.png':
data = testByteData(1.0);
break;
......@@ -66,6 +61,14 @@ class TestAssetBundle extends CachingAssetBundle {
return SynchronousFuture<ByteData>(data);
}
@override
Future<String> loadString(String key, { bool cache = true }) {
if (key == 'AssetManifest.json') {
return SynchronousFuture<String>(manifest);
}
return SynchronousFuture<String>('');
}
@override
String toString() => '${describeIdentity(this)}()';
}
......@@ -104,7 +107,7 @@ Widget buildImageAtRatio(String imageName, Key key, double ratio, bool inferSize
devicePixelRatio: ratio,
),
child: DefaultAssetBundle(
bundle: bundle ?? TestAssetBundle(manifest: testManifest),
bundle: bundle ?? TestAssetBundle(),
child: Center(
child: inferSize ?
Image(
......@@ -257,21 +260,46 @@ 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.
final Map<Object?, Object?> manifest = json.decode('''
const String manifest = '''
{
"assets/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}
"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"
]
}
''') as Map<Object?, Object?>;
''';
final AssetBundle bundle = TestAssetBundle(manifest: manifest);
const double ratio = 1.0;
......@@ -310,14 +338,14 @@ void main() {
// if higher resolution assets are not available we will pick the best
// available.
testWidgets('Low-resolution assets', (WidgetTester tester) async {
final Map<Object?, Object?> manifest = json.decode('''
final AssetBundle bundle = TestAssetBundle(manifest: '''
{
"assets/image.png" : [
{"asset": "assets/1.5x/image.png", "dpr": 1.5}
"assets/image.png",
"assets/1.5x/image.png"
]
}
''') as Map<Object?, Object?>;
final AssetBundle bundle = TestAssetBundle(manifest: manifest);
''');
Future<void> testRatio({required double ratio, required double expectedScale}) async {
Key key = GlobalKey();
......
......@@ -2000,9 +2000,10 @@ void main() {
);
expect(
tester.takeException().toString(),
equals('Unable to load asset with key "missing-asset".\n'
'The key was not found in the asset manifest.\n'
'Make sure the key is correct and the appropriate file or folder is specified in pubspec.yaml.'),
equals(
'Unable to load asset: "missing-asset".\n'
'The asset does not exist or has empty data.',
),
);
await tester.pump();
await expectLater(
......
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