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

[Zebediah Figura]

On 8/27/20 11:17 AM, Henri Verbeet wrote:
> 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.

Right, I can drop the reference to source and target types.

> 
>>  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"?

Probably, yes.

> 
>>  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. 

That was an error for "vkd3d_shader_compile()".

> 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.

Do you think it's a good idea to go into exact detail on what kind of
debug information is included for each target, then? I kind of figured
that'd be out of scope for this documentation.

> 
>> +/**
>> + * 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?

Yes, probably.

> 
>> +/** 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.)

I tried to cover that with "source code or byte code". I guess maybe
it'd be better to just remove that phrase.

> 
>>  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__.
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200827/720fda77/attachment.sig>