From f2745e97d53c6a29c7d40003dfaa4fc4c688cdd2 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 12 Jan 2024 09:49:54 -0800 Subject: [PATCH] When Impeller is enabled for flutter tester choose correct shader target. (#141391) When compiling shaders for flutter tester, include Vulkan shaders when targeting Impeller. --- .../build_system/targets/shader_compiler.dart | 6 ++- .../flutter_tools/lib/src/bundle_builder.dart | 9 +++- .../flutter_tools/lib/src/commands/test.dart | 42 +++++++++-------- .../lib/src/isolated/devfs_web.dart | 2 + .../test/general.shard/asset_bundle_test.dart | 5 ++ .../targets/shader_compiler_test.dart | 47 +++++++++++++++++++ 6 files changed, 88 insertions(+), 23 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_system/targets/shader_compiler.dart b/packages/flutter_tools/lib/src/build_system/targets/shader_compiler.dart index ce31f39cf16d..07e4a85f3784 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/shader_compiler.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/shader_compiler.dart @@ -24,6 +24,7 @@ import '../build_system.dart'; enum ShaderTarget { impellerAndroid(['--runtime-stage-gles', '--runtime-stage-vulkan']), impelleriOS(['--runtime-stage-metal']), + impellerSwiftShader(['--runtime-stage-vulkan']), sksl(['--sksl']); const ShaderTarget(this.stages); @@ -71,8 +72,9 @@ class DevelopmentShaderCompiler { case TargetPlatform.fuchsia_arm64: case TargetPlatform.fuchsia_x64: case TargetPlatform.tester: - assert(impellerStatus != ImpellerStatus.enabled); - _shaderTarget = ShaderTarget.sksl; + _shaderTarget = impellerStatus == ImpellerStatus.enabled + ? ShaderTarget.impellerSwiftShader + : ShaderTarget.sksl; case TargetPlatform.web_javascript: assert(impellerStatus != ImpellerStatus.enabled); _shaderTarget = ShaderTarget.sksl; diff --git a/packages/flutter_tools/lib/src/bundle_builder.dart b/packages/flutter_tools/lib/src/bundle_builder.dart index 496f2d0138e5..c970fa7e8e01 100644 --- a/packages/flutter_tools/lib/src/bundle_builder.dart +++ b/packages/flutter_tools/lib/src/bundle_builder.dart @@ -18,6 +18,7 @@ import 'build_system/targets/shader_compiler.dart'; import 'bundle.dart'; import 'cache.dart'; import 'devfs.dart'; +import 'device.dart'; import 'globals.dart' as globals; import 'project.dart'; @@ -140,6 +141,7 @@ Future writeBundle( Map entryKinds, { Logger? loggerOverride, required TargetPlatform targetPlatform, + required ImpellerStatus impellerStatus, }) async { loggerOverride ??= globals.logger; if (bundleDir.existsSync()) { @@ -168,6 +170,11 @@ Future writeBundle( artifacts: globals.artifacts!, ); + ShaderTarget shaderTarget = ShaderTarget.sksl; + if (targetPlatform == TargetPlatform.tester && impellerStatus == ImpellerStatus.enabled) { + shaderTarget = ShaderTarget.impellerSwiftShader; + } + // Limit number of open files to avoid running out of file descriptors. final Pool pool = Pool(64); await Future.wait( @@ -195,7 +202,7 @@ Future writeBundle( doCopy = !await shaderCompiler.compileShader( input: input, outputPath: file.path, - target: ShaderTarget.sksl, // TODO(zanderso): configure impeller target when enabled. + target: shaderTarget, json: targetPlatform == TargetPlatform.web_javascript, ); case AssetKind.model: diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart index 1c5f4abec59d..29b2f101b37f 100644 --- a/packages/flutter_tools/lib/src/commands/test.dart +++ b/packages/flutter_tools/lib/src/commands/test.dart @@ -345,13 +345,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ); } - String? testAssetDirectory; - if (buildTestAssets) { - await _buildTestAsset(flavor: buildInfo.flavor); - testAssetDirectory = globals.fs.path. - join(flutterProject.directory.path, 'build', 'unit_test_assets'); - } - final bool startPaused = boolArg('start-paused'); if (startPaused && _testFileUris.length != 1) { throwToolExit( @@ -360,6 +353,26 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ); } + final DebuggingOptions debuggingOptions = DebuggingOptions.enabled( + buildInfo, + startPaused: startPaused, + disableServiceAuthCodes: boolArg('disable-service-auth-codes'), + serveObservatory: boolArg('serve-observatory'), + // On iOS >=14, keeping this enabled will leave a prompt on the screen. + disablePortPublication: true, + enableDds: enableDds, + nullAssertions: boolArg(FlutterOptions.kNullAssertions), + usingCISystem: usingCISystem, + enableImpeller: ImpellerStatus.fromBool(argResults!['enable-impeller'] as bool?), + ); + + String? testAssetDirectory; + if (buildTestAssets) { + await _buildTestAsset(flavor: buildInfo.flavor, impellerStatus: debuggingOptions.enableImpeller); + testAssetDirectory = globals.fs.path. + join(flutterProject.directory.path, 'build', 'unit_test_assets'); + } + final String? concurrencyString = stringArg('concurrency'); int? jobs = concurrencyString == null ? null : int.tryParse(concurrencyString); if (jobs != null && (jobs <= 0 || !jobs.isFinite)) { @@ -427,19 +440,6 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { watcher = collector; } - final DebuggingOptions debuggingOptions = DebuggingOptions.enabled( - buildInfo, - startPaused: startPaused, - disableServiceAuthCodes: boolArg('disable-service-auth-codes'), - serveObservatory: boolArg('serve-observatory'), - // On iOS >=14, keeping this enabled will leave a prompt on the screen. - disablePortPublication: true, - enableDds: enableDds, - nullAssertions: boolArg(FlutterOptions.kNullAssertions), - usingCISystem: usingCISystem, - enableImpeller: ImpellerStatus.fromBool(argResults!['enable-impeller'] as bool?), - ); - Device? integrationTestDevice; if (_isIntegrationTest) { integrationTestDevice = await findTargetDevice(); @@ -569,6 +569,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { Future _buildTestAsset({ required String? flavor, + required ImpellerStatus impellerStatus, }) async { final AssetBundle assetBundle = AssetBundleFactory.instance.createBundle(); final int build = await assetBundle.build( @@ -584,6 +585,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { assetBundle.entries, assetBundle.entryKinds, targetPlatform: TargetPlatform.tester, + impellerStatus: impellerStatus, ); } } diff --git a/packages/flutter_tools/lib/src/isolated/devfs_web.dart b/packages/flutter_tools/lib/src/isolated/devfs_web.dart index 45ec712df4b6..15a304e78ec3 100644 --- a/packages/flutter_tools/lib/src/isolated/devfs_web.dart +++ b/packages/flutter_tools/lib/src/isolated/devfs_web.dart @@ -32,6 +32,7 @@ import '../compile.dart'; import '../convert.dart'; import '../dart/package_map.dart'; import '../devfs.dart'; +import '../device.dart'; import '../globals.dart' as globals; import '../html_utils.dart'; import '../project.dart'; @@ -885,6 +886,7 @@ class WebDevFS implements DevFS { bundle.entries, bundle.entryKinds, targetPlatform: TargetPlatform.web_javascript, + impellerStatus: ImpellerStatus.disabled, ); } } diff --git a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart index d325616dbcb5..e838560b19a2 100644 --- a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart +++ b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart @@ -16,6 +16,7 @@ import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/bundle_builder.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/devfs.dart'; +import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/project.dart'; import 'package:standard_message_codec/standard_message_codec.dart'; @@ -622,6 +623,7 @@ flutter: {}, loggerOverride: testLogger, targetPlatform: TargetPlatform.android, + impellerStatus: ImpellerStatus.disabled, ); expect(testLogger.warningText, contains('Expected Error Text')); @@ -744,6 +746,7 @@ flutter: bundle.entryKinds, loggerOverride: testLogger, targetPlatform: TargetPlatform.android, + impellerStatus: ImpellerStatus.disabled, ); }, overrides: { @@ -790,6 +793,7 @@ flutter: bundle.entryKinds, loggerOverride: testLogger, targetPlatform: TargetPlatform.web_javascript, + impellerStatus: ImpellerStatus.disabled, ); }, overrides: { @@ -873,6 +877,7 @@ flutter: bundle.entryKinds, loggerOverride: testLogger, targetPlatform: TargetPlatform.web_javascript, + impellerStatus: ImpellerStatus.disabled, ); expect((globals.processManager as FakeProcessManager).hasRemainingExpectations, false); }, overrides: { diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/shader_compiler_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/shader_compiler_test.dart index f8c83bef5ff9..e9e9437d62e7 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/shader_compiler_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/shader_compiler_test.dart @@ -287,6 +287,53 @@ void main() { expect(fileSystem.file('/.tmp_rand0/0.8255140718871702.temp'), isNot(exists)); }); + testWithoutContext('DevelopmentShaderCompiler can compile for Flutter Tester with Impeller and Vulkan', () async { + final FakeProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: [ + impellerc, + '--runtime-stage-vulkan', + '--iplr', + '--sl=/.tmp_rand0/0.8255140718871702.temp', + '--spirv=/.tmp_rand0/0.8255140718871702.temp.spirv', + '--input=$fragPath', + '--input-type=frag', + '--include=$fragDir', + '--include=$shaderLibDir', + ], + onRun: () { + fileSystem.file('/.tmp_rand0/0.8255140718871702.temp.spirv').createSync(); + fileSystem.file('/.tmp_rand0/0.8255140718871702.temp') + ..createSync() + ..writeAsBytesSync([1, 2, 3, 4]); + } + ), + ]); + fileSystem.file(fragPath).writeAsBytesSync([1, 2, 3, 4]); + final ShaderCompiler shaderCompiler = ShaderCompiler( + processManager: processManager, + logger: logger, + fileSystem: fileSystem, + artifacts: artifacts, + ); + final DevelopmentShaderCompiler developmentShaderCompiler = DevelopmentShaderCompiler( + shaderCompiler: shaderCompiler, + fileSystem: fileSystem, + random: math.Random(0), + ); + + developmentShaderCompiler.configureCompiler( + TargetPlatform.tester, + impellerStatus: ImpellerStatus.enabled, + ); + + final DevFSContent? content = await developmentShaderCompiler + .recompileShader(DevFSFileContent(fileSystem.file(fragPath))); + + expect(await content!.contentsAsBytes(), [1, 2, 3, 4]); + expect(processManager.hasRemainingExpectations, false); + }); + testWithoutContext('DevelopmentShaderCompiler can compile for android with impeller', () async { final FakeProcessManager processManager = FakeProcessManager.list([ FakeCommand(