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().