[PATCH vkd3d 1/2] vkd3d: Use Vulkan null descriptors if EXT_robustness2 is available.

[Henri Verbeet]

On Mon, 21 Mar 2022 at 16:25, Conor McCarthy <cmccarthy at codeweavers.com> wrote:
>
> Signed-off-by: Conor McCarthy <cmccarthy at codeweavers.com>
> ---
>  libs/vkd3d/device.c        |   8 +++
>  libs/vkd3d/resource.c      | 101 +++++++++++++++++++++++++++++++++++--
>  libs/vkd3d/vkd3d_private.h |   5 ++
>  3 files changed, 109 insertions(+), 5 deletions(-)
>
So what does this get us over the current scheme? I could of course
take some educated guesses, but ideally I wouldn't have to. Note also
that the commit log may be read by fellow developers that may be a
fair bit less familiar with Direct3D, Vulkan, vkd3d, or some
combination of those.

> @@ -2782,7 +2782,7 @@ void d3d12_desc_create_cbv(struct d3d12_desc *descriptor,
>          /* NULL descriptor */
>          buffer_info->buffer = device->null_resources.vk_buffer;
>          buffer_info->offset = 0;
> -        buffer_info->range = VKD3D_NULL_BUFFER_SIZE;
> +        buffer_info->range = device->null_resources.vk_buffer ? VKD3D_NULL_BUFFER_SIZE : VK_WHOLE_SIZE;
>      }
>
Wouldn't VK_WHOLE_SIZE work in either case?

> +static bool vkd3d_create_null_texture_view(const struct d3d12_device *device, struct vkd3d_view **view)
> +{
> +    struct vkd3d_view *object;
> +
> +    if (!(object = vkd3d_view_create(VKD3D_VIEW_TYPE_IMAGE)))
> +        return false;
> +
> +    object->u.vk_image_view = VK_NULL_HANDLE;
> +    object->format = vkd3d_get_format(device, VKD3D_NULL_VIEW_FORMAT, false);
> +    object->info.texture.vk_view_type = VK_IMAGE_VIEW_TYPE_2D;
> +    object->info.texture.miplevel_idx = 0;
> +    object->info.texture.layer_idx = 0;
> +    object->info.texture.layer_count = 1;
> +    *view = object;
> +    return true;
> +}
> +
> +static void vkd3d_create_vk_null_srv(struct d3d12_desc *descriptor, struct d3d12_device *device,
> +        const D3D12_SHADER_RESOURCE_VIEW_DESC *desc)
> +{
> +    struct vkd3d_view *view;
> +
> +    if (!desc)
> +    {
> +        WARN("View desc is required for NULL view.\n");
> +        return;
> +    }
> +
> +    if (desc->ViewDimension == D3D12_SRV_DIMENSION_BUFFER)
> +    {
> +        if (!vkd3d_create_buffer_view(device, VK_NULL_HANDLE, vkd3d_get_format(device, DXGI_FORMAT_R32_UINT, false),
> +                0, VKD3D_NULL_BUFFER_SIZE, &view))
> +        {
> +            return;
> +        }
> +        descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER;
> +    }
> +    else
> +    {
> +        if (!vkd3d_create_null_texture_view(device, &view))
> +            return;
> +        descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE;
> +    }
> +
> +    descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_SRV;
> +    descriptor->u.view_info.view = view;
> +    descriptor->u.view_info.written_serial_id = view->serial_id;
> +}
> +
This doesn't look all that different from the existing
vkd3d_create_null_srv() path; the main difference appears to be
passing VK_NULL_HANDLE instead one of the "null buffer" in the case of
buffer views, and vkd3d_create_null_texture_view() looks fairly
similar to calling vkd3d_create_texture_view() with VK_NULL_HANDLE as
"vk_image" parameter. Could we just handle EXT_robustness2 support in
vkd3d_create_buffer_view() and vkd3d_create_buffer_view() and avoid
the function pointers?

> +static void vkd3d_create_vk_null_uav(struct d3d12_desc *descriptor, struct d3d12_device *device,
> +        const D3D12_UNORDERED_ACCESS_VIEW_DESC *desc)
> +{
> +    struct vkd3d_view *view;
> +
> +    if (!desc)
> +    {
> +        WARN("View desc is required for NULL view.\n");
> +        return;
> +    }
> +
> +    if (desc->ViewDimension == D3D12_UAV_DIMENSION_BUFFER)
> +    {
> +        if (!vkd3d_create_buffer_view(device, VK_NULL_HANDLE, vkd3d_get_format(device, DXGI_FORMAT_R32_UINT, false),
> +                0, VKD3D_NULL_BUFFER_SIZE, &view))
> +        {
> +            return;
> +        }
> +        descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER;
> +    }
> +    else
> +    {
> +        if (!vkd3d_create_null_texture_view(device, &view))
> +            return;
> +        descriptor->vk_descriptor_type = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE;
> +    }
> +
> +    descriptor->magic = VKD3D_DESCRIPTOR_MAGIC_UAV;
> +    descriptor->u.view_info.view = view;
> +    descriptor->u.view_info.written_serial_id = view->serial_id;
> +}
> +
And I think that would largely apply here as well.

> @@ -4421,6 +4502,16 @@ HRESULT vkd3d_init_null_resources(struct vkd3d_null_resources *null_resources,
>
>      memset(null_resources, 0, sizeof(*null_resources));
>
> +    if (device->vk_info.EXT_robustness2)
> +    {
> +        device->create_null_srv = vkd3d_create_vk_null_srv;
> +        device->create_null_uav = vkd3d_create_vk_null_uav;
> +        return S_OK;
> +    }
> +
> +    device->create_null_srv = vkd3d_create_null_srv;
> +    device->create_null_uav = vkd3d_create_null_uav;
> +
If we are going to have these function pointers though, I don't think
the naming (e.g. vkd3d_create_null_srv() vs
vkd3d_create_vk_null_srv()) is very descriptive. Also, we'd typically
store these in an "ops" structure. The case for that perhaps isn't as
obvious here as it would be in some other cases, but there's something
to be said for consistency.