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

Allow declaring multiple members in one expression in shader structs #55903

Merged
merged 1 commit into from
Dec 15, 2021

Conversation

Chaosus
Copy link
Member

@Chaosus Chaosus commented Dec 13, 2021

Makes shader allowing such expressions:

struct Test {
	float a, b, c[2]; 
};

This has been fully supported by GLSL.

@Chaosus Chaosus requested a review from a team as a code owner December 13, 2021 14:47
@Chaosus Chaosus added this to the 4.0 milestone Dec 13, 2021
@akien-mga
Copy link
Member

Did you test this syntax too? Not sure if we want to support it but just to make sure it doesn't error/crash:

struct Test {
	float[2] a, b, c; 
};

And will it properly report a syntax error with:

struct Test {
	float[2] a, b, c[4]; 
};

@Chaosus
Copy link
Member Author

Chaosus commented Dec 13, 2021

@akien-mga Yeah, thanks for this notice - fixed it.
All following members would be arrays with the size defined in the first member.

@Chaosus Chaosus marked this pull request as draft December 13, 2021 15:23
@Chaosus Chaosus marked this pull request as ready for review December 13, 2021 15:34
@Chaosus
Copy link
Member Author

Chaosus commented Dec 13, 2021

That's, interesting (I didn't know about it before):

so float a[2], b, c; is different from float[2] a, b, c;
The second prevents the changing array size for the following members (and makes them arrays automatically) but the first allows it.

I should check the other places where it may happen and fix it if needed.

@Chaosus Chaosus force-pushed the shader_struct branch 2 times, most recently from 89979b0 to a6cc014 Compare December 13, 2021 16:04
@akien-mga akien-mga merged commit de8348a into godotengine:master Dec 15, 2021
@akien-mga
Copy link
Member

Thanks!

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.

2 participants