Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
impellerc GN rules
  • Loading branch information
bdero committed Dec 11, 2023
commit 34e063a7c6696b615a9f6345a65d99d89bc4deb9
4 changes: 2 additions & 2 deletions impeller/compiler/impellerc_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,9 @@ bool Main(const fml::CommandLine& command_line) {
options.use_half_textures = switches.use_half_textures;
options.require_framebuffer_fetch = switches.require_framebuffer_fetch;

if (!switches.iplr_bundle.empty()) {
if (!switches.shader_bundle.empty()) {
// Invoke the compiler multiple times to build a shader bundle with the
// given iplr_bundle spec.
// given shader_bundle spec.
return GenerateShaderBundle(switches, options);
}

Expand Down
2 changes: 1 addition & 1 deletion impeller/compiler/shader_bundle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ bool GenerateShaderBundle(Switches& switches, SourceOptions& options) {
///

std::optional<ShaderBundleConfig> bundle_config =
ParseShaderBundleConfig(switches.iplr_bundle);
ParseShaderBundleConfig(switches.shader_bundle);
if (!bundle_config) {
return false;
}
Expand Down
28 changes: 18 additions & 10 deletions impeller/compiler/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,17 @@ void Switches::PrintHelp(std::ostream& stream) {
}
stream << "}" << std::endl;
stream << "--sl=<sl_output_file>" << std::endl;
stream << "--spirv=<spirv_output_file>" << std::endl;
stream << "--spirv=<spirv_output_file> (ignored for --shader-bundle)"
<< std::endl;
stream << "[optional] --source-language=glsl|hlsl (default: glsl)"
<< std::endl;
stream << "[optional] --entry-point=<entry_point_name> (default: main; "
"ignored for glsl)"
<< std::endl;
stream << "[optional] --iplr (causes --sl file to be emitted in iplr format)"
<< std::endl;
stream << "[optional] --iplr-bundle=<bundle_spec> (causes --sl file to be "
"emitted in the iplr bundle format)"
stream << "[optional] --shader-bundle=<bundle_spec> (causes --sl file to be "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts about reading the json spec in from a file instead of the flag itself being json-valued? I wonder which will be easier for tools that wrap/call impellerc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hunch is that storing it in a file before invoking is more of a pain given the ephemeral nature. Although it would be slightly more convenient for the many of manual invocations I've been doing over the past couple of days. : )

"emitted in Flutter GPU's shader bundle format)"
<< std::endl;
stream << "[optional] --reflection-json=<reflection_json_file>" << std::endl;
stream << "[optional] --reflection-header=<reflection_header_file>"
Expand Down Expand Up @@ -122,7 +123,8 @@ Switches::Switches(const fml::CommandLine& command_line)
input_type(SourceTypeFromCommandLine(command_line)),
sl_file_name(command_line.GetOptionValueWithDefault("sl", "")),
iplr(command_line.HasOption("iplr")),
iplr_bundle(command_line.GetOptionValueWithDefault("iplr-bundle", "")),
shader_bundle(
command_line.GetOptionValueWithDefault("shader-bundle", "")),
spirv_file_name(command_line.GetOptionValueWithDefault("spirv", "")),
reflection_json_name(
command_line.GetOptionValueWithDefault("reflection-json", "")),
Expand Down Expand Up @@ -193,6 +195,12 @@ Switches::Switches(const fml::CommandLine& command_line)
}

bool Switches::AreValid(std::ostream& explain) const {
// When producing a shader bundle, all flags related to single shader inputs
// and outputs such as `--input` and `--spirv-file-name` are ignored. Instead,
// input files are read from the shader bundle spec and a single flatbuffer
// containing all compiled shaders and reflection state is output to `--sl`.
const bool shader_bundle_mode = !shader_bundle.empty();

bool valid = true;
if (target_platform == TargetPlatform::kUnknown) {
explain << "The target platform (only one) was not specified." << std::endl;
Expand All @@ -211,7 +219,7 @@ bool Switches::AreValid(std::ostream& explain) const {
valid = false;
}

if (source_file_name.empty()) {
if (source_file_name.empty() && !shader_bundle_mode) {
explain << "Input file name was empty." << std::endl;
valid = false;
}
Expand All @@ -221,15 +229,15 @@ bool Switches::AreValid(std::ostream& explain) const {
valid = false;
}

if (spirv_file_name.empty()) {
if (spirv_file_name.empty() && !shader_bundle_mode) {
explain << "Spirv file name was empty." << std::endl;
valid = false;
}

if (iplr && !iplr_bundle.empty()) {
explain
<< "--iplr and --iplr-bundle flag cannot be specified at the same time"
<< std::endl;
if (iplr && shader_bundle_mode) {
explain << "--iplr and --shader-bundle flag cannot be specified at the "
"same time"
<< std::endl;
valid = false;
}

Expand Down
5 changes: 4 additions & 1 deletion impeller/compiler/switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@ struct Switches {
std::vector<IncludeDir> include_directories = {};
std::string source_file_name = "";
SourceType input_type = SourceType::kUnknown;
/// The raw shader file output by the compiler. For --iplr and
/// --shader-bundle modes, this is used as the filename for the output
/// flatbuffer output.
std::string sl_file_name = "";
bool iplr = false;
std::string iplr_bundle = "";
std::string shader_bundle = "";
std::string spirv_file_name = "";
std::string reflection_json_name = "";
std::string reflection_header_name = "";
Expand Down
8 changes: 0 additions & 8 deletions impeller/compiler/types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,6 @@ SourceType SourceTypeFromString(std::string name) {
return SourceType::kFragmentShader;
}

if (name == "tessellationcontrol") {
return SourceType::kTessellationControlShader;
}

if (name == "tessellationevaluation") {
return SourceType::kTessellationEvaluationShader;
}

if (name == "compute") {
return SourceType::kComputeShader;
}
Expand Down
4 changes: 2 additions & 2 deletions impeller/fixtures/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ impellerc("flutter_gpu_shaders") {
"flutter_gpu_texture.frag",
"flutter_gpu_texture.vert",
]
sl_file_extension = "shaderbundle"
shader_target_flag = "--runtime-stage-metal"
iplr_bundle = "{\"UnlitFragment\": {\"type\": \"fragment\", \"file\": \"../../flutter/impeller/fixtures/flutter_gpu_unlit.frag\"}, \"UnlitVertex\": {\"type\": \"vertex\", \"file\": \"../../flutter/impeller/fixtures/flutter_gpu_unlit.vert\"}, \"TextureFragment\": {\"type\": \"fragment\", \"file\": \"../../flutter/impeller/fixtures/flutter_gpu_texture.frag\"}, \"TextureVertex\": {\"type\": \"vertex\", \"file\": \"../../flutter/impeller/fixtures/flutter_gpu_texture.vert\"}}"
shader_bundle = "{\"UnlitFragment\": {\"type\": \"fragment\", \"file\": \"../../flutter/impeller/fixtures/flutter_gpu_unlit.frag\"}, \"UnlitVertex\": {\"type\": \"vertex\", \"file\": \"../../flutter/impeller/fixtures/flutter_gpu_unlit.vert\"}, \"TextureFragment\": {\"type\": \"fragment\", \"file\": \"../../flutter/impeller/fixtures/flutter_gpu_texture.frag\"}, \"TextureVertex\": {\"type\": \"vertex\", \"file\": \"../../flutter/impeller/fixtures/flutter_gpu_texture.vert\"}}"
shader_bundle_output = "playground.shaderbundle"
}

test_fixtures("flutter_gpu_fixtures") {
Expand Down
139 changes: 103 additions & 36 deletions impeller/tools/impeller.gni
Original file line number Diff line number Diff line change
Expand Up @@ -242,18 +242,35 @@ template("embed_blob") {
# the impeller_use_prebuilt_impellerc argument. Forwards all variables to
# compiled_action_foreach or action_foreach as appropriate.
template("_impellerc") {
if (impeller_use_prebuilt_impellerc == "") {
compiled_action_foreach(target_name) {
forward_variables_from(invoker, "*")
tool = "//flutter/impeller/compiler:impellerc"
if (invoker.single_invocation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document the template parameter in the comment on the template.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if (impeller_use_prebuilt_impellerc == "") {
compiled_action(target_name) {
forward_variables_from(invoker, "*")
tool = "//flutter/impeller/compiler:impellerc"
}
} else {
action(target_name) {
forward_variables_from(invoker, "*", [ "args" ])
script = "//build/gn_run_binary.py"
impellerc_path =
rebase_path(impeller_use_prebuilt_impellerc, root_build_dir)
args = [ impellerc_path ] + invoker.args
}
}
} else {
action_foreach(target_name) {
forward_variables_from(invoker, "*", [ "args" ])
script = "//build/gn_run_binary.py"
impellerc_path =
rebase_path(impeller_use_prebuilt_impellerc, root_build_dir)
args = [ impellerc_path ] + invoker.args
if (impeller_use_prebuilt_impellerc == "") {
compiled_action_foreach(target_name) {
forward_variables_from(invoker, "*")
tool = "//flutter/impeller/compiler:impellerc"
}
} else {
action_foreach(target_name) {
forward_variables_from(invoker, "*", [ "args" ])
script = "//build/gn_run_binary.py"
impellerc_path =
rebase_path(impeller_use_prebuilt_impellerc, root_build_dir)
args = [ impellerc_path ] + invoker.args
}
}
}
}
Expand All @@ -262,8 +279,13 @@ template("impellerc") {
assert(defined(invoker.shaders), "Impeller shaders must be specified.")
assert(defined(invoker.shader_target_flag),
"The flag to impellerc for target selection must be specified.")
assert(defined(invoker.sl_file_extension),
assert(defined(invoker.sl_file_extension) || defined(invoker.shader_bundle),
"The extension of the SL file must be specified (metal, glsl, etc..).")
if (defined(invoker.shader_bundle)) {
assert(
defined(invoker.shader_bundle_output),
"When shader_bundle is specified, shader_output_bundle must also be specified.")
}

sksl = invoker.shader_target_flag == "--sksl"
iplr = false
Expand All @@ -279,17 +301,26 @@ template("impellerc") {
not_needed([
"iplr",
"sksl",
"shader_bundle",
"shader_bundle_output",
])

# Optional: invoker.iplr Causes --sl output to be in iplr format.
# Optional: invoker.iplr_bundle specifies a Flutter GPU shader bundle configuration.
# Optional: invoker.shader_bundle specifies a Flutter GPU shader bundle configuration.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments about the parameters are in a bit of weird spot. They should probably be moved up above line 278, and all the accepted parameters should be documented. Like if this template accepts single_invocation, then it should be documented in the docstring for this template.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alrighty, I moved these up to the top of the template and tried to document the missing ones accurately.

# Optional: invoker.shader_bundle_output specifies the output filename of the shader
# bundle. This is required if invoker.shader_bundle is supplied.
# Optional: invoker.defines specifies a list of valueless macro definitions.
# Optional: invoker.intermediates_subdir specifies the subdirectory in which
# to put intermediates.
# Optional: invoker.json Causes output format to be JSON instead of flatbuffer.

_impellerc(target_name) {
sources = invoker.shaders
shader_bundle = defined(invoker.shader_bundle)

# When single_invocation is true, impellerc will be invoked exactly once. When it's
# false, impellerc be invoked for each of the source file entries (invoker.shaders).
single_invocation = shader_bundle

if (defined(invoker.intermediates_subdir)) {
subdir = invoker.intermediates_subdir
generated_dir = "$target_gen_dir/$subdir"
Expand All @@ -299,22 +330,25 @@ template("impellerc") {

shader_target_flag = invoker.shader_target_flag

spirv_intermediate = "$generated_dir/{{source_file_part}}.spirv"
spirv_intermediate_path = rebase_path(spirv_intermediate, root_build_dir)

depfile_path = "$generated_dir/{{source_file_part}}.d"
depfile_intermediate_path = rebase_path(depfile_path, root_build_dir)
depfile = depfile_path

shader_lib_dir = rebase_path("//flutter/impeller/compiler/shader_lib")
args = [
"--input={{source}}",
"--include={{source_dir}}",
"--include=$shader_lib_dir",
"--depfile=$depfile_intermediate_path",
"$shader_target_flag",
]

# When we're in single invocation mode, we can't use source enumeration.
if (!single_invocation) {
args += [
"--input={{source}}",
"--include={{source_dir}}",
]
}

if (defined(invoker.gles_language_version)) {
gles_language_version = invoker.gles_language_version
args += [ "--gles-language-version=$gles_language_version" ]
Expand All @@ -338,27 +372,60 @@ template("impellerc") {
args += [ "--json" ]
}

if (sksl) {
sl_intermediate =
if (iplr) {
# When building in IPLR mode, the compiler may be executed twice
args += [ "--iplr" ]
}

# The `sl_output` is the raw shader file output by the compiler. For --iplr
# and --shader-bundle, this is used as the filename for the flatbuffer to
# output.

if (shader_bundle) {
# When a shader bundle is specified, don't bother supplying flags for
# the reflection state as these are ignored. In this mode, the compiler
# is invoked multiple times and the reflection state for each shader is
# written to the output flatbuffer.
sl_output = "$generated_dir/${invoker.shader_bundle_output}"
sl_output_path = rebase_path(sl_output, root_build_dir)

args += [
"--sl=$sl_output_path",
"--shader-bundle=${invoker.shader_bundle}",
]

outputs = [ sl_output ]
} else if (sksl) {
# When SkSL is selected as the `shader_target_flag`, 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
sl_output =
"$generated_dir/{{source_file_part}}.${invoker.sl_file_extension}"
sl_intermediate_path = rebase_path(sl_intermediate, root_build_dir)
sl_output_path = rebase_path(sl_output, root_build_dir)

spirv_intermediate = "$generated_dir/{{source_file_part}}.spirv"
spirv_intermediate_path = rebase_path(spirv_intermediate, root_build_dir)
args += [
"--sl=$sl_intermediate_path",
"--sl=$sl_output_path",
"--spirv=$spirv_intermediate_path",
]
if (iplr) {
args += [ "--iplr" ]
}

outputs = [ sl_intermediate ]
outputs = [ sl_output ]
} else {
sl_intermediate =
# 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)

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"

sl_intermediate_path = rebase_path(sl_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 =
Expand All @@ -367,21 +434,15 @@ template("impellerc") {
rebase_path(reflection_cc_intermediate, root_build_dir)

args += [
"--sl=$sl_intermediate_path",
"--sl=$sl_output_path",
"--spirv=$spirv_intermediate_path",
"--reflection-json=$reflection_json_path",
"--reflection-header=$reflection_header_path",
"--reflection-cc=$reflection_cc_path",
]
if (iplr) {
args += [ "--iplr" ]
}
if (defined(invoker.iplr_bundle)) {
args += [ "--iplr-bundle=${invoker.iplr_bundle}" ]
}

outputs = [
sl_intermediate,
sl_output,
reflection_header_intermediate,
reflection_cc_intermediate,
]
Expand All @@ -392,6 +453,12 @@ template("impellerc") {
args += [ "--define=$def" ]
}
}

if (single_invocation) {
inputs = invoker.shaders
} else {
sources = invoker.shaders
}
}
}

Expand Down