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

Fix false positive errors in gdshaderinc files #89752

Conversation

ze2j
Copy link
Contributor

@ze2j ze2j commented Mar 21, 2024

Fixes #89479

These language features are wrongly detected as not supported when used in gdshaderinc files:

  • instance uniforms
  • hint_normal_roughness_texture
  • hint_depth_texture

These types of error should only occur when the shader type is not spatial and not a gdshaderinc file.

Here is the project I used to test this PR: test_project.zip
There is one scene per feature in their own folder + 1 canvas item shader for non regression tests.

As expected:

  • When a spatial shader use one of these language feature, no error is raised in the spatial shader nor the gdshaderinc file it includes.
  • An error is raised in the canvas item shader with or without including a gdshaderinc file.

These language features are wrongly detected as errors (not supported)
when used in gdshaderinc files:
- instance uniforms
- hint_normal_roughness_texture
- hint_depth_texture

This type of error should only occur when the shader type is not spatial
and not a gdshaderinc file.
@ze2j ze2j requested a review from a team as a code owner March 21, 2024 15:18
@AThousandShips AThousandShips added this to the 4.3 milestone Mar 21, 2024
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Maybe @bitsawer could take a look as well

@akien-mga akien-mga requested a review from bitsawer April 5, 2024 10:08
@akien-mga
Copy link
Member

Tested with the MRP, this seems to work well, including when using those gdshaderinc files in canvas_item shaders.

@akien-mga akien-mga merged commit 50b89e6 into godotengine:master Apr 10, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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.

Per-instance uniforms produce an error in the gdshaderinc file editor but works fine
4 participants