From c5d9e67cb24667b659452e02650dd862b5cf1100 Mon Sep 17 00:00:00 2001 From: gdkchan Date: Fri, 14 Jul 2023 04:08:52 -0300 Subject: [PATCH] Fix some Vulkan validation errors (#5452) * Fix some validation errors and silence the annoying pipeline barrier error * Remove bogus decref/incref on index buffer state * Make unsafe blit opt-in rather than opt-out * Remove Vulkan debugger messages blacklist * Adjust GetImageUsage to not set the storage bit for multisample textures if not supported --- .../FramebufferParams.cs | 12 ++++++++- .../IndexBufferState.cs | 3 --- src/Ryujinx.Graphics.Vulkan/PipelineBase.cs | 27 ++++++++++++------- src/Ryujinx.Graphics.Vulkan/TextureStorage.cs | 6 ++--- src/Ryujinx.Graphics.Vulkan/TextureView.cs | 11 ++++++-- .../VulkanConfiguration.cs | 2 +- .../VulkanDebugMessenger.cs | 19 ------------- .../VulkanInitialization.cs | 4 +-- 8 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs b/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs index 7b40e96b2e..749d5929cc 100644 --- a/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs +++ b/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs @@ -168,6 +168,16 @@ namespace Ryujinx.Graphics.Vulkan return ComponentType.Float; } + public ImageAspectFlags GetDepthStencilAspectFlags() + { + if (_depthStencil == null) + { + return ImageAspectFlags.None; + } + + return _depthStencil.Info.Format.ConvertAspectFlags(); + } + public bool IsValidColorAttachment(int bindIndex) { return (uint)bindIndex < Constants.MaxRenderTargets && (_validColorAttachments & (1u << bindIndex)) != 0; @@ -226,7 +236,7 @@ namespace Ryujinx.Graphics.Vulkan _depthStencil?.Storage.SetModification( AccessFlags.DepthStencilAttachmentWriteBit, - PipelineStageFlags.ColorAttachmentOutputBit); + PipelineStageFlags.LateFragmentTestsBit); } public void InsertClearBarrier(CommandBufferScoped cbs, int index) diff --git a/src/Ryujinx.Graphics.Vulkan/IndexBufferState.cs b/src/Ryujinx.Graphics.Vulkan/IndexBufferState.cs index e81268e960..b839619e9c 100644 --- a/src/Ryujinx.Graphics.Vulkan/IndexBufferState.cs +++ b/src/Ryujinx.Graphics.Vulkan/IndexBufferState.cs @@ -152,9 +152,6 @@ namespace Ryujinx.Graphics.Vulkan { if (_buffer == from) { - _buffer.DecrementReferenceCount(); - to.IncrementReferenceCount(); - _buffer = to; } } diff --git a/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs b/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs index b76e482b54..b0bc1d0d07 100644 --- a/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs +++ b/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs @@ -245,13 +245,28 @@ namespace Ryujinx.Graphics.Vulkan public unsafe void ClearRenderTargetDepthStencil(int layer, int layerCount, float depthValue, bool depthMask, int stencilValue, int stencilMask) { - // TODO: Use stencilMask (fully) + // TODO: Use stencilMask (fully). if (FramebufferParams == null || !FramebufferParams.HasDepthStencil) { return; } + var clearValue = new ClearValue(null, new ClearDepthStencilValue(depthValue, (uint)stencilValue)); + var flags = depthMask ? ImageAspectFlags.DepthBit : 0; + + if (stencilMask != 0) + { + flags |= ImageAspectFlags.StencilBit; + } + + flags &= FramebufferParams.GetDepthStencilAspectFlags(); + + if (flags == ImageAspectFlags.None) + { + return; + } + if (_renderPass == null) { CreateRenderPass(); @@ -259,14 +274,6 @@ namespace Ryujinx.Graphics.Vulkan BeginRenderPass(); - var clearValue = new ClearValue(null, new ClearDepthStencilValue(depthValue, (uint)stencilValue)); - var flags = depthMask ? ImageAspectFlags.DepthBit : 0; - - if (stencilMask != 0) - { - flags |= ImageAspectFlags.StencilBit; - } - var attachment = new ClearAttachment(flags, 0, clearValue); var clearRect = FramebufferParams.GetClearRect(ClearScissor, layer, layerCount); @@ -935,7 +942,7 @@ namespace Ryujinx.Graphics.Vulkan SignalStateChange(); - if (_program.IsCompute) + if (internalProgram.IsCompute) { EndRenderPass(); } diff --git a/src/Ryujinx.Graphics.Vulkan/TextureStorage.cs b/src/Ryujinx.Graphics.Vulkan/TextureStorage.cs index 5ecd99073a..090f69dca2 100644 --- a/src/Ryujinx.Graphics.Vulkan/TextureStorage.cs +++ b/src/Ryujinx.Graphics.Vulkan/TextureStorage.cs @@ -78,7 +78,7 @@ namespace Ryujinx.Graphics.Vulkan var sampleCountFlags = ConvertToSampleCountFlags(gd.Capabilities.SupportedSampleCounts, (uint)info.Samples); - var usage = GetImageUsage(info.Format, info.Target, gd.Capabilities.SupportsShaderStorageImageMultisample); + var usage = GetImageUsage(info.Format, info.Target, gd.Capabilities.SupportsShaderStorageImageMultisample, forceStorage: true); var flags = ImageCreateFlags.CreateMutableFormatBit; @@ -291,7 +291,7 @@ namespace Ryujinx.Graphics.Vulkan } } - public static ImageUsageFlags GetImageUsage(Format format, Target target, bool supportsMsStorage) + public static ImageUsageFlags GetImageUsage(Format format, Target target, bool supportsMsStorage, bool forceStorage = false) { var usage = DefaultUsageFlags; @@ -304,7 +304,7 @@ namespace Ryujinx.Graphics.Vulkan usage |= ImageUsageFlags.ColorAttachmentBit; } - if (format.IsImageCompatible() && (supportsMsStorage || !target.IsMultisample())) + if (((forceStorage && !format.IsDepthOrStencil()) || format.IsImageCompatible()) && (supportsMsStorage || !target.IsMultisample())) { usage |= ImageUsageFlags.StorageBit; } diff --git a/src/Ryujinx.Graphics.Vulkan/TextureView.cs b/src/Ryujinx.Graphics.Vulkan/TextureView.cs index bb14ea6118..9fc50f67a9 100644 --- a/src/Ryujinx.Graphics.Vulkan/TextureView.cs +++ b/src/Ryujinx.Graphics.Vulkan/TextureView.cs @@ -116,7 +116,14 @@ namespace Ryujinx.Graphics.Vulkan return new Auto(new DisposableImageView(gd.Api, device, imageView), null, storage.GetImage()); } - _imageView = CreateImageView(componentMapping, subresourceRange, type, ImageUsageFlags.SampledBit); + ImageUsageFlags shaderUsage = ImageUsageFlags.SampledBit; + + if (info.Format.IsImageCompatible()) + { + shaderUsage |= ImageUsageFlags.StorageBit; + } + + _imageView = CreateImageView(componentMapping, subresourceRange, type, shaderUsage); // Framebuffer attachments and storage images requires a identity component mapping. var identityComponentMapping = new ComponentMapping( @@ -378,7 +385,7 @@ namespace Ryujinx.Graphics.Vulkan bool isDepthOrStencil = dst.Info.Format.IsDepthOrStencil(); - if (VulkanConfiguration.UseSlowSafeBlitOnAmd && (_gd.Vendor == Vendor.Amd || _gd.IsMoltenVk)) + if (!VulkanConfiguration.UseUnsafeBlit || (_gd.Vendor != Vendor.Nvidia && _gd.Vendor != Vendor.Intel)) { _gd.HelperShader.Blit( _gd, diff --git a/src/Ryujinx.Graphics.Vulkan/VulkanConfiguration.cs b/src/Ryujinx.Graphics.Vulkan/VulkanConfiguration.cs index 752d4f7cc6..5080ebf688 100644 --- a/src/Ryujinx.Graphics.Vulkan/VulkanConfiguration.cs +++ b/src/Ryujinx.Graphics.Vulkan/VulkanConfiguration.cs @@ -3,7 +3,7 @@ static class VulkanConfiguration { public const bool UseFastBufferUpdates = true; - public const bool UseSlowSafeBlitOnAmd = true; + public const bool UseUnsafeBlit = true; public const bool UsePushDescriptors = false; public const bool ForceD24S8Unsupported = false; diff --git a/src/Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs b/src/Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs index 82e30f5be2..496a90fbee 100644 --- a/src/Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs +++ b/src/Ryujinx.Graphics.Vulkan/VulkanDebugMessenger.cs @@ -10,17 +10,6 @@ namespace Ryujinx.Graphics.Vulkan { class VulkanDebugMessenger : IDisposable { - private static readonly string[] _excludedMessages = { - // 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; @@ -108,14 +97,6 @@ namespace Ryujinx.Graphics.Vulkan { 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); diff --git a/src/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs b/src/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs index b0a3ba37bf..02cfe91d9a 100644 --- a/src/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs +++ b/src/Ryujinx.Graphics.Vulkan/VulkanInitialization.cs @@ -377,8 +377,8 @@ namespace Ryujinx.Graphics.Vulkan ShaderFloat64 = supportedFeatures.ShaderFloat64, ShaderImageGatherExtended = supportedFeatures.ShaderImageGatherExtended, ShaderStorageImageMultisample = supportedFeatures.ShaderStorageImageMultisample, - // ShaderStorageImageReadWithoutFormat = true, - // ShaderStorageImageWriteWithoutFormat = true, + ShaderStorageImageReadWithoutFormat = supportedFeatures.ShaderStorageImageReadWithoutFormat, + ShaderStorageImageWriteWithoutFormat = supportedFeatures.ShaderStorageImageWriteWithoutFormat, TessellationShader = supportedFeatures.TessellationShader, VertexPipelineStoresAndAtomics = supportedFeatures.VertexPipelineStoresAndAtomics, RobustBufferAccess = useRobustBufferAccess,