KhronosGroup / Vulkan-Docs

The Vulkan API Specification and related tools
Other
2.7k stars 452 forks source link

`VUID-VkMemoryMapInfoKHR-flags-09572` and `VUID-VkMemoryMapInfoKHR-flags-09574` seem needlessly restrictive #2350

Closed marc0246 closed 1 month ago

marc0246 commented 2 months ago

VUID-VkMemoryMapInfoKHR-flags-09572 and VUID-VkMemoryMapInfoKHR-flags-09574 only accept VK_WHOLE_SIZE and not also the rest of the allocation size like the rest of the APIs that can take VK_WHOLE_SIZE. For example, without using the VK_EXT_map_memory_placed extension, one can specify the rest of the allocation or VK_WHOLE_SIZE. Same applies to VkMappedMemoryRange, you can use both the rest of the allocation size or VK_WHOLE_SIZE. It feels weird to have this special case only when using this extension. It means that, since we never used and don't intend on using VK_WHOLE_SIZE, we are currently special casing this such that the rest of the allocation size is translated to VK_WHOLE_SIZE, only when using this extension. That feels rather strange, and I think it would be better if the rest of the allocation size was also permitted natively.

spencer-lunarg commented 2 months ago

cc @gfxstrand

gfxstrand commented 2 months ago

I think this should be fine with the Mesa implementations. If @cubanismo can confirm it's okay on NVIDIA, we can make this change.

cubanismo commented 2 months ago

Thanks for the @. I've asked the engineer that implemented this to check.

cubanismo commented 2 months ago

We expect this will work fine on our implementation as-is, so fine with us to make the change.

gfxstrand commented 2 months ago

Here's a the validation change: https://github.com/KhronosGroup/Vulkan-ValidationLayers/pull/7888 I'll make a spec MR and run that through the internal process.

oddhack commented 1 month ago

This should be fixed in the 1.3.285 spec update.