From f5a6f45b27d43f8314227e4a8e7f9ddd681a4277 Mon Sep 17 00:00:00 2001
From: Mary <mary@mary.zone>
Date: Sat, 1 Apr 2023 10:05:02 +0200
Subject: [PATCH] vulkan: Separate debug utils logic from VulkanInitialization
 (#4609)

* vulkan: Separate debug utils logic from VulkanInitialization

Also checks for VK_EXT_debug_utils existence instead of force enabling it and allow possible error during messenger init

* Address gdkchan's comment

* Use CreateDebugUtilsMessenger Span variant
---
 .../VulkanDebugMessenger.cs                   | 153 ++++++++++++++++++
 .../VulkanInitialization.cs                   | 115 +------------
 Ryujinx.Graphics.Vulkan/VulkanRenderer.cs     |  15 +-
 3 files changed, 165 insertions(+), 118 deletions(-)
 create mode 100644 Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs

diff --git a/Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs b/Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs
new file mode 100644
index 000000000..7e39a251e
--- /dev/null
+++ b/Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs
@@ -0,0 +1,153 @@
+using Ryujinx.Common.Configuration;
+using Ryujinx.Common.Logging;
+using Ryujinx.Common.Utilities;
+using Silk.NET.Vulkan;
+using Silk.NET.Vulkan.Extensions.EXT;
+using System;
+using System.Runtime.InteropServices;
+
+namespace Ryujinx.Graphics.Vulkan
+{
+    class VulkanDebugMessenger : IDisposable
+    {
+        private static string[] _excludedMessages = new string[]
+        {
+            // NOTE: Done on purpose right now.
+            "UNASSIGNED-CoreValidation-Shader-OutputNotConsumed",
+            // TODO: Figure out if fixable
+            "VUID-vkCmdDrawIndexed-None-04584",
+            // TODO: Might be worth looking into making this happy to possibly optimize copies.
+            "UNASSIGNED-CoreValidation-DrawState-InvalidImageLayout",
+            // TODO: Fix this, it's causing too much noise right now.
+            "VUID-VkSubpassDependency-srcSubpass-00867"
+        };
+
+        private readonly Vk _api;
+        private readonly Instance _instance;
+        private readonly GraphicsDebugLevel _logLevel;
+        private readonly ExtDebugUtils _debugUtils;
+        private readonly DebugUtilsMessengerEXT? _debugUtilsMessenger;
+        private bool _disposed;
+
+        public VulkanDebugMessenger(Vk api, Instance instance, GraphicsDebugLevel logLevel)
+        {
+            _api = api;
+            _instance = instance;
+            _logLevel = logLevel;
+
+            _api.TryGetInstanceExtension(instance, out _debugUtils);
+
+            Result result = TryInitialize(out _debugUtilsMessenger);
+
+            if (result != Result.Success)
+            {
+                Logger.Error?.Print(LogClass.Gpu, $"Vulkan debug messenger initialization failed with error {result}");
+            }
+        }
+
+        private Result TryInitialize(out DebugUtilsMessengerEXT? debugUtilsMessengerHandle)
+        {
+            debugUtilsMessengerHandle = null;
+            
+            if (_debugUtils != null && _logLevel != GraphicsDebugLevel.None)
+            {
+                var messageType = _logLevel switch
+                {
+                    GraphicsDebugLevel.Error => DebugUtilsMessageTypeFlagsEXT.ValidationBitExt,
+                    GraphicsDebugLevel.Slowdowns => DebugUtilsMessageTypeFlagsEXT.ValidationBitExt |
+                                                    DebugUtilsMessageTypeFlagsEXT.PerformanceBitExt,
+                    GraphicsDebugLevel.All => DebugUtilsMessageTypeFlagsEXT.GeneralBitExt |
+                                              DebugUtilsMessageTypeFlagsEXT.ValidationBitExt |
+                                              DebugUtilsMessageTypeFlagsEXT.PerformanceBitExt,
+                    _ => throw new ArgumentException($"Invalid log level \"{_logLevel}\".")
+                };
+
+                var messageSeverity = _logLevel switch
+                {
+                    GraphicsDebugLevel.Error => DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt,
+                    GraphicsDebugLevel.Slowdowns => DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt |
+                                                    DebugUtilsMessageSeverityFlagsEXT.WarningBitExt,
+                    GraphicsDebugLevel.All => DebugUtilsMessageSeverityFlagsEXT.InfoBitExt |
+                                              DebugUtilsMessageSeverityFlagsEXT.WarningBitExt |
+                                              DebugUtilsMessageSeverityFlagsEXT.VerboseBitExt |
+                                              DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt,
+                    _ => throw new ArgumentException($"Invalid log level \"{_logLevel}\".")
+                };
+
+                var debugUtilsMessengerCreateInfo = new DebugUtilsMessengerCreateInfoEXT()
+                {
+                    SType = StructureType.DebugUtilsMessengerCreateInfoExt,
+                    MessageType = messageType,
+                    MessageSeverity = messageSeverity
+                };
+
+                unsafe
+                {
+                    debugUtilsMessengerCreateInfo.PfnUserCallback = new PfnDebugUtilsMessengerCallbackEXT(UserCallback);
+                }
+
+                DebugUtilsMessengerEXT messengerHandle = default;
+
+                Result result = _debugUtils.CreateDebugUtilsMessenger(_instance, SpanHelpers.AsReadOnlySpan(ref debugUtilsMessengerCreateInfo), ReadOnlySpan<AllocationCallbacks>.Empty, SpanHelpers.AsSpan(ref messengerHandle));
+
+                if (result == Result.Success)
+                {
+                    debugUtilsMessengerHandle = messengerHandle;
+                }
+
+                return result;
+            }
+
+            return Result.Success;
+        }
+
+        private unsafe static uint UserCallback(
+            DebugUtilsMessageSeverityFlagsEXT messageSeverity,
+            DebugUtilsMessageTypeFlagsEXT messageTypes,
+            DebugUtilsMessengerCallbackDataEXT* pCallbackData,
+            void* pUserData)
+        {
+            var msg = Marshal.PtrToStringAnsi((IntPtr)pCallbackData->PMessage);
+
+            foreach (string excludedMessagePart in _excludedMessages)
+            {
+                if (msg.Contains(excludedMessagePart))
+                {
+                    return 0;
+                }
+            }
+
+            if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt))
+            {
+                Logger.Error?.Print(LogClass.Gpu, msg);
+            }
+            else if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.WarningBitExt))
+            {
+                Logger.Warning?.Print(LogClass.Gpu, msg);
+            }
+            else if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.InfoBitExt))
+            {
+                Logger.Info?.Print(LogClass.Gpu, msg);
+            }
+            else // if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.VerboseBitExt))
+            {
+                Logger.Debug?.Print(LogClass.Gpu, msg);
+            }
+
+            return 0;
+        }
+
+        public void Dispose()
+        {
+            if (!_disposed)
+            {
+                if (_debugUtilsMessenger.HasValue)
+                {
+                    _debugUtils.DestroyDebugUtilsMessenger(_instance, _debugUtilsMessenger.Value, Span<AllocationCallbacks>.Empty);
+                }
+
+                _disposed = true;
+            }
+        }
+    }
+}
diff --git a/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs b/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs
index c4e9a6266..f04ab5c05 100644
--- a/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs
+++ b/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs
@@ -47,19 +47,7 @@ namespace Ryujinx.Graphics.Vulkan
             KhrSwapchain.ExtensionName
         };
 
