[Impeller] Build Impeller iOS runtime stage shaders when Impeller is enabled #113689
Conversation
bdero
added
the
e: impeller
flutter-dashboard
bot
added
the
tool
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
There are two ways to get the flag through to the build, and probably two different scenarios we need to handle:
- User types flutter run --enable-impeller or has updated the info.plist.
In this case we need to both pass the flag through and read the info.plist options. The current behavior is the info.plist wins over the flag.
- User types
flutter build X
Hopefully we don't support passing --enable-impeller to this option, but in this case we only check the plist, since the flag would not be persisted anyway.
Supporting the info.plist option is much easier. I would use the build directory and create a FlutterProject, and then access the iOS subproject and read the value of FLTEnableImpeller from the plist. We should already have the file path and ability to parse that file supported in the framework. See flutter/packages/flutter_tools/lib/src/build_system/targets/ios.dart L530 and then use the PlistParser class.
Separately, there is the runtime flag. I never really made adding new runtime flags easy to pass unfortunately. I think the best way to trace it would be to follow the path of a previous change to plumb through something like the SKSL bundle path. See #58879
|
Alrighty, I took a first pass at pulling |
bdero
changed the title
|
To get that to work, I think all you'll need to do is to get that plist file into the list of inputs for that target. Try something like: diff --git a/packages/flutter_tools/lib/src/build_system/targets/assets.dart b/packages/flutter_tools/lib/src/build_system/targets/assets.dart
index d719a1c3fd..4d7ff2a1b9 100644
--- a/packages/flutter_tools/lib/src/build_system/targets/assets.dart
+++ b/packages/flutter_tools/lib/src/build_system/targets/assets.dart
@@ -32,6 +32,7 @@ Future<Depfile> copyAssets(
required TargetPlatform targetPlatform,
BuildMode? buildMode,
required ShaderTarget shaderTarget,
+ List<File> additionalInputs = const <File>[],
}) async {
// Check for an SkSL bundle.
final String? shaderBundlePath = environment.defines[kBundleSkSLPath] ?? environment.inputs[kBundleSkSLPath];
@@ -65,6 +66,7 @@ Future<Depfile> copyAssets(
// An asset manifest with no assets would have zero inputs if not
// for this pubspec file.
pubspecFile,
+ ...additionalInputs,
];
final List<File> outputs = <File>[];
diff --git a/packages/flutter_tools/lib/src/build_system/targets/ios.dart b/packages/flutter_tools/lib/src/build_system/targets/ios.dart
index 644d48aff0..266e6fb019 100644
--- a/packages/flutter_tools/lib/src/build_system/targets/ios.dart
+++ b/packages/flutter_tools/lib/src/build_system/targets/ios.dart
@@ -515,6 +515,9 @@ abstract class IosAssetBundle extends Target {
assetDirectory,
targetPlatform: TargetPlatform.ios,
shaderTarget: ShaderTarget.sksl,
+ additionalInputs: <File>[
+ // path to plist
+ ]
);
final DepfileService depfileService = DepfileService(
fileSystem: environment.fileSystem, |
|
Nice, that's working! Another thing: I think shader hot reloading doesn't work with Impeller at the moment because we use the shader function entry point as the identifier and there's no way to "unregister" a shader function yet in the shader library. I'm gonna try to fix this when addressing #113719. |
| final FlutterProject flutterProject = FlutterProject.fromDirectory(environment.projectDir); | ||
|
|
||
| bool isImpellerEnabled() { | ||
| final PlistParser plistParser = PlistParser(fileSystem: globals.fs, logger: environment.logger, processManager: globals.processManager); |
There was a problem hiding this comment.
Nit, if you're using the cursed globals, might as well use globals. plistParser
| } | ||
| final Map<String, Object> info = plistParser.parseFile(flutterProject.ios.infoPlist.path); | ||
|
|
||
| if (!info.containsKey('FLTEnableImpeller')) { |
There was a problem hiding this comment.
Don't need this condition with the next condition, if they key is missing you'll get null back from the map
There was a problem hiding this comment.
Removed. I also made a cast below this less spicy.
|
|
||
| final Object? enableImpeller = info['FLTEnableImpeller']; | ||
|
|
||
| if (enableImpeller is bool?) { |
There was a problem hiding this comment.
maybe enableImpeller is bool ? enableImpeller : false
There was a problem hiding this comment.
Done. The linter also dropped some wisdom and this ended up at enableImpeller is bool && enableImpeller
There was a problem hiding this comment.
Question marks gone. I guess there's no question about it.
2 participants
Fixes the compiler flags for runtime stages.
The
ios.dartchange is a hack that needs to be replaced. Is there a good way to snake the Impeller flag intobuild_system/targets/*.dartso that we can set theshaderTargetbased on it?