From 9ace6b9285c60c70aba153110bef3eb5372badb7 Mon Sep 17 00:00:00 2001
From: gdkchan <gab.dark.100@gmail.com>
Date: Tue, 23 Oct 2018 17:59:52 -0300
Subject: [PATCH] Fix for render target and a shader compilation issue (#471)

* Fix render target using possibly deleted or wrong handles

* Fix basic blocks with only a KIL instruction on the shader translator

* Formatting fix
---
 .../Gal/OpenGL/OGLRenderTarget.cs             | 106 +++++++++++-------
 Ryujinx.Graphics/Gal/OpenGL/OGLTexture.cs     |   6 +-
 Ryujinx.Graphics/Gal/Shader/GlslDecompiler.cs |  98 +++++++++-------
 .../Gal/Shader/ShaderDecodeAlu.cs             |   2 +-
 4 files changed, 128 insertions(+), 84 deletions(-)

diff --git a/Ryujinx.Graphics/Gal/OpenGL/OGLRenderTarget.cs b/Ryujinx.Graphics/Gal/OpenGL/OGLRenderTarget.cs
index e0c4854ed2..17fddfface 100644
--- a/Ryujinx.Graphics/Gal/OpenGL/OGLRenderTarget.cs
+++ b/Ryujinx.Graphics/Gal/OpenGL/OGLRenderTarget.cs
@@ -6,6 +6,11 @@ namespace Ryujinx.Graphics.Gal.OpenGL
 {
     class OGLRenderTarget : IGalRenderTarget
     {
+        private const int NativeWidth  = 1280;
+        private const int NativeHeight = 720;
+
+        private const int RenderTargetsCount = GalPipelineState.RenderTargetsCount;
+
         private struct Rect
         {
             public int X      { get; private set; }
@@ -24,11 +29,13 @@ namespace Ryujinx.Graphics.Gal.OpenGL
 
         private class FrameBufferAttachments
         {
-            public long[] Colors;
-            public long Zeta;
+            public int MapCount { get; set; }
 
-            public int MapCount;
-            public DrawBuffersEnum[] Map;
+            public DrawBuffersEnum[] Map { get; private set; }
+
+            public long[] Colors { get; private set; }
+
+            public long Zeta { get; set; }
 
             public FrameBufferAttachments()
             {
@@ -37,37 +44,31 @@ namespace Ryujinx.Graphics.Gal.OpenGL
                 Map = new DrawBuffersEnum[RenderTargetsCount];
             }
 
-            public void SetAndClear(FrameBufferAttachments Source)
+            public void Update(FrameBufferAttachments Source)
             {
-                Zeta     = Source.Zeta;
-                MapCount = Source.MapCount;
-
-                Source.Zeta     = 0;
-                Source.MapCount = 0;
-
-                for (int i = 0; i < RenderTargetsCount; i++)
+                for (int Index = 0; Index < RenderTargetsCount; Index++)
                 {
-                    Colors[i] = Source.Colors[i];
-                    Map[i]    = Source.Map[i];
+                    Map[Index] = Source.Map[Index];
 
-                    Source.Colors[i] = 0;
-                    Source.Map[i]    = 0;
+                    Colors[Index] = Source.Colors[Index];
                 }
+
+                MapCount = Source.MapCount;
+                Zeta     = Source.Zeta;
             }
         }
 
-        private const int NativeWidth  = 1280;
-        private const int NativeHeight = 720;
-
-        private const int RenderTargetsCount = GalPipelineState.RenderTargetsCount;
+        private int[] ColorHandles;
+        private int   ZetaHandle;
 
         private OGLTexture Texture;
 
         private ImageHandler ReadTex;
 
-        private float[] Viewports;
         private Rect Window;
 
+        private float[] Viewports;
+
         private bool FlipX;
         private bool FlipY;
 
@@ -95,9 +96,31 @@ namespace Ryujinx.Graphics.Gal.OpenGL
 
             OldAttachments = new FrameBufferAttachments();
 
+            ColorHandles = new int[RenderTargetsCount];
+
             Viewports = new float[RenderTargetsCount * 4];
 
             this.Texture = Texture;
+
+            Texture.TextureDeleted += TextureDeletionHandler;
+        }
+
+        private void TextureDeletionHandler(object Sender, int Handle)
+        {
+            //Texture was deleted, the handle is no longer valid, so
+            //reset all uses of this handle on a render target.
+            for (int Attachment = 0; Attachment < RenderTargetsCount; Attachment++)
+            {
+                if (ColorHandles[Attachment] == Handle)
+                {
+                    ColorHandles[Attachment] = 0;
+                }
+            }
+
+            if (ZetaHandle == Handle)
+            {
+                ZetaHandle = 0;
+            }
         }
 
         public void Bind()
@@ -115,11 +138,6 @@ namespace Ryujinx.Graphics.Gal.OpenGL
             {
                 long Key = Attachments.Colors[Attachment];
 
-                if (Key == OldAttachments.Colors[Attachment])
-                {
-                    continue;
-                }
-
                 int Handle = 0;
 
                 if (Key != 0 && Texture.TryGetImageHandler(Key, out CachedImage))
@@ -127,16 +145,23 @@ namespace Ryujinx.Graphics.Gal.OpenGL
                     Handle = CachedImage.Handle;
                 }
 
+                if (Handle == ColorHandles[Attachment])
+                {
+                    continue;
+                }
+
                 GL.FramebufferTexture(
                     FramebufferTarget.DrawFramebuffer,
                     FramebufferAttachment.ColorAttachment0 + Attachment,
                     Handle,
                     0);
+
+                ColorHandles[Attachment] = Handle;
             }
 
-            if (Attachments.Zeta != OldAttachments.Zeta)
+            if (Attachments.Zeta != 0 && Texture.TryGetImageHandler(Attachments.Zeta, out CachedImage))
             {
-                if (Attachments.Zeta != 0 && Texture.TryGetImageHandler(Attachments.Zeta, out CachedImage))
+                if (CachedImage.Handle != ZetaHandle)
                 {
                     if (CachedImage.HasDepth && CachedImage.HasStencil)
                     {
@@ -162,17 +187,21 @@ namespace Ryujinx.Graphics.Gal.OpenGL
                     }
                     else
                     {
-                        throw new NotImplementedException();
+                        throw new InvalidOperationException("Invalid image format \"" + CachedImage.Format + "\" used as Zeta!");
                     }
+
+                    ZetaHandle = CachedImage.Handle;
                 }
-                else
-                {
-                    GL.FramebufferTexture(
-                        FramebufferTarget.DrawFramebuffer,
-                        FramebufferAttachment.DepthStencilAttachment,
-                        0,
-                        0);
-                }
+            }
+            else if (ZetaHandle != 0)
+            {
+                GL.FramebufferTexture(
+                    FramebufferTarget.DrawFramebuffer,
+                    FramebufferAttachment.DepthStencilAttachment,
+                    0,
+                    0);
+
+                ZetaHandle = 0;
             }
 
             if (OGLExtension.ViewportArray)
@@ -201,7 +230,7 @@ namespace Ryujinx.Graphics.Gal.OpenGL
                 GL.DrawBuffer(DrawBufferMode.None);
             }
 
-            OldAttachments.SetAndClear(Attachments);
+            OldAttachments.Update(Attachments);
         }
 
         public void BindColor(long Key, int Attachment)
