This is an attempt to invent a vkd3d-shader interface, and begin to implement the relevant parts of ID3D12ShaderReflection in vkd3d-utils.
This series is missing most Doxygen documentation, mostly because I feel very uncertain about this interface and don't want to put in the significant effort to write documentation until I'm sure it's settled.
This series also has tests for bindings but not the implementation, mainly because I didn't want to bother splitting those tests out to a separate patch right now, and I see no harm in having them already there and marked todo...
Here is a long list of already open questions:
* As always, should anything be named differently?
* In particular, I've decided to give the variables names that make it clear this is d3d-specific code. I think trying to make this generic enough to work for other shader types is a fool's errand.
* Should we even try to use this interface for sm1? It's mostly already a subset of dxbc; the only addition is the constant register set...
* Should we bother with interfaces? These count as "constant buffers", they have a distinct type, and they have some extra data that we'd need to account for.
* Should we bother with lib_* targets? This mostly means allowing for a single shader to hold multiple sets of reflection data. Note that this would also affect descriptor info and STAT info, although not signature info.
* I've tried to pass through most data directly, partly out of a concern that that's what d3dcompiler probably does, and so trying to interpret it could break an extremely hypothetical and unlikely application. More saliently, converting to a more sensible representation just to convert back is extra work that I'm not really sure I see a point for.
But if we want to change the interface regardless, here are some ideas:
- enum vkd3d_shader_d3d_buffer_type could just be more flags.
- I think a buffer's size is redundant and can be calculated from its contained variables. Same for field offsets (unless packoffset affects this?)
- "constant_register" could be flags too? But we already have a type for them, so I don't think that's better.
- The kind/type interface is weird in several ways. Majority could be a type or var flag; objects could be classes...
* Fields of struct vkd3d_shader_d3d_type could be 16-bit (like they are in the RDEF section). The main reason I didn't do this is because we can't force a 16-bit enum, although we could define the field as uint16_t. But we could also just define the other "count" fields as 16-bit.
* I suspect I already know the answer to this one, but... If this is all going to be d3d specific anyway... do we really care about having this in vkd3d-shader?
Or, alternatively, we could provide an interface that's friendlier than COM, but still use the d3d struct types instead of copy-pasting and changing all the names to vkd3d-shader (which is a bit exhausting, honestly—not just because of rewriting but because of documentation. The latter is certainly a self-created problem, but so is the former.)
* vkd3d_shader_d3d_field currently has a hole in it on LP64; is that a problem? (I think it's the only structure with a hole in it.)
* As long as this is a d3d interface, should we go ahead and make it "scan_d3d_info" and add all the d3d-specific STAT block stuff?
One extra anticipatory note on how this differs from Wine's implementation: This implementation does *not* store types in an rbtree, nor deduplicate them. The main reason for this is that native doesn't deduplicate types (I have tests for this, later in my local branch) and it avoids what seems like a significant complication to the vkd3d-shader interface.
From: Zebediah Figura zfigura@codeweavers.com
--- include/private/vkd3d_memory.h | 9 + include/vkd3d_shader.h | 213 +++++++++++++++++++++++ libs/vkd3d-shader/dxbc.c | 178 +++++++++++++++++++ libs/vkd3d-shader/dxil.c | 1 + libs/vkd3d-shader/tpf.c | 1 + libs/vkd3d-shader/vkd3d_shader.map | 1 + libs/vkd3d-shader/vkd3d_shader_main.c | 37 ++++ libs/vkd3d-shader/vkd3d_shader_private.h | 2 + 8 files changed, 442 insertions(+)
diff --git a/include/private/vkd3d_memory.h b/include/private/vkd3d_memory.h index 8a2edb100..d89f741b5 100644 --- a/include/private/vkd3d_memory.h +++ b/include/private/vkd3d_memory.h @@ -65,6 +65,15 @@ static inline char *vkd3d_strdup(const char *string) return ptr; }
+static inline void *vkd3d_memdup(const void *mem, size_t size) +{ + void *ptr; + + if ((ptr = vkd3d_malloc(size))) + memcpy(ptr, mem, size); + return ptr; +} + bool vkd3d_array_reserve(void **elements, size_t *capacity, size_t element_count, size_t element_size);
#endif /* __VKD3D_MEMORY_H */ diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index a8cc3a336..32156c5ed 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -102,6 +102,11 @@ enum vkd3d_shader_structure_type * \since 1.10 */ VKD3D_SHADER_STRUCTURE_TYPE_SCAN_COMBINED_RESOURCE_SAMPLER_INFO, + /** + * The structure is a vkd3d_shader_scan_d3d_buffer_info structure. + * \since 1.11 + */ + VKD3D_SHADER_STRUCTURE_TYPE_SCAN_D3D_BUFFER_INFO,
VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_STRUCTURE_TYPE), }; @@ -1922,6 +1927,210 @@ struct vkd3d_shader_varying_map_info unsigned int varying_count; };
+enum vkd3d_shader_d3d_type_kind +{ + VKD3D_SHADER_D3D_TYPE_KIND_SCALAR = 0, + VKD3D_SHADER_D3D_TYPE_KIND_VECTOR = 1, + VKD3D_SHADER_D3D_TYPE_KIND_MATRIX_ROW_MAJOR = 2, + VKD3D_SHADER_D3D_TYPE_KIND_MATRIX_COLUMN_MAJOR = 3, + VKD3D_SHADER_D3D_TYPE_KIND_OBJECT = 4, + VKD3D_SHADER_D3D_TYPE_KIND_STRUCT = 5, + + VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_D3D_TYPE_KIND), +}; + +enum vkd3d_shader_d3d_base_type +{ + VKD3D_SHADER_D3D_BASE_TYPE_VOID, + VKD3D_SHADER_D3D_BASE_TYPE_BOOL, + VKD3D_SHADER_D3D_BASE_TYPE_INT, + VKD3D_SHADER_D3D_BASE_TYPE_FLOAT, + VKD3D_SHADER_D3D_BASE_TYPE_STRING, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURE, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURE1D, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURE2D, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURE3D, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURECUBE, + VKD3D_SHADER_D3D_BASE_TYPE_SAMPLER, + VKD3D_SHADER_D3D_BASE_TYPE_SAMPLER1D, + VKD3D_SHADER_D3D_BASE_TYPE_SAMPLER2D, + VKD3D_SHADER_D3D_BASE_TYPE_SAMPLER3D, + VKD3D_SHADER_D3D_BASE_TYPE_SAMPLERCUBE, + VKD3D_SHADER_D3D_BASE_TYPE_PIXELSHADER, + VKD3D_SHADER_D3D_BASE_TYPE_VERTEXSHADER, + VKD3D_SHADER_D3D_BASE_TYPE_PIXELFRAGMENT, + VKD3D_SHADER_D3D_BASE_TYPE_VERTEXFRAGMENT, + VKD3D_SHADER_D3D_BASE_TYPE_UINT, + VKD3D_SHADER_D3D_BASE_TYPE_UINT8, + VKD3D_SHADER_D3D_BASE_TYPE_GEOMETRYSHADER, + VKD3D_SHADER_D3D_BASE_TYPE_RASTERIZER, + VKD3D_SHADER_D3D_BASE_TYPE_DEPTHSTENCIL, + VKD3D_SHADER_D3D_BASE_TYPE_BLEND, + VKD3D_SHADER_D3D_BASE_TYPE_BUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_CBUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_TBUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURE1DARRAY, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURE2DARRAY, + VKD3D_SHADER_D3D_BASE_TYPE_RENDERTARGETVIEW, + VKD3D_SHADER_D3D_BASE_TYPE_DEPTHSTENCILVIEW, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURE2DMS, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURE2DMSARRAY, + VKD3D_SHADER_D3D_BASE_TYPE_TEXTURECUBEARRAY, + VKD3D_SHADER_D3D_BASE_TYPE_HULLSHADER, + VKD3D_SHADER_D3D_BASE_TYPE_DOMAINSHADER, + VKD3D_SHADER_D3D_BASE_TYPE_INTERFACE_POINTER, + VKD3D_SHADER_D3D_BASE_TYPE_COMPUTESHADER, + VKD3D_SHADER_D3D_BASE_TYPE_DOUBLE, + VKD3D_SHADER_D3D_BASE_TYPE_RWTEXTURE1D, + VKD3D_SHADER_D3D_BASE_TYPE_RWTEXTURE1DARRAY, + VKD3D_SHADER_D3D_BASE_TYPE_RWTEXTURE2D, + VKD3D_SHADER_D3D_BASE_TYPE_RWTEXTURE2DARRAY, + VKD3D_SHADER_D3D_BASE_TYPE_RWTEXTURE3D, + VKD3D_SHADER_D3D_BASE_TYPE_RWBUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_BYTEADDRESS_BUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_RWBYTEADDRESS_BUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_STRUCTURED_BUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_RWSTRUCTURED_BUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_APPEND_STRUCTURED_BUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_CONSUME_STRUCTURED_BUFFER, + VKD3D_SHADER_D3D_BASE_TYPE_MIN8FLOAT, + VKD3D_SHADER_D3D_BASE_TYPE_MIN10FLOAT, + VKD3D_SHADER_D3D_BASE_TYPE_MIN16FLOAT, + VKD3D_SHADER_D3D_BASE_TYPE_MIN12INT, + VKD3D_SHADER_D3D_BASE_TYPE_MIN16INT, + VKD3D_SHADER_D3D_BASE_TYPE_MIN16UINT, + + VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_D3D_BASE_TYPE), +}; + +/** + * Information about a Direct3D type constructed from reflection data. + * + * Array types are represented by setting \ref element_count to the size of the + * array, and all other fields to those of the element type. + * + * This structure is used in struct vkd3d_shader_varying_map_info. + */ +struct vkd3d_shader_d3d_type +{ + /** Type kind, e.g. scalar, vector, struct, or object. */ + enum vkd3d_shader_d3d_type_kind kind; + /** Specific numeric or object type. */ + enum vkd3d_shader_d3d_base_type base_type; + /** Number of rows. */ + uint32_t row_count; + /** Number of columns. */ + uint32_t column_count; + /** Number of elements in an array, or 0 if this type is not an array. */ + uint32_t element_count; + /** Number of struct fields, or 0 if this type is not a struct. */ + uint32_t field_count; + /** Name of the type, or NULL if the type has no name. */ + const char *name; + /** + * Points to an array of \ref field_count fields, + * or NULL if this type is not a struct. + */ + const struct vkd3d_shader_d3d_field *fields; +}; + +struct vkd3d_shader_d3d_field +{ + const char *name; + /** Offset within the struct, in bytes. */ + uint32_t offset; + struct vkd3d_shader_d3d_type type; +}; + +enum vkd3d_shader_d3d_variable_flag +{ + VKD3D_SHADER_D3D_VARIABLE_USER_PACKED = 0x00000001, + VKD3D_SHADER_D3D_VARIABLE_USED = 0x00000002, +}; + +struct vkd3d_shader_d3d_variable +{ + /** Name of the variable. */ + const char *name; + /** Offset of this variable's numeric components, in bytes. */ + uint32_t offset; + /** Size of this variable's numeric components, in bytes. */ + uint32_t size; + /** + * A combination of zero or more elements of vkd3d_shader_d3d_variable_flag. + */ + uint32_t flags; + /** + * Base binding point of this variable's resource components, or ~0u + * if the variable has no resource components. + */ + uint32_t resource_binding; + /** Number of resource components associated with this variable. */ + uint32_t resource_count; + /** + * Base binding point of this variable's sampler components, or ~0u + * if the variable has no sampler components. + */ + uint32_t sampler_binding; + /** Number of sampler components associated with this variable. */ + uint32_t sampler_count; + /** + * If this is a legacy Direct3D shader, and the variable has numeric type + * (i.e. is not of type VKD3D_SHADER_D3D_TYPE_KIND_OBJECT), this field + * describes the register set to which this variable belongs. + * Otherwise, the contents of this field are undefined. + */ + enum vkd3d_shader_d3dbc_constant_register constant_register; + /** + * Default value of the variable, or NULL if the variable has no default + * value. + * If not NULL, this field points to an appropriately typed value + * of size \ref size. + */ + const void *default_value; + /** Type of the variable. */ + struct vkd3d_shader_d3d_type type; +}; + +enum vkd3d_shader_d3d_buffer_type +{ + VKD3D_SHADER_D3D_BUFFER_TYPE_CONSTANT = 0, + VKD3D_SHADER_D3D_BUFFER_TYPE_TEXTURE = 1, + + VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_D3D_BUFFER_TYPE), +}; + +enum vkd3d_shader_d3d_buffer_flag +{ + VKD3D_SHADER_D3D_BUFFER_USER_PACKED = 0x00000001, +}; + +struct vkd3d_shader_d3d_buffer +{ + const char *name; + enum vkd3d_shader_d3d_buffer_type type; + /** + * A combination of zero or more elements of vkd3d_shader_d3d_buffer_flag. + */ + uint32_t flags; + + const struct vkd3d_shader_d3d_variable *variables; + unsigned int variable_count; + + /** Size of the buffer's numeric components, in bytes. */ + unsigned int size; +}; + +/** \since 1.11 */ +struct vkd3d_shader_scan_d3d_buffer_info +{ + enum vkd3d_shader_structure_type type; + const void *next; + + const struct vkd3d_shader_d3d_buffer *buffers; + unsigned int buffer_count; +}; + #ifdef LIBVKD3D_SHADER_SOURCE # define VKD3D_SHADER_API VKD3D_EXPORT #else @@ -2481,6 +2690,10 @@ VKD3D_SHADER_API void vkd3d_shader_build_varying_map(const struct vkd3d_shader_s VKD3D_SHADER_API void vkd3d_shader_free_scan_combined_resource_sampler_info( struct vkd3d_shader_scan_combined_resource_sampler_info *info);
+/** \since 1.11 */ +VKD3D_SHADER_API void vkd3d_shader_free_scan_d3d_buffer_info(struct vkd3d_shader_scan_d3d_buffer_info *info); + + #endif /* VKD3D_SHADER_NO_PROTOTYPES */
/** Type of vkd3d_shader_get_version(). */ diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 37ebc73c0..e628f01c2 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -20,6 +20,11 @@
#include "vkd3d_shader_private.h"
+#define DXBC_PROFILE_MAJOR_VERSION_SHIFT 8 +#define DXBC_PROFILE_MAJOR_VERSION_MASK (0xff << DXBC_PROFILE_MAJOR_VERSION_SHIFT) +#define DXBC_PROFILE_MINOR_VERSION_SHIFT 0 +#define DXBC_PROFILE_MINOR_VERSION_MASK (0xff << DXBC_PROFILE_MINOR_VERSION_SHIFT) + void dxbc_writer_init(struct dxbc_writer *dxbc) { memset(dxbc, 0, sizeof(*dxbc)); @@ -482,6 +487,174 @@ int shader_parse_input_signature(const struct vkd3d_shader_code *dxbc, return ret; }
+static int parse_type(const struct vkd3d_shader_dxbc_section_desc *section, + uint32_t offset, struct vkd3d_shader_d3d_type *type, bool is_sm5) +{ + uint32_t temp, fields_offset, name_offset, field_type_offset; + const char *data = section->data.code; + const char *ptr = data + offset; + const char *name; + int ret; + + temp = read_u32(&ptr); + type->kind = temp & 0xffff; + type->base_type = temp >> 16; + temp = read_u32(&ptr); + type->row_count = temp & 0xffff; + type->column_count = temp >> 16; + temp = read_u32(&ptr); + type->element_count = temp & 0xffff; + type->field_count = temp >> 16; + fields_offset = read_u32(&ptr); + + if (is_sm5) + { + skip_dword_unknown(&ptr, 4); + name_offset = read_u32(&ptr); + + if ((name = shader_get_string(data, section->data.size, name_offset)) + && !(type->name = vkd3d_strdup(name))) + return VKD3D_ERROR_OUT_OF_MEMORY; + } + + if (fields_offset) + { + struct vkd3d_shader_d3d_field *fields; + + if (!(fields = vkd3d_calloc(type->field_count, sizeof(*fields)))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + ptr = data + fields_offset; + + for (uint32_t i = 0; i < type->field_count; ++i) + { + struct vkd3d_shader_d3d_field *field = &fields[i]; + + name_offset = read_u32(&ptr); + field_type_offset = read_u32(&ptr); + field->offset = read_u32(&ptr); + + if ((name = shader_get_string(data, section->data.size, name_offset)) + && !(field->name = vkd3d_strdup(name))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + if ((ret = parse_type(section, field_type_offset, &field->type, is_sm5)) < 0) + return ret; + } + + type->fields = fields; + } + + return VKD3D_OK; +} + +static int parse_variables(const struct vkd3d_shader_dxbc_section_desc *section, + uint32_t offset, struct vkd3d_shader_d3d_buffer *buffer, bool is_sm5) +{ + struct vkd3d_shader_d3d_variable *variables; + const char *data = section->data.code; + const char *ptr = data + offset; + int ret; + + if (!(variables = vkd3d_calloc(buffer->variable_count, sizeof(*variables)))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + for (uint32_t i = 0; i < buffer->variable_count; ++i) + { + struct vkd3d_shader_d3d_variable *variable = &variables[i]; + uint32_t name_offset, type_offset, default_value_offset; + const char *name; + + name_offset = read_u32(&ptr); + variable->offset = read_u32(&ptr); + variable->size = read_u32(&ptr); + variable->flags = read_u32(&ptr); + type_offset = read_u32(&ptr); + default_value_offset = read_u32(&ptr); + + if ((name = shader_get_string(data, section->data.size, name_offset)) + && !(variable->name = vkd3d_strdup(name))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + if (default_value_offset + && !(variable->default_value = vkd3d_memdup(data + default_value_offset, variable->size))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + if (is_sm5) + { + variable->resource_binding = read_u32(&ptr); + variable->resource_count = read_u32(&ptr); + variable->sampler_binding = read_u32(&ptr); + variable->sampler_count = read_u32(&ptr); + } + + if ((ret = parse_type(section, type_offset, &variable->type, is_sm5))) + return ret; + } + + buffer->variables = variables; + + return VKD3D_OK; +} + +static int parse_rdef(const struct vkd3d_shader_dxbc_section_desc *section, + struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_scan_d3d_buffer_info *buffer_info) +{ + uint32_t buffer_count, buffer_offset, profile; + const char *data = section->data.code; + const char *ptr = data; + bool is_sm5; + int ret; + + if (!buffer_info) + return VKD3D_OK; + + buffer_count = read_u32(&ptr); + buffer_offset = read_u32(&ptr); + /* Resource count and offset; not currently used. */ + read_u32(&ptr); + read_u32(&ptr); + profile = read_u32(&ptr); + + is_sm5 = ((profile >> DXBC_PROFILE_MAJOR_VERSION_SHIFT) & DXBC_PROFILE_MAJOR_VERSION_MASK) >= 5; + + if (buffer_count) + { + struct vkd3d_shader_d3d_buffer *buffers; + + if (!(buffers = vkd3d_calloc(buffer_count, sizeof(*buffers)))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + ptr = data + buffer_offset; + + for (uint32_t i = 0; i < buffer_count; ++i) + { + struct vkd3d_shader_d3d_buffer *buffer = &buffers[i]; + uint32_t name_offset, var_offset; + const char *name; + + name_offset = read_u32(&ptr); + buffer->variable_count = read_u32(&ptr); + var_offset = read_u32(&ptr); + buffer->size = read_u32(&ptr); + buffer->flags = read_u32(&ptr); + buffer->type = read_u32(&ptr); + + if ((name = shader_get_string(data, section->data.size, name_offset)) + && !(buffer->name = vkd3d_strdup(name))) + return VKD3D_ERROR_OUT_OF_MEMORY; + + if ((ret = parse_variables(section, var_offset, buffer, is_sm5))) + return ret; + } + + buffer_info->buffer_count = buffer_count; + buffer_info->buffers = buffers; + } + + return VKD3D_OK; +} + static int shdr_handler(const struct vkd3d_shader_dxbc_section_desc *section, struct vkd3d_shader_message_context *message_context, void *context) { @@ -538,6 +711,11 @@ static int shdr_handler(const struct vkd3d_shader_dxbc_section_desc *section, desc->byte_code_size = section->data.size; break;
+ case TAG_RDEF: + if ((ret = parse_rdef(section, message_context, desc->d3d_buffer_info)) < 0) + return ret; + break; + case TAG_AON9: TRACE("Skipping AON9 shader code chunk.\n"); break; diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 8a31d03c5..d4ed8e3b3 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -7290,6 +7290,7 @@ int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compi
shader_desc = &sm6->p.shader_desc; shader_desc->is_dxil = true; + shader_desc->d3d_buffer_info = vkd3d_find_struct(compile_info->next, SCAN_D3D_BUFFER_INFO); if ((ret = shader_extract_from_dxbc(&compile_info->source, message_context, compile_info->source_name, shader_desc)) < 0) { diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 50146c2c7..c7ad2287f 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -2642,6 +2642,7 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi
shader_desc = &sm4->p.shader_desc; shader_desc->is_dxil = false; + shader_desc->d3d_buffer_info = vkd3d_find_struct(compile_info->next, SCAN_D3D_BUFFER_INFO); if ((ret = shader_extract_from_dxbc(&compile_info->source, message_context, compile_info->source_name, shader_desc)) < 0) { diff --git a/libs/vkd3d-shader/vkd3d_shader.map b/libs/vkd3d-shader/vkd3d_shader.map index 973f90779..8cb79bd92 100644 --- a/libs/vkd3d-shader/vkd3d_shader.map +++ b/libs/vkd3d-shader/vkd3d_shader.map @@ -9,6 +9,7 @@ global: vkd3d_shader_free_messages; vkd3d_shader_free_root_signature; vkd3d_shader_free_scan_combined_resource_sampler_info; + vkd3d_shader_free_scan_d3d_buffer_info; vkd3d_shader_free_scan_descriptor_info; vkd3d_shader_free_scan_signature_info; vkd3d_shader_free_shader_code; diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index ac13be897..1d87f2499 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -1762,6 +1762,43 @@ void vkd3d_shader_free_scan_signature_info(struct vkd3d_shader_scan_signature_in vkd3d_shader_free_shader_signature(&info->patch_constant); }
+static void free_d3d_type(const struct vkd3d_shader_d3d_type *type) +{ + for (uint32_t i = 0; i < type->field_count; ++i) + { + const struct vkd3d_shader_d3d_field *field = &type->fields[i]; + + free_d3d_type(&field->type); + vkd3d_free((void *)field->name); + } + vkd3d_free((void *)type->fields); + vkd3d_free((void *)type->name); +} + +void vkd3d_shader_free_scan_d3d_buffer_info(struct vkd3d_shader_scan_d3d_buffer_info *info) +{ + TRACE("info %p.\n", info); + + for (uint32_t i = 0; i < info->buffer_count; ++i) + { + const struct vkd3d_shader_d3d_buffer *buffer = &info->buffers[i]; + + for (uint32_t j = 0; j < buffer->variable_count; ++j) + { + const struct vkd3d_shader_d3d_variable *variable = &buffer->variables[j]; + + free_d3d_type(&variable->type); + vkd3d_free((void *)variable->default_value); + vkd3d_free((void *)variable->name); + } + + vkd3d_free((void *)buffer->variables); + vkd3d_free((void *)buffer->name); + } + + vkd3d_free((void *)info->buffers); +} + void vkd3d_shader_free_shader_code(struct vkd3d_shader_code *shader_code) { TRACE("shader_code %p.\n", shader_code); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 288628845..20b1077a7 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1026,6 +1026,8 @@ struct vkd3d_shader_desc struct shader_signature output_signature; struct shader_signature patch_constant_signature;
+ struct vkd3d_shader_scan_d3d_buffer_info *d3d_buffer_info; + struct { uint32_t used, external;
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-utils/reflection.c | 105 +++++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-utils/reflection.c b/libs/vkd3d-utils/reflection.c index 2f604864b..38b26afb4 100644 --- a/libs/vkd3d-utils/reflection.c +++ b/libs/vkd3d-utils/reflection.c @@ -1,6 +1,7 @@ /* * Copyright 2009 Henri Verbeet for CodeWeavers * Copyright 2010 Rico Schüller + * Copyright 2024 Zebediah Figura for CodeWeavers * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -21,14 +22,82 @@ #include <vkd3d_d3dcommon.h> #include <vkd3d_d3d12shader.h>
+struct d3d12_buffer +{ + ID3D12ShaderReflectionConstantBuffer ID3D12ShaderReflectionConstantBuffer_iface; + const struct vkd3d_shader_d3d_buffer *buffer; +}; + struct d3d12_reflection { ID3D12ShaderReflection ID3D12ShaderReflection_iface; unsigned int refcount;
struct vkd3d_shader_scan_signature_info signature_info; + struct vkd3d_shader_scan_d3d_buffer_info buffer_info; + + struct d3d12_buffer *buffers; };
+static struct d3d12_buffer null_buffer; + +static struct d3d12_buffer *impl_from_ID3D12ShaderReflectionConstantBuffer(ID3D12ShaderReflectionConstantBuffer *iface) +{ + return CONTAINING_RECORD(iface, struct d3d12_buffer, ID3D12ShaderReflectionConstantBuffer_iface); +} + +static HRESULT STDMETHODCALLTYPE d3d12_buffer_GetDesc( + ID3D12ShaderReflectionConstantBuffer *iface, D3D12_SHADER_BUFFER_DESC *desc) +{ + struct d3d12_buffer *buffer = impl_from_ID3D12ShaderReflectionConstantBuffer(iface); + + TRACE("iface %p, desc %p.\n", iface, desc); + + if (buffer == &null_buffer) + { + WARN("Null constant buffer, returning E_FAIL.\n"); + return E_FAIL; + } + + if (!desc) + { + WARN("NULL pointer, returning E_FAIL.\n"); + return E_FAIL; + } + + desc->Name = buffer->buffer->name; + desc->Type = (D3D_CBUFFER_TYPE)buffer->buffer->type; + desc->Variables = buffer->buffer->variable_count; + desc->Size = buffer->buffer->size; + desc->uFlags = buffer->buffer->flags; + return S_OK; +} + +static ID3D12ShaderReflectionVariable * STDMETHODCALLTYPE d3d12_buffer_GetVariableByIndex( + ID3D12ShaderReflectionConstantBuffer *iface, UINT index) +{ + FIXME("iface %p, index %u, stub!\n", iface, index); + + return NULL; +} + +static ID3D12ShaderReflectionVariable * STDMETHODCALLTYPE d3d12_buffer_GetVariableByName( + ID3D12ShaderReflectionConstantBuffer *iface, const char *name) +{ + FIXME("iface %p, name %s, stub!\n", iface, debugstr_a(name)); + + return NULL; +} + +static const struct ID3D12ShaderReflectionConstantBufferVtbl d3d12_buffer_vtbl = +{ + d3d12_buffer_GetDesc, + d3d12_buffer_GetVariableByIndex, + d3d12_buffer_GetVariableByName, +}; + +static struct d3d12_buffer null_buffer = {{&d3d12_buffer_vtbl}}; + static struct d3d12_reflection *impl_from_ID3D12ShaderReflection(ID3D12ShaderReflection *iface) { return CONTAINING_RECORD(iface, struct d3d12_reflection, ID3D12ShaderReflection_iface); @@ -72,6 +141,7 @@ static ULONG STDMETHODCALLTYPE d3d12_reflection_Release(ID3D12ShaderReflection *
if (!refcount) { + vkd3d_shader_free_scan_d3d_buffer_info(&reflection->buffer_info); vkd3d_shader_free_scan_signature_info(&reflection->signature_info); free(reflection); } @@ -87,9 +157,11 @@ static HRESULT STDMETHODCALLTYPE d3d12_reflection_GetDesc(ID3D12ShaderReflection
FIXME("iface %p, desc %p partial stub!\n", iface, desc);
+ memset(desc, 0, sizeof(*desc)); desc->InputParameters = reflection->signature_info.input.element_count; desc->OutputParameters = reflection->signature_info.output.element_count; desc->PatchConstantParameters = reflection->signature_info.patch_constant.element_count; + desc->ConstantBuffers = reflection->buffer_info.buffer_count;
return S_OK; } @@ -97,9 +169,17 @@ static HRESULT STDMETHODCALLTYPE d3d12_reflection_GetDesc(ID3D12ShaderReflection static struct ID3D12ShaderReflectionConstantBuffer * STDMETHODCALLTYPE d3d12_reflection_GetConstantBufferByIndex( ID3D12ShaderReflection *iface, UINT index) { - FIXME("iface %p, index %u stub!\n", iface, index); + struct d3d12_reflection *reflection = impl_from_ID3D12ShaderReflection(iface);
- return NULL; + TRACE("iface %p, index %u.\n", iface, index); + + if (index > reflection->buffer_info.buffer_count) + { + WARN("Invalid index %u.\n", index); + return &null_buffer.ID3D12ShaderReflectionConstantBuffer_iface; + } + + return &reflection->buffers[index].ID3D12ShaderReflectionConstantBuffer_iface; }
static struct ID3D12ShaderReflectionConstantBuffer * STDMETHODCALLTYPE d3d12_reflection_GetConstantBufferByName( @@ -287,9 +367,16 @@ static const struct ID3D12ShaderReflectionVtbl d3d12_reflection_vtbl = d3d12_reflection_GetRequiresFlags, };
+static void d3d12_buffer_init(struct d3d12_buffer *buffer, const struct vkd3d_shader_d3d_buffer *vkd3d_buffer) +{ + buffer->ID3D12ShaderReflectionConstantBuffer_iface.lpVtbl = &d3d12_buffer_vtbl; + buffer->buffer = vkd3d_buffer; +} + static HRESULT d3d12_reflection_init(struct d3d12_reflection *reflection, const void *data, size_t data_size) { struct vkd3d_shader_compile_info compile_info = {.type = VKD3D_SHADER_STRUCTURE_TYPE_COMPILE_INFO}; + HRESULT hr;
reflection->ID3D12ShaderReflection_iface.lpVtbl = &d3d12_reflection_vtbl; reflection->refcount = 1; @@ -301,7 +388,19 @@ static HRESULT d3d12_reflection_init(struct d3d12_reflection *reflection, const compile_info.next = &reflection->signature_info; reflection->signature_info.type = VKD3D_SHADER_STRUCTURE_TYPE_SCAN_SIGNATURE_INFO;
- return hresult_from_vkd3d_result(vkd3d_shader_scan(&compile_info, NULL)); + reflection->signature_info.next = &reflection->buffer_info; + reflection->buffer_info.type = VKD3D_SHADER_STRUCTURE_TYPE_SCAN_D3D_BUFFER_INFO; + + if (FAILED(hr = hresult_from_vkd3d_result(vkd3d_shader_scan(&compile_info, NULL)))) + return hr; + + if (!(reflection->buffers = vkd3d_calloc(reflection->buffer_info.buffer_count, sizeof(*reflection->buffers)))) + return E_OUTOFMEMORY; + + for (uint32_t i = 0; i < reflection->buffer_info.buffer_count; ++i) + d3d12_buffer_init(&reflection->buffers[i], &reflection->buffer_info.buffers[i]); + + return S_OK; }
HRESULT WINAPI D3DReflect(const void *data, SIZE_T data_size, REFIID iid, void **reflection)
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-utils/reflection.c | 118 ++++++++++++++++++++++++++++++++-- 1 file changed, 114 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-utils/reflection.c b/libs/vkd3d-utils/reflection.c index 38b26afb4..c1fdf200f 100644 --- a/libs/vkd3d-utils/reflection.c +++ b/libs/vkd3d-utils/reflection.c @@ -22,10 +22,19 @@ #include <vkd3d_d3dcommon.h> #include <vkd3d_d3d12shader.h>
+struct d3d12_variable +{ + ID3D12ShaderReflectionVariable ID3D12ShaderReflectionVariable_iface; + const struct vkd3d_shader_d3d_variable *variable; + struct d3d12_buffer *buffer; +}; + struct d3d12_buffer { ID3D12ShaderReflectionConstantBuffer ID3D12ShaderReflectionConstantBuffer_iface; const struct vkd3d_shader_d3d_buffer *buffer; + + struct d3d12_variable *variables; };
struct d3d12_reflection @@ -40,6 +49,80 @@ struct d3d12_reflection };
static struct d3d12_buffer null_buffer; +static struct d3d12_variable null_variable; + +static struct d3d12_variable *impl_from_ID3D12ShaderReflectionVariable(ID3D12ShaderReflectionVariable *iface) +{ + return CONTAINING_RECORD(iface, struct d3d12_variable, ID3D12ShaderReflectionVariable_iface); +} + +static HRESULT STDMETHODCALLTYPE d3d12_variable_GetDesc( + ID3D12ShaderReflectionVariable *iface, D3D12_SHADER_VARIABLE_DESC *desc) +{ + struct d3d12_variable *variable = impl_from_ID3D12ShaderReflectionVariable(iface); + + TRACE("iface %p, desc %p.\n", iface, desc); + + if (variable == &null_variable) + { + WARN("Null variable, returning E_FAIL.\n"); + return E_FAIL; + } + + if (!desc) + { + WARN("NULL pointer, returning E_FAIL.\n"); + return E_FAIL; + } + + desc->Name = variable->variable->name; + desc->StartOffset = variable->variable->offset; + desc->Size = variable->variable->size; + desc->uFlags = variable->variable->flags; + desc->DefaultValue = (void *)variable->variable->default_value; + desc->StartTexture = variable->variable->resource_binding; + desc->TextureSize = variable->variable->resource_count; + desc->StartSampler = variable->variable->sampler_binding; + desc->SamplerSize = variable->variable->sampler_count; + + return S_OK; +} + +static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_variable_GetType( + ID3D12ShaderReflectionVariable *iface) +{ + FIXME("iface %p, stub!\n", iface); + + return NULL; +} + +static ID3D12ShaderReflectionConstantBuffer * STDMETHODCALLTYPE d3d12_variable_GetBuffer( + ID3D12ShaderReflectionVariable *iface) +{ + struct d3d12_variable *variable = impl_from_ID3D12ShaderReflectionVariable(iface); + + TRACE("iface %p.\n", iface); + + return &variable->buffer->ID3D12ShaderReflectionConstantBuffer_iface; +} + +static UINT STDMETHODCALLTYPE d3d12_variable_GetInterfaceSlot( + ID3D12ShaderReflectionVariable *iface, UINT index) +{ + FIXME("iface %p, index %u, stub!\n", iface, index); + + return 0; +} + +static const struct ID3D12ShaderReflectionVariableVtbl d3d12_variable_vtbl = +{ + d3d12_variable_GetDesc, + d3d12_variable_GetType, + d3d12_variable_GetBuffer, + d3d12_variable_GetInterfaceSlot, +}; + +static struct d3d12_variable null_variable = {{&d3d12_variable_vtbl}};
static struct d3d12_buffer *impl_from_ID3D12ShaderReflectionConstantBuffer(ID3D12ShaderReflectionConstantBuffer *iface) { @@ -76,9 +159,17 @@ static HRESULT STDMETHODCALLTYPE d3d12_buffer_GetDesc( static ID3D12ShaderReflectionVariable * STDMETHODCALLTYPE d3d12_buffer_GetVariableByIndex( ID3D12ShaderReflectionConstantBuffer *iface, UINT index) { - FIXME("iface %p, index %u, stub!\n", iface, index); + struct d3d12_buffer *buffer = impl_from_ID3D12ShaderReflectionConstantBuffer(iface);
- return NULL; + TRACE("iface %p, index %u.\n", iface, index); + + if (index > buffer->buffer->variable_count) + { + WARN("Invalid index %u.\n", index); + return &null_variable.ID3D12ShaderReflectionVariable_iface; + } + + return &buffer->variables[index].ID3D12ShaderReflectionVariable_iface; }
static ID3D12ShaderReflectionVariable * STDMETHODCALLTYPE d3d12_buffer_GetVariableByName( @@ -367,10 +458,26 @@ static const struct ID3D12ShaderReflectionVtbl d3d12_reflection_vtbl = d3d12_reflection_GetRequiresFlags, };
-static void d3d12_buffer_init(struct d3d12_buffer *buffer, const struct vkd3d_shader_d3d_buffer *vkd3d_buffer) +static void d3d12_variable_init(struct d3d12_variable *variable, + struct d3d12_buffer *buffer, const struct vkd3d_shader_d3d_variable *vkd3d_variable) +{ + variable->ID3D12ShaderReflectionVariable_iface.lpVtbl = &d3d12_variable_vtbl; + variable->buffer = buffer; + variable->variable = vkd3d_variable; +} + +static bool d3d12_buffer_init(struct d3d12_buffer *buffer, const struct vkd3d_shader_d3d_buffer *vkd3d_buffer) { buffer->ID3D12ShaderReflectionConstantBuffer_iface.lpVtbl = &d3d12_buffer_vtbl; buffer->buffer = vkd3d_buffer; + + if (!(buffer->variables = vkd3d_calloc(vkd3d_buffer->variable_count, sizeof(*buffer->variables)))) + return false; + + for (uint32_t i = 0; i < vkd3d_buffer->variable_count; ++i) + d3d12_variable_init(&buffer->variables[i], buffer, &vkd3d_buffer->variables[i]); + + return true; }
static HRESULT d3d12_reflection_init(struct d3d12_reflection *reflection, const void *data, size_t data_size) @@ -398,7 +505,10 @@ static HRESULT d3d12_reflection_init(struct d3d12_reflection *reflection, const return E_OUTOFMEMORY;
for (uint32_t i = 0; i < reflection->buffer_info.buffer_count; ++i) - d3d12_buffer_init(&reflection->buffers[i], &reflection->buffer_info.buffers[i]); + { + if (!d3d12_buffer_init(&reflection->buffers[i], &reflection->buffer_info.buffers[i])) + return E_OUTOFMEMORY; + }
return S_OK; }
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-utils/reflection.c | 155 +++++++++++++++++++++++++++++++++- 1 file changed, 153 insertions(+), 2 deletions(-)
diff --git a/libs/vkd3d-utils/reflection.c b/libs/vkd3d-utils/reflection.c index c1fdf200f..f6e545c56 100644 --- a/libs/vkd3d-utils/reflection.c +++ b/libs/vkd3d-utils/reflection.c @@ -22,11 +22,19 @@ #include <vkd3d_d3dcommon.h> #include <vkd3d_d3d12shader.h>
+struct d3d12_type +{ + ID3D12ShaderReflectionType ID3D12ShaderReflectionType_iface; + const struct vkd3d_shader_d3d_type *type; +}; + struct d3d12_variable { ID3D12ShaderReflectionVariable ID3D12ShaderReflectionVariable_iface; const struct vkd3d_shader_d3d_variable *variable; struct d3d12_buffer *buffer; + + struct d3d12_type type; };
struct d3d12_buffer @@ -50,6 +58,137 @@ struct d3d12_reflection
static struct d3d12_buffer null_buffer; static struct d3d12_variable null_variable; +static struct d3d12_type null_type; + +static struct d3d12_type *impl_from_ID3D12ShaderReflectionType(ID3D12ShaderReflectionType *iface) +{ + return CONTAINING_RECORD(iface, struct d3d12_type, ID3D12ShaderReflectionType_iface); +} + +static HRESULT STDMETHODCALLTYPE d3d12_type_GetDesc( + ID3D12ShaderReflectionType *iface, D3D12_SHADER_TYPE_DESC *desc) +{ + struct d3d12_type *type = impl_from_ID3D12ShaderReflectionType(iface); + + TRACE("iface %p, desc %p.\n", iface, desc); + + if (type == &null_type) + { + WARN("Null type, returning E_FAIL.\n"); + return E_FAIL; + } + + if (!desc) + { + WARN("NULL pointer, returning E_FAIL.\n"); + return E_FAIL; + } + + desc->Class = (D3D_SHADER_VARIABLE_CLASS)type->type->kind; + desc->Type = (D3D_SHADER_VARIABLE_TYPE)type->type->base_type; + desc->Rows = type->type->row_count; + desc->Columns = type->type->column_count; + desc->Elements = type->type->element_count; + desc->Members = type->type->field_count; + desc->Offset = 0; + desc->Name = type->type->name; + + return S_OK; +} + +static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_type_GetMemberTypeByIndex( + ID3D12ShaderReflectionType *iface, UINT index) +{ + FIXME("iface %p, index %u, stub!\n", iface, index); + + return NULL; +} + +static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_type_GetMemberTypeByName( + ID3D12ShaderReflectionType *iface, const char *name) +{ + FIXME("iface %p, name %s, stub!\n", iface, debugstr_a(name)); + + return NULL; +} + +static const char * STDMETHODCALLTYPE d3d12_type_GetMemberTypeName( + ID3D12ShaderReflectionType *iface, UINT index) +{ + FIXME("iface %p, index %u, stub!\n", iface, index); + + return NULL; +} + +static HRESULT STDMETHODCALLTYPE d3d12_type_IsEqual( + ID3D12ShaderReflectionType *iface, ID3D12ShaderReflectionType *other) +{ + FIXME("iface %p, other %p, stub!\n", iface, other); + return E_NOTIMPL; +} + +static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_type_GetSubType( + ID3D12ShaderReflectionType *iface) +{ + FIXME("iface %p stub!\n", iface); + + return NULL; +} + +static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_type_GetBaseClass( + ID3D12ShaderReflectionType *iface) +{ + FIXME("iface %p stub!\n", iface); + + return NULL; +} + +static UINT STDMETHODCALLTYPE d3d12_type_GetNumInterfaces( + ID3D12ShaderReflectionType *iface) +{ + FIXME("iface %p stub!\n", iface); + + return 0; +} + +static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_type_GetInterfaceByIndex( + ID3D12ShaderReflectionType *iface, UINT index) +{ + FIXME("iface %p, index %u stub!\n", iface, index); + + return NULL; +} + +static HRESULT STDMETHODCALLTYPE d3d12_type_IsOfType( + ID3D12ShaderReflectionType *iface, ID3D12ShaderReflectionType *type) +{ + FIXME("iface %p, type %p stub!\n", iface, type); + + return E_NOTIMPL; +} + +static HRESULT STDMETHODCALLTYPE d3d12_type_ImplementsInterface( + ID3D12ShaderReflectionType *iface, ID3D12ShaderReflectionType *base) +{ + FIXME("iface %p, base %p stub!\n", iface, base); + + return E_NOTIMPL; +} + +static const struct ID3D12ShaderReflectionTypeVtbl d3d12_type_vtbl = +{ + d3d12_type_GetDesc, + d3d12_type_GetMemberTypeByIndex, + d3d12_type_GetMemberTypeByName, + d3d12_type_GetMemberTypeName, + d3d12_type_IsEqual, + d3d12_type_GetSubType, + d3d12_type_GetBaseClass, + d3d12_type_GetNumInterfaces, + d3d12_type_GetInterfaceByIndex, + d3d12_type_IsOfType, + d3d12_type_ImplementsInterface, +};
static struct d3d12_variable *impl_from_ID3D12ShaderReflectionVariable(ID3D12ShaderReflectionVariable *iface) { @@ -91,9 +230,14 @@ static HRESULT STDMETHODCALLTYPE d3d12_variable_GetDesc( static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_variable_GetType( ID3D12ShaderReflectionVariable *iface) { - FIXME("iface %p, stub!\n", iface); + struct d3d12_variable *variable = impl_from_ID3D12ShaderReflectionVariable(iface);
- return NULL; + TRACE("iface %p.\n", iface); + + if (!variable->variable) + return &null_type.ID3D12ShaderReflectionType_iface; + + return &variable->type.ID3D12ShaderReflectionType_iface; }
static ID3D12ShaderReflectionConstantBuffer * STDMETHODCALLTYPE d3d12_variable_GetBuffer( @@ -458,12 +602,19 @@ static const struct ID3D12ShaderReflectionVtbl d3d12_reflection_vtbl = d3d12_reflection_GetRequiresFlags, };
+static void d3d12_type_init(struct d3d12_type *type, const struct vkd3d_shader_d3d_type *vkd3d_type) +{ + type->ID3D12ShaderReflectionType_iface.lpVtbl = &d3d12_type_vtbl; + type->type = vkd3d_type; +} + static void d3d12_variable_init(struct d3d12_variable *variable, struct d3d12_buffer *buffer, const struct vkd3d_shader_d3d_variable *vkd3d_variable) { variable->ID3D12ShaderReflectionVariable_iface.lpVtbl = &d3d12_variable_vtbl; variable->buffer = buffer; variable->variable = vkd3d_variable; + d3d12_type_init(&variable->type, &vkd3d_variable->type); }
static bool d3d12_buffer_init(struct d3d12_buffer *buffer, const struct vkd3d_shader_d3d_buffer *vkd3d_buffer)
From: Zebediah Figura zfigura@codeweavers.com
--- libs/vkd3d-utils/reflection.c | 41 +++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/libs/vkd3d-utils/reflection.c b/libs/vkd3d-utils/reflection.c index f6e545c56..7adbc9cfa 100644 --- a/libs/vkd3d-utils/reflection.c +++ b/libs/vkd3d-utils/reflection.c @@ -26,6 +26,9 @@ struct d3d12_type { ID3D12ShaderReflectionType ID3D12ShaderReflectionType_iface; const struct vkd3d_shader_d3d_type *type; + uint32_t field_offset; + + struct d3d12_type *fields; };
struct d3d12_variable @@ -90,7 +93,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_type_GetDesc( desc->Columns = type->type->column_count; desc->Elements = type->type->element_count; desc->Members = type->type->field_count; - desc->Offset = 0; + desc->Offset = type->field_offset; desc->Name = type->type->name;
return S_OK; @@ -99,9 +102,17 @@ static HRESULT STDMETHODCALLTYPE d3d12_type_GetDesc( static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_type_GetMemberTypeByIndex( ID3D12ShaderReflectionType *iface, UINT index) { - FIXME("iface %p, index %u, stub!\n", iface, index); + struct d3d12_type *type = impl_from_ID3D12ShaderReflectionType(iface);
- return NULL; + TRACE("iface %p, index %u.\n", iface, index); + + if (index > type->type->field_count) + { + WARN("Invalid index %u.\n", index); + return &null_type.ID3D12ShaderReflectionType_iface; + } + + return &type->fields[index].ID3D12ShaderReflectionType_iface; }
static ID3D12ShaderReflectionType * STDMETHODCALLTYPE d3d12_type_GetMemberTypeByName( @@ -602,19 +613,32 @@ static const struct ID3D12ShaderReflectionVtbl d3d12_reflection_vtbl = d3d12_reflection_GetRequiresFlags, };
-static void d3d12_type_init(struct d3d12_type *type, const struct vkd3d_shader_d3d_type *vkd3d_type) +static bool d3d12_type_init(struct d3d12_type *type, + const struct vkd3d_shader_d3d_type *vkd3d_type, uint32_t field_offset) { type->ID3D12ShaderReflectionType_iface.lpVtbl = &d3d12_type_vtbl; type->type = vkd3d_type; + type->field_offset = field_offset; + + if (!(type->fields = vkd3d_calloc(vkd3d_type->field_count, sizeof(*type->fields)))) + return false; + + for (uint32_t i = 0; i < vkd3d_type->field_count; ++i) + { + if (!d3d12_type_init(&type->fields[i], &vkd3d_type->fields[i].type, vkd3d_type->fields[i].offset)) + return false; + } + + return true; }
-static void d3d12_variable_init(struct d3d12_variable *variable, +static bool d3d12_variable_init(struct d3d12_variable *variable, struct d3d12_buffer *buffer, const struct vkd3d_shader_d3d_variable *vkd3d_variable) { variable->ID3D12ShaderReflectionVariable_iface.lpVtbl = &d3d12_variable_vtbl; variable->buffer = buffer; variable->variable = vkd3d_variable; - d3d12_type_init(&variable->type, &vkd3d_variable->type); + return d3d12_type_init(&variable->type, &vkd3d_variable->type, 0); }
static bool d3d12_buffer_init(struct d3d12_buffer *buffer, const struct vkd3d_shader_d3d_buffer *vkd3d_buffer) @@ -626,7 +650,10 @@ static bool d3d12_buffer_init(struct d3d12_buffer *buffer, const struct vkd3d_sh return false;
for (uint32_t i = 0; i < vkd3d_buffer->variable_count; ++i) - d3d12_variable_init(&buffer->variables[i], buffer, &vkd3d_buffer->variables[i]); + { + if (!d3d12_variable_init(&buffer->variables[i], buffer, &vkd3d_buffer->variables[i])) + return false; + }
return true; }
From: Elizabeth Figura zfigura@codeweavers.com
Ported from Wine. --- tests/hlsl_d3d12.c | 290 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 290 insertions(+)
diff --git a/tests/hlsl_d3d12.c b/tests/hlsl_d3d12.c index 6f76cda5a..b32744d6e 100644 --- a/tests/hlsl_d3d12.c +++ b/tests/hlsl_d3d12.c @@ -1317,6 +1317,295 @@ static void test_get_blob_part(void) ok(!refcount, "Got refcount %u.\n", refcount); }
+static void check_type_desc_(int line, const D3D12_SHADER_TYPE_DESC *type, const D3D12_SHADER_TYPE_DESC *expect) +{ + ok_(line)(type->Class == expect->Class, "Got class %#x.\n", type->Class); + ok_(line)(type->Type == expect->Type, "Got type %#x.\n", type->Type); + ok_(line)(type->Rows == expect->Rows, "Got %u rows.\n", type->Rows); + ok_(line)(type->Columns == expect->Columns, "Got %u columns.\n", type->Columns); + ok_(line)(type->Elements == expect->Elements, "Got %u elements.\n", type->Elements); + ok_(line)(type->Members == expect->Members, "Got %u members.\n", type->Members); + ok_(line)(type->Offset == expect->Offset, "Got %u members.\n", type->Members); + ok_(line)(!strcmp(type->Name, expect->Name), "Got name "%s".\n", type->Name); +} +#define check_type_desc(a, b) check_type_desc_(__LINE__, a, b) + +static void test_reflection(void) +{ + ULONG refcount; + HRESULT hr; + + static const char vs_source[] = + "typedef uint uint_t;\n" + "float m;\n" + "\n" + "cbuffer b1\n" + "{\n" + " float a;\n" + " float2 b;\n" + " float4 c;\n" + " float d;\n" + " struct\n" + " {\n" + " float4 a;\n" + " float b;\n" + " float c;\n" + " } s;\n" + /* In direct contradiction to the documentation, this does not align. */ + " bool g;\n" + " float h[2];\n" + " int i;\n" + " uint_t j;\n" + " float3x1 k;\n" + " row_major float3x1 l;\n" + "#pragma pack_matrix(row_major)\n" + " float3x1 o;\n" + " float4 p;\n" + " float q;\n" + " struct r_name {float a;} r;\n" + " column_major float3x1 t;\n" + "};\n" + "\n" + "cbuffer b5 : register(b5)\n" + "{\n" + " float4 u;\n" + "}\n" + "\n" + "float4 main(uniform float4 n) : SV_POSITION\n" + "{\n" + " return o._31 + m + n + u;\n" + "}"; + + struct shader_variable + { + D3D12_SHADER_VARIABLE_DESC var_desc; + D3D12_SHADER_TYPE_DESC type_desc; + const D3D12_SHADER_TYPE_DESC *field_types; + }; + + struct shader_buffer + { + D3D12_SHADER_BUFFER_DESC desc; + const struct shader_variable *vars; + }; + + static const D3D12_SHADER_TYPE_DESC s_field_types[] = + { + {D3D_SVC_VECTOR, D3D_SVT_FLOAT, 1, 4, 0, 0, 0, "float4"}, + {D3D_SVC_SCALAR, D3D_SVT_FLOAT, 1, 1, 0, 0, 16, "float"}, + {D3D_SVC_SCALAR, D3D_SVT_FLOAT, 1, 1, 0, 0, 20, "float"}, + }; + + static const D3D12_SHADER_TYPE_DESC r_field_types[] = + { + {D3D_SVC_SCALAR, D3D_SVT_FLOAT, 1, 1, 0, 0, 0, "float"}, + }; + + static const struct shader_variable globals_vars = + {{"m", 0, 4, D3D_SVF_USED, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_SCALAR, D3D_SVT_FLOAT, 1, 1, 0, 0, 0, "float"}}; + static const struct shader_variable params_vars = + {{"n", 0, 16, D3D_SVF_USED, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_VECTOR, D3D_SVT_FLOAT, 1, 4, 0, 0, 0, "float4"}}; + static const struct shader_variable buffer_vars[] = + { + {{"a", 0, 4, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_SCALAR, D3D_SVT_FLOAT, 1, 1, 0, 0, 0, "float"}}, + {{"b", 4, 8, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_VECTOR, D3D_SVT_FLOAT, 1, 2, 0, 0, 0, "float2"}}, + {{"c", 16, 16, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_VECTOR, D3D_SVT_FLOAT, 1, 4, 0, 0, 0, "float4"}}, + {{"d", 32, 4, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_SCALAR, D3D_SVT_FLOAT, 1, 1, 0, 0, 0, "float"}}, + {{"s", 48, 24, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_STRUCT, D3D_SVT_VOID, 1, 6, 0, ARRAY_SIZE(s_field_types), 0, "<unnamed>"}, s_field_types}, + {{"g", 72, 4, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_SCALAR, D3D_SVT_BOOL, 1, 1, 0, 0, 0, "bool"}}, + {{"h", 80, 20, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_SCALAR, D3D_SVT_FLOAT, 1, 1, 2, 0, 0, "float"}}, + {{"i", 100, 4, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_SCALAR, D3D_SVT_INT, 1, 1, 0, 0, 0, "int"}}, + {{"j", 104, 4, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_SCALAR, D3D_SVT_UINT, 1, 1, 0, 0, 0, "uint_t"}}, + {{"k", 112, 12, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_MATRIX_COLUMNS, D3D_SVT_FLOAT, 3, 1, 0, 0, 0, "float3x1"}}, + {{"l", 128, 36, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_MATRIX_ROWS, D3D_SVT_FLOAT, 3, 1, 0, 0, 0, "float3x1"}}, + {{"o", 176, 36, D3D_SVF_USED, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_MATRIX_ROWS, D3D_SVT_FLOAT, 3, 1, 0, 0, 0, "float3x1"}}, + {{"p", 224, 16, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_VECTOR, D3D_SVT_FLOAT, 1, 4, 0, 0, 0, "float4"}}, + {{"q", 240, 4, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_SCALAR, D3D_SVT_FLOAT, 1, 1, 0, 0, 0, "float"}}, + {{"r", 256, 4, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_STRUCT, D3D_SVT_VOID, 1, 1, 0, ARRAY_SIZE(r_field_types), 0, "r_name"}, r_field_types}, + {{"t", 260, 12, 0, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_MATRIX_COLUMNS, D3D_SVT_FLOAT, 3, 1, 0, 0, 0, "float3x1"}}, + }; + static const struct shader_variable b5_vars = + {{"u", 0, 16, D3D_SVF_USED, NULL, ~0u, 0, ~0u, 0}, {D3D_SVC_VECTOR, D3D_SVT_FLOAT, 1, 4, 0, 0, 0, "float4"}}; + + static const struct shader_buffer vs_buffers[] = + { + {{"$Globals", D3D_CT_CBUFFER, 1, 16}, &globals_vars}, + {{"$Params", D3D_CT_CBUFFER, 1, 16}, ¶ms_vars}, + {{"b1", D3D_CT_CBUFFER, ARRAY_SIZE(buffer_vars), 272}, buffer_vars}, + {{"b5", D3D_CT_CBUFFER, 1, 16}, &b5_vars}, + }; + + static const D3D12_SHADER_INPUT_BIND_DESC vs_bindings[] = + { + {"$Globals", D3D_SIT_CBUFFER, 0, 1}, + {"$Params", D3D_SIT_CBUFFER, 1, 1}, + {"b1", D3D_SIT_CBUFFER, 2, 1}, + {"b5", D3D_SIT_CBUFFER, 5, 1, D3D_SIF_USERPACKED}, + }; + + static const char ps_source[] = + "texture2D a;\n" + "sampler c {};\n" + "SamplerState d {};\n" + "sampler e\n" + "{\n" + " Texture = a;\n" + " foo = bar + 2;\n" + "};\n" + "SamplerState f\n" + "{\n" + " Texture = a;\n" + " foo = bar + 2;\n" + "};\n" + "sampler2D g;\n" + "sampler b : register(s5);\n" + "float4 main(float2 pos : texcoord) : SV_TARGET\n" + "{\n" + " return a.Sample(b, pos) + a.Sample(c, pos) + a.Sample(d, pos) + tex2D(f, pos) + tex2D(e, pos)" + " + tex2D(g, pos);\n" + "}"; + + static const D3D12_SHADER_INPUT_BIND_DESC ps_bindings[] = + { + {"c", D3D_SIT_SAMPLER, 0, 1}, + {"d", D3D_SIT_SAMPLER, 1, 1}, + {"e", D3D_SIT_SAMPLER, 2, 1}, + {"f", D3D_SIT_SAMPLER, 3, 1}, + {"g", D3D_SIT_SAMPLER, 4, 1}, + {"b", D3D_SIT_SAMPLER, 5, 1, D3D_SIF_USERPACKED}, + {"f", D3D_SIT_TEXTURE, 0, 1, D3D_SIF_TEXTURE_COMPONENTS, D3D_RETURN_TYPE_FLOAT, D3D_SRV_DIMENSION_TEXTURE2D, ~0u}, + {"e", D3D_SIT_TEXTURE, 1, 1, D3D_SIF_TEXTURE_COMPONENTS, D3D_RETURN_TYPE_FLOAT, D3D_SRV_DIMENSION_TEXTURE2D, ~0u}, + {"g", D3D_SIT_TEXTURE, 2, 1, D3D_SIF_TEXTURE_COMPONENTS, D3D_RETURN_TYPE_FLOAT, D3D_SRV_DIMENSION_TEXTURE2D, ~0u}, + {"a", D3D_SIT_TEXTURE, 3, 1, D3D_SIF_TEXTURE_COMPONENTS, D3D_RETURN_TYPE_FLOAT, D3D_SRV_DIMENSION_TEXTURE2D, ~0u}, + }; + + static const struct + { + const char *source; + const char *profile; + const D3D12_SHADER_INPUT_BIND_DESC *bindings; + size_t binding_count; + const struct shader_buffer *buffers; + size_t buffer_count; + } + tests[] = + { + {vs_source, "vs_5_0", vs_bindings, ARRAY_SIZE(vs_bindings), vs_buffers, ARRAY_SIZE(vs_buffers)}, + {ps_source, "ps_5_0", ps_bindings, ARRAY_SIZE(ps_bindings)}, + }; + + for (unsigned int t = 0; t < ARRAY_SIZE(tests); ++t) + { + ID3D10Blob *code = compile_shader_flags(tests[t].source, + tests[t].profile, D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY); + ID3D12ShaderReflection *reflection; + D3D12_SHADER_DESC shader_desc; + + hr = D3DReflect(ID3D10Blob_GetBufferPointer(code), ID3D10Blob_GetBufferSize(code), + &IID_ID3D12ShaderReflection, (void **)&reflection); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + + hr = reflection->lpVtbl->GetDesc(reflection, &shader_desc); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(shader_desc.ConstantBuffers == tests[t].buffer_count, "Got %u buffers.\n", shader_desc.ConstantBuffers); + todo ok(shader_desc.BoundResources == tests[t].binding_count, "Got %u resources.\n", shader_desc.BoundResources); + + for (unsigned int i = 0; i < shader_desc.ConstantBuffers; ++i) + { + const struct shader_buffer *expect_buffer = &tests[t].buffers[i]; + ID3D12ShaderReflectionConstantBuffer *cbuffer; + D3D12_SHADER_BUFFER_DESC buffer_desc; + + vkd3d_test_push_context("Buffer %u", i); + + cbuffer = reflection->lpVtbl->GetConstantBufferByIndex(reflection, i); + hr = cbuffer->lpVtbl->GetDesc(cbuffer, &buffer_desc); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(!strcmp(buffer_desc.Name, expect_buffer->desc.Name), "Got name "%s".\n", buffer_desc.Name); + ok(buffer_desc.Type == expect_buffer->desc.Type, "Got type %#x.\n", buffer_desc.Type); + ok(buffer_desc.Variables == expect_buffer->desc.Variables, "Got %u variables.\n", buffer_desc.Variables); + ok(buffer_desc.Size == expect_buffer->desc.Size, "Got size %u.\n", buffer_desc.Size); + ok(buffer_desc.uFlags == expect_buffer->desc.uFlags, "Got flags %#x.\n", buffer_desc.uFlags); + + for (unsigned int j = 0; j < buffer_desc.Variables; ++j) + { + const struct shader_variable *expect = &expect_buffer->vars[j]; + ID3D12ShaderReflectionType *type, *field; + ID3D12ShaderReflectionVariable *var; + D3D12_SHADER_VARIABLE_DESC var_desc; + D3D12_SHADER_TYPE_DESC type_desc; + + vkd3d_test_push_context("Variable %u", j); + + var = cbuffer->lpVtbl->GetVariableByIndex(cbuffer, j); + hr = var->lpVtbl->GetDesc(var, &var_desc); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(!strcmp(var_desc.Name, expect->var_desc.Name), "Got name "%s".\n", var_desc.Name); + ok(var_desc.StartOffset == expect->var_desc.StartOffset, "Got offset %u.\n", var_desc.StartOffset); + ok(var_desc.Size == expect->var_desc.Size, "Got size %u.\n", var_desc.Size); + ok(var_desc.uFlags == expect->var_desc.uFlags, "Got flags %#x.\n", var_desc.uFlags); + ok(!var_desc.DefaultValue, "Got default value %p.\n", var_desc.DefaultValue); + todo ok(var_desc.StartTexture == expect->var_desc.StartTexture, + "Got texture offset %u.\n", var_desc.StartTexture); + ok(var_desc.TextureSize == expect->var_desc.TextureSize, + "Got texture size %u.\n", var_desc.TextureSize); + todo ok(var_desc.StartSampler == expect->var_desc.StartSampler, + "Got sampler offset %u.\n", var_desc.StartSampler); + ok(var_desc.SamplerSize == expect->var_desc.SamplerSize, + "Got sampler size %u.\n", var_desc.SamplerSize); + + type = var->lpVtbl->GetType(var); + hr = type->lpVtbl->GetDesc(type, &type_desc); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + check_type_desc(&type_desc, &expect->type_desc); + + for (unsigned int k = 0; k < type_desc.Members; ++k) + { + vkd3d_test_push_context("Field %u", k); + + field = type->lpVtbl->GetMemberTypeByIndex(type, k); + hr = field->lpVtbl->GetDesc(field, &type_desc); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + check_type_desc(&type_desc, &expect->field_types[k]); + + vkd3d_test_pop_context(); + } + + vkd3d_test_pop_context(); + } + + vkd3d_test_pop_context(); + } + + for (unsigned int i = 0; i < shader_desc.BoundResources; ++i) + { + const D3D12_SHADER_INPUT_BIND_DESC *expect = &tests[t].bindings[i]; + D3D12_SHADER_INPUT_BIND_DESC desc; + + vkd3d_test_push_context("Binding %u", i); + + hr = reflection->lpVtbl->GetResourceBindingDesc(reflection, i, &desc); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + + ok(!strcmp(desc.Name, expect->Name), "Got name "%s".\n", desc.Name); + ok(desc.Type == expect->Type, "Got type %#x.\n", desc.Type); + ok(desc.BindPoint == expect->BindPoint, "Got bind point %u.\n", desc.BindPoint); + ok(desc.BindCount == expect->BindCount, "Got bind count %u.\n", desc.BindCount); + todo_if ((expect->uFlags & D3D_SIF_USERPACKED) && expect->Type != D3D_SIT_CBUFFER) + ok(desc.uFlags == expect->uFlags, "Got flags %#x.\n", desc.uFlags); + ok(desc.ReturnType == expect->ReturnType, "Got return type %#x.\n", desc.ReturnType); + ok(desc.Dimension == expect->Dimension, "Got dimension %#x.\n", desc.Dimension); + ok(desc.NumSamples == expect->NumSamples, "Got multisample count %u.\n", desc.NumSamples); + + vkd3d_test_pop_context(); + } + + ID3D10Blob_Release(code); + refcount = reflection->lpVtbl->Release(reflection); + ok(!refcount, "Got unexpected refcount %u.\n", refcount); + } +} + static void check_signature_element_(int line, const D3D12_SIGNATURE_PARAMETER_DESC *desc, const D3D12_SIGNATURE_PARAMETER_DESC *expect) { @@ -1524,5 +1813,6 @@ START_TEST(hlsl_d3d12) run_test(test_thread_id); run_test(test_create_blob); run_test(test_get_blob_part); + run_test(test_reflection); run_test(test_signature_reflection); }
- I suspect I already know the answer to this one, but... If this is all going to be d3d specific anyway... do we really care about having this in vkd3d-shader?
Maybe not; perhaps it's fine for this to live in vkd3d-utils. Note though that instead of reporting this through vkd3d_shader_scan() we could also consider introducing e.g. a vkd3d_shader_parse_rdef() function, analogous to vkd3d_shader_parse_root_signature(). That probably wouldn't change the implementation much, but it would perhaps avoid the issue of this interface being a fair bit more specific to DXBC/D3D shaders than the rest of vkd3d_shader_scan().
Note though that instead of reporting this through vkd3d_shader_scan() we could also consider introducing e.g. a vkd3d_shader_parse_rdef() function, analogous to vkd3d_shader_parse_root_signature(). That probably wouldn't change the implementation much, but it would perhaps avoid the issue of this interface being a fair bit more specific to DXBC/D3D shaders than the rest of vkd3d_shader_scan().
I guess... but it's not like that really solves any of the annoying parts, and it ends up just making the interface a bit more inconsistent. It also means we can't use the same interface for sm1, assuming we even want to.
Maybe not; perhaps it's fine for this to live in vkd3d-utils.
It would make it a bit harder to use this from e.g. d3dcompiler in Wine though, as Wine doesn't currently build/include vkd3d-utils.
It would make it a bit harder to use this from e.g. d3dcompiler in Wine though, as Wine doesn't currently build/include vkd3d-utils.
Is it a problem for Wine to embed vkd3d-utils as well, though? It doesn't seem implausible that more stuff can be shared there.
It would make it a bit harder to use this from e.g. d3dcompiler in Wine though, as Wine doesn't currently build/include vkd3d-utils.
Is it a problem for Wine to embed vkd3d-utils as well, though? It doesn't seem implausible that more stuff can be shared there.
I'm not aware of major issues with using vkd3d-utils in Wine, no. As far as I'm aware it would just mean a little more effort on the Wine side to get that going, as opposed to using vkd3d or vkd3d-shader APIs.
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
+struct vkd3d_shader_d3d_type +{
- /** Type kind, e.g. scalar, vector, struct, or object. */
- enum vkd3d_shader_d3d_type_kind kind;
- /** Specific numeric or object type. */
- enum vkd3d_shader_d3d_base_type base_type;
- /** Number of rows. */
- uint32_t row_count;
- /** Number of columns. */
- uint32_t column_count;
- /** Number of elements in an array, or 0 if this type is not an array. */
- uint32_t element_count;
- /** Number of struct fields, or 0 if this type is not a struct. */
- uint32_t field_count;
- /** Name of the type, or NULL if the type has no name. */
- const char *name;
In general I'd avoid using `const` in structure fields, which tends to require unnecessary casting and make the structure less useful in its own for the client application.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxbc.c:
- buffer_offset = read_u32(&ptr);
- /* Resource count and offset; not currently used. */
- read_u32(&ptr);
- read_u32(&ptr);
- profile = read_u32(&ptr);
- is_sm5 = ((profile >> DXBC_PROFILE_MAJOR_VERSION_SHIFT) & DXBC_PROFILE_MAJOR_VERSION_MASK) >= 5;
- if (buffer_count)
- {
struct vkd3d_shader_d3d_buffer *buffers;if (!(buffers = vkd3d_calloc(buffer_count, sizeof(*buffers))))return VKD3D_ERROR_OUT_OF_MEMORY;ptr = data + buffer_offset;
Shouldn't this and similar computations elsewhere require some bound checking?
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxbc.c:
+static int parse_rdef(const struct vkd3d_shader_dxbc_section_desc *section,
struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_scan_d3d_buffer_info *buffer_info)+{
- uint32_t buffer_count, buffer_offset, profile;
- const char *data = section->data.code;
- const char *ptr = data;
- bool is_sm5;
- int ret;
- if (!buffer_info)
return VKD3D_OK;- buffer_count = read_u32(&ptr);
- buffer_offset = read_u32(&ptr);
- /* Resource count and offset; not currently used. */
Meaning they're not used by native or by us?
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxbc.c:
for (uint32_t i = 0; i < buffer_count; ++i){struct vkd3d_shader_d3d_buffer *buffer = &buffers[i];uint32_t name_offset, var_offset;const char *name;name_offset = read_u32(&ptr);buffer->variable_count = read_u32(&ptr);var_offset = read_u32(&ptr);buffer->size = read_u32(&ptr);buffer->flags = read_u32(&ptr);buffer->type = read_u32(&ptr);if ((name = shader_get_string(data, section->data.size, name_offset))&& !(buffer->name = vkd3d_strdup(name)))
Doesn't this leak memory?
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-utils/reflection.c:
static struct ID3D12ShaderReflectionConstantBuffer * STDMETHODCALLTYPE d3d12_reflection_GetConstantBufferByIndex( ID3D12ShaderReflection *iface, UINT index) {
- FIXME("iface %p, index %u stub!\n", iface, index);
- struct d3d12_reflection *reflection = impl_from_ID3D12ShaderReflection(iface);
- return NULL;
- TRACE("iface %p, index %u.\n", iface, index);
- if (index > reflection->buffer_info.buffer_count)
Shouldn't this be `>=`?
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
unsigned int varying_count;};
+enum vkd3d_shader_d3d_type_kind +{
- VKD3D_SHADER_D3D_TYPE_KIND_SCALAR = 0,
- VKD3D_SHADER_D3D_TYPE_KIND_VECTOR = 1,
- VKD3D_SHADER_D3D_TYPE_KIND_MATRIX_ROW_MAJOR = 2,
- VKD3D_SHADER_D3D_TYPE_KIND_MATRIX_COLUMN_MAJOR = 3,
- VKD3D_SHADER_D3D_TYPE_KIND_OBJECT = 4,
- VKD3D_SHADER_D3D_TYPE_KIND_STRUCT = 5,
- VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_D3D_TYPE_KIND),
+};
Why, here and below, we don't reuse the corresponding D3D enumerations?
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-utils/reflection.c:
compile_info.next = &reflection->signature_info; reflection->signature_info.type = VKD3D_SHADER_STRUCTURE_TYPE_SCAN_SIGNATURE_INFO;
- return hresult_from_vkd3d_result(vkd3d_shader_scan(&compile_info, NULL));
- reflection->signature_info.next = &reflection->buffer_info;
- reflection->buffer_info.type = VKD3D_SHADER_STRUCTURE_TYPE_SCAN_D3D_BUFFER_INFO;
- if (FAILED(hr = hresult_from_vkd3d_result(vkd3d_shader_scan(&compile_info, NULL))))
return hr;- if (!(reflection->buffers = vkd3d_calloc(reflection->buffer_info.buffer_count, sizeof(*reflection->buffers))))
It doesn't look like this allocation is freed anywhere.
It would make it a bit harder to use this from e.g. d3dcompiler in Wine though, as Wine doesn't currently build/include vkd3d-utils.
Is it a problem for Wine to embed vkd3d-utils as well, though? It doesn't seem implausible that more stuff can be shared there.
I'm not aware of major issues with using vkd3d-utils in Wine, no. As far as I'm aware it would just mean a little more effort on the Wine side to get that going, as opposed to using vkd3d or vkd3d-shader APIs.
Well, we can't quite use it as-is. The actual interface ABI differs between d3d10 and d3d11, and then there's a few subtle differences between d3dcompiler versions. We could get around that with a D3DReflectVKD3D() though.
On Wed Jan 31 12:15:25 2024 +0000, Giovanni Mascellani wrote:
It doesn't look like this allocation is freed anywhere.
It's freed in vkd3d_shader_free_scan_d3d_buffer_info().
On Wed Jan 31 12:15:24 2024 +0000, Giovanni Mascellani wrote:
Shouldn't this be `>=`?
Yes.
On Wed Jan 31 11:54:16 2024 +0000, Giovanni Mascellani wrote:
Meaning they're not used by native or by us?
Not currently used by us, i.e. not currently used by the reflection interfaces that are implemented.
It would make it a bit harder to use this from e.g. d3dcompiler in Wine though, as Wine doesn't currently build/include vkd3d-utils.
Is it a problem for Wine to embed vkd3d-utils as well, though? It doesn't seem implausible that more stuff can be shared there.
I'm not aware of major issues with using vkd3d-utils in Wine, no. As far as I'm aware it would just mean a little more effort on the Wine side to get that going, as opposed to using vkd3d or vkd3d-shader APIs.
Well, we can't quite use it as-is. The actual interface ABI differs between d3d10 and d3d11, and then there's a few subtle differences between d3dcompiler versions. We could get around that with a D3DReflectVKD3D() though.
Alternatively we put this in vkd3d-shader but use the actual d3dcompiler types. I know we try to avoid those in vkd3d-shader, but maybe we can find a way to define them without unnecessarily pulling in base Windows types always.
On Wed Jan 31 12:15:25 2024 +0000, Giovanni Mascellani wrote:
Why, here and below, we don't reuse the corresponding D3D enumerations?
It's a choice that predates these changes. We don't want vkd3d-shader to depend on anything about the win32 environment. This isn't the first time we've done this sort of thing either :-/
I'd certainly like to find a better way, though. Duplicating and documenting all these types is not fun.
On Wed Jan 31 11:54:15 2024 +0000, Giovanni Mascellani wrote:
Doesn't this leak memory?
It's also freed by vkd3d_shader_free_scan_d3d_buffer_info().
On Wed Jan 31 11:54:15 2024 +0000, Giovanni Mascellani wrote:
Shouldn't this and similar computations elsewhere require some bound checking?
We don't do bounds checking elsewhere and we didn't do it in the Wine code that this was semi-copied from. I don't disagree with adding it in general but it didn't seem like enough of a priority. But I can add some if it's going to be a blocker.
On Wed Jan 31 11:54:14 2024 +0000, Giovanni Mascellani wrote:
In general I'd avoid using `const` in structure fields, which tends to require unnecessary casting and make the structure less useful in its own for the client application.
Besides freeing I don't think there should be anyone that cares?
On the other hand, it seems that precedent is not to use const, so I guess I'll drop this.
On Wed Jan 31 19:04:35 2024 +0000, Zebediah Figura wrote:
We don't do bounds checking elsewhere and we didn't do it in the Wine code that this was semi-copied from. I don't disagree with adding it in general but it didn't seem like enough of a priority. But I can add some if it's going to be a blocker.
I don't think we should allow user-provided data cause undefined behavior on our side. This might be a security bug, and in general it seems to me that we want to avoid that. We might have something slipped in somewhere, but we consider that a mistake.
On Wed Jan 31 18:51:31 2024 +0000, Zebediah Figura wrote:
It's freed in vkd3d_shader_free_scan_d3d_buffer_info().
Hmm, `vkd3d_shader_free_scan_d3d_buffer_info()` is only given a pointer to `reflection->buffer_info`, how can it free `reflection->buffers`, short of breaking the API barrier between vkd3d-shader and vkd3d-utils? Am I missing something?
On Wed Jan 31 19:01:19 2024 +0000, Zebediah Figura wrote:
It's a choice that predates these changes. We don't want vkd3d-shader to depend on anything about the win32 environment. This isn't the first time we've done this sort of thing either :-/ I'd certainly like to find a better way, though. Duplicating and documenting all these types is not fun.
Yeah, right, that's sensible. It's not nice but I can see it's a good feature to have.
On Thu Feb 1 10:48:17 2024 +0000, Henri Verbeet wrote:
- I suspect I already know the answer to this one, but... If this is
all going to be d3d specific anyway... do we really care about having this in vkd3d-shader? Maybe not; perhaps it's fine for this to live in vkd3d-utils. Note though that instead of reporting this through vkd3d_shader_scan() we could also consider introducing e.g. a vkd3d_shader_parse_rdef() function, analogous to vkd3d_shader_parse_root_signature(). That probably wouldn't change the implementation much, but it would perhaps avoid the issue of this interface being a fair bit more specific to DXBC/D3D shaders than the rest of vkd3d_shader_scan().
After some thinking, I am not too convinced that touching vkd3d-shader is the right thing to do here. The "maximally purist" choice here might be to introduce vkd3d-shader-utils, for D3D-specific bits that do not need a Vulkan implementation, and honestly I don't even see that as excessively far-fetched (it's a few lines of Makefile and boilerplate after all, I think). However, leaving that aside, putting everything in vkd3d-utils currently sounds like the next best option. Hopefully the ABI and slight differences between d3dcompiler version problems can be abstracted away and bridged by appropriate small thunks in Wine, but I don't know much about them.
On Wed Jan 31 18:52:35 2024 +0000, Zebediah Figura wrote:
Not currently used by us, i.e. not currently used by the reflection interfaces that are implemented.
Doesn't that mean that as soon as we'll want to return the data stored there we'll have a problem with the vkd3d-shader interface? Maybe we should design the vkd3d-shader interface so that it already allows returning the data we're skipping here?
On Thu Feb 1 17:03:20 2024 +0000, Zebediah Figura wrote:
It would make it a bit harder to use this from e.g.
d3dcompiler in Wine though, as Wine doesn't currently build/include vkd3d-utils.
Is it a problem for Wine to embed vkd3d-utils as well, though? It
doesn't seem implausible that more stuff can be shared there.
I'm not aware of major issues with using vkd3d-utils in Wine, no. As
far as I'm aware it would just mean a little more effort on the Wine side to get that going, as opposed to using vkd3d or vkd3d-shader APIs.
Well, we can't quite use it as-is. The actual interface ABI differs
between d3d10 and d3d11, and then there's a few subtle differences between d3dcompiler versions. We could get around that with a D3DReflectVKD3D() though. Alternatively we put this in vkd3d-shader but use the actual d3dcompiler types. I know we try to avoid those in vkd3d-shader, but maybe we can find a way to define them without unnecessarily pulling in base Windows types always.
I don't know that there is anything wrong in having some reasonable wrapping / unwrapping in d3dcompiler_xx. Of course, you can argue on what would be "reasonable".
On Thu Feb 1 10:48:16 2024 +0000, Giovanni Mascellani wrote:
Hmm, `vkd3d_shader_free_scan_d3d_buffer_info()` is only given a pointer to `reflection->buffer_info`, how can it free `reflection->buffers`, short of breaking the API barrier between vkd3d-shader and vkd3d-utils? Am I missing something?
No, that's my mistake, sorry. I'll fix this in v2.
On Thu Feb 1 10:48:17 2024 +0000, Giovanni Mascellani wrote:
Doesn't that mean that as soon as we'll want to return the data stored there we'll have a problem with the vkd3d-shader interface? Maybe we should design the vkd3d-shader interface so that it already allows returning the data we're skipping here?
The idea is that it'd go through a different vkd3d-shader interface, in this case specifically a (slightly extended) vkd3d_shader_scan_descriptor_info1. We already have vkd3d_shader_scan_signature_info which covers a different part of ID3D12ShaderReflection. But that's also one of the topics that I had proposed for discussion, maybe we do want everything in the same scan struct after all, or maybe we just want all the RDEF information together, or I don't know...
This merge request was closed by Zebediah Figura.
Superseded by 626.
-
Elizabeth Figura -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Matteo Bruni (＠Mystral)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)