From 1793b65ec1829a2cf8b0f49fc930027c71c78a39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 21 Jun 2020 22:49:44 +0200 Subject: [PATCH 1/4] Vulkan: Remove support for the old VK_EXT_debug_report extension --- CMakeLists.txt | 2 - Common/Vulkan/VulkanContext.cpp | 85 ++-------------------------- Common/Vulkan/VulkanContext.h | 9 +-- Common/Vulkan/VulkanDebug.cpp | 60 ++++---------------- Common/Vulkan/VulkanDebug.h | 1 - Common/Vulkan/VulkanLoader.cpp | 9 --- Common/Vulkan/VulkanLoader.h | 7 --- Windows/GPU/WindowsVulkanContext.cpp | 19 ++----- android/jni/AndroidVulkanContext.cpp | 35 +----------- android/jni/AndroidVulkanContext.h | 2 - 10 files changed, 24 insertions(+), 205 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a40d4334ec..9c352f1f0c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -398,8 +398,6 @@ source_group(Windows FILES ${CommonWindows}) set(CommonVulkan ${CommonExtra} Common/Vulkan/VulkanContext.cpp Common/Vulkan/VulkanContext.h - Common/Vulkan/VulkanDebug.cpp - Common/Vulkan/VulkanDebug.h Common/Vulkan/VulkanImage.cpp Common/Vulkan/VulkanImage.h Common/Vulkan/VulkanLoader.cpp diff --git a/Common/Vulkan/VulkanContext.cpp b/Common/Vulkan/VulkanContext.cpp index b64def5502..0017df5ba6 100644 --- a/Common/Vulkan/VulkanContext.cpp +++ b/Common/Vulkan/VulkanContext.cpp @@ -7,7 +7,8 @@ #include "base/basictypes.h" #include "base/display.h" -#include "VulkanContext.h" +#include "Common/Vulkan/VulkanContext.h" +#include "Common/Vulkan/VulkanDebug.h" #include "GPU/Common/ShaderCommon.h" #include "Common/StringUtils.h" #include "Core/Config.h" @@ -134,14 +135,6 @@ VkResult VulkanContext::CreateInstance(const CreateInfo &info) { instance_extensions_enabled_.push_back(VK_EXT_DEBUG_UTILS_EXTENSION_NAME); extensionsLookup_.EXT_debug_utils = true; ILOG("Vulkan debug_utils validation enabled."); - } else if (IsInstanceExtensionAvailable(VK_EXT_DEBUG_REPORT_EXTENSION_NAME)) { - for (size_t i = 0; i < ARRAY_SIZE(validationLayers); i++) { - instance_layer_names_.push_back(validationLayers[i]); - device_layer_names_.push_back(validationLayers[i]); - } - instance_extensions_enabled_.push_back(VK_EXT_DEBUG_REPORT_EXTENSION_NAME); - extensionsLookup_.EXT_debug_report = true; - ILOG("Vulkan debug_report validation enabled."); } else { ELOG("Validation layer extension not available - not enabling Vulkan validation."); flags_ &= ~VULKAN_FLAG_VALIDATE; @@ -668,47 +661,12 @@ VkResult VulkanContext::CreateDevice() { return res; } -VkResult VulkanContext::InitDebugMsgCallback(PFN_vkDebugReportCallbackEXT dbgFunc, int bits, void *userdata) { - VkDebugReportCallbackEXT msg_callback; - - if (!(flags_ & VULKAN_FLAG_VALIDATE)) { - WLOG("Not registering debug report callback - extension not enabled!"); - return VK_SUCCESS; - } - ILOG("Registering debug report callback"); - - VkDebugReportCallbackCreateInfoEXT cb{VK_STRUCTURE_TYPE_DEBUG_REPORT_CREATE_INFO_EXT}; - cb.flags = bits; - cb.pfnCallback = dbgFunc; - cb.pUserData = userdata; - VkResult res = vkCreateDebugReportCallbackEXT(instance_, &cb, nullptr, &msg_callback); - switch (res) { - case VK_SUCCESS: - msg_callbacks.push_back(msg_callback); - break; - case VK_ERROR_OUT_OF_HOST_MEMORY: - return VK_ERROR_INITIALIZATION_FAILED; - default: - return VK_ERROR_INITIALIZATION_FAILED; - } - return res; -} - -void VulkanContext::DestroyDebugMsgCallback() { - if (!extensionsLookup_.EXT_debug_report) - return; - while (msg_callbacks.size() > 0) { - vkDestroyDebugReportCallbackEXT(instance_, msg_callbacks.back(), nullptr); - msg_callbacks.pop_back(); - } -} - -VkResult VulkanContext::InitDebugUtilsCallback(PFN_vkDebugUtilsMessengerCallbackEXT callback, int bits, void *userdata) { +VkResult VulkanContext::InitDebugUtilsCallback(int bits, VulkanLogOptions *logOptions) { VkDebugUtilsMessengerCreateInfoEXT callback1{VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT}; callback1.messageSeverity = bits; callback1.messageType = VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_TYPE_VALIDATION_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT; - callback1.pfnUserCallback = callback; - callback1.pUserData = userdata; + callback1.pfnUserCallback = &VulkanDebugUtilsCallback; + callback1.pUserData = (void *)logOptions; VkDebugUtilsMessengerEXT messenger; VkResult res = vkCreateDebugUtilsMessengerEXT(instance_, &callback1, nullptr, &messenger); if (res != VK_SUCCESS) { @@ -1445,36 +1403,3 @@ std::string FormatDriverVersion(const VkPhysicalDeviceProperties &props) { uint32_t branch = VK_VERSION_PATCH(props.driverVersion); return StringFromFormat("%d.%d.%d (%08x)", major, minor, branch, props.driverVersion); } - -const char *VulkanObjTypeToString(VkDebugReportObjectTypeEXT type) { - switch (type) { - case VK_DEBUG_REPORT_OBJECT_TYPE_INSTANCE_EXT: return "Instance"; - case VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT: return "PhysicalDevice"; - case VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT: return "Device"; - case VK_DEBUG_REPORT_OBJECT_TYPE_QUEUE_EXT: return "Queue"; - case VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_BUFFER_EXT: return "CommandBuffer"; - case VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_MEMORY_EXT: return "DeviceMemory"; - case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_EXT: return "Buffer"; - case VK_DEBUG_REPORT_OBJECT_TYPE_BUFFER_VIEW_EXT: return "BufferView"; - case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_EXT: return "Image"; - case VK_DEBUG_REPORT_OBJECT_TYPE_IMAGE_VIEW_EXT: return "ImageView"; - case VK_DEBUG_REPORT_OBJECT_TYPE_SHADER_MODULE_EXT: return "ShaderModule"; - case VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_EXT: return "Pipeline"; - case VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_LAYOUT_EXT: return "PipelineLayout"; - case VK_DEBUG_REPORT_OBJECT_TYPE_SAMPLER_EXT: return "Sampler"; - case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_EXT: return "DescriptorSet"; - case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_SET_LAYOUT_EXT: return "DescriptorSetLayout"; - case VK_DEBUG_REPORT_OBJECT_TYPE_DESCRIPTOR_POOL_EXT: return "DescriptorPool"; - case VK_DEBUG_REPORT_OBJECT_TYPE_FENCE_EXT: return "Fence"; - case VK_DEBUG_REPORT_OBJECT_TYPE_SEMAPHORE_EXT: return "Semaphore"; - case VK_DEBUG_REPORT_OBJECT_TYPE_EVENT_EXT: return "Event"; - case VK_DEBUG_REPORT_OBJECT_TYPE_QUERY_POOL_EXT: return "QueryPool"; - case VK_DEBUG_REPORT_OBJECT_TYPE_FRAMEBUFFER_EXT: return "Framebuffer"; - case VK_DEBUG_REPORT_OBJECT_TYPE_RENDER_PASS_EXT: return "RenderPass"; - case VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_CACHE_EXT: return "PipelineCache"; - case VK_DEBUG_REPORT_OBJECT_TYPE_SURFACE_KHR_EXT: return "SurfaceKHR"; - case VK_DEBUG_REPORT_OBJECT_TYPE_SWAPCHAIN_KHR_EXT: return "SwapChainKHR"; - case VK_DEBUG_REPORT_OBJECT_TYPE_COMMAND_POOL_EXT: return "CommandPool"; - default: return ""; - } -} diff --git a/Common/Vulkan/VulkanContext.h b/Common/Vulkan/VulkanContext.h index db3caa454e..3bcae0e56c 100644 --- a/Common/Vulkan/VulkanContext.h +++ b/Common/Vulkan/VulkanContext.h @@ -7,6 +7,7 @@ #include "base/logging.h" #include "Common/Vulkan/VulkanLoader.h" +#include "Common/Vulkan/VulkanDebug.h" enum { VULKAN_FLAG_VALIDATE = 1, @@ -169,13 +170,9 @@ public: bool MemoryTypeFromProperties(uint32_t typeBits, VkFlags requirements_mask, uint32_t *typeIndex); - VkResult InitDebugUtilsCallback(PFN_vkDebugUtilsMessengerCallbackEXT callback, int bits, void *userdata); + VkResult InitDebugUtilsCallback(int bits, VulkanLogOptions *logOptions); void DestroyDebugUtilsCallback(); - // Legacy reporting - VkResult InitDebugMsgCallback(PFN_vkDebugReportCallbackEXT dbgFunc, int bits, void *userdata); - void DestroyDebugMsgCallback(); - VkPhysicalDevice GetPhysicalDevice(int n) const { return physical_devices_[n]; } @@ -342,7 +339,6 @@ private: // the next time the frame comes around again. VulkanDeleteList globalDeleteList_; - std::vector msg_callbacks; std::vector utils_callbacks; VkSwapchainKHR swapchain_ = VK_NULL_HANDLE; @@ -370,7 +366,6 @@ bool GLSLtoSPV(const VkShaderStageFlagBits shader_type, const char *pshader, std const char *VulkanResultToString(VkResult res); std::string FormatDriverVersion(const VkPhysicalDeviceProperties &props); -const char *VulkanObjTypeToString(VkDebugReportObjectTypeEXT type); // Simple heuristic. bool IsHashMaliDriverVersion(const VkPhysicalDeviceProperties &props); diff --git a/Common/Vulkan/VulkanDebug.cpp b/Common/Vulkan/VulkanDebug.cpp index 26526eca96..9397338ced 100644 --- a/Common/Vulkan/VulkanDebug.cpp +++ b/Common/Vulkan/VulkanDebug.cpp @@ -19,57 +19,9 @@ #include #include +#include "base/logging.h" #include "Common/Vulkan/VulkanContext.h" #include "Common/Vulkan/VulkanDebug.h" -#include "base/logging.h" - -VkBool32 VKAPI_CALL VulkanDebugReportCallback(VkDebugReportFlagsEXT msgFlags, VkDebugReportObjectTypeEXT objType, uint64_t srcObject, size_t location, int32_t msgCode, const char* pLayerPrefix, const char* pMsg, void *pUserData) { - const VulkanLogOptions *options = (const VulkanLogOptions *)pUserData; - std::ostringstream message; - - if (msgFlags & VK_DEBUG_REPORT_ERROR_BIT_EXT) { - message << "ERROR: "; - } else if (msgFlags & VK_DEBUG_REPORT_WARNING_BIT_EXT) { - message << "WARNING: "; - } else if (msgFlags & VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT) { - message << "PERFORMANCE WARNING: "; - } else if (msgFlags & VK_DEBUG_REPORT_INFORMATION_BIT_EXT) { - message << "INFO: "; - } else if (msgFlags & VK_DEBUG_REPORT_DEBUG_BIT_EXT) { - message << "DEBUG: "; - } - message << "[" << pLayerPrefix << "] " << VulkanObjTypeToString(objType) << " Code " << msgCode << " : " << pMsg << "\n"; - - if (msgCode == 64) // Another useless perf warning that will be seen less and less as we optimize - vkCmdClearAttachments() issued on command buffer object 0x00000195296C6D40 prior to any Draw Cmds. It is recommended you use RenderPass LOAD_OP_CLEAR on Attachments prior to any Draw. - return false; - if (msgCode == 5) - return false; // Not exactly a false positive, see https://github.com/KhronosGroup/glslang/issues/1418 -#ifdef _WIN32 - std::string msg = message.str(); - OutputDebugStringA(msg.c_str()); - if (msgFlags & VK_DEBUG_REPORT_ERROR_BIT_EXT) { - if (options->breakOnError && IsDebuggerPresent()) { - DebugBreak(); - } - if (options->msgBoxOnError) { - MessageBoxA(NULL, message.str().c_str(), "Alert", MB_OK); - } - } else if (msgFlags & VK_DEBUG_REPORT_WARNING_BIT_EXT) { - if (options->breakOnWarning && IsDebuggerPresent()) { - DebugBreak(); - } - } -#else - ILOG("%s", message.str().c_str()); -#endif - - // false indicates that layer should not bail-out of an - // API call that had validation failures. This may mean that the - // app dies inside the driver due to invalid parameter(s). - // That's what would happen without validation layers, so we'll - // keep that behavior here. - return false; -} VkBool32 VKAPI_CALL VulkanDebugUtilsCallback( VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity, @@ -108,8 +60,9 @@ VkBool32 VKAPI_CALL VulkanDebugUtilsCallback( } message << ":" << pCallbackData->messageIdNumber << ") " << pMessage << "\n"; -#ifdef _WIN32 std::string msg = message.str(); + +#ifdef _WIN32 OutputDebugStringA(msg.c_str()); if (messageSeverity & VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT) { if (options->breakOnError && IsDebuggerPresent()) { @@ -124,6 +77,13 @@ VkBool32 VKAPI_CALL VulkanDebugUtilsCallback( DebugBreak(); } } +#else + // TODO: Improve. + if (messageSeverity & VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT) { + ELOG("VKDEBUG: %s", msg.c_str()); + } else { + WLOG("VKDEBUG: %s", msg.c_str()); + } #endif // false indicates that layer should not bail-out of an diff --git a/Common/Vulkan/VulkanDebug.h b/Common/Vulkan/VulkanDebug.h index 10cb70d8ed..30159d3a23 100644 --- a/Common/Vulkan/VulkanDebug.h +++ b/Common/Vulkan/VulkanDebug.h @@ -25,5 +25,4 @@ struct VulkanLogOptions { bool msgBoxOnError; }; -VkBool32 VKAPI_CALL VulkanDebugReportCallback(VkDebugReportFlagsEXT msgFlags, VkDebugReportObjectTypeEXT objType, uint64_t srcObject, size_t location, int32_t msgCode, const char* pLayerPrefix, const char* pMsg, void *pUserData); VkBool32 VKAPI_CALL VulkanDebugUtilsCallback(VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity, VkDebugUtilsMessageTypeFlagsEXT messageType, const VkDebugUtilsMessengerCallbackDataEXT *pCallbackData, void *pUserData); diff --git a/Common/Vulkan/VulkanLoader.cpp b/Common/Vulkan/VulkanLoader.cpp index c748696c2c..43298ec9c4 100644 --- a/Common/Vulkan/VulkanLoader.cpp +++ b/Common/Vulkan/VulkanLoader.cpp @@ -199,10 +199,6 @@ PFN_vkGetSwapchainImagesKHR vkGetSwapchainImagesKHR; PFN_vkAcquireNextImageKHR vkAcquireNextImageKHR; PFN_vkQueuePresentKHR vkQueuePresentKHR; -// And the DEBUG_REPORT extension. We dynamically load this. -PFN_vkCreateDebugReportCallbackEXT vkCreateDebugReportCallbackEXT; -PFN_vkDestroyDebugReportCallbackEXT vkDestroyDebugReportCallbackEXT; - PFN_vkCreateDebugUtilsMessengerEXT vkCreateDebugUtilsMessengerEXT; PFN_vkDestroyDebugUtilsMessengerEXT vkDestroyDebugUtilsMessengerEXT; PFN_vkCmdBeginDebugUtilsLabelEXT vkCmdBeginDebugUtilsLabelEXT; @@ -542,11 +538,6 @@ void VulkanLoadInstanceFunctions(VkInstance instance, const VulkanDeviceExtensio LOAD_INSTANCE_FUNC(instance, vkGetPhysicalDeviceFeatures2KHR); } - if (enabledExtensions.EXT_debug_report) { - LOAD_INSTANCE_FUNC(instance, vkCreateDebugReportCallbackEXT); - LOAD_INSTANCE_FUNC(instance, vkDestroyDebugReportCallbackEXT); - } - if (enabledExtensions.EXT_debug_utils) { LOAD_INSTANCE_FUNC(instance, vkCreateDebugUtilsMessengerEXT); LOAD_INSTANCE_FUNC(instance, vkDestroyDebugUtilsMessengerEXT); diff --git a/Common/Vulkan/VulkanLoader.h b/Common/Vulkan/VulkanLoader.h index 99b560cfa6..e9c7f96156 100644 --- a/Common/Vulkan/VulkanLoader.h +++ b/Common/Vulkan/VulkanLoader.h @@ -199,11 +199,6 @@ extern PFN_vkGetSwapchainImagesKHR vkGetSwapchainImagesKHR; extern PFN_vkAcquireNextImageKHR vkAcquireNextImageKHR; extern PFN_vkQueuePresentKHR vkQueuePresentKHR; -// And the DEBUG_REPORT extension. Since we load this dynamically even in static -// linked mode, we have to rename it :( -extern PFN_vkCreateDebugReportCallbackEXT vkCreateDebugReportCallbackEXT; -extern PFN_vkDestroyDebugReportCallbackEXT vkDestroyDebugReportCallbackEXT; - extern PFN_vkCreateDebugUtilsMessengerEXT vkCreateDebugUtilsMessengerEXT; extern PFN_vkDestroyDebugUtilsMessengerEXT vkDestroyDebugUtilsMessengerEXT; extern PFN_vkCmdBeginDebugUtilsLabelEXT vkCmdBeginDebugUtilsLabelEXT; @@ -219,10 +214,8 @@ extern PFN_vkGetMemoryHostPointerPropertiesEXT vkGetMemoryHostPointerPropertiesE extern PFN_vkGetPhysicalDeviceProperties2KHR vkGetPhysicalDeviceProperties2KHR; extern PFN_vkGetPhysicalDeviceFeatures2KHR vkGetPhysicalDeviceFeatures2KHR; - // For fast extension-enabled checks. struct VulkanDeviceExtensions { - bool EXT_debug_report; bool EXT_debug_utils; bool KHR_maintenance1; // required for KHR_create_renderpass2 bool KHR_maintenance2; diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp index c747026e38..d9737f4858 100644 --- a/Windows/GPU/WindowsVulkanContext.cpp +++ b/Windows/GPU/WindowsVulkanContext.cpp @@ -55,7 +55,6 @@ #include "Core/System.h" #include "Common/Vulkan/VulkanLoader.h" #include "Common/Vulkan/VulkanContext.h" -#include "Common/Vulkan/VulkanDebug.h" #include "base/stringutil.h" #include "thin3d/thin3d.h" @@ -131,17 +130,12 @@ bool WindowsVulkanContext::Init(HINSTANCE hInst, HWND hWnd, std::string *error_m return false; } if (g_validate_) { - if (g_Vulkan->DeviceExtensions().EXT_debug_utils) { - int bits = VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT - | VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT - | VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT; - // We're intentionally skipping VK_DEBUG_UTILS_MESSAGE_SEVERITY_VERBOSE_BIT_EXT and - // VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT, just too spammy. - g_Vulkan->InitDebugUtilsCallback(&VulkanDebugUtilsCallback, bits, &g_LogOptions); - } else { - int bits = VK_DEBUG_REPORT_ERROR_BIT_EXT | VK_DEBUG_REPORT_WARNING_BIT_EXT | VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT; - g_Vulkan->InitDebugMsgCallback(&VulkanDebugReportCallback, bits, &g_LogOptions); - } + int bits = VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT + | VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT + | VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT; + // We're intentionally skipping VK_DEBUG_UTILS_MESSAGE_SEVERITY_VERBOSE_BIT_EXT and + // VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT, just too spammy. + g_Vulkan->InitDebugUtilsCallback(bits, &g_LogOptions); } g_Vulkan->InitSurface(WINDOWSYSTEM_WIN32, (void *)hInst, (void *)hWnd); if (!g_Vulkan->InitObjects()) { @@ -178,7 +172,6 @@ void WindowsVulkanContext::Shutdown() { g_Vulkan->DestroyObjects(); g_Vulkan->DestroyDevice(); g_Vulkan->DestroyDebugUtilsCallback(); - g_Vulkan->DestroyDebugMsgCallback(); g_Vulkan->DestroyInstance(); delete g_Vulkan; diff --git a/android/jni/AndroidVulkanContext.cpp b/android/jni/AndroidVulkanContext.cpp index 046fc29e36..3dbe65adb3 100644 --- a/android/jni/AndroidVulkanContext.cpp +++ b/android/jni/AndroidVulkanContext.cpp @@ -15,34 +15,7 @@ static VulkanLogOptions g_LogOptions; -static VKAPI_ATTR VkBool32 VKAPI_CALL Vulkan_Dbg(VkDebugReportFlagsEXT msgFlags, VkDebugReportObjectTypeEXT objType, uint64_t srcObject, size_t location, int32_t msgCode, const char* pLayerPrefix, const char* pMsg, void *pUserData) { - const VulkanLogOptions *options = (const VulkanLogOptions *)pUserData; - int loglevel = ANDROID_LOG_INFO; - if (msgFlags & VK_DEBUG_REPORT_ERROR_BIT_EXT) { - loglevel = ANDROID_LOG_ERROR; - } else if (msgFlags & VK_DEBUG_REPORT_WARNING_BIT_EXT) { - loglevel = ANDROID_LOG_WARN; - } else if (msgFlags & VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT) { - loglevel = ANDROID_LOG_WARN; - } else if (msgFlags & VK_DEBUG_REPORT_INFORMATION_BIT_EXT) { - loglevel = ANDROID_LOG_WARN; - } else if (msgFlags & VK_DEBUG_REPORT_DEBUG_BIT_EXT) { - loglevel = ANDROID_LOG_WARN; - } - - __android_log_print(loglevel, APP_NAME, "[%s] %s Code %d : %s", - pLayerPrefix, VulkanObjTypeToString(objType), msgCode, pMsg); - - // false indicates that layer should not bail-out of an - // API call that had validation failures. This may mean that the - // app dies inside the driver due to invalid parameter(s). - // That's what would happen without validation layers, so we'll - // keep that behavior here. - return false; -} - -AndroidVulkanContext::AndroidVulkanContext() { -} +AndroidVulkanContext::AndroidVulkanContext() {} AndroidVulkanContext::~AndroidVulkanContext() { delete g_Vulkan; @@ -128,11 +101,6 @@ bool AndroidVulkanContext::InitFromRenderThread(ANativeWindow *wnd, int desiredB return false; } - if (g_validate_) { - int bits = VK_DEBUG_REPORT_ERROR_BIT_EXT | VK_DEBUG_REPORT_WARNING_BIT_EXT | VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT; - g_Vulkan->InitDebugMsgCallback(&Vulkan_Dbg, bits, &g_LogOptions); - } - bool success = true; if (g_Vulkan->InitObjects()) { draw_ = Draw::T3DCreateVulkanContext(g_Vulkan, g_Config.bGfxDebugSplitSubmit); @@ -153,7 +121,6 @@ bool AndroidVulkanContext::InitFromRenderThread(ANativeWindow *wnd, int desiredB g_Vulkan->DestroyObjects(); g_Vulkan->DestroyDevice(); g_Vulkan->DestroyDebugUtilsCallback(); - g_Vulkan->DestroyDebugMsgCallback(); g_Vulkan->DestroyInstance(); } diff --git a/android/jni/AndroidVulkanContext.h b/android/jni/AndroidVulkanContext.h index 82a041d1b5..2e5cfb7328 100644 --- a/android/jni/AndroidVulkanContext.h +++ b/android/jni/AndroidVulkanContext.h @@ -2,8 +2,6 @@ #include "AndroidGraphicsContext.h" -static const bool g_validate_ = true; - class VulkanContext; class AndroidVulkanContext : public AndroidGraphicsContext { From 6532c345c8638c55cbf510a5d35f1d6b35436ec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 21 Jun 2020 22:56:01 +0200 Subject: [PATCH 2/4] Simplify the validation interface code, bugfixes --- Common/Vulkan/VulkanContext.cpp | 23 +++++++++++++++-------- Common/Vulkan/VulkanContext.h | 4 +++- SDL/SDLVulkanGraphicsContext.cpp | 1 - Windows/GPU/WindowsVulkanContext.cpp | 9 +-------- android/jni/AndroidVulkanContext.cpp | 2 -- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/Common/Vulkan/VulkanContext.cpp b/Common/Vulkan/VulkanContext.cpp index 0017df5ba6..abc16b9e09 100644 --- a/Common/Vulkan/VulkanContext.cpp +++ b/Common/Vulkan/VulkanContext.cpp @@ -36,6 +36,8 @@ #define new DBG_NEW #endif +VulkanLogOptions g_LogOptions; + static const char *validationLayers[] = { "VK_LAYER_KHRONOS_validation", /* @@ -661,12 +663,18 @@ VkResult VulkanContext::CreateDevice() { return res; } -VkResult VulkanContext::InitDebugUtilsCallback(int bits, VulkanLogOptions *logOptions) { +VkResult VulkanContext::InitDebugUtilsCallback() { + // We're intentionally skipping VK_DEBUG_UTILS_MESSAGE_SEVERITY_VERBOSE_BIT_EXT and + // VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT, just too spammy. + int bits = VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT + | VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT + | VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT; + VkDebugUtilsMessengerCreateInfoEXT callback1{VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT}; callback1.messageSeverity = bits; callback1.messageType = VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_TYPE_VALIDATION_BIT_EXT | VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT; callback1.pfnUserCallback = &VulkanDebugUtilsCallback; - callback1.pUserData = (void *)logOptions; + callback1.pUserData = (void *)&g_LogOptions; VkDebugUtilsMessengerEXT messenger; VkResult res = vkCreateDebugUtilsMessengerEXT(instance_, &callback1, nullptr, &messenger); if (res != VK_SUCCESS) { @@ -680,15 +688,14 @@ VkResult VulkanContext::InitDebugUtilsCallback(int bits, VulkanLogOptions *logOp } void VulkanContext::DestroyDebugUtilsCallback() { - if (!extensionsLookup_.EXT_debug_utils) - return; - while (utils_callbacks.size() > 0) { - vkDestroyDebugUtilsMessengerEXT(instance_, utils_callbacks.back(), nullptr); - utils_callbacks.pop_back(); + if (extensionsLookup_.EXT_debug_utils) { + while (utils_callbacks.size() > 0) { + vkDestroyDebugUtilsMessengerEXT(instance_, utils_callbacks.back(), nullptr); + utils_callbacks.pop_back(); + } } } - VkResult VulkanContext::InitSurface(WindowSystem winsys, void *data1, void *data2) { winsys_ = winsys; winsysData1_ = data1; diff --git a/Common/Vulkan/VulkanContext.h b/Common/Vulkan/VulkanContext.h index 3bcae0e56c..7043438ecf 100644 --- a/Common/Vulkan/VulkanContext.h +++ b/Common/Vulkan/VulkanContext.h @@ -170,7 +170,7 @@ public: bool MemoryTypeFromProperties(uint32_t typeBits, VkFlags requirements_mask, uint32_t *typeIndex); - VkResult InitDebugUtilsCallback(int bits, VulkanLogOptions *logOptions); + VkResult InitDebugUtilsCallback(); void DestroyDebugUtilsCallback(); VkPhysicalDevice GetPhysicalDevice(int n) const { @@ -369,3 +369,5 @@ std::string FormatDriverVersion(const VkPhysicalDeviceProperties &props); // Simple heuristic. bool IsHashMaliDriverVersion(const VkPhysicalDeviceProperties &props); + +extern VulkanLogOptions g_LogOptions; diff --git a/SDL/SDLVulkanGraphicsContext.cpp b/SDL/SDLVulkanGraphicsContext.cpp index 02123b105d..60c05583e4 100644 --- a/SDL/SDLVulkanGraphicsContext.cpp +++ b/SDL/SDLVulkanGraphicsContext.cpp @@ -114,7 +114,6 @@ void SDLVulkanGraphicsContext::Shutdown() { vulkan_->WaitUntilQueueIdle(); vulkan_->DestroyObjects(); vulkan_->DestroyDevice(); - vulkan_->DestroyDebugMsgCallback(); vulkan_->DestroyInstance(); delete vulkan_; vulkan_ = nullptr; diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp index d9737f4858..db03b6e802 100644 --- a/Windows/GPU/WindowsVulkanContext.cpp +++ b/Windows/GPU/WindowsVulkanContext.cpp @@ -71,8 +71,6 @@ static const bool g_validate_ = false; static VulkanContext *g_Vulkan; -static VulkanLogOptions g_LogOptions; - static uint32_t FlagsFromConfig() { uint32_t flags = 0; flags = g_Config.bVSync ? VULKAN_FLAG_PRESENT_FIFO : VULKAN_FLAG_PRESENT_MAILBOX; @@ -130,12 +128,7 @@ bool WindowsVulkanContext::Init(HINSTANCE hInst, HWND hWnd, std::string *error_m return false; } if (g_validate_) { - int bits = VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT - | VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT - | VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT; - // We're intentionally skipping VK_DEBUG_UTILS_MESSAGE_SEVERITY_VERBOSE_BIT_EXT and - // VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT, just too spammy. - g_Vulkan->InitDebugUtilsCallback(bits, &g_LogOptions); + g_Vulkan->InitDebugUtilsCallback(); } g_Vulkan->InitSurface(WINDOWSYSTEM_WIN32, (void *)hInst, (void *)hWnd); if (!g_Vulkan->InitObjects()) { diff --git a/android/jni/AndroidVulkanContext.cpp b/android/jni/AndroidVulkanContext.cpp index 3dbe65adb3..9ad5aa2031 100644 --- a/android/jni/AndroidVulkanContext.cpp +++ b/android/jni/AndroidVulkanContext.cpp @@ -142,8 +142,6 @@ void AndroidVulkanContext::Shutdown() { ILOG("Calling NativeShutdownGraphics"); g_Vulkan->DestroyDevice(); g_Vulkan->DestroyDebugUtilsCallback(); - g_Vulkan->DestroyDebugMsgCallback(); - g_Vulkan->DestroyInstance(); // We keep the g_Vulkan context around to avoid invalidating a ton of pointers around the app. finalize_glslang(); From fbc4fa5bca8ee4715962bc2bc95546f494b9cd84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 21 Jun 2020 22:59:55 +0200 Subject: [PATCH 3/4] Move the responsibility to register the debugutils callback to the VulkanContext. We only use one anyway. --- Common/Vulkan/VulkanContext.cpp | 21 ++++++++++++--------- Common/Vulkan/VulkanContext.h | 5 ++--- Windows/GPU/WindowsVulkanContext.cpp | 5 +---- android/jni/AndroidVulkanContext.cpp | 3 --- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/Common/Vulkan/VulkanContext.cpp b/Common/Vulkan/VulkanContext.cpp index abc16b9e09..96c170423d 100644 --- a/Common/Vulkan/VulkanContext.cpp +++ b/Common/Vulkan/VulkanContext.cpp @@ -247,6 +247,11 @@ VkResult VulkanContext::CreateInstance(const CreateInfo &info) { vkGetPhysicalDeviceProperties(physical_devices_[i], &physicalDeviceProperties_[i].properties); } } + + if (extensionsLookup_.EXT_debug_utils) { + InitDebugUtilsCallback(); + } + return VK_SUCCESS; } @@ -255,6 +260,13 @@ VulkanContext::~VulkanContext() { } void VulkanContext::DestroyInstance() { + if (extensionsLookup_.EXT_debug_utils) { + while (utils_callbacks.size() > 0) { + vkDestroyDebugUtilsMessengerEXT(instance_, utils_callbacks.back(), nullptr); + utils_callbacks.pop_back(); + } + } + vkDestroyInstance(instance_, nullptr); VulkanFree(); instance_ = VK_NULL_HANDLE; @@ -687,15 +699,6 @@ VkResult VulkanContext::InitDebugUtilsCallback() { return res; } -void VulkanContext::DestroyDebugUtilsCallback() { - if (extensionsLookup_.EXT_debug_utils) { - while (utils_callbacks.size() > 0) { - vkDestroyDebugUtilsMessengerEXT(instance_, utils_callbacks.back(), nullptr); - utils_callbacks.pop_back(); - } - } -} - VkResult VulkanContext::InitSurface(WindowSystem winsys, void *data1, void *data2) { winsys_ = winsys; winsysData1_ = data1; diff --git a/Common/Vulkan/VulkanContext.h b/Common/Vulkan/VulkanContext.h index 7043438ecf..3a8e519381 100644 --- a/Common/Vulkan/VulkanContext.h +++ b/Common/Vulkan/VulkanContext.h @@ -170,9 +170,6 @@ public: bool MemoryTypeFromProperties(uint32_t typeBits, VkFlags requirements_mask, uint32_t *typeIndex); - VkResult InitDebugUtilsCallback(); - void DestroyDebugUtilsCallback(); - VkPhysicalDevice GetPhysicalDevice(int n) const { return physical_devices_[n]; } @@ -276,6 +273,8 @@ public: void GetImageMemoryRequirements(VkImage image, VkMemoryRequirements *mem_reqs, bool *dedicatedAllocation); private: + VkResult InitDebugUtilsCallback(); + // A layer can expose extensions, keep track of those extensions here. struct LayerProperties { VkLayerProperties properties; diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp index db03b6e802..abdc03e603 100644 --- a/Windows/GPU/WindowsVulkanContext.cpp +++ b/Windows/GPU/WindowsVulkanContext.cpp @@ -127,9 +127,7 @@ bool WindowsVulkanContext::Init(HINSTANCE hInst, HWND hWnd, std::string *error_m g_Vulkan = nullptr; return false; } - if (g_validate_) { - g_Vulkan->InitDebugUtilsCallback(); - } + g_Vulkan->InitSurface(WINDOWSYSTEM_WIN32, (void *)hInst, (void *)hWnd); if (!g_Vulkan->InitObjects()) { *error_message = g_Vulkan->InitError(); @@ -164,7 +162,6 @@ void WindowsVulkanContext::Shutdown() { g_Vulkan->WaitUntilQueueIdle(); g_Vulkan->DestroyObjects(); g_Vulkan->DestroyDevice(); - g_Vulkan->DestroyDebugUtilsCallback(); g_Vulkan->DestroyInstance(); delete g_Vulkan; diff --git a/android/jni/AndroidVulkanContext.cpp b/android/jni/AndroidVulkanContext.cpp index 9ad5aa2031..adcf4738ae 100644 --- a/android/jni/AndroidVulkanContext.cpp +++ b/android/jni/AndroidVulkanContext.cpp @@ -120,8 +120,6 @@ bool AndroidVulkanContext::InitFromRenderThread(ANativeWindow *wnd, int desiredB if (!success) { g_Vulkan->DestroyObjects(); g_Vulkan->DestroyDevice(); - g_Vulkan->DestroyDebugUtilsCallback(); - g_Vulkan->DestroyInstance(); } return success; @@ -141,7 +139,6 @@ void AndroidVulkanContext::ShutdownFromRenderThread() { void AndroidVulkanContext::Shutdown() { ILOG("Calling NativeShutdownGraphics"); g_Vulkan->DestroyDevice(); - g_Vulkan->DestroyDebugUtilsCallback(); g_Vulkan->DestroyInstance(); // We keep the g_Vulkan context around to avoid invalidating a ton of pointers around the app. finalize_glslang(); From 3d743a5ae7ab9539840046e6e36e1b8bdb66e720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 21 Jun 2020 23:13:05 +0200 Subject: [PATCH 4/4] Buildfixes --- CMakeLists.txt | 2 ++ Common/Vulkan/VulkanDebug.cpp | 2 +- Common/Vulkan/VulkanDebug.h | 2 +- SDL/SDLVulkanGraphicsContext.h | 1 - android/jni/AndroidVulkanContext.cpp | 2 -- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9c352f1f0c..ee98014740 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -396,6 +396,8 @@ set(CommonWindows source_group(Windows FILES ${CommonWindows}) set(CommonVulkan ${CommonExtra} + Common/Vulkan/VulkanDebug.cpp + Common/Vulkan/VulkanDebug.h Common/Vulkan/VulkanContext.cpp Common/Vulkan/VulkanContext.h Common/Vulkan/VulkanImage.cpp diff --git a/Common/Vulkan/VulkanDebug.cpp b/Common/Vulkan/VulkanDebug.cpp index 9397338ced..3129e98938 100644 --- a/Common/Vulkan/VulkanDebug.cpp +++ b/Common/Vulkan/VulkanDebug.cpp @@ -23,7 +23,7 @@ #include "Common/Vulkan/VulkanContext.h" #include "Common/Vulkan/VulkanDebug.h" -VkBool32 VKAPI_CALL VulkanDebugUtilsCallback( +VKAPI_ATTR VkBool32 VKAPI_CALL VulkanDebugUtilsCallback( VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity, VkDebugUtilsMessageTypeFlagsEXT messageType, const VkDebugUtilsMessengerCallbackDataEXT* pCallbackData, diff --git a/Common/Vulkan/VulkanDebug.h b/Common/Vulkan/VulkanDebug.h index 30159d3a23..e622f542c7 100644 --- a/Common/Vulkan/VulkanDebug.h +++ b/Common/Vulkan/VulkanDebug.h @@ -25,4 +25,4 @@ struct VulkanLogOptions { bool msgBoxOnError; }; -VkBool32 VKAPI_CALL VulkanDebugUtilsCallback(VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity, VkDebugUtilsMessageTypeFlagsEXT messageType, const VkDebugUtilsMessengerCallbackDataEXT *pCallbackData, void *pUserData); +VKAPI_ATTR VkBool32 VKAPI_CALL VulkanDebugUtilsCallback(VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity, VkDebugUtilsMessageTypeFlagsEXT messageType, const VkDebugUtilsMessengerCallbackDataEXT *pCallbackData, void *pUserData); diff --git a/SDL/SDLVulkanGraphicsContext.h b/SDL/SDLVulkanGraphicsContext.h index 6ecfecfe1e..533406560e 100644 --- a/SDL/SDLVulkanGraphicsContext.h +++ b/SDL/SDLVulkanGraphicsContext.h @@ -42,5 +42,4 @@ public: private: Draw::DrawContext *draw_ = nullptr; VulkanContext *vulkan_ = nullptr; - VulkanLogOptions g_LogOptions; }; diff --git a/android/jni/AndroidVulkanContext.cpp b/android/jni/AndroidVulkanContext.cpp index adcf4738ae..39fe573057 100644 --- a/android/jni/AndroidVulkanContext.cpp +++ b/android/jni/AndroidVulkanContext.cpp @@ -13,8 +13,6 @@ #include "Core/ConfigValues.h" #include "Core/System.h" -static VulkanLogOptions g_LogOptions; - AndroidVulkanContext::AndroidVulkanContext() {} AndroidVulkanContext::~AndroidVulkanContext() {