On Mon, 21 Mar 2022 at 16:25, Conor McCarthy cmccarthy@codeweavers.com wrote:
Signed-off-by: Conor McCarthy cmccarthy@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.