Browse Source

Vk: tighten rules for extension-dependent code paths.

To ensure a non-success VkResult is never missed.
pull/491/head
Vladimír Vondruš 5 years ago
parent
commit
271f8b6fec
  1. 32
      doc/developers.dox
  2. 21
      src/Magnum/Vk/Buffer.cpp
  3. 6
      src/Magnum/Vk/Buffer.h
  4. 21
      src/Magnum/Vk/Image.cpp
  5. 6
      src/Magnum/Vk/Image.h
  6. 4
      src/Magnum/Vk/Implementation/DeviceState.h

32
doc/developers.dox

@ -778,6 +778,38 @@ In order to remove Vulkan functionality, be sure to touch all places mentioned
above, only in inverse --- but usually @ref developers-deprecation "deprecate first", above, only in inverse --- but usually @ref developers-deprecation "deprecate first",
unless it doesn't affect public API at all. unless it doesn't affect public API at all.
@section developers-vk-extension-dependent Checklist for Vulkan extension-dependent code paths
Every time an extension-dependent code path is expected to be used several
times (such as render pass creation, but not one-time device property query,
for example), it should be implemented through a `*Implementation*()` function
pointers on a class that needs them:
- The `*Implementation()` functions should be suffixed depending on version /
extension that adds them (so e.g. `ImplementationDefault`,
`ImplementationKHR`, `Implementation12`)
- The implementation should be `MAGNUM_VK_LOCAL` and @cpp static @ce, taking
the class via @cpp Type& self @ce if necessary. This avoids extra overhead
coming from member function pointers and doesn't require an @cpp #include @ce
of the full type on MSVC in order to avoid ABI issues (as insane as it
sounds, MSVC member function pointer size is different depending if the
type is incomplete or not, causing hard-to-debug crashes).
- Their API should match the latest functionality and doing compatibility
steps for previous versions, not the other way around (so for example
extracting "version 1" structures from "version 2" in order to call the
default functionality)
- The return value of the concrete Vulkan call should be always propagated
upwards with a @cpp return @ce, *even if* the call returns @cpp void @ce,
and the call site should be wrapping it in a
@ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(). This is preferred over having the
assert repeated for each code path, as it generates less code.
- @cpp Implementation::InstanceState @ce / @cpp Implementation::DeviceState @ce
then contains the dispatching function pointer, choosing the appropriate
version based on what version and extensions are available.
- Finally, the extension should be implicitly enabled during instance /
device creation as appropriate --- for example, if the extension is core in
1.2, the extension should be enabled only on 1.1 and below.
@section developers-dependency Checklist for adding, removing or updating a dependency @section developers-dependency Checklist for adding, removing or updating a dependency
1. Verify that there are no important clients stuck on the old version with no 1. Verify that there are no important clients stuck on the old version with no

21
src/Magnum/Vk/Buffer.cpp

