KhronosGroup / Vulkan-Docs

The Vulkan API Specification and related tools
Other
2.81k stars 470 forks source link

How to know the mandatory format support of VK_FORMAT_FEATURE_TRANSFER_SRC_BIT and VK_FORMAT_FEATURE_TRANSFER_DST_BIT #1223

Open Jiawei-Shao opened 4 years ago

Jiawei-Shao commented 4 years ago

Hi Vulkan Working Group:

I have a question about the Tables 58 - 68"Mandatory Format Support" in the Chapter 39.3 "Required Format Support".

In these tables we cannot find out if the support of VK_FORMAT_FEATURE_TRANSFER_SRC_BIT and VK_FORMAT_FEATURE_TRANSFER_DST_BIT is required or not for each format. I see the SPEC requires "formats that are required to support VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT must also support VK_FORMAT_FEATURE_TRANSFER_SRC_BIT and VK_FORMAT_FEATURE_TRANSFER_DST_BIT", but does it mean that if a format is not required to support VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT, then it is also not required to support VK_FORMAT_FEATURE_TRANSFER_SRC_BIT and VK_FORMAT_FEATURE_TRANSFER_DST_BIT?

I am meeting this issue when I am investigating if it is allowed to copy a texture with depth stencil textures in Vulkan SPEC. From Table 65 "Mandatory format support: depth/stencil with VkImageType VK_IMAGE_TYPE_2D" we can see only VK_FORMAT_D16_UNORM and VK_FORMAT_D32_SFLOAT support VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT, so does it mean all other depth stencil formats are not required to support being used in copy commands?

Furthermore, in Chapter 19.4 "Copying Data Between Buffers and Images", the statement "data copied to or from the depth aspect of a VK_FORMAT_X8_D24_UNORM_PACK32 or VK_FORMAT_D24_UNORM_S8_UINT format is packed with one 32-bit word per texel with the D24 value in the LSBs of the word, and undefined values in the eight MSBs" is not always true as VK_FORMAT_X8_D24_UNORM_PACK32 and VK_FORMAT_D24_UNORM_S8_UINT may not be supported to be used in copy commands. Is it correct?

NicolBolas commented 4 years ago

does it mean that if a format is not required to support VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT, then it is also not required to support VK_FORMAT_FEATURE_TRANSFER_SRC_BIT and VK_FORMAT_FEATURE_TRANSFER_DST_BIT?

That seems to be the obvious reading of the text.

the statement ... is not always true

I don't think it's necessary to explicitly condition that statement with "if copying is allowed". The statement is explaining what the behavior of an operation is. If that operation violates the valid usage, then its behavior is undefined. And since there's a valid usage rule that requires that copying only happens between images/buffers that can allow copies, the implication is that the statement only applies when copying is allowed.

krOoze commented 4 years ago

VK_FORMAT_FEATURE_TRANSFER_*_BIT are generally bit weird in the spec. The spec must be backwards compatible. That should imply any supported format (i.e. having any other feature bits set) does also support transfer.

The only difference should be when the VK_FORMAT_FEATURE_TRANSFER_*_BIT are the only bits, so these bits allow to express format that only supports transfer.

Jiawei-Shao commented 4 years ago

Hi @NicolBolas, do you mean that if a format is not required to support VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT, then the SPEC has no requirement on this format that it must support VK_FORMAT_FEATURE_TRANSFER_SRC_BIT and VK_FORMAT_FEATURE_TRANSFER_DST_BIT, so the Vulkan drivers are allowed not to support VK_FORMAT_FEATURE_TRANSFER_SRC_BIT and VK_FORMAT_FEATURE_TRANSFER_DST_BIT on the formats that are not required to support VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT?

NicolBolas commented 4 years ago

@Jiawei-Shao That is what the specification is saying.

However, krOoze brought up a good point (though obliquely). FEATURE_TRANSFER_SRC/DST were not actually a part of Vulkan 1.0. That is, an implementation was required to permit transfers on all image formats. Only with certain extensions and 1.1 could an implementation forbid transfer operations on a per-format basis.

But if Vulkan backwards compatibility is supposed to mean that any 1.0 application ought to be able to run on 1.1+ implementations, then an implementation can't forbid transfer operations on any formats that weren't actually available to 1.0. Or at the very least, if you ask for a 1.0 implementation, then the implementation can't forbid it.

krOoze commented 4 years ago

@NicolBolas Right. Except I don't think it is format dependent. Even newly added formats should behave that way as long as they do not say differently in the extension and the extension is for Vulkan 1.0. And except I don't think apiVersion passed to vkCreateInstance matters; new versions should be purely additive and app that ignores new enumerants returned from API should work the same as before. For that matter same should apply to maintenance1 extension; extensions are also allowed to be only purely additive.

PS: I am getting a deja-vu. This is actually the point of the https://github.com/KhronosGroup/Vulkan-Docs/issues/639 issue.

PPS: keep in mind we are talking about the spec. That this problem is not exactly 110 % clear means the drivers probably do whatever they want anyway...

Jiawei-Shao commented 4 years ago

Thanks @krOoze and @NicolBolas. So will Vulkan WG discuss the issue #639 again to give a clearer explanation?

gfxstrand commented 1 year ago

We talked about this in our working group meeting today and I volunteered to do a bit of archeology. I think the best bit to latch onto is this:

If the application apiVersion is Vulkan 1.0 and VK_KHR_maintenance1 is not supported, VK_FORMAT_FEATURE_TRANSFER_SRC_BIT is implied to be set when the format feature flag is not 0.

The transfer src/dst flags were originally added to answer the question "Can you create an image for a format with no supported feature bits?" Some implementations were returning success for vkPhysicalDeviceGetImageFormatFeatures() for formats which didn't support any other bits. Unfortunately, we weren't so careful when defining when drivers had to advertise the new transfer bits. There is one bit of spec text around this, though:

Formats that are required to support VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT must also support VK_FORMAT_FEATURE_TRANSFER_SRC_BIT and VK_FORMAT_FEATURE_TRANSFER_DST_BIT.

That means it's implicitly in the required feature bits table whenever SAMPLED is which is set for all required image formats except depth which gets a bit funny.

Based on my archeology, I believe the intent was that drivers should advertise TRANSFER_SRC and TRANSFER_DST whenever they advertise any other image format feature bit (as opposed to buffer format bits). However, it does not appear that we have any such spec text today and there are likely no tests for this either. We'll need to confirm within the working group before that understanding becomes spec, though.