[PATCH vkd3d 3/5] include: Document struct vkd3d_shader_compile_info and members.

[Henri Verbeet]

On Thu, 27 Aug 2020 at 07:46, Zebediah Figura <z.figura12 at gmail.com> wrote:
> +/**
> + * Determines how buffer UAVs are stored when compiling DXBC to SPIR-V.

This is slightly awkward; the actual shader in
VKD3D_SHADER_SOURCE_DXBC_TPF is the "TPF" part, while "DXBC" is the
container format, although people (including Microsoft) often use them
interchangeably. Perhaps reword "how buffer UAVs are stored" to
something along the lines of "the type of resource to use for buffer
UAV bindings". Note that in the future this may apply to e.g. a GLSL
target as well.

>  enum vkd3d_shader_compile_option_buffer_uav
>  {
> -    /* Use buffer textures for buffer UAVs, this is the default. */
> +    /** Use buffer textures for buffer UAVs. This is the default option. */
>      VKD3D_SHADER_COMPILE_OPTION_BUFFER_UAV_STORAGE_TEXEL_BUFFER = 0x00000000,
Should that be "the default value"?

>  enum vkd3d_shader_compile_option_name
>  {
> +    /**
> +     * If \a value is nonzero, do not include debug information in the
> +     * compiled shader. The default value is zero.
> +     *
> +     * This option is supported by vkd3d_compiler. However, not all compilers
> +     * support generating debug information.
> +     */
>      VKD3D_SHADER_COMPILE_OPTION_STRIP_DEBUG = 0x00000001,
I'm not sure I understand the vkd3d_compiler (vkd3d-compiler?)
reference. For what it's worth, in the context of SPIR-V, debug
information is things like variable names (i.e., it gets you names
like "%main" instead of "%1"), and source information. We don't
currently use the latter, but we could embed the D3D assembly for the
source shader, which would probably help a little with debugging
SPIR-V shaders.

> +/**
> + * Various settings which may affect shader compilation or scanning, passed as
> + * part of struct vkd3d_shader_compile_info. For more details, see the
> + * documentation for individual options.
> + */
>  struct vkd3d_shader_compile_option
>  {
> +    /** Name of the setting. */
>      enum vkd3d_shader_compile_option_name name;
> +    /**
> +     * A value associated with the setting. The type and interpretation of the
> +     * value depends on the setting in question.
> +     */
>      unsigned int value;
>  };
"Compile option"/"option" instead of "setting" above?

> +/** A generic structure containing GPU shader source or byte code. */
>  struct vkd3d_shader_code
>  {
> +    /** Pointer to the byte code. */
>      const void *code;
> +    /** Size of \a code, in bytes. */
>      size_t size;
>  };
We use this for the output as well, which may not necessarily be byte
code. (E.g. GLSL or SPIR-V text.)

>  struct vkd3d_shader_compile_info
>  {
> +    /** Must be set to VKD3D_SHADER_STRUCTURE_TYPE_COMPILE_INFO. */
>      enum vkd3d_shader_structure_type type;
> +    /**
> +     * Optional pointer to a structure containing further parameters. For a list
> +     * of valid structures, refer to the respective function documentation. If
> +     * no further parameters are needed, this field should be set to NULL.
> +     */
>      const void *next;
>
> +    /** Input source code or byte code. */
>      struct vkd3d_shader_code source;
>
> +    /** Format of the input code passed in \ref source. */
>      enum vkd3d_shader_source_type source_type;
> +    /** Desired output format. */
>      enum vkd3d_shader_target_type target_type;
>
> +    /**
> +     * An array of compilation options. This field is ignored if
> +     * \ref option_count is zero, but must be valid otherwise.
> +     *
A pointer to an array, strictly speaking, because C.

> +     * It is undefined behaviour to pass the same option multiple times.
> +     *
I'd be happy to provide a stronger guarantee than "undefined" here.
E.g., "if the same option is specified multiple times, the last value
is used".

> +     * Options not relevant to or not supported by a particular shader compiler
> +     * or scanner will be ignored.
> +     */
>      const struct vkd3d_shader_compile_option *options;
> +    /** Size, in elements, of \ref options. */
>      unsigned int option_count;
>
> +    /** Minimum severity of messages returned from the shader function. */
>      enum vkd3d_shader_log_level log_level;
> +    /**
> +     * Name of the initial source file, used only in error messages. This
> +     * parameter is optional and may be NULL.
> +     */
>      const char *source_name;
>  };
>
I think the HLSL preprocessor defines __FILE__.