-        private static string[] _excludedMessages = new string[]
-        {
-            // NOTE: Done on purpose right now.
-            "UNASSIGNED-CoreValidation-Shader-OutputNotConsumed",
-            // TODO: Figure out if fixable
-            "VUID-vkCmdDrawIndexed-None-04584",
-            // TODO: Might be worth looking into making this happy to possibly optimize copies.
-            "UNASSIGNED-CoreValidation-DrawState-InvalidImageLayout",
-            // TODO: Fix this, it's causing too much noise right now.
-            "VUID-VkSubpassDependency-srcSubpass-00867"
-        };
-
-        internal static Instance CreateInstance(Vk api, GraphicsDebugLevel logLevel, string[] requiredExtensions, out ExtDebugUtils debugUtils, out DebugUtilsMessengerEXT debugUtilsMessenger)
+        internal static Instance CreateInstance(Vk api, GraphicsDebugLevel logLevel, string[] requiredExtensions)
         {
             var enabledLayers = new List<string>();
 
@@ -95,7 +83,12 @@ namespace Ryujinx.Graphics.Vulkan
                 AddAvailableLayer("VK_LAYER_KHRONOS_validation");
             }
 
-            var enabledExtensions = requiredExtensions.Append(ExtDebugUtils.ExtensionName).ToArray();
+            var enabledExtensions = requiredExtensions;
+
+            if (api.IsInstanceExtensionPresent("VK_EXT_debug_utils"))
+            {
+                enabledExtensions = enabledExtensions.Append(ExtDebugUtils.ExtensionName).ToArray();
+            }
 
             var appName = Marshal.StringToHGlobalAnsi(AppName);
 
