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

Crash in Project Dialog on Windows - Vulkan #96722

Closed
Gaktan opened this issue Sep 8, 2024 · 4 comments · Fixed by #97006
Closed

Crash in Project Dialog on Windows - Vulkan #96722

Gaktan opened this issue Sep 8, 2024 · 4 comments · Fixed by #97006

Comments

@Gaktan
Copy link
Contributor

Gaktan commented Sep 8, 2024

Tested versions

4.4 custom build at revision 5675c76

System information

Windows 10 - Vulkan

Issue description

Editor crashes when in the project dialog.

Crash callstack shows:

 	0000000000000000()	Unknown
>	godot.windows.editor.dev.x86_64.exe!RenderingContextDriverVulkanWindows::surface_create(const void * p_platform_data) Line 67	C++
 	godot.windows.editor.dev.x86_64.exe!RenderingContextDriver::window_create(int p_window, const void * p_platform_data) Line 46	C++
 	godot.windows.editor.dev.x86_64.exe!DisplayServerWindows::_create_window(DisplayServer::WindowMode p_mode, DisplayServer::VSyncMode p_vsync_mode, unsigned int p_flags, const Rect2i & p_rect, bool p_exclusive, int p_transient_parent) Line 5659	C++
 	godot.windows.editor.dev.x86_64.exe!DisplayServerWindows::create_sub_window(DisplayServer::WindowMode p_mode, DisplayServer::VSyncMode p_vsync_mode, unsigned int p_flags, const Rect2i & p_rect, bool p_exclusive, int p_transient_parent) Line 1472	C++
 	godot.windows.editor.dev.x86_64.exe!Window::_make_window() Line 634	C++
 	godot.windows.editor.dev.x86_64.exe!Window::set_visible(bool p_visible) Line 853	C++
 	godot.windows.editor.dev.x86_64.exe!Window::popup(const Rect2i & p_screen_rect) Line 1866	C++
 	godot.windows.editor.dev.x86_64.exe!Window::popup_centered(const Vector2i & p_minsize) Line 1796	C++
 	godot.windows.editor.dev.x86_64.exe!ProjectManager::_show_quick_settings() Line 390	C++

Where vkCreateWin32SurfaceKHR is nullptr.

But the issue comes much earlier.
The problem is that the vkInstance is created twice. Once for the windows platform by the DisplayServerWindows class as intended.
And a second time for a platform agnostic DisplayServer when calling DisplayServer::can_create_rendering_device().

This is a major issue, since Godot is using Volk, we cannot instatiate multiple vkInstances. Even a dummy one. This is because Volk uses global pointers for storing vulkan functions. The functions are initialized when calling volkLoadInstance. If this function is called multiple times, pointers are overwritten.

I found the incriminating change causing the crash: 04d0e7f.

Function pointers should only be initialized once.
Asserts should be added so we do not overwrite the function pointers in the future.

Steps to reproduce

  1. Open project dialog
  2. Open settings

Minimal reproduction project (MRP)

N/A

@clayjohn
Copy link
Member

clayjohn commented Sep 9, 2024

Where is the vkInstance being created for the first time? The ProjectManager always uses OpenGL, so DisplayServerWindows should never be creating a vkInstance

@Gaktan
Copy link
Contributor Author

Gaktan commented Sep 9, 2024

Ah, I see. I did not realize this ProjectManager was supposed to be using OpenGL.
I disabled the OpenGL module in my custom.py, that explains why it's trying to use Vulkan.

It's a bit strange to me that it uses a different API than the editor and the game (edit: nevermind, I understand. Depending on the project, we could select one API or another. We fallback to OpenGL for maximum compatibility in case an API is not supported).

I still believe that it should work regardless of the API used.

@clayjohn
Copy link
Member

clayjohn commented Sep 9, 2024

I still believe that it should work regardless of the API used.

It should! Just to double check, did you make any other relevant changes other than just disabling OpenGL? Just trying to figure out how to reproduce the crash

@Gaktan
Copy link
Contributor Author

Gaktan commented Sep 9, 2024

No major changes I can think of, apart maybe a few compilation error fixes here and there from disabling modules. Here is my whole custom.py just in case:

# Disabled modules
deprecated = "no"
minizip = "no"
opengl3 = "no"
module_camera_enabled = "no"
module_csg_enabled = "no"
module_enet_enabled = "no"
module_fbx_enabled = "no"
module_gltf_enabled = "no"
module_ktx_enabled = "no"
module_mbedtls_enabled = "no"
module_meshoptimizer_enabled = "no"
module_mobile_vr_enabled = "no"
module_multiplayer_enabled = "no"
module_openxr_enabled = "no"
module_raycast_enabled = "no"
module_squish_enabled = "no"
module_upnp_enabled = "no"
module_vhacd_enabled = "no"
module_webrtc_enabled = "no"
module_websocket_enabled = "no"
module_webxr_enabled = "no"
module_zip_enabled = "no"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants