search

godotengine / godot

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

Setting shader parameter to const array causes error when material is freed #97343

Open Stovehead opened 1 week ago

Stovehead commented 1 week ago

Tested versions

System information

Godot v4.3.stable (77dcf97d8) - Windows 10.0.22631 - Vulkan (Mobile) - integrated Intel(R) Iris(R) Xe Graphics (Intel Corporation; 31.0.101.5186) - 13th Gen Intel(R) Core(TM) i7-1360P (16 Threads)

Issue description

If a shader parameter on a material is set to an array that is declared as const, the engine erroneously attempts to clear the array when the material is freed. This results in an error in the debugger tab, "clear: Array is in read-only state". In the MRP, I achieved this with the following code:

const TEST_CONST = []

func _ready() -> void:
    var new_node = Node2D.new()
    new_node.material = ShaderMaterial.new()
    new_node.material.set_shader_parameter("test", TEST_CONST)
    new_node.queue_free()

Steps to reproduce

Run the minimal reproduction project and check the debugger tab.

Minimal reproduction project (MRP)

clearbug.zip

Stovehead commented 6 days ago

The issue seems to come from this code in material_storage.cpp:

void MaterialStorage::material_free(RID p_rid) {
    Material *material = material_owner.get_or_null(p_rid);
    ERR_FAIL_NULL(material);

    // Need to clear texture arrays to prevent spin locking of their RID's.
    // This happens when the app is being closed.
    for (KeyValue<StringName, Variant> &E : material->params) {
        if (E.value.get_type() == Variant::ARRAY) {
            Array(E.value).clear();
        }
    }

    material_set_shader(p_rid, RID()); //clean up shader
    material->dependency.deleted_notify(p_rid);

    material_owner.free(p_rid);
}

Adding a check for the array being read-only seems to do the trick.

void MaterialStorage::material_free(RID p_rid) {
    Material *material = material_owner.get_or_null(p_rid);
    ERR_FAIL_NULL(material);

    // Need to clear texture arrays to prevent spin locking of their RID's.
    // This happens when the app is being closed.
    for (KeyValue<StringName, Variant> &E : material->params) {
        if (E.value.get_type() == Variant::ARRAY && !Array(E.value).is_read_only()) {
            Array(E.value).clear();
        }
    }

    material_set_shader(p_rid, RID()); //clean up shader
    material->dependency.deleted_notify(p_rid);

    material_owner.free(p_rid);
}