@ -105,7 +105,7 @@ void Buffer::bindMemory(Memory& memory, const UnsignedLong offset) {
info.buffer = _handle; info.buffer = _handle;
info.memory = memory; info.memory = memory;
info.memoryOffset = offset; info.memoryOffset = offset;
_device->state().bindBufferMemoryImplementation(*_device, 1, &info); MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(_device->state().bindBufferMemoryImplementation(*_device, 1, &info));
} }
void Buffer::bindDedicatedMemory(Memory&& memory) { void Buffer::bindDedicatedMemory(Memory&& memory) {
@ -132,28 +132,29 @@ VkBuffer Buffer::release() {
} }
void Buffer::getMemoryRequirementsImplementationDefault(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) { void Buffer::getMemoryRequirementsImplementationDefault(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) {
device->GetBufferMemoryRequirements(device, info.buffer, &requirements.memoryRequirements); return device->GetBufferMemoryRequirements(device, info.buffer, &requirements.memoryRequirements);
} }
void Buffer::getMemoryRequirementsImplementationKHR(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) { void Buffer::getMemoryRequirementsImplementationKHR(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) {
device->GetBufferMemoryRequirements2KHR(device, &info, &requirements); return device->GetBufferMemoryRequirements2KHR(device, &info, &requirements);
} }
void Buffer::getMemoryRequirementsImplementation11(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) { void Buffer::getMemoryRequirementsImplementation11(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) {
device->GetBufferMemoryRequirements2(device, &info, &requirements); return device->GetBufferMemoryRequirements2(device, &info, &requirements);
} }
void Buffer::bindMemoryImplementationDefault(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* const infos) { VkResult Buffer::bindMemoryImplementationDefault(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* const infos) {
for(std::size_t i = 0; i != count; ++i) for(std::size_t i = 0; i != count; ++i)
device->BindBufferMemory(device, infos[i].buffer, infos[i].memory, infos[i].memoryOffset); if(VkResult result = device->BindBufferMemory(device, infos[i].buffer, infos[i].memory, infos[i].memoryOffset)) return result;
return VK_SUCCESS;
} }
void Buffer::bindMemoryImplementationKHR(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* const infos) { VkResult Buffer::bindMemoryImplementationKHR(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* const infos) {
device->BindBufferMemory2KHR(device, count, infos); return device->BindBufferMemory2KHR(device, count, infos);
} }
void Buffer::bindMemoryImplementation11(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* const infos) { VkResult Buffer::bindMemoryImplementation11(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* const infos) {
device->BindBufferMemory2(device, count, infos); return device->BindBufferMemory2(device, count, infos);
} }
}} }}

6
src/Magnum/Vk/Buffer.h

@ -349,9 +349,9 @@ class MAGNUM_VK_EXPORT Buffer {
MAGNUM_VK_LOCAL static void getMemoryRequirementsImplementationKHR(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements); MAGNUM_VK_LOCAL static void getMemoryRequirementsImplementationKHR(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements);
MAGNUM_VK_LOCAL static void getMemoryRequirementsImplementation11(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements); MAGNUM_VK_LOCAL static void getMemoryRequirementsImplementation11(Device& device, const VkBufferMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements);
MAGNUM_VK_LOCAL static void bindMemoryImplementationDefault(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* infos); MAGNUM_VK_LOCAL static VkResult bindMemoryImplementationDefault(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* infos);
MAGNUM_VK_LOCAL static void bindMemoryImplementationKHR(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* infos); MAGNUM_VK_LOCAL static VkResult bindMemoryImplementationKHR(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* infos);
MAGNUM_VK_LOCAL static void bindMemoryImplementation11(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* infos); MAGNUM_VK_LOCAL static VkResult bindMemoryImplementation11(Device& device, UnsignedInt count, const VkBindBufferMemoryInfo* infos);
/* Can't be a reference because of the NoCreate constructor */ /* Can't be a reference because of the NoCreate constructor */
Device* _device; Device* _device;

21
src/Magnum/Vk/Image.cpp

@ -122,7 +122,7 @@ void Image::bindMemory(Memory& memory, const UnsignedLong offset) {
info.image = _handle; info.image = _handle;
info.memory = memory; info.memory = memory;
info.memoryOffset = offset; info.memoryOffset = offset;
_device->state().bindImageMemoryImplementation(*_device, 1, &info); MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(_device->state().bindImageMemoryImplementation(*_device, 1, &info));
} }
void Image::bindDedicatedMemory(Memory&& memory) { void Image::bindDedicatedMemory(Memory&& memory) {
@ -149,28 +149,29 @@ VkImage Image::release() {
} }
void Image::getMemoryRequirementsImplementationDefault(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) { void Image::getMemoryRequirementsImplementationDefault(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) {
device->GetImageMemoryRequirements(device, info.image, &requirements.memoryRequirements); return device->GetImageMemoryRequirements(device, info.image, &requirements.memoryRequirements);
} }
void Image::getMemoryRequirementsImplementationKHR(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) { void Image::getMemoryRequirementsImplementationKHR(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) {
device->GetImageMemoryRequirements2KHR(device, &info, &requirements); return device->GetImageMemoryRequirements2KHR(device, &info, &requirements);
} }
void Image::getMemoryRequirementsImplementation11(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) { void Image::getMemoryRequirementsImplementation11(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements) {
device->GetImageMemoryRequirements2(device, &info, &requirements); return device->GetImageMemoryRequirements2(device, &info, &requirements);
} }
void Image::bindMemoryImplementationDefault(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* const infos) { VkResult Image::bindMemoryImplementationDefault(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* const infos) {
for(std::size_t i = 0; i != count; ++i) for(std::size_t i = 0; i != count; ++i)
device->BindImageMemory(device, infos[i].image, infos[i].memory, infos[i].memoryOffset); if(VkResult result = device->BindImageMemory(device, infos[i].image, infos[i].memory, infos[i].memoryOffset)) return result;
return VK_SUCCESS;
} }
void Image::bindMemoryImplementationKHR(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* const infos) { VkResult Image::bindMemoryImplementationKHR(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* const infos) {
device->BindImageMemory2KHR(device, count, infos); return device->BindImageMemory2KHR(device, count, infos);
} }
void Image::bindMemoryImplementation11(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* const infos) { VkResult Image::bindMemoryImplementation11(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* const infos) {
device->BindImageMemory2(device, count, infos); return device->BindImageMemory2(device, count, infos);
} }
}} }}

