Fix compiler.gni logic around when to generate reflection state.

- This used to incorrectly generate reflection state whenever the last shader_target_flags is not "--sksl".
  - Instead, generate reflection state when any non-runtime target is in shader_target_flags.
  - Consolidate some of the if/else logic to reduce duplicate code.
Remove the TargetPlatformNeedsReflection check in impeller_main.cc.
  - Instead, whether reflection state is generated depends only on the presence or absense of "reflection_{json|header|cc}_name" flags.
  - The logic of whether to include these flags is already in compiler.gni. So it's redundant to also have logic for whether to generate the reflection state here.
  - The TargetPlatformNeedsReflection method had faulty logic.
    - It returned true for everything except SkSL, even though reflection state isn't needed for runtime targets.
    - It was called on the target from Switches::SelectDefaultTargetPlatform. When impellerc is used with multiple runtime targets, this would return the runtime target that is first alphabetically by flag name. So if --sksl is provided along with any other --runtime-stage-* target, SelectDefaultTargetPlatform returns the non-sksl runtime target. Effectively this meant that TargetPlatformNeedsReflection returns true except for when --sksl is the only provided runtime target.
  - Removes the TargetPlatformNeedsReflection function entirely.
This commit is contained in:
Benson Luk 2026-02-18 15:37:01 -08:00
parent c2d552f698
commit d8d6d161d7
No known key found for this signature in database
GPG Key ID: 4767132954BA452F
5 changed files with 97 additions and 134 deletions

View File

@ -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;
}

View File

@ -116,43 +116,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;

View File

@ -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) {

View File

@ -122,8 +122,6 @@ std::string EntryPointFunctionNameFromSourceName(
SourceLanguage source_language,
const std::string& entry_point_name);
bool TargetPlatformNeedsReflection(TargetPlatform platform);
std::string ShaderCErrorToString(shaderc_compilation_status status);
shaderc_shader_kind ToShaderCShaderKind(SourceType type);

View File

@ -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)) {