[flutter_tool] Pass --input-type to impellerc (flutter#106845)

engine-flutter-autoroll · Jun 30, 2022 · b005a34 · b005a34
1 parent eb3cf24
commit b005a34
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 20 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
@@ -5,7 +5,6 @@
 import 'package:process/process.dart';
 
 import '../../artifacts.dart';
-import '../../base/common.dart';
 import '../../base/file_system.dart';
 import '../../base/io.dart';
 import '../../base/logger.dart';
@@ -43,17 +42,13 @@ class ShaderCompiler {
   ///
   /// All parameters are required.
   ///
-  /// If the input file is not a fragment shader, this returns false.
-  /// If the shader compiler subprocess fails, it will [throwToolExit].
-  /// Otherwise, it will return true.
+  /// If the shader compiler subprocess fails, it will print the stdout and
+  /// stderr to the log and throw a [ShaderCompilerException]. Otherwise, it
+  /// will return true.
   Future<bool> compileShader({
     required File input,
     required String outputPath,
   }) async {
-    if (!input.path.endsWith('.frag')) {
-      return false;
-    }
-
     final File impellerc = _fs.file(
       _artifacts.getHostArtifact(HostArtifact.impellerc),
     );
@@ -72,6 +67,7 @@ class ShaderCompiler {
       '--flutter-spirv',
       '--spirv=$outputPath',
       '--input=${input.path}',
+      '--input-type=frag',
     ];
     final Process impellercProcess = await _processManager.start(cmd);
     final int code = await impellercProcess.exitCode;

diff --git a/packages/flutter_tools/lib/src/bundle_builder.dart b/packages/flutter_tools/lib/src/bundle_builder.dart
@@ -169,11 +169,25 @@ Future<void> writeBundle(
         // platform channels in the framework will URI encode these values,
         // and the native APIs will look for files this way.
         final File file = globals.fs.file(globals.fs.path.join(bundleDir.path, entry.key));
+        final AssetKind assetKind = entryKinds[entry.key] ?? AssetKind.regular;
         file.parent.createSync(recursive: true);
         final DevFSContent devFSContent = entry.value;
         if (devFSContent is DevFSFileContent) {
           final File input = devFSContent.file as File;
-          if (!await shaderCompiler.compileShader(input: input, outputPath: file.path)) {
+          bool doCopy = true;
+          switch (assetKind) {
+            case AssetKind.regular:
+              break;
+            case AssetKind.font:
+              break;
+            case AssetKind.shader:
+              doCopy = !await shaderCompiler.compileShader(
+                input: input,
+                outputPath: file.path,
+              );
+              break;
+          }
+          if (doCopy) {
             input.copySync(file.path);
           }
         } else {

diff --git a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart
@@ -444,6 +444,7 @@ flutter:
             '--flutter-spirv',
             '--spirv=$outputPath',
             '--input=/$shaderPath',
+            '--input-type=frag',
           ],
           onRun: () {
             fileSystem.file(outputPath).createSync(recursive: true);

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
@@ -10,8 +10,8 @@ import 'package:flutter_tools/src/build_system/targets/shader_compiler.dart';
 import '../../../src/common.dart';
 import '../../../src/fake_process_manager.dart';
 
-const String shaderPath = '/shaders/my_shader.frag';
-const String notShaderPath = '/shaders/not_a_shader.file';
+const String fragPath = '/shaders/my_shader.frag';
+const String notFragPath = '/shaders/not_a_frag.file';
 const String outputPath = '/output/shaders/my_shader.spv';
 
 void main() {
@@ -27,35 +27,51 @@ void main() {
     impellerc = artifacts.getHostArtifact(HostArtifact.impellerc).path;
 
     fileSystem.file(impellerc).createSync(recursive: true);
-    fileSystem.file(shaderPath).createSync(recursive: true);
-    fileSystem.file(notShaderPath).createSync(recursive: true);
+    fileSystem.file(fragPath).createSync(recursive: true);
+    fileSystem.file(notFragPath).createSync(recursive: true);
   });
 
-  testWithoutContext('compileShader returns false for non-shader files', () async {
+  testWithoutContext('compileShader invokes impellerc for .frag files', () async {
+    final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
+      FakeCommand(
+        command: <String>[
+          impellerc,
+          '--flutter-spirv',
+          '--spirv=$outputPath',
+          '--input=$fragPath',
+          '--input-type=frag',
+        ],
+        onRun: () {
+          fileSystem.file(outputPath).createSync(recursive: true);
+        },
+      ),
+    ]);
     final ShaderCompiler shaderCompiler = ShaderCompiler(
-      processManager: FakeProcessManager.empty(),
+      processManager: processManager,
       logger: logger,
       fileSystem: fileSystem,
       artifacts: artifacts,
     );
 
     expect(
       await shaderCompiler.compileShader(
-        input: fileSystem.file(notShaderPath),
+        input: fileSystem.file(fragPath),
         outputPath: outputPath,
       ),
-      false,
+      true,
     );
+    expect(fileSystem.file(outputPath).existsSync(), true);
   });
 
-  testWithoutContext('compileShader returns true for shader files', () async {
+  testWithoutContext('compileShader invokes impellerc for non-.frag files', () async {
     final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
       FakeCommand(
         command: <String>[
           impellerc,
           '--flutter-spirv',
           '--spirv=$outputPath',
-          '--input=$shaderPath',
+          '--input=$notFragPath',
+          '--input-type=frag',
         ],
         onRun: () {
           fileSystem.file(outputPath).createSync(recursive: true);
@@ -71,11 +87,41 @@ void main() {
 
     expect(
       await shaderCompiler.compileShader(
-        input: fileSystem.file(shaderPath),
+        input: fileSystem.file(notFragPath),
         outputPath: outputPath,
       ),
       true,
     );
     expect(fileSystem.file(outputPath).existsSync(), true);
   });
+
+  testWithoutContext('compileShader throws an exception when impellerc fails', () async {
+    final FakeProcessManager processManager = FakeProcessManager.list(<FakeCommand>[
+      FakeCommand(
+        command: <String>[
+          impellerc,
+          '--flutter-spirv',
+          '--spirv=$outputPath',
+          '--input=$notFragPath',
+          '--input-type=frag',
+        ],
+        exitCode: 1,
+      ),
+    ]);
+    final ShaderCompiler shaderCompiler = ShaderCompiler(
+      processManager: processManager,
+      logger: logger,
+      fileSystem: fileSystem,
+      artifacts: artifacts,
+    );
+
+    await expectLater(
+      () => shaderCompiler.compileShader(
+        input: fileSystem.file(notFragPath),
+        outputPath: outputPath,
+      ),
+      throwsA(isA<ShaderCompilerException>()),
+    );
+    expect(fileSystem.file(outputPath).existsSync(), false);
+  });
 }