blender
/
blender
136
406
Fork 590
Code Issues 6.2k Pull Requests 764 Projects 19 Wiki Activity

Fix : Added prerequisite checks for using VK_Layer_Validation #105922

Merged
Jeroen Bakker merged 16 commits from :vk_debug_break_down into main 2023-03-28 10:45:56 +02:00
Conversation 3 Commits 16 Files Changed 1 +4 -13
1 changed files with 4 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit c0f2c1d00d - Show all commits

17
intern/ghost/intern/GHOST_ContextVK.cpp
View File

@ -82,6 +82,9 @@ static const char *vulkan_error_as_string(VkResult result)
static bool is_vklayer_exist(const char* vk_extension_config)
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-24 11:59:21 +01:00
Outdated
Review
Copy Link

vklayer_config_exists might be a better name.

`vklayer_config_exists` might be a better name.
{
const char *ev_val = getenv("VK_LAYER_PATH");
if (ev_val == nullptr) {
return false;
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-21 13:09:27 +01:00
Outdated
Review
Copy Link

Best to change this to an early exit.

Best to change this to an early exit.
Kazashi Yoshioka commented 2023-03-21 14:12:32 +01:00
Outdated
Review
Copy Link

If you return when ev_val is nullptr, you can't print the warnings. It will check if there is no VK_LAYER_PATH and if there is no json file, so it will not return early.

If you return when ev_val is nullptr, you can't print the warnings. It will check if there is no VK_LAYER_PATH and if there is no json file, so it will not return early.
}
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-21 13:07:36 +01:00
Outdated
Review
Copy Link

The filename should be a parameter. In the future we might want to enable other layers as well. This way it is clear to the developer where to change it.

Not sure concatinating strings is the best solution. would rather use a pathseq

Variables should not start with an _.

The filename should be a parameter. In the future we might want to enable other layers as well. This way it is clear to the developer where to change it. Not sure concatinating strings is the best solution. would rather use a pathseq Variables should not start with an `_`.
bool exists = false;
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-21 13:11:32 +01:00
Outdated
Review
Copy Link

Not sure we need to open the file. can we use file stats?

Not sure we need to open the file. can we use file stats?
Jeroen Bakker commented 2023-03-24 11:53:42 +01:00
Outdated
Review
Copy Link

This section is too C. I would use std::stringstream.

This section is too C. I would use `std::stringstream`.
if (ev_val != nullptr) {
const size_t size_max = strlen(ev_val) + strlen(vk_extension_config) + 2;
@ -94,20 +97,8 @@ static bool is_vklayer_exist(const char* vk_extension_config)
exists = (stat(filename, &buffer) == 0);
free(filename);
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-21 13:10:29 +01:00
Outdated
Review
Copy Link

Just use #else.

Just use #else.
}
if (exists) {
return exists;
}
#if defined(_WIN32)
printf("Warning: VK_LAYER_KHRONOS_validation is deactivated.\nSet `..VulkanSDK/1.2.198.1/Bin` of VulkanSDK (version1.2.198.1) to "
"VK_LAYER_PATH.");
#else
printf("Warning: VK_LAYER_KHRONOS_validation is deactivated.\nSet `..vulkan/explicit_layer.d` of VulkanSDK (version1.2.198.1) to "
"VK_LAYER_PATH.");
#endif
return false;
return exists;
}
vnapdv marked this conversation as resolved Outdated
Jeroen Bakker commented 2023-03-21 13:11:47 +01:00
Outdated
Review
Copy Link

Seems to be an error.

Seems to be an error.
#define __STR(A) "" #A
#define VK_CHECK(__expression) \