Resource formats are immutable and the format object is static data. Storing it saves a function call and error check in many locations. The current implementation for finding a format iterates over the entire list of formats.
The format is checked for NULL during resource initialisation, so accessing the format object is safe where buffers are excluded. In some cases the format query effectively excluded buffers, so an explicit check is needed.
Signed-off-by: Conor McCarthy cmccarthy@codeweavers.com --- libs/vkd3d/command.c | 72 ++++++++++---------------------------- libs/vkd3d/device.c | 4 +-- libs/vkd3d/resource.c | 35 +++++++++--------- libs/vkd3d/vkd3d_private.h | 4 ++- 4 files changed, 39 insertions(+), 76 deletions(-)
diff --git a/libs/vkd3d/command.c b/libs/vkd3d/command.c index 3e252aee..5f107384 100644 --- a/libs/vkd3d/command.c +++ b/libs/vkd3d/command.c @@ -2116,17 +2116,10 @@ static void d3d12_command_list_transition_resource_to_initial_state(struct d3d12 const struct vkd3d_vk_device_procs *vk_procs = &list->device->vk_procs; const struct vkd3d_vulkan_info *vk_info = &list->device->vk_info; VkPipelineStageFlags src_stage_mask, dst_stage_mask; - const struct vkd3d_format *format; VkImageMemoryBarrier barrier;
assert(d3d12_resource_is_texture(resource));
- if (!(format = vkd3d_format_from_d3d12_resource_desc(list->device, &resource->desc, 0))) - { - ERR("Resource %p has invalid format %#x.\n", resource, resource->desc.Format); - return; - } - barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; barrier.pNext = NULL;
@@ -2146,7 +2139,7 @@ static void d3d12_command_list_transition_resource_to_initial_state(struct d3d12 barrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; barrier.image = resource->u.vk_image; - barrier.subresourceRange.aspectMask = format->vk_aspect_mask; + barrier.subresourceRange.aspectMask = resource->format->vk_aspect_mask; barrier.subresourceRange.baseMipLevel = 0; barrier.subresourceRange.levelCount = VK_REMAINING_MIP_LEVELS; barrier.subresourceRange.baseArrayLayer = 0; @@ -3531,22 +3524,15 @@ static void STDMETHODCALLTYPE d3d12_command_list_CopyTextureRegion(ID3D12Graphic else if (src->Type == D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX && dst->Type == D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX) { - assert(d3d12_resource_is_texture(dst_resource)); - assert(d3d12_resource_is_texture(src_resource)); - - if (!(dst_format = vkd3d_format_from_d3d12_resource_desc(list->device, - &dst_resource->desc, DXGI_FORMAT_UNKNOWN))) - { - WARN("Invalid format %#x.\n", dst_resource->desc.Format); - return; - } - if (!(src_format = vkd3d_format_from_d3d12_resource_desc(list->device, - &src_resource->desc, DXGI_FORMAT_UNKNOWN))) + if (!d3d12_resource_is_texture(dst_resource) || !d3d12_resource_is_texture(src_resource)) { - WARN("Invalid format %#x.\n", src_resource->desc.Format); + WARN("Both dst and src must contain textures.\n"); return; }
+ dst_format = dst_resource->format; + src_format = src_resource->format; + if ((dst_format->vk_aspect_mask & VK_IMAGE_ASPECT_DEPTH_BIT) && (dst_format->vk_aspect_mask & VK_IMAGE_ASPECT_STENCIL_BIT)) FIXME("Depth-stencil format %#x not fully supported yet.\n", dst_format->dxgi_format); @@ -3580,7 +3566,6 @@ static void STDMETHODCALLTYPE d3d12_command_list_CopyResource(ID3D12GraphicsComm { struct d3d12_command_list *list = impl_from_ID3D12GraphicsCommandList2(iface); struct d3d12_resource *dst_resource, *src_resource; - const struct vkd3d_format *src_format, *dst_format; const struct vkd3d_vk_device_procs *vk_procs; VkBufferCopy vk_buffer_copy; VkImageCopy vk_image_copy; @@ -3612,30 +3597,21 @@ static void STDMETHODCALLTYPE d3d12_command_list_CopyResource(ID3D12GraphicsComm } else { - if (!(dst_format = vkd3d_format_from_d3d12_resource_desc(list->device, - &dst_resource->desc, DXGI_FORMAT_UNKNOWN))) - { - WARN("Invalid format %#x.\n", dst_resource->desc.Format); - return; - } - if (!(src_format = vkd3d_format_from_d3d12_resource_desc(list->device, - &src_resource->desc, DXGI_FORMAT_UNKNOWN))) + if (!d3d12_resource_is_texture(dst_resource) || !d3d12_resource_is_texture(src_resource)) { - WARN("Invalid format %#x.\n", src_resource->desc.Format); + WARN("Both dst and src must be textures.\n"); return; }
layer_count = d3d12_resource_desc_get_layer_count(&dst_resource->desc);
- assert(d3d12_resource_is_texture(dst_resource)); - assert(d3d12_resource_is_texture(src_resource)); assert(dst_resource->desc.MipLevels == src_resource->desc.MipLevels); assert(layer_count == d3d12_resource_desc_get_layer_count(&src_resource->desc));
for (i = 0; i < dst_resource->desc.MipLevels; ++i) { - vk_image_copy_from_d3d12(&vk_image_copy, i, i, - &src_resource->desc, &dst_resource->desc, src_format, dst_format, NULL, 0, 0, 0); + vk_image_copy_from_d3d12(&vk_image_copy, i, i, &src_resource->desc, &dst_resource->desc, + src_resource->format, dst_resource->format, NULL, 0, 0, 0); vk_image_copy.dstSubresource.layerCount = layer_count; vk_image_copy.srcSubresource.layerCount = layer_count; VK_CALL(vkCmdCopyImage(list->vk_command_buffer, src_resource->u.vk_image, @@ -3676,24 +3652,19 @@ static void STDMETHODCALLTYPE d3d12_command_list_ResolveSubresource(ID3D12Graphi dst_resource = unsafe_impl_from_ID3D12Resource(dst); src_resource = unsafe_impl_from_ID3D12Resource(src);
- assert(d3d12_resource_is_texture(dst_resource)); - assert(d3d12_resource_is_texture(src_resource)); + if (!d3d12_resource_is_texture(dst_resource) || !d3d12_resource_is_texture(src_resource)) + { + WARN("Both dst and src must be textures.\n"); + return; + }
d3d12_command_list_track_resource_usage(list, dst_resource); d3d12_command_list_track_resource_usage(list, src_resource);
d3d12_command_list_end_current_render_pass(list);
- if (!(dst_format = vkd3d_format_from_d3d12_resource_desc(device, &dst_resource->desc, DXGI_FORMAT_UNKNOWN))) - { - WARN("Invalid format %#x.\n", dst_resource->desc.Format); - return; - } - if (!(src_format = vkd3d_format_from_d3d12_resource_desc(device, &src_resource->desc, DXGI_FORMAT_UNKNOWN))) - { - WARN("Invalid format %#x.\n", src_resource->desc.Format); - return; - } + dst_format = dst_resource->format; + src_format = src_resource->format;
if (dst_format->type == VKD3D_FORMAT_TYPE_TYPELESS || src_format->type == VKD3D_FORMAT_TYPE_TYPELESS) { @@ -4064,15 +4035,8 @@ static void STDMETHODCALLTYPE d3d12_command_list_ResourceBarrier(ID3D12GraphicsC } else { - const struct vkd3d_format *format; VkImageMemoryBarrier vk_barrier;
- if (!(format = vkd3d_format_from_d3d12_resource_desc(list->device, &resource->desc, 0))) - { - ERR("Resource %p has invalid format %#x.\n", resource, resource->desc.Format); - continue; - } - vk_barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; vk_barrier.pNext = NULL; vk_barrier.srcAccessMask = src_access_mask; @@ -4083,7 +4047,7 @@ static void STDMETHODCALLTYPE d3d12_command_list_ResourceBarrier(ID3D12GraphicsC vk_barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; vk_barrier.image = resource->u.vk_image;
- vk_barrier.subresourceRange.aspectMask = format->vk_aspect_mask; + vk_barrier.subresourceRange.aspectMask = resource->format->vk_aspect_mask; if (sub_resource_idx == D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES) { vk_barrier.subresourceRange.baseMipLevel = 0; diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c index 4ce3ed81..5012c91c 100644 --- a/libs/vkd3d/device.c +++ b/libs/vkd3d/device.c @@ -3394,7 +3394,7 @@ static D3D12_RESOURCE_ALLOCATION_INFO * STDMETHODCALLTYPE d3d12_device_GetResour
desc = &resource_descs[0];
- if (FAILED(d3d12_resource_validate_desc(desc, device))) + if (FAILED(d3d12_resource_validate_desc(desc, NULL, device))) { WARN("Invalid resource desc.\n"); goto invalid; @@ -3688,7 +3688,7 @@ static void STDMETHODCALLTYPE d3d12_device_GetCopyableFootprints(ID3D12Device *i return; }
- if (FAILED(d3d12_resource_validate_desc(desc, device))) + if (FAILED(d3d12_resource_validate_desc(desc, NULL, device))) { WARN("Invalid resource desc.\n"); return; diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 78b32bde..2ac7ad95 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -855,7 +855,11 @@ static HRESULT vkd3d_create_image(struct d3d12_device *device, VkImageCreateInfo image_info; VkResult vr;
- if (!(format = vkd3d_format_from_d3d12_resource_desc(device, desc, 0))) + if (resource) + { + format = resource->format; + } + else if (!(format = vkd3d_format_from_d3d12_resource_desc(device, desc, 0))) { WARN("Invalid DXGI format %#x.\n", desc->Format); return E_INVALIDARG; @@ -1001,7 +1005,7 @@ HRESULT vkd3d_get_image_allocation_info(struct d3d12_device *device, HRESULT hr;
assert(desc->Dimension != D3D12_RESOURCE_DIMENSION_BUFFER); - assert(d3d12_resource_validate_desc(desc, device) == S_OK); + assert(d3d12_resource_validate_desc(desc, NULL, device) == S_OK);
if (!desc->MipLevels) { @@ -1076,7 +1080,6 @@ static bool d3d12_resource_validate_box(const struct d3d12_resource *resource, unsigned int sub_resource_idx, const D3D12_BOX *box) { unsigned int mip_level = sub_resource_idx % resource->desc.MipLevels; - struct d3d12_device *device = resource->device; const struct vkd3d_format *vkd3d_format; uint32_t width_mask, height_mask; uint64_t width, height, depth; @@ -1085,7 +1088,7 @@ static bool d3d12_resource_validate_box(const struct d3d12_resource *resource, height = d3d12_resource_desc_get_height(&resource->desc, mip_level); depth = d3d12_resource_desc_get_depth(&resource->desc, mip_level);
- vkd3d_format = vkd3d_format_from_d3d12_resource_desc(device, &resource->desc, 0); + vkd3d_format = resource->format; assert(vkd3d_format); width_mask = vkd3d_format->block_width - 1; height_mask = vkd3d_format->block_height - 1; @@ -1350,11 +1353,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_WriteToSubresource(ID3D12Resourc device = resource->device; vk_procs = &device->vk_procs;
- if (!(format = vkd3d_format_from_d3d12_resource_desc(device, &resource->desc, 0))) - { - ERR("Invalid DXGI format %#x.\n", resource->desc.Format); - return E_INVALIDARG; - } + format = resource->format; if (format->vk_aspect_mask != VK_IMAGE_ASPECT_COLOR_BIT) { FIXME("Not supported for format %#x.\n", format->dxgi_format); @@ -1442,11 +1441,7 @@ static HRESULT STDMETHODCALLTYPE d3d12_resource_ReadFromSubresource(ID3D12Resour device = resource->device; vk_procs = &device->vk_procs;
- if (!(format = vkd3d_format_from_d3d12_resource_desc(device, &resource->desc, 0))) - { - ERR("Invalid DXGI format %#x.\n", resource->desc.Format); - return E_INVALIDARG; - } + format = resource->format; if (format->vk_aspect_mask != VK_IMAGE_ASPECT_COLOR_BIT) { FIXME("Not supported for format %#x.\n", format->dxgi_format); @@ -1649,10 +1644,9 @@ static bool d3d12_resource_validate_texture_alignment(const D3D12_RESOURCE_DESC return true; }
-HRESULT d3d12_resource_validate_desc(const D3D12_RESOURCE_DESC *desc, struct d3d12_device *device) +HRESULT d3d12_resource_validate_desc(const D3D12_RESOURCE_DESC *desc, const struct vkd3d_format *format, + struct d3d12_device *device) { - const struct vkd3d_format *format; - switch (desc->Dimension) { case D3D12_RESOURCE_DIMENSION_BUFFER: @@ -1687,7 +1681,7 @@ HRESULT d3d12_resource_validate_desc(const D3D12_RESOURCE_DESC *desc, struct d3d return E_INVALIDARG; }
- if (!(format = vkd3d_format_from_d3d12_resource_desc(device, desc, 0))) + if (!format && !(format = vkd3d_format_from_d3d12_resource_desc(device, desc, 0))) { WARN("Invalid format %#x.\n", desc->Format); return E_INVALIDARG; @@ -1775,7 +1769,9 @@ static HRESULT d3d12_resource_init(struct d3d12_resource *resource, struct d3d12 resource->gpu_address = 0; resource->flags = 0;
- if (FAILED(hr = d3d12_resource_validate_desc(&resource->desc, device))) + resource->format = (desc->Dimension == D3D12_RESOURCE_DIMENSION_BUFFER) ? NULL + : vkd3d_format_from_d3d12_resource_desc(device, desc, 0); + if (FAILED(hr = d3d12_resource_validate_desc(&resource->desc, resource->format, device))) return hr;
switch (desc->Dimension) @@ -2030,6 +2026,7 @@ HRESULT vkd3d_create_image_resource(ID3D12Device *device, object->refcount = 1; object->internal_refcount = 1; object->desc = create_info->desc; + object->format = vkd3d_format_from_d3d12_resource_desc(d3d12_device, &create_info->desc, 0); object->u.vk_image = create_info->vk_image; object->flags = VKD3D_RESOURCE_EXTERNAL; object->flags |= create_info->flags & VKD3D_RESOURCE_PUBLIC_FLAGS; diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index 047f4a29..b1291804 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -427,6 +427,7 @@ struct d3d12_resource LONG internal_refcount;
D3D12_RESOURCE_DESC desc; + const struct vkd3d_format *format;
D3D12_GPU_VIRTUAL_ADDRESS gpu_address; union @@ -460,7 +461,8 @@ static inline bool d3d12_resource_is_texture(const struct d3d12_resource *resour }
bool d3d12_resource_is_cpu_accessible(const struct d3d12_resource *resource); -HRESULT d3d12_resource_validate_desc(const D3D12_RESOURCE_DESC *desc, struct d3d12_device *device); +HRESULT d3d12_resource_validate_desc(const D3D12_RESOURCE_DESC *desc, const struct vkd3d_format *format, + struct d3d12_device *device);
HRESULT d3d12_committed_resource_create(struct d3d12_device *device, const D3D12_HEAP_PROPERTIES *heap_properties, D3D12_HEAP_FLAGS heap_flags,
vk_image_aspect_flags_from_d3d12_plane_slice() is based on a vkd3d-proton implementation by Philip Rebohle.
Signed-off-by: Conor McCarthy cmccarthy@codeweavers.com --- libs/vkd3d/resource.c | 42 +++++++++++++++++++++++++++++++------- libs/vkd3d/vkd3d_private.h | 1 + 2 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/libs/vkd3d/resource.c b/libs/vkd3d/resource.c index 2ac7ad95..5bf46896 100644 --- a/libs/vkd3d/resource.c +++ b/libs/vkd3d/resource.c @@ -2441,6 +2441,7 @@ static bool init_default_texture_view_desc(struct vkd3d_texture_view_desc *desc, desc->miplevel_count = 1; desc->layer_idx = 0; desc->layer_count = d3d12_resource_desc_get_layer_count(&resource->desc); + desc->vk_image_aspect = desc->format->vk_aspect_mask;
switch (resource->desc.Dimension) { @@ -2524,7 +2525,7 @@ bool vkd3d_create_texture_view(struct d3d12_device *device, VkImage vk_image, vkd3d_set_view_swizzle_for_format(&view_desc.components, format, desc->allowed_swizzle); if (desc->allowed_swizzle) vk_component_mapping_compose(&view_desc.components, &desc->components); - view_desc.subresourceRange.aspectMask = format->vk_aspect_mask; + view_desc.subresourceRange.aspectMask = desc->vk_image_aspect; view_desc.subresourceRange.baseMipLevel = desc->miplevel_idx; view_desc.subresourceRange.levelCount = desc->miplevel_count; view_desc.subresourceRange.baseArrayLayer = desc->layer_idx; @@ -2692,6 +2693,27 @@ static void vkd3d_create_buffer_srv(struct d3d12_desc *descriptor, descriptor->u.view = view; }
+static VkImageAspectFlags vk_image_aspect_flags_from_d3d12_plane_slice(const struct vkd3d_format *format, + unsigned int plane_slice) +{ + VkImageAspectFlags aspect_flags = format->vk_aspect_mask; + unsigned int i; + + /* For all formats we currently handle, the n-th aspect bit in Vulkan + * corresponds to the n-th plane in D3D12, so isolate the respective + * bit in the aspect flags. */ + for (i = 0; i < plane_slice; i++) + aspect_flags &= aspect_flags - 1; + + if (!aspect_flags) + { + WARN("Invalid plane slice %u for format %#x.\n", plane_slice, format->vk_format); + aspect_flags = format->vk_aspect_mask; + } + + return aspect_flags & -aspect_flags; +} + void d3d12_desc_create_srv(struct d3d12_desc *descriptor, struct d3d12_device *device, struct d3d12_resource *resource, const D3D12_SHADER_RESOURCE_VIEW_DESC *desc) @@ -2734,7 +2756,8 @@ void d3d12_desc_create_srv(struct d3d12_desc *descriptor, vkd3d_desc.miplevel_idx = desc->u.Texture2D.MostDetailedMip; vkd3d_desc.miplevel_count = desc->u.Texture2D.MipLevels; if (desc->u.Texture2D.PlaneSlice) - FIXME("Ignoring plane slice %u.\n", desc->u.Texture2D.PlaneSlice); + vkd3d_desc.vk_image_aspect = vk_image_aspect_flags_from_d3d12_plane_slice(resource->format, + desc->u.Texture2D.PlaneSlice); if (desc->u.Texture2D.ResourceMinLODClamp) FIXME("Unhandled min LOD clamp %.8e.\n", desc->u.Texture2D.ResourceMinLODClamp); break; @@ -2745,7 +2768,8 @@ void d3d12_desc_create_srv(struct d3d12_desc *descriptor, vkd3d_desc.layer_idx = desc->u.Texture2DArray.FirstArraySlice; vkd3d_desc.layer_count = desc->u.Texture2DArray.ArraySize; if (desc->u.Texture2DArray.PlaneSlice) - FIXME("Ignoring plane slice %u.\n", desc->u.Texture2DArray.PlaneSlice); + vkd3d_desc.vk_image_aspect = vk_image_aspect_flags_from_d3d12_plane_slice(resource->format, + desc->u.Texture2D.PlaneSlice); if (desc->u.Texture2DArray.ResourceMinLODClamp) FIXME("Unhandled min LOD clamp %.8e.\n", desc->u.Texture2DArray.ResourceMinLODClamp); vkd3d_texture_view_desc_normalise(&vkd3d_desc, &resource->desc); @@ -2942,7 +2966,8 @@ static void vkd3d_create_texture_uav(struct d3d12_desc *descriptor, case D3D12_UAV_DIMENSION_TEXTURE2D: vkd3d_desc.miplevel_idx = desc->u.Texture2D.MipSlice; if (desc->u.Texture2D.PlaneSlice) - FIXME("Ignoring plane slice %u.\n", desc->u.Texture2D.PlaneSlice); + vkd3d_desc.vk_image_aspect = vk_image_aspect_flags_from_d3d12_plane_slice(resource->format, + desc->u.Texture2D.PlaneSlice); break; case D3D12_UAV_DIMENSION_TEXTURE2DARRAY: vkd3d_desc.view_type = VK_IMAGE_VIEW_TYPE_2D_ARRAY; @@ -2950,7 +2975,8 @@ static void vkd3d_create_texture_uav(struct d3d12_desc *descriptor, vkd3d_desc.layer_idx = desc->u.Texture2DArray.FirstArraySlice; vkd3d_desc.layer_count = desc->u.Texture2DArray.ArraySize; if (desc->u.Texture2DArray.PlaneSlice) - FIXME("Ignoring plane slice %u.\n", desc->u.Texture2DArray.PlaneSlice); + vkd3d_desc.vk_image_aspect = vk_image_aspect_flags_from_d3d12_plane_slice(resource->format, + desc->u.Texture2D.PlaneSlice); vkd3d_texture_view_desc_normalise(&vkd3d_desc, &resource->desc); break; case D3D12_UAV_DIMENSION_TEXTURE3D: @@ -3193,7 +3219,8 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev case D3D12_RTV_DIMENSION_TEXTURE2D: vkd3d_desc.miplevel_idx = desc->u.Texture2D.MipSlice; if (desc->u.Texture2D.PlaneSlice) - FIXME("Ignoring plane slice %u.\n", desc->u.Texture2D.PlaneSlice); + vkd3d_desc.vk_image_aspect = vk_image_aspect_flags_from_d3d12_plane_slice(resource->format, + desc->u.Texture2D.PlaneSlice); break; case D3D12_RTV_DIMENSION_TEXTURE2DARRAY: vkd3d_desc.view_type = VK_IMAGE_VIEW_TYPE_2D_ARRAY; @@ -3201,7 +3228,8 @@ void d3d12_rtv_desc_create_rtv(struct d3d12_rtv_desc *rtv_desc, struct d3d12_dev vkd3d_desc.layer_idx = desc->u.Texture2DArray.FirstArraySlice; vkd3d_desc.layer_count = desc->u.Texture2DArray.ArraySize; if (desc->u.Texture2DArray.PlaneSlice) - FIXME("Ignoring plane slice %u.\n", desc->u.Texture2DArray.PlaneSlice); + vkd3d_desc.vk_image_aspect = vk_image_aspect_flags_from_d3d12_plane_slice(resource->format, + desc->u.Texture2D.PlaneSlice); vkd3d_texture_view_desc_normalise(&vkd3d_desc, &resource->desc); break; case D3D12_RTV_DIMENSION_TEXTURE2DMS: diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index b1291804..612cc56e 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -532,6 +532,7 @@ struct vkd3d_texture_view_desc unsigned int miplevel_count; unsigned int layer_idx; unsigned int layer_count; + VkImageAspectFlags vk_image_aspect; VkComponentMapping components; bool allowed_swizzle; };
On Fri, 14 Jan 2022 at 05:57, Conor McCarthy cmccarthy@codeweavers.com wrote:
+static VkImageAspectFlags vk_image_aspect_flags_from_d3d12_plane_slice(const struct vkd3d_format *format,
unsigned int plane_slice)+{
- VkImageAspectFlags aspect_flags = format->vk_aspect_mask;
- unsigned int i;
- /* For all formats we currently handle, the n-th aspect bit in Vulkan
* corresponds to the n-th plane in D3D12, so isolate the respective* bit in the aspect flags. */- for (i = 0; i < plane_slice; i++)
aspect_flags &= aspect_flags - 1;
So above we're clearing the least significant bit set.
- if (!aspect_flags)
- {
WARN("Invalid plane slice %u for format %#x.\n", plane_slice, format->vk_format);aspect_flags = format->vk_aspect_mask;- }
- return aspect_flags & -aspect_flags;
And then here we return the least significant bit set.
+}
That works, but I wouldn't say it's the most obvious code I've ever seen; some comments wouldn't be inappropriate.
On Fri, 14 Jan 2022 at 05:57, Conor McCarthy cmccarthy@codeweavers.com wrote:
Resource formats are immutable and the format object is static data. Storing it saves a function call and error check in many locations. The current implementation for finding a format iterates over the entire list of formats.
I think the basic idea is fine (and we do this in e.g. wined3d as well), but changing everything at the same time makes it harder to review than needed. Please split this.
The format is checked for NULL during resource initialisation, so accessing the format object is safe where buffers are excluded. In some cases the format query effectively excluded buffers, so an explicit check is needed.
Is what you're saying here simply that texture resources will always have a non-NULL "format" pointer because that's validated during resource creation, or something else? Note that I don't think there's anything necessarily preventing us from adding format information for DXGI_FORMAT_UNKNOWN as well, so that buffers too would always have a non-NULL format.
@@ -1649,10 +1644,9 @@ static bool d3d12_resource_validate_texture_alignment(const D3D12_RESOURCE_DESC return true; }
-HRESULT d3d12_resource_validate_desc(const D3D12_RESOURCE_DESC *desc, struct d3d12_device *device) +HRESULT d3d12_resource_validate_desc(const D3D12_RESOURCE_DESC *desc, const struct vkd3d_format *format,
struct d3d12_device *device){
- const struct vkd3d_format *format;
- switch (desc->Dimension) { case D3D12_RESOURCE_DIMENSION_BUFFER:
@@ -1687,7 +1681,7 @@ HRESULT d3d12_resource_validate_desc(const D3D12_RESOURCE_DESC *desc, struct d3d return E_INVALIDARG; }
if (!(format = vkd3d_format_from_d3d12_resource_desc(device, desc, 0)))
if (!format && !(format = vkd3d_format_from_d3d12_resource_desc(device, desc, 0))) {
The optional "format" parameter to d3d12_resource_validate_desc() is a bit awkward, I think. As far as I can tell, it's only ever non-NULL when called from d3d12_resource_init(), but that also seems like a place where we're unlikely to care about looking up the format twice.
- if (FAILED(hr = d3d12_resource_validate_desc(&resource->desc, device)))
- resource->format = (desc->Dimension == D3D12_RESOURCE_DIMENSION_BUFFER) ? NULL
: vkd3d_format_from_d3d12_resource_desc(device, desc, 0);- if (FAILED(hr = d3d12_resource_validate_desc(&resource->desc, resource->format, device))) return hr;
The buffer check above looks superfluous. If the format is DXGI_FORMAT_UNKNOWN, vkd3d_format_from_d3d12_resource_desc() should already return NULL. If it's something else, resource creation should fail due to d3d12_resource_validate_desc().
-
Conor McCarthy -
Henri Verbeet