From 991784868f278b62f7e847321f0cfd7309fe2f79 Mon Sep 17 00:00:00 2001 From: gdkchan Date: Tue, 28 Jul 2020 19:01:11 -0300 Subject: [PATCH] Fix shader regression on Intel iGPUs by reverting layout changes (#1425) --- Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs | 32 ++++++++------ .../CodeGen/Glsl/Declarations.cs | 40 +++++++++++++----- .../CodeGen/Glsl/GlslGenerator.cs | 17 +++++--- .../CodeGen/Glsl/Instructions/InstGen.cs | 2 +- .../Glsl/Instructions/InstGenMemory.cs | 2 +- .../CodeGen/Glsl/OperandManager.cs | 42 ++++++++++++------- .../Translation/TranslationFlags.cs | 3 +- 7 files changed, 93 insertions(+), 45 deletions(-) diff --git a/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs b/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs index ab777162..853351db 100644 --- a/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs +++ b/Ryujinx.Graphics.Gpu/Shader/ShaderCache.cs @@ -125,19 +125,28 @@ namespace Ryujinx.Graphics.Gpu.Shader ShaderCodeHolder[] shaders = new ShaderCodeHolder[Constants.ShaderStages]; + var tfd = GetTransformFeedbackDescriptors(state); + + TranslationFlags flags = DefaultFlags; + + if (tfd != null) + { + flags |= TranslationFlags.Feedback; + } + if (addresses.VertexA != 0) { - shaders[0] = TranslateGraphicsShader(state, ShaderStage.Vertex, addresses.Vertex, addresses.VertexA); + shaders[0] = TranslateGraphicsShader(state, flags, ShaderStage.Vertex, addresses.Vertex, addresses.VertexA); } else { - shaders[0] = TranslateGraphicsShader(state, ShaderStage.Vertex, addresses.Vertex); + shaders[0] = TranslateGraphicsShader(state, flags, ShaderStage.Vertex, addresses.Vertex); } - shaders[1] = TranslateGraphicsShader(state, ShaderStage.TessellationControl, addresses.TessControl); - shaders[2] = TranslateGraphicsShader(state, ShaderStage.TessellationEvaluation, addresses.TessEvaluation); - shaders[3] = TranslateGraphicsShader(state, ShaderStage.Geometry, addresses.Geometry); - shaders[4] = TranslateGraphicsShader(state, ShaderStage.Fragment, addresses.Fragment); + shaders[1] = TranslateGraphicsShader(state, flags, ShaderStage.TessellationControl, addresses.TessControl); + shaders[2] = TranslateGraphicsShader(state, flags, ShaderStage.TessellationEvaluation, addresses.TessEvaluation); + shaders[3] = TranslateGraphicsShader(state, flags, ShaderStage.Geometry, addresses.Geometry); + shaders[4] = TranslateGraphicsShader(state, flags, ShaderStage.Fragment, addresses.Fragment); List hostShaders = new List(); @@ -150,8 +159,6 @@ namespace Ryujinx.Graphics.Gpu.Shader continue; } - var tfd = GetTransformFeedbackDescriptors(state); - IShader hostShader = _context.Renderer.CompileShader(program); shaders[stage].HostShader = hostShader; @@ -159,7 +166,7 @@ namespace Ryujinx.Graphics.Gpu.Shader hostShaders.Add(hostShader); } - IProgram hostProgram = _context.Renderer.CreateProgram(hostShaders.ToArray(), GetTransformFeedbackDescriptors(state)); + IProgram hostProgram = _context.Renderer.CreateProgram(hostShaders.ToArray(), tfd); ShaderBundle gpShaders = new ShaderBundle(hostProgram, shaders); @@ -327,11 +334,12 @@ namespace Ryujinx.Graphics.Gpu.Shader /// This will combine the "Vertex A" and "Vertex B" shader stages, if specified, into one shader. /// /// Current GPU state + /// Flags that controls shader translation /// Shader stage /// GPU virtual address of the shader code /// Optional GPU virtual address of the "Vertex A" shader code /// Compiled graphics shader code - private ShaderCodeHolder TranslateGraphicsShader(GpuState state, ShaderStage stage, ulong gpuVa, ulong gpuVaA = 0) + private ShaderCodeHolder TranslateGraphicsShader(GpuState state, TranslationFlags flags, ShaderStage stage, ulong gpuVa, ulong gpuVaA = 0) { if (gpuVa == 0) { @@ -342,7 +350,7 @@ namespace Ryujinx.Graphics.Gpu.Shader if (gpuVaA != 0) { - ShaderProgram program = Translator.Translate(gpuVaA, gpuVa, gpuAccessor, DefaultFlags); + ShaderProgram program = Translator.Translate(gpuVaA, gpuVa, gpuAccessor, flags); byte[] codeA = _context.MemoryManager.GetSpan(gpuVaA, program.SizeA).ToArray(); byte[] codeB = _context.MemoryManager.GetSpan(gpuVa, program.Size).ToArray(); @@ -362,7 +370,7 @@ namespace Ryujinx.Graphics.Gpu.Shader } else { - ShaderProgram program = Translator.Translate(gpuVa, gpuAccessor, DefaultFlags); + ShaderProgram program = Translator.Translate(gpuVa, gpuAccessor, flags); byte[] code = _context.MemoryManager.GetSpan(gpuVa, program.Size).ToArray(); diff --git a/Ryujinx.Graphics.Shader/CodeGen/Glsl/Declarations.cs b/Ryujinx.Graphics.Shader/CodeGen/Glsl/Declarations.cs index 40e277e0..a7b67a60 100644 --- a/Ryujinx.Graphics.Shader/CodeGen/Glsl/Declarations.cs +++ b/Ryujinx.Graphics.Shader/CodeGen/Glsl/Declarations.cs @@ -430,11 +430,20 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl }; } - for (int c = 0; c < 4; c++) - { - char swzMask = "xyzw"[c]; + string name = $"{DefaultNames.IAttributePrefix}{attr}"; - context.AppendLine($"layout (location = {attr}, component = {c}) {iq}in float {DefaultNames.IAttributePrefix}{attr}_{swzMask}{suffix};"); + if ((context.Config.Flags & TranslationFlags.Feedback) != 0) + { + for (int c = 0; c < 4; c++) + { + char swzMask = "xyzw"[c]; + + context.AppendLine($"layout (location = {attr}, component = {c}) {iq}in float {name}_{swzMask}{suffix};"); + } + } + else + { + context.AppendLine($"layout (location = {attr}) {iq}in vec4 {name}{suffix};"); } } } @@ -463,23 +472,32 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl { for (int attr = 0; attr < MaxAttributes; attr++) { - for (int c = 0; c < 4; c++) - { - char swzMask = "xyzw"[c]; - - context.AppendLine($"layout (location = {attr}, component = {c}) out float {DefaultNames.OAttributePrefix}{attr}_{swzMask};"); - } + DeclareOutputAttribute(context, attr); } foreach (int attr in info.OAttributes.OrderBy(x => x).Where(x => x >= MaxAttributes)) + { + DeclareOutputAttribute(context, attr); + } + } + + private static void DeclareOutputAttribute(CodeGenContext context, int attr) + { + string name = $"{DefaultNames.OAttributePrefix}{attr}"; + + if ((context.Config.Flags & TranslationFlags.Feedback) != 0) { for (int c = 0; c < 4; c++) { char swzMask = "xyzw"[c]; - context.AppendLine($"layout (location = {attr}, component = {c}) out float {DefaultNames.OAttributePrefix}{attr}_{swzMask};"); + context.AppendLine($"layout (location = {attr}, component = {c}) out float {name}_{swzMask};"); } } + else + { + context.AppendLine($"layout (location = {attr}) out vec4 {name};"); + } } private static void AppendHelperFunction(CodeGenContext context, string filename) diff --git a/Ryujinx.Graphics.Shader/CodeGen/Glsl/GlslGenerator.cs b/Ryujinx.Graphics.Shader/CodeGen/Glsl/GlslGenerator.cs index 2105560a..673fe6a3 100644 --- a/Ryujinx.Graphics.Shader/CodeGen/Glsl/GlslGenerator.cs +++ b/Ryujinx.Graphics.Shader/CodeGen/Glsl/GlslGenerator.cs @@ -56,10 +56,17 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl continue; } - context.AppendLine($"{DefaultNames.OAttributePrefix}{attr}_x = 0;"); - context.AppendLine($"{DefaultNames.OAttributePrefix}{attr}_y = 0;"); - context.AppendLine($"{DefaultNames.OAttributePrefix}{attr}_z = 0;"); - context.AppendLine($"{DefaultNames.OAttributePrefix}{attr}_w = 0;"); + if ((context.Config.Flags & TranslationFlags.Feedback) != 0) + { + context.AppendLine($"{DefaultNames.OAttributePrefix}{attr}_x = 0;"); + context.AppendLine($"{DefaultNames.OAttributePrefix}{attr}_y = 0;"); + context.AppendLine($"{DefaultNames.OAttributePrefix}{attr}_z = 0;"); + context.AppendLine($"{DefaultNames.OAttributePrefix}{attr}_w = 0;"); + } + else + { + context.AppendLine($"{DefaultNames.OAttributePrefix}{attr} = vec4(0);"); + } } } @@ -123,7 +130,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl if (assignment.Destination is AstOperand operand && operand.Type == OperandType.Attribute) { - dest = OperandManager.GetOutAttributeName(operand, context.Config.Stage); + dest = OperandManager.GetOutAttributeName(operand, context.Config); } else { diff --git a/Ryujinx.Graphics.Shader/CodeGen/Glsl/Instructions/InstGen.cs b/Ryujinx.Graphics.Shader/CodeGen/Glsl/Instructions/InstGen.cs index f1537c3d..551fb229 100644 --- a/Ryujinx.Graphics.Shader/CodeGen/Glsl/Instructions/InstGen.cs +++ b/Ryujinx.Graphics.Shader/CodeGen/Glsl/Instructions/InstGen.cs @@ -19,7 +19,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl.Instructions } else if (node is AstOperand operand) { - return context.OperandManager.GetExpression(operand, context.Config.Stage); + return context.OperandManager.GetExpression(operand, context.Config); } throw new ArgumentException($"Invalid node type \"{node?.GetType().Name ?? "null"}\"."); diff --git a/Ryujinx.Graphics.Shader/CodeGen/Glsl/Instructions/InstGenMemory.cs b/Ryujinx.Graphics.Shader/CodeGen/Glsl/Instructions/InstGenMemory.cs index b951798d..8866cd25 100644 --- a/Ryujinx.Graphics.Shader/CodeGen/Glsl/Instructions/InstGenMemory.cs +++ b/Ryujinx.Graphics.Shader/CodeGen/Glsl/Instructions/InstGenMemory.cs @@ -113,7 +113,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl.Instructions string indexExpr = GetSoureExpr(context, src2, GetSrcVarType(operation.Inst, 1)); - return OperandManager.GetAttributeName(attr, context.Config.Stage, isOutAttr: false, indexExpr); + return OperandManager.GetAttributeName(attr, context.Config, isOutAttr: false, indexExpr); } public static string LoadConstant(CodeGenContext context, AstOperation operation) diff --git a/Ryujinx.Graphics.Shader/CodeGen/Glsl/OperandManager.cs b/Ryujinx.Graphics.Shader/CodeGen/Glsl/OperandManager.cs index 4ae9a00a..e04ce649 100644 --- a/Ryujinx.Graphics.Shader/CodeGen/Glsl/OperandManager.cs +++ b/Ryujinx.Graphics.Shader/CodeGen/Glsl/OperandManager.cs @@ -92,18 +92,18 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl return name; } - public string GetExpression(AstOperand operand, ShaderStage stage) + public string GetExpression(AstOperand operand, ShaderConfig config) { switch (operand.Type) { case OperandType.Attribute: - return GetAttributeName(operand, stage); + return GetAttributeName(operand, config); case OperandType.Constant: return NumberFormatter.FormatInt(operand.Value); case OperandType.ConstantBuffer: - return GetConstantBufferName(operand.CbufSlot, operand.CbufOffset, stage); + return GetConstantBufferName(operand.CbufSlot, operand.CbufOffset, config.Stage); case OperandType.LocalVariable: return _locals[operand]; @@ -148,12 +148,12 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl return GetVec4Indexed(ubName + index0, offsetExpr + " & 3"); } - public static string GetOutAttributeName(AstOperand attr, ShaderStage stage) + public static string GetOutAttributeName(AstOperand attr, ShaderConfig config) { - return GetAttributeName(attr, stage, isOutAttr: true); + return GetAttributeName(attr, config, isOutAttr: true); } - public static string GetAttributeName(AstOperand attr, ShaderStage stage, bool isOutAttr = false, string indexExpr = "0") + public static string GetAttributeName(AstOperand attr, ShaderConfig config, bool isOutAttr = false, string indexExpr = "0") { int value = attr.Value; @@ -167,14 +167,28 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl ? DefaultNames.OAttributePrefix : DefaultNames.IAttributePrefix; - string name = $"{prefix}{(value >> 4)}_{swzMask}"; - - if (stage == ShaderStage.Geometry && !isOutAttr) + if ((config.Flags & TranslationFlags.Feedback) != 0) { - name += $"[{indexExpr}]"; - } + string name = $"{prefix}{(value >> 4)}_{swzMask}"; - return name; + if (config.Stage == ShaderStage.Geometry && !isOutAttr) + { + name += $"[{indexExpr}]"; + } + + return name; + } + else + { + string name = $"{prefix}{(value >> 4)}"; + + if (config.Stage == ShaderStage.Geometry && !isOutAttr) + { + name += $"[{indexExpr}]"; + } + + return name + '.' + swzMask; + } } else { @@ -187,7 +201,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl else if (_builtInAttributes.TryGetValue(value & ~3, out BuiltInAttribute builtInAttr)) { // TODO: There must be a better way to handle this... - if (stage == ShaderStage.Fragment) + if (config.Stage == ShaderStage.Fragment) { switch (value & ~3) { @@ -200,7 +214,7 @@ namespace Ryujinx.Graphics.Shader.CodeGen.Glsl string name = builtInAttr.Name; - if (stage == ShaderStage.Geometry && !isOutAttr) + if (config.Stage == ShaderStage.Geometry && !isOutAttr) { name = $"gl_in[{indexExpr}].{name}"; } diff --git a/Ryujinx.Graphics.Shader/Translation/TranslationFlags.cs b/Ryujinx.Graphics.Shader/Translation/TranslationFlags.cs index 1874dec3..9af95389 100644 --- a/Ryujinx.Graphics.Shader/Translation/TranslationFlags.cs +++ b/Ryujinx.Graphics.Shader/Translation/TranslationFlags.cs @@ -9,6 +9,7 @@ namespace Ryujinx.Graphics.Shader.Translation VertexA = 1 << 0, Compute = 1 << 1, - DebugMode = 1 << 2 + Feedback = 1 << 2, + DebugMode = 1 << 3 } } \ No newline at end of file