This patch is a proposal for considering separating texture_dim and sampler_dim, it probably will be resent after Matteo's batch is reviewed. Opinions are appreciated.
So far, the
enum hlsl_sampler_dim sampler_dim;
member of struct hlsl_type has been used to store the dimension type of both HLSL_TYPE_SAMPLER and HLSL_TYPE_TEXTURE types.
However, textures have more possible options, e.g. Texture1DArray and Texture2DArray, which are ld and sampled with vectors of 2 and 3 dimensions respectively. Support for these is added in this patch.
The new enum hlsl_texture_dim is added to include these cases and the
enum hlsl_texture_dim texture_dim;
member is added to struct hlsl_type to store the dimension type of HLSL_TYPE_TEXTURE types.
Nevertheless, sampler_dim is still used on HLSL_TYPE_TEXTURE types to know the compatible types for sampling (albeit this could be deducted from texture_dim).
Future support for Texture2DMS and Texture2DMSArray would be affected if this strategy is chosen.
Signed-off-by: Francisco Casas fcasas@codeweavers.com --- libs/vkd3d-shader/hlsl.c | 48 +++++++++++++++++++++++++----------- libs/vkd3d-shader/hlsl.h | 14 ++++++++++- libs/vkd3d-shader/hlsl.y | 24 ++++++++++++------ libs/vkd3d-shader/hlsl_sm4.c | 48 ++++++++++++++++++++++++------------ 4 files changed, 95 insertions(+), 39 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 9824c56d..ed061d42 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -261,7 +261,7 @@ struct hlsl_type *hlsl_new_struct_type(struct hlsl_ctx *ctx, const char *name, s return type; }
-struct hlsl_type *hlsl_new_texture_type(struct hlsl_ctx *ctx, enum hlsl_sampler_dim dim, struct hlsl_type *format) +struct hlsl_type *hlsl_new_texture_type(struct hlsl_ctx *ctx, enum hlsl_texture_dim dim, struct hlsl_type *format) { struct hlsl_type *type;
@@ -271,7 +271,14 @@ struct hlsl_type *hlsl_new_texture_type(struct hlsl_ctx *ctx, enum hlsl_sampler_ type->base_type = HLSL_TYPE_TEXTURE; type->dimx = 4; type->dimy = 1; - type->sampler_dim = dim; + type->texture_dim = dim; + if (dim == HLSL_TEXTURE_DIM_1DARRAY) + type->sampler_dim = HLSL_SAMPLER_DIM_2D; + else if (dim == HLSL_TEXTURE_DIM_2DARRAY) + type->sampler_dim = HLSL_SAMPLER_DIM_3D; + else + type->sampler_dim = (enum hlsl_sampler_dim) dim; + type->e.resource_format = format; list_add_tail(&ctx->types, &type->entry); return type; @@ -345,11 +352,16 @@ bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2 return false; if (t1->base_type != t2->base_type) return false; - if (t1->base_type == HLSL_TYPE_SAMPLER || t1->base_type == HLSL_TYPE_TEXTURE) + if (t1->base_type == HLSL_TYPE_SAMPLER) { if (t1->sampler_dim != t2->sampler_dim) return false; - if (t1->base_type == HLSL_TYPE_TEXTURE && t1->sampler_dim != HLSL_SAMPLER_DIM_GENERIC + } + if (t1->base_type == HLSL_TYPE_TEXTURE) + { + if (t1->texture_dim != t2->texture_dim) + return false; + if (t1->texture_dim != HLSL_TEXTURE_DIM_GENERIC && !hlsl_types_are_equal(t1->e.resource_format, t2->e.resource_format)) return false; } @@ -414,6 +426,7 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old, if (!(type->modifiers & HLSL_MODIFIERS_MAJORITY_MASK)) type->modifiers |= default_majority; type->sampler_dim = old->sampler_dim; + type->texture_dim = old->texture_dim; switch (old->type) { case HLSL_CLASS_ARRAY: @@ -778,12 +791,17 @@ static int compare_param_hlsl_types(const struct hlsl_type *t1, const struct hls } if (t1->base_type != t2->base_type) return t1->base_type - t2->base_type; - if ((t1->base_type == HLSL_TYPE_SAMPLER || t1->base_type == HLSL_TYPE_TEXTURE) - && t1->sampler_dim != t2->sampler_dim) + if (t1->base_type == HLSL_TYPE_SAMPLER) { if (t1->sampler_dim != t2->sampler_dim) return t1->sampler_dim - t2->sampler_dim; - if (t1->base_type == HLSL_TYPE_TEXTURE && t1->sampler_dim != HLSL_SAMPLER_DIM_GENERIC + + } + if (t1->base_type == HLSL_TYPE_TEXTURE) + { + if (t1->texture_dim != t2->texture_dim) + return t1->texture_dim - t2->texture_dim; + if (t1->texture_dim != HLSL_TEXTURE_DIM_GENERIC && (r = compare_param_hlsl_types(t1->e.resource_format, t2->e.resource_format))) return r; } @@ -917,24 +935,26 @@ struct vkd3d_string_buffer *hlsl_type_to_string(struct hlsl_ctx *ctx, const stru { static const char *const dimensions[] = { - [HLSL_SAMPLER_DIM_1D] = "1D", - [HLSL_SAMPLER_DIM_2D] = "2D", - [HLSL_SAMPLER_DIM_3D] = "3D", - [HLSL_SAMPLER_DIM_CUBE] = "Cube" + [HLSL_TEXTURE_DIM_1D] = "1D", + [HLSL_TEXTURE_DIM_1DARRAY] = "1DArray", + [HLSL_TEXTURE_DIM_2D] = "2D", + [HLSL_TEXTURE_DIM_2DARRAY] = "2DArray", + [HLSL_TEXTURE_DIM_3D] = "3D", + [HLSL_TEXTURE_DIM_CUBE] = "Cube" };
switch (type->base_type) { case HLSL_TYPE_TEXTURE: - if (type->sampler_dim == HLSL_SAMPLER_DIM_GENERIC) + if (type->texture_dim == HLSL_TEXTURE_DIM_GENERIC) { vkd3d_string_buffer_printf(string, "Texture"); return string; }
- assert(type->sampler_dim < ARRAY_SIZE(dimensions)); + assert(type->texture_dim < ARRAY_SIZE(dimensions)); assert(type->e.resource_format->base_type < ARRAY_SIZE(base_types)); - vkd3d_string_buffer_printf(string, "Texture%s<%s%u>", dimensions[type->sampler_dim], + vkd3d_string_buffer_printf(string, "Texture%s<%s%u>", dimensions[type->texture_dim], base_types[type->e.resource_format->base_type], type->e.resource_format->dimx); return string;
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index e7bdb45e..9d73d979 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -101,6 +101,17 @@ enum hlsl_sampler_dim HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBE };
+enum hlsl_texture_dim +{ + HLSL_TEXTURE_DIM_GENERIC = HLSL_SAMPLER_DIM_GENERIC, + HLSL_TEXTURE_DIM_1D = HLSL_SAMPLER_DIM_1D, + HLSL_TEXTURE_DIM_2D = HLSL_SAMPLER_DIM_2D, + HLSL_TEXTURE_DIM_3D = HLSL_SAMPLER_DIM_3D, + HLSL_TEXTURE_DIM_CUBE = HLSL_SAMPLER_DIM_CUBE, + HLSL_TEXTURE_DIM_1DARRAY, + HLSL_TEXTURE_DIM_2DARRAY, +}; + enum hlsl_matrix_majority { HLSL_COLUMN_MAJOR, @@ -114,6 +125,7 @@ struct hlsl_type enum hlsl_type_class type; enum hlsl_base_type base_type; enum hlsl_sampler_dim sampler_dim; + enum hlsl_texture_dim texture_dim; const char *name; unsigned int modifiers; unsigned int dimx; @@ -716,7 +728,7 @@ struct hlsl_ir_swizzle *hlsl_new_swizzle(struct hlsl_ctx *ctx, DWORD s, unsigned struct hlsl_ir_node *val, struct vkd3d_shader_location *loc); struct hlsl_ir_var *hlsl_new_synthetic_var(struct hlsl_ctx *ctx, const char *name, struct hlsl_type *type, const struct vkd3d_shader_location loc); -struct hlsl_type *hlsl_new_texture_type(struct hlsl_ctx *ctx, enum hlsl_sampler_dim dim, struct hlsl_type *format); +struct hlsl_type *hlsl_new_texture_type(struct hlsl_ctx *ctx, enum hlsl_texture_dim dim, struct hlsl_type *format); struct hlsl_ir_constant *hlsl_new_uint_constant(struct hlsl_ctx *ctx, unsigned int n, const struct vkd3d_shader_location loc); struct hlsl_ir_node *hlsl_new_unary_expr(struct hlsl_ctx *ctx, enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg, diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ff35c09e..13853127 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1831,7 +1831,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl struct hlsl_ir_load *object_load;
if (object_type->type != HLSL_CLASS_OBJECT || object_type->base_type != HLSL_TYPE_TEXTURE - || object_type->sampler_dim == HLSL_SAMPLER_DIM_GENERIC) + || object_type->texture_dim == HLSL_TEXTURE_DIM_GENERIC) { struct vkd3d_string_buffer *string;
@@ -1959,7 +1959,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl struct parse_colon_attribute colon_attribute; struct hlsl_semantic semantic; enum hlsl_buffer_type buffer_type; - enum hlsl_sampler_dim sampler_dim; + enum hlsl_texture_dim texture_dim; }
%token KW_BLENDSTATE @@ -2134,7 +2134,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl
%type <reg_reservation> register_opt
-%type <sampler_dim> texture_type +%type <texture_dim> texture_type
%type <semantic> semantic
@@ -2554,19 +2554,27 @@ input_mod: texture_type: KW_TEXTURE1D { - $$ = HLSL_SAMPLER_DIM_1D; + $$ = HLSL_TEXTURE_DIM_1D; } | KW_TEXTURE2D { - $$ = HLSL_SAMPLER_DIM_2D; + $$ = HLSL_TEXTURE_DIM_2D; } | KW_TEXTURE3D { - $$ = HLSL_SAMPLER_DIM_3D; + $$ = HLSL_TEXTURE_DIM_3D; } | KW_TEXTURECUBE { - $$ = HLSL_SAMPLER_DIM_CUBE; + $$ = HLSL_TEXTURE_DIM_CUBE; + } + | KW_TEXTURE1DARRAY + { + $$ = HLSL_TEXTURE_DIM_1DARRAY; + } + | KW_TEXTURE2DARRAY + { + $$ = HLSL_TEXTURE_DIM_2DARRAY; }
type: @@ -2654,7 +2662,7 @@ type: } | KW_TEXTURE { - $$ = hlsl_new_texture_type(ctx, HLSL_SAMPLER_DIM_GENERIC, NULL); + $$ = hlsl_new_texture_type(ctx, HLSL_TEXTURE_DIM_GENERIC, NULL); } | texture_type { diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index c0c26f80..5275be53 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -302,18 +302,22 @@ static D3D_SHADER_VARIABLE_TYPE sm4_base_type(const struct hlsl_type *type) case HLSL_TYPE_STRING: return D3D_SVT_STRING; case HLSL_TYPE_TEXTURE: - switch (type->sampler_dim) + switch (type->texture_dim) { - case HLSL_SAMPLER_DIM_1D: + case HLSL_TEXTURE_DIM_1D: return D3D_SVT_TEXTURE1D; - case HLSL_SAMPLER_DIM_2D: + case HLSL_TEXTURE_DIM_2D: return D3D_SVT_TEXTURE2D; - case HLSL_SAMPLER_DIM_3D: + case HLSL_TEXTURE_DIM_3D: return D3D_SVT_TEXTURE3D; - case HLSL_SAMPLER_DIM_CUBE: + case HLSL_TEXTURE_DIM_CUBE: return D3D_SVT_TEXTURECUBE; - case HLSL_SAMPLER_DIM_GENERIC: + case HLSL_TEXTURE_DIM_GENERIC: return D3D_SVT_TEXTURE; + case HLSL_TEXTURE_DIM_1DARRAY: + return D3D_SVT_TEXTURE1DARRAY; + case HLSL_TEXTURE_DIM_2DARRAY: + return D3D_SVT_TEXTURE2DARRAY; default: assert(0); } @@ -424,16 +428,22 @@ 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) + enum hlsl_texture_dim tex_dim = (type->base_type == HLSL_TYPE_TEXTURE)? + type->texture_dim : (enum hlsl_texture_dim) type->sampler_dim; + switch (tex_dim) { - case HLSL_SAMPLER_DIM_1D: + case HLSL_TEXTURE_DIM_1D: return D3D_SRV_DIMENSION_TEXTURE1D; - case HLSL_SAMPLER_DIM_2D: + case HLSL_TEXTURE_DIM_2D: return D3D_SRV_DIMENSION_TEXTURE2D; - case HLSL_SAMPLER_DIM_3D: + case HLSL_TEXTURE_DIM_3D: return D3D_SRV_DIMENSION_TEXTURE3D; - case HLSL_SAMPLER_DIM_CUBE: + case HLSL_TEXTURE_DIM_CUBE: return D3D_SRV_DIMENSION_TEXTURECUBE; + case HLSL_TEXTURE_DIM_1DARRAY: + return D3D_SRV_DIMENSION_TEXTURE1DARRAY; + case HLSL_TEXTURE_DIM_2DARRAY: + return D3D_SRV_DIMENSION_TEXTURE2DARRAY; default: assert(0); return D3D_SRV_DIMENSION_UNKNOWN; @@ -711,16 +721,22 @@ 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) + enum hlsl_texture_dim tex_dim = (type->base_type == HLSL_TYPE_TEXTURE)? + type->texture_dim : (enum hlsl_texture_dim) type->sampler_dim; + switch (tex_dim) { - case HLSL_SAMPLER_DIM_1D: + case HLSL_TEXTURE_DIM_1D: return VKD3D_SM4_RESOURCE_TEXTURE_1D; - case HLSL_SAMPLER_DIM_2D: + case HLSL_TEXTURE_DIM_2D: return VKD3D_SM4_RESOURCE_TEXTURE_2D; - case HLSL_SAMPLER_DIM_3D: + case HLSL_TEXTURE_DIM_3D: return VKD3D_SM4_RESOURCE_TEXTURE_3D; - case HLSL_SAMPLER_DIM_CUBE: + case HLSL_TEXTURE_DIM_CUBE: return VKD3D_SM4_RESOURCE_TEXTURE_CUBE; + case HLSL_TEXTURE_DIM_1DARRAY: + return VKD3D_SM4_RESOURCE_TEXTURE_1DARRAY; + case HLSL_TEXTURE_DIM_2DARRAY: + return VKD3D_SM4_RESOURCE_TEXTURE_2DARRAY; default: assert(0); return 0;
Hi,
On 03/12/21 18:57, Francisco Casas wrote:
This patch is a proposal for considering separating texture_dim and sampler_dim, it probably will be resent after Matteo's batch is reviewed. Opinions are appreciated.
So far, the
enum hlsl_sampler_dim sampler_dim;
member of struct hlsl_type has been used to store the dimension type of both HLSL_TYPE_SAMPLER and HLSL_TYPE_TEXTURE types.
However, textures have more possible options, e.g. Texture1DArray and Texture2DArray, which are ld and sampled with vectors of 2 and 3 dimensions respectively. Support for these is added in this patch.
The new enum hlsl_texture_dim is added to include these cases and the
enum hlsl_texture_dim texture_dim;
member is added to struct hlsl_type to store the dimension type of HLSL_TYPE_TEXTURE types.
Nevertheless, sampler_dim is still used on HLSL_TYPE_TEXTURE types to know the compatible types for sampling (albeit this could be deducted from texture_dim).
Future support for Texture2DMS and Texture2DMSArray would be affected if this strategy is chosen.
Signed-off-by: Francisco Casas fcasas@codeweavers.com
As a general suggestion, I think the commit message should be already in the version that would be committed if the commit were to be accepted. I.e., just describe what the patch does and why, not the various other process issues like the fact that it will be submitted again after another patch set or what does change if one or another strategy is chosen.
If you want to add this kind of content, that in this case and in many other is useful and important, separate it from the rest in this way:
""" foo/bar: Add baz field.
The baz field can be used by downstream consumers to ensure that the frobnicator is correctly spingled.
Signed-off-by: John Doe jdoe@example.com --- I think this will be useful when we'll be implementing droobster simplification, becase yada yada yada. """
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index e7bdb45e..9d73d979 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -101,6 +101,17 @@ enum hlsl_sampler_dim HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBE };
+enum hlsl_texture_dim +{
- HLSL_TEXTURE_DIM_GENERIC = HLSL_SAMPLER_DIM_GENERIC,
- HLSL_TEXTURE_DIM_1D = HLSL_SAMPLER_DIM_1D,
- HLSL_TEXTURE_DIM_2D = HLSL_SAMPLER_DIM_2D,
- HLSL_TEXTURE_DIM_3D = HLSL_SAMPLER_DIM_3D,
- HLSL_TEXTURE_DIM_CUBE = HLSL_SAMPLER_DIM_CUBE,
- HLSL_TEXTURE_DIM_1DARRAY,
- HLSL_TEXTURE_DIM_2DARRAY,
+};
Thinking again about this and looking at the patches I have in my queue, I now think this looks a bit too much of a duplication. I'd rather unify the two enums in a singled "agnostic" one along these lines:
enum hlsl_dimension { HLSL_DIM_GENERIC, HLSL_DIM_1D, HLSL_DIM_2D, HLSL_DIM_3D, HLSL_DIM_CUBE, HLSL_DIM_MAX_SAMPLER = HLSL_DIM_CUBE, HLSL_DIM_1DARRAY, HLSL_DIM_2DARRAY, HLSL_DIM_2DMS, HLSL_DIM_MAX = HLSL_DIM_2DMS, };
The value HLSL_DIM_MAX_SAMPLER can be used to check whether a dimension value is valid in the context of a sampler dimension.
enum hlsl_matrix_majority { HLSL_COLUMN_MAJOR, @@ -114,6 +125,7 @@ struct hlsl_type enum hlsl_type_class type; enum hlsl_base_type base_type; enum hlsl_sampler_dim sampler_dim;
- enum hlsl_texture_dim texture_dim;
Consequently, we'd also have just one field of type hlsl_dimension. As for the name, it might be sensible to avoid "dim" or "dimension", which might get confused with the fields in hlsl_type. Possibly "resource_dim"? I am not sure.
My two cents.
Giovanni.
As a general suggestion, I think the commit message should be already in the version that would be committed if the commit were to be accepted. I.e., just describe what the patch does and why, not the various other process issues like the fact that it will be submitted again after another patch set or what does change if one or another strategy is chosen.
If you want to add this kind of content, that in this case and in many other is useful and important, separate it from the rest in this way:
""" foo/bar: Add baz field.
The baz field can be used by downstream consumers to ensure that the frobnicator is correctly spingled.
Signed-off-by: John Doe jdoe@example.com
I think this will be useful when we'll be implementing droobster simplification, becase yada yada yada. """
Okay, thanks for the tip!
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index e7bdb45e..9d73d979 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -101,6 +101,17 @@ enum hlsl_sampler_dim HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBE };
+enum hlsl_texture_dim
+{
- HLSL_TEXTURE_DIM_GENERIC = HLSL_SAMPLER_DIM_GENERIC,
- HLSL_TEXTURE_DIM_1D = HLSL_SAMPLER_DIM_1D,
- HLSL_TEXTURE_DIM_2D = HLSL_SAMPLER_DIM_2D,
- HLSL_TEXTURE_DIM_3D = HLSL_SAMPLER_DIM_3D,
- HLSL_TEXTURE_DIM_CUBE = HLSL_SAMPLER_DIM_CUBE,
- HLSL_TEXTURE_DIM_1DARRAY,
- HLSL_TEXTURE_DIM_2DARRAY,
+};
Thinking again about this and looking at the patches I have in my queue, I now think this looks a bit too much of a duplication. I'd rather unify the two enums in a singled "agnostic" one along these lines:
enum hlsl_dimension { HLSL_DIM_GENERIC, HLSL_DIM_1D, HLSL_DIM_2D, HLSL_DIM_3D, HLSL_DIM_CUBE, HLSL_DIM_MAX_SAMPLER = HLSL_DIM_CUBE, HLSL_DIM_1DARRAY, HLSL_DIM_2DARRAY, HLSL_DIM_2DMS, HLSL_DIM_MAX = HLSL_DIM_2DMS, };
The value HLSL_DIM_MAX_SAMPLER can be used to check whether a dimension value is valid in the context of a sampler dimension.
Okay, I will work on the same patch but using this strategy.
enum hlsl_matrix_majority { HLSL_COLUMN_MAJOR, @@ -114,6 +125,7 @@ struct hlsl_type enum hlsl_type_class type; enum hlsl_base_type base_type; enum hlsl_sampler_dim sampler_dim;
- enum hlsl_texture_dim texture_dim;
Consequently, we'd also have just one field of type hlsl_dimension. As for the name, it might be sensible to avoid "dim" or "dimension", which might get confused with the fields in hlsl_type. Possibly "resource_dim"? I am not sure.
I like the name change to avoid regressions, it forces to pay attention to all related parts of the code.
The only options for names I can think of are "resource_arrangement" (resource_argmt) or "resource_layout". Probably "resource_dim" is better.
Best regards, Francisco