@@ -431,7 +460,6 @@ namespace Ryujinx.Graphics.Gal.OpenGL
             GL.GetTexImage(TextureTarget.Texture2D, 0, Format, Type, IntPtr.Zero);
 
             GL.BindBuffer(BufferTarget.PixelPackBuffer, 0);
-
             GL.BindBuffer(BufferTarget.PixelUnpackBuffer, CopyPBO);
 
             Texture.Create(Key, ImageUtils.GetSize(NewImage), NewImage);
diff --git a/Ryujinx.Graphics/Gal/OpenGL/OGLTexture.cs b/Ryujinx.Graphics/Gal/OpenGL/OGLTexture.cs
index 6c60852801..274b94ea87 100644
--- a/Ryujinx.Graphics/Gal/OpenGL/OGLTexture.cs
+++ b/Ryujinx.Graphics/Gal/OpenGL/OGLTexture.cs
@@ -8,6 +8,8 @@ namespace Ryujinx.Graphics.Gal.OpenGL
     {
         private OGLCachedResource<ImageHandler> TextureCache;
 
+        public EventHandler<int> TextureDeleted { get; set; }
+
         public OGLTexture()
         {
             TextureCache = new OGLCachedResource<ImageHandler>(DeleteTexture);
@@ -23,8 +25,10 @@ namespace Ryujinx.Graphics.Gal.OpenGL
             TextureCache.Unlock();
         }
 
-        private static void DeleteTexture(ImageHandler CachedImage)
+        private void DeleteTexture(ImageHandler CachedImage)
         {
+            TextureDeleted?.Invoke(this, CachedImage.Handle);
+
             GL.DeleteTexture(CachedImage.Handle);
         }
 
diff --git a/Ryujinx.Graphics/Gal/Shader/GlslDecompiler.cs b/Ryujinx.Graphics/Gal/Shader/GlslDecompiler.cs
index d0f9223b91..5b62ac3a6e 100644
--- a/Ryujinx.Graphics/Gal/Shader/GlslDecompiler.cs
+++ b/Ryujinx.Graphics/Gal/Shader/GlslDecompiler.cs
@@ -252,10 +252,9 @@ namespace Ryujinx.Graphics.Gal.Shader
                 SB.AppendLine(IdentationStr + "int " + GlslDecl.InstanceUniformName + ";");
 
                 SB.AppendLine("};");
+                SB.AppendLine();
             }
 