@@ -145,47 +138,9 @@ namespace Ryujinx.Graphics.Vulkan
                 Marshal.FreeHGlobal(ppEnabledLayers[i]);
             }
 
-            CreateDebugMessenger(api, logLevel, instance, out debugUtils, out debugUtilsMessenger);
-
             return instance;
         }
 
-        private unsafe static uint DebugMessenger(
-            DebugUtilsMessageSeverityFlagsEXT messageSeverity,
-            DebugUtilsMessageTypeFlagsEXT messageTypes,
-            DebugUtilsMessengerCallbackDataEXT* pCallbackData,
-            void* pUserData)
-        {
-            var msg = Marshal.PtrToStringAnsi((IntPtr)pCallbackData->PMessage);
-
-            foreach (string excludedMessagePart in _excludedMessages)
-            {
-                if (msg.Contains(excludedMessagePart))
-                {
-                    return 0;
-                }
-            }
-
-            if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt))
-            {
-                Logger.Error?.Print(LogClass.Gpu, msg);
-            }
-            else if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.WarningBitExt))
-            {
-                Logger.Warning?.Print(LogClass.Gpu, msg);
-            }
-            else if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.InfoBitExt))
-            {
-                Logger.Info?.Print(LogClass.Gpu, msg);
-            }
-            else // if (messageSeverity.HasFlag(DebugUtilsMessageSeverityFlagsEXT.VerboseBitExt))
-            {
-                Logger.Debug?.Print(LogClass.Gpu, msg);
-            }
-
-            return 0;
-        }
-
         internal static PhysicalDevice FindSuitablePhysicalDevice(Vk api, Instance instance, SurfaceKHR surface, string preferredGpuId)
         {
             uint physicalDeviceCount;
@@ -671,61 +626,5 @@ namespace Ryujinx.Graphics.Vulkan
 
             return extensionProperties.Select(x => Marshal.PtrToStringAnsi((IntPtr)x.ExtensionName)).ToArray();
         }
