blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 773 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 +60 -9
2 changed files with 60 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit fad7e72e51 - Show all commits

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

@ -76,7 +76,6 @@ namespace blender {
tools.enabled = false;
}
}
bool init_vk_callbacks(VKContext* ctx, PFN_vkGetInstanceProcAddr instload)
{
CLOG_ENSURE(&LOG);
@ -87,7 +86,6 @@ namespace blender {
};
return false;
}
void destroy_vk_callbacks(VKContext* ctx)
{
VKDebuggingTools tools = ctx->debuggingtools_get();
@ -95,7 +93,6 @@ namespace blender {
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();
@ -108,6 +105,60 @@ namespace blender {
tools.vkSetDebugUtilsObjectNameEXT_r(ctx->device_get(), &info);
}
}
void pushMarker(blender::gpu::VKContext *ctx, VkCommandBuffer cmd, const char *name)
{
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)
{
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)
{
VKDebuggingTools tools = ctx->debuggingtools_get();
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 (tools.enabled) {
Jeroen Bakker commented 2023-04-04 12:09:11 +02:00
Outdated
Review
Copy Link

objType -> vk_object_type

`objType` -> `vk_object_type`
tools.vkCmdEndDebugUtilsLabelEXT_r(cmd);
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.
}
}
void pushMarker(blender::gpu::VKContext *ctx, VkQueue q, const char *name)
{
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
VkDebugUtilsLabelEXT info = {};
info.sType = VK_STRUCTURE_TYPE_DEBUG_UTILS_LABEL_EXT;
info.pLabelName = name;
tools.vkQueueBeginDebugUtilsLabelEXT_r(q, &info);
}
}
void setMarker(blender::gpu::VKContext *ctx, VkQueue q, const char *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.
{
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)
{
VKDebuggingTools tools = ctx->debuggingtools_get();
if (tools.enabled) {
tools.vkQueueEndDebugUtilsLabelEXT_r(q);
}
}
}
}

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

@ -44,12 +44,12 @@ namespace blender {
object_vk_label(ctx, to_vk_object_type(obj), (uint64_t)obj, label);
};
void object_vk_label(VKContext* ctx, VkObjectType objType, uint64_t obj, const char* name);
void pushMarker(VkCommandBuffer cmd, const char* name);
void setMarker(VkCommandBuffer cmd, const char* name);
void popMarker(VkCommandBuffer cmd);
void pushMarker(VkQueue q, const char* name);
void setMarker(VkQueue q, const char* name);
void popMarker(VkQueue q);
void pushMarker(VKContext *ctx, VkCommandBuffer cmd, const char *name);
void setMarker(VKContext *ctx, VkCommandBuffer cmd, const char *name);
void popMarker(VKContext *ctx, VkCommandBuffer cmd);
void pushMarker(VKContext *ctx, VkQueue q, const char *name);
void setMarker(VKContext *ctx, VkQueue q, const char *name);
void popMarker(VKContext *ctx, VkQueue q);
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.
}
}
}