vkd3d-shader: Add a stub constant buffers tracing for fx -> text path.
Signed-off-by: Nikolay Sivov nsivov@codeweavers.com
Merge request reports
Activity
+ /** + * Binary format used by Direct3D 9/10.x/11 effects. + * For fx_4_x binaries both raw sections and dxbc containers are accepted. \since 1.14 + */ + VKD3D_SHADER_SOURCE_FX,
Do we have to accept DXBC containers here? I notice the MR doesn't actually implement that functionality. If this is mainly a matter of convenience for vkd3d-compiler, I think I'd prefer vkd3d-compiler handling that by itself.
+struct fx_parser +{ + const char *ptr, *start, *end; + struct vkd3d_shader_message_context *message_context; + struct vkd3d_string_buffer buffer; + struct + { + const char *ptr; + const char *end; + uint32_t size; + } unstructured; + uint32_t buffer_count; + bool failed; +};
It's perhaps largely cosmetic, but "uint8_t" seems more appropriate than "char".
+static int fx_2_parse(struct fx_parser *parser) +{ + WARN("Parsing of fx_2_0 binaries is not implemented.\n"); + + return -1; +}
Using WARN here isn't wrong, but it seems slightly nicer to use vkd3d_shader_error() to return a message to the user. That would also mean the "message_context" field of struct fx_parser would actually get used.
+static int fx_4_parse(struct fx_parser *parser) +{ + struct fx_4_header + { + uint32_t version; + uint32_t buffer_count; + uint32_t numeric_variable_count; + uint32_t object_variable_count; + uint32_t shared_buffer_count; + uint32_t shared_numeric_variable_count; + uint32_t shared_object_count; + uint32_t technique_count; + uint32_t unstructured_size; + uint32_t string_count; + uint32_t texture_count; + uint32_t depth_stencil_state_count; + uint32_t blend_state_count; + uint32_t rasterizer_state_count; + uint32_t sampler_state_count; + uint32_t rtv_count; + uint32_t dsv_count; + uint32_t shader_count; + uint32_t inline_shader_count; + } header; + + fx_parser_read_u32s(parser, &header, sizeof(header)); + parser->buffer_count = header.buffer_count; + + parser->unstructured.ptr = parser->ptr; + parser->unstructured.end = parser->ptr + header.unstructured_size; + parser->unstructured.size = header.unstructured_size; + + if (parser->unstructured.end > parser->end) + { + parser->failed = true; + return -1; + }
The addition of "parser->ptr" and "header.unstructured_size" can overflow in principle, although it's perhaps harder to make that actually happen on 64-bit builds. It seems more robust to write the check as "if (parser->end - parser->unstructured.ptr < header.unstructured_size)", or something similar.
+static void fx_parser_read_u32s_unstructured(struct fx_parser *parser, void *dst, uint32_t offset, size_t size) +{ + const char *ptr = parser->unstructured.ptr; + + memset(dst, 0, size); + if (offset >= parser->unstructured.size + || size > parser->unstructured.size + || offset > parser->unstructured.size - size) + { + parser->failed = true; + return; + } + + ptr += offset; + memcpy(dst, ptr, size); +}
Doesn't that read bytes instead of u32's? Also, if we know that "offset < parser->unstructured.size", we can just check that "size <= parser->unstructured.size - offset".
Edited by Henri Verbeet
Do we have to accept DXBC containers here? I notice the MR doesn't actually implement that functionality. If this is mainly a matter of convenience for vkd3d-compiler, I think I'd prefer vkd3d-compiler handling that by itself.
That would mean to do the same in D3DDisassemble(). But yes, I can see why it's better to have consistent input.
I'll fix up the rest.
Do we have to accept DXBC containers here? I notice the MR doesn't actually implement that functionality. If this is mainly a matter of convenience for vkd3d-compiler, I think I'd prefer vkd3d-compiler handling that by itself.
That would mean to do the same in D3DDisassemble(). But yes, I can see why it's better to have consistent input.
Yeah, I think we're fine with that. We could have an internal helper to avoid writing that more that once; shouldn't be more complicated than vkd3d_shader_parse_dxbc_source_type().
added 44 commits
-
1d788e20...cd74461d - 41 commits from branch
wine:master
- a338ed9c - vkd3d-shader: Add a stub constant buffers tracing for fx -> text path.
- 2d4815a8 - vkd3d-shader/fx: Add support for parsing constant buffer elements.
- c9dd7150 - vkd3d-shader/fx: Add support for tracing string variables.
Toggle commit list-
1d788e20...cd74461d - 41 commits from branch
+static uint32_t fx_parser_read_u32(struct fx_parser *parser) +{ + uint32_t ret; + + if (parser->ptr == parser->end || (parser->end - parser->ptr) < sizeof(uint32_t)) + {
Do we need the "parser->ptr == parser->end" check above? The other check would evaluate to "0 < sizeof(uint32_t)", wouldn't it?
+static void fx_parser_skip(struct fx_parser *parser, size_t size) +{ + if ((parser->end - parser->ptr) < size) + parser->failed = true; + parser->ptr += size; +}
It may be safer to set "parser->ptr" to "parser->end" on failure (or just don't touch it at all) here, instead of moving it past the end anyway.
+static int fx_2_parse(struct fx_parser *parser) +{ + vkd3d_shader_error(parser->message_context, NULL, VKD3D_SHADER_ERROR_FX_NOT_IMPLEMENTED, + "Parsing fx_2_0 binaries is not implemented.\n"); + + return -1; +}
Just a note, doesn't have to be in this MR, but you could instead implement a "fx_parser_error()" function on top of vkd3d_shader_verror(), which would then set "parser->failed = true", much like e.g. vkd3d_shader_parser_error() and hlsl_error().
+static int fx_5_parse(struct fx_parser *parser) +{ + struct fx_5_header + { + uint32_t version; + uint32_t buffer_count; + uint32_t numeric_variable_count; + uint32_t object_variable_count; + uint32_t shared_buffer_count; + uint32_t shared_numeric_variable_count; + uint32_t shared_object_count; + uint32_t technique_count; + uint32_t unstructured_size; + uint32_t string_count; + uint32_t texture_count; + uint32_t depth_stencil_state_count; + uint32_t blend_state_count; + uint32_t rasterizer_state_count; + uint32_t sampler_state_count; + uint32_t rtv_count; + uint32_t dsv_count; + uint32_t shader_count; + uint32_t inline_shader_count; + uint32_t group_count; + uint32_t uav_count; + uint32_t interface_variable_count; + uint32_t interface_variable_element_count; + uint32_t class_instance_element_count; + } header; + + fx_parser_read_u32s(parser, &header, sizeof(header)); + parser->buffer_count = header.buffer_count; + + parser->unstructured.ptr = parser->ptr; + parser->unstructured.end = parser->ptr + header.unstructured_size; + parser->unstructured.size = header.unstructured_size; + + if (parser->unstructured.end > parser->end) + { + parser->failed = true; + return -1; + } + fx_parser_skip(parser, header.unstructured_size); + + return fx_parse_buffers(parser); +}
This still has the same issue that fx_4_parse() used to have. (I.e., "parser->ptr + header.unstructured_size" potentially overflowing.)
+ unsigned int base_type, comp_count; + size_t j; + + if (type.class == 1) + base_type = (type.typeinfo >> 3) & 0xf; + else + base_type = 0;
"3" above is NUMERIC_BASE_TYPE_SHIFT, right?
+ for (j = 0; j < comp_count; ++j) + { + union hlsl_constant_value_component value; + + fx_parser_read_unstructured(parser, &value, var.value + j * sizeof(uint32_t), sizeof(uint32_t)); + + if (base_type == 1) + vkd3d_string_buffer_printf(&parser->buffer, "%f", value.f); + else if (base_type == 2) + vkd3d_string_buffer_printf(&parser->buffer, "%d", value.i); + else if (base_type == 3) + vkd3d_string_buffer_printf(&parser->buffer, "%u", value.u); + else if (base_type == 4) + vkd3d_string_buffer_printf(&parser->buffer, "%s", value.u ? "true" : "false" );
And I imagine 1/2/3/4 above are the values from fx_4_numeric_base_type[]. Might not be bad to have some constants for these.
added 37 commits
-
c9dd7150...f0e31dd6 - 34 commits from branch
wine:master
- 7ebe72b4 - vkd3d-shader: Add a stub constant buffers tracing for fx -> text path.
- 998cc77d - vkd3d-shader/fx: Add support for parsing constant buffer elements.
- 873e9a5a - vkd3d-shader/fx: Add support for tracing string variables.
Toggle commit list-
c9dd7150...f0e31dd6 - 34 commits from branch
added 11 commits
-
873e9a5a...cd249a47 - 8 commits from branch
wine:master
- 6801ad9b - vkd3d-shader/fx: Introduce a parser/disassembler.
- 907b6705 - vkd3d-shader/fx: Add support for parsing constant buffer elements.
- 35d2df14 - vkd3d-shader/fx: Add support for tracing string variables.
Toggle commit list-
873e9a5a...cd249a47 - 8 commits from branch