From 2bcb5d6d9ef3dc0bd00e5cb6ea0c63dee6475b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 30 Dec 2020 16:12:36 +0100 Subject: [PATCH] Vk: more useful MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(). Another breaking change here, sorry -- I don't expect anyone to be seriously using these ... yet :) --- doc/snippets/MagnumVk.cpp | 16 ++++++++ src/Magnum/Vk/Assert.h | 50 ++++++++++++++--------- src/Magnum/Vk/DeviceProperties.cpp | 2 +- src/Magnum/Vk/Result.h | 2 +- src/Magnum/Vk/Test/AssertDisabledTest.cpp | 26 ++++++++---- src/Magnum/Vk/Test/AssertTest.cpp | 48 ++++++++++++++-------- src/Magnum/Vk/Test/CMakeLists.txt | 14 +++---- 7 files changed, 104 insertions(+), 54 deletions(-) diff --git a/doc/snippets/MagnumVk.cpp b/doc/snippets/MagnumVk.cpp index 41a88a7a4..b8fe74927 100644 --- a/doc/snippets/MagnumVk.cpp +++ b/doc/snippets/MagnumVk.cpp @@ -30,6 +30,7 @@ #include "Magnum/Magnum.h" #include "Magnum/Math/Color.h" +#include "Magnum/Vk/Assert.h" #include "Magnum/Vk/BufferCreateInfo.h" #include "Magnum/Vk/CommandBuffer.h" #include "Magnum/Vk/CommandPoolCreateInfo.h" @@ -48,6 +49,7 @@ #include "Magnum/Vk/MemoryAllocateInfo.h" #include "Magnum/Vk/Queue.h" #include "Magnum/Vk/RenderPassCreateInfo.h" +#include "Magnum/Vk/Result.h" #include "Magnum/Vk/ShaderCreateInfo.h" #include "MagnumExternal/Vulkan/flextVkGlobal.h" @@ -133,6 +135,20 @@ Vk::Device device{instance, std::move(info)}; /* [wrapping-optimizing-properties-device-move] */ } +{ +Vk::Device device{NoCreate}; +VkFence fence{}; +/* [MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR] */ +const Vk::Result result = MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(NotReady, + vkGetFenceStatus(device, fence)); +if(result == Vk::Result::Success) { + // signaled +} else { + // Vk::Result::NotReady, not signaled yet +} +/* [MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR] */ +} + { Vk::Device device{NoCreate}; /* The include should be a no-op here since it was already included above */ diff --git a/src/Magnum/Vk/Assert.h b/src/Magnum/Vk/Assert.h index a3f30856e..e8203dc1e 100644 --- a/src/Magnum/Vk/Assert.h +++ b/src/Magnum/Vk/Assert.h @@ -26,7 +26,7 @@ */ /** @file - * @brief Macro @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(), @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE() + * @brief Macro @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(), @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR() * @m_since_latest */ @@ -35,7 +35,7 @@ #include "Magnum/configure.h" -#if !defined(CORRADE_NO_ASSERT) && (!defined(MAGNUM_VK_INTERNAL_ASSERT_SUCCESS) || !defined(MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE)) +#if !defined(CORRADE_NO_ASSERT) && (!defined(MAGNUM_VK_INTERNAL_ASSERT_SUCCESS) || !defined(MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR)) #ifndef CORRADE_STANDARD_ASSERT #include #include @@ -60,7 +60,7 @@ Vulkan functions returning @type_vk{Result} and APIs returning You can override this implementation by placing your own @cpp #define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS @ce before including the @ref Magnum/Vk/Assert.h header. -@see @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE() +@see @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR() */ #ifndef MAGNUM_VK_INTERNAL_ASSERT_SUCCESS #if defined(CORRADE_NO_ASSERT) || (defined(CORRADE_STANDARD_ASSERT) && defined(NDEBUG)) @@ -82,36 +82,48 @@ You can override this implementation by placing your own #endif /** -@brief Assert that a Vulkan function call succeeds or returns incomplete data +@brief Assert that a Vulkan function call succeeds or returns the specified result @m_since_latest A variant of @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS() that allows the call to -return @ref Magnum::Vk::Result::Incomplete "Vk::Result::Incomplete" in addition -to @ref Magnum::Vk::Result::Success "Vk::Result::Success". +return specified @p result in addition to +@ref Magnum::Vk::Result::Success "Vk::Result::Success". The value specified in +@p result is directly the (unscoped) enum value and the macro returns the +actual result value. Example usage: -You can override this implementation by placing your own -@cpp #define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE @ce before +@snippet MagnumVk.cpp MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR + +Similarly to @ref CORRADE_INTERNAL_ASSERT_EXPRESSION() this macro is usable in +any expression such as @cpp if @ce and @cpp return @ce statements. You can +override this implementation by placing your own +@cpp #define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR @ce before including the @ref Magnum/Vk/Assert.h header. */ -#ifndef MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE +#ifndef MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR #if defined(CORRADE_NO_ASSERT) || (defined(CORRADE_STANDARD_ASSERT) && defined(NDEBUG)) -#define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(call) \ - static_cast(call) +/* Defining it to just Magnum::Vk::Result(call) causes ugly warnings with + asserts disabled, so it has to be a lambda even here :( */ +#define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(result, call) \ + [&]() { \ + return Magnum::Vk::Result(call); \ + }() #elif defined(CORRADE_STANDARD_ASSERT) -#define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(call) \ - do { \ +#define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(result, call) \ + [&]() { \ const Magnum::Vk::Result _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) = Magnum::Vk::Result(call); \ - assert(_CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) == Magnum::Vk::Result::Success || _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) == Magnum::Vk::Result::Incomplete); \ - } while(false) + assert(_CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) == Magnum::Vk::Result::Success || _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) == Magnum::Vk::Result::result); \ + return _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__); \ + }() #else -#define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(call) \ - do { \ +#define MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(result, call) \ + [&]() { \ const Magnum::Vk::Result _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) = Magnum::Vk::Result(call); \ - if(_CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) != Magnum::Vk::Result::Success && _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) != Magnum::Vk::Result::Incomplete) { \ + if(_CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) != Magnum::Vk::Result::Success && _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) != Magnum::Vk::Result::result) { \ Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Call " #call " failed with" << _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__) << "at " __FILE__ ":" CORRADE_LINE_STRING; \ std::abort(); \ } \ - } while(false) + return _CORRADE_HELPER_PASTE(magnumVkResult, __LINE__); \ + }() #endif #endif diff --git a/src/Magnum/Vk/DeviceProperties.cpp b/src/Magnum/Vk/DeviceProperties.cpp index aab422e16..8b566102d 100644 --- a/src/Magnum/Vk/DeviceProperties.cpp +++ b/src/Magnum/Vk/DeviceProperties.cpp @@ -545,7 +545,7 @@ UnsignedInt enumerateDevicesInto(Instance& instance, Containers::ArrayView handles{reinterpret_cast(out.data()), out.size()}; UnsignedInt count = out.size(); - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(instance->EnumeratePhysicalDevices(instance, &count, handles.data())); + MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(Incomplete, instance->EnumeratePhysicalDevices(instance, &count, handles.data())); /* Expect the final count isn't larger than the output array */ CORRADE_INTERNAL_ASSERT(count <= out.size()); diff --git a/src/Magnum/Vk/Result.h b/src/Magnum/Vk/Result.h index 883e78a6b..10cee4d66 100644 --- a/src/Magnum/Vk/Result.h +++ b/src/Magnum/Vk/Result.h @@ -43,7 +43,7 @@ namespace Magnum { namespace Vk { Wraps a @type_vk_keyword{Result}. @m_enum_values_as_keywords @see @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS(), - @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE() + @ref MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR() */ enum class Result: Int { /** Command successfully completed */ diff --git a/src/Magnum/Vk/Test/AssertDisabledTest.cpp b/src/Magnum/Vk/Test/AssertDisabledTest.cpp index 06805350a..98bec18de 100644 --- a/src/Magnum/Vk/Test/AssertDisabledTest.cpp +++ b/src/Magnum/Vk/Test/AssertDisabledTest.cpp @@ -41,16 +41,16 @@ struct AssertDisabledTest: TestSuite::Tester { explicit AssertDisabledTest(); void success(); - void successOrIncomplete(); + void successOr(); void vkSuccess(); - void vkSuccessOrIncomplete(); + void vkSuccessOr(); }; AssertDisabledTest::AssertDisabledTest() { addTests({&AssertDisabledTest::success, - &AssertDisabledTest::successOrIncomplete, + &AssertDisabledTest::successOr, &AssertDisabledTest::vkSuccess, - &AssertDisabledTest::vkSuccessOrIncomplete}); + &AssertDisabledTest::vkSuccessOr}); #ifdef CORRADE_STANDARD_ASSERT setTestName("Magum::Vk::Test::AssertStandardDisabledTest"); @@ -69,16 +69,21 @@ void AssertDisabledTest::success() { CORRADE_COMPARE(out.str(), ""); } -void AssertDisabledTest::successOrIncomplete() { +void AssertDisabledTest::successOr() { std::ostringstream out; Error redirectError{&out}; Result a = Result::ErrorUnknown; Result r = Result::ErrorExtensionNotPresent; - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(a = r); + Result a2 = MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(Incomplete, a = r); CORRADE_COMPARE(a, Result::ErrorExtensionNotPresent); + CORRADE_COMPARE(a2, a); CORRADE_COMPARE(out.str(), ""); + + /* Test also that a standalone macro won't cause warnings about unused + expression results */ + MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(ErrorDeviceLost, Result::ErrorDeviceLost); } void AssertDisabledTest::vkSuccess() { @@ -93,16 +98,21 @@ void AssertDisabledTest::vkSuccess() { CORRADE_COMPARE(out.str(), ""); } -void AssertDisabledTest::vkSuccessOrIncomplete() { +void AssertDisabledTest::vkSuccessOr() { std::ostringstream out; Error redirectError{&out}; VkResult b = VK_ERROR_UNKNOWN; VkResult s = VK_ERROR_EXTENSION_NOT_PRESENT; - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(b = s); + Result b2 = MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(Incomplete, b = s); CORRADE_COMPARE(Result(b), Result::ErrorExtensionNotPresent); + CORRADE_COMPARE(b2, Result(b)); CORRADE_COMPARE(out.str(), ""); + + /* Test also that a standalone macro won't cause warnings about unused + expression results */ + MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(ErrorDeviceLost, VK_ERROR_DEVICE_LOST); } }}}} diff --git a/src/Magnum/Vk/Test/AssertTest.cpp b/src/Magnum/Vk/Test/AssertTest.cpp index 0add9ed65..5459e10ce 100644 --- a/src/Magnum/Vk/Test/AssertTest.cpp +++ b/src/Magnum/Vk/Test/AssertTest.cpp @@ -39,31 +39,31 @@ struct AssertTest: TestSuite::Tester { explicit AssertTest(); void success(); - void successOrIncomplete(); + void successOr(); void vkSuccess(); - void vkSuccessOrIncomplete(); + void vkSuccessOr(); - bool _failAssertSuccess, _failAssertSuccessOrIncomplete, - _failAssertVkSuccess, _failAssertVkSuccessOrIncomplete; + bool _failAssertSuccess, _failAssertSuccessOr, + _failAssertVkSuccess, _failAssertVkSuccessOr; }; AssertTest::AssertTest(): TestSuite::Tester{TesterConfiguration{}.setSkippedArgumentPrefixes({"fail-on"})} { addTests({&AssertTest::success, - &AssertTest::successOrIncomplete, + &AssertTest::successOr, &AssertTest::vkSuccess, - &AssertTest::vkSuccessOrIncomplete}); + &AssertTest::vkSuccessOr}); Utility::Arguments args{"fail-on"}; args.addOption("assert-success", "false").setHelp("assert-success", "fail on MAGNUM_VK_INTERNAL_ASSERT_SUCCESS() with Vk::Result", "BOOL") - .addOption("assert-success-or-incomplete", "false").setHelp("assert-success", "fail on MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE() with Vk::Result", "BOOL") + .addOption("assert-success-or", "false").setHelp("assert-success-or", "fail on MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR() with Vk::Result", "BOOL") .addOption("assert-vk-success", "false").setHelp("assert-vk-success", "fail on MAGNUM_VK_INTERNAL_ASSERT_SUCCESS() with VkResult", "BOOL") - .addOption("assert-vk-success-or-incomplete", "false").setHelp("assert-vk-success-or-incomplete", "fail on MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE() with VkResult", "BOOL") + .addOption("assert-vk-success-or", "false").setHelp("assert-vk-success-or", "fail on MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR() with VkResult", "BOOL") .parse(arguments().first, arguments().second); _failAssertSuccess = args.value("assert-success"); - _failAssertSuccessOrIncomplete = args.value("assert-success-or-incomplete"); + _failAssertSuccessOr = args.value("assert-success-or"); _failAssertVkSuccess = args.value("assert-vk-success"); - _failAssertVkSuccessOrIncomplete = args.value("assert-vk-success-or-incomplete"); + _failAssertVkSuccessOr = args.value("assert-vk-success-or"); #ifdef CORRADE_STANDARD_ASSERT setTestName("Magum::Vk::Test::AssertStandardTest"); @@ -79,14 +79,20 @@ void AssertTest::success() { CORRADE_COMPARE(a, Result::Success); } -void AssertTest::successOrIncomplete() { +void AssertTest::successOr() { Result a = Result::ErrorUnknown; - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(a = Result::Success); + Result a2 = MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(Incomplete, a = Result::Success); + CORRADE_COMPARE(a2, a); - Result r = _failAssertSuccessOrIncomplete ? Result::ErrorExtensionNotPresent : Result::Incomplete; - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(a = r); + Result r = _failAssertSuccessOr ? Result::ErrorExtensionNotPresent : Result::Incomplete; + Result a3 = MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(Incomplete, a = r); CORRADE_COMPARE(a, Result::Incomplete); + CORRADE_COMPARE(a3, a); + + /* Test also that a standalone macro won't cause warnings about unused + expression results */ + MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(ErrorDeviceLost, Result::ErrorDeviceLost); } void AssertTest::vkSuccess() { @@ -98,14 +104,20 @@ void AssertTest::vkSuccess() { CORRADE_COMPARE(Result(a), Result::Success); } -void AssertTest::vkSuccessOrIncomplete() { +void AssertTest::vkSuccessOr() { VkResult a = VK_ERROR_UNKNOWN; - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(a = VK_SUCCESS); + Result a2 = MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(Incomplete, a = VK_SUCCESS); + CORRADE_COMPARE(a2, Result(a)); - VkResult s = _failAssertVkSuccessOrIncomplete ? VK_ERROR_EXTENSION_NOT_PRESENT : VK_INCOMPLETE; - MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR_INCOMPLETE(a = s); + VkResult s = _failAssertVkSuccessOr ? VK_ERROR_EXTENSION_NOT_PRESENT : VK_INCOMPLETE; + Result a3 = MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(Incomplete, a = s); CORRADE_COMPARE(Result(a), Result::Incomplete); + CORRADE_COMPARE(a3, Result(a)); + + /* Test also that a standalone macro won't cause warnings about unused + expression results */ + MAGNUM_VK_INTERNAL_ASSERT_SUCCESS_OR(ErrorDeviceLost, VK_ERROR_DEVICE_LOST); } }}}} diff --git a/src/Magnum/Vk/Test/CMakeLists.txt b/src/Magnum/Vk/Test/CMakeLists.txt index 78e25361e..64d1760b6 100644 --- a/src/Magnum/Vk/Test/CMakeLists.txt +++ b/src/Magnum/Vk/Test/CMakeLists.txt @@ -67,12 +67,12 @@ if(CORRADE_TARGET_ANDROID) LIBRARIES MagnumVk) set_tests_properties(VkAssertTestFailAssertSuccess PROPERTIES PASS_REGULAR_EXPRESSION "Call a = r failed with Vk::Result::ErrorFragmentedPool at ") - corrade_add_test(VkAssertTestFailAssertSucce___Incomplete + corrade_add_test(VkAssertTestFailAssertSuccessOr $ ${PROJECT_SOURCE_DIR}/src/dummy.cpp - ARGUMENTS --fail-on-assert-success-or-incomplete true + ARGUMENTS --fail-on-assert-success-or true LIBRARIES MagnumVk) - set_tests_properties(VkAssertTestFailAssertSucce___Incomplete PROPERTIES + set_tests_properties(VkAssertTestFailAssertSuccessOr PROPERTIES PASS_REGULAR_EXPRESSION "Call a = r failed with Vk::Result::ErrorExtensionNotPresent at ") corrade_add_test(VkAssertTestFailAssertVkSuccess $ @@ -81,19 +81,19 @@ if(CORRADE_TARGET_ANDROID) LIBRARIES MagnumVk) set_tests_properties(VkAssertTestFailAssertVkSuccess PROPERTIES PASS_REGULAR_EXPRESSION "Call a = s failed with Vk::Result::ErrorFragmentedPool at ") - corrade_add_test(VkAssertTestFailAssertVkSuc___Incomplete + corrade_add_test(VkAssertTestFailAssertVkSuccessOr $ ${PROJECT_SOURCE_DIR}/src/dummy.cpp ARGUMENTS --fail-on-assert-vk-success-or-incomplete true LIBRARIES MagnumVk) - set_tests_properties(VkAssertTestFailAssertVkSuc___Incomplete PROPERTIES + set_tests_properties(VkAssertTestFailAssertVkSuccessOr PROPERTIES PASS_REGULAR_EXPRESSION "Call a = s failed with Vk::Result::ErrorExtensionNotPresent at ") set_target_properties( VkAssertTestFailAssertSuccess - VkAssertTestFailAssertSucce___Incomplete + VkAssertTestFailAssertSuccessOr VkAssertTestFailAssertVkSuccess - VkAssertTestFailAssertVkSuc___Incomplete + VkAssertTestFailAssertVkSuccessOr PROPERTIES FOLDER "Magnum/Vk/Test") endif()