Compute Texture Demo is failing on Metal rendering backend

[starry-abyss]

Tested versions

4.4-beta1

System information

macOS 13.7.2, Apple M1 Pro 16 GB, Godot 4.4-beta1

Issue description

Compute Texture Demo is failing on Metal rendering backend, but works as usual on Vulkan

Steps to reproduce

1. Run Godot 4.4-beta1
2. Install the Compute Texture Demo from AssetLib
3. Edit it (press OK to convert the project from Godot 4.2 to Godot 4.4)
4. Run it or just look in the console in the Godot editor
5. When running no mouse interaction is happening
6. There is endless error spam:

  ERROR: Set: 1 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 2 Binding: 0 Type: Image Writable: Y Length: 1
  ERROR: Uniforms supplied for set (2):
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: are not the same format as required by the pipeline shader. Pipeline shader requires the following bindings:
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 1 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 2 Binding: 0 Type: Image Writable: Y Length: 1
  ERROR: Uniforms supplied for set (2):
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: are not the same format as required by the pipeline shader. Pipeline shader requires the following bindings:
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 1 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 2 Binding: 0 Type: Image Writable: Y Length: 1
  ERROR: Uniforms supplied for set (2):
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: are not the same format as required by the pipeline shader. Pipeline shader requires the following bindings:
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 1 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 2 Binding: 0 Type: Image Writable: Y Length: 1
  ERROR: Uniforms supplied for set (2):
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: are not the same format as required by the pipeline shader. Pipeline shader requires the following bindings:
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 1 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 2 Binding: 0 Type: Image Writable: Y Length: 1
  ERROR: Uniforms supplied for set (2):
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: are not the same format as required by the pipeline shader. Pipeline shader requires the following bindings:
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 1 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 2 Binding: 0 Type: Image Writable: Y Length: 1

7. Switch the renderer from Metal to Vulkan in Project Settings and restart Godot
8. All is working then, no error spam

Minimal reproduction project (MRP)

Compute Texture Demo from AssetLib

[huisedenanhai]

The issue still exists in 4.4 stable release.

[akien-mga]

CC @stuartcarnie

[stuartcarnie]

The issue is that the example is using some hacks, as these errors are coming from the RenderingDriver when it validates the uniform sets.

As in noted in the GDScript code, it creates 3 uniform set for the same slot 0:

        # Even though we're using 3 sets, they are identical, so we're kinda cheating.
        return rd.uniform_set_create([uniform], shader, 0)

and then rotates them here

        var next_set = texture_sets[with_next_texture]
        var current_set = texture_sets[(with_next_texture - 1) % 3]
        var previous_set = texture_sets[(with_next_texture - 2) % 3]

but binds them to slots 0, 1 and 2 here:

        rd.compute_list_bind_uniform_set(compute_list, current_set, 0)
        rd.compute_list_bind_uniform_set(compute_list, previous_set, 1)
        rd.compute_list_bind_uniform_set(compute_list, next_set, 2)

And as the RenderingDevice validation states (it rotates as the next, current and previous change):

  ERROR: Uniforms supplied for set (2):
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: are not the same format as required by the pipeline shader. Pipeline shader requires the following bindings:
  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 1 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 2 Binding: 0 Type: Image Writable: Y Length: 1

In this instance, uniform set 2 is incorrect, as it is Writable: N*, whereas Set:2 expects Writable: Y. And we can see in the .glsl source this is also true:

layout(r32f, set = 0, binding = 0) uniform restrict readonly image2D current_image;
layout(r32f, set = 1, binding = 0) uniform restrict readonly image2D previous_image;
layout(r32f, set = 2, binding = 0) uniform restrict writeonly image2D output_image;

Note that slot 0 and 1 are readonly as the log messages state:

  ERROR: Set: 0 Binding: 0 Type: Image Writable: N Length: 1
  ERROR: Set: 1 Binding: 0 Type: Image Writable: N Length: 1

and slot 2 is writeonly as stated:

  ERROR: Set: 2 Binding: 0 Type: Image Writable: Y Length: 1

When the Metal driver reflects the SPIR-V, it is being more strict apparently.

[stuartcarnie]

