Skip to content
New issue

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

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

Already on GitHub? # to your account

Unclear whether VK_FORMAT_UNDEFINED is a "valid VkFormat" #2165

Closed
Rua opened this issue Jul 13, 2023 · 8 comments
Closed

Unclear whether VK_FORMAT_UNDEFINED is a "valid VkFormat" #2165

Rua opened this issue Jul 13, 2023 · 8 comments

Comments

@Rua
Copy link
Contributor

Rua commented Jul 13, 2023

Many APIs that take VkFormat don't explicitly say whether VK_FORMAT_UNDEFINED is allowed as one of the values or not. For example, vkGetPhysicalDeviceFormatProperties2 only says "format must be a valid VkFormat value". Since VK_FORMAT_UNDEFINED is technically a valid value of the VkFormat enum, that would imply that it's allowed for vkGetPhysicalDeviceFormatProperties2, but that seems unlikely.

On the other hand, if we take "valid" to mean "must not be VK_FORMAT_UNDEFINED", then that leads to a contradiction for APIs that do allow for that value, such as VkImageCreateInfo. That struct, too, has a VUID that says it must be a valid VkFormat value.

So the situation is very unclear. Is VK_FORMAT_UNDEFINED a "valid VkFormat value" or not? If it is, then extra VUIDs are needed where that value is disallowed. If it isn't, then adjustment is needed where that value is allowed.

@krOoze
Copy link
Contributor

krOoze commented Jul 13, 2023

@Rua
Copy link
Contributor Author

Rua commented Jul 13, 2023

That points towards VK_FORMAT_UNDEFINED being considered "valid". But then it seems that a lot of APIs are missing VUIDs saying that VK_FORMAT_UNDEFINED is not allowed?

@krOoze
Copy link
Contributor

krOoze commented Jul 13, 2023

getFormatProperties can return "not supported" . Can you point out a function where this input would undeniably be a problem?

PS: case of vkGetPhysicalDeviceImageFormatProperties previously mentioned in #672

@spencer-lunarg
Copy link

I understand @Rua concern here, I also have found times where I am unsure if VK_FORMAT_UNDEFINED is valid or not (especially because of things like external Android Hardware Buffers using this)

Happy to spend some time and go over where we input VkFormat and making sure everything is explicit

@spencer-lunarg
Copy link

(self note) here are all the function/struct that take a VkFormat from the application

vkGetPhysicalDeviceFormatProperties
vkGetPhysicalDeviceImageFormatProperties
vkGetPhysicalDeviceSparseImageFormatProperties
vkGetPhysicalDeviceFormatProperties2
vkGetPhysicalDeviceExternalImageFormatPropertiesNV

VkBufferViewCreateInfo
VkImageCreateInfo
VkImageViewCreateInfo
VkVertexInputAttributeDescription
VkAttachmentDescription
VkPhysicalDeviceImageFormatInfo2
VkPhysicalDeviceSparseImageFormatInfo2
VkSamplerYcbcrConversionCreateInfo
VkImageFormatListCreateInfo
VkAttachmentDescription2
VkFramebufferAttachmentImageInfo
VkPipelineRenderingCreateInfo
VkCommandBufferInheritanceRenderingInfo
VkSwapchainCreateInfoKHR
VkVideoSessionCreateInfoKHR
VkImageViewASTCDecodeModeEXT
VkGeometryTrianglesNV
VkSamplerCustomBorderColorCreateInfoEXT
VkDescriptorAddressInfoEXT
VkVertexInputAttributeDescription2EXT
VkAccelerationStructureTrianglesDisplacementMicromapNV
VkOpticalFlowSessionCreateInfoNV
VkAccelerationStructureGeometryTrianglesDataKHR

@stonesthrow
Copy link
Contributor

stonesthrow commented Jul 21, 2023

Here is the description:
VK_FORMAT_UNDEFINED specifies that the format is not specified.

"not specified", does that mean not provided? in light of the functions above. Is there no error to reject this for all those functions? of are we saying UNDEFINED is OK in some situations - in which case it would be valid.

@spencer-lunarg
Copy link

@stonesthrow I did create an internal MR at https://gitlab.khronos.org/vulkan/vulkan/-/merge_requests/6017

I think the issue is we can have VK_FORMAT_UNDEFINED for a vkImage, but still have

VUID-VkImageCreateInfo-format-parameter
format must be a valid VkFormat value

@oddhack
Copy link
Contributor

oddhack commented Jan 25, 2024

This should be fixed in the 1.3.276 spec update.

@oddhack oddhack closed this as completed Jan 25, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

5 participants