Re: [PATCH vkd3d 2/5] vkd3d-shader: Implement parsing of root signature 1.1.

[Henri Verbeet]

On Fri, 12 Apr 2019 at 19:43, Józef Kucia  wrote:
> +static int shader_parse_descriptor_ranges(struct root_signature_parser_context *context,
> -
>    unsigned int offset, unsigned int count, struct vkd3d_descriptor_range *ranges)
>
> +{
> - unsigned int element_size = context->version >= VKD3D_ROOT_SIGNATURE_VERSION_1_1 ? 6: 5;
> const char *ptr;
> unsigned int i;
> - if (!require_space(offset, 5 * count, sizeof(DWORD), data_size))
> - if (!require_space(offset, element_size * count, sizeof(DWORD), context->data_size))
> {
> -
>    WARN("Invalid data size %#x (offset %u, count %u).\n", data_size, offset, count);
> -
>    WARN("Invalid data size %#x (offset %u, count %u).\n", context->data_size, offset, count);
>    return VKD3D_ERROR_INVALID_ARGUMENT;
> }
> - ptr = &data[offset];
> -
>
> ptr = &context->data[offset];
>
> for (i = 0; i < count; ++i)
> {
>
> @@ -2106,6 +2121,8 @@ static int shader_parse_descriptor_ranges(const char *data, DWORD data_size,
>          read_dword(&ptr, &ranges[i].descriptor_count);
>          read_dword(&ptr, &ranges[i].base_shader_register);
>          read_dword(&ptr, &ranges[i].register_space);
> -
>    if (context->version >= VKD3D_ROOT_SIGNATURE_VERSION_1_1)
> -
>        skip_dword(&ptr, 1);
>    read_dword(&ptr, &ranges[i].descriptor_table_offset);
>
>    TRACE("Type %#x, descriptor count %u, base shader register %u, "
>
> @@ -2118,19 +2135,52 @@ static int shader_parse_descriptor_ranges(const char *data, DWORD data_size,
>      return VKD3D_OK;
>  }
>
> -static int shader_parse_descriptor_table(const char *data, DWORD data_size,
> -
>    DWORD offset, struct vkd3d_root_descriptor_table *table)
>
> +static int shader_parse_descriptor_ranges1(struct root_signature_parser_context *context,
> -
>    unsigned int offset, unsigned int count, struct vkd3d_descriptor_range1 *ranges)
>
> +{
> - const char *ptr;
> - unsigned int i;
> -
> - assert(context->version == VKD3D_ROOT_SIGNATURE_VERSION_1_1);
> - if (!require_space(offset, 6 * count, sizeof(uint32_t), context->data_size))
> - {
> -
>    WARN("Invalid data size %#x (offset %u, count %u).\n", context->data_size, offset, count);
> -
>    return VKD3D_ERROR_INVALID_ARGUMENT;
> - }
> - ptr = &context->data[offset];
> -
> - for (i = 0; i < count; ++i)
> - {
> -
>    read_dword(&ptr, &ranges[i].range_type);
> -
>    read_dword(&ptr, &ranges[i].descriptor_count);
> -
>    read_dword(&ptr, &ranges[i].base_shader_register);
> -
>    read_dword(&ptr, &ranges[i].register_space);
> -
>    read_dword(&ptr, &ranges[i].flags);
> -
>    read_dword(&ptr, &ranges[i].descriptor_table_offset);
> -
> -
>    TRACE("Type %#x, descriptor count %u, base shader register %u, "
> -
>            "register space %u, flags %#x, offset %u.\n",
> -
>            ranges[i].range_type, ranges[i].descriptor_count,
> -
>            ranges[i].base_shader_register, ranges[i].register_space,
> -
>            ranges[i].flags, ranges[i].descriptor_table_offset);
> - }
> -
> - return VKD3D_OK;
>
> +}

Do we really need shader_parse_descriptor_ranges() to be able to
handle 1.1 signatures if we also have
shader_parse_descriptor_ranges1()? Or alternatively, do we really need
shader_parse_descriptor_ranges1()? Likewise for
shader_parse_root_descriptor() and shader_parse_root_parameters().
> int vkd3d_shader_parse_root_signature(const struct vkd3d_shader_code *dxbc,
>          struct vkd3d_root_signature_desc *root_signature)
> +{
> - struct vkd3d_versioned_root_signature_desc desc = {.version = VKD3D_ROOT_SIGNATURE_VERSION_1_0};
> - int ret;
> -
> - TRACE("dxbc {%p, %zu}, root_signature %p.\n", dxbc->code, dxbc->size, root_signature);
> -
> - memset(root_signature, 0, sizeof(*root_signature));
> - if ((ret = parse_dxbc(dxbc->code, dxbc->size, rts0_handler, &desc)) < 0)
> - {
> -
>    vkd3d_shader_free_versioned_root_signature(&desc);
> -
>    return ret;
> - }
> -
> - assert(desc.version == VKD3D_ROOT_SIGNATURE_VERSION_1_0);
> -
> - *root_signature = desc.u.v_1_0;
> -
> - return VKD3D_OK;
>
> +}

Since vkd3d_shader_parse_root_signature() isn't in any released
version of libvkd3d-shader, could we just get rid of it?