Add support for struct uniforms in shaders

[Chaosus]

• Closes https://github.com/godotengine/godot-proposals/issues/3569

Notes:

• This version uses a Dictionary type for the uniform type. I did not find the other type which fits better for this. ~Still, the interface sucks and do not auto-reflect the changes made in a shader struct, but at least It's something (user need to manually adds members to it and change their types)~ -> (UPDATE: Dictionary in the inspector now reflects the changes from the shader).

• Structs needs to be declared before MaterialUniform in order to they can be used for uniforms, that's why I pushed them under #STRUCT statements inside each shader file.

• There are few limitations with this implementation, currently:

  • You cannot assign a default value, e.g:

  truct Foo {
  vec3 v;
  ;
  niform Foo foo = Foo(vec3(0, 1, 0));
  

  • instance/global scope initializers are not supported

• Arrays of common datatype in struct are supported (you can pass an array of conformal type):

• Compatibility renderer is supported

• set_shader_parameter via script is supported (You need to pass a dictionary, remember).

• UPDATE: You can declare an array of struct, e.g:

  truct Foo {
  vec4 v;
  ;
  niform Foo foos[2];
  

• UPDATE 2: You can use a struct inside a struct, e.g.:

  truct A {
  float t;
  ;
  truct B {
  A a;
  ;
  niform B b;
  

@QbieShay, @Calinou If you're interested to check and play with it, you're welcome.

[radiantgurl]

I've tested the uniform struct out, it's a wildly amazing idea, but it doesn't work. It seems to be either writing 2 floats before the actual vec4, or reading 2 floats after the vec4

shader_type spatial;
render_mode unshaded;

instance uniform vec3 color : source_color;

struct RayColorIdk {
	vec3 orig;
	vec3 dir;
	vec4 color;
};
uniform RayColorIdk ray;

void fragment() {
	ALBEDO = ray.color.rgb;
	//EMISSION = color;
}

This is the entirety of the code used in the example, hope this helps!

[radiantgurl]

Another issue spotted is me being able to delete fields or add fields to that dictionary. Other than that, amazing!

[Chaosus]

I've tested the uniform struct out, it's a wildly amazing idea, but it doesn't work.

Yeah, this is because that struct layout is not aligned to 16 bytes, if you add a member to it, it will work:

struct RayColorIdk {
	vec3 orig;
	float reserved;
	vec3 dir;
	float reserved2;
	vec4 color;
};

We should probably add reserved members internally.

Another issue spotted is me being able to delete fields or add fields to that dictionary.

Well, this is ok, at least for now, as noted in the topic description. If the member is absent, the default value (0) is passed to the compiler.

[Chaosus]

It seems to be either writing 2 floats before the actual vec4, or reading 2 floats after the vec4

@RadiantUwU I think I've fixed it!

[radiantgurl]

https://github.com/RadiantUwU/godot/commit/6d9cfab6ed55d6c1c073998b073d2f04a65e8438 This would fix the issue of the tests failing. (Line numbers are different for me since this is another godot fork)

[Chaosus]

Also fixed the dictionary in the inspector to reflect the changes from the shader.

[Mickeon]

How about the "Add key/value pair" button? Having this whole PR work is kind of hacky, admittedly, but there needs to be a way to hide that one.

[Chaosus]

How about the "Add key/value pair" button? Having this whole PR work is kind of hacky, admittedly, but there needs to be a way to hide that one.

Sorry, but such GUI modification is not a target for this PR. It can be done at late. The whole PR is indeed a hacky, otherwise it wouldn't work properly.

[Chaosus]

Looks fine for the most part, i'd recommend in a future pull request to implement a way for developers to have a way of setting default values.

It's a draft pull request that mean I will add new features and bug fixes until a certain point in time. Don't worry about default values, it's on the list to implement.

[Chaosus]

Added support for the structs inside structs (they will be parsed as mixed Array/Dictionary hierarchy):

[Chaosus]

I've hardy tried to make default arguments works, but currently they work very buggy, changed arguments from a shader does not handle by UI correctly, filling UBO with zero values etc. I've decided to turn off this feature, until all bugs fixed (probably in other PR, maybe created by someone else). As other features of this PR working correctly, I leave is at is (very tired to make this PR).

[eericjacobson]

this makes me happy. i have had enough with converting data to images to be passed to a shader uniform and needing to make sure everything is byte aligned correctly.

[eericjacobson]

I dont know if im doing the byte alignment wrong but it seems like uniforms that are defined after the structured uniform are always 0 for example

this is how i had the uniforms arranged before, and all the uniforms were 0 regardless of what value was fed

uniform Sphere Spheres[10];
uniform int NumSpheres;

uniform int NumRaysPerPixel;
uniform int MaxBounceCount;
uniform float DisplayAlbedo;

by changing it like this the values behaved normally again

uniform int NumRaysPerPixel;
uniform int MaxBounceCount;
uniform float DisplayAlbedo;

uniform int NumSpheres;
uniform Sphere Spheres[10];

these are the structs im using

struct RayTracingMaterial {
	vec4 color;
	vec4 emissionColor;
	float emissionStrength; };

struct Sphere {
	vec3 position;
	float radius;
	RayTracingMaterial material; };

i tried aligning the raytracingmaterial struct to 12 bytes which makes the sphere structure align to 16 bytes but that didnt work. and even if it was a byte alignment issue i would assume that some of the values would be detected and not be zero

[Chaosus]

@HeeboE Try again, I think I've fixed it.

[Chaosus]

The issue goes away if I add float three; at the end of the struct (float four; is not needed for 16-byte alignment for some reason). Stranglely enough, the issue does not reappear if I remove float three; until I reload the project entirely (or run it from the editor). Maybe there's an issue with how automatic padding is added?

Yeah, I know about these issues, I did not figure out how to fix it yet (I've tried hardly but did not success).

Is it planned to allow using hints such as : source_color for struct values?

Surely, this should be possible, IDK in this PR or not…

[Chaosus]

Not sure if I can finish this PR properly, I mark it as salvageable for now.

[Chaosus]

So as I understand the structure must have a layout of std140 standard (https://www.oreilly.com/library/view/opengl-programming-guide/9780132748445/app09lev1sec2.html) since it's presented in uniform block with it. Without follow the rules in that standard, the various bugs will happen and distract user from use this feature. I did not figure myself how to do it properly - if someone wants to take this - you are welcome to salvage this issue and make proper PR.