[PATCH vkd3d v3 07/12] vkd3d-shader/hlsl: Add dimension types for textures to enum hlsl_sampler_dim.
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 25 +++++++++++++++---------- libs/vkd3d-shader/hlsl.h | 10 ++++++++-- libs/vkd3d-shader/hlsl_sm4.c | 2 +- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index c664d5b1..2bd4b5c8 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -929,10 +929,15 @@ 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_SAMPLER_DIM_1D] = "1D", + [HLSL_SAMPLER_DIM_2D] = "2D", + [HLSL_SAMPLER_DIM_3D] = "3D", + [HLSL_SAMPLER_DIM_CUBE] = "Cube", + [HLSL_SAMPLER_DIM_1DARRAY] = "1DArray", + [HLSL_SAMPLER_DIM_2DARRAY] = "2DArray", + [HLSL_SAMPLER_DIM_2DMS] = "2DMS", + [HLSL_SAMPLER_DIM_2DMSARRAY] = "2DMSArray", + [HLSL_SAMPLER_DIM_CUBEARRAY] = "CubeArray", }; switch (type->base_type) @@ -1776,11 +1781,11 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) static const char *const sampler_names[] = { - "sampler", - "sampler1D", - "sampler2D", - "sampler3D", - "samplerCUBE" + [HLSL_SAMPLER_DIM_GENERIC] = "sampler", + [HLSL_SAMPLER_DIM_1D] = "sampler1D", + [HLSL_SAMPLER_DIM_2D] = "sampler2D", + [HLSL_SAMPLER_DIM_3D] = "sampler3D", + [HLSL_SAMPLER_DIM_CUBE] = "samplerCUBE", }; static const struct @@ -1832,7 +1837,7 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) } } - for (bt = 0; bt <= HLSL_SAMPLER_DIM_MAX; ++bt) + for (bt = 0; bt <= HLSL_SAMPLER_DIM_LAST_SAMPLER; ++bt) { type = hlsl_new_type(ctx, sampler_names[bt], HLSL_CLASS_OBJECT, HLSL_TYPE_SAMPLER, 1, 1); type->sampler_dim = bt; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 49fa8d9d..334610df 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -98,7 +98,13 @@ enum hlsl_sampler_dim HLSL_SAMPLER_DIM_2D, HLSL_SAMPLER_DIM_3D, HLSL_SAMPLER_DIM_CUBE, - HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBE + HLSL_SAMPLER_DIM_LAST_SAMPLER = HLSL_SAMPLER_DIM_CUBE, + HLSL_SAMPLER_DIM_1DARRAY, + HLSL_SAMPLER_DIM_2DARRAY, + HLSL_SAMPLER_DIM_2DMS, + HLSL_SAMPLER_DIM_2DMSARRAY, + HLSL_SAMPLER_DIM_CUBEARRAY, + HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBEARRAY, }; enum hlsl_matrix_majority @@ -489,7 +495,7 @@ struct hlsl_ctx struct hlsl_type *vector[HLSL_TYPE_LAST_SCALAR + 1][4]; /* matrix[float][2][4] is a float4x2, i.e. dimx = 2, dimy = 4 */ struct hlsl_type *matrix[HLSL_TYPE_LAST_SCALAR + 1][4][4]; - struct hlsl_type *sampler[HLSL_SAMPLER_DIM_MAX + 1]; + struct hlsl_type *sampler[HLSL_SAMPLER_DIM_LAST_SAMPLER + 1]; struct hlsl_type *Void; } builtin_types; diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index be35d125..cefe173d 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -1291,7 +1291,7 @@ static void write_sm4_ld(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buf case HLSL_SAMPLER_DIM_CUBE: break; - case HLSL_SAMPLER_DIM_GENERIC: + default: assert(0); } -- 2.25.1
On 12/17/21 13:12, Francisco Casas wrote:
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index be35d125..cefe173d 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -1291,7 +1291,7 @@ static void write_sm4_ld(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buf case HLSL_SAMPLER_DIM_CUBE: break;
- case HLSL_SAMPLER_DIM_GENERIC: + default: assert(0); }
I prefer to err on the side of avoiding 'default', to force one to explicitly deal with adding new cases to an enum. In this case especially it seems especially incorrect, since at least some of the new enum values are valid cases and should be handled.
On 12/17/21 17:29, Zebediah Figura (she/her) wrote:
On 12/17/21 13:12, Francisco Casas wrote:
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index be35d125..cefe173d 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -1291,7 +1291,7 @@ static void write_sm4_ld(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buf case HLSL_SAMPLER_DIM_CUBE: break; - case HLSL_SAMPLER_DIM_GENERIC: + default: assert(0); }
I prefer to err on the side of avoiding 'default', to force one to explicitly deal with adding new cases to an enum.
In this case especially it seems especially incorrect, since at least some of the new enum values are valid cases and should be handled.
Along the same lines I would probably merge patches 08/12 and 10/12 into this one. Note also that fixing this hunk makes the subject of patch 11/12 not make sense anymore, so that should be rewritten. I would also suggest reordering that patch before this one, to save yourself some work.
December 17, 2021 8:35 PM, "Zebediah Figura (she/her)" <zfigura(a)codeweavers.com> wrote:
On 12/17/21 17:29, Zebediah Figura (she/her) wrote:
On 12/17/21 13:12, Francisco Casas wrote:
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index be35d125..cefe173d 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -1291,7 +1291,7 @@ static void write_sm4_ld(struct hlsl_ctx *ctx, >> struct vkd3d_bytecode_buffer *buf case HLSL_SAMPLER_DIM_CUBE: break; - case HLSL_SAMPLER_DIM_GENERIC: + default: assert(0); }
I prefer to err on the side of avoiding 'default', to force one to > explicitly deal with adding new cases to an enum. In this case especially it seems especially incorrect, since at least > some of the new enum values are valid cases and should be handled.
Along the same lines I would probably merge patches 08/12 and 10/12 into this one.
Note also that fixing this hunk makes the subject of patch 11/12 not make sense anymore, so that should be rewritten. I would also suggest reordering that patch before this one, to save yourself some work.
I gave it some thought, I think it makes sense to merge 07/12, 09/12, 10/12 and 11/12 in the same patch: vkd3d-shader/hlsl: Handle additional dimension types for textures.
Hi, On 17/12/21 20:12, Francisco Casas wrote:
@@ -98,7 +98,13 @@ enum hlsl_sampler_dim HLSL_SAMPLER_DIM_2D, HLSL_SAMPLER_DIM_3D, HLSL_SAMPLER_DIM_CUBE, - HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBE + HLSL_SAMPLER_DIM_LAST_SAMPLER = HLSL_SAMPLER_DIM_CUBE, + HLSL_SAMPLER_DIM_1DARRAY, + HLSL_SAMPLER_DIM_2DARRAY, + HLSL_SAMPLER_DIM_2DMS, + HLSL_SAMPLER_DIM_2DMSARRAY, + HLSL_SAMPLER_DIM_CUBEARRAY, + HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBEARRAY, };
I would say that it doesn't make sense any more to call this enumeration hlsl_sampler_dim and its values HLSL_SAMPLER_DIM_*, given that they are used also for things different from the dimension of a sampler (e.g., the dimension of a texture). I would rather go for hlsl_dim and HLSL_DIM_*, or maybe "dimension" and "DIMENSION". At the same time I have a vague feeling that we had already discussed this and it was decided to keep "sampler" and "SAMPLER" and I forgot about it, in which case please ignore this email. Thanks, Giovanni.
On 12/21/21 06:04, Giovanni Mascellani wrote:
Hi,
On 17/12/21 20:12, Francisco Casas wrote:
@@ -98,7 +98,13 @@ enum hlsl_sampler_dim HLSL_SAMPLER_DIM_2D, HLSL_SAMPLER_DIM_3D, HLSL_SAMPLER_DIM_CUBE, - HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBE + HLSL_SAMPLER_DIM_LAST_SAMPLER = HLSL_SAMPLER_DIM_CUBE, + HLSL_SAMPLER_DIM_1DARRAY, + HLSL_SAMPLER_DIM_2DARRAY, + HLSL_SAMPLER_DIM_2DMS, + HLSL_SAMPLER_DIM_2DMSARRAY, + HLSL_SAMPLER_DIM_CUBEARRAY, + HLSL_SAMPLER_DIM_MAX = HLSL_SAMPLER_DIM_CUBEARRAY, };
I would say that it doesn't make sense any more to call this enumeration hlsl_sampler_dim and its values HLSL_SAMPLER_DIM_*, given that they are used also for things different from the dimension of a sampler (e.g., the dimension of a texture). I would rather go for hlsl_dim and HLSL_DIM_*, or maybe "dimension" and "DIMENSION".
At the same time I have a vague feeling that we had already discussed this and it was decided to keep "sampler" and "SAMPLER" and I forgot about it, in which case please ignore this email.
See <https://www.winehq.org/pipermail/wine-devel/2021-December/202738.html>. I don't really have strong feelings, but it can be saved for a later patch if necessary.
On 21/12/21 18:48, Zebediah Figura (she/her) wrote:
See <https://www.winehq.org/pipermail/wine-devel/2021-December/202738.html>. I don't really have strong feelings, but it can be saved for a later patch if necessary.
Ah right. Neither do I have strong feelings. It feels a bit funny and overly verbose, but whatever. Giovanni.
participants (3)
-
Francisco Casas -
Giovanni Mascellani -
Zebediah Figura (she/her)