Fix : Added prerequisite checks for using VK_Layer_Validation #105922
|
@ -23,6 +23,9 @@
|
|||
#include <cstdio>
|
||||
#include <cstring>
|
||||
#include <iostream>
|
||||
#include <sstream>
|
||||
vnapdv marked this conversation as resolved
Outdated
|
||||
|
||||
#include <sys/stat.h>
|
||||
|
||||
/* Set to 0 to allow devices that do not have the required features.
|
||||
* This allows development on OSX until we really needs these features. */
|
||||
|
@ -80,6 +83,21 @@ static const char *vulkan_error_as_string(VkResult result)
|
|||
}
|
||||
}
|
||||
|
||||
enum class VkLayer : uint8_t { KHRONOS_validation };
|
||||
vnapdv marked this conversation as resolved
Outdated
Jeroen Bakker
commented
Best to change this to an early exit. Best to change this to an early exit.
Kazashi Yoshioka
commented
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
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 `_`.
|
||||
static bool vklayer_config_exist(const char *vk_extension_config)
|
||||
vnapdv marked this conversation as resolved
Outdated
Jeroen Bakker
commented
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
This section is too C. I would use This section is too C. I would use `std::stringstream`.
|
||||
{
|
||||
const char *ev_val = getenv("VK_LAYER_PATH");
|
||||
if (ev_val == nullptr) {
|
||||
return false;
|
||||
}
|
||||
vnapdv marked this conversation as resolved
Outdated
Jeroen Bakker
commented
Not sure we should print any message to the user. So perhaps we just want to change the messahe in But then, what is the value of this PR? Not sure we should print any message to the user. `--debug-gpu` is also used by users when reporting bugs. This message will confuse them and ask questions. I rather see this fail silently.
So perhaps we just want to change the messahe in `enableLayer` (supporter=>available).
But then, what is the value of this PR?
Kazashi Yoshioka
commented
What we check with Vulkan Error : GHOST_ContextVK.cpp:937 : vkCreateInstance(&create_info, 0, &m_instance) failled with VK_ERROR_LAYER_NOT_PRESENT What we check with `enableLayer` is not whether it's actually ready for dynamic linking.
If the dll or so files are not properly configured, vkCreateInstance will fail.
This is a check to avoid that error.
-----
Vulkan Error : GHOST_ContextVK.cpp:937 : vkCreateInstance(&create_info, 0, &m_instance) failled with VK_ERROR_LAYER_NOT_PRESENT
|
||||
std::stringstream filename;
|
||||
filename << ev_val;
|
||||
filename << "/" << vk_extension_config;
|
||||
struct stat buffer;
|
||||
return (stat(filename.str().c_str(), &buffer) == 0);
|
||||
vnapdv marked this conversation as resolved
Outdated
Jeroen Bakker
commented
Just use #else. Just use #else.
|
||||
}
|
||||
|
||||
#define __STR(A) "" #A
|
||||
#define VK_CHECK(__expression) \
|
||||
vnapdv marked this conversation as resolved
Outdated
Jeroen Bakker
commented
Seems to be an error. Seems to be an error.
|
||||
do { \
|
||||
|
@ -401,16 +419,38 @@ static bool checkLayerSupport(vector<VkLayerProperties> &layers_available, const
|
|||
|
||||
static void enableLayer(vector<VkLayerProperties> &layers_available,
|
||||
vector<const char *> &layers_enabled,
|
||||
const char *layer_name,
|
||||
const bool debug)
|
||||
const VkLayer layer,
|
||||
const bool display_warning)
|
||||
{
|
||||
if (checkLayerSupport(layers_available, layer_name)) {
|
||||
layers_enabled.push_back(layer_name);
|
||||
#define PUSH_VKLAYER(name, name2) \
|
||||
if (vklayer_config_exist("VkLayer_" #name ".json") && \
|
||||
checkLayerSupport(layers_available, "VK_LAYER_" #name2)) { \
|
||||
layers_enabled.push_back("VK_LAYER_" #name2); \
|
||||
vnapdv marked this conversation as resolved
Outdated
Jeroen Bakker
commented
I think the pattern you're using here will be different when adding the second extension. You don't want to copy the warning for each extension. I think the pattern you're using here will be different when adding the second extension. You don't want to copy the warning for each extension.
|
||||
enabled = true; \
|
||||
} \
|
||||
else { \
|
||||
warnings << "VK_LAYER_" #name2; \
|
||||
}
|
||||
else if (debug) {
|
||||
fprintf(
|
||||
stderr, "Warning: Layer requested, but not supported by the platform. [%s]\n", layer_name);
|
||||
|
||||
bool enabled = false;
|
||||
std::stringstream warnings;
|
||||
|
||||
vnapdv marked this conversation as resolved
Outdated
Jeroen Bakker
commented
I think you wanted to refer to I think you wanted to refer to `layer_name` and `config_name` here.
|
||||
switch (layer) {
|
||||
case VkLayer::KHRONOS_validation:
|
||||
PUSH_VKLAYER(khronos_validation, KHRONOS_validation);
|
||||
};
|
||||
|
||||
if (enabled) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (display_warning) {
|
||||
fprintf(stderr,
|
||||
"Warning: Layer requested, but not supported by the platform. [%s] \n",
|
||||
warnings.str().c_str());
|
||||
}
|
||||
|
||||
#undef PUSH_VKLAYER
|
||||
}
|
||||
|
||||
static bool device_extensions_support(VkPhysicalDevice device, vector<const char *> required_exts)
|
||||
|
@ -864,7 +904,7 @@ GHOST_TSuccess GHOST_ContextVK::initializeDrawingContext()
|
|||
|
||||
vector<const char *> layers_enabled;
|
||||
if (m_debug) {
|
||||
enableLayer(layers_available, layers_enabled, "VK_LAYER_KHRONOS_validation", m_debug);
|
||||
enableLayer(layers_available, layers_enabled, VkLayer::KHRONOS_validation, m_debug);
|
||||
}
|
||||
|
||||
vector<const char *> extensions_device;
|
||||
|
|
System includes should use
<>