[PATCH vkd3d 1/2] vkd3d: Store the vkd3d format in resource objects.

[Henri Verbeet]

On Fri, 14 Jan 2022 at 05:57, Conor McCarthy <cmccarthy at 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().