godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.43k stars 21.26k forks source link

Shader uniforms has null as default value #44454

Open KoBeWi opened 3 years ago

KoBeWi commented 3 years ago

Godot version:

3.2.4 beta4

Steps to reproduce:

  1. Create a 2D node with shader material
  2. Paste this code
    
    shader_type canvas_item;

uniform mat4 test;

3. Save the scene
4. Open the tscn in a text editor
5.

[sub_resource type="ShaderMaterial" id=2] shader = SubResource( 1 ) shader_param/test = null

Calinou commented 3 years ago

cc @Chaosus

Chaosus commented 3 years ago

It's a common bug across the engine - the variables set to null by default in some places (visual scripts for example). It seems not only for mat4 but for the vector and scalars uniforms too(though they are not been detected by the end-user), I may try to fix it, one of these days.

theraot commented 3 years ago

I can reproduce described behavior in master. However, it does not appear to affect the end user. Why is this a problem?

KoBeWi commented 2 years ago

Why is this a problem?

I originally discovered it when using get_shader_param() method and it broke my script. When you declare a uniform type, you expect it to be of that type. Especially when it shows correctly in the inspector.

EDIT: Another problem is that uniforms without default value provided will have no revert arrow and also if you provide a default value the uniform will still be null when created.

E.g. when you do uniform float test = 1.0;, the newly created uniform in the inspector will have value 0 and a revert arrow.

timothyparez commented 2 years ago

I can confirm this is still an issue in v3.4.3

Why is this a problem?

I am currently making a tool that exports ShaderMaterials and
their entire uniform configuration to another format.

var parameters = VisualServer.ShaderGetParamList(shader.GetRid());
foreach (Godot.Collections.Dictionary item in parameters)
{
    var parameterName = item["name"].ToString();
    var parameterValue = shaderMaterial.GetShaderParam(parameterName);  
    .....
    .....
}

This means that in the example above I sometimes get an empty string (if the user did not change any uniform value)
and other times I get the correct value in a string, for example "1" or "0" (if the user did change that uniform value)

JoanPotatoes2021 commented 2 years ago

Seems to be valid in v4.0.alpha14.official [106b68050] atm, but I didn't understood this issue so clearly before, the uniforms of a shader are not initialized until a value is given, if no default values were set previously.

This creates a problem when a user tries to check if the shader has the uniform before setting it up, which fails silently.

Testing with RenderingServer.shader_get_shader_uniform_list( shader_material.shader.get_rid() ) to see the uniform list displayed as expected, but when doing shader_material.shader.has_uniform( "a_uniform_X" ) returns false, unless if you set the uniform first, then it get's initialized and has_uniform() returns true.

has_uniform() should be able to return true even if no values were given for the uniform, unless this is a perfomance opmization in which then I can understand, so this is easy to workaround but hard to discover.

lostminds commented 1 year ago

Ran into this issue in 4.0b13 (as described here https://github.com/godotengine/godot/issues/71720), and specifically the issue that default values of shader uniforms aren't returned by get_shader_parameter.

Instead of trying to automatically initialize all shader uniforms as mentioned above, wouldn't an easier solution be to add a fallback in get_shader_parameter (and get_instance_shader_parameter) to check the shader for a default value for the uniform if there isn't a value set for it in the material (or geometry)?

Zylann commented 1 year ago

Still happening in 4.1.1. (initially found https://github.com/godotengine/godot/issues/71720 which was closed in favor of the current issue) It creates this oddity in my terrain plugin for values that havent been set by the user yet, which internally uses get/set_shader_parameter to expose inspector properties:

image

Therefore I use this workaround:

static func get_shader_material_parameter(material: ShaderMaterial, param_name: StringName):
    var v = material.get_shader_parameter(param_name)
    if v == null:
        var shader : Shader = material.shader
        if shader != null:
            v = RenderingServer.shader_get_parameter_default(shader.get_rid(), param_name)
    return v
F3der1co commented 4 months ago

I still encounter this in 4.3 beta 2. Couldn't the workaround Zylann suggests be done in the get_shader_parameter method?