diff --git a/engine/src/flutter/impeller/compiler/compiler.cc b/engine/src/flutter/impeller/compiler/compiler.cc index ee6e84afafe..89d9ab20dfb 100644 --- a/engine/src/flutter/impeller/compiler/compiler.cc +++ b/engine/src/flutter/impeller/compiler/compiler.cc @@ -242,7 +242,6 @@ static CompilerBackend CreateCompiler(const spirv_cross::ParsedIR& ir, case TargetPlatform::kRuntimeStageVulkan: compiler = CreateVulkanCompiler(ir, source_options); break; - case TargetPlatform::kUnknown: case TargetPlatform::kOpenGLES: case TargetPlatform::kOpenGLDesktop: case TargetPlatform::kRuntimeStageGLES: @@ -251,6 +250,9 @@ static CompilerBackend CreateCompiler(const spirv_cross::ParsedIR& ir, break; case TargetPlatform::kSkSL: compiler = CreateSkSLCompiler(ir, source_options); + break; + case TargetPlatform::kUnknown: + FML_UNREACHABLE(); } if (!compiler) { return {}; diff --git a/engine/src/flutter/impeller/compiler/compiler_test.cc b/engine/src/flutter/impeller/compiler/compiler_test.cc index 683e6bea578..5dc1d566ad4 100644 --- a/engine/src/flutter/impeller/compiler/compiler_test.cc +++ b/engine/src/flutter/impeller/compiler/compiler_test.cc @@ -142,53 +142,50 @@ bool CompilerTestBase::CanCompileAndReflect( return false; } - if (TargetPlatformNeedsReflection(GetParam())) { - auto reflector = compiler.GetReflector(); - if (!reflector) { - VALIDATION_LOG - << "No reflector was found for target platform SL compiler."; - return false; - } + auto reflector = compiler.GetReflector(); + if (!reflector) { + VALIDATION_LOG << "No reflector was found for target platform SL compiler."; + return false; + } - auto reflection_json = reflector->GetReflectionJSON(); - auto reflection_header = reflector->GetReflectionHeader(); - auto reflection_source = reflector->GetReflectionCC(); + auto reflection_json = reflector->GetReflectionJSON(); + auto reflection_header = reflector->GetReflectionHeader(); + auto reflection_source = reflector->GetReflectionCC(); - if (!reflection_json) { - VALIDATION_LOG << "Reflection JSON was not found."; - return false; - } + if (!reflection_json) { + VALIDATION_LOG << "Reflection JSON was not found."; + return false; + } - if (!reflection_header) { - VALIDATION_LOG << "Reflection header was not found."; - return false; - } + if (!reflection_header) { + VALIDATION_LOG << "Reflection header was not found."; + return false; + } - if (!reflection_source) { - VALIDATION_LOG << "Reflection source was not found."; - return false; - } + if (!reflection_source) { + VALIDATION_LOG << "Reflection source was not found."; + return false; + } - if (!fml::WriteAtomically(intermediates_directory_, - ReflectionHeaderName(fixture_name).c_str(), - *reflection_header)) { - VALIDATION_LOG << "Could not write reflection header intermediates."; - return false; - } + if (!fml::WriteAtomically(intermediates_directory_, + ReflectionHeaderName(fixture_name).c_str(), + *reflection_header)) { + VALIDATION_LOG << "Could not write reflection header intermediates."; + return false; + } - if (!fml::WriteAtomically(intermediates_directory_, - ReflectionCCName(fixture_name).c_str(), - *reflection_source)) { - VALIDATION_LOG << "Could not write reflection CC intermediates."; - return false; - } + if (!fml::WriteAtomically(intermediates_directory_, + ReflectionCCName(fixture_name).c_str(), + *reflection_source)) { + VALIDATION_LOG << "Could not write reflection CC intermediates."; + return false; + } - if (!fml::WriteAtomically(intermediates_directory_, - ReflectionJSONName(fixture_name).c_str(), - *reflection_json)) { - VALIDATION_LOG << "Could not write reflection json intermediates."; - return false; - } + if (!fml::WriteAtomically(intermediates_directory_, + ReflectionJSONName(fixture_name).c_str(), + *reflection_json)) { + VALIDATION_LOG << "Could not write reflection json intermediates."; + return false; } return true; } diff --git a/engine/src/flutter/impeller/compiler/compiler_test.h b/engine/src/flutter/impeller/compiler/compiler_test.h index 6e3f507f950..a8da01cc55d 100644 --- a/engine/src/flutter/impeller/compiler/compiler_test.h +++ b/engine/src/flutter/impeller/compiler/compiler_test.h @@ -45,9 +45,11 @@ class CompilerTestBase : public ::testing::TestWithParam { class CompilerTest : public CompilerTestBase {}; +class CompilerTestRuntime : public CompilerTestBase {}; + class CompilerTestSkSL : public CompilerTestBase {}; -class CompilerTestRuntime : public CompilerTestBase {}; +class CompilerTestUnknownPlatform : public CompilerTestBase {}; } // namespace testing } // namespace compiler diff --git a/engine/src/flutter/impeller/compiler/compiler_unittests.cc b/engine/src/flutter/impeller/compiler/compiler_unittests.cc index 71750ba41f2..c680558d0d6 100644 --- a/engine/src/flutter/impeller/compiler/compiler_unittests.cc +++ b/engine/src/flutter/impeller/compiler/compiler_unittests.cc @@ -15,6 +15,46 @@ namespace impeller { namespace compiler { namespace testing { +INSTANTIATE_TEST_SUITE_P( + CompilerSuite, + CompilerTest, + ::testing::Values(TargetPlatform::kOpenGLES, + TargetPlatform::kOpenGLDesktop, + TargetPlatform::kMetalDesktop, + TargetPlatform::kMetalIOS, + TargetPlatform::kVulkan), + [](const ::testing::TestParamInfo& info) { + return TargetPlatformToString(info.param); + }); + +INSTANTIATE_TEST_SUITE_P( + CompilerSuite, + CompilerTestRuntime, + ::testing::Values(TargetPlatform::kRuntimeStageMetal, + TargetPlatform::kRuntimeStageGLES, + TargetPlatform::kRuntimeStageGLES3, + TargetPlatform::kRuntimeStageVulkan, + TargetPlatform::kSkSL), + [](const ::testing::TestParamInfo& info) { + return TargetPlatformToString(info.param); + }); + +INSTANTIATE_TEST_SUITE_P( + CompilerSuite, + CompilerTestSkSL, + ::testing::Values(TargetPlatform::kSkSL), + [](const ::testing::TestParamInfo& info) { + return TargetPlatformToString(info.param); + }); + +INSTANTIATE_TEST_SUITE_P( + CompilerSuite, + CompilerTestUnknownPlatform, + ::testing::Values(TargetPlatform::kUnknown), + [](const ::testing::TestParamInfo& info) { + return TargetPlatformToString(info.param); + }); + TEST(CompilerTest, Defines) { std::shared_ptr fixture = flutter::testing::OpenFixtureAsMapping("check_gles_definition.frag"); @@ -48,9 +88,6 @@ TEST(CompilerTest, ShaderKindMatchingIsSuccessful) { } TEST_P(CompilerTest, CanCompile) { - if (GetParam() == TargetPlatform::kSkSL) { - GTEST_SKIP() << "Not supported with SkSL"; - } ASSERT_TRUE(CanCompileAndReflect("sample.vert")); ASSERT_TRUE(CanCompileAndReflect("sample.vert", SourceType::kVertexShader)); ASSERT_TRUE(CanCompileAndReflect("sample.vert", SourceType::kVertexShader, @@ -58,17 +95,11 @@ TEST_P(CompilerTest, CanCompile) { } TEST_P(CompilerTest, CanCompileHLSL) { - if (GetParam() == TargetPlatform::kSkSL) { - GTEST_SKIP() << "Not supported with SkSL"; - } ASSERT_TRUE(CanCompileAndReflect( "simple.vert.hlsl", SourceType::kVertexShader, SourceLanguage::kHLSL)); } TEST_P(CompilerTest, CanCompileHLSLWithMultipleStages) { - if (GetParam() == TargetPlatform::kSkSL) { - GTEST_SKIP() << "Not supported with SkSL"; - } ASSERT_TRUE(CanCompileAndReflect("multiple_stages.hlsl", SourceType::kVertexShader, SourceLanguage::kHLSL, "VertexShader")); @@ -87,18 +118,12 @@ TEST_P(CompilerTest, CanCompileComputeShader) { } TEST_P(CompilerTest, MustFailDueToExceedingResourcesLimit) { - if (GetParam() == TargetPlatform::kSkSL) { - GTEST_SKIP() << "Not supported with SkSL"; - } ScopedValidationDisable disable_validation; ASSERT_FALSE( CanCompileAndReflect("resources_limit.vert", SourceType::kVertexShader)); } TEST_P(CompilerTest, MustFailDueToMultipleLocationPerStructMember) { - if (GetParam() == TargetPlatform::kSkSL) { - GTEST_SKIP() << "Not supported with SkSL"; - } ScopedValidationDisable disable_validation; ASSERT_FALSE(CanCompileAndReflect("struct_def_bug.vert")); } @@ -202,6 +227,12 @@ inline std::ostream& operator<<(std::ostream& out, const UniformInfo& info) { } // namespace TEST_P(CompilerTestRuntime, UniformsAppearInJson) { + if (GetParam() == TargetPlatform::kRuntimeStageVulkan) { + // TODO(https://github.com/flutter/flutter/issues/182578): Investigate why + // this does not pass. + GTEST_SKIP() << "Not supported with Vulkan"; + } + ASSERT_TRUE(CanCompileAndReflect("sample_with_uniforms.frag", SourceType::kFragmentShader, SourceLanguage::kGLSL)); @@ -248,6 +279,12 @@ TEST_P(CompilerTestRuntime, UniformsAppearInJson) { } TEST_P(CompilerTestRuntime, PositionedUniformsAppearInJson) { + if (GetParam() == TargetPlatform::kRuntimeStageVulkan) { + // TODO(https://github.com/flutter/flutter/issues/182578): Investigate why + // this does not pass. + GTEST_SKIP() << "Not supported with Vulkan"; + } + ASSERT_TRUE(CanCompileAndReflect("sample_with_positioned_uniforms.frag", SourceType::kFragmentShader, SourceLanguage::kGLSL)); @@ -377,39 +414,11 @@ TEST_P(CompilerTestSkSL, CompilesWithValidArrayInitialization) { SourceType::kFragmentShader)); } -#define INSTANTIATE_TARGET_PLATFORM_TEST_SUITE_P(suite_name) \ - INSTANTIATE_TEST_SUITE_P( \ - suite_name, CompilerTest, \ - ::testing::Values(TargetPlatform::kOpenGLES, \ - TargetPlatform::kOpenGLDesktop, \ - TargetPlatform::kMetalDesktop, \ - TargetPlatform::kMetalIOS, TargetPlatform::kSkSL), \ - [](const ::testing::TestParamInfo& info) { \ - return TargetPlatformToString(info.param); \ - }); - -INSTANTIATE_TARGET_PLATFORM_TEST_SUITE_P(CompilerSuite); - -#define INSTANTIATE_RUNTIME_TARGET_PLATFORM_TEST_SUITE_P(suite_name) \ - INSTANTIATE_TEST_SUITE_P( \ - suite_name, CompilerTestRuntime, \ - ::testing::Values(TargetPlatform::kRuntimeStageMetal), \ - [](const ::testing::TestParamInfo& info) { \ - return TargetPlatformToString(info.param); \ - }); - -INSTANTIATE_RUNTIME_TARGET_PLATFORM_TEST_SUITE_P(CompilerSuite); - -#define INSTANTIATE_SKSL_TARGET_PLATFORM_TEST_SUITE_P(suite_name) \ - INSTANTIATE_TEST_SUITE_P( \ - suite_name, CompilerTestSkSL, ::testing::Values(TargetPlatform::kSkSL), \ - [](const ::testing::TestParamInfo& info) { \ - return TargetPlatformToString(info.param); \ - }); - -INSTANTIATE_SKSL_TARGET_PLATFORM_TEST_SUITE_P(CompilerSuite); - TEST_P(CompilerTestRuntime, Mat2Reflection) { + if (GetParam() == TargetPlatform::kSkSL) { + GTEST_SKIP() << "Not supported with SkSL"; + } + ASSERT_TRUE(CanCompileAndReflect( "mat2_test.frag", SourceType::kFragmentShader, SourceLanguage::kGLSL)); @@ -448,6 +457,11 @@ TEST_P(CompilerTestRuntime, Mat2Reflection) { EXPECT_EQ(mat2Member["offset"], 0u); } +TEST_P(CompilerTestUnknownPlatform, MustFailDueToUnknownPlatform) { + ASSERT_FALSE( + CanCompileAndReflect("sample.frag", SourceType::kFragmentShader)); +} + } // namespace testing } // namespace compiler } // namespace impeller diff --git a/engine/src/flutter/impeller/compiler/impellerc_main.cc b/engine/src/flutter/impeller/compiler/impellerc_main.cc index 1e838c65cdb..6316affa7af 100644 --- a/engine/src/flutter/impeller/compiler/impellerc_main.cc +++ b/engine/src/flutter/impeller/compiler/impellerc_main.cc @@ -32,48 +32,15 @@ static Reflector::Options CreateReflectorOptions(const SourceOptions& options, return reflector_options; } -/// Run the shader compiler to geneate SkSL reflection data. -/// If there is an error, prints error text and returns `nullptr`. -static std::shared_ptr CompileSkSL( - std::shared_ptr source_file_mapping, - const Switches& switches) { - auto options = switches.CreateSourceOptions(TargetPlatform::kSkSL); - - Reflector::Options sksl_reflector_options = - CreateReflectorOptions(options, switches); - sksl_reflector_options.target_platform = TargetPlatform::kSkSL; - - Compiler sksl_compiler = - Compiler(std::move(source_file_mapping), options, sksl_reflector_options); - if (!sksl_compiler.IsValid()) { - std::cerr << "Compilation to SkSL failed." << std::endl; - std::cerr << sksl_compiler.GetErrorMessages() << std::endl; - return nullptr; - } - return sksl_compiler.GetReflector()->GetRuntimeStageShaderData(); -} - static bool OutputIPLR( const Switches& switches, const std::shared_ptr& source_file_mapping) { FML_DCHECK(switches.iplr); RuntimeStageData stages; - std::shared_ptr sksl_shader; - if (TargetPlatformBundlesSkSL(switches.SelectDefaultTargetPlatform())) { - sksl_shader = CompileSkSL(source_file_mapping, switches); - if (!sksl_shader) { - return false; - } - stages.AddShader(sksl_shader); - } - for (const auto& platform : switches.PlatformsToCompile()) { - if (platform == TargetPlatform::kSkSL) { - // Already handled above. - continue; - } - SourceOptions options = switches.CreateSourceOptions(platform); + SourceOptions options = switches.CreateSourceOptions(); + options.target_platform = platform; // Invoke the compiler and generate reflection data for a single shader. @@ -81,7 +48,8 @@ static bool OutputIPLR( CreateReflectorOptions(options, switches); Compiler compiler(source_file_mapping, options, reflector_options); if (!compiler.IsValid()) { - std::cerr << "Compilation failed." << std::endl; + std::cerr << "Compilation failed for target: " + << TargetPlatformToString(platform) << std::endl; std::cerr << compiler.GetErrorMessages() << std::endl; return false; } @@ -149,43 +117,40 @@ static bool OutputReflectionData(const Compiler& compiler, /// May include a JSON file, a C++ header, and/or a C++ TU. /// - if (TargetPlatformNeedsReflection(options.target_platform)) { - if (!switches.reflection_json_name.empty()) { - auto reflection_json_name = std::filesystem::absolute( - std::filesystem::current_path() / switches.reflection_json_name); - if (!fml::WriteAtomically( - *switches.working_directory, - Utf8FromPath(reflection_json_name).c_str(), - *compiler.GetReflector()->GetReflectionJSON())) { - std::cerr << "Could not write reflection json to " - << switches.reflection_json_name << std::endl; - return false; - } + if (!switches.reflection_json_name.empty()) { + auto reflection_json_name = std::filesystem::absolute( + std::filesystem::current_path() / switches.reflection_json_name); + if (!fml::WriteAtomically(*switches.working_directory, + Utf8FromPath(reflection_json_name).c_str(), + *compiler.GetReflector()->GetReflectionJSON())) { + std::cerr << "Could not write reflection json to " + << switches.reflection_json_name << std::endl; + return false; } + } - if (!switches.reflection_header_name.empty()) { - auto reflection_header_name = std::filesystem::absolute( - std::filesystem::current_path() / switches.reflection_header_name); - if (!fml::WriteAtomically( - *switches.working_directory, - Utf8FromPath(reflection_header_name).c_str(), - *compiler.GetReflector()->GetReflectionHeader())) { - std::cerr << "Could not write reflection header to " - << switches.reflection_header_name << std::endl; - return false; - } + if (!switches.reflection_header_name.empty()) { + auto reflection_header_name = std::filesystem::absolute( + std::filesystem::current_path() / switches.reflection_header_name); + if (!fml::WriteAtomically( + *switches.working_directory, + Utf8FromPath(reflection_header_name).c_str(), + *compiler.GetReflector()->GetReflectionHeader())) { + std::cerr << "Could not write reflection header to " + << switches.reflection_header_name << std::endl; + return false; } + } - if (!switches.reflection_cc_name.empty()) { - auto reflection_cc_name = std::filesystem::absolute( - std::filesystem::current_path() / switches.reflection_cc_name); - if (!fml::WriteAtomically(*switches.working_directory, - Utf8FromPath(reflection_cc_name).c_str(), - *compiler.GetReflector()->GetReflectionCC())) { - std::cerr << "Could not write reflection CC to " - << switches.reflection_cc_name << std::endl; - return false; - } + if (!switches.reflection_cc_name.empty()) { + auto reflection_cc_name = std::filesystem::absolute( + std::filesystem::current_path() / switches.reflection_cc_name); + if (!fml::WriteAtomically(*switches.working_directory, + Utf8FromPath(reflection_cc_name).c_str(), + *compiler.GetReflector()->GetReflectionCC())) { + std::cerr << "Could not write reflection CC to " + << switches.reflection_cc_name << std::endl; + return false; } } return true; @@ -197,24 +162,7 @@ static bool OutputDepfile(const Compiler& compiler, const Switches& switches) { /// if (!switches.depfile_path.empty()) { - std::string result_file; - switch (switches.SelectDefaultTargetPlatform()) { - case TargetPlatform::kMetalDesktop: - case TargetPlatform::kMetalIOS: - case TargetPlatform::kOpenGLES: - case TargetPlatform::kOpenGLDesktop: - case TargetPlatform::kRuntimeStageMetal: - case TargetPlatform::kRuntimeStageGLES: - case TargetPlatform::kRuntimeStageGLES3: - case TargetPlatform::kRuntimeStageVulkan: - case TargetPlatform::kSkSL: - case TargetPlatform::kVulkan: - result_file = Utf8FromPath(switches.sl_file_name); - break; - case TargetPlatform::kUnknown: - result_file = Utf8FromPath(switches.spirv_file_name); - break; - } + std::string result_file = Utf8FromPath(switches.sl_file_name); auto depfile_path = std::filesystem::absolute( std::filesystem::current_path() / switches.depfile_path); if (!fml::WriteAtomically(*switches.working_directory, @@ -264,6 +212,11 @@ bool Main(const fml::CommandLine& command_line) { // depfile. SourceOptions options = switches.CreateSourceOptions(); + // If there are multiple platform compile targets, the specific target + // platform that is used does not matter because the output files won't depend + // on the target platform. Arbitrarily choose the first one from + // PlatformsToCompile(). + options.target_platform = switches.PlatformsToCompile().front(); // Invoke the compiler and generate reflection data for a single shader. diff --git a/engine/src/flutter/impeller/compiler/switches.cc b/engine/src/flutter/impeller/compiler/switches.cc index 8c42598ad47..057bc86aa39 100644 --- a/engine/src/flutter/impeller/compiler/switches.cc +++ b/engine/src/flutter/impeller/compiler/switches.cc @@ -25,12 +25,13 @@ static const std::map kKnownPlatforms = { {"opengl-desktop", TargetPlatform::kOpenGLDesktop}, }; -static const std::map kKnownRuntimeStages = { - {"sksl", TargetPlatform::kSkSL}, - {"runtime-stage-metal", TargetPlatform::kRuntimeStageMetal}, - {"runtime-stage-gles", TargetPlatform::kRuntimeStageGLES}, - {"runtime-stage-gles3", TargetPlatform::kRuntimeStageGLES3}, - {"runtime-stage-vulkan", TargetPlatform::kRuntimeStageVulkan}, +static const std::vector> + kKnownRuntimeStages = { + {"sksl", TargetPlatform::kSkSL}, + {"runtime-stage-metal", TargetPlatform::kRuntimeStageMetal}, + {"runtime-stage-gles", TargetPlatform::kRuntimeStageGLES}, + {"runtime-stage-gles3", TargetPlatform::kRuntimeStageGLES3}, + {"runtime-stage-vulkan", TargetPlatform::kRuntimeStageVulkan}, }; static const std::map kKnownSourceTypes = { @@ -309,19 +310,8 @@ std::vector Switches::PlatformsToCompile() const { return {target_platform_}; } -TargetPlatform Switches::SelectDefaultTargetPlatform() const { - if (target_platform_ == TargetPlatform::kUnknown && - !runtime_stages_.empty()) { - return runtime_stages_.front(); - } - return target_platform_; -} - -SourceOptions Switches::CreateSourceOptions( - std::optional target_platform) const { +SourceOptions Switches::CreateSourceOptions() const { SourceOptions options; - options.target_platform = - target_platform.value_or(SelectDefaultTargetPlatform()); options.source_language = source_language; if (input_type == SourceType::kUnknown) { options.type = SourceTypeFromFileName(source_file_name); diff --git a/engine/src/flutter/impeller/compiler/switches.h b/engine/src/flutter/impeller/compiler/switches.h index 689bd265358..044659d02f5 100644 --- a/engine/src/flutter/impeller/compiler/switches.h +++ b/engine/src/flutter/impeller/compiler/switches.h @@ -56,12 +56,12 @@ class Switches { /// A vector containing at least one valid platform. std::vector PlatformsToCompile() const; - TargetPlatform SelectDefaultTargetPlatform() const; - // Creates source options from these switches for the specified - // TargetPlatform. Uses SelectDefaultTargetPlatform if not specified. - SourceOptions CreateSourceOptions( - std::optional target_platform = std::nullopt) const; + // Creates source options from these switches. The returned options does not + // have a set TargetPlatform because that cannot be determined based purely + // on the switches. Clients must set a valid TargetPlatform on the returned + // options before before it is used. + SourceOptions CreateSourceOptions() const; static void PrintHelp(std::ostream& stream); diff --git a/engine/src/flutter/impeller/compiler/switches_unittests.cc b/engine/src/flutter/impeller/compiler/switches_unittests.cc index e8a8da1d902..6372809a764 100644 --- a/engine/src/flutter/impeller/compiler/switches_unittests.cc +++ b/engine/src/flutter/impeller/compiler/switches_unittests.cc @@ -10,6 +10,7 @@ #include "flutter/fml/string_conversion.h" #include "flutter/testing/testing.h" #include "impeller/compiler/switches.h" +#include "impeller/compiler/types.h" #include "impeller/compiler/utilities.h" namespace impeller { diff --git a/engine/src/flutter/impeller/compiler/types.cc b/engine/src/flutter/impeller/compiler/types.cc index c570e1244ca..57723c372a4 100644 --- a/engine/src/flutter/impeller/compiler/types.cc +++ b/engine/src/flutter/impeller/compiler/types.cc @@ -127,25 +127,6 @@ std::string EntryPointFunctionNameFromSourceName( return stream.str(); } -bool TargetPlatformNeedsReflection(TargetPlatform platform) { - switch (platform) { - case TargetPlatform::kMetalIOS: - case TargetPlatform::kMetalDesktop: - case TargetPlatform::kOpenGLES: - case TargetPlatform::kOpenGLDesktop: - case TargetPlatform::kRuntimeStageMetal: - case TargetPlatform::kRuntimeStageGLES: - case TargetPlatform::kRuntimeStageGLES3: - case TargetPlatform::kRuntimeStageVulkan: - case TargetPlatform::kVulkan: - return true; - case TargetPlatform::kUnknown: - case TargetPlatform::kSkSL: - return false; - } - FML_UNREACHABLE(); -} - std::string ShaderCErrorToString(shaderc_compilation_status status) { using Status = shaderc_compilation_status; switch (status) { @@ -312,24 +293,5 @@ bool TargetPlatformIsVulkan(TargetPlatform platform) { FML_UNREACHABLE(); } -bool TargetPlatformBundlesSkSL(TargetPlatform platform) { - switch (platform) { - case TargetPlatform::kSkSL: - case TargetPlatform::kRuntimeStageMetal: - case TargetPlatform::kRuntimeStageGLES: - case TargetPlatform::kRuntimeStageGLES3: - case TargetPlatform::kRuntimeStageVulkan: - return true; - case TargetPlatform::kMetalDesktop: - case TargetPlatform::kMetalIOS: - case TargetPlatform::kUnknown: - case TargetPlatform::kOpenGLES: - case TargetPlatform::kOpenGLDesktop: - case TargetPlatform::kVulkan: - return false; - } - FML_UNREACHABLE(); -} - } // namespace compiler } // namespace impeller diff --git a/engine/src/flutter/impeller/compiler/types.h b/engine/src/flutter/impeller/compiler/types.h index 02ef699d14f..7adddff9e26 100644 --- a/engine/src/flutter/impeller/compiler/types.h +++ b/engine/src/flutter/impeller/compiler/types.h @@ -122,10 +122,6 @@ std::string EntryPointFunctionNameFromSourceName( SourceLanguage source_language, const std::string& entry_point_name); -bool TargetPlatformNeedsReflection(TargetPlatform platform); - -bool TargetPlatformBundlesSkSL(TargetPlatform platform); - std::string ShaderCErrorToString(shaderc_compilation_status status); shaderc_shader_kind ToShaderCShaderKind(SourceType type); diff --git a/engine/src/flutter/impeller/fixtures/BUILD.gn b/engine/src/flutter/impeller/fixtures/BUILD.gn index 73309350e13..cba95658798 100644 --- a/engine/src/flutter/impeller/fixtures/BUILD.gn +++ b/engine/src/flutter/impeller/fixtures/BUILD.gn @@ -104,6 +104,22 @@ impellerc("runtime_stages") { ] sl_file_extension = "iplr" + shader_target_flags = [ + "--sksl", + "--runtime-stage-metal", + "--runtime-stage-gles", + "--runtime-stage-gles3", + "--runtime-stage-vulkan", + ] + + iplr = true +} + +impellerc("runtime_stages_non_sksl") { + mnemonic = "IMPELLERC_IPLR" + shaders = [ "runtime_stage_simple_no_sksl.frag" ] + sl_file_extension = "iplr" + shader_target_flags = [ "--runtime-stage-metal", "--runtime-stage-gles", @@ -172,7 +188,12 @@ test_fixtures("file_fixtures") { } fixtures += filter_include(get_target_outputs(":runtime_stages"), [ "*.iplr" ]) - deps = [ ":runtime_stages" ] + fixtures += filter_include(get_target_outputs(":runtime_stages_non_sksl"), + [ "*.iplr" ]) + deps = [ + ":runtime_stages", + ":runtime_stages_non_sksl", + ] } impellerc("flutter_gpu_shaders") { diff --git a/engine/src/flutter/impeller/fixtures/runtime_stage_simple_no_sksl.frag b/engine/src/flutter/impeller/fixtures/runtime_stage_simple_no_sksl.frag new file mode 100644 index 00000000000..38b00be8d32 --- /dev/null +++ b/engine/src/flutter/impeller/fixtures/runtime_stage_simple_no_sksl.frag @@ -0,0 +1,9 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +out vec4 frag_color; + +void main() { + frag_color = vec4(1.0, 0.0, 0.0, 1.0); +} diff --git a/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc b/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc index 36e7ad4c76d..456c56470b5 100644 --- a/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc +++ b/engine/src/flutter/impeller/runtime_stage/runtime_stage_unittests.cc @@ -471,12 +471,18 @@ TEST_P(RuntimeStageTest, ContainsExpectedShaderTypes) { auto stages_result = OpenAssetAsRuntimeStage("ink_sparkle.frag.iplr"); ABSL_ASSERT_OK(stages_result); auto stages = stages_result.value(); - // Right now, SkSL gets implicitly bundled regardless of what the build rule - // for this test requested. After - // https://github.com/flutter/flutter/issues/138919, this may require a build - // rule change or a new test. EXPECT_TRUE(stages[RuntimeStageBackend::kSkSL]); + EXPECT_TRUE(stages[RuntimeStageBackend::kOpenGLES]); + EXPECT_TRUE(stages[RuntimeStageBackend::kMetal]); + EXPECT_TRUE(stages[RuntimeStageBackend::kVulkan]); +} +TEST_P(RuntimeStageTest, ContainsExpectedShaderTypesNoSksl) { + auto stages_result = + OpenAssetAsRuntimeStage("runtime_stage_simple_no_sksl.frag.iplr"); + ABSL_ASSERT_OK(stages_result); + auto stages = stages_result.value(); + EXPECT_FALSE(stages[RuntimeStageBackend::kSkSL]); EXPECT_TRUE(stages[RuntimeStageBackend::kOpenGLES]); EXPECT_TRUE(stages[RuntimeStageBackend::kMetal]); EXPECT_TRUE(stages[RuntimeStageBackend::kVulkan]); diff --git a/engine/src/flutter/impeller/tools/compiler.gni b/engine/src/flutter/impeller/tools/compiler.gni index edaa4f9b65a..7435a7308ca 100644 --- a/engine/src/flutter/impeller/tools/compiler.gni +++ b/engine/src/flutter/impeller/tools/compiler.gni @@ -75,10 +75,6 @@ template("impellerc") { shader_target_flags = [] } - sksl = false - foreach(shader_target_flag, shader_target_flags) { - sksl = shader_target_flag == "--sksl" - } iplr = false if (defined(invoker.iplr) && invoker.iplr) { iplr = invoker.iplr @@ -91,7 +87,6 @@ template("impellerc") { # Not needed on every path. not_needed([ "iplr", - "sksl", "shader_bundle", "shader_bundle_output", ]) @@ -181,11 +176,7 @@ template("impellerc") { ] outputs = [ sl_output ] - } else if (sksl) { - # When SkSL is selected as a `shader_target_flags`, don't generate - # C++ reflection state. Nothing needs to use it and it's likely invalid - # given the special cases when generating SkSL. - # Note that this configuration is orthogonal to the "--iplr" flag + } else { sl_output = "$generated_dir/{{source_file_part}}.${invoker.sl_file_extension}" sl_output_path = rebase_path(sl_output, root_build_dir) @@ -198,40 +189,39 @@ template("impellerc") { ] outputs = [ sl_output ] - } else { - # The default branch. Here we just generate one shader along with all of - # its C++ reflection state. - sl_output = - "$generated_dir/{{source_file_part}}.${invoker.sl_file_extension}" - sl_output_path = rebase_path(sl_output, root_build_dir) + # Generate reflection state only if a non-runtime target is provided. + if (filter_include(shader_target_flags, + [ + "--metal-ios", + "--metal-desktop", + "--opengl-es", + "--opengl-desktop", + "--vulkan", + ]) != []) { + reflection_json_intermediate = + "$generated_dir/{{source_file_part}}.json" + reflection_header_intermediate = "$generated_dir/{{source_file_part}}.h" + reflection_cc_intermediate = "$generated_dir/{{source_file_part}}.cc" - reflection_json_intermediate = "$generated_dir/{{source_file_part}}.json" - reflection_header_intermediate = "$generated_dir/{{source_file_part}}.h" - reflection_cc_intermediate = "$generated_dir/{{source_file_part}}.cc" + reflection_json_path = + rebase_path(reflection_json_intermediate, root_build_dir) + reflection_header_path = + rebase_path(reflection_header_intermediate, root_build_dir) + reflection_cc_path = + rebase_path(reflection_cc_intermediate, root_build_dir) - spirv_intermediate = "$generated_dir/{{source_file_part}}.spirv" - spirv_intermediate_path = rebase_path(spirv_intermediate, root_build_dir) - reflection_json_path = - rebase_path(reflection_json_intermediate, root_build_dir) - reflection_header_path = - rebase_path(reflection_header_intermediate, root_build_dir) - reflection_cc_path = - rebase_path(reflection_cc_intermediate, root_build_dir) + args += [ + "--reflection-json=$reflection_json_path", + "--reflection-header=$reflection_header_path", + "--reflection-cc=$reflection_cc_path", + ] - args += [ - "--sl=$sl_output_path", - "--spirv=$spirv_intermediate_path", - "--reflection-json=$reflection_json_path", - "--reflection-header=$reflection_header_path", - "--reflection-cc=$reflection_cc_path", - ] - - outputs = [ - sl_output, - reflection_header_intermediate, - reflection_cc_intermediate, - ] + outputs += [ + reflection_header_intermediate, + reflection_cc_intermediate, + ] + } } if (defined(invoker.defines)) {