Re: [PATCH 0/4] MR2: tests: Test float4 UAV buffer load/store.

[Conor McCarthy (＠cmccarthy)]

On Mon Aug  8 13:15:07 2022 +0000, Henri Verbeet wrote:
> From patch 3/4:
> > @@ -1391,6 +1391,45 @@ static void
>
> vkd3d_device_vk_heaps_descriptor_limits_init(struct vkd3d_device_des
> >  limits->sampler_max_descriptors =
>
> min(limits->sampler_max_descriptors, VKD3D_MAX_DESCRIPTOR_SET_SAMPLERS);
> > }
> >
> > +static bool
>
> d3d12_device_supports_typed_uav_load_additional_formats(const struct
> d3d12_device *device)
> > +{
> > - const struct vkd3d_vk_instance_procs *vk_procs = &device->vkd3d_instance->vk_procs;
> > - const struct vkd3d_format *format;
> > - VkFormatProperties properties;
> > - unsigned int i;
> > -
> > - static const DXGI_FORMAT additional_formats[] =
> > - {
> > -
> >    DXGI_FORMAT_R32G32B32A32_FLOAT,
> > -
> >    DXGI_FORMAT_R32G32B32A32_UINT,
> > -
> >    DXGI_FORMAT_R32G32B32A32_SINT,
> > -
> >    DXGI_FORMAT_R16G16B16A16_FLOAT,
> > -
> >    DXGI_FORMAT_R16G16B16A16_UINT,
> > -
> >    DXGI_FORMAT_R16G16B16A16_SINT,
> > -
> >    DXGI_FORMAT_R8G8B8A8_UNORM,
> > -
> >    DXGI_FORMAT_R8G8B8A8_UINT,
> > -
> >    DXGI_FORMAT_R8G8B8A8_SINT,
> > -
> >    DXGI_FORMAT_R16_FLOAT,
> > -
> >    DXGI_FORMAT_R16_UINT,
> > -
> >    DXGI_FORMAT_R16_SINT,
> > -
> >    DXGI_FORMAT_R8_UNORM,
> > -
> >    DXGI_FORMAT_R8_UINT,
> > -
> >    DXGI_FORMAT_R8_SINT,
> > - };
> > -
> > - for (i = 0; i < ARRAY_SIZE(additional_formats); ++i)
> > - {
> > -
> >    format = vkd3d_get_format(device, additional_formats[i], false);
> > -
> >    assert(format);
> > -
> > -
>
> VK_CALL(vkGetPhysicalDeviceFormatProperties(device->vk_physical_device,
> format->vk_format, &properties));
> > -
> >    if (!((properties.linearTilingFeatures |
>
> properties.optimalTilingFeatures) & VK_FORMAT_FEATURE_STORAGE_IMAGE_BIT))
> > -
> >        return false;
> > - }
> > -
> > - return true;
> >
> > +}
> > -
> >
> > static HRESULT vkd3d_init_device_caps(struct d3d12_device *device,
> >          const struct vkd3d_device_create_info *create_info,
> >          struct vkd3d_physical_device_info *physical_device_info,
> > @@ -1455,7 +1494,7 @@ static HRESULT vkd3d_init_device_caps(struct
>
> d3d12_device *device,
> >  else
> >      device->feature_options.ResourceBindingTier = D3D12_RESOURCE_BINDING_TIER_3;
> >
> > - device->feature_options.TypedUAVLoadAdditionalFormats = features->shaderStorageImageExtendedFormats;
> > - device->feature_options.TypedUAVLoadAdditionalFormats = d3d12_device_supports_typed_uav_load_additional_formats(device);
> > /* GL_INTEL_fragment_shader_ordering, no Vulkan equivalent. */
> > device->feature_options.ROVsSupported = FALSE;
> > /* GL_INTEL_conservative_rasterization, no Vulkan equivalent. */
>
> I think we should also require `shaderStorageImageReadWithoutFormat`
> here; vkd3d-shader will be unable to properly use those additional
> formats unless that capability is available.
> From patch 4/4:
> > @@ -1310,6 +1310,7 @@ static void vkd3d_init_feature_level(struct
>
> vkd3d_vulkan_info *vk_info,
> >  CHECK_FEATURE(shaderClipDistance);
> >  CHECK_FEATURE(shaderCullDistance);
> >  CHECK_FEATURE(shaderImageGatherExtended);
> > - CHECK_FEATURE(shaderStorageImageReadWithoutFormat);
> > CHECK_FEATURE(shaderStorageImageWriteWithoutFormat);
> > CHECK_FEATURE(tessellationShader);
>
> I'm not quite sure where we left this discussion last time, but to be
> clear, I think this change shouldn't be included. Based on
> [[1]](https://docs.microsoft.com/en-us/windows/win32/direct3d12/hardware-feature-l...),
> I'd say `shaderStorageImageReadWithoutFormat` should only be a
> requirement for feature level 12_0 and up. We already include a check
> for `TypedUAVLoadAdditionalFormats` for feature level 12_0 and up, which
> should cover this.

I'm not quite sure where we left this discussion last time, but to be clear, I think this change shouldn't be included. Based on [1] (https://docs.microsoft.com/en-us/windows/win32/direct3d12/hardware-feature-l...), I'd say shaderStorageImageReadWithoutFormat should only be a requirement for feature level 12_0 and up. We already include a check for TypedUAVLoadAdditionalFormats for feature level 12_0 and up, which should cover this.
Oops, I forgot to delete that.