LibGfx: Add a debug macro to enable Vulkan validations layers #7293
Conversation
Libraries/LibGfx/VulkanContext.cpp
Outdated
| VkDebugUtilsMessengerCallbackDataEXT const* pCallbackData, | ||
| void*) { | ||
| dbgln("Vulkan validation layer: {}", pCallbackData->pMessage); | ||
| // NOTE: The 11 comes from how many frames there are to skip including dump_backtrace, libVkLayer_khronos_validation and libvulkan |
There was a problem hiding this comment.
11 might not always work on all platforms due to optimizations / inlining and such, maybe dial it back to 8 to be safe.
There was a problem hiding this comment.
I think a better solution is to just not cut any frames to not risk cutting too much or too little, and just leave it as is, it's only a debugging tool afterwards.
| find_all_source_files "$@" | ||
| fi \ | ||
| | xargs grep -E '(_DEBUG|DEBUG_)' \ | ||
| | grep -v 'VK_' \ |
There was a problem hiding this comment.
instead of doing this, why not add the false positives at the end of the Meta/CMake/all_the_debug_macros.cmake file
There was a problem hiding this comment.
I feel like this solution is much cleaner, as we shouldn't be setting some random values in CMake just to make a linting script happy.
There was a problem hiding this comment.
I understand, but won't the grep you are using match anything with VK_ in it, could this cause some debugs to be missed.
|
Glad you made this. While waiting I also did my best to forward port it, seems to work well for me. |
cqundefine
force-pushed
the
vulkan_validation_layer
branch
2 times, most recently
from
215507e to
4c71ac3
Compare
This allows to easily toggle Vulkan validation layers which can be really helpful while debugging Vulkan code.
This just filters any of the "VK_*" defines that were mistaken for our debug macros, for example VK_EXT_DEBUG_UTILS_EXTENSION_NAME.
cqundefine
force-pushed
the
vulkan_validation_layer
branch
from
4c71ac3 to
9119eae
Compare
| Array<char const*, 1> extensions = { VK_EXT_DEBUG_UTILS_EXTENSION_NAME }; | ||
|
|
||
| bool validation_layer_available = check_layer_support("VK_LAYER_KHRONOS_validation"sv); | ||
| if (!validation_layer_available) { |
There was a problem hiding this comment.
instead of this big if statement
dbgln("Vulkan validation layers: {}", validation_layer_available ? "active"sv : "not available"sv);
|
You have two commits, probably you are keeping the same ones as the original PR. I think it is probably best to combine everything in one commit as they all work together, and won't be used separately anyway. |
|
Commit message "LibGfx: Add a debug macro to enable Vulkan validations layers", it should be |
|
I created a small section in master...rcorsi:ladybird:vulkan_layer_validation If you like it, feel free to copy it in. |
Honestly, looking at your branch it looks to be in a much cleaner state, so if you would be willing to PR that, I could just close this PR and we could just use your version. |
|
Okay, I don't mind, even though yours and mine were starting to blend together slowly. Its always a bit interesting to see two implementations and where they differ, and sometimes you can even catch some bugs in both implementations and have nice improvements in each. I'll leave you as the author on my commit and I'll be Co-authored-by. Will PR it soon. |
|
Superseded by #7302 |
2 participants
This is a split off from #6976 to make the amount of changes in one PR smaller and this being part being useful for other people.