Skip to content

Commit

Permalink
Merge pull request godotengine#72174 from bitsawer/shader_include_fixes
Browse files Browse the repository at this point in the history
Close undefined
Fix undefined
  • Loading branch information
Chaosus authored Jan 28, 2023
2 parents a43db5a + 0acacce commit 2afa175
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 4 deletions.
5 changes: 3 additions & 2 deletions editor/plugins/text_shader_editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,11 +383,12 @@ void ShaderTextEditor::_code_complete_script(const String &p_code, List<ScriptLa
List<ScriptLanguage::CodeCompletionOption> pp_defines;
ShaderPreprocessor preprocessor;
String code;
complete_from_path = (shader.is_valid() ? shader->get_path() : shader_inc->get_path()).get_base_dir();
String resource_path = (shader.is_valid() ? shader->get_path() : shader_inc->get_path());
complete_from_path = resource_path.get_base_dir();
if (!complete_from_path.ends_with("/")) {
complete_from_path += "/";
}
preprocessor.preprocess(p_code, "", code, nullptr, nullptr, nullptr, nullptr, &pp_options, &pp_defines, _complete_include_paths);
preprocessor.preprocess(p_code, resource_path, code, nullptr, nullptr, nullptr, nullptr, &pp_options, &pp_defines, _complete_include_paths);
complete_from_path = String();
if (pp_options.size()) {
for (const ScriptLanguage::CodeCompletionOption &E : pp_options) {
Expand Down
13 changes: 12 additions & 1 deletion scene/resources/shader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ void Shader::set_path(const String &p_path, bool p_take_over) {
RS::get_singleton()->shader_set_path_hint(shader, p_path);
}

void Shader::set_include_path(const String &p_path) {
// Used only if the shader does not have a resource path set,
// for example during loading stage or when created by code.
include_path = p_path;
}

void Shader::set_code(const String &p_code) {
for (Ref<ShaderInclude> E : include_dependencies) {
E->disconnect(SNAME("changed"), callable_mp(this, &Shader::_dependency_changed));
Expand All @@ -80,11 +86,15 @@ void Shader::set_code(const String &p_code) {
HashSet<Ref<ShaderInclude>> new_include_dependencies;

{
String path = get_path();
if (path.is_empty()) {
path = include_path;
}
// Preprocessor must run here and not in the server because:
// 1) Need to keep track of include dependencies at resource level
// 2) Server does not do interaction with Resource filetypes, this is a scene level feature.
ShaderPreprocessor preprocessor;
preprocessor.preprocess(p_code, "", pp_code, nullptr, nullptr, nullptr, &new_include_dependencies);
preprocessor.preprocess(p_code, path, pp_code, nullptr, nullptr, nullptr, &new_include_dependencies);
}

// This ensures previous include resources are not freed and then re-loaded during parse (which would make compiling slower)
Expand Down Expand Up @@ -231,6 +241,7 @@ Ref<Resource> ResourceFormatLoaderShader::load(const String &p_path, const Strin
String str;
str.parse_utf8((const char *)buffer.ptr(), buffer.size());

shader->set_include_path(p_path);
shader->set_code(str);

if (r_error) {
Expand Down
2 changes: 2 additions & 0 deletions scene/resources/shader.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class Shader : public Resource {
Mode mode = MODE_SPATIAL;
HashSet<Ref<ShaderInclude>> include_dependencies;
String code;
String include_path;

HashMap<StringName, HashMap<int, Ref<Texture2D>>> default_textures;

Expand All @@ -72,6 +73,7 @@ class Shader : public Resource {
virtual Mode get_mode() const;

virtual void set_path(const String &p_path, bool p_take_over = false) override;
void set_include_path(const String &p_path);

void set_code(const String &p_code);
String get_code() const;
Expand Down
12 changes: 11 additions & 1 deletion scene/resources/shader_include.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,14 @@ void ShaderInclude::set_code(const String &p_code) {
}

{
String path = get_path();
if (path.is_empty()) {
path = include_path;
}

String pp_code;
ShaderPreprocessor preprocessor;
preprocessor.preprocess(p_code, "", pp_code, nullptr, nullptr, nullptr, &new_dependencies);
preprocessor.preprocess(p_code, path, pp_code, nullptr, nullptr, nullptr, &new_dependencies);
}

// This ensures previous include resources are not freed and then re-loaded during parse (which would make compiling slower)
Expand All @@ -64,6 +69,10 @@ String ShaderInclude::get_code() const {
return code;
}

void ShaderInclude::set_include_path(const String &p_path) {
include_path = p_path;
}

void ShaderInclude::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_code", "code"), &ShaderInclude::set_code);
ClassDB::bind_method(D_METHOD("get_code"), &ShaderInclude::get_code);
Expand All @@ -86,6 +95,7 @@ Ref<Resource> ResourceFormatLoaderShaderInclude::load(const String &p_path, cons
String str;
str.parse_utf8((const char *)buffer.ptr(), buffer.size());

shader_inc->set_include_path(p_path);
shader_inc->set_code(str);

if (r_error) {
Expand Down
3 changes: 3 additions & 0 deletions scene/resources/shader_include.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class ShaderInclude : public Resource {

private:
String code;
String include_path;
HashSet<Ref<ShaderInclude>> dependencies;
void _dependency_changed();

Expand All @@ -51,6 +52,8 @@ class ShaderInclude : public Resource {
public:
void set_code(const String &p_text);
String get_code() const;

void set_include_path(const String &p_path);
};

class ResourceFormatLoaderShaderInclude : public ResourceFormatLoader {
Expand Down
5 changes: 5 additions & 0 deletions servers/rendering/shader_preprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,11 @@ void ShaderPreprocessor::process_include(Tokenizer *p_tokenizer) {
return;
}

path = path.simplify_path();
if (path.is_relative_path()) {
path = state->current_filename.get_base_dir().path_join(path);
}

Ref<Resource> res = ResourceLoader::load(path);
if (res.is_null()) {
set_error(RTR("Shader include load failed. Does the shader include exist? Is there a cyclic dependency?"), line);
Expand Down

0 comments on commit 2afa175

Please sign in to comment.