Turns out there is a bug in RenderingDeviceDriver::_reflect_spirv:

godot/servers/rendering/rendering_device_driver.cpp

Lines 163 to 167 in 1af1edf

	if (may_be_writable) {
	uniform.writable = !(binding.type_description->decoration_flags & SPV_REFLECT_DECORATION_NON_WRITABLE) && !(binding.block.decoration_flags & SPV_REFLECT_DECORATION_NON_WRITABLE);
	} else {
	uniform.writable = false;
	}

When reflecting an image type, the binding.block struct is invalid, as that only applies to struct types. Here is the dumped value from the debugger, abbreviated:

{
  spirv_id = 65
  name = 0x000000011c28fd80 "current_image"
  binding = 0
  input_attachment_index = 0
  set = 0
  descriptor_type = SPV_REFLECT_DESCRIPTOR_TYPE_STORAGE_IMAGE
  resource_type = SPV_REFLECT_RESOURCE_FLAG_UAV
  image = (dim = SpvDim2D, depth = 0, arrayed = 0, ms = 0, sampled = 2, image_format = SpvImageFormatR32f)
  block = {
    spirv_id = 0
    name = 0x0000000000000000
    offset = 0
    absolute_offset = 0
    size = 0
    padded_size = 0
    decoration_flags = 0
    flags = 0
    member_count = 0
    members = nullptr
    type_description = nullptr
    word_offset = (offset = 0)
  }
  count = 1
  accessed = 1
  uav_counter_id = 4294967295
  uav_counter_binding = nullptr
  byte_address_buffer_offset_count = 0
  byte_address_buffer_offsets = 0x0000000000000000
  type_description = 0x000000011c295470
  word_offset = (binding = 175, set = 171)
  decoration_flags = 128
  user_type = SPV_REFLECT_USER_TYPE_INVALID
}

Note that the spirv_id is 0, and the binding.block.decoration_flags is 0, which means this will evaluate to true, as the second part of the expression does not have the SPV_REFLECT_DECORATION_NON_WRITABLE bit set, as it is invalid and always 0:

uniform.writable = !(binding.type_description->decoration_flags & SPV_REFLECT_DECORATION_NON_WRITABLE) && !(binding.block.decoration_flags & SPV_REFLECT_DECORATION_NON_WRITABLE);

Metal has its own implementation to reflect the SPIR-V, which distinguishes the types:

godot/drivers/metal/rendering_device_driver_metal.mm

Lines 1787 to 1796 in d89cc01

	if (a_type.basetype == BT::Struct) {
	Bitset flags = compiler.get_buffer_block_flags(res.id);
	uniform.writable = !compiler.has_decoration(res.id, spv::DecorationNonWritable) && !flags.get(spv::DecorationNonWritable);
	} else if (a_type.basetype == BT::Image) {
	if (a_type.image.access == spv::AccessQualifierMax) {
	uniform.writable = !compiler.has_decoration(res.id, spv::DecorationNonWritable);
	} else {
	uniform.writable = a_type.image.access != spv::AccessQualifierReadOnly;
	}
	}

[stuartcarnie]

If I fix RenderingDeviceDriver::_reflect_spirv, then Vulkan produces the same error:

ERROR: Uniforms supplied for set (2):
Set: 0 Binding: 0 Type: Image Writable: N Length: 1
are not the same format as required by the pipeline shader. Pipeline shader requires the following bindings:
Set: 0 Binding: 0 Type: Image Writable: N Length: 1
Set: 1 Binding: 0 Type: Image Writable: N Length: 1
Set: 2 Binding: 0 Type: Image Writable: Y Length: 1

[huisedenanhai]

But why perform read/write validation for descriptor sets? As far as I know Vulkan does not care readwrite state when creating descriptor sets. Metal might need to specify readwrite state when calling useResoruces, but that seems to be tracked in MDCommandBuffer. There is no need to bake readwrite state in uniform set.

Surely I can modify the sample to create 3 sets for each image (9 sets in total) and make it run on metal without error. But that feels redundent and hints some potential performance issue.

[stuartcarnie]

The demo will be fixed by godotengine/godot-demo-projects#1174