Strange Texture2DRD behavour when used in multimesh

[Zatarita]

Tested versions

• Tested in: 4.2.2.stable

System information

Windows 10 - Godot 4.2.2.stable - Vulkan Forward+

Issue description

I was following the tutorial found here: https://github.com/godotengine/godot-demo-projects/tree/master/compute/texture

And I augmented some of the behavior for my own purposes. I used this to create a function that precalculates hashes to be passed off to a shader. This functionality is used as an autoload script and it is called on the render thread. This appears to work fine (Although the demo project is broken in current builds, that's not entirely the purpose of the bug report)

I use this hash in a shell shader. I create a multimesh instance 3d and assign it the shader. I pass the height offset as custom data in the multimesh. None of this is actually related to the bug; however, for sake of being concise I included it.

It appears that in some cases (So far I have only identified when used in a multimeshinstance3d) the Texture2DRD is initialized with an RID that is invalid and causes an error to be thrown:

scene/resources/texture_rd.cpp:78 - Condition "!RD::get_singleton()->texture_is_valid(p_texture_rd_rid)" is true.

So I put on my detective hat and built a debug version of Godot from the source code (SCons still doesn't build debug_symbols yes by default btw) and tried to trace the issue. It appears on load sometimes when a Texture2DRD is created it is assigned an invalid RID causing the engine to throw this error. I made some changes in the code for the Texture2DRD to make the default RID for Texture2DRD 0, and then I added a check in the set RID function to see if it was null:

line 37: ClassDB::bind_method(D_METHOD("set_texture_rd_rid", "texture_rd_rid"), &Texture2DRD::set_texture_rd_rid, DEFVAL(RID::from_uint64(0)));

line 74: ERR_FAIL_NULL(RS::get_singleton());
line 75: if (p_texture_rd_rid == RID::from_uint64(0))
    return;

This "fixed" the error message that was being thrown in my code; however, I don't think this is a complete fix. I personally feel that the Texture2DRD should always be initialized with a null value, and being assigned a null value should not be invalid. I personally think it's more important to check the texture's validity when it is being used instead of when it is being set, as a null reference is a valid value for something that uses a RID reference.

Also, I discussed this in a comment here as well; however, I felt as this was a standalone issue from the issue being discussed in that thread it may require it's own bug report: https://github.com/godotengine/godot/issues/90174

Steps to reproduce

This is a part of my code stripped back all the way until the error is produced

@tool
extends MultiMeshInstance3D

var texture : Texture2DRD

# Called when the node enters the scene tree for the first time.
func _ready() -> void:
    self.multimesh = MultiMesh.new()
    self.multimesh.mesh = PlaneMesh.new()
    self.multimesh.transform_format = MultiMesh.TRANSFORM_3D
    self.multimesh.instance_count = 1
    self.material_override = ShaderMaterial.new()
    self.material_override.shader = load("res://test.gdshader")    # empty shader that has a sampler2d uniform "hash_texture"
    texture = Texture2DRD.new()
    texture.texture_rd_rid = Hash.get_hash(Vector2i(256, 256))
    self.material_override.set_shader_parameter("hash_texture", texture)

# Called every frame. 'delta' is the elapsed time since the previous frame.
func _process(delta: float) -> void:
    pass

This is the hash function used as an autoloaded script; however, I don't believe this exactly would be an issue. I feel any compute shader that returns an RID texture would result in this

@tool
extends Node

const shader_file := preload("res://Shaders/Global/Compute/src/Hash.glsl")
var compiled_shader : RDShaderSPIRV
var shader: RID
var pipeline: RID
var render_device: RenderingDevice
var texture_format: RDTextureFormat = RDTextureFormat.new()

# For seed generation
const SEED_LIMIT : int = 2147483647                          # should be plenty
var rng := RandomNumberGenerator.new()

# Keep track of the generated RIDs
var dispatched_hashes : Array[RID]

## Attempts to free the memory of a texture dispatched with get_hash_texture.
## Params:
##   texture_id: The RID for the has dispatched by get_hash_texture
## Return: Bool
##   True if texture_id existed
##   False if texture_id wasn't dispatched by get_hash_texture
func free_hash_texture(texture_id : RID) -> bool:
    for index in dispatched_hashes.size():
        if texture_id == dispatched_hashes[index]:
            render_device.free_rid(texture_id)
            dispatched_hashes.remove_at(index)
            return true

    return false

## After a new hash has been generated, use this to get the RID of the new hash
func get_last_id():
    if dispatched_hashes.size() > 0:
        return dispatched_hashes[dispatched_hashes.size() - 1]
    printerr("Requested a hash ID when one hasn't been generated. (Call create_new_texture)")
    return null

## Initializes the texture uniform and creates a uniform set from it
## Params:
##    Render Device:  Local render device used for the calculations
##    Output Texture: Pre-generated RID for the output texture uniform
## Returns: RID
##    Uniform set containing the output texture data
func _generate_texture_uniform(output_texture: RID) -> RID:
    var texture_uniform := RDUniform.new()
    texture_uniform.uniform_type = RenderingDevice.UNIFORM_TYPE_IMAGE
    texture_uniform.binding = 0
    texture_uniform.add_id(output_texture)
    return render_device.uniform_set_create([texture_uniform], shader, 0)

## If one isn't passed, Generates a seed to be passed to the hashing function.
## Then creates the uniform set from the seed to be passed to the compute shader
## Params:
##    Render Device: Local render device used for the calculations
##    [Static Seed]: If a static seed is passed it will be used instead of random seed
## Returns: RID
##    Uniform set containing the seed uniform set
func _generate_seed_uniform(static_seed: int = -1) -> RID:
    var seed_value := static_seed if static_seed >= 0 else rng.randi_range(0, SEED_LIMIT)
    var seed_bytes := PackedInt32Array([seed_value]).to_byte_array()
    var buffer := render_device.storage_buffer_create(seed_bytes.size(), seed_bytes)

    var seed_uniform := RDUniform.new()
    seed_uniform.uniform_type = RenderingDevice.UNIFORM_TYPE_STORAGE_BUFFER
    seed_uniform.binding = 1
    seed_uniform.add_id(buffer)
    return render_device.uniform_set_create([seed_uniform], shader, 1)

## Runs the compute shader
## Params: 
##    Uniforms: Array containing out texture set uniform and seed set uniform
##    Size:     Dimensions of the texture to be output
func _run_compute(uniforms: Array[RID], size: Vector2i) -> void:
    var compute_list := render_device.compute_list_begin()
    render_device.compute_list_bind_compute_pipeline(compute_list, pipeline)
    render_device.compute_list_bind_uniform_set(compute_list, uniforms[0], 0)
    render_device.compute_list_bind_uniform_set(compute_list, uniforms[1], 1)
    render_device.compute_list_dispatch(compute_list, size.x, size.y, 1)
    render_device.compute_list_end()

## Generate a hash texture using a compute shader and cache the resulting hash textures.
## Params:
##    Size: Dimensions of the texture as Vec2i(Width, Height)
##    StaticSeed: Use a specific seed if desired 
## Return: RID | null on failure
## NOTE: free memory when texture no longer needed using free_hash_texture
func create_new_texture(size : Vector2i, static_seed: int = -1):
    if pipeline == null:
        print_debug("Unable to create texture, Hash shader failed to compile")
        return null

    texture_format.set_width(size.x)
    texture_format.set_height(size.y)

    var output_texture      := render_device.texture_create(texture_format, RDTextureView.new(), [])
    var texture_uniform_set := _generate_texture_uniform(output_texture)
    var seed_uniform_set    := _generate_seed_uniform(static_seed)

    _run_compute([texture_uniform_set, seed_uniform_set], size)

    dispatched_hashes.append(output_texture)
    return output_texture

func get_hash(size : Vector2i, static_seed: int = -1) -> RID:
    RenderingServer.call_on_render_thread(create_new_texture.bind(size, static_seed))
    return get_last_id()

# Since there is some initializion done on ready, we may have to reload the
# editor after setting this all up to force the update.
func _ready() -> void:
    texture_format.set_format(RenderingDevice.DATA_FORMAT_R32_SFLOAT)
    texture_format.set_texture_type(RenderingDevice.TEXTURE_TYPE_2D)
    texture_format.set_depth(1)
    texture_format.set_array_layers(1)
    texture_format.set_mipmaps(1)
    texture_format.set_usage_bits(
            RenderingDevice.TEXTURE_USAGE_SAMPLING_BIT |
            RenderingDevice.TEXTURE_USAGE_STORAGE_BIT |
            RenderingDevice.TEXTURE_USAGE_CAN_COPY_TO_BIT | 
            RenderingDevice.TEXTURE_USAGE_CAN_COPY_FROM_BIT
    )

    compiled_shader = shader_file.get_spirv()
    if compiled_shader == null:
        printerr("Compute shader failed compilation")
        return
    render_device = RenderingServer.get_rendering_device()
    shader = render_device.shader_create_from_spirv(compiled_shader)
    pipeline = render_device.compute_pipeline_create(shader)

func free_hash_textures() -> void:
    for index in dispatched_hashes.size():
        render_device.free_rid(dispatched_hashes[index])
        dispatched_hashes.remove_at(index)

Hash Function - Compute Shader

#[compute]
#version 450

layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

layout(r32f, set = 0, binding = 0) uniform restrict writeonly image2D output_image;

layout(set = 1, binding = 1, std430) restrict buffer SeedBuffer {
    int seed;
} seed_buffer;

// Inigo Quilez
// https://www.shadertoy.com/view/4tXyWN
float hash( uvec2 x )
{
    uvec2 q = 1103515245U * ( (x>>1U) ^ (x.yx   ) );
    uint  n = 1103515245U * ( (q.x  ) ^ (q.y>>3U) );
    return float(n) * (1.0/float(0xffffffffU));
}

void main() {
    float hash2 = hash(uvec2(gl_GlobalInvocationID.xy) + seed_buffer.seed);
    vec4 result = vec4(hash2, 0.0, 0.0, 1.0);
    imageStore(output_image, ivec2(gl_GlobalInvocationID.xy), result);
}

NOTE the MRP isn't fully "optimized code" It doesn't free any of the textures or anything. I just kept removing parts from my finished code until I had the bare minimum that caused the bug. The hash RIDs that are dispatched might not be freed, but that's known

Once you have this all in place reload the project, sometimes it happens as well when you reload the scene. I'm not entirely sure why it doesn't happen EVERY time; however, sometimes it doesn't say the error. Not sure if maybe it has something to do with a shader cache or something. Sometimes I have to open the multimesh override material in the editor and preview the shader parameters before restarting the scene, but once it starts happening it keeps happening

Minimal reproduction project (MRP)

MRP.zip

[clayjohn]

The texture_rd_rid needs to be a valid RID. You appear to be generating the RID after the assignment to texture_rd_rid

Look at the process of your code:

1. Set the texture_rd_rid to the result of get_hash()

   texture.texture_rd_rid = Hash.get_hash(Vector2i(256, 256))

2. get_hash() queues up a function to create a texture on the rendering thread, then returns get_last_id()

func get_hash(size : Vector2i, static_seed: int = -1) -> RID:
    RenderingServer.call_on_render_thread(create_new_texture.bind(size, static_seed))
    return get_last_id()

3. The texture is not created, then get_last_id() returns either the latest texture or null

func get_last_id():
    if dispatched_hashes.size() > 0:
        return dispatched_hashes[dispatched_hashes.size() - 1]
    printerr("Requested a hash ID when one hasn't been generated. (Call create_new_texture)")
    return null

4. texture_rd_rid is either null or receives the RID of the last texture created last frame.
5. After the scripting frame is done, the rendering frame begins and the texture is created

I think its likely that the error is correct and is showing you that you have made an error by passing an erroneous value

[Zatarita]

The texture_rd_rid needs to be a valid RID. You appear to be generating the RID after the assignment to texture_rd_rid

Look at the process of your code:

1. Set the texture_rd_rid to the result of get_hash()

texture.texture_rd_rid = Hash.get_hash(Vector2i(256, 256))

2. get_hash() queues up a function to create a texture on the rendering thread, then returns get_last_id()

func get_hash(size : Vector2i, static_seed: int = -1) -> RID:

  RenderingServer.call_on_render_thread(create_new_texture.bind(size, static_seed))

  return get_last_id()

3. The texture is not created, then get_last_id() returns either the latest texture or null

func get_last_id():

  if dispatched_hashes.size() > 0:

      return dispatched_hashes[dispatched_hashes.size() - 1]

  printerr("Requested a hash ID when one hasn't been generated. (Call create_new_texture)")

  return null

4. texture_rd_rid is either null or receives the RID of the last texture created last frame.

5. After the scripting frame is done, the rendering frame begins and the texture is created

I think its likely that the error is correct and is showing you that you have made an error by passing an erroneous value

I understand what you're saying. This falls under a race condition since I'm rendering the hash on the render thread which I wasn't thinking about (and something I'll fix in my code thank you for pointing that out); however, the code doesn't print my printerr stating that I'm requesting a hash ID that hasn't been generated. That was one of the thoughts that maybe it was somehow returning a null value. That's why I put that debug error message in there.

Also, why would modifying the default value correct this error message if the issue was a race condition?

Also another thing is the error message gets printed to the screen before any of my gd script is even called. I actually had to use a vs code debug to catch the error macro