Skip to content

Commit

Permalink
Merge pull request godotengine#95737 from Chaosus/shader_fix_varyings
Browse files Browse the repository at this point in the history 
Fix shader crash when using varyings with non-`flat` integer type
  • Loading branch information
@Repiteo
Repiteo committed Dec 11, 2024
2 parents 66dd289 + a7bb85d commit 3dacc5f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 43 deletions.
2 changes: 1 addition & 1 deletion servers/rendering/shader_compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ String ShaderCompiler::_dump_node_code(const SL::Node *p_node, int p_level, Gene
const StringName &varying_name = varying_names[k];
const SL::ShaderNode::Varying &varying = pnode->varyings[varying_name];

if (varying.stage == SL::ShaderNode::Varying::STAGE_FRAGMENT_TO_LIGHT || varying.stage == SL::ShaderNode::Varying::STAGE_FRAGMENT) {
if (varying.stage == SL::ShaderNode::Varying::STAGE_FRAGMENT) {
var_frag_to_light.push_back(Pair<StringName, SL::ShaderNode::Varying>(varying_name, varying));
fragment_varyings.insert(varying_name);
continue;
Expand Down
56 changes: 18 additions & 38 deletions servers/rendering/shader_language.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5063,25 +5063,23 @@ bool ShaderLanguage::_validate_varying_assign(ShaderNode::Varying &p_varying, St
case ShaderNode::Varying::STAGE_UNKNOWN: // first assign
if (current_function == varying_function_names.vertex) {
if (p_varying.type < TYPE_INT) {
*r_message = vformat(RTR("Varying with '%s' data type may only be assigned in the 'fragment' function."), get_datatype_name(p_varying.type));
*r_message = vformat(RTR("Varying with '%s' data type may only be assigned in the '%s' function."), get_datatype_name(p_varying.type), "fragment");
return false;
}
p_varying.stage = ShaderNode::Varying::STAGE_VERTEX;
} else if (current_function == varying_function_names.fragment) {
p_varying.stage = ShaderNode::Varying::STAGE_FRAGMENT;
}
break;
case ShaderNode::Varying::STAGE_VERTEX_TO_FRAGMENT_LIGHT:
case ShaderNode::Varying::STAGE_VERTEX:
if (current_function == varying_function_names.fragment) {
*r_message = RTR("Varyings which assigned in 'vertex' function may not be reassigned in 'fragment' or 'light'.");
*r_message = vformat(RTR("Varyings which assigned in '%s' function may not be reassigned in '%s' or '%s'."), "vertex", "fragment", "light");
return false;
}
break;
case ShaderNode::Varying::STAGE_FRAGMENT_TO_LIGHT:
case ShaderNode::Varying::STAGE_FRAGMENT:
if (current_function == varying_function_names.vertex) {
*r_message = RTR("Varyings which assigned in 'fragment' function may not be reassigned in 'vertex' or 'light'.");
*r_message = vformat(RTR("Varyings which assigned in '%s' function may not be reassigned in '%s' or '%s'."), "fragment", "vertex", "light");
return false;
}
break;
Expand Down Expand Up @@ -6039,15 +6037,11 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons
error = true;
}
break;
case ShaderNode::Varying::STAGE_VERTEX_TO_FRAGMENT_LIGHT:
[[fallthrough]];
case ShaderNode::Varying::STAGE_VERTEX:
if (is_out_arg && current_function != varying_function_names.vertex) { // inout/out
error = true;
}
break;
case ShaderNode::Varying::STAGE_FRAGMENT_TO_LIGHT:
[[fallthrough]];
case ShaderNode::Varying::STAGE_FRAGMENT:
if (!is_out_arg) {
if (current_function != varying_function_names.fragment && current_function != varying_function_names.light) {
Expand Down Expand Up @@ -6247,37 +6241,15 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons
return nullptr;
}
} else {
switch (var.stage) {
case ShaderNode::Varying::STAGE_UNKNOWN: {
if (var.type < TYPE_INT) {
if (current_function == varying_function_names.vertex) {
_set_error(vformat(RTR("Varying with '%s' data type may only be used in the 'fragment' function."), get_datatype_name(var.type)));
} else {
_set_error(vformat(RTR("Varying '%s' must be assigned in the 'fragment' function first."), identifier));
}
return nullptr;
}
} break;
case ShaderNode::Varying::STAGE_VERTEX:
if (current_function == varying_function_names.fragment || current_function == varying_function_names.light) {
var.stage = ShaderNode::Varying::STAGE_VERTEX_TO_FRAGMENT_LIGHT;
}
break;
case ShaderNode::Varying::STAGE_FRAGMENT:
if (current_function == varying_function_names.light) {
var.stage = ShaderNode::Varying::STAGE_FRAGMENT_TO_LIGHT;
}
break;
default:
break;
if (var.stage == ShaderNode::Varying::STAGE_UNKNOWN && var.type < TYPE_INT) {
if (current_function == varying_function_names.vertex) {
_set_error(vformat(RTR("Varying with '%s' data type may only be used in the '%s' function."), get_datatype_name(var.type), "fragment"));
} else {
_set_error(vformat(RTR("Varying '%s' must be assigned in the '%s' function first."), identifier, "fragment"));
}
return nullptr;
}
}

if ((var.stage != ShaderNode::Varying::STAGE_FRAGMENT && var.stage != ShaderNode::Varying::STAGE_FRAGMENT_TO_LIGHT) && var.type < TYPE_FLOAT && var.interpolation != INTERPOLATION_FLAT) {
_set_tkpos(var.tkpos);
_set_error(RTR("Varying with integer data type must be declared with `flat` interpolation qualifier."));
return nullptr;
}
}

if (ident_type == IDENTIFIER_FUNCTION) {
Expand Down Expand Up @@ -10698,6 +10670,14 @@ Error ShaderLanguage::_parse_shader(const HashMap<StringName, FunctionInfo> &p_f
tk = _get_token();
}

for (const KeyValue<StringName, ShaderNode::Varying> &kv : shader->varyings) {
if (kv.value.stage != ShaderNode::Varying::STAGE_FRAGMENT && (kv.value.type > TYPE_BVEC4 && kv.value.type < TYPE_FLOAT) && kv.value.interpolation != INTERPOLATION_FLAT) {
_set_tkpos(kv.value.tkpos);
_set_error(vformat(RTR("Varying with integer data type must be declared with `%s` interpolation qualifier."), "flat"));
return ERR_PARSE_ERROR;
}
}

#ifdef DEBUG_ENABLED
if (check_device_limit_warnings && uniform_buffer_exceeded_line != -1) {
_add_warning(ShaderWarning::DEVICE_LIMIT_EXCEEDED, uniform_buffer_exceeded_line, RTR("uniform buffer"), { uniform_buffer_size, max_uniform_buffer_size });
Expand Down
6 changes: 2 additions & 4 deletions servers/rendering/shader_language.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -620,10 +620,8 @@ class ShaderLanguage {
struct Varying {
enum Stage {
STAGE_UNKNOWN,
STAGE_VERTEX, // transition stage to STAGE_VERTEX_TO_FRAGMENT_LIGHT, emits warning if it's not used
STAGE_FRAGMENT, // transition stage to STAGE_FRAGMENT_TO_LIGHT, emits warning if it's not used
STAGE_VERTEX_TO_FRAGMENT_LIGHT,
STAGE_FRAGMENT_TO_LIGHT,
STAGE_VERTEX,
STAGE_FRAGMENT,
};

Stage stage = STAGE_UNKNOWN;
Expand Down

0 comments on commit 3dacc5f

Please sign in to comment.