From: Henri Verbeet <hverbeet@gmail.com>
Subject: Re: [PATCH vkd3d 2/5] vkd3d-shader: Implement parsing of root signature 1.1.
Message-Id: <CAOsNvww+wSOaVvs=KdwHV7v_PvgFKGwdOFmJp7bczYumPZv21Q@mail.gmail.com>
Date: Mon, 15 Apr 2019 19:25:04 +0430
In-Reply-To: <20190412151321.5644-2-joseph.kucia@gmail.com>
References: <20190412151321.5644-2-joseph.kucia@gmail.com>

On Fri, 12 Apr 2019 at 19:43, Józef Kucia <joseph.kucia@gmail.com> 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?