Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fog shader: Fix undeclared identifier global_variables #82877

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

tomissj2
Copy link
Contributor

@tomissj2 tomissj2 commented Oct 5, 2023

If user try to use a global shader variable in a fog type shader we are getting shader error. The reason of this there is a typo in the fog.cpp. In other well working shaders types like sky, the "action.global_buffer_array_variable" is "global_shader_uniforms.data". The investigation tracked is here:
https://discord.com/channels/212250894228652034/1158918161337434172

If user try to use a global shader variable in a fog type shader we are getting shader error. The reason of this there is a typo in the fog.cpp. I other well working shaders types like sky the "action.global_buffer_array_variable" is "global_shader_uniforms.data". 
The investigation tracked here:
https://discord.com/channels/212250894228652034/1158918161337434172
@tomissj2 tomissj2 requested a review from a team as a code owner October 5, 2023 20:23
@akien-mga akien-mga changed the title Fog Shader bugfix: 'global_variables' : undeclared identifier Fog shader: Fix undeclared identifier global_variables Oct 5, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct, this makes it consistent with other shaders and there are no other occurrences of this old name (missed in a previous rename):

$ rg "global_shader_uniforms.data"
servers/rendering/renderer_rd/environment/sky.cpp
821:            actions.global_buffer_array_variable = "global_shader_uniforms.data";

servers/rendering/renderer_rd/forward_mobile/scene_shader_forward_mobile.cpp
622:            actions.global_buffer_array_variable = "global_shader_uniforms.data";

servers/rendering/renderer_rd/storage_rd/particles_storage.cpp
120:            actions.global_buffer_array_variable = "global_shader_uniforms.data";

servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp
2495:           actions.global_buffer_array_variable = "global_shader_uniforms.data";

servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp
719:            actions.global_buffer_array_variable = "global_shader_uniforms.data";

$ rg "global_variables"
servers/rendering/renderer_rd/environment/fog.cpp
231:            actions.global_buffer_array_variable = "global_variables.data";

The same issue exists in 4.1 and 4.0.

@akien-mga akien-mga added bug topic:rendering cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Oct 5, 2023
@akien-mga akien-mga added this to the 4.2 milestone Oct 5, 2023
@akien-mga akien-mga merged commit d351d40 into godotengine:master Oct 5, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 24, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants