Public API for parsing and serialising DXBC blobs.
-- v2: vkd3d-shader/dxbc: Introduce API for serialising DXBC blobs. vkd3d-shader/dxbc: Introduce API for parsing DXBC blobs.
From: Henri Verbeet hverbeet@codeweavers.com
--- libs/vkd3d-shader/dxbc.c | 8 ++++---- libs/vkd3d-shader/hlsl_sm4.c | 2 +- libs/vkd3d-shader/vkd3d_shader_private.h | 3 +-- 3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 4041e0f5..a0be8d74 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -39,8 +39,8 @@ void dxbc_writer_add_section(struct dxbc_writer *dxbc, uint32_t tag, const void
section = &dxbc->sections[dxbc->section_count++]; section->tag = tag; - section->data = data; - section->size = size; + section->data.code = data; + section->data.size = size; }
int dxbc_writer_write(struct dxbc_writer *dxbc, struct vkd3d_shader_code *out) @@ -67,8 +67,8 @@ int dxbc_writer_write(struct dxbc_writer *dxbc, struct vkd3d_shader_code *out) { set_u32(&buffer, offsets_position + i * sizeof(uint32_t), bytecode_get_size(&buffer)); put_u32(&buffer, dxbc->sections[i].tag); - put_u32(&buffer, dxbc->sections[i].size); - bytecode_put_bytes(&buffer, dxbc->sections[i].data, dxbc->sections[i].size); + put_u32(&buffer, dxbc->sections[i].data.size); + bytecode_put_bytes(&buffer, dxbc->sections[i].data.code, dxbc->sections[i].data.size); } set_u32(&buffer, size_position, bytecode_get_size(&buffer));
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index 10bb1e3d..bb03098a 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -2510,6 +2510,6 @@ int hlsl_sm4_write(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_fun if (!(ret = ctx->result)) ret = dxbc_writer_write(&dxbc, out); for (i = 0; i < dxbc.section_count; ++i) - vkd3d_free((void *)dxbc.sections[i].data); + vkd3d_shader_free_shader_code(&dxbc.sections[i].data); return ret; } diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 6c5a1917..56100e46 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1311,8 +1311,7 @@ static inline void *vkd3d_find_struct_(const struct vkd3d_struct *chain, struct dxbc_writer_section { uint32_t tag; - const uint8_t *data; - size_t size; + struct vkd3d_shader_code data; };
#define DXBC_MAX_SECTION_COUNT 5
From: Henri Verbeet hverbeet@codeweavers.com
In preparation of exposing it in the public API. --- libs/vkd3d-shader/dxbc.c | 2 +- libs/vkd3d-shader/vkd3d_shader_private.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index a0be8d74..4b692337 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -33,7 +33,7 @@ void dxbc_writer_init(struct dxbc_writer *dxbc)
void dxbc_writer_add_section(struct dxbc_writer *dxbc, uint32_t tag, const void *data, size_t size) { - struct dxbc_writer_section *section; + struct vkd3d_shader_dxbc_section_desc *section;
assert(dxbc->section_count < ARRAY_SIZE(dxbc->sections));
diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 56100e46..eeb1626b 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1308,7 +1308,7 @@ static inline void *vkd3d_find_struct_(const struct vkd3d_struct *chain, #define TAG_SHEX VKD3D_MAKE_TAG('S', 'H', 'E', 'X') #define TAG_TEXT VKD3D_MAKE_TAG('T', 'E', 'X', 'T')
-struct dxbc_writer_section +struct vkd3d_shader_dxbc_section_desc { uint32_t tag; struct vkd3d_shader_code data; @@ -1319,7 +1319,7 @@ struct dxbc_writer_section struct dxbc_writer { unsigned int section_count; - struct dxbc_writer_section sections[DXBC_MAX_SECTION_COUNT]; + struct vkd3d_shader_dxbc_section_desc sections[DXBC_MAX_SECTION_COUNT]; };
void dxbc_writer_add_section(struct dxbc_writer *dxbc, uint32_t tag, const void *data, size_t size);
From: Henri Verbeet hverbeet@codeweavers.com
--- libs/vkd3d-shader/dxbc.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 4b692337..0042f538 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -1829,18 +1829,19 @@ static int parse_dxbc(const char *data, size_t data_size, return ret; }
-static int shader_parse_signature(DWORD tag, const char *data, DWORD data_size, +static int shader_parse_signature(const struct vkd3d_shader_dxbc_section_desc *section, struct vkd3d_shader_signature *s) { bool has_stream_index, has_min_precision; struct vkd3d_shader_signature_element *e; + const char *data = section->data.code; const char *ptr = data; unsigned int i; uint32_t count;
- if (!require_space(0, 2, sizeof(DWORD), data_size)) + if (!require_space(0, 2, sizeof(uint32_t), section->data.size)) { - WARN("Invalid data size %#x.\n", data_size); + WARN("Invalid data size %#zx.\n", section->data.size); return VKD3D_ERROR_INVALID_ARGUMENT; }
@@ -1849,9 +1850,9 @@ static int shader_parse_signature(DWORD tag, const char *data, DWORD data_size,
skip_dword_unknown(&ptr, 1); /* It seems to always be 0x00000008. */
- if (!require_space(ptr - data, count, 6 * sizeof(DWORD), data_size)) + if (!require_space(ptr - data, count, 6 * sizeof(uint32_t), section->data.size)) { - WARN("Invalid count %#x (data size %#x).\n", count, data_size); + WARN("Invalid count %#x (data size %#zx).\n", count, section->data.size); return VKD3D_ERROR_INVALID_ARGUMENT; }
@@ -1861,8 +1862,8 @@ static int shader_parse_signature(DWORD tag, const char *data, DWORD data_size, return VKD3D_ERROR_OUT_OF_MEMORY; }
- has_min_precision = tag == TAG_OSG1 || tag == TAG_PSG1 || tag == TAG_ISG1; - has_stream_index = tag == TAG_OSG5 || has_min_precision; + has_min_precision = section->tag == TAG_OSG1 || section->tag == TAG_PSG1 || section->tag == TAG_ISG1; + has_stream_index = section->tag == TAG_OSG5 || has_min_precision;
for (i = 0; i < count; ++i) { @@ -1874,9 +1875,9 @@ static int shader_parse_signature(DWORD tag, const char *data, DWORD data_size, e[i].stream_index = 0;
read_dword(&ptr, &name_offset); - if (!(e[i].semantic_name = shader_get_string(data, data_size, name_offset))) + if (!(e[i].semantic_name = shader_get_string(data, section->data.size, name_offset))) { - WARN("Invalid name offset %#x (data size %#x).\n", name_offset, data_size); + WARN("Invalid name offset %#x (data size %#zx).\n", name_offset, section->data.size); vkd3d_free(e); return VKD3D_ERROR_INVALID_ARGUMENT; } @@ -1887,7 +1888,7 @@ static int shader_parse_signature(DWORD tag, const char *data, DWORD data_size, read_dword(&ptr, &mask); e[i].mask = mask & 0xff; e[i].used_mask = (mask >> 8) & 0xff; - switch (tag) + switch (section->tag) { case TAG_OSGN: case TAG_OSG1: @@ -1917,6 +1918,7 @@ static int shader_parse_signature(DWORD tag, const char *data, DWORD data_size,
static int isgn_handler(const char *data, DWORD data_size, DWORD tag, void *ctx) { + struct vkd3d_shader_dxbc_section_desc section = {.tag = tag, .data = {.code = data, .size = data_size}}; struct vkd3d_shader_signature *is = ctx;
if (tag != TAG_ISGN) @@ -1927,7 +1929,7 @@ static int isgn_handler(const char *data, DWORD data_size, DWORD tag, void *ctx) FIXME("Multiple input signatures.\n"); vkd3d_shader_free_shader_signature(is); } - return shader_parse_signature(tag, data, data_size, is); + return shader_parse_signature(§ion, is); }
int shader_parse_input_signature(const void *dxbc, size_t dxbc_length, @@ -1944,6 +1946,7 @@ int shader_parse_input_signature(const void *dxbc, size_t dxbc_length,
static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *context) { + struct vkd3d_shader_dxbc_section_desc section = {.tag = tag, .data = {.code = data, .size = data_size}}; struct vkd3d_shader_desc *desc = context; int ret;
@@ -1956,7 +1959,7 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont FIXME("Multiple input signatures.\n"); break; } - if ((ret = shader_parse_signature(tag, data, data_size, &desc->input_signature)) < 0) + if ((ret = shader_parse_signature(§ion, &desc->input_signature)) < 0) return ret; break;
@@ -1968,7 +1971,7 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont FIXME("Multiple output signatures.\n"); break; } - if ((ret = shader_parse_signature(tag, data, data_size, &desc->output_signature)) < 0) + if ((ret = shader_parse_signature(§ion, &desc->output_signature)) < 0) return ret; break;
@@ -1979,7 +1982,7 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont FIXME("Multiple patch constant signatures.\n"); break; } - if ((ret = shader_parse_signature(tag, data, data_size, &desc->patch_constant_signature)) < 0) + if ((ret = shader_parse_signature(§ion, &desc->patch_constant_signature)) < 0) return ret; break;
From: Henri Verbeet hverbeet@codeweavers.com
--- libs/vkd3d-shader/dxbc.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 0042f538..8fd2fc7c 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -2451,21 +2451,21 @@ static int shader_parse_static_samplers(struct root_signature_parser_context *co return VKD3D_OK; }
-static int shader_parse_root_signature(const char *data, unsigned int data_size, +static int shader_parse_root_signature(const struct vkd3d_shader_code *data, struct vkd3d_shader_versioned_root_signature_desc *desc) { struct vkd3d_shader_root_signature_desc *v_1_0 = &desc->u.v_1_0; struct root_signature_parser_context context; unsigned int count, offset, version; - const char *ptr = data; + const char *ptr = data->code; int ret;
- context.data = data; - context.data_size = data_size; + context.data = data->code; + context.data_size = data->size;
- if (!require_space(0, 6, sizeof(DWORD), data_size)) + if (!require_space(0, 6, sizeof(uint32_t), data->size)) { - WARN("Invalid data size %#x.\n", data_size); + WARN("Invalid data size %#zx.\n", data->size); return VKD3D_ERROR_INVALID_ARGUMENT; }
@@ -2537,11 +2537,12 @@ static int shader_parse_root_signature(const char *data, unsigned int data_size, static int rts0_handler(const char *data, DWORD data_size, DWORD tag, void *context) { struct vkd3d_shader_versioned_root_signature_desc *desc = context; + struct vkd3d_shader_code code = {.code = data, .size = data_size};
if (tag != TAG_RTS0) return VKD3D_OK;
- return shader_parse_root_signature(data, data_size, desc); + return shader_parse_root_signature(&code, desc); }
int vkd3d_shader_parse_root_signature(const struct vkd3d_shader_code *dxbc,
From: Henri Verbeet hverbeet@codeweavers.com
--- libs/vkd3d-shader/dxbc.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 8fd2fc7c..50c73ec6 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -1727,7 +1727,7 @@ static const char *shader_get_string(const char *data, size_t data_size, DWORD o
static int parse_dxbc(const char *data, size_t data_size, struct vkd3d_shader_message_context *message_context, const char *source_name, - int (*chunk_handler)(const char *data, DWORD data_size, DWORD tag, void *ctx), void *ctx) + int (*section_handler)(const struct vkd3d_shader_dxbc_section_desc *section, void *ctx), void *ctx) { const struct vkd3d_shader_location location = {.source_name = source_name}; uint32_t checksum[4], calculated_checksum[4]; @@ -1792,6 +1792,7 @@ static int parse_dxbc(const char *data, size_t data_size,
for (i = 0; i < chunk_count; ++i) { + struct vkd3d_shader_dxbc_section_desc section; uint32_t chunk_tag, chunk_size; const char *chunk_ptr; uint32_t chunk_offset; @@ -1822,7 +1823,10 @@ static int parse_dxbc(const char *data, size_t data_size, return VKD3D_ERROR_INVALID_ARGUMENT; }
- if ((ret = chunk_handler(chunk_ptr, chunk_size, chunk_tag, ctx)) < 0) + section.tag = chunk_tag; + section.data.code = chunk_ptr; + section.data.size = chunk_size; + if ((ret = section_handler(§ion, ctx)) < 0) break; }
@@ -1916,12 +1920,11 @@ static int shader_parse_signature(const struct vkd3d_shader_dxbc_section_desc *s return VKD3D_OK; }
-static int isgn_handler(const char *data, DWORD data_size, DWORD tag, void *ctx) +static int isgn_handler(const struct vkd3d_shader_dxbc_section_desc *section, void *ctx) { - struct vkd3d_shader_dxbc_section_desc section = {.tag = tag, .data = {.code = data, .size = data_size}}; struct vkd3d_shader_signature *is = ctx;
- if (tag != TAG_ISGN) + if (section->tag != TAG_ISGN) return VKD3D_OK;
if (is->elements) @@ -1929,7 +1932,7 @@ static int isgn_handler(const char *data, DWORD data_size, DWORD tag, void *ctx) FIXME("Multiple input signatures.\n"); vkd3d_shader_free_shader_signature(is); } - return shader_parse_signature(§ion, is); + return shader_parse_signature(section, is); }
int shader_parse_input_signature(const void *dxbc, size_t dxbc_length, @@ -1944,13 +1947,12 @@ int shader_parse_input_signature(const void *dxbc, size_t dxbc_length, return ret; }
-static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *context) +static int shdr_handler(const struct vkd3d_shader_dxbc_section_desc *section, void *context) { - struct vkd3d_shader_dxbc_section_desc section = {.tag = tag, .data = {.code = data, .size = data_size}}; struct vkd3d_shader_desc *desc = context; int ret;
- switch (tag) + switch (section->tag) { case TAG_ISGN: case TAG_ISG1: @@ -1959,7 +1961,7 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont FIXME("Multiple input signatures.\n"); break; } - if ((ret = shader_parse_signature(§ion, &desc->input_signature)) < 0) + if ((ret = shader_parse_signature(section, &desc->input_signature)) < 0) return ret; break;
@@ -1971,7 +1973,7 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont FIXME("Multiple output signatures.\n"); break; } - if ((ret = shader_parse_signature(§ion, &desc->output_signature)) < 0) + if ((ret = shader_parse_signature(section, &desc->output_signature)) < 0) return ret; break;
@@ -1982,7 +1984,7 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont FIXME("Multiple patch constant signatures.\n"); break; } - if ((ret = shader_parse_signature(§ion, &desc->patch_constant_signature)) < 0) + if ((ret = shader_parse_signature(section, &desc->patch_constant_signature)) < 0) return ret; break;
@@ -1990,8 +1992,8 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont case TAG_SHEX: if (desc->byte_code) FIXME("Multiple shader code chunks.\n"); - desc->byte_code = (const uint32_t *)data; - desc->byte_code_size = data_size; + desc->byte_code = section->data.code; + desc->byte_code_size = section->data.size; break;
case TAG_AON9: @@ -2003,7 +2005,7 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont break;
default: - TRACE("Skipping chunk %#x.\n", tag); + TRACE("Skipping chunk %#x.\n", section->tag); break; }
@@ -2534,15 +2536,14 @@ static int shader_parse_root_signature(const struct vkd3d_shader_code *data, return VKD3D_OK; }
-static int rts0_handler(const char *data, DWORD data_size, DWORD tag, void *context) +static int rts0_handler(const struct vkd3d_shader_dxbc_section_desc *section, void *context) { struct vkd3d_shader_versioned_root_signature_desc *desc = context; - struct vkd3d_shader_code code = {.code = data, .size = data_size};
- if (tag != TAG_RTS0) + if (section->tag != TAG_RTS0) return VKD3D_OK;
- return shader_parse_root_signature(&code, desc); + return shader_parse_root_signature(§ion->data, desc); }
int vkd3d_shader_parse_root_signature(const struct vkd3d_shader_code *dxbc,
From: Henri Verbeet hverbeet@codeweavers.com
--- libs/vkd3d-shader/dxbc.c | 4 ++-- libs/vkd3d-shader/vkd3d_shader_main.c | 2 +- libs/vkd3d-shader/vkd3d_shader_private.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 50c73ec6..0da574c7 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -1935,13 +1935,13 @@ static int isgn_handler(const struct vkd3d_shader_dxbc_section_desc *section, vo return shader_parse_signature(section, is); }
-int shader_parse_input_signature(const void *dxbc, size_t dxbc_length, +int shader_parse_input_signature(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_signature *signature) { int ret;
memset(signature, 0, sizeof(*signature)); - if ((ret = parse_dxbc(dxbc, dxbc_length, message_context, NULL, isgn_handler, signature)) < 0) + if ((ret = parse_dxbc(dxbc->code, dxbc->size, message_context, NULL, isgn_handler, signature)) < 0) ERR("Failed to parse input signature.\n");
return ret; diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 1244fb48..5d9f2d7d 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -1402,7 +1402,7 @@ int vkd3d_shader_parse_input_signature(const struct vkd3d_shader_code *dxbc, *messages = NULL; vkd3d_shader_message_context_init(&message_context, VKD3D_SHADER_LOG_INFO);
- ret = shader_parse_input_signature(dxbc->code, dxbc->size, &message_context, signature); + ret = shader_parse_input_signature(dxbc, &message_context, signature); vkd3d_shader_message_context_trace_messages(&message_context); if (!vkd3d_shader_message_context_copy_messages(&message_context, messages)) ret = VKD3D_ERROR_OUT_OF_MEMORY; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index eeb1626b..e7d05848 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1125,7 +1125,7 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi
void free_shader_desc(struct vkd3d_shader_desc *desc);
-int shader_parse_input_signature(const void *dxbc, size_t dxbc_length, +int shader_parse_input_signature(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_signature *signature);
struct vkd3d_glsl_generator;
From: Henri Verbeet hverbeet@codeweavers.com
--- libs/vkd3d-shader/dxbc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 0da574c7..03ebc38f 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -2019,7 +2019,7 @@ void free_shader_desc(struct vkd3d_shader_desc *desc) vkd3d_shader_free_shader_signature(&desc->patch_constant_signature); }
-static int shader_extract_from_dxbc(const void *dxbc, size_t dxbc_length, +static int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, const char *source_name, struct vkd3d_shader_desc *desc) { int ret; @@ -2030,7 +2030,7 @@ static int shader_extract_from_dxbc(const void *dxbc, size_t dxbc_length, memset(&desc->output_signature, 0, sizeof(desc->output_signature)); memset(&desc->patch_constant_signature, 0, sizeof(desc->patch_constant_signature));
- ret = parse_dxbc(dxbc, dxbc_length, message_context, source_name, shdr_handler, desc); + ret = parse_dxbc(dxbc->code, dxbc->size, message_context, source_name, shdr_handler, desc); if (!desc->byte_code) ret = VKD3D_ERROR_INVALID_ARGUMENT;
@@ -2059,7 +2059,7 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi }
shader_desc = &sm4->p.shader_desc; - if ((ret = shader_extract_from_dxbc(compile_info->source.code, compile_info->source.size, + if ((ret = shader_extract_from_dxbc(&compile_info->source, message_context, compile_info->source_name, shader_desc)) < 0) { WARN("Failed to extract shader, vkd3d result %d.\n", ret);
From: Henri Verbeet hverbeet@codeweavers.com
--- libs/vkd3d-shader/dxbc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 03ebc38f..a3e79503 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -1725,12 +1725,14 @@ static const char *shader_get_string(const char *data, size_t data_size, DWORD o return data + offset; }
-static int parse_dxbc(const char *data, size_t data_size, +static int parse_dxbc(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, const char *source_name, int (*section_handler)(const struct vkd3d_shader_dxbc_section_desc *section, void *ctx), void *ctx) { const struct vkd3d_shader_location location = {.source_name = source_name}; uint32_t checksum[4], calculated_checksum[4]; + const char *data = dxbc->code; + size_t data_size = dxbc->size; const char *ptr = data; int ret = VKD3D_OK; uint32_t chunk_count; @@ -1941,7 +1943,7 @@ int shader_parse_input_signature(const struct vkd3d_shader_code *dxbc, int ret;
memset(signature, 0, sizeof(*signature)); - if ((ret = parse_dxbc(dxbc->code, dxbc->size, message_context, NULL, isgn_handler, signature)) < 0) + if ((ret = parse_dxbc(dxbc, message_context, NULL, isgn_handler, signature)) < 0) ERR("Failed to parse input signature.\n");
return ret; @@ -2030,7 +2032,7 @@ static int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, memset(&desc->output_signature, 0, sizeof(desc->output_signature)); memset(&desc->patch_constant_signature, 0, sizeof(desc->patch_constant_signature));
- ret = parse_dxbc(dxbc->code, dxbc->size, message_context, source_name, shdr_handler, desc); + ret = parse_dxbc(dxbc, message_context, source_name, shdr_handler, desc); if (!desc->byte_code) ret = VKD3D_ERROR_INVALID_ARGUMENT;
@@ -2559,7 +2561,7 @@ int vkd3d_shader_parse_root_signature(const struct vkd3d_shader_code *dxbc, *messages = NULL; vkd3d_shader_message_context_init(&message_context, VKD3D_SHADER_LOG_INFO);
- ret = parse_dxbc(dxbc->code, dxbc->size, &message_context, NULL, rts0_handler, root_signature); + ret = parse_dxbc(dxbc, &message_context, NULL, rts0_handler, root_signature); vkd3d_shader_message_context_trace_messages(&message_context); if (!vkd3d_shader_message_context_copy_messages(&message_context, messages)) ret = VKD3D_ERROR_OUT_OF_MEMORY;
From: Henri Verbeet hverbeet@codeweavers.com
--- libs/vkd3d-shader/dxbc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index a3e79503..72f67ef1 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -1725,7 +1725,7 @@ static const char *shader_get_string(const char *data, size_t data_size, DWORD o return data + offset; }
-static int parse_dxbc(const struct vkd3d_shader_code *dxbc, +static int for_each_dxbc_section(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, const char *source_name, int (*section_handler)(const struct vkd3d_shader_dxbc_section_desc *section, void *ctx), void *ctx) { @@ -1943,7 +1943,7 @@ int shader_parse_input_signature(const struct vkd3d_shader_code *dxbc, int ret;
memset(signature, 0, sizeof(*signature)); - if ((ret = parse_dxbc(dxbc, message_context, NULL, isgn_handler, signature)) < 0) + if ((ret = for_each_dxbc_section(dxbc, message_context, NULL, isgn_handler, signature)) < 0) ERR("Failed to parse input signature.\n");
return ret; @@ -2032,7 +2032,7 @@ static int shader_extract_from_dxbc(const struct vkd3d_shader_code *dxbc, memset(&desc->output_signature, 0, sizeof(desc->output_signature)); memset(&desc->patch_constant_signature, 0, sizeof(desc->patch_constant_signature));
- ret = parse_dxbc(dxbc, message_context, source_name, shdr_handler, desc); + ret = for_each_dxbc_section(dxbc, message_context, source_name, shdr_handler, desc); if (!desc->byte_code) ret = VKD3D_ERROR_INVALID_ARGUMENT;
@@ -2561,7 +2561,7 @@ int vkd3d_shader_parse_root_signature(const struct vkd3d_shader_code *dxbc, *messages = NULL; vkd3d_shader_message_context_init(&message_context, VKD3D_SHADER_LOG_INFO);
- ret = parse_dxbc(dxbc, &message_context, NULL, rts0_handler, root_signature); + ret = for_each_dxbc_section(dxbc, &message_context, NULL, rts0_handler, root_signature); vkd3d_shader_message_context_trace_messages(&message_context); if (!vkd3d_shader_message_context_copy_messages(&message_context, messages)) ret = VKD3D_ERROR_OUT_OF_MEMORY;
From: Henri Verbeet hverbeet@codeweavers.com
--- include/vkd3d_shader.h | 83 ++++++++++++++++++++++ libs/vkd3d-shader/dxbc.c | 89 +++++++++++++++++++++--- libs/vkd3d-shader/vkd3d_shader.map | 2 + libs/vkd3d-shader/vkd3d_shader_private.h | 7 +- 4 files changed, 165 insertions(+), 16 deletions(-)
diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 86ffb9d5..f4dc822d 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -1472,6 +1472,42 @@ enum vkd3d_shader_swizzle_component VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_SWIZZLE_COMPONENT), };
+/** + * A description of a DXBC section. + * + * \since 1.7 + */ +struct vkd3d_shader_dxbc_section_desc +{ + /** The section tag. */ + uint32_t tag; + /** The contents of the section. */ + struct vkd3d_shader_code data; +}; + +/** + * A description of a DXBC blob, as returned by vkd3d_shader_parse_dxbc(). + * + * \since 1.7 + */ +struct vkd3d_shader_dxbc_desc +{ + /** The DXBC tag. This should always be "DXBC". */ + uint32_t tag; + /** A checksum of the DXBC contents. */ + uint32_t checksum[4]; + /** + * The DXBC version. The only supported DXBC version in this version of + * vkd3d-shader is 1. */ + uint32_t version; + /** The total size of the DXBC blob. */ + uint32_t size; + /** The number of sections contained in the DXBC. */ + uint32_t section_count; + /** Descriptions of the sections contained in the DXBC. */ + struct vkd3d_shader_dxbc_section_desc *sections; +}; + /** * A mask selecting one component from a vkd3d-shader swizzle. The component has * type \ref vkd3d_shader_swizzle_component. @@ -1865,6 +1901,47 @@ VKD3D_SHADER_API int vkd3d_shader_preprocess(const struct vkd3d_shader_compile_i */ VKD3D_SHADER_API void vkd3d_shader_set_log_callback(PFN_vkd3d_log callback);
+/** + * Free the contents of a vkd3d_shader_dxbc_desc structure allocated by + * another vkd3d-shader function, such as vkd3d_shader_parse_dxbc(). + * + * This function may free the \ref vkd3d_shader_dxbc_desc.sections member, but + * does not free the structure itself. + * + * \param dxbc The vkd3d_shader_dxbc_desc structure to free. + * + * \since 1.7 + */ +VKD3D_SHADER_API void vkd3d_shader_free_dxbc(struct vkd3d_shader_dxbc_desc *dxbc); + +/** + * Parse a DXBC blob contained in a vkd3d_shader_code structure. + * + * \param dxbc A vkd3d_shader_code structure containing the DXBC blob to parse. + * + * \param flags A set of flags modifying the behaviour of the function. No + * flags are defined for this version of vkd3d-shader, and this parameter + * should be set to 0. + * + * \param desc A vkd3d_shader_dxbc_desc structure describing the contents of + * the DXBC blob. Its vkd3d_shader_dxbc_section_desc structures will contain + * pointers into the input blob; its contents are only valid while the input + * blob is valid. The contents of this structure should be freed with + * vkd3d_shader_free_dxbc() when no longer needed. + * + * \param messages Optional output location for error or informational messages + * produced by the parser. + * \n + * This parameter behaves identically to the \a messages parameter of + * vkd3d_shader_compile(). + * + * \return A member of \ref vkd3d_result. + * + * \since 1.7 + */ +VKD3D_SHADER_API int vkd3d_shader_parse_dxbc(const struct vkd3d_shader_code *dxbc, + uint32_t flags, struct vkd3d_shader_dxbc_desc *desc, char **messages); + #endif /* VKD3D_SHADER_NO_PROTOTYPES */
/** Type of vkd3d_shader_get_version(). */ @@ -1921,6 +1998,12 @@ typedef void (*PFN_vkd3d_shader_preprocess)(struct vkd3d_shader_compile_info *co /** Type of vkd3d_shader_set_log_callback(). \since 1.4 */ typedef void (*PFN_vkd3d_shader_set_log_callback)(PFN_vkd3d_log callback);
+/** Type of vkd3d_shader_free_dxbc(). \since 1.7 */ +typedef void (*PFN_vkd3d_shader_free_dxbc)(struct vkd3d_shader_dxbc_desc *dxbc); +/** Type of vkd3d_shader_parse_dxbc(). \since 1.7 */ +typedef int (*PFN_vkd3d_shader_parse_dxbc)(const struct vkd3d_shader_code *dxbc, + uint32_t flags, struct vkd3d_shader_dxbc_desc *desc, char **messages); + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 72f67ef1..8dfb9408 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -1725,20 +1725,19 @@ static const char *shader_get_string(const char *data, size_t data_size, DWORD o return data + offset; }
-static int for_each_dxbc_section(const struct vkd3d_shader_code *dxbc, - struct vkd3d_shader_message_context *message_context, const char *source_name, - int (*section_handler)(const struct vkd3d_shader_dxbc_section_desc *section, void *ctx), void *ctx) +static int parse_dxbc(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, + const char *source_name, struct vkd3d_shader_dxbc_desc *desc) { const struct vkd3d_shader_location location = {.source_name = source_name}; + struct vkd3d_shader_dxbc_section_desc *sections, *section; uint32_t checksum[4], calculated_checksum[4]; const char *data = dxbc->code; size_t data_size = dxbc->size; const char *ptr = data; - int ret = VKD3D_OK; uint32_t chunk_count; uint32_t total_size; - unsigned int i; uint32_t version; + unsigned int i; uint32_t tag;
if (data_size < VKD3D_DXBC_HEADER_SIZE) @@ -1792,9 +1791,14 @@ static int for_each_dxbc_section(const struct vkd3d_shader_code *dxbc, read_dword(&ptr, &chunk_count); TRACE("chunk count: %#x\n", chunk_count);
+ if (!(sections = vkd3d_calloc(chunk_count, sizeof(*sections)))) + { + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXBC_OUT_OF_MEMORY, "Out of memory."); + return VKD3D_ERROR_OUT_OF_MEMORY; + } + for (i = 0; i < chunk_count; ++i) { - struct vkd3d_shader_dxbc_section_desc section; uint32_t chunk_tag, chunk_size; const char *chunk_ptr; uint32_t chunk_offset; @@ -1807,6 +1811,7 @@ static int for_each_dxbc_section(const struct vkd3d_shader_code *dxbc, WARN("Invalid chunk offset %#x (data size %zu).\n", chunk_offset, data_size); vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXBC_INVALID_CHUNK_OFFSET, "DXBC chunk %u has invalid offset %#x (data size %#zx).", i, chunk_offset, data_size); + vkd3d_free(sections); return VKD3D_ERROR_INVALID_ARGUMENT; }
@@ -1822,16 +1827,80 @@ static int for_each_dxbc_section(const struct vkd3d_shader_code *dxbc, vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXBC_INVALID_CHUNK_SIZE, "DXBC chunk %u has invalid size %#x (data size %#zx, chunk offset %#x).", i, chunk_offset, data_size, chunk_offset); + vkd3d_free(sections); return VKD3D_ERROR_INVALID_ARGUMENT; }
- section.tag = chunk_tag; - section.data.code = chunk_ptr; - section.data.size = chunk_size; - if ((ret = section_handler(§ion, ctx)) < 0) + section = §ions[i]; + section->tag = chunk_tag; + section->data.code = chunk_ptr; + section->data.size = chunk_size; + } + + desc->tag = tag; + memcpy(desc->checksum, checksum, sizeof(checksum)); + desc->version = version; + desc->size = total_size; + desc->section_count = chunk_count; + desc->sections = sections; + + return VKD3D_OK; +} + +void vkd3d_shader_free_dxbc(struct vkd3d_shader_dxbc_desc *dxbc) +{ + TRACE("dxbc %p.\n", dxbc); + + vkd3d_free(dxbc->sections); +} + +static int for_each_dxbc_section(const struct vkd3d_shader_code *dxbc, + struct vkd3d_shader_message_context *message_context, const char *source_name, + int (*section_handler)(const struct vkd3d_shader_dxbc_section_desc *section, void *ctx), void *ctx) +{ + struct vkd3d_shader_dxbc_desc desc; + unsigned int i; + int ret; + + if ((ret = parse_dxbc(dxbc, message_context, source_name, &desc)) < 0) + return ret; + + for (i = 0; i < desc.section_count; ++i) + { + if ((ret = section_handler(&desc.sections[i], ctx)) < 0) break; }
+ vkd3d_shader_free_dxbc(&desc); + + return ret; +} + +int vkd3d_shader_parse_dxbc(const struct vkd3d_shader_code *dxbc, + uint32_t flags, struct vkd3d_shader_dxbc_desc *desc, char **messages) +{ + struct vkd3d_shader_message_context message_context; + int ret; + + TRACE("dxbc {%p, %zu}, flags %#x, desc %p, messages %p.\n", dxbc->code, dxbc->size, flags, desc, messages); + + if (messages) + *messages = NULL; + vkd3d_shader_message_context_init(&message_context, VKD3D_SHADER_LOG_INFO); + + ret = parse_dxbc(dxbc, &message_context, NULL, desc); + + vkd3d_shader_message_context_trace_messages(&message_context); + if (!vkd3d_shader_message_context_copy_messages(&message_context, messages) && ret >= 0) + { + vkd3d_shader_free_dxbc(desc); + ret = VKD3D_ERROR_OUT_OF_MEMORY; + } + vkd3d_shader_message_context_cleanup(&message_context); + + if (ret < 0) + memset(desc, 0, sizeof(*desc)); + return ret; }
diff --git a/libs/vkd3d-shader/vkd3d_shader.map b/libs/vkd3d-shader/vkd3d_shader.map index cac96972..096cb848 100644 --- a/libs/vkd3d-shader/vkd3d_shader.map +++ b/libs/vkd3d-shader/vkd3d_shader.map @@ -4,6 +4,7 @@ global: vkd3d_shader_compile; vkd3d_shader_convert_root_signature; vkd3d_shader_find_signature_element; + vkd3d_shader_free_dxbc; vkd3d_shader_free_messages; vkd3d_shader_free_root_signature; vkd3d_shader_free_scan_descriptor_info; @@ -12,6 +13,7 @@ global: vkd3d_shader_get_supported_source_types; vkd3d_shader_get_supported_target_types; vkd3d_shader_get_version; + vkd3d_shader_parse_dxbc; vkd3d_shader_parse_input_signature; vkd3d_shader_parse_root_signature; vkd3d_shader_preprocess; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index e7d05848..78a48e55 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -68,6 +68,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_DXBC_INVALID_VERSION = 4, VKD3D_SHADER_ERROR_DXBC_INVALID_CHUNK_OFFSET = 5, VKD3D_SHADER_ERROR_DXBC_INVALID_CHUNK_SIZE = 6, + VKD3D_SHADER_ERROR_DXBC_OUT_OF_MEMORY = 7,
VKD3D_SHADER_ERROR_TPF_MISMATCHED_CF = 1000, VKD3D_SHADER_ERROR_TPF_INVALID_REGISTER_RANGE = 1001, @@ -1308,12 +1309,6 @@ static inline void *vkd3d_find_struct_(const struct vkd3d_struct *chain, #define TAG_SHEX VKD3D_MAKE_TAG('S', 'H', 'E', 'X') #define TAG_TEXT VKD3D_MAKE_TAG('T', 'E', 'X', 'T')
-struct vkd3d_shader_dxbc_section_desc -{ - uint32_t tag; - struct vkd3d_shader_code data; -}; - #define DXBC_MAX_SECTION_COUNT 5
struct dxbc_writer
From: Henri Verbeet hverbeet@codeweavers.com
--- include/vkd3d_shader.h | 31 ++++++++++++++++++++++++++++++ libs/vkd3d-shader/dxbc.c | 29 +++++++++++++++++++--------- libs/vkd3d-shader/vkd3d_shader.map | 1 + 3 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index f4dc822d..5eb0d911 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -1942,6 +1942,34 @@ VKD3D_SHADER_API void vkd3d_shader_free_dxbc(struct vkd3d_shader_dxbc_desc *dxbc VKD3D_SHADER_API int vkd3d_shader_parse_dxbc(const struct vkd3d_shader_code *dxbc, uint32_t flags, struct vkd3d_shader_dxbc_desc *desc, char **messages);
+/** + * Serialise a DXBC description into a blob stored in a vkd3d_shader_code + * structure. + * + * \param section_count The number of DXBC sections to serialise. + * + * \param sections An array of vkd3d_shader_dxbc_section_desc structures + * to serialise. + * + * \param dxbc A pointer to a vkd3d_shader_code structure in which the + * serialised blob will be stored. + * \n + * The output blob is allocated by vkd3d-shader and should be freed with + * vkd3d_shader_free_shader_code() when no longer needed. + * + * \param messages Optional output location for error or informational messages + * produced by the serialiser. + * \n + * This parameter behaves identically to the \a messages parameter of + * vkd3d_shader_compile(). + * + * \return A member of \ref vkd3d_result. + * + * \since 1.7 + */ +VKD3D_SHADER_API int vkd3d_shader_serialize_dxbc(size_t section_count, + const struct vkd3d_shader_dxbc_section_desc *sections, struct vkd3d_shader_code *dxbc, char **messages); + #endif /* VKD3D_SHADER_NO_PROTOTYPES */
/** Type of vkd3d_shader_get_version(). */ @@ -2003,6 +2031,9 @@ typedef void (*PFN_vkd3d_shader_free_dxbc)(struct vkd3d_shader_dxbc_desc *dxbc); /** Type of vkd3d_shader_parse_dxbc(). \since 1.7 */ typedef int (*PFN_vkd3d_shader_parse_dxbc)(const struct vkd3d_shader_code *dxbc, uint32_t flags, struct vkd3d_shader_dxbc_desc *desc, char **messages); +/** Type of vkd3d_shader_serialize_dxbc(). \since 1.7 */ +typedef int (*PFN_vkd3d_shader_serialize_dxbc)(size_t section_count, + const struct vkd3d_shader_dxbc_section_desc *sections, struct vkd3d_shader_code *dxbc, char **messages);
#ifdef __cplusplus } diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 8dfb9408..d8eef8a5 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -43,12 +43,18 @@ void dxbc_writer_add_section(struct dxbc_writer *dxbc, uint32_t tag, const void section->data.size = size; }
-int dxbc_writer_write(struct dxbc_writer *dxbc, struct vkd3d_shader_code *out) +int vkd3d_shader_serialize_dxbc(size_t section_count, const struct vkd3d_shader_dxbc_section_desc *sections, + struct vkd3d_shader_code *dxbc, char **messages) { size_t size_position, offsets_position, checksum_position, i; struct vkd3d_bytecode_buffer buffer = {0}; uint32_t checksum[4];
+ TRACE("section_count %zu, sections %p, dxbc %p, messages %p.\n", section_count, sections, dxbc, messages); + + if (messages) + *messages = NULL; + put_u32(&buffer, TAG_DXBC);
checksum_position = bytecode_get_size(&buffer); @@ -57,18 +63,18 @@ int dxbc_writer_write(struct dxbc_writer *dxbc, struct vkd3d_shader_code *out)
put_u32(&buffer, 1); /* version */ size_position = put_u32(&buffer, 0); - put_u32(&buffer, dxbc->section_count); + put_u32(&buffer, section_count);
offsets_position = bytecode_get_size(&buffer); - for (i = 0; i < dxbc->section_count; ++i) + for (i = 0; i < section_count; ++i) put_u32(&buffer, 0);
- for (i = 0; i < dxbc->section_count; ++i) + for (i = 0; i < section_count; ++i) { set_u32(&buffer, offsets_position + i * sizeof(uint32_t), bytecode_get_size(&buffer)); - put_u32(&buffer, dxbc->sections[i].tag); - put_u32(&buffer, dxbc->sections[i].data.size); - bytecode_put_bytes(&buffer, dxbc->sections[i].data.code, dxbc->sections[i].data.size); + put_u32(&buffer, sections[i].tag); + put_u32(&buffer, sections[i].data.size); + bytecode_put_bytes(&buffer, sections[i].data.code, sections[i].data.size); } set_u32(&buffer, size_position, bytecode_get_size(&buffer));
@@ -78,12 +84,17 @@ int dxbc_writer_write(struct dxbc_writer *dxbc, struct vkd3d_shader_code *out)
if (!buffer.status) { - out->code = buffer.data; - out->size = buffer.size; + dxbc->code = buffer.data; + dxbc->size = buffer.size; } return buffer.status; }
+int dxbc_writer_write(struct dxbc_writer *dxbc, struct vkd3d_shader_code *out) +{ + return vkd3d_shader_serialize_dxbc(dxbc->section_count, dxbc->sections, out, NULL); +} + struct vkd3d_shader_src_param_entry { struct list entry; diff --git a/libs/vkd3d-shader/vkd3d_shader.map b/libs/vkd3d-shader/vkd3d_shader.map index 096cb848..6afc526a 100644 --- a/libs/vkd3d-shader/vkd3d_shader.map +++ b/libs/vkd3d-shader/vkd3d_shader.map @@ -18,6 +18,7 @@ global: vkd3d_shader_parse_root_signature; vkd3d_shader_preprocess; vkd3d_shader_scan; + vkd3d_shader_serialize_dxbc; vkd3d_shader_serialize_root_signature; vkd3d_shader_set_log_callback;
It's not terribly useful, no. I didn't see the harm in including it though, mostly with the idea that it would allow the (largely hypothetical at this point) vkd3d-blob program to dump the actual content of the blob instead of making something up.
We currently check for TAG_DXBC and version 1 anyway, though. I guess that could change, but...
And while a version 2 doesn't seem terribly likely at this point, I could certainly imagine a version 2 that e.g. just has some extra fields or e.g. 64-bit offsets and sizes that should be representable with the current structure, even if it wouldn't include everything contained in the blob. That said, I don't mind dropping these if you object to them.
I suppose. I don't feel strongly about the version field, I just figured I'd put the question up at least.
The DXBC tag field does seem a little silly though, if only because a DXBC shader that doesn't begin with DXBC magic sounds to me like "not a DXBC shader". (On the other hand, that kind of thing has happened before, so maybe that's a specious objection...)
Right, I'm mostly thinking about a scenario like e.g. OSGN/OSG1/OSG5 or SHDR/SHEX.
Should we go farther than "may" and specify "will"? In order to be potentially more useful to the API consumer, and generally follow the principle of "be conservative in what you provide".
I had "will" originally and weakened it. I don't mind changing it back, although I'm not sure that's a guarantee we're necessarily willing to commit to?
I don't think it'd cost us anything to commit to it, at least.
I've pushed a v2, how's that?
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
+struct vkd3d_shader_dxbc_section_desc +{
- /** The section tag. */
- uint32_t tag;
- /** The contents of the section. */
- struct vkd3d_shader_code data;
+};
+/**
- A description of a DXBC blob, as returned by vkd3d_shader_parse_dxbc().
- \since 1.7
- */
+struct vkd3d_shader_dxbc_desc +{
- /** The DXBC tag. This should always be "DXBC". */
Should we probably make `VKD3D_MAKE_TAG()` public too? And possibly `TAG_DXBC` too, after renaming? Some users might find not completely obvious how to assign the value `"DXBC"` to a `uint32_t` variable. And while a user trying `desc.tag = "DXBC";` will probably find other problems in their way to a working program, it wouldn't be unpolite to give them some more useful tool.
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
*/ VKD3D_SHADER_API void vkd3d_shader_set_log_callback(PFN_vkd3d_log callback);
+/**
- Free the contents of a vkd3d_shader_dxbc_desc structure allocated by
- another vkd3d-shader function, such as vkd3d_shader_parse_dxbc().
- This function may free the \ref vkd3d_shader_dxbc_desc.sections member, but
- does not free the structure itself.
- \param dxbc The vkd3d_shader_dxbc_desc structure to free.
- \since 1.7
- */
+VKD3D_SHADER_API void vkd3d_shader_free_dxbc(struct vkd3d_shader_dxbc_desc *dxbc);
My understanding is that `free` usually means that the pointer itself is freed. Maybe this should be renamed to `destroy`?
Though I notice that this problem is also with `vkd3d_shader_free_shader_code()`, so maybe that's a ship that has already sailed.
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
- \param flags A set of flags modifying the behaviour of the function. No
- flags are defined for this version of vkd3d-shader, and this parameter
- should be set to 0.
- \param desc A vkd3d_shader_dxbc_desc structure describing the contents of
- the DXBC blob. Its vkd3d_shader_dxbc_section_desc structures will contain
- pointers into the input blob; its contents are only valid while the input
- blob is valid. The contents of this structure should be freed with
- vkd3d_shader_free_dxbc() when no longer needed.
- \param messages Optional output location for error or informational messages
- produced by the parser.
- \n
- This parameter behaves identically to the \a messages parameter of
- vkd3d_shader_compile().
I don't like this too much. It behaves identically in what respect? Being NULL-terminated and UTF-8? Being regulated by `log_level`? I would just repeat the sentences that apply. It's a bit more verbose, but clearer, and you don't have to scroll to `vkd3d_shader_compile()` to understand the remark.
I do, actually. The idea was to provide the ability to disable validation of the checksum, and possibly sizes and offsets. Again mostly for the benefit of a hypothetical vkd3d-blob program.
In that case it would make sense to also provide a field for returning the expected checksum too, so the program can include it in the diagnostic message.
On Wed Feb 22 15:08:30 2023 +0000, Henri Verbeet wrote:
It's not terribly useful, no. I didn't see the harm in including it
though, mostly with the idea that it would allow the (largely hypothetical at this point) vkd3d-blob program to dump the actual content of the blob instead of making something up.
We currently check for TAG_DXBC and version 1 anyway, though. I guess
that could change, but...
And while a version 2 doesn't seem terribly likely at this point, I
could certainly imagine a version 2 that e.g. just has some extra fields or e.g. 64-bit offsets and sizes that should be representable with the current structure, even if it wouldn't include everything contained in the blob. That said, I don't mind dropping these if you object to them.
I suppose. I don't feel strongly about the version field, I just
figured I'd put the question up at least.
The DXBC tag field does seem a little silly though, if only because a
DXBC shader that doesn't begin with DXBC magic sounds to me like "not a DXBC shader". (On the other hand, that kind of thing has happened before, so maybe that's a specious objection...) Right, I'm mostly thinking about a scenario like e.g. OSGN/OSG1/OSG5 or SHDR/SHEX.
Should we go farther than "may" and specify "will"? In order to be
potentially more useful to the API consumer, and generally follow the principle of "be conservative in what you provide".
I had "will" originally and weakened it. I don't mind changing it
back, although I'm not sure that's a guarantee we're necessarily willing to commit to?
I don't think it'd cost us anything to commit to it, at least.
I've pushed a v2, how's that?
FWIF, I agree it makes sense to include the tag and version fields.
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
- \param section_count The number of DXBC sections to serialise.
- \param sections An array of vkd3d_shader_dxbc_section_desc structures
- to serialise.
- \param dxbc A pointer to a vkd3d_shader_code structure in which the
- serialised blob will be stored.
- \n
- The output blob is allocated by vkd3d-shader and should be freed with
- vkd3d_shader_free_shader_code() when no longer needed.
- \param messages Optional output location for error or informational messages
- produced by the serialiser.
- \n
- This parameter behaves identically to the \a messages parameter of
- vkd3d_shader_compile().
Same as the previous patch.
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
- \n
- The output blob is allocated by vkd3d-shader and should be freed with
- vkd3d_shader_free_shader_code() when no longer needed.
- \param messages Optional output location for error or informational messages
- produced by the serialiser.
- \n
- This parameter behaves identically to the \a messages parameter of
- vkd3d_shader_compile().
- \return A member of \ref vkd3d_result.
- \since 1.7
- */
+VKD3D_SHADER_API int vkd3d_shader_serialize_dxbc(size_t section_count,
const struct vkd3d_shader_dxbc_section_desc *sections, struct vkd3d_shader_code *dxbc, char **messages);
I am tempted to say we might want to allow for specifying the tag and version here too. Or rather take a `struct vkd3d_shader_dxbc_desc` as input and ignore size and checksum (or even allow for flags to use them even if they're invalid).
Not that I have any real usage in mind, of course, so that should probably be ignored. I happen to like symmetry, and I don't see reason for having the parsing interface allowing for the hypothesis that a format change might in theory happen, and the serializing interface not doing the same.
As a bottom line, I don't think a few tests would be harmful...
Should we probably make `VKD3D_MAKE_TAG()` public too? And possibly `TAG_DXBC` too, after renaming?
An argument could probably be made for VKD3D_MAKE_TAG or something similar; I'm not as convinced about TAG_DXBC and all the others. Note that Windows already has MAKEFOURCC though. Personally I'd also happily defer this to a future release unless someone has strong feelings about it.
My understanding is that `free` usually means that the pointer itself is freed. Maybe this should be renamed to `destroy`?
Though I notice that this problem is also with `vkd3d_shader_free_shader_code()`, so maybe that's a ship that has already sailed.
It's a fair point, but we already have vkd3d_shader_free_messages(), vkd3d_shader_free_root_signature(), vkd3d_shader_free_scan_descriptor_info(), vkd3d_shader_free_shader_code(), and vkd3d_shader_free_shader_signature() with the same behaviour. I think that at this point doing something different for vkd3d_shader_free_dxbc() would just make things more confusing.
I don't like this too much. It behaves identically in what respect? Being NULL-terminated and UTF-8? Being regulated by `log_level`? I would just repeat the sentences that apply. It's a bit more verbose, but clearer, and you don't have to scroll to `vkd3d_shader_compile()` to understand the remark.
It's mostly just consistent with the existing documentation for functions taking a "messages" argument. Improvements welcome, of course, but this MR doesn't seem like the place for it.
I am tempted to say we might want to allow for specifying the tag and version here too. Or rather take a `struct vkd3d_shader_dxbc_desc` as input and ignore size and checksum (or even allow for flags to use them even if they're invalid).
Not that I have any real usage in mind, of course, so that should probably be ignored. I happen to like symmetry, and I don't see reason for having the parsing interface allowing for the hypothesis that a format change might in theory happen, and the serializing interface not doing the same.
I could be convinced, but I had this taking a vkd3d_shader_dxbc_desc structure originally, and it was mostly just an inconvenience when implementing D3DGetBlobPart() and D3DStripShader() on top of vkd3d_shader_serialize_dxbc(). It also implies adding validation for the tag and version. How about I change it if you can convince Zeb?
As a bottom line, I don't think a few tests would be harmful...
Fair enough. Between future vkd3d-utils and d3dcompiler usage and the existing usage of most of this code in vkd3d and vkd3d-shader I figured we had enough coverage, but I'll add some vkd3d_shader_api.c tests; either in a potential v3 or in a follow-up MR.
I don't like this too much. It behaves identically in what respect? Being NULL-terminated and UTF-8? Being regulated by `log_level`? I would just repeat the sentences that apply. It's a bit more verbose, but clearer, and you don't have to scroll to `vkd3d_shader_compile()` to understand the remark.
Yeah, that one's my fault for doing it with other functions...
I am tempted to say we might want to allow for specifying the tag and version here too. Or rather take a `struct vkd3d_shader_dxbc_desc` as input and ignore size and checksum (or even allow for flags to use them even if they're invalid).
Not that I have any real usage in mind, of course, so that should probably be ignored. I happen to like symmetry, and I don't see reason for having the parsing interface allowing for the hypothesis that a format change might in theory happen, and the serializing interface not doing the same.
I could be convinced, but I had this taking a vkd3d_shader_dxbc_desc structure originally, and it was mostly just an inconvenience when implementing D3DGetBlobPart() and D3DStripShader() on top of vkd3d_shader_serialize_dxbc(). It also implies adding validation for the tag and version. How about I change it if you can convince Zeb?
Heh. My position wrt issues like this, as a consumer of many APIs of different styles, is that the easiest APIs to work with are the ones with relatively simple functions, and also that rerolling an API, even rerolling it several times, doesn't actually make things any harder to work with. (Nor to implement, I think). Especially when all you're rerolling is the function signature itself. So if it were up to me, I'd just remove the tag and version from both sides.
But that said, I'm not going to bikeshed this very hard, since it's a pretty minor concern. I just wanted to make sure that some alternatives were thought of (or, more saliently, discussed in a public place; I do quite trust Henri to think of design questions like that.)
Actually, looking at this again, the phrasing on the documentation for the tag and version is a little odd. The use of "should" and "only supported" suggests that they are meant to be filled by the user, when in fact they're currently only output parameters. I'm not sure what's better phrasing for this, though.
An argument could probably be made for VKD3D_MAKE_TAG or something similar; I'm not as convinced about TAG_DXBC and all the others. Note that Windows already has MAKEFOURCC though. Personally I'd also happily defer this to a future release unless someone has strong feelings about it.
My point of view is that in general we expect (or hope?) vkd3d to be helpful also for people not using it on top of Wine/Windows. So what is available in Windows doesn't count a lot. I think that having `VKD3D_MAKE_TAG()` at least would make sense.
It's a fair point, but we already have vkd3d_shader_free_messages(), vkd3d_shader_free_root_signature(), vkd3d_shader_free_scan_descriptor_info(), vkd3d_shader_free_shader_code(), and vkd3d_shader_free_shader_signature() with the same behaviour. I think that at this point doing something different for vkd3d_shader_free_dxbc() would just make things more confusing.
Yeah, right.
I could be convinced, but I had this taking a vkd3d_shader_dxbc_desc structure originally, and it was mostly just an inconvenience when implementing D3DGetBlobPart() and D3DStripShader() on top of vkd3d_shader_serialize_dxbc(). It also implies adding validation for the tag and version. How about I change it if you can convince Zeb?
It seems I couldn't. And after all what she says about possibly rerolling the API in the future makes sense.
On Thu Feb 23 11:33:11 2023 +0000, Zebediah Figura wrote:
Actually, looking at this again, the phrasing on the documentation for the tag and version is a little odd. The use of "should" and "only supported" suggests that they are meant to be filled by the user, when in fact they're currently only output parameters. I'm not sure what's better phrasing for this, though.
One possibility is to move them in the docstring for `vkd3d_shader_parse_dxbc()`.
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura (＠zfigura)