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

VideoConfig: Add flag for whether the system supports setting object names #10422

Merged
merged 11 commits into from Jan 31, 2022

Conversation

OatmealDome
Copy link
Member

Basically, the proper version of #10342.

A flag has been added to VideoConfig: bSupportsSettingObjectNames. This is now checked before calling glObjectLabel in the OpenGL backend and vkSetDebugUtilsObjectNameEXT in the Vulkan backend.

For the OpenGL backend, support is only set if on OpenGL >4.3 or OpenGL ES 3.2. This fixes a crash when using the OpenGL backend on macOS, the Android Emulator, and probably some other devices.

For the Vulkan backend, support is only set if the VK_EXT_debug_utils extension was loaded correctly. In addition, a bug has been fixed where vkSetDebugUtilsObjectNameEXT was loaded as if it were a device function. It is actually an instance function. (This caused Dolphin to fail when loading the function from MoltenVK.)

For the Direct3D backends, support is assumed because I haven't heard of any crashing related to this.

@Pokechu22
Copy link
Contributor

You're missing includes for VideoCommon/VideoConfig.h in all of the Vulkan files (probably one of the other Vulkan files includes it, so it doesn't cause issues, but you should add the include anyways)

@OatmealDome
Copy link
Member Author

Fixed, thanks.

@Pokechu22
Copy link
Contributor

You're only setting bSupportsSettingObjectNames to true for D3D11; VideoBackends/D3D/D3DMain.cpp is for D3D11 while the D3D12 equivalent is VideoBackends/D3D12/VideoBackend.cpp.

Also, to make sure, AddExtension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME, false) returning true implies that vkSetDebugUtilsObjectNameEXT is non-null, right?

@OatmealDome
Copy link
Member Author

Fixed.

And, yes, I believe so. Otherwise, the driver would be in violation of the spec.

@JMC47 JMC47 merged commit 4d1e6ff into dolphin-emu:master Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants