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

Jump to bottom

Vulkan: Make validation layers optional #42361

Merged
merged 1 commit into from Oct 28, 2020
Merged

Vulkan: Make validation layers optional #42361

merged 1 commit into from Oct 28, 2020

Conversation

akien-mga
Copy link
Member

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

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
Copy link
Member Author

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
Copy link
Member Author

akien-mga commented Oct 27, 2020 •
edited

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

@akien-mga
Vulkan: Make validation layers optional
54e6338 
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 godotengine#37102.
@akien-mga
Copy link
Member Author

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
@akien-mga akien-mga merged commit 2eaedcf into godotengine:master Oct 28, 2020
@akien-mga akien-mga deleted the vulkan-layers-opt-in branch October 28, 2020 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reduz reduz reduz approved these changes

Assignees
No one assigned
Labels
enhancement topic:rendering
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

ERROR: Cant find layer: VK_LAYER_LUNARG_standard_validation and VK_LAYER_GOOGLE_threading
2 participants
@akien-mga @reduz