6
src/Magnum/Vk/Image.h

@ -512,9 +512,9 @@ class MAGNUM_VK_EXPORT Image {
MAGNUM_VK_LOCAL static void getMemoryRequirementsImplementationKHR(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements); MAGNUM_VK_LOCAL static void getMemoryRequirementsImplementationKHR(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements);
MAGNUM_VK_LOCAL static void getMemoryRequirementsImplementation11(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements); MAGNUM_VK_LOCAL static void getMemoryRequirementsImplementation11(Device& device, const VkImageMemoryRequirementsInfo2& info, VkMemoryRequirements2& requirements);
MAGNUM_VK_LOCAL static void bindMemoryImplementationDefault(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* infos); MAGNUM_VK_LOCAL static VkResult bindMemoryImplementationDefault(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* infos);
MAGNUM_VK_LOCAL static void bindMemoryImplementationKHR(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* infos); MAGNUM_VK_LOCAL static VkResult bindMemoryImplementationKHR(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* infos);
MAGNUM_VK_LOCAL static void bindMemoryImplementation11(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* infos); MAGNUM_VK_LOCAL static VkResult bindMemoryImplementation11(Device& device, UnsignedInt count, const VkBindImageMemoryInfo* infos);
/* Can't be a reference because of the NoCreate constructor */ /* Can't be a reference because of the NoCreate constructor */
Device* _device; Device* _device;

4
src/Magnum/Vk/Implementation/DeviceState.h

@ -37,8 +37,8 @@ struct DeviceState {
/** @todo put this eventually into a dedicated buffer / image state struct? */ /** @todo put this eventually into a dedicated buffer / image state struct? */
void(*getBufferMemoryRequirementsImplementation)(Device&, const VkBufferMemoryRequirementsInfo2&, VkMemoryRequirements2&); void(*getBufferMemoryRequirementsImplementation)(Device&, const VkBufferMemoryRequirementsInfo2&, VkMemoryRequirements2&);
void(*getImageMemoryRequirementsImplementation)(Device&, const VkImageMemoryRequirementsInfo2&, VkMemoryRequirements2&); void(*getImageMemoryRequirementsImplementation)(Device&, const VkImageMemoryRequirementsInfo2&, VkMemoryRequirements2&);
void(*bindBufferMemoryImplementation)(Device&, UnsignedInt, const VkBindBufferMemoryInfo*); VkResult(*bindBufferMemoryImplementation)(Device&, UnsignedInt, const VkBindBufferMemoryInfo*);
void(*bindImageMemoryImplementation)(Device&, UnsignedInt, const VkBindImageMemoryInfo*); VkResult(*bindImageMemoryImplementation)(Device&, UnsignedInt, const VkBindImageMemoryInfo*);
}; };
}}} }}}

Loading…
Cancel
Save