Skip to content

Commit cb5b5c3

Browse files
andrewkolosandrewkolos
authored
Tighten asset variant detection criteria to only include device-pixel-ratio variants (#110721)
1 parent 2853a60 commit cb5b5c3

File tree

6 files changed

+277
-144
lines changed

6 files changed

+277
-144
lines changed

‎dev/integration_tests/flutter_gallery/pubspec.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ flutter:
169169
- packages/flutter_gallery_assets/products/table.png
170170
- packages/flutter_gallery_assets/products/teaset.png
171171
- packages/flutter_gallery_assets/products/top.png
172-
- packages/flutter_gallery_assets/people/ali.png
173172
- packages/flutter_gallery_assets/people/square/ali.png
174173
- packages/flutter_gallery_assets/people/square/peter.png
175174
- packages/flutter_gallery_assets/people/square/sandra.png

‎packages/flutter_tools/lib/src/asset.dart‎

Lines changed: 66 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ const String defaultManifestPath = 'pubspec.yaml';
2323

2424
const String kFontManifestJson = 'FontManifest.json';
2525

26+
// Should match '2x', '/1x', '1.5x', etc.
27+
final RegExp _assetVariantDirectoryRegExp = RegExp(r'/?(\d+(\.\d*)?)x$');
28+
2629
/// The effect of adding `uses-material-design: true` to the pubspec is to insert
2730
/// the following snippet into the asset manifest:
2831
///
@@ -92,7 +95,6 @@ abstract class AssetBundle {
9295
/// Returns 0 for success; non-zero for failure.
9396
Future<int> build({
9497
String manifestPath = defaultManifestPath,
95-
String? assetDirPath,
9698
required String packagesPath,
9799
bool deferredComponentsEnabled = false,
98100
TargetPlatform? targetPlatform,
@@ -205,23 +207,22 @@ class ManifestAssetBundle implements AssetBundle {
205207
@override
206208
Future<int> build({
207209
String manifestPath = defaultManifestPath,
208-
String? assetDirPath,
210+
FlutterProject? flutterProject,
209211
required String packagesPath,
210212
bool deferredComponentsEnabled = false,
211213
TargetPlatform? targetPlatform,
212214
}) async {
213-
assetDirPath ??= getAssetBuildDirectory();
214-
FlutterProject flutterProject;
215-
try {
216-
flutterProject = FlutterProject.fromDirectory(_fileSystem.file(manifestPath).parent);
217-
} on Exception catch (e) {
218-
_logger.printStatus('Error detected in pubspec.yaml:', emphasis: true);
219-
_logger.printError('$e');
220-
return 1;
221-
}
215+
222216
if (flutterProject == null) {
223-
return 1;
217+
try {
218+
flutterProject = FlutterProject.fromDirectory(_fileSystem.file(manifestPath).parent);
219+
} on Exception catch (e) {
220+
_logger.printStatus('Error detected in pubspec.yaml:', emphasis: true);
221+
_logger.printError('$e');
222+
return 1;
223+
}
224224
}
225+
225226
final FlutterManifest flutterManifest = flutterProject.manifest;
226227
// If the last build time isn't set before this early return, empty pubspecs will
227228
// hang on hot reload, as the incremental dill files will never be copied to the
@@ -243,27 +244,14 @@ class ManifestAssetBundle implements AssetBundle {
243244
final List<Uri> wildcardDirectories = <Uri>[];
244245

245246
// The _assetVariants map contains an entry for each asset listed
246-
// in the pubspec.yaml file's assets and font and sections. The
247+
// in the pubspec.yaml file's assets and font sections. The
247248
// value of each image asset is a list of resolution-specific "variants",
248249
// see _AssetDirectoryCache.
249-
final List<String> excludeDirs = <String>[
250-
assetDirPath,
251-
getBuildDirectory(),
252-
if (flutterProject.ios.existsSync())
253-
flutterProject.ios.hostAppRoot.path,
254-
if (flutterProject.macos.existsSync())
255-
flutterProject.macos.managedDirectory.path,
256-
if (flutterProject.windows.existsSync())
257-
flutterProject.windows.managedDirectory.path,
258-
if (flutterProject.linux.existsSync())
259-
flutterProject.linux.managedDirectory.path,
260-
];
261250
final Map<_Asset, List<_Asset>>? assetVariants = _parseAssets(
262251
packageConfig,
263252
flutterManifest,
264253
wildcardDirectories,
265254
assetBasePath,
266-
excludeDirs: excludeDirs,
267255
);
268256

269257
if (assetVariants == null) {
@@ -277,7 +265,6 @@ class ManifestAssetBundle implements AssetBundle {
277265
assetBasePath,
278266
wildcardDirectories,
279267
flutterProject.directory,
280-
excludeDirs: excludeDirs,
281268
);
282269
if (!_splitDeferredAssets || !deferredComponentsEnabled) {
283270
// Include the assets in the regular set of assets if not using deferred
@@ -373,8 +360,7 @@ class ManifestAssetBundle implements AssetBundle {
373360
// variant files exist. An image's main entry is treated the same as a
374361
// "1x" resolution variant and if both exist then the explicit 1x
375362
// variant is preferred.
376-
if (assetFile.existsSync()) {
377-
assert(!variants.contains(asset));
363+
if (assetFile.existsSync() && !variants.contains(asset)) {
378364
variants.insert(0, asset);
379365
}
380366
for (final _Asset variant in variants) {
@@ -407,8 +393,7 @@ class ManifestAssetBundle implements AssetBundle {
407393
// variant files exist. An image's main entry is treated the same as a
408394
// "1x" resolution variant and if both exist then the explicit 1x
409395
// variant is preferred.
410-
if (assetFile.existsSync()) {
411-
assert(!assetsMap[asset]!.contains(asset));
396+
if (assetFile.existsSync() && !assetsMap[asset]!.contains(asset)) {
412397
assetsMap[asset]!.insert(0, asset);
413398
}
414399
for (final _Asset variant in assetsMap[asset]!) {
@@ -606,7 +591,7 @@ class ManifestAssetBundle implements AssetBundle {
606591
}
607592
for (final DeferredComponent component in components) {
608593
deferredComponentsAssetVariants[component.name] = <_Asset, List<_Asset>>{};
609-
final _AssetDirectoryCache cache = _AssetDirectoryCache(<String>[], _fileSystem);
594+
final _AssetDirectoryCache cache = _AssetDirectoryCache(_fileSystem);
610595
for (final Uri assetUri in component.assets) {
611596
if (assetUri.path.endsWith('/')) {
612597
wildcardDirectories.add(assetUri);
@@ -617,7 +602,6 @@ class ManifestAssetBundle implements AssetBundle {
617602
cache,
618603
deferredComponentsAssetVariants[component.name]!,
619604
assetUri,
620-
excludeDirs: excludeDirs,
621605
);
622606
} else {
623607
_parseAssetFromFile(
@@ -728,13 +712,12 @@ class ManifestAssetBundle implements AssetBundle {
728712
FlutterManifest flutterManifest,
729713
List<Uri> wildcardDirectories,
730714
String assetBase, {
731-
List<String> excludeDirs = const <String>[],
732715
String? packageName,
733716
Package? attributedPackage,
734717
}) {
735718
final Map<_Asset, List<_Asset>> result = <_Asset, List<_Asset>>{};
736719

737-
final _AssetDirectoryCache cache = _AssetDirectoryCache(excludeDirs, _fileSystem);
720+
final _AssetDirectoryCache cache = _AssetDirectoryCache(_fileSystem);
738721
for (final Uri assetUri in flutterManifest.assets) {
739722
if (assetUri.path.endsWith('/')) {
740723
wildcardDirectories.add(assetUri);
@@ -745,7 +728,6 @@ class ManifestAssetBundle implements AssetBundle {
745728
cache,
746729
result,
747730
assetUri,
748-
excludeDirs: excludeDirs,
749731
packageName: packageName,
750732
attributedPackage: attributedPackage,
751733
);
@@ -757,7 +739,6 @@ class ManifestAssetBundle implements AssetBundle {
757739
cache,
758740
result,
759741
assetUri,
760-
excludeDirs: excludeDirs,
761742
packageName: packageName,
762743
attributedPackage: attributedPackage,
763744
);
@@ -772,7 +753,6 @@ class ManifestAssetBundle implements AssetBundle {
772753
cache,
773754
result,
774755
shaderUri,
775-
excludeDirs: excludeDirs,
776756
packageName: packageName,
777757
attributedPackage: attributedPackage,
778758
assetKind: AssetKind.shader,
@@ -808,7 +788,6 @@ class ManifestAssetBundle implements AssetBundle {
808788
_AssetDirectoryCache cache,
809789
Map<_Asset, List<_Asset>> result,
810790
Uri assetUri, {
811-
List<String> excludeDirs = const <String>[],
812791
String? packageName,
813792
Package? attributedPackage,
814793
}) {
@@ -820,10 +799,9 @@ class ManifestAssetBundle implements AssetBundle {
820799
return;
821800
}
822801

823-
final Iterable<File> files = _fileSystem
824-
.directory(directoryPath)
825-
.listSync()
826-
.whereType<File>();
802+
final Iterable<FileSystemEntity> entities = _fileSystem.directory(directoryPath).listSync();
803+
804+
final Iterable<File> files = entities.whereType<File>();
827805
for (final File file in files) {
828806
final String relativePath = _fileSystem.path.relative(file.path, from: assetBase);
829807
final Uri uri = Uri.file(relativePath, windows: _platform.isWindows);
@@ -839,6 +817,22 @@ class ManifestAssetBundle implements AssetBundle {
839817
attributedPackage: attributedPackage,
840818
);
841819
}
820+
821+
final Iterable<Directory> nonVariantSubDirectories = entities
822+
.whereType<Directory>()
823+
.where((Directory directory) => !_assetVariantDirectoryRegExp.hasMatch(directory.basename));
824+
for (final Directory dir in nonVariantSubDirectories) {
825+
final String relativePath = _fileSystem.path.relative(dir.path, from: assetBase);
826+
final Uri relativePathsUri = Uri.directory(relativePath, windows: _platform.isWindows);
827+
828+
_parseAssetsFromFolder(packageConfig,
829+
flutterManifest,
830+
assetBase,
831+
cache,
832+
result,
833+
relativePathsUri
834+
);
835+
}
842836
}
843837

844838
void _parseAssetFromFile(
@@ -1011,54 +1005,48 @@ class _Asset {
10111005

10121006
// Given an assets directory like this:
10131007
//
1014-
// assets/foo
1015-
// assets/var1/foo
1016-
// assets/var2/foo
1017-
// assets/bar
1008+
// assets/foo.png
1009+
// assets/2x/foo.png
1010+
// assets/3.0x/foo.png
1011+
// assets/bar/foo.png
1012+
// assets/bar.png
10181013
//
1019-
// variantsFor('assets/foo') => ['/assets/var1/foo', '/assets/var2/foo']
1020-
// variantsFor('assets/bar') => []
1014+
// variantsFor('assets/foo.png') => ['/assets/foo.png', '/assets/2x/foo.png', 'assets/3.0x/foo.png']
1015+
// variantsFor('assets/bar.png') => ['/assets/bar.png']
1016+
// variantsFor('assets/bar/foo.png') => ['/assets/bar/foo.png']
10211017
class _AssetDirectoryCache {
1022-
_AssetDirectoryCache(Iterable<String> excluded, this._fileSystem)
1023-
: _excluded = excluded
1024-
.map<String>(_fileSystem.path.absolute)
1025-
.toList();
1018+
_AssetDirectoryCache(this._fileSystem);
10261019

10271020
final FileSystem _fileSystem;
1028-
final List<String> _excluded;
1029-
final Map<String, Map<String, List<String>>> _cache = <String, Map<String, List<String>>>{};
1021+
final Map<String, List<String>> _cache = <String, List<String>>{};
10301022

10311023
List<String> variantsFor(String assetPath) {
1032-
final String assetName = _fileSystem.path.basename(assetPath);
10331024
final String directory = _fileSystem.path.dirname(assetPath);
10341025

10351026
if (!_fileSystem.directory(directory).existsSync()) {
10361027
return const <String>[];
10371028
}
10381029

1039-
if (_cache[directory] == null) {
1040-
final List<String> paths = <String>[];
1041-
for (final FileSystemEntity entity in _fileSystem.directory(directory).listSync(recursive: true)) {
1042-
final String path = entity.path;
1043-
if (_fileSystem.isFileSync(path)
1044-
&& assetPath != path
1045-
&& !_excluded.any((String exclude) => _fileSystem.path.isWithin(exclude, path))) {
1046-
paths.add(path);
1047-
}
1048-
}
1049-
1050-
final Map<String, List<String>> variants = <String, List<String>>{};
1051-
for (final String path in paths) {
1052-
final String variantName = _fileSystem.path.basename(path);
1053-
if (directory == _fileSystem.path.dirname(path)) {
1054-
continue;
1055-
}
1056-
variants[variantName] ??= <String>[];
1057-
variants[variantName]!.add(path);
1058-
}
1059-
_cache[directory] = variants;
1030+
if (_cache.containsKey(assetPath)) {
1031+
return _cache[assetPath]!;
10601032
}
10611033

1062-
return _cache[directory]![assetName] ?? const <String>[];
1034+
final List<FileSystemEntity> entitiesInDirectory = _fileSystem.directory(directory).listSync();
1035+
1036+
final List<String> pathsOfVariants = <String>[
1037+
// It's possible that the user specifies only explicit variants (e.g. .../1x/asset.png),
1038+
// so there does not necessarily need to be a file at the given path.
1039+
if (_fileSystem.file(assetPath).existsSync())
1040+
assetPath,
1041+
...entitiesInDirectory
1042+
.whereType<Directory>()
1043+
.where((Directory dir) => _assetVariantDirectoryRegExp.hasMatch(dir.basename))
1044+
.expand((Directory dir) => dir.listSync())
1045+
.whereType<File>()
1046+
.map((File file) => file.path),
1047+
];
1048+
1049+
_cache[assetPath] = pathsOfVariants;
1050+
return pathsOfVariants;
10631051
}
10641052
}

‎packages/flutter_tools/lib/src/bundle_builder.dart‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ Future<AssetBundle?> buildAssets({
121121
final AssetBundle assetBundle = AssetBundleFactory.instance.createBundle();
122122
final int result = await assetBundle.build(
123123
manifestPath: manifestPath,
124-
assetDirPath: assetDirPath,
125124
packagesPath: packagesPath,
126125
targetPlatform: targetPlatform,
127126
);

0 commit comments

Comments
 (0)