Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.h:
};
+/* An HLSL data type. */ struct hlsl_type {
- /* Linked list entry to store this hlsl_type in the context's linked list of hlsl_types. */ struct list entry;
- /* Search tree entry to store this hlsl_type in the context's search tree of types. The type's
struct rb_entry scope_entry;* name is used as key. */
- /* These fields indicate to which category of hlsl_types this hlsl_type belongs.
* If type is HLSL_CLASS_OBJECT, base_type is used to indicate a subcategory of the type.
* If type is numeric, base_type is used to indicate the type of its components.
enum hlsl_type_class type; enum hlsl_base_type base_type;* base_type is not used when type is HLSL_CLASS_STRUCT or HLSL_CLASS_ARRAY. */
Here too the first line doesn't tell much that is not sort of obvious from the field declaration, and the others, while certainly useful, are a bit messy. I would avoid writing any comment for `type` and write something like this for `base_type`:
If `type` is `HLSL_CLASS_SCALAR`, `HLSL_CLASS_VECTOR` or `HLSL_CLASS_MATRIX`, then `base_type` is one of `HLSL_TYPE_FLOAT`, `HLSL_TYPE_HALF`, `HLSL_TYPE_DOUBLE`, `HLSL_TYPE_INT`, `HLSL_TYPE_UINT` and `HLSL_TYPE_BOOL`. If `type` is `HLSL_CLASS_OBJECT`, then `base_type` is one of `HLSL_TYPE_SAMPLER`, `HLSL_TYPE_TEXTURE`, `HLSL_TYPE_UAV`, `HLSL_TYPE_PIXELSHADER`, `HLSL_TYPE_VERTEXSHADER`, `HLSL_TYPE_STRING` and `HLSL_TYPE_VOID`. In all the other cases `base_type` is not used.
Not directly related to your patches, but field declarations could be fixed in the first place to make them more consistent. Right now we have an `hlsl_type` struct with fields `type` (of type `enum hlsl_type_class` with enum values beginning with `HLSL_CLASS_`) and `base_type` (of type `enum hlsl_base_type` with enum values beginning with `HLSL_TYPE_`). I would rename to `kind` (of type `enum hlsl_kind` with enum values beginning with `HLSL_KIND_`) and `base_type` (of type `enum hlsl_base_type` with enum values beginning with `HLSL_BASE_TYPE_`). But that's for another patch set.