From 4f19a9fa46b2264f49033dea5cb5cd009a50e9a2 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 31 Oct 2022 09:52:09 -0700 Subject: [PATCH] [flutter_tools] Add support for compiling shaders to JSON bundle for web (#114295) --- packages/flutter_tools/lib/src/artifacts.dart | 11 ++-- packages/flutter_tools/lib/src/asset.dart | 33 +++++------- .../lib/src/build_system/targets/assets.dart | 1 + .../build_system/targets/shader_compiler.dart | 9 ++++ .../flutter_tools/lib/src/bundle_builder.dart | 8 +-- .../flutter_tools/lib/src/commands/test.dart | 8 ++- .../lib/src/isolated/devfs_web.dart | 1 + .../test/general.shard/artifacts_test.dart | 18 +++++++ .../test/general.shard/asset_bundle_test.dart | 51 ++++++++++++++++--- .../targets/shader_compiler_test.dart | 50 ++++++++++++++++++ .../shader_compiler_test.dart | 1 + 11 files changed, 156 insertions(+), 35 deletions(-) diff --git a/packages/flutter_tools/lib/src/artifacts.dart b/packages/flutter_tools/lib/src/artifacts.dart index 3c1174f1613..5ac870497bf 100644 --- a/packages/flutter_tools/lib/src/artifacts.dart +++ b/packages/flutter_tools/lib/src/artifacts.dart @@ -751,7 +751,8 @@ class CachedLocalEngineArtifacts implements LocalEngineArtifacts { _cache = cache, _processManager = processManager, _platform = platform, - _operatingSystemUtils = operatingSystemUtils; + _operatingSystemUtils = operatingSystemUtils, + _backupCache = CachedArtifacts(fileSystem: fileSystem, platform: platform, cache: cache, operatingSystemUtils: operatingSystemUtils); @override final String engineOutPath; @@ -765,7 +766,7 @@ class CachedLocalEngineArtifacts implements LocalEngineArtifacts { final ProcessManager _processManager; final Platform _platform; final OperatingSystemUtils _operatingSystemUtils; - + final CachedArtifacts _backupCache; @override FileSystemEntity getHostArtifact(HostArtifact artifact) { @@ -838,7 +839,11 @@ class CachedLocalEngineArtifacts implements LocalEngineArtifacts { case HostArtifact.impellerc: case HostArtifact.libtessellator: final String artifactFileName = _hostArtifactToFileName(artifact, _platform); - return _fileSystem.file(_fileSystem.path.join(_hostEngineOutPath, artifactFileName)); + final File file = _fileSystem.file(_fileSystem.path.join(_hostEngineOutPath, artifactFileName)); + if (!file.existsSync()) { + return _backupCache.getHostArtifact(artifact); + } + return file; } } diff --git a/packages/flutter_tools/lib/src/asset.dart b/packages/flutter_tools/lib/src/asset.dart index 8fb68115d21..41c2d531ac2 100644 --- a/packages/flutter_tools/lib/src/asset.dart +++ b/packages/flutter_tools/lib/src/asset.dart @@ -409,11 +409,10 @@ class ManifestAssetBundle implements AssetBundle { final List<_Asset> materialAssets = <_Asset>[ if (flutterManifest.usesMaterialDesign) ..._getMaterialFonts(), - // For non-web platforms, include the shaders unconditionally. They are + // For all platforms, include the shaders unconditionally. They are // small, and whether they're used is determined only by the app source // code and not by the Flutter manifest. - if (targetPlatform != TargetPlatform.web_javascript) - ..._getMaterialShaders(), + ..._getMaterialShaders(), ]; for (final _Asset asset in materialAssets) { final File assetFile = asset.lookupAssetFile(_fileSystem); @@ -754,23 +753,19 @@ class ManifestAssetBundle implements AssetBundle { } } - // No shader compilation for the web. - if (targetPlatform != TargetPlatform.web_javascript) { - for (final Uri shaderUri in flutterManifest.shaders) { - _parseAssetFromFile( - packageConfig, - flutterManifest, - assetBase, - cache, - result, - shaderUri, - packageName: packageName, - attributedPackage: attributedPackage, - assetKind: AssetKind.shader, - ); - } + for (final Uri shaderUri in flutterManifest.shaders) { + _parseAssetFromFile( + packageConfig, + flutterManifest, + assetBase, + cache, + result, + shaderUri, + packageName: packageName, + attributedPackage: attributedPackage, + assetKind: AssetKind.shader, + ); } - // Add assets referenced in the fonts section of the manifest. for (final Font font in flutterManifest.fonts) { for (final FontAsset fontAsset in font.fontAssets) { 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 4d7ff2a1b9b..6fdf81bd130 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/assets.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/assets.dart @@ -128,6 +128,7 @@ Future copyAssets( input: content.file as File, outputPath: file.path, target: shaderTarget, + json: targetPlatform == TargetPlatform.web_javascript, ); break; } 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 b63307f737f..a1721a097ce 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 @@ -47,6 +47,7 @@ class DevelopmentShaderCompiler { late ShaderTarget _shaderTarget; bool _debugConfigured = false; + bool _jsonMode = false; /// Configure the output format of the shader compiler for a particular /// flutter device. @@ -69,9 +70,13 @@ class DevelopmentShaderCompiler { case TargetPlatform.fuchsia_arm64: case TargetPlatform.fuchsia_x64: case TargetPlatform.tester: + assert(!enableImpeller); + _shaderTarget = ShaderTarget.sksl; + break; case TargetPlatform.web_javascript: assert(!enableImpeller); _shaderTarget = ShaderTarget.sksl; + _jsonMode = true; break; case null: return; @@ -102,6 +107,7 @@ class DevelopmentShaderCompiler { outputPath: output.path, target: _shaderTarget, fatal: false, + json: _jsonMode, ); if (!success) { return null; @@ -157,6 +163,7 @@ class ShaderCompiler { required String outputPath, required ShaderTarget target, bool fatal = true, + required bool json, }) async { final File impellerc = _fs.file( _artifacts.getHostArtifact(HostArtifact.impellerc), @@ -172,6 +179,8 @@ class ShaderCompiler { impellerc.path, target.target, '--iplr', + if (json) + '--json', '--sl=$outputPath', '--spirv=$outputPath.spirv', '--input=${input.path}', diff --git a/packages/flutter_tools/lib/src/bundle_builder.dart b/packages/flutter_tools/lib/src/bundle_builder.dart index 50480ba6901..e14a685b767 100644 --- a/packages/flutter_tools/lib/src/bundle_builder.dart +++ b/packages/flutter_tools/lib/src/bundle_builder.dart @@ -134,9 +134,10 @@ Future buildAssets({ Future writeBundle( Directory bundleDir, Map assetEntries, - Map entryKinds, - { Logger? loggerOverride } -) async { + Map entryKinds, { + Logger? loggerOverride, + required TargetPlatform targetPlatform, +}) async { loggerOverride ??= globals.logger; if (bundleDir.existsSync()) { try { @@ -185,6 +186,7 @@ Future writeBundle( input: input, outputPath: file.path, target: ShaderTarget.sksl, // TODO(zanderso): configure impeller target when enabled. + json: targetPlatform == TargetPlatform.web_javascript, ); break; } diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart index b2c5a335d16..69240bbaa06 100644 --- a/packages/flutter_tools/lib/src/commands/test.dart +++ b/packages/flutter_tools/lib/src/commands/test.dart @@ -502,8 +502,12 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { throwToolExit('Error: Failed to build asset bundle'); } if (_needRebuild(assetBundle.entries)) { - await writeBundle(globals.fs.directory(globals.fs.path.join('build', 'unit_test_assets')), - assetBundle.entries, assetBundle.entryKinds); + await writeBundle( + globals.fs.directory(globals.fs.path.join('build', 'unit_test_assets')), + assetBundle.entries, + assetBundle.entryKinds, + targetPlatform: TargetPlatform.tester, + ); } } diff --git a/packages/flutter_tools/lib/src/isolated/devfs_web.dart b/packages/flutter_tools/lib/src/isolated/devfs_web.dart index 54d416c8db3..6e6fc1389b9 100644 --- a/packages/flutter_tools/lib/src/isolated/devfs_web.dart +++ b/packages/flutter_tools/lib/src/isolated/devfs_web.dart @@ -868,6 +868,7 @@ class WebDevFS implements DevFS { globals.fs.directory(getAssetBuildDirectory()), bundle.entries, bundle.entryKinds, + targetPlatform: TargetPlatform.web_javascript, ); } } diff --git a/packages/flutter_tools/test/general.shard/artifacts_test.dart b/packages/flutter_tools/test/general.shard/artifacts_test.dart index 2e00ce86468..2fdb19ea6eb 100644 --- a/packages/flutter_tools/test/general.shard/artifacts_test.dart +++ b/packages/flutter_tools/test/general.shard/artifacts_test.dart @@ -322,6 +322,13 @@ void main() { fileSystem.path.join('/out', 'host_debug_unopt', 'dart-sdk', 'bin', 'snapshots', 'frontend_server.dart.snapshot') ); + + + fileSystem.file(fileSystem.path.join('/out', 'host_debug_unopt', 'impellerc')) + .createSync(recursive: true); + fileSystem.file(fileSystem.path.join('/out', 'host_debug_unopt', 'libtessellator.so')) + .createSync(recursive: true); + expect( artifacts.getHostArtifact(HostArtifact.impellerc).path, fileSystem.path.join('/out', 'host_debug_unopt', 'impellerc'), @@ -332,6 +339,17 @@ void main() { ); }); + testWithoutContext('falls back to bundled impeller artifacts if the files do not exist in the local engine', () { + expect( + artifacts.getHostArtifact(HostArtifact.impellerc).path, + fileSystem.path.join('root', 'bin', 'cache', 'artifacts', 'engine', 'linux-x64', 'impellerc'), + ); + expect( + artifacts.getHostArtifact(HostArtifact.libtessellator).path, + fileSystem.path.join('root', 'bin', 'cache', 'artifacts', 'engine', 'linux-x64', 'libtessellator.so'), + ); + }); + testWithoutContext('falls back to prebuilt dart sdk', () { final String failureMessage = 'Unable to find a built dart sdk at:' ' "${fileSystem.path.join('/out', 'host_debug_unopt', 'dart-sdk')}"' 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 2c6fbaf17f4..61d31e63640 100644 --- a/packages/flutter_tools/test/general.shard/asset_bundle_test.dart +++ b/packages/flutter_tools/test/general.shard/asset_bundle_test.dart @@ -313,6 +313,7 @@ flutter: {}, {}, loggerOverride: testLogger, + targetPlatform: TargetPlatform.android, ); expect(testLogger.warningText, contains('Expected Error Text')); @@ -434,6 +435,7 @@ flutter: bundle.entries, bundle.entryKinds, loggerOverride: testLogger, + targetPlatform: TargetPlatform.android, ); }, overrides: { @@ -459,7 +461,7 @@ flutter: ]), }); - testUsingContext('Included shaders are not compiled for the web', () async { + testUsingContext('Included shaders are compiled for the web', () async { fileSystem.file('.packages').createSync(); fileSystem.file('pubspec.yaml') ..createSync() @@ -478,18 +480,34 @@ flutter: bundle.entries, bundle.entryKinds, loggerOverride: testLogger, + targetPlatform: TargetPlatform.web_javascript, ); }, overrides: { Artifacts: () => artifacts, FileSystem: () => fileSystem, ProcessManager: () => FakeProcessManager.list([ - // No impeller commands are expected here because shader compilation is - // not supposed to happen for the web. + FakeCommand( + command: [ + impellerc, + '--sksl', + '--iplr', + '--json', + '--sl=$outputPath', + '--spirv=$outputPath.spirv', + '--input=/$shaderPath', + '--input-type=frag', + '--include=/$assetsPath', + ], + onRun: () { + fileSystem.file(outputPath).createSync(recursive: true); + fileSystem.file('$outputPath.spirv').createSync(recursive: true); + }, + ), ]), }); - testUsingContext('Material shaders are not compiled for the web', () async { + testUsingContext('Material shaders are compiled for the web', () async { fileSystem.file('.packages').createSync(); final String materialIconsPath = fileSystem.path.join( @@ -508,6 +526,25 @@ flutter: materialDir.childFile(shader).createSync(recursive: true); } + (globals.processManager as FakeProcessManager) + .addCommand(FakeCommand( + command: [ + impellerc, + '--sksl', + '--iplr', + '--json', + '--sl=${fileSystem.path.join(output.path, 'shaders', 'ink_sparkle.frag')}', + '--spirv=${fileSystem.path.join(output.path, 'shaders', 'ink_sparkle.frag.spirv')}', + '--input=${fileSystem.path.join(materialDir.path, 'shaders', 'ink_sparkle.frag')}', + '--input-type=frag', + '--include=${fileSystem.path.join(materialDir.path, 'shaders')}', + ], + onRun: () { + fileSystem.file(outputPath).createSync(recursive: true); + fileSystem.file('$outputPath.spirv').createSync(recursive: true); + }, + )); + fileSystem.file('pubspec.yaml') ..createSync() ..writeAsStringSync(r''' @@ -524,15 +561,13 @@ flutter: bundle.entries, bundle.entryKinds, loggerOverride: testLogger, + targetPlatform: TargetPlatform.web_javascript, ); }, overrides: { Artifacts: () => artifacts, FileSystem: () => fileSystem, - ProcessManager: () => FakeProcessManager.list([ - // No impeller commands are expected here because shader compilation is - // not supposed to happen for the web. - ]), + ProcessManager: () => FakeProcessManager.list([]), }); }); 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 7fca2959858..198bebb3287 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 @@ -69,6 +69,7 @@ void main() { input: fileSystem.file(fragPath), outputPath: outputPath, target: ShaderTarget.sksl, + json: false, ), true, ); @@ -106,6 +107,7 @@ void main() { input: fileSystem.file(fragPath), outputPath: outputPath, target: ShaderTarget.impelleriOS, + json: false, ), true, ); @@ -142,6 +144,7 @@ void main() { input: fileSystem.file(fragPath), outputPath: outputPath, target: ShaderTarget.impellerAndroid, + json: false, ), true, ); @@ -179,6 +182,7 @@ void main() { input: fileSystem.file(notFragPath), outputPath: outputPath, target: ShaderTarget.sksl, + json: false, ), true, ); @@ -216,6 +220,7 @@ void main() { input: fileSystem.file(notFragPath), outputPath: outputPath, target: ShaderTarget.sksl, + json: false, ); fail('unreachable'); } on ShaderCompilerException catch (e) { @@ -313,4 +318,49 @@ void main() { expect(fileSystem.file('/.tmp_rand0/0.8255140718871702.temp.spirv'), isNot(exists)); expect(fileSystem.file('/.tmp_rand0/0.8255140718871702.temp'), isNot(exists)); }); + + testWithoutContext('DevelopmentShaderCompiler can compile JSON for web targts', () async { + final FakeProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: [ + impellerc, + '--sksl', + '--iplr', + '--json', + '--sl=/.tmp_rand0/0.8255140718871702.temp', + '--spirv=/.tmp_rand0/0.8255140718871702.temp.spirv', + '--input=$fragPath', + '--input-type=frag', + '--include=$fragDir', + ], + 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.web_javascript, enableImpeller: false); + + final DevFSContent? content = await developmentShaderCompiler + .recompileShader(DevFSFileContent(fileSystem.file(fragPath))); + + expect(await content!.contentsAsBytes(), [1, 2, 3, 4]); + expect(fileSystem.file('/.tmp_rand0/0.8255140718871702.temp.spirv'), isNot(exists)); + expect(fileSystem.file('/.tmp_rand0/0.8255140718871702.temp'), isNot(exists)); + }); } diff --git a/packages/flutter_tools/test/integration.shard/shader_compiler_test.dart b/packages/flutter_tools/test/integration.shard/shader_compiler_test.dart index 8f82e2733fa..3a950068b61 100644 --- a/packages/flutter_tools/test/integration.shard/shader_compiler_test.dart +++ b/packages/flutter_tools/test/integration.shard/shader_compiler_test.dart @@ -43,6 +43,7 @@ void main() { input: globals.fs.file(inkSparklePath), outputPath: inkSparkleOutputPath, target: ShaderTarget.sksl, + json: false, ); final File resultFile = globals.fs.file(inkSparkleOutputPath);