blender
/
blender
137
415
Fork 599
Code Issues 6.3k Pull Requests 768 Projects 19 Wiki Activity

Vulkan: Provide utilities to put markers and labels where needed in the GPUmodule #106098

Merged
Jeroen Bakker merged 29 commits from :vk_debug_update into main 2023-04-21 12:32:52 +02:00
Conversation 1 Commits 29 Files Changed 8 +183 -9
3 changed files with 183 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 353ca23ca8 - Show all commits

7
source/blender/gpu/vulkan/vk_common.hh
View File

@ -16,6 +16,7 @@
#include "vk_mem_alloc.h"
#include "gpu_texture_private.hh"
#include <typeinfo>
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-24 14:16:50 +01:00
Outdated
Review
Copy Link

Add one line between imports of different categories. Put system includes on top.

Add one line between imports of different categories. Put system includes on top.
namespace blender::gpu {
@ -25,11 +26,11 @@ VkComponentMapping to_vk_component_mapping(const eGPUTextureFormat format);
VkImageViewType to_vk_image_view_type(const eGPUTextureType type);
VkImageType to_vk_image_type(const eGPUTextureType type);
#ifdef __cplusplus
template<typename T> VkObjectType to_vk_object_type(T vk_obj)
template<typename T> VkObjectType to_vk_object_type(T /*vk_obj*/)
{
const type_info &tid = typeid(T);
const std::type_info &tid = typeid(T);
#define VK_EQ_TYPEID(name, name2) \
if (tid == typeid(name##)) { \
if (tid == typeid(name)) { \
return VK_OBJECT_TYPE_##name2; \
}

174
source/blender/gpu/vulkan/vk_debug.cc
View File

@ -1,5 +1,175 @@
void VKContext::debug_capture_scope_end(void * /*scope*/)
{
/** \file
* \ingroup gpu
*
* Debug features of Vulkan.
*/
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-24 14:25:21 +01:00
Outdated
Review
Copy Link

Add empty lines between each category of includes

Add empty lines between each category of includes
#include "BLI_set.hh"
#include "BLI_system.h"
#include "BKE_global.h"
#include "CLG_log.h"
#include "GPU_debug.h"
#include "GPU_platform.h"
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-24 14:25:58 +01:00
Outdated
Review
Copy Link

Add empty line between includes and code.

Add empty line between includes and code.
#include "vk_common.hh"
#include "vk_debug.hh"
#include "vk_context.hh"
static CLG_LogRef LOG = { "gpu.debug.vulkan" };
namespace blender {
namespace gpu {
namespace debug {
static void vulkan_dynamic_debug_functions(VKContext* ctx,
PFN_vkGetInstanceProcAddr instload)
{
VKDebuggingTools &tools = ctx->debuggingtools_get();
VkInstance instance = ctx->instance_get();
if (instload) {
tools.enabled = false;
tools.vkCmdBeginDebugUtilsLabelEXT_r = (PFN_vkCmdBeginDebugUtilsLabelEXT)instload(
instance, "vkCmdBeginDebugUtilsLabelEXT");
tools.vkCmdEndDebugUtilsLabelEXT_r = (PFN_vkCmdEndDebugUtilsLabelEXT)instload(
instance, "vkCmdEndDebugUtilsLabelEXT");
tools.vkCmdInsertDebugUtilsLabelEXT_r = (PFN_vkCmdInsertDebugUtilsLabelEXT)instload(
instance, "vkCmdInsertDebugUtilsLabelEXT");
tools.vkCreateDebugUtilsMessengerEXT_r = (PFN_vkCreateDebugUtilsMessengerEXT)instload(
instance, "vkCreateDebugUtilsMessengerEXT");
tools.vkDestroyDebugUtilsMessengerEXT_r = (PFN_vkDestroyDebugUtilsMessengerEXT)instload(
instance, "vkDestroyDebugUtilsMessengerEXT");
tools.vkQueueBeginDebugUtilsLabelEXT_r = (PFN_vkQueueBeginDebugUtilsLabelEXT)instload(
instance, "vkQueueBeginDebugUtilsLabelEXT");
tools.vkQueueEndDebugUtilsLabelEXT_r = (PFN_vkQueueEndDebugUtilsLabelEXT)instload(
instance, "vkQueueEndDebugUtilsLabelEXT");
tools.vkQueueInsertDebugUtilsLabelEXT_r = (PFN_vkQueueInsertDebugUtilsLabelEXT)instload(
instance, "vkQueueInsertDebugUtilsLabelEXT");
tools.vkSetDebugUtilsObjectNameEXT_r = (PFN_vkSetDebugUtilsObjectNameEXT)instload(
instance, "vkSetDebugUtilsObjectNameEXT");
tools.vkSetDebugUtilsObjectTagEXT_r = (PFN_vkSetDebugUtilsObjectTagEXT)instload(
instance, "vkSetDebugUtilsObjectTagEXT");
tools.vkSubmitDebugUtilsMessageEXT_r = (PFN_vkSubmitDebugUtilsMessageEXT)instload(
instance, "vkSubmitDebugUtilsMessageEXT");
if (tools.vkCmdBeginDebugUtilsLabelEXT_r) {
tools.enabled = true;
}
}
else {
tools.vkCmdBeginDebugUtilsLabelEXT_r = nullptr;
Jeroen Bakker commented 2023-04-17 09:10:41 +02:00
Outdated
Review
Copy Link

rename function to load_dynamic_functions. the terms vulkan and debug are redundant.

rename function to `load_dynamic_functions`. the terms vulkan and debug are redundant.
tools.vkCmdEndDebugUtilsLabelEXT_r = nullptr;
tools.vkCmdInsertDebugUtilsLabelEXT_r = nullptr;
tools.vkCreateDebugUtilsMessengerEXT_r = nullptr;
tools.vkDestroyDebugUtilsMessengerEXT_r = nullptr;
tools.vkQueueBeginDebugUtilsLabelEXT_r = nullptr;
tools.vkQueueEndDebugUtilsLabelEXT_r = nullptr;
tools.vkQueueInsertDebugUtilsLabelEXT_r = nullptr;
tools.vkSetDebugUtilsObjectNameEXT_r = nullptr;
tools.vkSetDebugUtilsObjectTagEXT_r = nullptr;
tools.vkSubmitDebugUtilsMessageEXT_r = nullptr;
tools.enabled = false;
}
}
bool init_vk_callbacks(VKContext* ctx, PFN_vkGetInstanceProcAddr instload)
{
CLOG_ENSURE(&LOG);
if (instload ) {
vulkan_dynamic_debug_functions(ctx, instload);
return true;
};
return false;
}
void destroy_vk_callbacks(VKContext* ctx)
{
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
vulkan_dynamic_debug_functions(ctx, nullptr);
}
}
void object_vk_label(blender::gpu::VKContext* ctx, VkObjectType objType, uint64_t obj, const char* name)
{
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
VkDebugUtilsObjectNameInfoEXT info = {};
info.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_OBJECT_NAME_INFO_EXT;
info.objectType = objType;
info.objectHandle = obj;
info.pObjectName = name;
tools.vkSetDebugUtilsObjectNameEXT_r(ctx->device_get(), &info);
}
}
void pushMarker(blender::gpu::VKContext *ctx, VkCommandBuffer cmd, const char *name)
{
if (G.debug & G_DEBUG_GPU) {
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
VkDebugUtilsLabelEXT info = {};
info.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT;
info.pLabelName = name;
tools.vkCmdBeginDebugUtilsLabelEXT_r(cmd, &info);
}
}
}
void setMarker(blender::gpu::VKContext *ctx, VkCommandBuffer cmd, const char *name)
{
if (G.debug & G_DEBUG_GPU) {
vnapdv marked this conversation as resolved
Jeroen Bakker commented 2023-04-03 13:40:40 +02:00
Review
Copy Link

use context when referring to VKContext (be consistent with other areas of the code-base)

When referring to instances of the Vulkan API it is used. Eg VkCommandBuffer parameters should be named vk_command_buffer VKCommandBuffer (internal class) should be referred to as command_buffer.

Abbreviations like q should not be used, just call them what they are queue.

etc

use `context` when referring to `VKContext` (be consistent with other areas of the code-base) When referring to instances of the Vulkan API it is used. Eg VkCommandBuffer parameters should be named `vk_command_buffer` VKCommandBuffer (internal class) should be referred to as `command_buffer`. Abbreviations like `q` should not be used, just call them what they are `queue`. etc
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
VkDebugUtilsLabelEXT info = {};
info.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT;
info.pLabelName = name;
tools.vkCmdInsertDebugUtilsLabelEXT_r(cmd, &info);
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-24 14:14:14 +01:00
Outdated
Review
Copy Link

/home/jeroen/blender-git/blender/source/blender/gpu/vulkan/vk_debug.cc:125:26: warning: variable ‘tools’ set but not used [-Wunused-but-set-variable]

`/home/jeroen/blender-git/blender/source/blender/gpu/vulkan/vk_debug.cc:125:26: warning: variable ‘tools’ set but not used [-Wunused-but-set-variable]`
}
}
}
void popMarker(blender::gpu::VKContext *ctx, VkCommandBuffer cmd)
{
Jeroen Bakker commented 2023-04-17 09:12:50 +02:00
Outdated
Review
Copy Link

obj isn't very useful. would use object_handle

`obj` isn't very useful. would use `object_handle`
if (G.debug & G_DEBUG_GPU) {
Jeroen Bakker commented 2023-04-04 12:09:11 +02:00
Outdated
Review
Copy Link

objType -> vk_object_type

`objType` -> `vk_object_type`
VKDebuggingTools tools = ctx->debuggingtools_get();
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-24 14:26:44 +01:00
Outdated
Review
Copy Link

Add empty line between each function.

Add empty line between each function.
if (tools.enabled) {
tools.vkCmdEndDebugUtilsLabelEXT_r(cmd);
}
}
}
void pushMarker(blender::gpu::VKContext *ctx, VkQueue q, const char *name)
{
if (G.debug & G_DEBUG_GPU) {
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
VkDebugUtilsLabelEXT info = {};
info.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT;
info.pLabelName = name;
Jeroen Bakker commented 2023-04-17 09:14:23 +02:00
Review
Copy Link

cmd -> vk_command_buffer. No need to confuse readers with incorrect abbreviations.

Also check the other functions.

`cmd` -> `vk_command_buffer`. No need to confuse readers with incorrect abbreviations. Also check the other functions.
tools.vkQueueBeginDebugUtilsLabelEXT_r(q, &info);
}
}
}
void setMarker(blender::gpu::VKContext *ctx, VkQueue q, const char *name)
{
if (G.debug & G_DEBUG_GPU) {
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
VkDebugUtilsLabelEXT info = {};
info.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT;
info.pLabelName = name;
tools.vkQueueInsertDebugUtilsLabelEXT_r(q, &info);
}
}
}
void popMarker(blender::gpu::VKContext *ctx, VkQueue q)
{
if (G.debug & G_DEBUG_GPU) {
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
tools.vkQueueEndDebugUtilsLabelEXT_r(q);
}
}
}
}
}
}
} // namespace blender::gpu

