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

VideoBackends:Vulkan: Migrate to VK_EXT_debug_utils #11219

Merged
merged 1 commit into from Dec 28, 2022
Merged

VideoBackends:Vulkan: Migrate to VK_EXT_debug_utils #11219

merged 1 commit into from Dec 28, 2022

Conversation

K0bin
Copy link
Contributor

@K0bin K0bin commented Oct 27, 2022 •
edited

Dolphin currently uses VK_EXT_debug_report, which is deprecated. The replacement VK_EXT_debug_utils is widely supported by desktop drivers but not on Android. Thankfully the Vulkan validation layers provide an implementation and the extension is only useful with the validation layers (at least the way Dolphin uses it)

https://developer.android.com/ndk/guides/graphics/validation-layer#enable_the_debug_callback

@K0bin K0bin changed the title Android vk alidation VideoBackends:Vulkan: Fix enabling validation layers on Android Oct 27, 2022
@K0bin K0bin force-pushed the android-vk-alidation branch 3 times, most recently from a9b3ab5 to 5893964 Compare October 27, 2022 21:11
@K0bin
Copy link
Contributor Author

K0bin commented Oct 27, 2022

Also, Dolphin should probably automatically enable the log channel HOST_GPU when validation layers are enabled in the settings.
Otherwise idiots (totally not me) might end up wasting half an hour wondering why they're not getting any validation errors.

@JMC47
Copy link
Contributor

JMC47 commented Dec 19, 2022

Is anyone else going to look at this or is it good to go?

@K0bin K0bin changed the title VideoBackends:Vulkan: Fix enabling validation layers on Android VideoBackends:Vulkan: Migrate to VK_EXT_debug_utils Dec 27, 2022
@K0bin K0bin requested a review from Pokechu22 December 27, 2022 00:59
Pokechu22
Pokechu22 reviewed Dec 27, 2022
View reviewed changes
Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me apart from a few minor things. I haven't tested extensively (only tested on my laptop, and renderdoc+vulkan isn't behaving nicely for me), but I did still see some host GPU messages.

Source/Core/VideoBackends/Vulkan/VulkanContext.cpp Outdated Show resolved Hide resolved
Source/Core/VideoBackends/Vulkan/VulkanContext.cpp Show resolved Hide resolved
Source/Core/VideoBackends/Vulkan/VulkanContext.cpp Show resolved Hide resolved
@K0bin
VideoBackends:Vulkan: Replace debug_report with debug_utils
19a4653 
The former is deprecated and pretty much all modern drivers
support VK_EXT_debug_utils.
Android drivers dont support it. On those drivers,
we use the implementation provided by the validation layers.
@K0bin
Copy link
Contributor Author

K0bin commented Dec 27, 2022

Fixed the nits.

@JMC47 JMC47 merged commit 82e5f43 into dolphin-emu:master Dec 28, 2022
10 of 11 checks passed
@K0bin K0bin deleted the android-vk-alidation branch December 28, 2022 19:20
@iwubcode
Copy link
Contributor

For my machine (AMD rx480 on slightly older drivers) this seems to break validation layers. Reverting allows me to see them again. It's possible there was an improper installation when I upgraded last, I will try and find some time to look at this in more detail.

@iwubcode iwubcode mentioned this pull request Jan 14, 2023
Open
6 tasks
phire
phire reviewed Feb 16, 2023
View reviewed changes
Source/Core/VideoBackends/Vulkan/VulkanContext.cpp
VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT |
VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT |
VK_DEBUG_UTILS_MESSAGE_TYPE_PERFORMANCE_BIT_EXT,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Lets report the performance messages twice and completely ignore the validation errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, that's pretty bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR to fix it is up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phire phire phire left review comments

@Pokechu22 Pokechu22 Pokechu22 approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
5 participants
@K0bin @JMC47 @iwubcode @phire @Pokechu22