Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

F1 2017 creates invalid sRGB images #504

Closed
gfxstrand opened this issue Jul 20, 2018 · 5 comments
Closed

F1 2017 creates invalid sRGB images #504

gfxstrand opened this issue Jul 20, 2018 · 5 comments
Labels

Comments

@gfxstrand
Copy link

F1 2017 running through DXVK creates an image with VK_IMAGE_USAGE_STORAGE_BIT set and then creates a view of that image with a format of VK_FORMAT_R8G8B8A8_SRGB. This is illegal according to the Vulkan spec:

If image was created with VK_IMAGE_TILING_OPTIMAL, and format is not VK_FORMAT_UNDEFINED, and usage contains VK_IMAGE_USAGE_STORAGE_BIT, format must be supported for storage images, as specified by the VK_FORMAT_FEATURE_STORAGE_IMAGE_BIT flag in VkFormatProperties::optimalTilingFeatures returned by vkGetPhysicalDeviceFormatProperties with the same value of format.

What F1 and DXVK are trying to do is sensible and that's why we added support for this with VK_KHR_maintenance2. What you need to do is the following:

  1. Create the image with VK_IMAGE_CREATE_EXTENDED_USAGE_BIT_KHR and all of the possible usages of the image even if they aren't supported by the format.
  2. Chain a VkImageViewUsageCreateInfoKHR struct into VkImageViewCreateInfo and provide a view-specific usage whenever you create an image view. For a UAV, for instance, it would just be VK_IMAGE_USAGE_STORAGE_BIT.

This should also slightly improve memory usage on Intel because we create different hardware surface states for storage vs. texture so right now the texture surface state is unnecessarily being created for UAVs and the storage surface state is unnecessarily being created for sampler views.

@doitsujin doitsujin added the bug label Jul 20, 2018
@doitsujin
Copy link
Owner

doitsujin commented Jul 20, 2018

Thanks for the report, wasn't aware that VK_KHR_maintenance2 provided a way to deal with this properly.

doitsujin added a commit that referenced this issue Jul 21, 2018
Must be used when view formats are used that do not support all
usage bits of the underlying image. Refs #504.
@doitsujin
Copy link
Owner

5fe4c4f should fix the Non-SRGB image -> SRGB view case. Is the image that the game creates using VK_FORMAT_R8G8B8A8_UNORM?

VK_IMAGE_CREATE_EXTENDED_USAGE_BIT_KHR should not be necessary since I believe that setting D3D11 bind flags not supported for the image format is illegal and should fail (which it currently does), but I'll check if there are maybe exceptions for SRGB formats.

@MrChebik
Copy link

It worked.

@gfxstrand
Copy link
Author

Thanks for working on this! I'm not sure if the image in question was created as RGB or sRGB off-hand. I didn't bother digging that far because I new it was wrong either way and debugging in wine is a pain. I'll try and dig that information op on Monday to make sure you don't need VK_IMAGE_CREATE_EXTENDED_USAGE_BIT_KHR.

@doitsujin
Copy link
Owner

doitsujin commented Jul 22, 2018

I checked what happens on Windows when trying to create an UNORM view for an SRGB image and it fails there, so this is indeed illegal. Supported usage flags for UNORM images should be a superset of the supported flags for the corresponding SRGB format.

Typeless formats might still cause trouble but as long as the usage flags supported for the corresponding Vulkan format are a superset of the usage flags supported for all compatible view formats, it should be fine.

The problem is that I'm using vkGetPhysicalDeviceImageFormatProperties with the usage flags in order to determine whether the image has to be created with OPTIMAL or LINEAR tiling and to check whether the given set of image parameters is supported by the device at all, but that function doesn't take image create flags into account so having to deal with unsupported usage flags would be rather painful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants