godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.1k stars 21.18k forks source link

Shader Uniforms Documentation comments do not work when used in gdshaderinc files #97844

Open gilbert72 opened 1 month ago

gilbert72 commented 1 month ago

Tested versions

v4.3.stable.official [77dcf97d8]

System information

Godot v4.3.stable - Windows 10.0.22631 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4050 Laptop GPU (NVIDIA; 31.0.15.5161) - 12th Gen Intel(R) Core(TM) i7-12650H (16 Threads)

Issue description

/**
* This is a documentation comment.
*/
uniform bool something = false;

If this piece of shader code lives inside a gdshader file, it works. Placed in a gdshaderinc file, it doesn't.

Steps to reproduce

/**
* This is a documentation comment.
*/
uniform bool something = false;

If this piece of shader code lives inside a gdshader file, it works. Placed in a gdshaderinc file, it doesn't.

Minimal reproduction project (MRP)

N/A

dustdfg commented 2 weeks ago

The roots of problem is that doc generation happens from a raw code by using regex. It can be solved by using preprocessed code but preprocessed code deletes all comments comments (including doc comments). So I have a very dirty solution that just works but decreases editor (only) performance by disabling comment stripping

diff --git a/scene/resources/shader.cpp b/scene/resources/shader.cpp
index d163a42fa9..e29edb1a75 100644
--- a/scene/resources/shader.cpp
+++ b/scene/resources/shader.cpp
@@ -55,7 +55,6 @@ void Shader::_check_shader_rid() const {
        MutexLock lock(shader_rid_mutex);
        if (shader_rid.is_null() && !preprocessed_code.is_empty()) {
                shader_rid = RenderingServer::get_singleton()->shader_create_from_code(preprocessed_code, get_path());
-               preprocessed_code = String();
        }
 }

@@ -128,7 +127,6 @@ void Shader::set_code(const String &p_code) {

        if (shader_rid.is_valid()) {
                RenderingServer::get_singleton()->shader_set_code(shader_rid, preprocessed_code);
-               preprocessed_code = String();
        }

        emit_changed();
@@ -181,7 +179,7 @@ void Shader::get_shader_uniform_list(List<PropertyInfo> *p_params, bool p_get_gr
                                prop_doc.name = "shader_parameter/" + pi.name;
 #ifdef MODULE_REGEX_ENABLED
                                const RegEx pattern("/\\*\\*\\s([^*]|[\\r\\n]|(\\*+([^*/]|[\\r\\n])))*\\*+/\\s*uniform\\s+\\w+\\s+" + pi.name + "(?=[\\s:;=])");
-                               Ref<RegExMatch> pattern_ref = pattern.search(code);
+                               Ref<RegExMatch> pattern_ref = pattern.search(preprocessed_code);
                                if (pattern_ref.is_valid()) {
                                        RegExMatch *match = pattern_ref.ptr();
                                        const RegEx pattern_tip("\\/\\*\\*([\\s\\S]*?)\\*/");
diff --git a/servers/rendering/shader_preprocessor.cpp b/servers/rendering/shader_preprocessor.cpp
index 0e41a178b5..b7309d5dc8 100644
--- a/servers/rendering/shader_preprocessor.cpp
+++ b/servers/rendering/shader_preprocessor.cpp
@@ -256,6 +256,7 @@ bool ShaderPreprocessor::CommentRemover::advance(char32_t p_what) {
 }

 String ShaderPreprocessor::CommentRemover::strip() {
+#ifndef TOOLS_ENABLED
        stripped.clear();
        index = 0;
        line = 0;
@@ -309,6 +310,9 @@ String ShaderPreprocessor::CommentRemover::strip() {
                }
        }
        return vector_to_string(stripped);
+#else // TOOLS_ENABLED
+       return code;
+#endif
 }

 ShaderPreprocessor::CommentRemover::CommentRemover(const String &p_code) {

I've tried a better solution but it turned to be less easy and much less readable...

Calinou commented 2 weeks ago

So I have a very dirty solution that just works but decreases editor (only) performance by disabling comment stripping

Is the performance impact noticeable? I'd think the preprocessor code doesn't spend much time parsing comments, so it probably wouldn't make a big difference anyway.

dustdfg commented 2 weeks ago

So I have a very dirty solution that just works but decreases editor (only) performance by disabling comment stripping

Is the performance impact noticeable? I'd think the preprocessor code doesn't spend much time parsing comments, so it probably wouldn't make a big difference anyway.

I almost sure it recompiles shader each time it is changed in the editor. I also saw how preprocessed code was deleted as like it was cache. Reasons? I don't know them but assume there were some reasons so I sure if I create PR it won't be merged