Skip to content

Commit

Permalink
spirv-hopper: Make more things const ref where possible
Browse files Browse the repository at this point in the history
  • Loading branch information
@spencer-lunarg
spencer-lunarg committed Jul 5, 2023
1 parent a5bbee8 commit bc9d55d
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 41 deletions.
60 changes: 30 additions & 30 deletions tests/spirv_hopper/pass_through_shaders.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ std::string Hopper::GetTypeDescription(SpvReflectTypeDescription& description, S
return type;
}

std::string Hopper::DefineCustomStruct(SpvReflectInterfaceVariable& variable) {
std::string Hopper::DefineCustomStruct(const SpvReflectInterfaceVariable& variable) {
SpvReflectTypeDescription& description = *variable.type_description;
std::string shader = "struct ";
shader += (description.type_name) ? description.type_name : "UNKNOWN_STRUCT_" + std::to_string(description.id);
Expand Down Expand Up @@ -133,7 +133,7 @@ std::string Hopper::DefineCustomStruct(SpvReflectInterfaceVariable& variable) {
void Hopper::BuildOrderedVariableMap(std::vector<SpvReflectInterfaceVariable*>& variables,
std::map<uint32_t, SpvReflectInterfaceVariable*>& variable_ordered_map) {
for (auto variable : variables) {
if (!IsBuiltinType(variable) && variable->type_description->op == SpvOp::SpvOpTypeStruct) {
if (!IsBuiltinType(*variable) && variable->type_description->op == SpvOp::SpvOpTypeStruct) {
variable_ordered_map.insert({variable->type_description->id, variable});
} else {
variable_ordered_map.insert({variable->spirv_id, variable});
Expand All @@ -147,37 +147,37 @@ bool Hopper::CreatePassThroughVertex() {

std::string shader = "#version 450\n";
for (auto entry : variable_ordered_map) {
SpvReflectInterfaceVariable* variable = entry.second;
const SpvReflectInterfaceVariable& variable = *entry.second;
if (IsBuiltinType(variable)) {
continue;
} else if ((shader_stage == VK_SHADER_STAGE_GEOMETRY_BIT && variable->format == SPV_REFLECT_FORMAT_UNDEFINED)) {
} else if ((shader_stage == VK_SHADER_STAGE_GEOMETRY_BIT && variable.format == SPV_REFLECT_FORMAT_UNDEFINED)) {
// TODO - Figure out why some Geometry shaders can these bogus variables
continue;
}

// Need to define struct to match
if (variable->type_description->op == SpvOp::SpvOpTypeStruct) {
shader += DefineCustomStruct(*variable);
if (variable.type_description->op == SpvOp::SpvOpTypeStruct) {
shader += DefineCustomStruct(variable);
}

// over 3 is invalid, means not set, zero is default implicit value
const uint32_t component = (variable->component > 3) ? 0 : variable->component;
const uint32_t component = (variable.component > 3) ? 0 : variable.component;

shader += "layout(location = ";
shader += std::to_string(variable->location);
shader += std::to_string(variable.location);
if (component > 0) {
shader += ", component = " + std::to_string(component);
}
shader += ") out ";
shader += GetTypeDescription(*variable->type_description, variable->format);
shader += GetTypeDescription(*variable.type_description, variable.format);
shader += " ";
// Names might not be valid GLSL names, so just give unique name
shader += "var_" + std::to_string(variable->location) + "_" + std::to_string(component);
shader += "var_" + std::to_string(variable.location) + "_" + std::to_string(component);

// Vertex output into Gemometry are not actually arrays
if (shader_stage != VK_SHADER_STAGE_GEOMETRY_BIT) {
for (uint32_t i = 0; i < variable->array.dims_count; i++) {
shader += "[" + std::to_string(variable->array.dims[i]) + "]";
for (uint32_t i = 0; i < variable.array.dims_count; i++) {
shader += "[" + std::to_string(variable.array.dims[i]) + "]";
}
}
shader += ";\n";
Expand All @@ -202,32 +202,32 @@ bool Hopper::CreatePassThroughTessellationEval() {
shader += "layout(triangles, equal_spacing, cw) in;\n";

for (auto entry : variable_ordered_map) {
SpvReflectInterfaceVariable* variable = entry.second;
const SpvReflectInterfaceVariable& variable = *entry.second;
if (IsBuiltinType(variable) == true) {
continue;
}

// Need to define struct to match
if (variable->type_description->op == SpvOp::SpvOpTypeStruct) {
shader += DefineCustomStruct(*variable);
if (variable.type_description->op == SpvOp::SpvOpTypeStruct) {
shader += DefineCustomStruct(variable);
}

// over 3 is invalid, means not set, zero is default implicit value
const uint32_t component = (variable->component > 3) ? 0 : variable->component;
const uint32_t component = (variable.component > 3) ? 0 : variable.component;

shader += "layout(location = ";
shader += std::to_string(variable->location);
shader += std::to_string(variable.location);
if (component > 0) {
shader += ", component = " + std::to_string(component);
}
shader += ") in ";
if (variable->type_description->type_name != nullptr) {
std::string patch = DefineCustomStruct(*variable);
if (variable.type_description->type_name != nullptr) {
std::string patch = DefineCustomStruct(variable);
shader.insert(patchIndex, patch);
}
shader += GetTypeDescription(*variable->type_description, variable->format);
shader += GetTypeDescription(*variable.type_description, variable.format);
shader += " ";
shader += "var_" + std::to_string(variable->location) + "_" + std::to_string(component);
shader += "var_" + std::to_string(variable.location) + "_" + std::to_string(component);
shader += "[]";
shader += ";\n";
}
Expand All @@ -244,32 +244,32 @@ bool Hopper::CreatePassThroughTessellationControl() {
shader += "layout(vertices = 3) out;\n";

for (auto entry : variable_ordered_map) {
SpvReflectInterfaceVariable* variable = entry.second;
const SpvReflectInterfaceVariable& variable = *entry.second;
if (IsBuiltinType(variable) == true) {
continue;
}

// Need to define struct to match
if (variable->type_description->op == SpvOp::SpvOpTypeStruct) {
shader += DefineCustomStruct(*variable);
if (variable.type_description->op == SpvOp::SpvOpTypeStruct) {
shader += DefineCustomStruct(variable);
}

// over 3 is invalid, means not set, zero is default implicit value
const uint32_t component = (variable->component > 3) ? 0 : variable->component;
const uint32_t component = (variable.component > 3) ? 0 : variable.component;

shader += "layout(location = ";
shader += std::to_string(variable->location);
shader += std::to_string(variable.location);
if (component > 0) {
shader += ", component = " + std::to_string(component);
}
shader += ") out ";
if (variable->type_description->type_name != nullptr) {
std::string patch = DefineCustomStruct(*variable);
if (variable.type_description->type_name != nullptr) {
std::string patch = DefineCustomStruct(variable);
shader.insert(patchIndex, patch);
}
shader += GetTypeDescription(*variable->type_description, variable->format);
shader += GetTypeDescription(*variable.type_description, variable.format);
shader += " ";
shader += "var_" + std::to_string(variable->location) + "_" + std::to_string(component);
shader += "var_" + std::to_string(variable.location) + "_" + std::to_string(component);
shader += "[]";
shader += ";\n";
}
Expand Down
16 changes: 8 additions & 8 deletions tests/spirv_hopper/spirv_hopper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ bool Hopper::CreatePipelineLayout() {

// Some Builtins like 'gl_in' of tessellation shaders are structs and so the gl_* identifiers are reserved. Can't assume all
// structs are Builtins.
bool Hopper::IsBuiltinType(SpvReflectInterfaceVariable* variable) {
return (variable->built_in >= 0 || (variable->name && std::string(variable->name).find("gl_") == 0));
bool Hopper::IsBuiltinType(const SpvReflectInterfaceVariable& variable) {
return (variable.built_in >= 0 || (variable.name && std::string(variable.name).find("gl_") == 0));
}

bool Hopper::CreateShaderStage(size_t code_size, const void* code, VkShaderStageFlagBits stage, const char* name) {
Expand All @@ -223,7 +223,7 @@ bool Hopper::CreateShaderStage(size_t code_size, const void* code, VkShaderStage
return true;
}

static uint32_t DescriptionLocationsConsumed(SpvReflectTypeDescription& description) {
static uint32_t DescriptionLocationsConsumed(const SpvReflectTypeDescription& description) {
const uint32_t scalar_bytes = description.traits.numeric.scalar.width / 8;
const uint32_t bytes_in_location = 16;
uint32_t vector_location_consumed = 1;
Expand All @@ -242,9 +242,9 @@ static uint32_t DescriptionLocationsConsumed(SpvReflectTypeDescription& descript
return vector_location_consumed * column_count;
}

bool Hopper::CreateVertexAttributeDescriptions(SpvReflectInterfaceVariable& variable) {
bool Hopper::CreateVertexAttributeDescriptions(const SpvReflectInterfaceVariable& variable) {
bool success = true;
SpvReflectTypeDescription& description = *variable.type_description;
const SpvReflectTypeDescription& description = *variable.type_description;
if (variable.member_count > 0) {
if (description.op == SpvOp::SpvOpTypeArray) {
// SPIRV-Reflect can't handle array-of-structs currently
Expand Down Expand Up @@ -333,7 +333,7 @@ bool Hopper::CreateGraphicsPipeline() {

if (shader_stage == VK_SHADER_STAGE_VERTEX_BIT) {
for (uint32_t i = 0; i < input_variables.size(); i++) {
SpvReflectInterfaceVariable* input_variable = input_variables[i];
const SpvReflectInterfaceVariable& input_variable = *input_variables[i];
// built in types (gl_VertexIndex, etc) are not part of the vertex input
// Any negative value means it is not part of the SpvBuiltIn
if (IsBuiltinType(input_variable) == true) {
Expand All @@ -344,9 +344,9 @@ bool Hopper::CreateGraphicsPipeline() {
// It is not valid to have Locations decorations on nested struct blocks.
// Since the value is "zero" for not being set, simpler to just know here where it is decorated.
// No decoration == "zero" implicitly (which is the default)
block_location = input_variable->location;
block_location = input_variable.location;

success = CreateVertexAttributeDescriptions(*input_variable);
success = CreateVertexAttributeDescriptions(input_variable);
if (!success) return false;
}

Expand Down
6 changes: 3 additions & 3 deletions tests/spirv_hopper/spirv_hopper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ class Hopper {
bool Reflect();
bool CreateShaderStage(size_t code_size, const void* code, VkShaderStageFlagBits stage, const char* name = "main");
bool CreatePipelineLayout();
bool CreateVertexAttributeDescriptions(SpvReflectInterfaceVariable& variable);
bool CreateVertexAttributeDescriptions(const SpvReflectInterfaceVariable& variable);
bool CreateGraphicsPipeline();
bool CreateGraphicsMeshPipeline();
bool CreateComputePipeline();

// For Pass Through shaders
bool IsBuiltinType(SpvReflectInterfaceVariable* variable);
bool IsBuiltinType(const SpvReflectInterfaceVariable& variable);
std::string GetTypeDescription(SpvReflectTypeDescription& description, SpvReflectFormat format);
std::string DefineCustomStruct(SpvReflectInterfaceVariable& variable);
std::string DefineCustomStruct(const SpvReflectInterfaceVariable& variable);
void BuildOrderedVariableMap(std::vector<SpvReflectInterfaceVariable*>& variables,
std::map<uint32_t, SpvReflectInterfaceVariable*>& variable_ordered_map);
bool BuildPassThroughShader(std::string& source, VkShaderStageFlagBits stage);
Expand Down

0 comments on commit bc9d55d

Please sign in to comment.