-
-        internal unsafe static void CreateDebugMessenger(
-            Vk api,
-            GraphicsDebugLevel logLevel,
-            Instance instance,
-            out ExtDebugUtils debugUtils,
-            out DebugUtilsMessengerEXT debugUtilsMessenger)
-        {
-            debugUtils = default;
-
-            if (logLevel != GraphicsDebugLevel.None)
-            {
-                if (!api.TryGetInstanceExtension(instance, out debugUtils))
-                {
-                    debugUtilsMessenger = default;
-                    return;
-                }
-
-                var filterLogType = logLevel switch
-                {
-                    GraphicsDebugLevel.Error => DebugUtilsMessageTypeFlagsEXT.ValidationBitExt,
-                    GraphicsDebugLevel.Slowdowns => DebugUtilsMessageTypeFlagsEXT.ValidationBitExt |
-                                                    DebugUtilsMessageTypeFlagsEXT.PerformanceBitExt,
-                    GraphicsDebugLevel.All => DebugUtilsMessageTypeFlagsEXT.GeneralBitExt |
-                                              DebugUtilsMessageTypeFlagsEXT.ValidationBitExt |
-                                              DebugUtilsMessageTypeFlagsEXT.PerformanceBitExt,
-                    _ => throw new ArgumentException($"Invalid log level \"{logLevel}\".")
-                };
-
-                var filterLogSeverity = logLevel switch
-                {
-                    GraphicsDebugLevel.Error => DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt,
-                    GraphicsDebugLevel.Slowdowns => DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt |
-                                                    DebugUtilsMessageSeverityFlagsEXT.WarningBitExt,
-                    GraphicsDebugLevel.All => DebugUtilsMessageSeverityFlagsEXT.InfoBitExt |
-                                              DebugUtilsMessageSeverityFlagsEXT.WarningBitExt |
-                                              DebugUtilsMessageSeverityFlagsEXT.VerboseBitExt |
-                                              DebugUtilsMessageSeverityFlagsEXT.ErrorBitExt,
-                    _ => throw new ArgumentException($"Invalid log level \"{logLevel}\".")
-                };
-
-                var debugUtilsMessengerCreateInfo = new DebugUtilsMessengerCreateInfoEXT()
-                {
-                    SType = StructureType.DebugUtilsMessengerCreateInfoExt,
-                    MessageType = filterLogType,
-                    MessageSeverity = filterLogSeverity,
-                    PfnUserCallback = new PfnDebugUtilsMessengerCallbackEXT(DebugMessenger)
-                };
-
-                debugUtils.CreateDebugUtilsMessenger(instance, in debugUtilsMessengerCreateInfo, null, out debugUtilsMessenger).ThrowOnError();
-            }
-            else
-            {
-                debugUtilsMessenger = default;
-            }
-        }
     }
 }
diff --git a/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs b/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs
index d8cb5e2b6..81dec12ec 100644
--- a/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs
+++ b/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs
@@ -36,7 +36,6 @@ namespace Ryujinx.Graphics.Vulkan
         internal KhrPushDescriptor PushDescriptorApi { get; private set; }
         internal ExtTransformFeedback TransformFeedbackApi { get; private set; }
         internal KhrDrawIndirectCount DrawIndirectCountApi { get; private set; }
-        internal ExtDebugUtils DebugUtilsApi { get; private set; }
 
         internal uint QueueFamilyIndex { get; private set; }
         internal Queue Queue { get; private set; }
@@ -57,11 +56,11 @@ namespace Ryujinx.Graphics.Vulkan
         internal HashSet<ITexture> Textures { get; }
         internal HashSet<SamplerHolder> Samplers { get; }
 
+        private VulkanDebugMessenger _debugMessenger;
         private Counters _counters;
         private SyncManager _syncManager;
 
         private PipelineFull _pipeline;
-        private DebugUtilsMessengerEXT _debugUtilsMessenger;
 
         internal HelperShader HelperShader { get; private set; }
         internal PipelineFull PipelineInternal => _pipeline;
@@ -345,9 +344,8 @@ namespace Ryujinx.Graphics.Vulkan
 
             Api = api;
 
-            _instance = VulkanInitialization.CreateInstance(api, logLevel, _getRequiredExtensions(), out ExtDebugUtils debugUtils, out _debugUtilsMessenger);
-
-            DebugUtilsApi = debugUtils;
+            _instance = VulkanInitialization.CreateInstance(api, logLevel, _getRequiredExtensions());
+            _debugMessenger = new VulkanDebugMessenger(api, _instance, logLevel);
 
             if (api.TryGetInstanceExtension(_instance, out KhrSurface surfaceApi))
             {
@@ -794,11 +792,6 @@ namespace Ryujinx.Graphics.Vulkan
 
             MemoryAllocator.Dispose();
 
-            if (_debugUtilsMessenger.Handle != 0)
-            {
-                DebugUtilsApi.DestroyDebugUtilsMessenger(_instance, _debugUtilsMessenger, null);
-            }
-
             foreach (var shader in Shaders)
             {
                 shader.Dispose();
@@ -818,6 +811,8 @@ namespace Ryujinx.Graphics.Vulkan
 
             Api.DestroyDevice(_device, null);
 
+            _debugMessenger.Dispose();
+
             // Last step destroy the instance
             Api.DestroyInstance(_instance, null);
         }