Re: [PATCH v2 0/4] MR263: vkd3d-shader/spirv: Refactor the type system.

[Henri Verbeet (＠hverbeet)]

> +#define DATA_TYPE_WIDTH_MASK         0xffu
> +#define DATA_TYPE_CLASS_SHIFT           8u
> +#define DATA_TYPE_CLASS_MASK        (0x3fu << DATA_TYPE_CLASS_SHIFT)
> +#define DATA_TYPE_NORM_SHIFT           14u
> +#define DATA_TYPE_NORM_MASK            (1u << DATA_TYPE_NORM_SHIFT)
> +#define DATA_TYPE_IS_SIGNED_SHIFT      15u
> +#define DATA_TYPE_IS_SIGNED_MASK       (1u << DATA_TYPE_IS_SIGNED_SHIFT)
> +
> +enum vkd3d_data_type_class
> +{
> +    VKD3D_DATA_CLASS_FLOAT,
> +    VKD3D_DATA_CLASS_INT,
> +    VKD3D_DATA_CLASS_RESOURCE,
> +    VKD3D_DATA_CLASS_SAMPLER,
> +    VKD3D_DATA_CLASS_UAV,
> +    VKD3D_DATA_CLASS_OPAQUE,
> +    VKD3D_DATA_CLASS_MIXED,
> +    VKD3D_DATA_CLASS_CONTINUED,
> +    VKD3D_DATA_CLASS_UNUSED,
> +};
> +
>  enum vkd3d_data_type
>  {
> -    VKD3D_DATA_FLOAT,
> -    VKD3D_DATA_INT,
> -    VKD3D_DATA_RESOURCE,
> -    VKD3D_DATA_SAMPLER,
> -    VKD3D_DATA_UAV,
> -    VKD3D_DATA_UINT,
> -    VKD3D_DATA_UNORM,
> -    VKD3D_DATA_SNORM,
> -    VKD3D_DATA_OPAQUE,
> -    VKD3D_DATA_MIXED,
> -    VKD3D_DATA_DOUBLE,
> -    VKD3D_DATA_CONTINUED,
> -    VKD3D_DATA_UNUSED,
> +    VKD3D_DATA_FLOAT     = (VKD3D_DATA_CLASS_FLOAT     << DATA_TYPE_CLASS_SHIFT) | 32,
> +    VKD3D_DATA_INT       = (VKD3D_DATA_CLASS_INT       << DATA_TYPE_CLASS_SHIFT) | 32 | DATA_TYPE_IS_SIGNED_MASK,
> +    VKD3D_DATA_RESOURCE  =  VKD3D_DATA_CLASS_RESOURCE  << DATA_TYPE_CLASS_SHIFT,
> +    VKD3D_DATA_SAMPLER   =  VKD3D_DATA_CLASS_SAMPLER   << DATA_TYPE_CLASS_SHIFT,
> +    VKD3D_DATA_UAV       =  VKD3D_DATA_CLASS_UAV       << DATA_TYPE_CLASS_SHIFT,
> +    VKD3D_DATA_UINT      = (VKD3D_DATA_CLASS_INT       << DATA_TYPE_CLASS_SHIFT) | 32,
> +    VKD3D_DATA_UNORM     = (VKD3D_DATA_CLASS_INT       << DATA_TYPE_CLASS_SHIFT) | 32 | DATA_TYPE_NORM_MASK,
> +    VKD3D_DATA_SNORM     = (VKD3D_DATA_CLASS_INT       << DATA_TYPE_CLASS_SHIFT) | 32 | DATA_TYPE_NORM_MASK | DATA_TYPE_IS_SIGNED_MASK,
> +    VKD3D_DATA_OPAQUE    =  VKD3D_DATA_CLASS_OPAQUE    << DATA_TYPE_CLASS_SHIFT,
> +    VKD3D_DATA_MIXED     =  VKD3D_DATA_CLASS_MIXED     << DATA_TYPE_CLASS_SHIFT,
> +    VKD3D_DATA_DOUBLE    = (VKD3D_DATA_CLASS_FLOAT     << DATA_TYPE_CLASS_SHIFT) | 64,
> +    VKD3D_DATA_CONTINUED =  VKD3D_DATA_CLASS_CONTINUED << DATA_TYPE_CLASS_SHIFT,
> +    VKD3D_DATA_UNUSED    =  VKD3D_DATA_CLASS_UNUSED    << DATA_TYPE_CLASS_SHIFT,
>  };

I'll wait for @giomasce, and possibly @etang-cw to take a look at this before looking more closely myself, but from just a quick look, do we really need the shifts and masks here? It would seem more straightforward to just store the required information in a structure, possibly using bitfields if we're particularly concerned about the size of that structure.

It's also not immediately clear to me from the commit messages what the benefit of using enum vkd3d_data_type instead of enum vkd3d_shader_component_type is. If it's just about supporting 16-bit and 64-bit components in struct signature_element, adding a "component_size" field or something similar would seem nicer.