[PATCH vkd3d 5/5] include: Document struct vkd3d_shader_interface_info.

[Henri Verbeet]

On Thu, 27 Aug 2020 at 07:46, Zebediah Figura <z.figura12 at gmail.com> wrote:
> +/** The type of a shader resource descriptor. */
>  enum vkd3d_shader_descriptor_type
>  {
> -    VKD3D_SHADER_DESCRIPTOR_TYPE_SRV     = 0x0, /* t#  */
> +    /** The descriptor is a shader resource view, bound to a t# register. */
> +    VKD3D_SHADER_DESCRIPTOR_TYPE_SRV     = 0x0,
> +    /** The descriptor is an unordered access view, bound to a u# register. */
>      VKD3D_SHADER_DESCRIPTOR_TYPE_UAV     = 0x1, /* u#  */
> +    /** The descriptor is a constant buffer view, bound to a cb# register. */
>      VKD3D_SHADER_DESCRIPTOR_TYPE_CBV     = 0x2, /* cb# */
> +    /** The descriptor is a sampler, bound to a s# register. */
>      VKD3D_SHADER_DESCRIPTOR_TYPE_SAMPLER = 0x3, /* s#  */
>
>      VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_DESCRIPTOR_TYPE),
>  };
>
I think I'd either leave the t#/u#/cb#/s# comments as they are, or
write something slightly more elaborate. They make sense in the
context of D3D assembly or byte code, but wouldn't necessarily when
compiling e.g. HLSL to GLSL.

> +/**
> + * A common structure describing the mapping a DXBC descriptor or descriptor
> + * array to a SPIR-V descriptor or descriptor array.
> + */
>  struct vkd3d_shader_descriptor_binding
>  {
> +    /** The set of the Vulkan descriptor. */
>      unsigned int set;
> +    /** The binding index, within the set, of the Vulkan descriptor. */
>      unsigned int binding;
> -    unsigned int count; /* This must be 1 in this version of vkd3d-shader. */
> +    /**
> +     * The size of this descriptor array. Descriptor arrays are not supported in
> +     * this version of vkd3d-shader, and therefore this value must be 1.
> +     */
> +    unsigned int count;
>  };
The vkd3d_shader_descriptor_binding structure doesn't describe the
mapping as such; it describes the bind point in the target
environment. It should also be noted that the target environment isn't
necessarily Vulkan; SPIR-V for example can be used with OpenGL as
well.

> +/**
> + * Describes the mapping of a single DXBC resource or resource array to SPIR-V.
> + * This structure is used in struct vkd3d_shader_interface_info.
> + */
>  struct vkd3d_shader_resource_binding
>  {
> +    /** The type of this descriptor. */
>      enum vkd3d_shader_descriptor_type type;
> +    /**
> +     * Register space of the DXBC descriptor. If the source format does not
> +     * support multiple register spaces, this parameter must be set to 0.
> +     */
>      unsigned int register_space;
> +    /** Register index of the DXBC descriptor. */
>      unsigned int register_index;
>      enum vkd3d_shader_visibility shader_visibility;
> -    unsigned int flags; /* vkd3d_shader_binding_flag */
> +    /** A combination of zero or more elements of vkd3d_shader_binding_flag. */
> +    unsigned int flags;
>
>      struct vkd3d_shader_descriptor_binding binding;
>  };
Somewhat similarly, I think it makes more sense to describe this in
terms of mapping between the shader and its target environment. I.e.,
the vkd3d_shader_interface_info structure performs a function similar
to the root signature in d3d12.

> +/**
> + * Describes the mapping of a DXBC resource-sampler pair to a SPIR-V combined
> + * sampler (i.e. sampled image). This structure is used in struct
> + * vkd3d_shader_interface_info.
> + */
>  struct vkd3d_shader_combined_resource_sampler
>  {
> +    /**
> +     * Register space of the DXBC resource. Ifthe source format does not
> +     * support multiple register spaces, this parameter must be set to 0.
> +     */
>      unsigned int resource_space;
"If the"

> +/**
> + * Describes the mapping of a single Direct3D push constant buffer to SPIR-V.
> + * This structure is used in struct vkd3d_shader_interface_info.
> + */
>  struct vkd3d_shader_push_constant_buffer
>  {
> +    /**
> +     * Register space of the Direct3D descriptor. If the source format does not
> +     * support multiple register spaces, this parameter must be set to 0.
> +     */
>      unsigned int register_space;
> +    /** Register index of the DXBC descriptor. */
>      unsigned int register_index;
>      enum vkd3d_shader_visibility shader_visibility;
>
> -    unsigned int offset; /* in bytes */
> -    unsigned int size;   /* in bytes */
> +    /** Offset, in bytes, in the SPIR-V push constant buffer. */
> +    unsigned int offset;
> +    /** Size, in bytes. */
> +    unsigned int size;
>  };
>
This maps a range of push constants to a constant buffer.

> +/**
> + * A chained structure describing the correspondence between source and target
> + * bindings of a compiled shader. This structure consists of several similar
> + * arrays of structures, each element of which describes a single mapping from
> + * a DXBC binding descriptor to a SPIR-V binding descriptor.
> + *
Like above, think of it as describing the interface between the shader
(either source or target) and the target environment. (In some ways
that's equivalent to describing a mapping between the source and
target shaders of course, but the specifics depend on the target
shader type.)

> + * This structure is optional. If omitted, vkd3d_shader_compile() will behave
> + * as if no descriptors had been passed.
> + *
Actually, it will use a default mapping; see the "done:" label in
vkd3d_dxbc_compiler_get_descriptor_binding(). It will also do that
when the binding count is 0, which we should probably fix.

> + * This structure extends vkd3d_shader_compile_info.
> + *
> + * This structure contains only input parameters.
> + */
>  struct vkd3d_shader_interface_info
>  {
> +    /** Must be set to VKD3D_SHADER_STRUCTURE_TYPE_INTERFACE_INFO. */
>      enum vkd3d_shader_structure_type type;
> +    /** Optional pointer to a structure containing further parameters. */
>      const void *next;
>
> +    /** An array of bindings for shader resource descriptors. */
>      const struct vkd3d_shader_resource_binding *bindings;
> +    /** Size, in elements, of \ref bindings. */
>      unsigned int binding_count;
>
> +    /** An array of bindings for push constant buffers. */
>      const struct vkd3d_shader_push_constant_buffer *push_constant_buffers;
> +    /** Size, in elements, of \ref push_constant_buffers. */
>      unsigned int push_constant_buffer_count;
>
> +    /** An array of bindings for combined samplers. */
>      const struct vkd3d_shader_combined_resource_sampler *combined_samplers;
> +    /** Size, in elements, of \ref combined_samplers. */
>      unsigned int combined_sampler_count;
>
> +    /** An array of bindings for UAV counters. */
>      const struct vkd3d_shader_uav_counter_binding *uav_counters;
> +    /** Size, in elements, of \ref uav_counters. */
>      unsigned int uav_counter_count;
>  };
"pointer to an array"