[PATCH v4 0/10] MR106: vkd3d-shader/hlsl: packoffset() support.
Support manually packing constant buffer elements through the `packoffset(·)` syntax. Support not included yet for simultaneously having semantics, `register(·)`, and `packoffset(·)`, for abnormalities such as: ```hlsl Texture2D tex; cbuffer buff { float4 a : packoffset(c0); sampler sam : packoffset(c0) : register(s1) : SEMANTIC; } float4 main() : sv_target { return tex.Sample(sam, float2(0, 0)) + a; } ``` but this motivated the addition of the `hlsl_ir_var.offset_reservation` field instead of reusing `hlsl_ir_var.reg_reservation`. -- v4: vkd3d-shader/hlsl: Consider register() as manual packing for resource fields. tests: Test packoffset() with resources inside cbuffers. vkd3d-shader/hlsl: Ignore packoffset() contents for SM1. vkd3d-shader/hlsl: Don't allow manual and automatic cbuffer offset packing. vkd3d-shader/hlsl: Detect overlaps in cbuffer offsets. vkd3d-shader/hlsl: Support packoffset(). vkd3d-shader/hlsl: Parse packoffset(). vkd3d-shader/hlsl: Rename struct hlsl_reg_reservation fields. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- tests/cbuffer.shader_test | 78 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test index 07ef18db..d60132ea 100644 --- a/tests/cbuffer.shader_test +++ b/tests/cbuffer.shader_test @@ -15,3 +15,81 @@ float4 main() : sv_target uniform 0 float4 1.0 2.0 3.0 4.0 draw quad probe all rgba (1.0, 2.0, 3.0, 4.0) + + +% SM1 buffer offset allocation follows different rules than SM4. +% Those would have to be tested separately. +[require] +shader model >= 4.0 + + +% Respect register boundaries +[pixel shader] +cbuffer buffer +{ + float2 a; + float2 b; + float2 c; + float3 d; +} + +float4 main() : sv_target +{ + return float4(a.x, b.x, c.x, d.x); +} + +[test] +uniform 0 float4 0.0 1.0 2.0 3.0 +uniform 4 float4 4.0 5.0 6.0 7.0 +uniform 8 float4 8.0 9.0 10.0 11.0 +uniform 12 float4 12.0 13.0 14.0 15.0 +draw quad +probe all rgba (0.0, 2.0, 4.0, 8.0) + + +[pixel shader] +cbuffer buffer +{ + float a; + float b[2]; + float c; +} + +float4 main() : sv_target +{ + return float4(a, b[0], b[1], c); +} + +[test] +uniform 0 float4 0.0 1.0 2.0 3.0 +uniform 4 float4 4.0 5.0 6.0 7.0 +uniform 8 float4 8.0 9.0 10.0 11.0 +draw quad +probe all rgba (0.0, 4.0, 8.0, 9.0) + + +[pixel shader] +cbuffer buffer +{ + float a; + struct + { + float b; + float c; + } p; + float d; +} + +float4 main() : sv_target +{ + return float4(a, p.b, p.c, d); +} + +[test] +uniform 0 float4 0.0 1.0 2.0 3.0 +uniform 4 float4 4.0 5.0 6.0 7.0 +uniform 8 float4 8.0 9.0 10.0 11.0 +uniform 12 float4 12.0 13.0 14.0 15.0 +draw quad +probe all rgba (0.0, 4.0, 5.0, 6.0) + -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- tests/cbuffer.shader_test | 339 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 339 insertions(+) diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test index d60132ea..7fa787d9 100644 --- a/tests/cbuffer.shader_test +++ b/tests/cbuffer.shader_test @@ -93,3 +93,342 @@ uniform 12 float4 12.0 13.0 14.0 15.0 draw quad probe all rgba (0.0, 4.0, 5.0, 6.0) + +[pixel shader fail] +// Elements cannot overlap if buffer is used. +cbuffer buffer +{ + float c : packoffset(c0); + float4 a[2] : packoffset(c1); + float4 b[2] : packoffset(c2); +} + +float4 main() : sv_target +{ + return c; +} + + +[pixel shader todo] +// Elements can overlap if buffer is not used. +cbuffer buffer +{ + float4 a[2] : packoffset(c1); + float4 b[2] : packoffset(c2); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader todo] +cbuffer buffer +{ + float4 a : packoffset(c1); + float4 b : packoffset(c2); +} + +float4 main() : sv_target +{ + return 100 * a + b; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +uniform 4 float4 5.0 6.0 7.0 8.0 +uniform 8 float4 9.0 10.0 11.0 12.0 +todo draw quad +todo probe all rgba (509, 610, 711, 812) + + +[pixel shader todo] +struct apple +{ + float2 a; + float b; + float4 c; +}; + +cbuffer buffer +{ + float4 foo : packoffset(c3); + struct apple bar : packoffset(c1); +} + +float4 main() : sv_target +{ + return 1000 * foo + 100 * float4(bar.a, 0, 0) + 10 * float4(bar.b, 0, 0, 0) + bar.c; +} + +[test] +uniform 0 float4 0.0 1.0 2.0 3.0 +uniform 4 float4 4.0 5.0 6.0 7.0 +uniform 8 float4 8.0 9.0 10.0 11.0 +uniform 12 float4 12.0 13.0 14.0 15.0 +todo draw quad +todo probe all rgba (12468.0, 13509.0, 14010.0, 15011.0) + + +[pixel shader todo] +cbuffer buffer +{ + float2 c : packoffset(c0.y); +} + +float4 main() : sv_target +{ + return float4(c, c); +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +todo draw quad +todo probe all rgba (2.0, 3.0, 2.0, 3.0) + + +[pixel shader fail] +// Elements must respect register boundaries. +cbuffer buffer +{ + float4 c : packoffset(c0.z); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +// Matrices must be aligned. +cbuffer buffer +{ + float1x1 m : packoffset(c0.y); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +// Arrays must be aligned. +cbuffer buffer +{ + float a[1] : packoffset(c0.y); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +// Structs must be aligned. +struct apple +{ + float p; +}; + +cbuffer buffer +{ + struct apple a : packoffset(c0.y); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +// Invalid offset on unused buffer. +cbuffer buffer +{ + float3 a : packoffset(c0.z); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +// Invalid offset on unused variable. +cbuffer buffer +{ + float a : packoffset(c0); + float3 b : packoffset(c0.z); +} + +float4 main() : sv_target +{ + return a; +} + + +[pixel shader todo] +cbuffer buffer +{ + float4 a : packoffset(c1); + float1 b : packoffset(c0); + float1 c : packoffset(c0.y); +} + +float4 main() : sv_target +{ + return 100 * a + 10 * b + c; +} + +[test] +uniform 0 float 1.0 +uniform 1 float 2.0 +uniform 4 float4 5.0 6.0 7.0 8.0 +todo draw quad +todo probe all rgba (512.0, 612.0, 712.0, 812.0) + + +[pixel shader fail] +// packoffset cannot be used unless all elements use it. +cbuffer buffer +{ + float4 a : packoffset(c0); + float4 b; +} + +float4 main() : sv_target +{ + return a + b; +} + + +[pixel shader todo] +cbuffer buffer +{ + float2 c : packoffset(c0.b); +} + +float4 main() : sv_target +{ + return float4(c, c); +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +todo draw quad +todo probe all rgba (3.0, 4.0, 3.0, 4.0) + + +[pixel shader fail] +cbuffer buffer +{ + float2 c : packoffset(c0.xy); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +cbuffer buffer +{ + float4x4 mat : packoffset(c0._m00); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +cbuffer buffer +{ + float4 a : packoffset(c0._m00); +} + +float4 main() : sv_target +{ + return 0; +} + + +[pixel shader fail] +cbuffer buffer +{ + float2 c : packoffset(c0.xz); +} + +float4 main() : sv_target +{ + return 0; +} + + +% packoffset cannot be used outside a constant buffer. +[pixel shader fail] +float4 a : packoffset(c0); + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +float4 foo(float4 a : packoffset(c0)) +{ + return a; +} + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +float4 foo(float4 a) : packoffset(c0) +{ + return a; +} + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +struct apple +{ + float4 a : packoffset(c0); +}; + +cbuffer buffer +{ + struct apple a; +} + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +cbuffer buffer +{ + struct + { + float4 a : packoffset(c0); + } s; +} + +float4 main() : sv_target +{ + return 0; +} -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 4 ++-- libs/vkd3d-shader/hlsl.y | 10 +++++----- libs/vkd3d-shader/hlsl_codegen.c | 25 +++++++++++++------------ libs/vkd3d-shader/hlsl_sm4.c | 4 ++-- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 6fa627ed..38f324a8 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -350,8 +350,8 @@ struct hlsl_attribute * using the register(·) syntax */ struct hlsl_reg_reservation { - char type; - unsigned int index; + char reg_type; + unsigned int reg_index; }; struct hlsl_ir_var diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 6c618912..29a68f4f 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1100,12 +1100,12 @@ static struct hlsl_reg_reservation parse_reg_reservation(const char *reg_string) { struct hlsl_reg_reservation reservation = {0}; - if (!sscanf(reg_string + 1, "%u", &reservation.index)) + if (!sscanf(reg_string + 1, "%u", &reservation.reg_index)) { FIXME("Unsupported register reservation syntax.\n"); return reservation; } - reservation.type = reg_string[0]; + reservation.reg_type = reg_string[0]; return reservation; } @@ -4314,7 +4314,7 @@ func_prototype_no_attrs: "Semantics are not allowed on void functions."); } - if ($7.reg_reservation.type) + if ($7.reg_reservation.reg_type) FIXME("Unexpected register reservation for a function.\n"); if (($$.decl = get_func_decl(&ctx->functions, $3, &$5))) @@ -4442,12 +4442,12 @@ colon_attribute: %empty { $$.semantic.name = NULL; - $$.reg_reservation.type = 0; + $$.reg_reservation.reg_type = 0; } | semantic { $$.semantic = $1; - $$.reg_reservation.type = 0; + $$.reg_reservation.reg_type = 0; } | register_opt { diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 65bd9d4c..5c0e3659 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2262,9 +2262,9 @@ static void allocate_register_reservations(struct hlsl_ctx *ctx) continue; regset = hlsl_type_get_regset(var->data_type); - if (var->reg_reservation.type) + if (var->reg_reservation.reg_type) { - if (var->reg_reservation.type != get_regset_name(regset)) + if (var->reg_reservation.reg_type != get_regset_name(regset)) { struct vkd3d_string_buffer *type_string; @@ -2277,8 +2277,9 @@ static void allocate_register_reservations(struct hlsl_ctx *ctx) else { var->regs[regset].allocated = true; - var->regs[regset].id = var->reg_reservation.index; - TRACE("Allocated reserved %s to %c%u.\n", var->name, var->reg_reservation.type, var->reg_reservation.index); + var->regs[regset].id = var->reg_reservation.reg_index; + TRACE("Allocated reserved %s to %c%u.\n", var->name, var->reg_reservation.reg_type, + var->reg_reservation.reg_index); } } } @@ -2853,7 +2854,7 @@ static const struct hlsl_buffer *get_reserved_buffer(struct hlsl_ctx *ctx, uint3 LIST_FOR_EACH_ENTRY(buffer, &ctx->buffers, const struct hlsl_buffer, entry) { - if (buffer->used_size && buffer->reservation.type == 'b' && buffer->reservation.index == index) + if (buffer->used_size && buffer->reservation.reg_type == 'b' && buffer->reservation.reg_index == index) return buffer; } return NULL; @@ -2896,23 +2897,23 @@ static void allocate_buffers(struct hlsl_ctx *ctx) if (buffer->type == HLSL_BUFFER_CONSTANT) { - if (buffer->reservation.type == 'b') + if (buffer->reservation.reg_type == 'b') { - const struct hlsl_buffer *reserved_buffer = get_reserved_buffer(ctx, buffer->reservation.index); + const struct hlsl_buffer *reserved_buffer = get_reserved_buffer(ctx, buffer->reservation.reg_index); if (reserved_buffer && reserved_buffer != buffer) { hlsl_error(ctx, &buffer->loc, VKD3D_SHADER_ERROR_HLSL_OVERLAPPING_RESERVATIONS, - "Multiple buffers bound to cb%u.", buffer->reservation.index); + "Multiple buffers bound to cb%u.", buffer->reservation.reg_index); hlsl_note(ctx, &reserved_buffer->loc, VKD3D_SHADER_LOG_ERROR, - "Buffer %s is already bound to cb%u.", reserved_buffer->name, buffer->reservation.index); + "Buffer %s is already bound to cb%u.", reserved_buffer->name, buffer->reservation.reg_index); } - buffer->reg.id = buffer->reservation.index; + buffer->reg.id = buffer->reservation.reg_index; buffer->reg.allocated = true; TRACE("Allocated reserved %s to cb%u.\n", buffer->name, index); } - else if (!buffer->reservation.type) + else if (!buffer->reservation.reg_type) { while (get_reserved_buffer(ctx, index)) ++index; @@ -2998,7 +2999,7 @@ static void allocate_objects(struct hlsl_ctx *ctx, enum hlsl_regset regset) regset_name, index); } - var->regs[regset].id = var->reg_reservation.index; + var->regs[regset].id = var->reg_reservation.reg_index; var->regs[regset].allocated = true; TRACE("Allocated reserved %s to %c%u.\n", var->name, regset_name, var->regs[regset].id); } diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index b95aab4e..7559ca99 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -594,7 +594,7 @@ static void write_sm4_rdef(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc) var = extern_resources[i]; regset = hlsl_type_get_regset(var->data_type); - if (var->reg_reservation.type) + if (var->reg_reservation.reg_type) flags |= D3D_SIF_USERPACKED; put_u32(&buffer, 0); /* name */ @@ -624,7 +624,7 @@ static void write_sm4_rdef(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc) if (!cbuffer->reg.allocated) continue; - if (cbuffer->reservation.type) + if (cbuffer->reservation.reg_type) flags |= D3D_SIF_USERPACKED; put_u32(&buffer, 0); /* name */ -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 12 ++++-- libs/vkd3d-shader/hlsl.l | 1 + libs/vkd3d-shader/hlsl.y | 90 ++++++++++++++++++++++++++++++++++++++- tests/cbuffer.shader_test | 50 +++++++++++++--------- 4 files changed, 129 insertions(+), 24 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 38f324a8..5fb05010 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -346,12 +346,17 @@ struct hlsl_attribute #define HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT 0 -/* Reservation of a specific register to a variable, field, or buffer, written in the HLSL source - * using the register(·) syntax */ +/* Reservation of a register and/or an offset for objects inside constant buffers, to be used as a + * starting point of their allocation. They are available through the register(·) and the + * packoffset(·) syntaxes, respectivelly. + * The costant buffer offset is measured register components. */ struct hlsl_reg_reservation { char reg_type; unsigned int reg_index; + + char offset_type; + unsigned int offset_index; }; struct hlsl_ir_var @@ -364,8 +369,7 @@ struct hlsl_ir_var struct hlsl_buffer *buffer; /* Bitfield for storage modifiers (type modifiers are stored in data_type->modifiers). */ unsigned int storage_modifiers; - /* Optional register to be used as a starting point for the variable allocation, specified - * by the user via the register(·) syntax. */ + /* Optional reservations of registers and/or offsets for variables within constant buffers. */ struct hlsl_reg_reservation reg_reservation; /* Item entry in hlsl_scope.vars. Specifically hlsl_ctx.globals.vars if the variable is global. */ diff --git a/libs/vkd3d-shader/hlsl.l b/libs/vkd3d-shader/hlsl.l index 69ed9d9d..d27fd803 100644 --- a/libs/vkd3d-shader/hlsl.l +++ b/libs/vkd3d-shader/hlsl.l @@ -95,6 +95,7 @@ matrix {return KW_MATRIX; } namespace {return KW_NAMESPACE; } nointerpolation {return KW_NOINTERPOLATION; } out {return KW_OUT; } +packoffset {return KW_PACKOFFSET; } pass {return KW_PASS; } PixelShader {return KW_PIXELSHADER; } precise {return KW_PRECISE; } diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 29a68f4f..dc9a15d7 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -997,6 +997,9 @@ static bool gen_struct_fields(struct hlsl_ctx *ctx, struct parse_fields *fields, hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SYNTAX, "Illegal initializer on a struct field."); free_parse_initializer(&v->initializer); } + if (v->reg_reservation.offset_type) + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() is not allowed inside struct definitions."); vkd3d_free(v); } vkd3d_free(defs); @@ -1079,7 +1082,12 @@ static bool add_func_parameter(struct hlsl_ctx *ctx, struct hlsl_func_parameters hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, "Parameter '%s' is declared as both \"out\" and \"uniform\".", param->name); - if (!(var = hlsl_new_var(ctx, param->name, param->type, loc, ¶m->semantic, param->modifiers, ¶m->reg_reservation))) + if (param->reg_reservation.offset_type) + hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() is not allowed on function parameters."); + + if (!(var = hlsl_new_var(ctx, param->name, param->type, loc, ¶m->semantic, param->modifiers, + ¶m->reg_reservation))) return false; var->is_param = 1; @@ -1109,6 +1117,52 @@ static struct hlsl_reg_reservation parse_reg_reservation(const char *reg_string) return reservation; } +static struct hlsl_reg_reservation parse_packoffset(struct hlsl_ctx *ctx, const char *reg_string, + const char *swizzle, const struct vkd3d_shader_location *loc) +{ + struct hlsl_reg_reservation reservation = {0}; + char *endptr; + + reservation.offset_index = strtoul(reg_string + 1, &endptr, 10); + if (*endptr) + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid packoffset() syntax."); + return reservation; + } + + reservation.offset_type = reg_string[0]; + if (reservation.offset_type != 'c') + { + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Only 'c' registers are allowed in packoffset()."); + return reservation; + } + + reservation.offset_index *= 4; + + if (swizzle) + { + if (strlen(swizzle) != 1) + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid packoffset() component \"%s\".", swizzle); + + if (swizzle[0] == 'x' || swizzle[0] == 'r') + reservation.offset_index += 0; + else if (swizzle[0] == 'y' || swizzle[0] == 'g') + reservation.offset_index += 1; + else if (swizzle[0] == 'z' || swizzle[0] == 'b') + reservation.offset_index += 2; + else if (swizzle[0] == 'w' || swizzle[0] == 'a') + reservation.offset_index += 3; + else + hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid packoffset() component \"%s\".", swizzle); + } + + return reservation; +} + static struct hlsl_ir_function_decl *get_func_decl(struct rb_tree *funcs, const char *name, const struct hlsl_func_parameters *parameters) { @@ -2059,6 +2113,13 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t var->buffer = ctx->cur_buffer; + if (var->buffer == ctx->globals_buffer) + { + if (var->reg_reservation.offset_type) + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() is only allowed inside constant buffer declarations."); + } + if (ctx->cur_scope == ctx->globals) { local = false; @@ -3811,6 +3872,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl %token KW_NAMESPACE %token KW_NOINTERPOLATION %token KW_OUT +%token KW_PACKOFFSET %token KW_PASS %token KW_PIXELSHADER %token KW_PRECISE @@ -3964,6 +4026,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl %type <parameters> parameters %type <reg_reservation> register_opt +%type <reg_reservation> packoffset_opt %type <sampler_dim> texture_type uav_type @@ -4316,6 +4379,9 @@ func_prototype_no_attrs: if ($7.reg_reservation.reg_type) FIXME("Unexpected register reservation for a function.\n"); + if ($7.reg_reservation.offset_type) + hlsl_error(ctx, &@5, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() is not allowed on functions."); if (($$.decl = get_func_decl(&ctx->functions, $3, &$5))) { @@ -4443,17 +4509,24 @@ colon_attribute: { $$.semantic.name = NULL; $$.reg_reservation.reg_type = 0; + $$.reg_reservation.offset_type = 0; } | semantic { $$.semantic = $1; $$.reg_reservation.reg_type = 0; + $$.reg_reservation.offset_type = 0; } | register_opt { $$.semantic.name = NULL; $$.reg_reservation = $1; } + | packoffset_opt + { + $$.semantic.name = NULL; + $$.reg_reservation = $1; + } semantic: ':' any_identifier @@ -4483,6 +4556,21 @@ register_opt: vkd3d_free($6); } +packoffset_opt: + ':' KW_PACKOFFSET '(' any_identifier ')' + { + $$ = parse_packoffset(ctx, $4, NULL, &@$); + + vkd3d_free($4); + } + | ':' KW_PACKOFFSET '(' any_identifier '.' any_identifier ')' + { + $$ = parse_packoffset(ctx, $4, $6, &@$); + + vkd3d_free($4); + vkd3d_free($6); + } + parameters: scope_start { diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test index 7fa787d9..628a441a 100644 --- a/tests/cbuffer.shader_test +++ b/tests/cbuffer.shader_test @@ -23,6 +23,18 @@ probe all rgba (1.0, 2.0, 3.0, 4.0) shader model >= 4.0 +[pixel shader fail] +cbuffer buffer +{ + float4 a : packoffset(c1invalid_extra_chars); +} + +float4 main() : sv_target +{ + return 0; +} + + % Respect register boundaries [pixel shader] cbuffer buffer @@ -94,7 +106,7 @@ draw quad probe all rgba (0.0, 4.0, 5.0, 6.0) -[pixel shader fail] +[pixel shader fail todo] // Elements cannot overlap if buffer is used. cbuffer buffer { @@ -109,7 +121,7 @@ float4 main() : sv_target } -[pixel shader todo] +[pixel shader] // Elements can overlap if buffer is not used. cbuffer buffer { @@ -123,7 +135,7 @@ float4 main() : sv_target } -[pixel shader todo] +[pixel shader] cbuffer buffer { float4 a : packoffset(c1); @@ -139,11 +151,11 @@ float4 main() : sv_target uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float4 5.0 6.0 7.0 8.0 uniform 8 float4 9.0 10.0 11.0 12.0 -todo draw quad +draw quad todo probe all rgba (509, 610, 711, 812) -[pixel shader todo] +[pixel shader] struct apple { float2 a; @@ -167,11 +179,11 @@ uniform 0 float4 0.0 1.0 2.0 3.0 uniform 4 float4 4.0 5.0 6.0 7.0 uniform 8 float4 8.0 9.0 10.0 11.0 uniform 12 float4 12.0 13.0 14.0 15.0 -todo draw quad +draw quad todo probe all rgba (12468.0, 13509.0, 14010.0, 15011.0) -[pixel shader todo] +[pixel shader] cbuffer buffer { float2 c : packoffset(c0.y); @@ -184,11 +196,11 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 -todo draw quad +draw quad todo probe all rgba (2.0, 3.0, 2.0, 3.0) -[pixel shader fail] +[pixel shader fail todo] // Elements must respect register boundaries. cbuffer buffer { @@ -201,7 +213,7 @@ float4 main() : sv_target } -[pixel shader fail] +[pixel shader fail todo] // Matrices must be aligned. cbuffer buffer { @@ -214,7 +226,7 @@ float4 main() : sv_target } -[pixel shader fail] +[pixel shader fail todo] // Arrays must be aligned. cbuffer buffer { @@ -227,7 +239,7 @@ float4 main() : sv_target } -[pixel shader fail] +[pixel shader fail todo] // Structs must be aligned. struct apple { @@ -245,7 +257,7 @@ float4 main() : sv_target } -[pixel shader fail] +[pixel shader fail todo] // Invalid offset on unused buffer. cbuffer buffer { @@ -258,7 +270,7 @@ float4 main() : sv_target } -[pixel shader fail] +[pixel shader fail todo] // Invalid offset on unused variable. cbuffer buffer { @@ -272,7 +284,7 @@ float4 main() : sv_target } -[pixel shader todo] +[pixel shader] cbuffer buffer { float4 a : packoffset(c1); @@ -289,11 +301,11 @@ float4 main() : sv_target uniform 0 float 1.0 uniform 1 float 2.0 uniform 4 float4 5.0 6.0 7.0 8.0 -todo draw quad +draw quad todo probe all rgba (512.0, 612.0, 712.0, 812.0) -[pixel shader fail] +[pixel shader fail todo] // packoffset cannot be used unless all elements use it. cbuffer buffer { @@ -307,7 +319,7 @@ float4 main() : sv_target } -[pixel shader todo] +[pixel shader] cbuffer buffer { float2 c : packoffset(c0.b); @@ -320,7 +332,7 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 -todo draw quad +draw quad todo probe all rgba (3.0, 4.0, 3.0, 4.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 46 +++++++++++++++++++++++++++----- tests/cbuffer.shader_test | 22 +++++++-------- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 5c0e3659..959a690f 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2860,17 +2860,51 @@ static const struct hlsl_buffer *get_reserved_buffer(struct hlsl_ctx *ctx, uint3 return NULL; } -static void calculate_buffer_offset(struct hlsl_ir_var *var) +static void calculate_buffer_offset(struct hlsl_ctx *ctx, struct hlsl_ir_var *var) { + unsigned int var_reg_size = var->data_type->reg_size[HLSL_REGSET_NUMERIC]; + enum hlsl_type_class var_type = var->data_type->type; struct hlsl_buffer *buffer = var->buffer; - buffer->size = hlsl_type_get_sm4_offset(var->data_type, buffer->size); + if (var->reg_reservation.offset_type == 'c') + { + if (var->reg_reservation.offset_index % 4) + { + if (var_type == HLSL_CLASS_MATRIX) + { + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() reservations with matrix types must be aligned with the beginning of a register."); + } + else if (var_type == HLSL_CLASS_ARRAY) + { + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() reservations with array types must be aligned with the beginning of a register."); + } + else if (var_type == HLSL_CLASS_STRUCT) + { + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() reservations with struct types must be aligned with the beginning of a register."); + } + else if (var_type == HLSL_CLASS_VECTOR) + { + unsigned int aligned_offset = hlsl_type_get_sm4_offset(var->data_type, var->reg_reservation.offset_index); + + if (var->reg_reservation.offset_index != aligned_offset) + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() reservations with vector types cannot span multiple registers."); + } + } + var->buffer_offset = var->reg_reservation.offset_index; + } + else + { + var->buffer_offset = hlsl_type_get_sm4_offset(var->data_type, buffer->size); + } - var->buffer_offset = buffer->size; TRACE("Allocated buffer offset %u to %s.\n", var->buffer_offset, var->name); - buffer->size += var->data_type->reg_size[HLSL_REGSET_NUMERIC]; + buffer->size = max(buffer->size, var->buffer_offset + var_reg_size); if (var->last_read) - buffer->used_size = buffer->size; + buffer->used_size = max(buffer->used_size, var->buffer_offset + var_reg_size); } static void allocate_buffers(struct hlsl_ctx *ctx) @@ -2886,7 +2920,7 @@ static void allocate_buffers(struct hlsl_ctx *ctx) if (var->is_param) var->buffer = ctx->params_buffer; - calculate_buffer_offset(var); + calculate_buffer_offset(ctx, var); } } diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test index 628a441a..bf711629 100644 --- a/tests/cbuffer.shader_test +++ b/tests/cbuffer.shader_test @@ -152,7 +152,7 @@ uniform 0 float4 1.0 2.0 3.0 4.0 uniform 4 float4 5.0 6.0 7.0 8.0 uniform 8 float4 9.0 10.0 11.0 12.0 draw quad -todo probe all rgba (509, 610, 711, 812) +probe all rgba (509, 610, 711, 812) [pixel shader] @@ -180,7 +180,7 @@ uniform 4 float4 4.0 5.0 6.0 7.0 uniform 8 float4 8.0 9.0 10.0 11.0 uniform 12 float4 12.0 13.0 14.0 15.0 draw quad -todo probe all rgba (12468.0, 13509.0, 14010.0, 15011.0) +probe all rgba (12468.0, 13509.0, 14010.0, 15011.0) [pixel shader] @@ -197,10 +197,10 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 draw quad -todo probe all rgba (2.0, 3.0, 2.0, 3.0) +probe all rgba (2.0, 3.0, 2.0, 3.0) -[pixel shader fail todo] +[pixel shader fail] // Elements must respect register boundaries. cbuffer buffer { @@ -213,7 +213,7 @@ float4 main() : sv_target } -[pixel shader fail todo] +[pixel shader fail] // Matrices must be aligned. cbuffer buffer { @@ -226,7 +226,7 @@ float4 main() : sv_target } -[pixel shader fail todo] +[pixel shader fail] // Arrays must be aligned. cbuffer buffer { @@ -239,7 +239,7 @@ float4 main() : sv_target } -[pixel shader fail todo] +[pixel shader fail] // Structs must be aligned. struct apple { @@ -257,7 +257,7 @@ float4 main() : sv_target } -[pixel shader fail todo] +[pixel shader fail] // Invalid offset on unused buffer. cbuffer buffer { @@ -270,7 +270,7 @@ float4 main() : sv_target } -[pixel shader fail todo] +[pixel shader fail] // Invalid offset on unused variable. cbuffer buffer { @@ -302,7 +302,7 @@ uniform 0 float 1.0 uniform 1 float 2.0 uniform 4 float4 5.0 6.0 7.0 8.0 draw quad -todo probe all rgba (512.0, 612.0, 712.0, 812.0) +probe all rgba (512.0, 612.0, 712.0, 812.0) [pixel shader fail todo] @@ -333,7 +333,7 @@ float4 main() : sv_target [test] uniform 0 float4 1.0 2.0 3.0 4.0 draw quad -todo probe all rgba (3.0, 4.0, 3.0, 4.0) +probe all rgba (3.0, 4.0, 3.0, 4.0) [pixel shader fail] -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 42 ++++++++++++++++++++++++++++++++ tests/cbuffer.shader_test | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 959a690f..c36b1ab6 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2907,6 +2907,46 @@ static void calculate_buffer_offset(struct hlsl_ctx *ctx, struct hlsl_ir_var *va buffer->used_size = max(buffer->used_size, var->buffer_offset + var_reg_size); } +static void validate_buffer_offsets(struct hlsl_ctx *ctx) +{ + struct hlsl_ir_var *var1, *var2; + struct hlsl_buffer *buffer; + + LIST_FOR_EACH_ENTRY(var1, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) + { + if (!var1->is_uniform || var1->data_type->type == HLSL_CLASS_OBJECT) + continue; + + buffer = var1->buffer; + if (!buffer->used_size) + continue; + + LIST_FOR_EACH_ENTRY(var2, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) + { + unsigned int var1_reg_size, var2_reg_size; + + if (!var2->is_uniform || var2->data_type->type == HLSL_CLASS_OBJECT) + continue; + + if (var1 == var2 || var1->buffer != var2->buffer) + continue; + + /* This is to avoid reporting the error twice for the same pair of overlapping variables. */ + if (strcmp(var1->name, var2->name) >= 0) + continue; + + var1_reg_size = var1->data_type->reg_size[HLSL_REGSET_NUMERIC]; + var2_reg_size = var2->data_type->reg_size[HLSL_REGSET_NUMERIC]; + + if (var1->buffer_offset < var2->buffer_offset + var2_reg_size + && var2->buffer_offset < var1->buffer_offset + var1_reg_size) + hlsl_error(ctx, &buffer->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid packoffset() reservation: Variables %s and %s overlap.", + var1->name, var2->name); + } + } +} + static void allocate_buffers(struct hlsl_ctx *ctx) { struct hlsl_buffer *buffer; @@ -2924,6 +2964,8 @@ static void allocate_buffers(struct hlsl_ctx *ctx) } } + validate_buffer_offsets(ctx); + LIST_FOR_EACH_ENTRY(buffer, &ctx->buffers, struct hlsl_buffer, entry) { if (!buffer->used_size) diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test index bf711629..d8cfc351 100644 --- a/tests/cbuffer.shader_test +++ b/tests/cbuffer.shader_test @@ -106,7 +106,7 @@ draw quad probe all rgba (0.0, 4.0, 5.0, 6.0) -[pixel shader fail todo] +[pixel shader fail] // Elements cannot overlap if buffer is used. cbuffer buffer { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 3 +++ libs/vkd3d-shader/hlsl_codegen.c | 19 +++++++++++++++++++ tests/cbuffer.shader_test | 2 +- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 5fb05010..a0fea3be 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -682,6 +682,9 @@ struct hlsl_buffer unsigned size, used_size; /* Register of type 'b' on which the buffer is allocated. */ struct hlsl_reg reg; + + bool manually_packed_elements; + bool automatically_packed_elements; }; struct hlsl_ctx diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index c36b1ab6..7224d938 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2945,6 +2945,25 @@ static void validate_buffer_offsets(struct hlsl_ctx *ctx) var1->name, var2->name); } } + + LIST_FOR_EACH_ENTRY(var1, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) + { + buffer = var1->buffer; + if (!buffer) + continue; + + if (var1->reg_reservation.offset_type == 'c') + buffer->manually_packed_elements = true; + else + buffer->automatically_packed_elements = true; + + if (buffer->manually_packed_elements && buffer->automatically_packed_elements) + { + hlsl_error(ctx, &buffer->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "packoffset() must be specified for all the buffer elements, or none of them."); + break; + } + } } static void allocate_buffers(struct hlsl_ctx *ctx) diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test index d8cfc351..8c6e1724 100644 --- a/tests/cbuffer.shader_test +++ b/tests/cbuffer.shader_test @@ -305,7 +305,7 @@ draw quad probe all rgba (512.0, 612.0, 712.0, 812.0) -[pixel shader fail todo] +[pixel shader fail] // packoffset cannot be used unless all elements use it. cbuffer buffer { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index dc9a15d7..63fa8e80 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1123,6 +1123,9 @@ static struct hlsl_reg_reservation parse_packoffset(struct hlsl_ctx *ctx, const struct hlsl_reg_reservation reservation = {0}; char *endptr; + if (ctx->profile->major_version < 4) + return reservation; + reservation.offset_index = strtoul(reg_string + 1, &endptr, 10); if (*endptr) { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- tests/cbuffer.shader_test | 259 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 259 insertions(+) diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test index 8c6e1724..e95a5235 100644 --- a/tests/cbuffer.shader_test +++ b/tests/cbuffer.shader_test @@ -444,3 +444,262 @@ float4 main() : sv_target { return 0; } + + +[texture 0] +size (1, 1) +0.0 0.0 0.0 4.0 + +[sampler 0] +filter linear linear linear +address clamp clamp clamp + + +[pixel shader] +// Resources are allowed inside constant buffers but they behave as regular resources. +cbuffer buffer +{ + float4 a; + Texture2D tex; + sampler sam; + float4 b; +} + +float4 main() : sv_target +{ + return a + b + tex.Sample(sam, float2(0, 0)); +} + +[test] +uniform 0 float4 1.0 0.0 0.0 0.0 +uniform 4 float4 0.0 2.0 0.0 0.0 +uniform 8 float4 0.0 0.0 3.0 0.0 +draw quad +probe all rgba (1.0, 2.0, 0.0, 4.0) + + +% packoffset() cannot be used to specify other types of registers +[pixel shader fail] +cbuffer buffer +{ + sampler sam : packoffset(s0); +} + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +cbuffer buffer +{ + Texture2D tex : packoffset(t0); +} + +float4 main() : sv_target +{ + return 0; +} + + +[texture 0] +size (1, 1) +1.0 1.0 1.0 1.0 + +[texture 1] +size (1, 1) +2.0 2.0 2.0 2.0 + + +[pixel shader] +// packoffset() can be used in Textures, doesn't change the allocated t register. +cbuffer buffer +{ + Texture2D tex : packoffset(c1); +} + +float4 main() : sv_target +{ + return tex.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) + + +% Samplers cannot have packoffset(), unless register() is also specified, or they are not used. +% Note: In SM1 the rules are different: packoffset() is allowed for samplers, but they cannot be +% used together with other numeric fields, which seems like a bug. +[pixel shader fail todo] +Texture2D tex; + +cbuffer buffer +{ + sampler sam : packoffset(c1); +} + +float4 main() : sv_target +{ + return tex.Sample(sam, float2(0, 0)); +} + +[pixel shader todo] +Texture2D tex; + +cbuffer buffer +{ + sampler sam : packoffset(c1) : register(s0); +} + +float4 main() : sv_target +{ + return tex.Sample(sam, float2(0, 0)); +} + +[pixel shader] +cbuffer buffer +{ + sampler sam : packoffset(c1); +} + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader] +cbuffer buffer +{ + sampler sam : packoffset(c1); + float4 a : packoffset(c0); +} + +float4 main() : sv_target +{ + return a; +} + + +% When packoffset is used in one field, resources are also expected to have a reservation. +[pixel shader fail] +cbuffer buffer +{ + float4 foo : packoffset(c0); + Texture2D tex; +} + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +cbuffer buffer +{ + float4 foo : packoffset(c0); + sampler sam; +} + +float4 main() : sv_target +{ + return 0; +} + +% register() can be used instead of packoffset(). +[pixel shader todo] +cbuffer buffer +{ + float4 foo : packoffset(c0); + Texture2D tex : register(t1); +} + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader todo] +cbuffer buffer +{ + float4 foo : packoffset(c0); + sampler sam : register(s1); +} + +float4 main() : sv_target +{ + return 0; +} + +% Using register() alone is considered manual packing for resources, so the other fields expect packoffset(). +[pixel shader fail todo] +cbuffer buffer +{ + float4 foo; + Texture2D tex : register(t1); +} + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +cbuffer buffer +{ + float4 foo; + sampler sam : register(s1); +} + +float4 main() : sv_target +{ + return 0; +} + +% Using register() alone is not considered manual packing for non-resources. +[pixel shader] +cbuffer buffer +{ + float4 foo : register(c0); + Texture2D tex; +} + +float4 main() : sv_target +{ + return 0; +} + + +[require] +shader model >= 5.0 + +[texture 0] +size (1, 1) +0.0 0.0 0.0 0.5 + +[pixel shader todo] +struct apple +{ + float2 a; + Texture2D tex; + float b; +}; + +cbuffer buffer +{ + float4 foo : packoffset(c3); + struct apple bar : packoffset(c1); +} + +float4 main() : sv_target +{ + return 10 * foo + float4(bar.a, 0, 0) + float4(0, 0, bar.b, 0) + bar.tex.Load(int3(0, 0, 0)); +} + +[test] +uniform 0 float4 0.0 1.0 2.0 3.0 +uniform 4 float4 4.0 5.0 6.0 7.0 +uniform 8 float4 8.0 9.0 10.0 11.0 +uniform 12 float4 12.0 13.0 14.0 15.0 +todo draw quad +todo probe all rgba (124.0, 135.0, 146.0, 150.5) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
From: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 5 +++-- tests/cbuffer.shader_test | 8 ++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 7224d938..a544296a 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -2949,10 +2949,11 @@ static void validate_buffer_offsets(struct hlsl_ctx *ctx) LIST_FOR_EACH_ENTRY(var1, &ctx->extern_vars, struct hlsl_ir_var, extern_entry) { buffer = var1->buffer; - if (!buffer) + if (!buffer || buffer == ctx->globals_buffer) continue; - if (var1->reg_reservation.offset_type == 'c') + if (var1->reg_reservation.offset_type + || (var1->data_type->type == HLSL_CLASS_OBJECT && var1->reg_reservation.reg_type)) buffer->manually_packed_elements = true; else buffer->automatically_packed_elements = true; diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test index e95a5235..9cf42d9d 100644 --- a/tests/cbuffer.shader_test +++ b/tests/cbuffer.shader_test @@ -607,7 +607,7 @@ float4 main() : sv_target } % register() can be used instead of packoffset(). -[pixel shader todo] +[pixel shader] cbuffer buffer { float4 foo : packoffset(c0); @@ -619,7 +619,7 @@ float4 main() : sv_target return 0; } -[pixel shader todo] +[pixel shader] cbuffer buffer { float4 foo : packoffset(c0); @@ -632,7 +632,7 @@ float4 main() : sv_target } % Using register() alone is considered manual packing for resources, so the other fields expect packoffset(). -[pixel shader fail todo] +[pixel shader fail] cbuffer buffer { float4 foo; @@ -644,7 +644,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] cbuffer buffer { float4 foo; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
On Tue Mar 7 00:48:47 2023 +0000, Francisco Casas wrote:
changed this line in [version 4 of the diff](/wine/vkd3d/-/merge_requests/106/diffs?diff_id=35799&start_sha=adc4e1f16635dc46a880109f05af46ae771557a2#c6c9a40d72060550079c91bb6dd5741748169928_852_851) Well, perhaps that alone is not a big thing as I thought in the first place.
I merged both things in the same struct, I pushed the result for your inspection. There are benefits: * Removing the `offset_reservation` fields from the parse symbols structs (`struct parse_parameter`, `struct parse_colon_attribute`, and `struct parse_variable_def`). * Removing the additional argument from `hlsl_new_var()`. There are downsides: * An extra refactoring patch for renaming the current fields in `struct hlsl_reg_reservation` was required. * In the parser, both the `register_opt` and the `packoffset_opt` symbols have to be `struct hlsl_reg_reservation`, so they both have to left the fields that don't correspond to them as zero. * `struct hlsl_buffer` can have a register reservation but not an offset reservation, which makes its `reservation` field a little unintuitive. * It is tempting to rename `reg_reservation` to just `reservation` in `struct hlsl_ir_var` and the parse symbols structs now, since they no longer contain just the reservation of the register, and `reg_reservation.offset_index` sounds a little odd. That would be a lot of refactoring though. If you don't like how this changed ended up, I can revert it. I made a backup branch. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106#note_26278
On Tue Mar 7 00:49:58 2023 +0000, Francisco Casas wrote:
Well, perhaps that alone is not a big thing as I thought in the first place. I merged both things in the same struct, I pushed the result for your inspection. There are benefits: * Removing the `offset_reservation` fields from the parse symbols structs (`struct parse_parameter`, `struct parse_colon_attribute`, and `struct parse_variable_def`). * Removing the additional argument from `hlsl_new_var()`. There are downsides: * An extra refactoring patch for renaming the current fields in `struct hlsl_reg_reservation` was required. * In the parser, both the `register_opt` and the `packoffset_opt` symbols have to be `struct hlsl_reg_reservation`, so they both have to left the fields that don't correspond to them as zero. * `struct hlsl_buffer` can have a register reservation but not an offset reservation, which makes its `reservation` field a little unintuitive. * It is tempting to rename `reg_reservation` to just `reservation` in `struct hlsl_ir_var` and the parse symbols structs now, since they no longer contain just the reservation of the register, and `reg_reservation.offset_index` sounds a little odd. That would be a lot of refactoring though. If you don't like how this change ended up, I can revert it. I made a backup branch. I think it's an improvement, yeah. Not that it's a big deal to begin with, of course, but the extra patch to rename doesn't faze me if the code is improved as a result.
`struct hlsl_buffer` can have a register reservation but not an offset reservation, which makes its `reservation` field a little unintuitive.
True, but I'm not that bothered by that, it's not the only instance of fields that don't all apply. E.g. the writemask of hlsl_reg is irrelevant for buffers, or for large variables. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106#note_26281
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106
This merge request looks good to me; I'm not sure that it's good for a freeze however. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106#note_26283
This needs a rebase. Also, from patch 01/10: ```diff
+[test] +uniform 0 float4 0.0 1.0 2.0 3.0 +uniform 4 float4 4.0 5.0 6.0 7.0 +uniform 8 float4 8.0 9.0 10.0 11.0 +uniform 12 float4 12.0 13.0 14.0 15.0 +draw quad +probe all rgba (0.0, 4.0, 5.0, 6.0) + --
That introduces an empty line at EOF, which git considers a whitespace error. I'm not sure we care quite that much, but I don't think git is wrong there.
--
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106#note_26534
participants (4)
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura (＠zfigura)