godotengine / godot

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

RenderingServer.global_shader_parameter_add doesn't seem to add a parameter #77988

Open adamscoble opened 1 year ago

adamscoble commented 1 year ago

Godot version

4.1.dev4.official

System information

Godot v4.1.dev4 - Windows 10

Issue description

Using RenderingServer.global_shader_parameter_add doesn't appear to add an entry in Project Settings -> Shader Globals, and the shader editor doesn't pick up on its existence.

Steps to reproduce

Here is an example script to place on a SubViewport. Add it to a scene and note the global is not added.

@tool
extends SubViewport

func _ready() -> void:
    await RenderingServer.frame_post_draw
    RenderingServer.global_shader_parameter_add(HEIGHT_MAP_GLOBAL, RenderingServer.GLOBAL_VAR_TYPE_SAMPLER2D, get_texture())

Minimal reproduction project

See above

adamscoble commented 1 year ago

In case it's relevant, I just tried to add it view the editor and it told me it already exists, then the editor crashed.

It also appears that by adding it manually it was added to project.godot, but still doesn't show up in the editor window, even after a restart. And any shader referencing the global says it doesn't exist.

AThousandShips commented 1 year ago

I don't believe this is supported, see here, the editor and the project are not the same, input is separate, and I think there's other cases where globals are not expected to match

Edit: it says that it's for runtime

You can also add or remove global uniforms at run-time

But would be good to add functionality to script side, unsure how to access it via ProjectSettings

Calinou commented 1 year ago

It should be possible for scripts to manipulate shader globals in a way that is visible in the Project Settings, so that editor plugins can create/modify/remove shader globals. This makes it more seamless for users to enable/disable well-designed plugins, so they can configure themselves automatically.

jsjtxietian commented 1 year ago

But would be good to add functionality to script side, unsure how to access it via ProjectSettings

What about add functions like add_shader_global to ProjectSettings? The setting itself can be acquired by get_setting but we still need to call global_shader_parameter_add, so add a new function somewhere is needed.

noidexe commented 10 months ago

In 4.2.1 I can confirm doing RenderingServer.global_shader_parameter_add() in a tool script will make the parameters show up in ProjectSettings.

However, they are not really there when the game runs.

I have found a workaround. Instead of using RenderingServer.global_shader_parameter_add() use this:

func _add_shader_global(param_name: StringName, type: RenderingServer.GlobalShaderParameterType, default_value, overwrite := false) -> void:
    const prefix = "shader_globals/"

    # from https://github.com/godotengine/godot/blob/17e7f85c06366b427e5068c5b3e2940e27ff5f1d/servers/rendering/renderer_rd/storage_rd/material_storage.cpp#L1614
    const enum_to_string = ["bool","bvec2","bvec3","bvec4","int","ivec2","ivec3","ivec4","rect2i",
                            "uint","uvec2","uvec3","uvec4","float","vec2","vec3","vec4","color",
                            "rect2","mat2","mat3","mat4","transform_2d","transform","sampler2D",
                            "sampler2DArray","sampler3D","samplerCube"]

    # only the path is serialized for those
    const resource_types = ["sampler2D","sampler2DArray","sampler3D","samplerCube"]

    var exists = ProjectSettings.has_setting(prefix + param_name)

    if not overwrite and exists:
        return

    var type_as_string = enum_to_string[type]
    var value = default_value

    if type_as_string in resource_types and default_value is Resource:
        value = (default_value as Resource).resource_path

    # global shader params are saved in project.godot as a dictionary with 
    # the following structure
    var dict = {
        "type" = type_as_string,
        "value" = value
    }

    # Save the param to disk
    ProjectSettings.set_setting(prefix + param_name, dict)
    ProjectSettings.save()

    # We still need to manually add them to the RenderingServer so they are available
    # in the editor
    if exists:
        RenderingServer.global_shader_parameter_set(param_name, default_value)
    else:
        RenderingServer.global_shader_parameter_add(param_name, type, default_value)

You should call it as early as possible so the parameters are available in the editor to any open scenes with meshes that use them.

However we definitely need a built in way to do this

clayjohn commented 10 months ago

To add some context here, it seems the design of global shader parameters did not consider adding permanent shader parameters at run time (either through a tool script or running the game). The ProjectSettings manages its global shader paramaters essentially the same way that @noidexe has proposed in the post above.

I agree we need to have a better way to manage this as plugins are going to need to rely on global shader paramaters and it sucks if users have to register the global paramaters in the ProjectSettings themselves.

Unless someone else disagrees, I think we should implement @jsjtxietian's suggestion and add a helper function to ProjectSettings that allows you to register a global shader parameter.

noidexe commented 10 months ago

it sucks if users have to register the global paramaters in the ProjectSettings themselves.

Also if you download any assets that rely on global shader params you need to make sure you have all the params present in Project Settings before you open any of those assets in the editor. Otherwise shader compilation will fail. If shader compilation fails (local) shader parameters are no longer exposed to the material editor and as soon as the material gets saved they are gone from disk and memory.

image