From b34c0a47b4d5d9cc4b3a6b51fbc76fe2b493b07d Mon Sep 17 00:00:00 2001 From: gdkchan Date: Thu, 20 May 2021 15:12:15 -0300 Subject: [PATCH] Fix constant buffer array size when indexing is used and other buffer descriptor and resolution scale regressions (#2298) * Fix constant buffer array size when indexing is used * Change default QueryConstantBufferUse value * Fix more regressions * Ensure proper order --- Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs | 4 +- .../CodeGen/Glsl/Declarations.cs | 8 +-- Ryujinx.Graphics.Shader/IGpuAccessor.cs | 2 +- .../StructuredIr/StructuredProgram.cs | 2 +- .../Translation/ShaderConfig.cs | 61 ++++++++++++++----- 5 files changed, 54 insertions(+), 23 deletions(-) diff --git a/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs b/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs index 86709277..158b791a 100644 --- a/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs +++ b/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs @@ -36,7 +36,7 @@ namespace Ryujinx.Graphics.Gpu.Shader /// /// Version of the codegen (to be changed when codegen or guest format change). /// - private const ulong ShaderCodeGenVersion = 2290; + private const ulong ShaderCodeGenVersion = 2298; // Progress reporting helpers private volatile int _shaderCount; @@ -415,7 +415,7 @@ namespace Ryujinx.Graphics.Gpu.Shader if (activeTasks.Count == maxTaskCount) { // Wait for a task to be done, or for 1ms. - // Host shader compilation cannot signal when it is done, + // Host shader compilation cannot signal when it is done, // so the 1ms timeout is required to poll status. taskDoneEvent.WaitOne(1); diff --git a/Ryujinx.Graphics.Shader/CodeGen/Glsl/Declarations.cs b/Ryujinx.Graphics.Shader/CodeGen/Glsl/Declarations.cs index 6e67b682..cb17f18d 100644 --- a/Ryujinx.Graphics.Shader/CodeGen/Glsl/Declarations.cs +++ b/Ryujinx.Graphics.Shader/CodeGen/Glsl/Declarations.cs @@ -262,10 +262,10 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl string blockName = $"{ubName}_{DefaultNames.BlockSuffix}"; - context.AppendLine($"layout (binding = {descriptors[0].Binding}, std140) uniform {blockName}"); + context.AppendLine($"layout (binding = {context.Config.FirstConstantBufferBinding}, std140) uniform {blockName}"); context.EnterScope(); context.AppendLine("vec4 " + DefaultNames.DataName + ubSize + ";"); - context.LeaveScope($" {ubName}[{NumberFormatter.FormatInt(descriptors.Length)}];"); + context.LeaveScope($" {ubName}[{NumberFormatter.FormatInt(descriptors.Max(x => x.Slot) + 1)}];"); } else { @@ -291,10 +291,10 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl string blockName = $"{sbName}_{DefaultNames.BlockSuffix}"; - context.AppendLine($"layout (binding = {descriptors[0].Binding}, std430) buffer {blockName}"); + context.AppendLine($"layout (binding = {context.Config.FirstStorageBufferBinding}, std430) buffer {blockName}"); context.EnterScope(); context.AppendLine("uint " + DefaultNames.DataName + "[];"); - context.LeaveScope($" {sbName}[{NumberFormatter.FormatInt(descriptors.Length)}];"); + context.LeaveScope($" {sbName}[{NumberFormatter.FormatInt(descriptors.Max(x => x.Slot) + 1)}];"); } private static void DeclareSamplers(CodeGenContext context, TextureDescriptor[] descriptors) diff --git a/Ryujinx.Graphics.Shader/IGpuAccessor.cs b/Ryujinx.Graphics.Shader/IGpuAccessor.cs index 1e05cdcd..6b584e53 100644 --- a/Ryujinx.Graphics.Shader/IGpuAccessor.cs +++ b/Ryujinx.Graphics.Shader/IGpuAccessor.cs @@ -41,7 +41,7 @@ uint QueryConstantBufferUse() { - return 0xffff; + return 0; } bool QueryIsTextureBuffer(int handle, int cbufSlot = -1) diff --git a/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgram.cs b/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgram.cs index 25e5edc9..4acfa80a 100644 --- a/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgram.cs +++ b/Ryujinx.Graphics.Shader/StructuredIr/StructuredProgram.cs @@ -293,7 +293,7 @@ namespace Ryujinx.Graphics.Shader.StructuredIr Instruction.Branch or Instruction.BranchIfFalse or Instruction.BranchIfTrue => true, - _ => false, + _ => false }; } diff --git a/Ryujinx.Graphics.Shader/Translation/ShaderConfig.cs b/Ryujinx.Graphics.Shader/Translation/ShaderConfig.cs index 3230f4e6..8004e313 100644 --- a/Ryujinx.Graphics.Shader/Translation/ShaderConfig.cs +++ b/Ryujinx.Graphics.Shader/Translation/ShaderConfig.cs @@ -91,6 +91,9 @@ namespace Ryujinx.Graphics.Shader.Translation private TextureDescriptor[] _cachedTextureDescriptors; private TextureDescriptor[] _cachedImageDescriptors; + public int FirstConstantBufferBinding { get; private set; } + public int FirstStorageBufferBinding { get; private set; } + public ShaderConfig(IGpuAccessor gpuAccessor, TranslationFlags flags, TranslationCounts counts) { Stage = ShaderStage.Compute; @@ -215,7 +218,7 @@ namespace Ryujinx.Graphics.Shader.Translation } } - private static void SetUsedTextureOrImage( + private void SetUsedTextureOrImage( Dictionary dict, int cbufSlot, int handle, @@ -235,7 +238,10 @@ namespace Ryujinx.Graphics.Shader.Translation { usageFlags |= TextureUsageFlags.NeedsScaleValue; - var canScale = (dimensions == 2 && !isArray) || (dimensions == 3 && isArray); + var canScale = (Stage == ShaderStage.Fragment || Stage == ShaderStage.Compute) && !isIndexed && !write && + ((dimensions == 2 && !isArray) || + (dimensions == 3 && isArray)); + if (!canScale) { // Resolution scaling cannot be applied to this texture right now. @@ -293,34 +299,59 @@ namespace Ryujinx.Graphics.Shader.Translation if (UsedFeatures.HasFlag(FeatureFlags.CbIndexing)) { - usedMask = FillMask(usedMask); + usedMask |= (int)GpuAccessor.QueryConstantBufferUse(); } - return _cachedConstantBufferDescriptors = GetBufferDescriptors(usedMask, 0, _counts.IncrementUniformBuffersCount); + FirstConstantBufferBinding = _counts.UniformBuffersCount; + + return _cachedConstantBufferDescriptors = GetBufferDescriptors( + usedMask, + 0, + UsedFeatures.HasFlag(FeatureFlags.CbIndexing), + _counts.IncrementUniformBuffersCount); } public BufferDescriptor[] GetStorageBufferDescriptors() { - return _cachedStorageBufferDescriptors ??= GetBufferDescriptors(FillMask(_usedStorageBuffers), _usedStorageBuffersWrite, _counts.IncrementStorageBuffersCount); + if (_cachedStorageBufferDescriptors != null) + { + return _cachedStorageBufferDescriptors; + } + + FirstStorageBufferBinding = _counts.StorageBuffersCount; + + return _cachedStorageBufferDescriptors = GetBufferDescriptors( + _usedStorageBuffers, + _usedStorageBuffersWrite, + true, + _counts.IncrementStorageBuffersCount); } - private static int FillMask(int mask) - { - // When the storage or uniform buffers are used as array, we must allocate a binding - // even for the "gaps" that are not used on the shader. - // For this reason, fill up the gaps so that all slots up to the highest one are - // marked as "used". - return mask != 0 ? (int)(uint.MaxValue >> BitOperations.LeadingZeroCount((uint)mask)) : 0; - } - - private static BufferDescriptor[] GetBufferDescriptors(int usedMask, int writtenMask, Func getBindingCallback) + private static BufferDescriptor[] GetBufferDescriptors( + int usedMask, + int writtenMask, + bool isArray, + Func getBindingCallback) { var descriptors = new BufferDescriptor[BitOperations.PopCount((uint)usedMask)]; + int lastSlot = -1; + for (int i = 0; i < descriptors.Length; i++) { int slot = BitOperations.TrailingZeroCount(usedMask); + if (isArray) + { + // The next array entries also consumes bindings, even if they are unused. + for (int j = lastSlot + 1; j < slot; j++) + { + getBindingCallback(); + } + } + + lastSlot = slot; + descriptors[i] = new BufferDescriptor(getBindingCallback(), slot); if ((writtenMask & (1 << slot)) != 0)