Re: [PATCH v2 0/5] MR319: vkd3d-shader: Introduce vkd3d_shader_register.dimension, part 1.

[Henri Verbeet (＠hverbeet)]

From patch 2/5:
> @@ -1899,7 +1900,29 @@ static bool shader_sm4_read_param(struct vkd3d_shader_sm4_parser *priv, const ui
>                  break;
>          }
>      }
> -    else if (!shader_is_sm_5_1(priv) && sm4_register_is_descriptor(register_type))
> +    else
> +    {
> +        switch (dimension)
> +        {
> +            case VKD3D_SM4_DIMENSION_NONE:
> +                param->dimension = VKD3D_SHADER_DIMENSION_NONE;
> +                break;
> +
> +            case VKD3D_SM4_DIMENSION_SCALAR:
> +                param->dimension = VKD3D_SHADER_DIMENSION_SCALAR;
> +                break;
> +
> +            case VKD3D_SM4_DIMENSION_VEC4:
> +                param->dimension = VKD3D_SHADER_DIMENSION_VEC4;
> +                break;
> +
> +            default:
> +                FIXME("Unknown dimension %#x.\n", dimension);
> +                break;
> +        }
> +    }
> +
> +    if (!shader_is_sm_5_1(priv) && sm4_register_is_descriptor(register_type))
>      {
>          /* SM5.1 places a symbol identifier in idx[0] and moves
>           * other values up one slot. Normalize to SM5.1. */

I think this is a weird way to do this. IMO we should just do this:

```c
    param->dimension = dimension_from_sm4((token & VKD3D_SM4_DIMENSION_MASK) >> VKD3D_SM4_DIMENSION_SHIFT);
    if (register_type == VKD3D_SM4_RT_IMMCONST || register_type == VKD3D_SM4_RT_IMMCONST64)
    {
        unsigned int dword_count;

switch (param->dimension)
        {
        ...
    }

```

The other issue I have with this MR is that I think it would be more practical to first rename shader_register_init() to vsir_register_init() and start using it in individual functions like shader_sm4_read_param(), shader_sm1_parse_src_param(), and so on, and only then make the other changes. I'd suggest breaking patch 5/5 into separate commits for each function.