From 486b9eef1e48545cfe2f1d344bbf8e6906ab448e Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 21 Feb 2024 15:29:31 -0800 Subject: [PATCH] vk: add debug names to shader modules (#7577) Also removed debugUtils workaround for Mesa since it's been addressed properly from the vk backend. --- filament/backend/src/vulkan/VulkanConstants.h | 22 +++- filament/backend/src/vulkan/VulkanDriver.cpp | 117 ++++++++++++------ filament/backend/src/vulkan/VulkanDriver.h | 25 +++- filament/backend/src/vulkan/VulkanHandles.cpp | 26 +++- .../src/vulkan/platform/VulkanPlatform.cpp | 33 +++-- 5 files changed, 162 insertions(+), 61 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanConstants.h b/filament/backend/src/vulkan/VulkanConstants.h index 429060535d..a2b62bd34c 100644 --- a/filament/backend/src/vulkan/VulkanConstants.h +++ b/filament/backend/src/vulkan/VulkanConstants.h @@ -39,7 +39,12 @@ // not required for validation. // FVK is short for Filament Vulkan + +// Enables Android systrace #define FVK_DEBUG_SYSTRACE 0x00000001 + +// Group markers are used to denote collection of GPU commands. It is typically at the granualarity +// of a renderpass. #define FVK_DEBUG_GROUP_MARKERS 0x00000002 #define FVK_DEBUG_TEXTURE 0x00000004 #define FVK_DEBUG_LAYOUT_TRANSITION 0x00000008 @@ -55,6 +60,9 @@ #define FVK_DEBUG_PIPELINE_CACHE 0x00002000 #define FVK_DEBUG_ALLOCATION 0x00004000 +// Enable the debug utils extension if it is available. +#define FVK_DEBUG_DEBUG_UTILS 0x00008000 + // Usefaul default combinations #define FVK_DEBUG_EVERYTHING 0xFFFFFFFF #define FVK_DEBUG_PERFORMANCE \ @@ -72,7 +80,7 @@ FVK_DEBUG_PRINT_GROUP_MARKERS #ifndef NDEBUG -#define FVK_DEBUG_FLAGS FVK_DEBUG_PERFORMANCE +#define FVK_DEBUG_FLAGS (FVK_DEBUG_PERFORMANCE | FVK_DEBUG_DEBUG_UTILS | FVK_DEBUG_VALIDATION) #else #define FVK_DEBUG_FLAGS 0 #endif @@ -80,11 +88,23 @@ #define FVK_ENABLED(flags) ((FVK_DEBUG_FLAGS) & (flags)) #define FVK_ENABLED_BOOL(flags) ((bool) FVK_ENABLED(flags)) + +// Group marker only works only if validation or debug utils is enabled since it uses +// vkCmd(Begin/End)DebugUtilsLabelEXT or vkCmdDebugMarker(Begin/End)EXT +#if FVK_ENABLED(FVK_DEBUG_PRINT_GROUP_MARKERS) +static_assert(FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) || FVK_ENABLED(FVK_DEBUG_VALIDATION)); +#endif + // Ensure dependencies are met between debug options #if FVK_ENABLED(FVK_DEBUG_PRINT_GROUP_MARKERS) static_assert(FVK_ENABLED(FVK_DEBUG_GROUP_MARKERS)); #endif +// Only enable debug utils if validation is enabled. +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) +static_assert(FVK_ENABLED(FVK_DEBUG_VALIDATION)); +#endif + // end dependcy checks #if FVK_ENABLED(FVK_DEBUG_SYSTRACE) diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 4dcd9bfa79..cfb57501aa 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -24,6 +24,7 @@ #include "VulkanHandles.h" #include "VulkanImageUtility.h" #include "VulkanMemory.h" +#include "VulkanTexture.h" #include @@ -117,7 +118,9 @@ VKAPI_ATTR VkBool32 VKAPI_CALL debugReportCallback(VkDebugReportFlagsEXT flags, utils::slog.e << utils::io::endl; return VK_FALSE; } +#endif // FVK_EANBLED(FVK_DEBUG_VALIDATION) +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) VKAPI_ATTR VkBool32 VKAPI_CALL debugUtilsCallback(VkDebugUtilsMessageSeverityFlagBitsEXT severity, VkDebugUtilsMessageTypeFlagsEXT types, const VkDebugUtilsMessengerCallbackDataEXT* cbdata, void* pUserData) { @@ -136,9 +139,63 @@ VKAPI_ATTR VkBool32 VKAPI_CALL debugUtilsCallback(VkDebugUtilsMessageSeverityFla utils::slog.e << utils::io::endl; return VK_FALSE; } -#endif // FVK_EANBLED(FVK_DEBUG_VALIDATION) +#endif // FVK_EANBLED(FVK_DEBUG_DEBUG_UTILS) -} // anonymous namespace +}// anonymous namespace + +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) +using DebugUtils = VulkanDriver::DebugUtils; +DebugUtils* DebugUtils::mSingleton = nullptr; + +DebugUtils::DebugUtils(VkInstance instance, VkDevice device, VulkanContext const* context) + : mInstance(instance), mDevice(device), mEnabled(context->isDebugUtilsSupported()) { + +#if FVK_ENABLED(FVK_DEBUG_VALIDATION) + // Also initialize debug utils messenger here + if (mEnabled) { + VkDebugUtilsMessengerCreateInfoEXT const createInfo = { + .sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT, + .pNext = nullptr, + .flags = 0, + .messageSeverity = VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT + | VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT, + .messageType = VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT + | VK_DEBUG_UTILS_MESSAGE_TYPE_VALIDATION_BIT_EXT, + .pfnUserCallback = debugUtilsCallback, + }; + VkResult result = vkCreateDebugUtilsMessengerEXT(instance, &createInfo, + VKALLOC, &mDebugMessenger); + ASSERT_POSTCONDITION(result == VK_SUCCESS, "Unable to create Vulkan debug messenger."); + } +#endif // FVK_EANBLED(FVK_DEBUG_VALIDATION) +} + +DebugUtils* DebugUtils::get() { + assert_invariant(DebugUtils::mSingleton); + return DebugUtils::mSingleton; +} + +DebugUtils::~DebugUtils() { + if (mDebugMessenger) { + vkDestroyDebugUtilsMessengerEXT(mInstance, mDebugMessenger, VKALLOC); + } +} + +void DebugUtils::setName(VkObjectType type, uint64_t handle, char const* name) { + auto impl = DebugUtils::get(); + if (!impl->mEnabled) { + return; + } + VkDebugUtilsObjectNameInfoEXT const info = { + .sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_OBJECT_NAME_INFO_EXT, + .pNext = nullptr, + .objectType = type, + .objectHandle = handle, + .pObjectName = name, + }; + vkSetDebugUtilsObjectNameEXT(impl->mDevice, &info); +} +#endif // FVK_EANBLED(FVK_DEBUG_DEBUG_UTILS) using ImgUtil = VulkanImageUtility; @@ -159,35 +216,26 @@ VulkanDriver::VulkanDriver(VulkanPlatform* platform, VulkanContext const& contex mReadPixels(mPlatform->getDevice()), mIsSRGBSwapChainSupported(mPlatform->getCustomization().isSRGBSwapChainSupported) { +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) + DebugUtils::mSingleton = + new DebugUtils(mPlatform->getInstance(), mPlatform->getDevice(), &context); +#endif + #if FVK_ENABLED(FVK_DEBUG_VALIDATION) UTILS_UNUSED const PFN_vkCreateDebugReportCallbackEXT createDebugReportCallback = vkCreateDebugReportCallbackEXT; - VkResult result; - if (mContext.isDebugUtilsSupported()) { - VkDebugUtilsMessengerCreateInfoEXT const createInfo = { - .sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT, - .pNext = nullptr, - .flags = 0, - .messageSeverity = VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT - | VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT, - .messageType = VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT - | VK_DEBUG_UTILS_MESSAGE_TYPE_VALIDATION_BIT_EXT, - .pfnUserCallback = debugUtilsCallback, - }; - result = vkCreateDebugUtilsMessengerEXT(mPlatform->getInstance(), &createInfo, VKALLOC, - &mDebugMessenger); - ASSERT_POSTCONDITION(result == VK_SUCCESS, "Unable to create Vulkan debug messenger."); - } else if (createDebugReportCallback) { + if (!context.isDebugUtilsSupported() && createDebugReportCallback) { VkDebugReportCallbackCreateInfoEXT const cbinfo = { .sType = VK_STRUCTURE_TYPE_DEBUG_REPORT_CALLBACK_CREATE_INFO_EXT, .flags = VK_DEBUG_REPORT_WARNING_BIT_EXT | VK_DEBUG_REPORT_ERROR_BIT_EXT, .pfnCallback = debugReportCallback, }; - result = createDebugReportCallback(mPlatform->getInstance(), &cbinfo, VKALLOC, + VkResult result = createDebugReportCallback(mPlatform->getInstance(), &cbinfo, VKALLOC, &mDebugCallback); ASSERT_POSTCONDITION(result == VK_SUCCESS, "Unable to create Vulkan debug callback."); } #endif + mTimestamps = std::make_unique(mPlatform->getDevice()); mCommands = std::make_unique(mPlatform->getDevice(), mPlatform->getGraphicsQueue(), mPlatform->getGraphicsQueueFamilyIndex(), &mContext, @@ -286,9 +334,12 @@ void VulkanDriver::terminate() { if (mDebugCallback) { vkDestroyDebugReportCallbackEXT(mPlatform->getInstance(), mDebugCallback, VKALLOC); } - if (mDebugMessenger) { - vkDestroyDebugUtilsMessengerEXT(mPlatform->getInstance(), mDebugMessenger, VKALLOC); - } + +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) + assert_invariant(DebugUtils::mSingleton); + delete DebugUtils::mSingleton; +#endif + mPlatform->terminate(); } @@ -1219,22 +1270,14 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP } VkFramebuffer vkfb = mFramebufferCache.getFramebuffer(fbkey); - // Assign a label to the framebuffer for debugging purposes. - #if FVK_ENABLED(FVK_DEBUG_GROUP_MARKERS) - if (UTILS_UNLIKELY(mContext.isDebugUtilsSupported())) { - auto const topMarker = mCommands->getTopGroupMarker(); - if (!topMarker.empty()) { - const VkDebugUtilsObjectNameInfoEXT info = { - VK_STRUCTURE_TYPE_DEBUG_UTILS_OBJECT_NAME_INFO_EXT, - nullptr, - VK_OBJECT_TYPE_FRAMEBUFFER, - reinterpret_cast(vkfb), - topMarker.c_str(), - }; - vkSetDebugUtilsObjectNameEXT(mPlatform->getDevice(), &info); - } +// Assign a label to the framebuffer for debugging purposes. +#if FVK_ENABLED(FVK_DEBUG_GROUP_MARKERS | FVK_DEBUG_DEBUG_UTILS) + auto const topMarker = mCommands->getTopGroupMarker(); + if (!topMarker.empty()) { + DebugUtils::setName(VK_OBJECT_TYPE_FRAMEBUFFER, reinterpret_cast(vkfb), + topMarker.c_str()); } - #endif +#endif // The current command buffer now owns a reference to the render target and its attachments. // Note that we must acquire parent textures, not sidecars. diff --git a/filament/backend/src/vulkan/VulkanDriver.h b/filament/backend/src/vulkan/VulkanDriver.h index d76b474dcf..8e311b9bcc 100644 --- a/filament/backend/src/vulkan/VulkanDriver.h +++ b/filament/backend/src/vulkan/VulkanDriver.h @@ -45,6 +45,30 @@ public: static Driver* create(VulkanPlatform* platform, VulkanContext const& context, Platform::DriverConfig const& driverConfig) noexcept; +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) + // Encapsulates the VK_EXT_debug_utils extension. In particular, we use + // vkSetDebugUtilsObjectNameEXT and vkCreateDebugUtilsMessengerEXT + class DebugUtils { + public: + static void setName(VkObjectType type, uint64_t handle, char const* name); + + private: + static DebugUtils* get(); + + DebugUtils(VkInstance instance, VkDevice device, VulkanContext const* context); + ~DebugUtils(); + + VkInstance const mInstance; + VkDevice const mDevice; + bool const mEnabled; + VkDebugUtilsMessengerEXT mDebugMessenger = VK_NULL_HANDLE; + + static DebugUtils* mSingleton; + + friend class VulkanDriver; + }; +#endif // FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) + private: void debugCommandBegin(CommandStream* cmds, bool synchronous, const char* methodName) noexcept override; @@ -93,7 +117,6 @@ private: VulkanRenderPass mCurrentRenderPass = {}; VmaAllocator mAllocator = VK_NULL_HANDLE; VkDebugReportCallbackEXT mDebugCallback = VK_NULL_HANDLE; - VkDebugUtilsMessengerEXT mDebugMessenger = VK_NULL_HANDLE; VulkanContext mContext = {}; VulkanResourceAllocator mResourceAllocator; diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index bb7f986993..53014949d8 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -17,6 +17,7 @@ #include "VulkanHandles.h" #include "VulkanConstants.h" +#include "VulkanDriver.h" #include "VulkanMemory.h" #include "spirv/VulkanSpirvUtils.h" @@ -78,6 +79,23 @@ VulkanProgram::VulkanProgram(VkDevice device, Program const& builder) noexcept }; VkResult result = vkCreateShaderModule(mDevice, &moduleInfo, VKALLOC, &module); ASSERT_POSTCONDITION(result == VK_SUCCESS, "Unable to create shader module."); + +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) + std::string name{ builder.getName().c_str(), builder.getName().size() }; + switch (static_cast(i)) { + case ShaderStage::VERTEX: + name += "_vs"; + break; + case ShaderStage::FRAGMENT: + name += "_fs"; + break; + default: + PANIC_POSTCONDITION("Unexpected stage"); + break; + } + VulkanDriver::DebugUtils::setName(VK_OBJECT_TYPE_SHADER_MODULE, + reinterpret_cast(module), name.c_str()); +#endif } auto& groupInfo = builder.getSamplerGroupInfo(); @@ -93,10 +111,10 @@ VulkanProgram::VulkanProgram(VkDevice device, Program const& builder) noexcept } } - #if FVK_ENABLED(FVK_DEBUG_SHADER_MODULE) - utils::slog.d << "Created VulkanProgram " << builder << ", shaders = (" << bundle.vertex - << ", " << bundle.fragment << ")" << utils::io::endl; - #endif +#if FVK_ENABLED(FVK_DEBUG_SHADER_MODULE) + utils::slog.d << "Created VulkanProgram " << builder << ", shaders = (" << bundle.vertex + << ", " << bundle.fragment << ")" << utils::io::endl; +#endif } VulkanProgram::VulkanProgram(VkDevice device, VkShaderModule vs, VkShaderModule fs, diff --git a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp index 3f3502ce68..ab13e8859a 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp @@ -23,6 +23,7 @@ #include #include +#include #define SWAPCHAIN_RET_FUNC(func, handle, ...) \ if (mImpl->mSurfaceSwapChains.find(handle) != mImpl->mSurfaceSwapChains.end()) { \ @@ -158,7 +159,9 @@ ExtensionSet getInstanceExtensions() { VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME, // Request these if available. +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) VK_EXT_DEBUG_UTILS_EXTENSION_NAME, +#endif VK_KHR_PORTABILITY_ENUMERATION_EXTENSION_NAME, #if FVK_ENABLED(FVK_DEBUG_VALIDATION) @@ -181,7 +184,9 @@ ExtensionSet getInstanceExtensions() { ExtensionSet getDeviceExtensions(VkPhysicalDevice device) { std::string_view const TARGET_EXTS[] = { +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) VK_EXT_DEBUG_MARKER_EXTENSION_NAME, +#endif VK_KHR_PORTABILITY_SUBSET_EXTENSION_NAME, VK_KHR_MAINTENANCE1_EXTENSION_NAME, VK_KHR_MAINTENANCE2_EXTENSION_NAME, @@ -337,36 +342,22 @@ std::tuple pruneExtensions(VkPhysicalDevice device, ExtensionSet const& instExts, ExtensionSet const& deviceExts) { ExtensionSet newInstExts = instExts; ExtensionSet newDeviceExts = deviceExts; - if (vkGetPhysicalDeviceProperties2KHR) { - VkPhysicalDeviceDriverProperties driverProperties = { - .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DRIVER_PROPERTIES, - }; - VkPhysicalDeviceProperties2 physicalDeviceProperties2 = { - .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2, - .pNext = &driverProperties, - }; - vkGetPhysicalDeviceProperties2KHR(device, &physicalDeviceProperties2); - char* driverInfo = driverProperties.driverInfo; - - if (newInstExts.find(VK_EXT_DEBUG_UTILS_EXTENSION_NAME) != newInstExts.end()) { - // Workaround for Mesa drivers. See issue #6192 - if ((driverInfo && strstr(driverInfo, "Mesa"))) { - newInstExts.erase(VK_EXT_DEBUG_UTILS_EXTENSION_NAME); - } - } - } +#if FVK_ENABLED(FVK_DEBUG_DEBUG_UTILS) // debugUtils and debugMarkers extensions are used mutually exclusively. if (newInstExts.find(VK_EXT_DEBUG_UTILS_EXTENSION_NAME) != newInstExts.end() && newDeviceExts.find(VK_EXT_DEBUG_MARKER_EXTENSION_NAME) != newDeviceExts.end()) { newDeviceExts.erase(VK_EXT_DEBUG_MARKER_EXTENSION_NAME); } +#endif +#if FVK_ENABLED(FVK_DEBUG_VALIDATION) // debugMarker must also request debugReport the instance extension. So check if that's present. if (newDeviceExts.find(VK_EXT_DEBUG_MARKER_EXTENSION_NAME) != newDeviceExts.end() && newInstExts.find(VK_EXT_DEBUG_REPORT_EXTENSION_NAME) == newInstExts.end()) { newDeviceExts.erase(VK_EXT_DEBUG_MARKER_EXTENSION_NAME); } +#endif return std::tuple(newInstExts, newDeviceExts); } @@ -671,6 +662,12 @@ Driver* VulkanPlatform::createDriver(void* sharedContext, context.mDebugMarkersSupported = deviceExts.find(VK_EXT_DEBUG_MARKER_EXTENSION_NAME) != deviceExts.end(); +#ifdef NDEBUG + // If we are in release build, we should not have turned on debug extensions + ASSERT_POSTCONDITION(!context.mDebugUtilsSupported && !context.mDebugMarkersSupported, + "Debug utils should not be enabled in release build."); +#endif + context.mDepthFormats = findAttachmentDepthFormats(mImpl->mPhysicalDevice); assert_invariant(context.mDepthFormats.size() > 0);