-            SB.AppendLine();
-
             foreach (ShaderDeclInfo DeclInfo in Decl.Uniforms.Values.OrderBy(DeclKeySelector))
             {
                 SB.AppendLine($"layout (std140) uniform {DeclInfo.Name} {{");
@@ -312,17 +311,27 @@ namespace Ryujinx.Graphics.Gal.Shader
         {
             if (Decl.ShaderType == GalShaderType.Fragment)
             {
+                int Count = 0;
+
                 for (int Attachment = 0; Attachment < 8; Attachment++)
                 {
                     if (Header.OmapTargets[Attachment].Enabled)
                     {
                         SB.AppendLine("layout (location = " + Attachment + ") out vec4 " + GlslDecl.FragmentOutputName + Attachment + ";");
+
+                        Count++;
                     }
                 }
+
+                if (Count > 0)
+                {
+                    SB.AppendLine();
+                }
             }
             else
             {
                 SB.AppendLine("layout (location = " + GlslDecl.PositionOutAttrLocation + ") out vec4 " + GlslDecl.PositionOutAttrName + ";");
+                SB.AppendLine();
             }
 
             PrintDeclAttributes(Decl.OutAttributes.Values, "out");
@@ -558,6 +567,49 @@ namespace Ryujinx.Graphics.Gal.Shader
             }
         }
 