11
source/blender/gpu/vulkan/vk_debug.hh
View File

@ -34,16 +34,19 @@ namespace blender {
}VKDebuggingTools;
bool init_vk_callbacks(VKContext* ctx, PFN_vkGetInstanceProcAddr instload);
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-04-03 13:49:41 +02:00
Outdated
Review
Copy Link

we should remove vk from the function names it is redundant in these cases

we should remove vk from the function names it is redundant in these cases
void destroy_vk_callbacks(VKContext* ctx);
void object_vk_label(VKContext* ctx, VkObjectType objType, uint64_t obj, const char* name);
template<typename T> void object_vk_label(VKContext* ctx, T obj, const char* name) {
if (!(G.debug & G_DEBUG_GPU)) {
Jeroen Bakker commented 2023-04-17 09:17:40 +02:00
Outdated
Review
Copy Link

obj -> object_handle

`obj` -> `object_handle`
return;
//return;
}
char label[64];
const size_t label_size = 64;
char label[label_size];
memset(label,0,label_size);
static int stats = 0;
SNPRINTF(label, "%s_%d", name, stats++);
object_vk_label(ctx, to_vk_object_type(obj), (uint64_t)obj, label);
object_vk_label(ctx, to_vk_object_type(obj), (uint64_t)obj, (const char*)label);
};
void object_vk_label(VKContext* ctx, VkObjectType objType, uint64_t obj, const char* name);
void pushMarker(VKContext *ctx, VkCommandBuffer cmd, const char *name);
void setMarker(VKContext *ctx, VkCommandBuffer cmd, const char *name);
void popMarker(VKContext *ctx, VkCommandBuffer cmd);
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-04-03 13:50:34 +02:00
Outdated
Review
Copy Link

push_marker / pop_marker / set_marker

`push_marker` / `pop_marker` / `set_marker`
Jeroen Bakker commented 2023-04-17 09:18:14 +02:00
Outdated
Review
Copy Link

Also update the definitions to match the parameter names of the implementation.

Also update the definitions to match the parameter names of the implementation.