From 7ccff037e87f82f3461f3e1422235e29800eaa2f Mon Sep 17 00:00:00 2001 From: gdkchan Date: Thu, 14 Sep 2023 14:58:11 -0300 Subject: [PATCH] Fix some Vulkan validation errors (mostly related to barriers) (#5603) * Replace image barriers inside render pass with more generic memory barrier * Remove forceStorage since it was creating images with storage bit for formats that are not StorageImage compatible * Add missing flags on subpass dependency * Don't call vkCmdSetScissor with a scissor count of 0 * One semaphore per swapchain image * Remove compute stage from read to write barriers * Try to improve Pipeline.Barrier nonsense * Set PipelineStateFlags based on supported stages --- .../FramebufferParams.cs | 12 +++- .../HardwareCapabilities.cs | 3 + src/Ryujinx.Graphics.Vulkan/PipelineBase.cs | 16 ++++- .../PipelineConverter.cs | 16 +++-- .../PipelineDynamicState.cs | 5 +- src/Ryujinx.Graphics.Vulkan/TextureStorage.cs | 46 ++++++------ src/Ryujinx.Graphics.Vulkan/TextureView.cs | 28 ++++++++ src/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs | 1 + src/Ryujinx.Graphics.Vulkan/Window.cs | 72 ++++++++++++++----- 9 files changed, 145 insertions(+), 54 deletions(-) diff --git a/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs b/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs index 7600c2d5e..662bb80f8 100644 --- a/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs +++ b/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs @@ -257,14 +257,22 @@ namespace Ryujinx.Graphics.Vulkan if (realIndex != -1) { - _colors[realIndex].Storage?.InsertReadToWriteBarrier(cbs, AccessFlags.ColorAttachmentWriteBit, PipelineStageFlags.ColorAttachmentOutputBit); + _colors[realIndex].Storage?.InsertReadToWriteBarrier( + cbs, + AccessFlags.ColorAttachmentWriteBit, + PipelineStageFlags.ColorAttachmentOutputBit, + insideRenderPass: true); } } } public void InsertClearBarrierDS(CommandBufferScoped cbs) { - _depthStencil?.Storage?.InsertReadToWriteBarrier(cbs, AccessFlags.DepthStencilAttachmentWriteBit, PipelineStageFlags.LateFragmentTestsBit); + _depthStencil?.Storage?.InsertReadToWriteBarrier( + cbs, + AccessFlags.DepthStencilAttachmentWriteBit, + PipelineStageFlags.LateFragmentTestsBit, + insideRenderPass: true); } } } diff --git a/src/Ryujinx.Graphics.Vulkan/HardwareCapabilities.cs b/src/Ryujinx.Graphics.Vulkan/HardwareCapabilities.cs index 798de5c90..32e941dba 100644 --- a/src/Ryujinx.Graphics.Vulkan/HardwareCapabilities.cs +++ b/src/Ryujinx.Graphics.Vulkan/HardwareCapabilities.cs @@ -41,6 +41,7 @@ namespace Ryujinx.Graphics.Vulkan public readonly bool SupportsPreciseOcclusionQueries; public readonly bool SupportsPipelineStatisticsQuery; public readonly bool SupportsGeometryShader; + public readonly bool SupportsTessellationShader; public readonly bool SupportsViewportArray2; public readonly bool SupportsHostImportedMemory; public readonly bool SupportsDepthClipControl; @@ -77,6 +78,7 @@ namespace Ryujinx.Graphics.Vulkan bool supportsPreciseOcclusionQueries, bool supportsPipelineStatisticsQuery, bool supportsGeometryShader, + bool supportsTessellationShader, bool supportsViewportArray2, bool supportsHostImportedMemory, bool supportsDepthClipControl, @@ -112,6 +114,7 @@ namespace Ryujinx.Graphics.Vulkan SupportsPreciseOcclusionQueries = supportsPreciseOcclusionQueries; SupportsPipelineStatisticsQuery = supportsPipelineStatisticsQuery; SupportsGeometryShader = supportsGeometryShader; + SupportsTessellationShader = supportsTessellationShader; SupportsViewportArray2 = supportsViewportArray2; SupportsHostImportedMemory = supportsHostImportedMemory; SupportsDepthClipControl = supportsDepthClipControl; diff --git a/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs b/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs index 54b67f35e..156b3db16 100644 --- a/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs +++ b/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs @@ -149,10 +149,22 @@ namespace Ryujinx.Graphics.Vulkan DstAccessMask = AccessFlags.MemoryReadBit | AccessFlags.MemoryWriteBit, }; + PipelineStageFlags pipelineStageFlags = PipelineStageFlags.VertexShaderBit | PipelineStageFlags.FragmentShaderBit; + + if (Gd.Capabilities.SupportsGeometryShader) + { + pipelineStageFlags |= PipelineStageFlags.GeometryShaderBit; + } + + if (Gd.Capabilities.SupportsTessellationShader) + { + pipelineStageFlags |= PipelineStageFlags.TessellationControlShaderBit | PipelineStageFlags.TessellationEvaluationShaderBit; + } + Gd.Api.CmdPipelineBarrier( CommandBuffer, - PipelineStageFlags.FragmentShaderBit, - PipelineStageFlags.FragmentShaderBit, + pipelineStageFlags, + pipelineStageFlags, 0, 1, memoryBarrier, diff --git a/src/Ryujinx.Graphics.Vulkan/PipelineConverter.cs b/src/Ryujinx.Graphics.Vulkan/PipelineConverter.cs index 7c1ddef8d..b2da61031 100644 --- a/src/Ryujinx.Graphics.Vulkan/PipelineConverter.cs +++ b/src/Ryujinx.Graphics.Vulkan/PipelineConverter.cs @@ -9,8 +9,12 @@ namespace Ryujinx.Graphics.Vulkan { static class PipelineConverter { - private const AccessFlags SubpassSrcAccessMask = AccessFlags.MemoryReadBit | AccessFlags.MemoryWriteBit | AccessFlags.ColorAttachmentWriteBit; - private const AccessFlags SubpassDstAccessMask = AccessFlags.MemoryReadBit | AccessFlags.MemoryWriteBit | AccessFlags.ShaderReadBit; + private const AccessFlags SubpassAccessMask = + AccessFlags.MemoryReadBit | + AccessFlags.MemoryWriteBit | + AccessFlags.ShaderReadBit | + AccessFlags.ColorAttachmentWriteBit | + AccessFlags.DepthStencilAttachmentWriteBit; public static unsafe DisposableRenderPass ToRenderPass(this ProgramPipelineState state, VulkanRenderer gd, Device device) { @@ -132,8 +136,8 @@ namespace Ryujinx.Graphics.Vulkan 0, PipelineStageFlags.AllGraphicsBit, PipelineStageFlags.AllGraphicsBit, - SubpassSrcAccessMask, - SubpassDstAccessMask, + SubpassAccessMask, + SubpassAccessMask, 0); } @@ -146,8 +150,8 @@ namespace Ryujinx.Graphics.Vulkan 0, PipelineStageFlags.AllGraphicsBit, PipelineStageFlags.AllGraphicsBit, - SubpassSrcAccessMask, - SubpassDstAccessMask, + SubpassAccessMask, + SubpassAccessMask, 0); } diff --git a/src/Ryujinx.Graphics.Vulkan/PipelineDynamicState.cs b/src/Ryujinx.Graphics.Vulkan/PipelineDynamicState.cs index ff88e4cf4..a5c218ac2 100644 --- a/src/Ryujinx.Graphics.Vulkan/PipelineDynamicState.cs +++ b/src/Ryujinx.Graphics.Vulkan/PipelineDynamicState.cs @@ -146,7 +146,10 @@ namespace Ryujinx.Graphics.Vulkan private void RecordScissor(Vk api, CommandBuffer commandBuffer) { - api.CmdSetScissor(commandBuffer, 0, (uint)ScissorsCount, _scissors.AsSpan()); + if (ScissorsCount != 0) + { + api.CmdSetScissor(commandBuffer, 0, (uint)ScissorsCount, _scissors.AsSpan()); + } } private readonly void RecordStencilMasks(Vk api, CommandBuffer commandBuffer) diff --git a/src/Ryujinx.Graphics.Vulkan/TextureStorage.cs b/src/Ryujinx.Graphics.Vulkan/TextureStorage.cs index 090f69dca..e9c2bf1ec 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, forceStorage: true); + var usage = GetImageUsage(info.Format, info.Target, gd.Capabilities.SupportsShaderStorageImageMultisample); var flags = ImageCreateFlags.CreateMutableFormatBit; @@ -291,7 +291,7 @@ namespace Ryujinx.Graphics.Vulkan } } - public static ImageUsageFlags GetImageUsage(Format format, Target target, bool supportsMsStorage, bool forceStorage = false) + public static ImageUsageFlags GetImageUsage(Format format, Target target, bool supportsMsStorage) { var usage = DefaultUsageFlags; @@ -304,7 +304,7 @@ namespace Ryujinx.Graphics.Vulkan usage |= ImageUsageFlags.ColorAttachmentBit; } - if (((forceStorage && !format.IsDepthOrStencil()) || format.IsImageCompatible()) && (supportsMsStorage || !target.IsMultisample())) + if (format.IsImageCompatible() && (supportsMsStorage || !target.IsMultisample())) { usage |= ImageUsageFlags.StorageBit; } @@ -440,25 +440,27 @@ namespace Ryujinx.Graphics.Vulkan _lastModificationStage = stage; } - public void InsertReadToWriteBarrier(CommandBufferScoped cbs, AccessFlags dstAccessFlags, PipelineStageFlags dstStageFlags) + public void InsertReadToWriteBarrier(CommandBufferScoped cbs, AccessFlags dstAccessFlags, PipelineStageFlags dstStageFlags, bool insideRenderPass) { - if (_lastReadAccess != AccessFlags.None) - { - ImageAspectFlags aspectFlags = Info.Format.ConvertAspectFlags(); + var lastReadStage = _lastReadStage; - TextureView.InsertImageBarrier( + if (insideRenderPass) + { + // We can't have barrier from compute inside a render pass, + // as it is invalid to specify compute in the subpass dependency stage mask. + + lastReadStage &= ~PipelineStageFlags.ComputeShaderBit; + } + + if (lastReadStage != PipelineStageFlags.None) + { + TextureView.InsertMemoryBarrier( _gd.Api, cbs.CommandBuffer, - _imageAuto.Get(cbs).Value, _lastReadAccess, dstAccessFlags, - _lastReadStage, - dstStageFlags, - aspectFlags, - 0, - 0, - _info.GetLayers(), - _info.Levels); + lastReadStage, + dstStageFlags); _lastReadAccess = AccessFlags.None; _lastReadStage = PipelineStageFlags.None; @@ -472,21 +474,13 @@ namespace Ryujinx.Graphics.Vulkan if (_lastModificationAccess != AccessFlags.None) { - ImageAspectFlags aspectFlags = Info.Format.ConvertAspectFlags(); - - TextureView.InsertImageBarrier( + TextureView.InsertMemoryBarrier( _gd.Api, cbs.CommandBuffer, - _imageAuto.Get(cbs).Value, _lastModificationAccess, dstAccessFlags, _lastModificationStage, - dstStageFlags, - aspectFlags, - 0, - 0, - _info.GetLayers(), - _info.Levels); + dstStageFlags); _lastModificationAccess = AccessFlags.None; } diff --git a/src/Ryujinx.Graphics.Vulkan/TextureView.cs b/src/Ryujinx.Graphics.Vulkan/TextureView.cs index 9fc50f67a..09128f007 100644 --- a/src/Ryujinx.Graphics.Vulkan/TextureView.cs +++ b/src/Ryujinx.Graphics.Vulkan/TextureView.cs @@ -435,6 +435,34 @@ namespace Ryujinx.Graphics.Vulkan ImageAspectFlags.ColorBit); } + public static unsafe void InsertMemoryBarrier( + Vk api, + CommandBuffer commandBuffer, + AccessFlags srcAccessMask, + AccessFlags dstAccessMask, + PipelineStageFlags srcStageMask, + PipelineStageFlags dstStageMask) + { + MemoryBarrier memoryBarrier = new() + { + SType = StructureType.MemoryBarrier, + SrcAccessMask = srcAccessMask, + DstAccessMask = dstAccessMask, + }; + + api.CmdPipelineBarrier( + commandBuffer, + srcStageMask, + dstStageMask, + DependencyFlags.None, + 1, + memoryBarrier, + 0, + null, + 0, + null); + } + public static unsafe void InsertImageBarrier( Vk api, CommandBuffer commandBuffer, diff --git a/src/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs b/src/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs index 7b6b89a74..ac598c587 100644 --- a/src/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs +++ b/src/Ryujinx.Graphics.Vulkan/VulkanRenderer.cs @@ -327,6 +327,7 @@ namespace Ryujinx.Graphics.Vulkan features2.Features.OcclusionQueryPrecise, _physicalDevice.PhysicalDeviceFeatures.PipelineStatisticsQuery, _physicalDevice.PhysicalDeviceFeatures.GeometryShader, + _physicalDevice.PhysicalDeviceFeatures.TessellationShader, _physicalDevice.IsDeviceExtensionPresent("VK_NV_viewport_array2"), _physicalDevice.IsDeviceExtensionPresent(ExtExternalMemoryHost.ExtensionName), supportsDepthClipControl && featuresDepthClipControl.DepthClipControl, diff --git a/src/Ryujinx.Graphics.Vulkan/Window.cs b/src/Ryujinx.Graphics.Vulkan/Window.cs index 2d0ad664c..afaa7defe 100644 --- a/src/Ryujinx.Graphics.Vulkan/Window.cs +++ b/src/Ryujinx.Graphics.Vulkan/Window.cs @@ -22,8 +22,10 @@ namespace Ryujinx.Graphics.Vulkan private Image[] _swapchainImages; private Auto[] _swapchainImageViews; - private Semaphore _imageAvailableSemaphore; - private Semaphore _renderFinishedSemaphore; + private Semaphore[] _imageAvailableSemaphores; + private Semaphore[] _renderFinishedSemaphores; + + private int _frameIndex; private int _width; private int _height; @@ -48,14 +50,6 @@ namespace Ryujinx.Graphics.Vulkan _surface = surface; CreateSwapchain(); - - var semaphoreCreateInfo = new SemaphoreCreateInfo - { - SType = StructureType.SemaphoreCreateInfo, - }; - - gd.Api.CreateSemaphore(device, semaphoreCreateInfo, null, out _imageAvailableSemaphore).ThrowOnError(); - gd.Api.CreateSemaphore(device, semaphoreCreateInfo, null, out _renderFinishedSemaphore).ThrowOnError(); } private void RecreateSwapchain() @@ -69,7 +63,22 @@ namespace Ryujinx.Graphics.Vulkan } // Destroy old Swapchain. + _gd.Api.DeviceWaitIdle(_device); + + unsafe + { + for (int i = 0; i < _imageAvailableSemaphores.Length; i++) + { + _gd.Api.DestroySemaphore(_device, _imageAvailableSemaphores[i], null); + } + + for (int i = 0; i < _renderFinishedSemaphores.Length; i++) + { + _gd.Api.DestroySemaphore(_device, _renderFinishedSemaphores[i], null); + } + } + _gd.SwapchainApi.DestroySwapchain(_device, oldSwapchain, Span.Empty); CreateSwapchain(); @@ -151,6 +160,25 @@ namespace Ryujinx.Graphics.Vulkan { _swapchainImageViews[i] = CreateSwapchainImageView(_swapchainImages[i], surfaceFormat.Format); } + + var semaphoreCreateInfo = new SemaphoreCreateInfo + { + SType = StructureType.SemaphoreCreateInfo, + }; + + _imageAvailableSemaphores = new Semaphore[imageCount]; + + for (int i = 0; i < _imageAvailableSemaphores.Length; i++) + { + _gd.Api.CreateSemaphore(_device, semaphoreCreateInfo, null, out _imageAvailableSemaphores[i]).ThrowOnError(); + } + + _renderFinishedSemaphores = new Semaphore[imageCount]; + + for (int i = 0; i < _renderFinishedSemaphores.Length; i++) + { + _gd.Api.CreateSemaphore(_device, semaphoreCreateInfo, null, out _renderFinishedSemaphores[i]).ThrowOnError(); + } } private unsafe Auto CreateSwapchainImageView(Image swapchainImage, VkFormat format) @@ -185,6 +213,7 @@ namespace Ryujinx.Graphics.Vulkan { return new SurfaceFormatKHR(VkFormat.B8G8R8A8Unorm, ColorSpaceKHR.PaceSrgbNonlinearKhr); } + var formatToReturn = availableFormats[0]; if (colorSpacePassthroughEnabled) { @@ -212,6 +241,7 @@ namespace Ryujinx.Graphics.Vulkan } } } + return formatToReturn; } @@ -265,6 +295,7 @@ namespace Ryujinx.Graphics.Vulkan _gd.PipelineInternal.AutoFlush.Present(); uint nextImage = 0; + int semaphoreIndex = _frameIndex++ % _imageAvailableSemaphores.Length; while (true) { @@ -272,7 +303,7 @@ namespace Ryujinx.Graphics.Vulkan _device, _swapchain, ulong.MaxValue, - _imageAvailableSemaphore, + _imageAvailableSemaphores[semaphoreIndex], new Fence(), ref nextImage); @@ -411,12 +442,12 @@ namespace Ryujinx.Graphics.Vulkan _gd.CommandBufferPool.Return( cbs, - stackalloc[] { _imageAvailableSemaphore }, + stackalloc[] { _imageAvailableSemaphores[semaphoreIndex] }, stackalloc[] { PipelineStageFlags.ColorAttachmentOutputBit }, - stackalloc[] { _renderFinishedSemaphore }); + stackalloc[] { _renderFinishedSemaphores[semaphoreIndex] }); // TODO: Present queue. - var semaphore = _renderFinishedSemaphore; + var semaphore = _renderFinishedSemaphores[semaphoreIndex]; var swapchain = _swapchain; Result result; @@ -593,14 +624,21 @@ namespace Ryujinx.Graphics.Vulkan { unsafe { - _gd.Api.DestroySemaphore(_device, _renderFinishedSemaphore, null); - _gd.Api.DestroySemaphore(_device, _imageAvailableSemaphore, null); - for (int i = 0; i < _swapchainImageViews.Length; i++) { _swapchainImageViews[i].Dispose(); } + for (int i = 0; i < _imageAvailableSemaphores.Length; i++) + { + _gd.Api.DestroySemaphore(_device, _imageAvailableSemaphores[i], null); + } + + for (int i = 0; i < _renderFinishedSemaphores.Length; i++) + { + _gd.Api.DestroySemaphore(_device, _renderFinishedSemaphores[i], null); + } + _gd.SwapchainApi.DestroySwapchain(_device, _swapchain, null); }