Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vulkan: Make validation layers optional #42361

Merged
merged 1 commit into from Oct 28, 2020

Conversation

@akien-mga
Copy link
Member

@akien-mga akien-mga commented Sep 27, 2020

They're now disabled by default, and can be enabled with the command line
argument --vk-layers.

When enabled, the errors about them being missing are now warnings, as
users were confused and thought this meant Vulkan is broken for them.

Fix crash in ~VulkanContext when validation layers are disabled (exposed by
this PR since before they could not be disabled without source modification).

Also moved VulkanContext member initializations to header.

Fixes #37102.

@akien-mga akien-mga added this to the 4.0 milestone Sep 27, 2020
@akien-mga akien-mga requested a review from reduz Sep 27, 2020
@akien-mga akien-mga force-pushed the akien-mga:vulkan-layers-opt-in branch from da2c836 to a89f17d Sep 27, 2020
@akien-mga
Copy link
Member Author

@akien-mga akien-mga commented Sep 28, 2020

Quick discussion on IRC yesterday:

10:05 <Akien> reduz: OK with making Vk validation layers opt-in? You'll have to build with `vk_layers=yes` or `dev=yes` to get them: #42361.
13:18 <reduz> Akien: I think validation layers are only enabled when an envvar is present, so we may aswell just detect that
13:24 <Akien> reduz: ah I didn't know about that, but indeed it seems they can be enabled using VK_INSTANCE_LAYERS. So I guess we should refactor our code to stop hardcoding them but only rely on the env var?
13:25 <reduz> Akien: yes that is probably best, before going with optimization I also need to add support for named vulkan resources (putting a name to everything basically when compilng with debug) because at this point renderdocs require some deciphering

So it sounds like most of this code could actually be removed. I'll have to do some testing and dive back into the Vulkan tutorial to figure out how to test env-enabled validation layers.

@akien-mga akien-mga changed the title Vulkan: Make validation layers optional [WIP] Vulkan: Make validation layers optional Sep 28, 2020
@akien-mga akien-mga force-pushed the akien-mga:vulkan-layers-opt-in branch from a89f17d to 7fe2683 Oct 27, 2020
@akien-mga
Copy link
Member Author

@akien-mga akien-mga commented Oct 27, 2020

Regarding the previous comment, VK_INSTANCE_LAYERS is used to tell the loader where to find layers if they're not installed in locations that the loader knows about, so it's not useful in this case.

Instead, I moved this to a command line option --vk-layers that enables Vulkan validation layers for debug builds.

Note that currently the engine crashes on cleanup when not using layers, that's a separate bug exposed by this PR which needs to be fixed. Stacktrace:

Thread 1 "godot-git" received signal SIGSEGV, Segmentation fault.
0x0000000002dfb78e in VulkanContext::~VulkanContext (this=0x7e7c800, __in_chrg=<optimized out>) at drivers/vulkan/vulkan_context.cpp:1590
1590                            DestroyDebugUtilsMessengerEXT(inst, dbg_messenger, nullptr);
(gdb) bt
#0  0x0000000002dfb78e in VulkanContext::~VulkanContext (this=0x7e7c800, __in_chrg=<optimized out>) at drivers/vulkan/vulkan_context.cpp:1590
#1  0x0000000001d5d090 in VulkanContextX11::~VulkanContextX11 (this=0x7e7c800, __in_chrg=<optimized out>) at platform/linuxbsd/vulkan_context_x11.cpp:55
#2  0x0000000001d559dc in memdelete<VulkanContextX11> (p_class=0x7e7c800) at ./core/os/memory.h:115
#3  0x0000000001d4e0b5 in DisplayServerX11::~DisplayServerX11 (this=0x7e59550, __in_chrg=<optimized out>) at platform/linuxbsd/display_server_x11.cpp:4068
#4  0x0000000001d7059f in memdelete<DisplayServer> (p_class=0x7e59550) at ./core/os/memory.h:115
#5  0x0000000001d5d8b5 in finalize_display () at main/main.cpp:227
#6  0x0000000001d6f88b in Main::cleanup () at main/main.cpp:2583
#7  0x0000000001d359ac in main (argc=2, argv=0x7fffffffd698) at platform/linuxbsd/godot_linuxbsd.cpp:60
They're now disabled by default, and can be enabled with the command line
argument `--vk-layers`.

When enabled, the errors about them being missing are now warnings, as
users were confused and thought this meant Vulkan is broken for them.

Fix crash in `~VulkanContext` when validation layers are disabled (exposed by
this PR since before they could not be disabled without source modification).

Also moved VulkanContext member initializations to header.

Fixes #37102.
@akien-mga akien-mga force-pushed the akien-mga:vulkan-layers-opt-in branch from 7fe2683 to 54e6338 Oct 27, 2020
@akien-mga
Copy link
Member Author

@akien-mga akien-mga commented Oct 27, 2020

Fixed the crash mentioned above.

@akien-mga akien-mga changed the title [WIP] Vulkan: Make validation layers optional Vulkan: Make validation layers optional Oct 27, 2020
@reduz
reduz approved these changes Oct 28, 2020
@akien-mga akien-mga merged commit 2eaedcf into godotengine:master Oct 28, 2020
10 checks passed
10 checks passed
Editor (target=release_debug, tools=yes, tests=yes)
Details
Template (target=release, tools=no)
Details
Editor (target=release_debug, tools=yes, tests=yes)
Details
Editor w/ Mono (target=release_debug, tools=yes, tests=yes)
Details
Static Checks (clang-format, black format, file format, documentation checks)
Details
Template (target=release, tools=no)
Details
Template (target=release, tools=no)
Details
Template (target=release, tools=no)
Details
Editor with sanitizers (target=debug, tools=yes, tests=yes, use_asan=yes, use_ubsan=yes)
Details
Template w/ Mono (target=release, tools=no)
Details
@akien-mga akien-mga deleted the akien-mga:vulkan-layers-opt-in branch Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.