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