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

Re-allows constants in global space to be initialized with function call #81619

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

Chaosus
Copy link
Member

@Chaosus Chaosus commented Sep 13, 2023

Allows using built-in functions in global constant initialization (was locked by mistake in #72494). The custom functions should be disallowed from using (lead to shader crash).

shader_type spatial;

const float s = 1.0f / sqrt(2.0f); // correct (error, previously)

void foo() {
return 0.0;
}

const float s2 = 1.0f / foo(); // error (otherwise lead to shader failure)

void fragment() {
}

Also, I've forbidden using custom function call in non-global constant declaration (its non-standard behavior and be restricted on shadertoy/shadered) eg:

void foo(){
return 1.0f;
}

void fragment(){
const float t = 1.0f / foo(); // error now
}

PS: I've added the separate hashmap for functions, so it is easy to check up, the actual change is at line 5211-5217 shader_language.cpp.

@Chaosus Chaosus requested a review from a team as a code owner September 13, 2023 16:17
@Chaosus Chaosus added this to the 4.2 milestone Sep 13, 2023
@Chaosus Chaosus force-pushed the fix_shader_const branch 3 times, most recently from 2a2f009 to e5ca8b3 Compare September 13, 2023 16:27
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.

Looks good to me

Let's not cherrypick as users might be relying on the previous erroneous behaviour (previously users could assign to a local constant using a function and that violates spec)

@akien-mga akien-mga merged commit 24c166d into godotengine:master Oct 3, 2023
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Chaosus Chaosus deleted the fix_shader_const branch October 3, 2023 15:48
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