Merge pull request #90587 from clayjohn/reversez-shader-warn

Warn users when assigning `VERTEX` directly to `POSITION` due to compatibility breakage from Reverse Z changes

doc/classes/ProjectSettings.xml

@@ -637,6 +637,9 @@
 		<member name="debug/shader_language/warnings/formatting_error" type="bool" setter="" getter="" default="true">
 			When set to [code]true[/code], produces a warning upon encountering certain formatting errors. Currently this only checks for empty statements. More formatting errors may be added over time.
 		</member>
+		<member name="debug/shader_language/warnings/magic_position_write" type="bool" setter="" getter="" default="true">
+			When set to [code]true[/code], produces a warning when the shader contains [code]POSITION = vec4(vertex,[/code] as this was very common code written in Godot 4.2 and earlier that was paired with a QuadMesh to produce a full screen post processes pass. With the switch to reversed z in 4.3, this trick no longer works, as it implicitly relied on the [code]VERTEX.z[/code] being 0.
+		</member>
 		<member name="debug/shader_language/warnings/treat_warnings_as_errors" type="bool" setter="" getter="" default="false">
 			When set to [code]true[/code], warnings are treated as errors.
 		</member>

servers/rendering/shader_language.cpp

@@ -5029,6 +5029,10 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons
 	Vector<Expression> expression;
 
 	//Vector<TokenType> operators;
+#ifdef DEBUG_ENABLED
+	bool check_position_write = check_warnings && HAS_WARNING(ShaderWarning::MAGIC_POSITION_WRITE_FLAG);
+	check_position_write = check_position_write && String(shader_type_identifier) == "spatial" && current_function == "vertex";
+#endif
 
 	while (true) {
 		Node *expr = nullptr;
@@ -5589,6 +5593,24 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons
 						_set_error(vformat(RTR("Can't use function as identifier: '%s'."), String(identifier)));
 						return nullptr;
 					}
+#ifdef DEBUG_ENABLED
+					if (check_position_write && ident_type == IDENTIFIER_BUILTIN_VAR) {
+						if (String(identifier) == "POSITION") {
+							// Check if the user wrote "POSITION = vec4(VERTEX," and warn if they did.
+							TkPos prev_pos = _get_tkpos();
+							if (_get_token().type == TK_OP_ASSIGN &&
+									_get_token().type == TK_TYPE_VEC4 &&
+									_get_token().type == TK_PARENTHESIS_OPEN &&
+									_get_token().text == "VERTEX" &&
+									_get_token().type == TK_COMMA) {
+								_add_line_warning(ShaderWarning::MAGIC_POSITION_WRITE);
+							}
+
+							// Reset the position so compiling can continue as normal.
+							_set_tkpos(prev_pos);
+						}
+					}
+#endif
 					if (is_const) {
 						last_type = IDENTIFIER_CONSTANT;
 					} else {

servers/rendering/shader_warnings.cpp

@@ -65,6 +65,8 @@ String ShaderWarning::get_message() const {
 			return subject;
 		case DEVICE_LIMIT_EXCEEDED:
 			return vformat(RTR("The total size of the %s for this shader on this device has been exceeded (%d/%d). The shader may not work correctly."), subject, (int)extra_args[0], (int)extra_args[1]);
+		case MAGIC_POSITION_WRITE:
+			return vformat(RTR("You are attempting to assign the VERTEX position in model space to the vertex POSITION in clip space. The definition of clip space changed in version 4.3, so if this code was written prior to 4.3, it will not continue to work. Consider specifying the clip space z-component directly i.e. use `vec4(VERTEX.xy, 1.0, 1.0)`."));
 		default:
 			break;
 	}
@@ -92,6 +94,7 @@ String ShaderWarning::get_name_from_code(Code p_code) {
 		"UNUSED_LOCAL_VARIABLE",
 		"FORMATTING_ERROR",
 		"DEVICE_LIMIT_EXCEEDED",
+		"MAGIC_POSITION_WRITE",
 	};
 
 	static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names.");
@@ -122,6 +125,7 @@ static void init_code_to_flags_map() {
 	code_to_flags_map->insert(ShaderWarning::UNUSED_LOCAL_VARIABLE, ShaderWarning::UNUSED_LOCAL_VARIABLE_FLAG);
 	code_to_flags_map->insert(ShaderWarning::FORMATTING_ERROR, ShaderWarning::FORMATTING_ERROR_FLAG);
 	code_to_flags_map->insert(ShaderWarning::DEVICE_LIMIT_EXCEEDED, ShaderWarning::DEVICE_LIMIT_EXCEEDED_FLAG);
+	code_to_flags_map->insert(ShaderWarning::MAGIC_POSITION_WRITE, ShaderWarning::MAGIC_POSITION_WRITE_FLAG);
 }
 
 ShaderWarning::CodeFlags ShaderWarning::get_flags_from_codemap(const HashMap<Code, bool> &p_map) {

servers/rendering/shader_warnings.h

@@ -51,6 +51,7 @@ public:
 		UNUSED_LOCAL_VARIABLE,
 		FORMATTING_ERROR,
 		DEVICE_LIMIT_EXCEEDED,
+		MAGIC_POSITION_WRITE,
 		WARNING_MAX,
 	};
 
@@ -65,6 +66,7 @@ public:
 		UNUSED_LOCAL_VARIABLE_FLAG = 64U,
 		FORMATTING_ERROR_FLAG = 128U,
 		DEVICE_LIMIT_EXCEEDED_FLAG = 256U,
+		MAGIC_POSITION_WRITE_FLAG = 512U,
 	};
 
 private: