[PATCH vkd3d] vkd3d-shader/hlsl: Add support for texture arrays. Replace sampler_dim with resource_dim.

[Francisco Casas]

December 7, 2021 2:58 PM, "Zebediah Figura" <zfigura at codeweavers.com> wrote:

>
> As one can tell from the commit message, this patch does at least two things at once, which is at
> least one too many ;-)
> 

Acknowledged!


> Personally I'm not sure that "sampler_dim" (or any of the associated names) need to be changed. I
> may be looking at things with GLSL-coloured glasses, but it seems to me there's no conceptual
> reason why a sampler couldn't have an array dimension; but rather HLSL just doesn't have a
> "sample2DArray" keyword.

Okay then. After some thought I have been more inclined to think that way too.
More because the number of things that have to be changed is significant.
At least this experiment helped me to understand on which parts of the codebase
sampler_dim is used.


>> @@ -1845,9 +1828,11 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs,
>> struct hl
>> /* Only HLSL_IR_LOAD can return an object. */
>> object_load = hlsl_ir_load(object);
>>> - if (!strcmp(name, "Load"))
>> + if (!strcmp(name, "Load")
>> + && object_type->resource_dim != HLSL_RESOURCE_DIM_CUBE
>> + && object_type->resource_dim != HLSL_RESOURCE_DIM_CUBEARRAY)
>> {
>> - const unsigned int sampler_dim = sampler_dim_count(object_type->sampler_dim);
>> + const unsigned int sampler_dim = sampler_dim_count(object_type->resource_dim);
>> struct hlsl_ir_resource_load *load;
>> struct hlsl_ir_node *coords;
> 
> This should also be a separate patch.

Okay.
 
>> @@ -1862,7 +1847,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct
>> hl
>>> /* +1 for the mipmap level */
>> if (!(coords = add_implicit_conversion(ctx, instrs, params->args[0],
>> - hlsl_get_vector_type(ctx, HLSL_TYPE_INT, sampler_dim + 1), loc)))
>> + hlsl_get_vector_type(ctx, HLSL_TYPE_INT, (sampler_dim==4)? 4 : sampler_dim + 1), loc)))
>> return false;
>>> if (!(load = hlsl_new_resource_load(ctx, object_type->e.resource_format, HLSL_RESOURCE_LOAD,
> This seems redundant; with the above hunk we can't hit this path for cube arrays.

Yes, my mistake.

>> @@ -1871,9 +1856,11 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs,
>> struct hl
>> list_add_tail(instrs, &load->node.entry);
>> return true;
>> }
>> - else if (!strcmp(name, "Sample"))
>> + else if (!strcmp(name, "Sample")
>> + && object_type->resource_dim != HLSL_RESOURCE_DIM_2DMS
>> + && object_type->resource_dim != HLSL_RESOURCE_DIM_2DMSARRAY)
>> {
>> - const unsigned int sampler_dim = sampler_dim_count(object_type->sampler_dim);
>> + const unsigned int sampler_dim = sampler_dim_count(object_type->resource_dim);
>> const struct hlsl_type *sampler_type;
>> struct hlsl_ir_resource_load *load;
>> struct hlsl_ir_load *sampler_load;
> 
> This should also be a separate patch.

O-ho-kay (I changed the intonation of the Okay to not sound repetitive).

>> @@ -424,20 +428,34 @@ static D3D_RESOURCE_RETURN_TYPE sm4_resource_format(const struct hlsl_type
>> *type
>>> static D3D_SRV_DIMENSION sm4_rdef_resource_dimension(const struct hlsl_type *type)
>> {
>> - switch (type->sampler_dim)
>> + if (type->base_type == HLSL_TYPE_TEXTURE)
> 
> Does this need to be a conditional? Should this just be an assertion instead?

I did it because the return type D3D_SRV_DIMENSION had the
D3D_SRV_DIMENSION_BUFFER
D3D_SRV_DIMENSION_BUFFEREX
values for buffer types and because sm4_rdef_resource_dimension() is called from a
path that seems could be used for other types than HLSL_TYPE_TEXTURE.
But maybe is too soon to care about that, yes, an assertion for now would do.

>> {
>> - case HLSL_SAMPLER_DIM_1D:
>> - return D3D_SRV_DIMENSION_TEXTURE1D;
>> - case HLSL_SAMPLER_DIM_2D:
>> - return D3D_SRV_DIMENSION_TEXTURE2D;
>> - case HLSL_SAMPLER_DIM_3D:
>> - return D3D_SRV_DIMENSION_TEXTURE3D;
>> - case HLSL_SAMPLER_DIM_CUBE:
>> - return D3D_SRV_DIMENSION_TEXTURECUBE;
>> - default:
>> - assert(0);
>> - return D3D_SRV_DIMENSION_UNKNOWN;
>> + switch (type->resource_dim)
>> + {
>> + case HLSL_RESOURCE_DIM_1D:
>> + return D3D_SRV_DIMENSION_TEXTURE1D;
>> + case HLSL_RESOURCE_DIM_1DARRAY:
>> + return D3D_SRV_DIMENSION_TEXTURE1DARRAY;
>> + case HLSL_RESOURCE_DIM_2D:
>> + return D3D_SRV_DIMENSION_TEXTURE2D;
>> + case HLSL_RESOURCE_DIM_2DARRAY:
>> + return D3D_SRV_DIMENSION_TEXTURE2DARRAY;
>> + case HLSL_RESOURCE_DIM_2DMS:
>> + return D3D_SRV_DIMENSION_TEXTURE2DMS;
>> + case HLSL_RESOURCE_DIM_2DMSARRAY:
>> + return D3D_SRV_DIMENSION_TEXTURE2DMSARRAY;
>> + case HLSL_RESOURCE_DIM_3D:
>> + return D3D_SRV_DIMENSION_TEXTURE3D;
>> + case HLSL_RESOURCE_DIM_CUBE:
>> + return D3D_SRV_DIMENSION_TEXTURECUBE;
>> + case HLSL_RESOURCE_DIM_CUBEARRAY:
>> + return D3D_SRV_DIMENSION_TEXTURECUBEARRAY;
>> + default:
>> + assert(0);
>> + return D3D_SRV_DIMENSION_UNKNOWN;
>> + }
>> }
>> + assert(0);
>> }
>>> static int sm4_compare_externs(const struct hlsl_ir_var *a, const struct hlsl_ir_var *b)
>> @@ -711,20 +729,34 @@ static void write_sm4_rdef(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc)
>>> static enum vkd3d_sm4_resource_type sm4_resource_dimension(const struct hlsl_type *type)
>> {
>> - switch (type->sampler_dim)
>> - {
>> - case HLSL_SAMPLER_DIM_1D:
>> - return VKD3D_SM4_RESOURCE_TEXTURE_1D;
>> - case HLSL_SAMPLER_DIM_2D:
>> - return VKD3D_SM4_RESOURCE_TEXTURE_2D;
>> - case HLSL_SAMPLER_DIM_3D:
>> - return VKD3D_SM4_RESOURCE_TEXTURE_3D;
>> - case HLSL_SAMPLER_DIM_CUBE:
>> - return VKD3D_SM4_RESOURCE_TEXTURE_CUBE;
>> - default:
>> - assert(0);
>> - return 0;
>> + if (type->base_type == HLSL_TYPE_TEXTURE) {
> 
> Same here.
> 

Yes, same answer as before.

>> + switch (type->resource_dim)
>> + {
>> + case HLSL_RESOURCE_DIM_1D:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_1D;
>> + case HLSL_RESOURCE_DIM_2D:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_2D;
>> + case HLSL_RESOURCE_DIM_3D:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_3D;
>> + case HLSL_RESOURCE_DIM_CUBE:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_CUBE;
>> + case HLSL_RESOURCE_DIM_1DARRAY:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_1DARRAY;
>> + case HLSL_RESOURCE_DIM_2DARRAY:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_2DARRAY;
>> + case HLSL_RESOURCE_DIM_2DMS:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_2DMS;
>> + case HLSL_RESOURCE_DIM_2DMSARRAY:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_2DMSARRAY;
>> + case HLSL_RESOURCE_DIM_CUBEARRAY:
>> + return VKD3D_SM4_RESOURCE_TEXTURE_CUBEARRAY;
>> + default:
>> + assert(0);
>> + return 0;
>> + }
>> }
>> + assert(0);
>> + return 0;
>> }
>>> struct sm4_register


I will split the patch and and fix the problems to make (another)
batch for sending after Matteo's is reviewed.  

Thanks for the patience!