Browse Source

Fix shader crash when using varyings with non-`flat` integer type

Chaosus 1 year ago
parent
commit
a7bb85d2b7

+ 1 - 1
servers/rendering/shader_compiler.cpp

@@ -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 StringName &varying_name = varying_names[k];
 				const SL::ShaderNode::Varying &varying = pnode->varyings[varying_name];
 				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));
 					var_frag_to_light.push_back(Pair<StringName, SL::ShaderNode::Varying>(varying_name, varying));
 					fragment_varyings.insert(varying_name);
 					fragment_varyings.insert(varying_name);
 					continue;
 					continue;

+ 18 - 38
servers/rendering/shader_language.cpp

@@ -5062,7 +5062,7 @@ bool ShaderLanguage::_validate_varying_assign(ShaderNode::Varying &p_varying, St
 		case ShaderNode::Varying::STAGE_UNKNOWN: // first assign
 		case ShaderNode::Varying::STAGE_UNKNOWN: // first assign
 			if (current_function == varying_function_names.vertex) {
 			if (current_function == varying_function_names.vertex) {
 				if (p_varying.type < TYPE_INT) {
 				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;
 					return false;
 				}
 				}
 				p_varying.stage = ShaderNode::Varying::STAGE_VERTEX;
 				p_varying.stage = ShaderNode::Varying::STAGE_VERTEX;
@@ -5070,17 +5070,15 @@ bool ShaderLanguage::_validate_varying_assign(ShaderNode::Varying &p_varying, St
 				p_varying.stage = ShaderNode::Varying::STAGE_FRAGMENT;
 				p_varying.stage = ShaderNode::Varying::STAGE_FRAGMENT;
 			}
 			}
 			break;
 			break;
-		case ShaderNode::Varying::STAGE_VERTEX_TO_FRAGMENT_LIGHT:
 		case ShaderNode::Varying::STAGE_VERTEX:
 		case ShaderNode::Varying::STAGE_VERTEX:
 			if (current_function == varying_function_names.fragment) {
 			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;
 				return false;
 			}
 			}
 			break;
 			break;
-		case ShaderNode::Varying::STAGE_FRAGMENT_TO_LIGHT:
 		case ShaderNode::Varying::STAGE_FRAGMENT:
 		case ShaderNode::Varying::STAGE_FRAGMENT:
 			if (current_function == varying_function_names.vertex) {
 			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;
 				return false;
 			}
 			}
 			break;
 			break;
@@ -6038,15 +6036,11 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons
 														error = true;
 														error = true;
 													}
 													}
 													break;
 													break;
-												case ShaderNode::Varying::STAGE_VERTEX_TO_FRAGMENT_LIGHT:
-													[[fallthrough]];
 												case ShaderNode::Varying::STAGE_VERTEX:
 												case ShaderNode::Varying::STAGE_VERTEX:
 													if (is_out_arg && current_function != varying_function_names.vertex) { // inout/out
 													if (is_out_arg && current_function != varying_function_names.vertex) { // inout/out
 														error = true;
 														error = true;
 													}
 													}
 													break;
 													break;
-												case ShaderNode::Varying::STAGE_FRAGMENT_TO_LIGHT:
-													[[fallthrough]];
 												case ShaderNode::Varying::STAGE_FRAGMENT:
 												case ShaderNode::Varying::STAGE_FRAGMENT:
 													if (!is_out_arg) {
 													if (!is_out_arg) {
 														if (current_function != varying_function_names.fragment && current_function != varying_function_names.light) {
 														if (current_function != varying_function_names.fragment && current_function != varying_function_names.light) {
@@ -6246,37 +6240,15 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons
 								return nullptr;
 								return nullptr;
 							}
 							}
 						} else {
 						} 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) {
 					if (ident_type == IDENTIFIER_FUNCTION) {
@@ -10697,6 +10669,14 @@ Error ShaderLanguage::_parse_shader(const HashMap<StringName, FunctionInfo> &p_f
 		tk = _get_token();
 		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
 #ifdef DEBUG_ENABLED
 	if (check_device_limit_warnings && uniform_buffer_exceeded_line != -1) {
 	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 });
 		_add_warning(ShaderWarning::DEVICE_LIMIT_EXCEEDED, uniform_buffer_exceeded_line, RTR("uniform buffer"), { uniform_buffer_size, max_uniform_buffer_size });

+ 2 - 4
servers/rendering/shader_language.h

@@ -620,10 +620,8 @@ public:
 		struct Varying {
 		struct Varying {
 			enum Stage {
 			enum Stage {
 				STAGE_UNKNOWN,
 				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;
 			Stage stage = STAGE_UNKNOWN;