+        private void PrintNodes(ShaderIrBlock Block, ShaderIrNode[] Nodes)
+        {
+            foreach (ShaderIrNode Node in Nodes)
+            {
+                PrintNode(Block, Node, IdentationStr);
+            }
+
+            if (Nodes.Length == 0)
+            {
+                SB.AppendLine(IdentationStr + "return 0u;");
+
+                return;
+            }
+
+            ShaderIrNode Last = Nodes[Nodes.Length - 1];
+
+            bool UnconditionalFlowChange = false;
+
+            if (Last is ShaderIrOp Op)
+            {
+                switch (Op.Inst)
+                {
+                    case ShaderIrInst.Bra:
+                    case ShaderIrInst.Exit:
+                    case ShaderIrInst.Sync:
+                        UnconditionalFlowChange = true;
+                        break;
+                }
+            }
+
+            if (!UnconditionalFlowChange)
+            {
+                if (Block.Next != null)
+                {
+                    SB.AppendLine(IdentationStr + "return " + GetBlockPosition(Block.Next) + ";");
+                }
+                else
+                {
+                    SB.AppendLine(IdentationStr + "return 0u;");
+                }
+            }
+        }
+
         private void PrintNode(ShaderIrBlock Block, ShaderIrNode Node, string Identation)
         {
             if (Node is ShaderIrCond Cond)
@@ -571,14 +623,7 @@ namespace Ryujinx.Graphics.Gal.Shader
 
                 SB.AppendLine(Identation + "if (" + IfExpr + ") {");
 
-                if (Cond.Child is ShaderIrOp Op && Op.Inst == ShaderIrInst.Bra)
-                {
-                    SB.AppendLine(Identation + IdentationStr + "return " + GetBlockPosition(Block.Branch) + ";");
-                }
-                else
-                {
-                    PrintNode(Block, Cond.Child, Identation + IdentationStr);
-                }
+                PrintNode(Block, Cond.Child, Identation + IdentationStr);
 
                 SB.AppendLine(Identation + "}");
             }
@@ -655,39 +700,6 @@ namespace Ryujinx.Graphics.Gal.Shader
             }
         }
 
-        private void PrintNodes(ShaderIrBlock Block, ShaderIrNode[] Nodes)
-        {
-            foreach (ShaderIrNode Node in Nodes)
-            {
-                PrintNode(Block, Node, IdentationStr);
-            }
-
-            if (Nodes.Length > 0)
-            {
-                ShaderIrNode Last = Nodes[Nodes.Length - 1];
-
-                bool UnconditionalFlowChange = false;
-
-                if (Last is ShaderIrOp Op)
-                {
-                    switch (Op.Inst)
-                    {
-                        case ShaderIrInst.Bra:
-                        case ShaderIrInst.Exit:
-                        case ShaderIrInst.Kil:
-                        case ShaderIrInst.Sync:
-                            UnconditionalFlowChange = true;
-                            break;
-                    }
-                }
-
-                if (!UnconditionalFlowChange)
-                {
-                    SB.AppendLine(IdentationStr + "return " + GetBlockPosition(Block.Next) + ";");
-                }
-            }
-        }
-
         private bool IsValidOutOper(ShaderIrNode Node)
         {
             if (Node is ShaderIrOperGpr Gpr && Gpr.IsConst)
diff --git a/Ryujinx.Graphics/Gal/Shader/ShaderDecodeAlu.cs b/Ryujinx.Graphics/Gal/Shader/ShaderDecodeAlu.cs
index d4a76bc936..6957e30b2a 100644
--- a/Ryujinx.Graphics/Gal/Shader/ShaderDecodeAlu.cs
+++ b/Ryujinx.Graphics/Gal/Shader/ShaderDecodeAlu.cs
@@ -730,7 +730,7 @@ namespace Ryujinx.Graphics.Gal.Shader
             }
 
             ShaderIrNode Src1 = GetAluIneg(ApplyHeight(OpCode.Gpr8(),  Height1), Neg1);
-            ShaderIrNode Src2 = GetAluIneg(ApplyHeight(OperB,                Height2), Neg2);
+            ShaderIrNode Src2 = GetAluIneg(ApplyHeight(OperB,          Height2), Neg2);
             ShaderIrNode Src3 = GetAluIneg(ApplyHeight(OpCode.Gpr39(), Height3), Neg3);
 
             ShaderIrOp Sum = new ShaderIrOp(ShaderIrInst.Add, Src1, Src2);