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 passing Matrix and Vector4 to shaders #258

Closed
galmiza opened this issue Nov 27, 2019 · 16 comments
Closed

Allow passing Matrix and Vector4 to shaders #258

galmiza opened this issue Nov 27, 2019 · 16 comments
Labels
requires core feedback Feature needs feedback from core developers topic:rendering topic:shaders
Milestone

Comments

@galmiza
Copy link

galmiza commented Nov 27, 2019

Describe the project you are working on:

I am working on a 3d first person shooter for mobile.
I want to implement light space perspective shadow maps to optimise the use of the shadow maps.
(I had it working on my own rendering engine which I dropped to adopt Godot.)

Describe the problem or limitation you are having in your project:

I need to send a perspective matrix to a shader (mat4 shader type).
Godot doesn't support 4x4 matrices (just 4x3 matrices with class Transform).
Godot doesn't support Vector4 either (note that class Color has some range limitations)

I had to create my own simplified Matrix class (in GDScript) and pass it to the shader using 8 Vector2 uniforms that I merge in the shader code.

highp mat4 uLookAtView = mat4(
  vec4(uLookAtView_0a,uLookAtView_0b),
  vec4(uLookAtView_1a,uLookAtView_1b),
  vec4(uLookAtView_2a,uLookAtView_2b),
  vec4(uLookAtView_3a,uLookAtView_3b));

This also comes with performance impacts (and tons of side-coding).

Describe how this feature / enhancement will help you overcome this problem or limitation:

These features will simplify both GDScript codes and shader codes, and bring a performance boost.

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:

Describe implementation detail for your proposal (in code), if possible:

Godot should implement the classes Matrix and Vector4 in C++ to maximise performance.
It should also be possible to pass them as shader uniforms in one line of GDScript.

material.set_shader_param("uMyMatrix4x4",Matrix.IDENTITY)
material.set_shader_param("uMyVector4",Vector4.ONES)

If this enhancement will not be used often, can it be worked around with a few lines of script?:

I believe these features will be used mainly by game engineers who will eventually contribute to the improvement of the core Godot engine.
But more importantly the lack of these feature is almost a no go for me. So it may be a definite no go for others who would.

Is there a reason why this should be core and not an add-on in the asset library?:

Godot says it let users write their own shaders.
This is currently not true without these features. So yes, it should be implemented in the core besides Vector3 and Transform.
Actually this is more a "limitation removal request" than a "feature request".

@Calinou
Copy link
Member

Calinou commented Nov 27, 2019

Godot says it let users write their own shaders.
This is currently not true without these features.

It's still true even if it has some limitations 🙂

@galmiza
Copy link
Author

galmiza commented Nov 27, 2019

J'avoue.
But going around these limitations is a real pain in this case.
And it feels like these features are just missing.
The matching between GDScript types and shader basic is incomplete.

@clayjohn
Copy link
Member

Transforms are converted into matrices when passed as uniforms.

https://github.com/godotengine/godot/blob/a87e2f85ee838245f0d5e702561faa3502b14800/gles_builders.py#L257-L274

So there is no need for GDScript to implement a new Matrix class. That would be incredibly confusing for users.

Colors are treated as vec4s when passed to shaders. What limitations are you describing?

@galmiza
Copy link
Author

galmiza commented Nov 27, 2019

Yes, Transform are matrices (4x3 matrices). They represent a combination of rotations, translations and scaling. Perfect for moving objects around.
But they can't represent, for instance, perspective transformations.
It won't be confusing if documented properly. Transform is still the way to go to move objects around.

Yes, Color contain 4 components and can be passed as vec4.
But the component values are clipped to [0,1]. They are only suitable to pass colors.

@Calinou
Copy link
Member

Calinou commented Nov 27, 2019

But the component values are clipped to [0,1]. They are only suitable to pass colors.

The Color type definitely allows defining "overbright" colors whose components go above 1. Maybe there's a limitation when passing them to custom shaders? (Try print(Color(200, 300, 400, 500)) in GDScript.)

@galmiza
Copy link
Author

galmiza commented Nov 27, 2019

I actually tried to pass my Matrix as 4 Color but it failed while passing them as 8 Vector2 succeeded.
I ran your command. It indeed shows the values over 1. Also shows negative values. So basically Color can hold any float.
Maybe something is done while passing them to a shader?

@galmiza
Copy link
Author

galmiza commented Nov 28, 2019

I checked the source code of Godot. The Color is linearised as it is passed to the shader.
The flag p_linear_color is true for spatial shaders.

case ShaderLanguage::TYPE_VEC4: {

	GLfloat *gui = (GLfloat *)data;

	if (value.get_type() == Variant::COLOR) {
		Color v = value;

		if (p_linear_color) {
			v = v.to_linear();
		}

		gui[0] = v.r;
		gui[1] = v.g;
		gui[2] = v.b;
		gui[3] = v.a;
	}

So Color can't be used to reliably transfer 4 floats.

@clayjohn
Copy link
Member

@galmiza could you send a link to where you got that code from?

@clayjohn
Copy link
Member

Thanks! Looks like using a rect2 or a plane is a decent workaround for now.

@galmiza
Copy link
Author

galmiza commented Nov 28, 2019

@clayjohn It does work with Plane, thanks!

So it is possible to recreate the mat4 using 4 Plane rather than 8 Vector2 ;)

highp mat4 uLookAtView = mat4(
  vec4(uLookAtView_0),
  vec4(uLookAtView_1),
  vec4(uLookAtView_2),
  vec4(uLookAtView_3));

@aaronfranke
Copy link
Member

aaronfranke commented Dec 3, 2019

See also: #629

It's certainly doable, it just needs enough use cases to be justified.

You may be able to shoehorn this data into a Color or Plane or Quat or Rect2, all of these types do have expectations of the range of values they hold, and that can cause issues. I'd recommend not using Color though.

@Malkverbena

This comment has been minimized.

@Calinou
Copy link
Member

Calinou commented Aug 2, 2020

@Malkverbena Please don't bump issues without contributing significant new information. Use the 👍 reaction button on the first post instead.

@fire
Copy link
Member

fire commented Jul 30, 2022

Resolved by godotengine/godot#63219. See the pr for the rationale and limitations.

Two versions of Vector4 (float and integer).
A Projection class, which is a 4x4 matrix specialized in projection types.

Not sure if this is passed to shaders yet.

@Calinou
Copy link
Member

Calinou commented Aug 23, 2022

This is now implemented in 4.0 according to @Chaosus, closing.

@Calinou Calinou closed this as completed Aug 23, 2022
@Calinou Calinou added this to the 4.0 milestone Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requires core feedback Feature needs feedback from core developers topic:rendering topic:shaders
Projects
Status: Implemented
Development

No branches or pull requests

7 participants