From e65effcb05c40247fb717b3c2409abce7ffa10fc Mon Sep 17 00:00:00 2001 From: gdkchan Date: Thu, 23 May 2024 01:05:32 -0300 Subject: [PATCH] Workaround AMD bug on logic op with float framebuffer (#6852) * Workaround AMD bug on logic op with float framebuffer * Format whitespace * Update comment --- src/Ryujinx.Graphics.GAL/Format.cs | 31 +++++++++++++++++++ .../FramebufferParams.cs | 15 +++++++-- src/Ryujinx.Graphics.Vulkan/PipelineBase.cs | 1 + .../PipelineConverter.cs | 4 +++ src/Ryujinx.Graphics.Vulkan/PipelineState.cs | 6 +++- src/Ryujinx.Graphics.Vulkan/PipelineUid.cs | 1 + 6 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/Ryujinx.Graphics.GAL/Format.cs b/src/Ryujinx.Graphics.GAL/Format.cs index bd3b38a9..17c42d2d 100644 --- a/src/Ryujinx.Graphics.GAL/Format.cs +++ b/src/Ryujinx.Graphics.GAL/Format.cs @@ -711,5 +711,36 @@ namespace Ryujinx.Graphics.GAL { return format.IsUint() || format.IsSint(); } + + /// + /// Checks if the texture format is a float or sRGB color format. + /// + /// + /// Does not include normalized, compressed or depth formats. + /// Float and sRGB formats do not participate in logical operations. + /// + /// Texture format + /// True if the format is a float or sRGB color format, false otherwise + public static bool IsFloatOrSrgb(this Format format) + { + switch (format) + { + case Format.R8G8B8A8Srgb: + case Format.B8G8R8A8Srgb: + case Format.R16Float: + case Format.R16G16Float: + case Format.R16G16B16Float: + case Format.R16G16B16A16Float: + case Format.R32Float: + case Format.R32G32Float: + case Format.R32G32B32Float: + case Format.R32G32B32A32Float: + case Format.R11G11B10Float: + case Format.R9G9B9E5Float: + return true; + } + + return false; + } } } diff --git a/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs b/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs index 8079e5ff..ea0fd42e 100644 --- a/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs +++ b/src/Ryujinx.Graphics.Vulkan/FramebufferParams.cs @@ -24,6 +24,7 @@ namespace Ryujinx.Graphics.Vulkan public VkFormat[] AttachmentFormats { get; } public int[] AttachmentIndices { get; } public uint AttachmentIntegerFormatMask { get; } + public bool LogicOpsAllowed { get; } public int AttachmentsCount { get; } public int MaxColorAttachmentIndex => AttachmentIndices.Length > 0 ? AttachmentIndices[^1] : -1; @@ -32,7 +33,9 @@ namespace Ryujinx.Graphics.Vulkan public FramebufferParams(Device device, TextureView view, uint width, uint height) { - bool isDepthStencil = view.Info.Format.IsDepthOrStencil(); + var format = view.Info.Format; + + bool isDepthStencil = format.IsDepthOrStencil(); _device = device; _attachments = new[] { view.GetImageViewForAttachment() }; @@ -56,6 +59,8 @@ namespace Ryujinx.Graphics.Vulkan AttachmentSamples = new[] { (uint)view.Info.Samples }; AttachmentFormats = new[] { view.VkFormat }; AttachmentIndices = isDepthStencil ? Array.Empty() : new[] { 0 }; + AttachmentIntegerFormatMask = format.IsInteger() ? 1u : 0u; + LogicOpsAllowed = !format.IsFloatOrSrgb(); AttachmentsCount = 1; @@ -85,6 +90,7 @@ namespace Ryujinx.Graphics.Vulkan int index = 0; int bindIndex = 0; uint attachmentIntegerFormatMask = 0; + bool allFormatsFloatOrSrgb = colorsCount != 0; foreach (ITexture color in colors) { @@ -101,11 +107,15 @@ namespace Ryujinx.Graphics.Vulkan AttachmentFormats[index] = texture.VkFormat; AttachmentIndices[index] = bindIndex; - if (texture.Info.Format.IsInteger()) + var format = texture.Info.Format; + + if (format.IsInteger()) { attachmentIntegerFormatMask |= 1u << bindIndex; } + allFormatsFloatOrSrgb &= format.IsFloatOrSrgb(); + width = Math.Min(width, (uint)texture.Width); height = Math.Min(height, (uint)texture.Height); layers = Math.Min(layers, (uint)texture.Layers); @@ -120,6 +130,7 @@ namespace Ryujinx.Graphics.Vulkan } AttachmentIntegerFormatMask = attachmentIntegerFormatMask; + LogicOpsAllowed = !allFormatsFloatOrSrgb; if (depthStencil is TextureView dsTexture && dsTexture.Valid) { diff --git a/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs b/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs index 41ab84d9..3776e2f6 100644 --- a/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs +++ b/src/Ryujinx.Graphics.Vulkan/PipelineBase.cs @@ -1498,6 +1498,7 @@ namespace Ryujinx.Graphics.Vulkan var dstAttachmentFormats = _newState.Internal.AttachmentFormats.AsSpan(); FramebufferParams.AttachmentFormats.CopyTo(dstAttachmentFormats); _newState.Internal.AttachmentIntegerFormatMask = FramebufferParams.AttachmentIntegerFormatMask; + _newState.Internal.LogicOpsAllowed = FramebufferParams.LogicOpsAllowed; for (int i = FramebufferParams.AttachmentFormats.Length; i < dstAttachmentFormats.Length; i++) { diff --git a/src/Ryujinx.Graphics.Vulkan/PipelineConverter.cs b/src/Ryujinx.Graphics.Vulkan/PipelineConverter.cs index 95b480a5..41618c73 100644 --- a/src/Ryujinx.Graphics.Vulkan/PipelineConverter.cs +++ b/src/Ryujinx.Graphics.Vulkan/PipelineConverter.cs @@ -302,6 +302,7 @@ namespace Ryujinx.Graphics.Vulkan int attachmentCount = 0; int maxColorAttachmentIndex = -1; uint attachmentIntegerFormatMask = 0; + bool allFormatsFloatOrSrgb = true; for (int i = 0; i < Constants.MaxRenderTargets; i++) { @@ -314,6 +315,8 @@ namespace Ryujinx.Graphics.Vulkan { attachmentIntegerFormatMask |= 1u << i; } + + allFormatsFloatOrSrgb &= state.AttachmentFormats[i].IsFloatOrSrgb(); } } @@ -325,6 +328,7 @@ namespace Ryujinx.Graphics.Vulkan pipeline.ColorBlendAttachmentStateCount = (uint)(maxColorAttachmentIndex + 1); pipeline.VertexAttributeDescriptionsCount = (uint)Math.Min(Constants.MaxVertexAttributes, state.VertexAttribCount); pipeline.Internal.AttachmentIntegerFormatMask = attachmentIntegerFormatMask; + pipeline.Internal.LogicOpsAllowed = attachmentCount == 0 || !allFormatsFloatOrSrgb; return pipeline; } diff --git a/src/Ryujinx.Graphics.Vulkan/PipelineState.cs b/src/Ryujinx.Graphics.Vulkan/PipelineState.cs index 49c12b37..21160858 100644 --- a/src/Ryujinx.Graphics.Vulkan/PipelineState.cs +++ b/src/Ryujinx.Graphics.Vulkan/PipelineState.cs @@ -560,10 +560,14 @@ namespace Ryujinx.Graphics.Vulkan } } + // AMD has a bug where it enables logical operations even for float formats, + // so we need to force disable them here. + bool logicOpEnable = LogicOpEnable && (gd.Vendor != Vendor.Amd || Internal.LogicOpsAllowed); + var colorBlendState = new PipelineColorBlendStateCreateInfo { SType = StructureType.PipelineColorBlendStateCreateInfo, - LogicOpEnable = LogicOpEnable, + LogicOpEnable = logicOpEnable, LogicOp = LogicOp, AttachmentCount = ColorBlendAttachmentStateCount, PAttachments = pColorBlendAttachmentState, diff --git a/src/Ryujinx.Graphics.Vulkan/PipelineUid.cs b/src/Ryujinx.Graphics.Vulkan/PipelineUid.cs index 3448d974..238f06e2 100644 --- a/src/Ryujinx.Graphics.Vulkan/PipelineUid.cs +++ b/src/Ryujinx.Graphics.Vulkan/PipelineUid.cs @@ -34,6 +34,7 @@ namespace Ryujinx.Graphics.Vulkan public Array8 ColorBlendAttachmentState; public Array9 AttachmentFormats; public uint AttachmentIntegerFormatMask; + public bool LogicOpsAllowed; public readonly override bool Equals(object obj) {