Fixed Scissor Test on Intel based GPUs (#595)

* Reworked scissor tests to remove fixme and handle issues with intel gpu's

* Error handling for scissor tests

* Disable strict opengl by default

* Reformatting for JD

* Updated scheme for new property in config

* Fixed typo

* Moved magic value to constant. I liked the magic :(

* Fixed ordering for undertale

* Fixed undertale bug

* Removed strict opengl in favour of required. With this an exception is no longer thrown, just a warning for required extensions

* Uses clamp instead of if's

* Removed evil tabs and no longer used include

Ryujinx.Graphics/Gal/IGalPipeline.cs

@@ -3,6 +3,7 @@
     public interface IGalPipeline
     {
         void Bind(GalPipelineState State);
+        void Unbind(GalPipelineState State);
 
         void ResetDepthMask();
         void ResetColorMask(int Index);

Ryujinx.Graphics/Gal/OpenGL/OGLExtension.cs

@@ -1,19 +1,23 @@
 using OpenTK.Graphics.OpenGL;
+using Ryujinx.Common.Logging;
 using System;
 
 namespace Ryujinx.Graphics.Gal.OpenGL
 {
     static class OGLExtension
     {
+        // Private lazy backing variables
         private static Lazy<bool> s_EnhancedLayouts    = new Lazy<bool>(() => HasExtension("GL_ARB_enhanced_layouts"));
         private static Lazy<bool> s_TextureMirrorClamp = new Lazy<bool>(() => HasExtension("GL_EXT_texture_mirror_clamp"));
         private static Lazy<bool> s_ViewportArray      = new Lazy<bool>(() => HasExtension("GL_ARB_viewport_array"));
 
         private static Lazy<bool> s_NvidiaDriver      = new Lazy<bool>(() => IsNvidiaDriver());
 
+        // Public accessors
         public static bool EnhancedLayouts    => s_EnhancedLayouts.Value;
         public static bool TextureMirrorClamp => s_TextureMirrorClamp.Value;
         public static bool ViewportArray      => s_ViewportArray.Value;
+
         public static bool NvidiaDrvier       => s_NvidiaDriver.Value;
 
         private static bool HasExtension(string Name)
@@ -28,11 +32,39 @@ namespace Ryujinx.Graphics.Gal.OpenGL
                 }
             }
 
+            Logger.PrintInfo(LogClass.Gpu, $"OpenGL extension {Name} unavailable. You may experience some performance degredation");
+
             return false;
         }
 
-        private static bool IsNvidiaDriver() {
+        private static bool IsNvidiaDriver()
+        {
             return GL.GetString(StringName.Vendor).Equals("NVIDIA Corporation");
         }
+
+        public static class Required
+        {
+            // Public accessors
+            public static bool EnhancedLayouts    => s_EnhancedLayoutsRequired.Value;
+            public static bool TextureMirrorClamp => s_TextureMirrorClampRequired.Value;
+            public static bool ViewportArray      => s_ViewportArrayRequired.Value;
+
+            // Private lazy backing variables
+            private static Lazy<bool> s_EnhancedLayoutsRequired    = new Lazy<bool>(() => HasExtensionRequired(OGLExtension.EnhancedLayouts,    "GL_ARB_enhanced_layouts"));
+            private static Lazy<bool> s_TextureMirrorClampRequired = new Lazy<bool>(() => HasExtensionRequired(OGLExtension.TextureMirrorClamp, "GL_EXT_texture_mirror_clamp"));
+            private static Lazy<bool> s_ViewportArrayRequired      = new Lazy<bool>(() => HasExtensionRequired(OGLExtension.ViewportArray,      "GL_ARB_viewport_array"));
+
+            private static bool HasExtensionRequired(bool Value, string Name)
+            {
+                if (Value)
+                {
+                    return true;
+                }
+
+                Logger.PrintWarning(LogClass.Gpu, $"Required OpenGL extension {Name} unavailable. You may experience some rendering issues");
+
+                return false;
+            }
+        }
     }
-}
+}

Ryujinx.Graphics/Gal/OpenGL/OGLPipeline.cs

@@ -270,47 +270,52 @@ namespace Ryujinx.Graphics.Gal.OpenGL
 
 
             // Scissor Test
-            bool forceUpdate;
-
-            for (int Index = 0; Index < New.ScissorTestCount; Index++)
+            // All scissor test are disabled before drawing final framebuffer to screen so we don't need to handle disabling
+            // Skip if there are no scissor tests to enable
+            if (New.ScissorTestCount != 0)
             {
-                forceUpdate = false;
+                int  scissorsApplied = 0;
+                bool applyToAll      = false;
 
-                if (New.ScissorTestEnabled[Index])
+                for (int Index = 0; Index < GalPipelineState.RenderTargetsCount; Index++)
                 {
-                    // If there is only 1 scissor test, geometry shaders are disabled so the scissor test applies to all viewports
-                    if (New.ScissorTestCount == 1)
-                    {
-                        GL.Enable(EnableCap.ScissorTest);
-                    }
-                    else
+                    if (New.ScissorTestEnabled[Index])
                     {
-                        GL.Enable(IndexedEnableCap.ScissorTest, Index);
-                    }
-                    forceUpdate = true;
-                }
-                else
-                {
-                    GL.Disable(IndexedEnableCap.ScissorTest, Index);
-                }
+                        // If viewport arrays are unavailable apply first scissor test to all or
+                        // there is only 1 scissor test and it's the first, the scissor test applies to all viewports
+                        if (!OGLExtension.Required.ViewportArray || (Index == 0 && New.ScissorTestCount == 1))
+                        {
+                            GL.Enable(EnableCap.ScissorTest);
+                            applyToAll = true;
+                        }
+                        else
+                        {
+                            GL.Enable(IndexedEnableCap.ScissorTest, Index);
+                        }
 
-                if (New.ScissorTestEnabled[Index] &&
-                   (New.ScissorTestX[Index]      != Old.ScissorTestX[Index]      ||
-                    New.ScissorTestY[Index]      != Old.ScissorTestY[Index]      ||
-                    New.ScissorTestWidth[Index]  != Old.ScissorTestWidth[Index]  ||
-                    New.ScissorTestHeight[Index] != Old.ScissorTestHeight[Index] ||
-                    forceUpdate)) // Force update intentionally last to reduce if comparisons
-                {
-                    // If there is only 1 scissor test geometry shaders are disables so the scissor test applies to all viewports
-                    if (New.ScissorTestCount == 1)
-                    {
-                        GL.Scissor(New.ScissorTestX[Index], New.ScissorTestY[Index],
-                                   New.ScissorTestWidth[Index], New.ScissorTestHeight[Index]);
-                    }
-                    else
-                    {
-                        GL.ScissorIndexed(Index, New.ScissorTestX[Index], New.ScissorTestY[Index],
-                                                 New.ScissorTestWidth[Index], New.ScissorTestHeight[Index]);
+                        if (New.ScissorTestEnabled[Index] != Old.ScissorTestEnabled[Index] ||
+                            New.ScissorTestX[Index]       != Old.ScissorTestX[Index]       ||
+                            New.ScissorTestY[Index]       != Old.ScissorTestY[Index]       ||
+                            New.ScissorTestWidth[Index]   != Old.ScissorTestWidth[Index]   ||
+                            New.ScissorTestHeight[Index]  != Old.ScissorTestHeight[Index])
+                        {
+                            if (applyToAll)
+                            {
+                                GL.Scissor(New.ScissorTestX[Index],     New.ScissorTestY[Index],
+                                           New.ScissorTestWidth[Index], New.ScissorTestHeight[Index]);
+                            }
+                            else
+                            {
+                                GL.ScissorIndexed(Index, New.ScissorTestX[Index],     New.ScissorTestY[Index],
+                                                         New.ScissorTestWidth[Index], New.ScissorTestHeight[Index]);
+                            }
+                        }
+
+                        // If all scissor tests have been applied, or viewport arrays are unavailable we can skip remaining itterations
+                        if (!OGLExtension.Required.ViewportArray || ++scissorsApplied == New.ScissorTestCount)
+                        {
+                            break;
+                        }
                     }
                 }
             }
@@ -378,6 +383,14 @@ namespace Ryujinx.Graphics.Gal.OpenGL
             Old = New;
         }
 
+        public void Unbind(GalPipelineState State)
+        {
+            if (State.ScissorTestCount > 0)
+            {
+                GL.Disable(EnableCap.ScissorTest);
+            }
+        }
+
         private void SetAllBlendState(BlendState New)
         {
             Enable(EnableCap.Blend, New.Enabled);

Ryujinx.Graphics/Gal/OpenGL/OGLRenderTarget.cs

@@ -367,9 +367,6 @@ namespace Ryujinx.Graphics.Gal.OpenGL
 
             GL.Disable(EnableCap.FramebufferSrgb);
 
-            // Will be re-enabled if needed while binding, called before any game GL calls
-            GL.Disable(EnableCap.ScissorTest);
-
             GL.BlitFramebuffer(
                 SrcX0,
                 SrcY0,

Ryujinx.Graphics/Graphics3d/NvGpuEngine3d.cs

@@ -25,7 +25,12 @@ namespace Ryujinx.Graphics.Graphics3d
 
         private ConstBuffer[][] ConstBuffers;
 
-        // Height kept for flipping y axis
+        // Viewport dimensions kept for scissor test limits
+        private int ViewportX0 = 0;
+        private int ViewportY0 = 0;
+        private int ViewportX1 = 0;
+        private int ViewportY1 = 0;
+        private int ViewportWidth = 0;
         private int ViewportHeight = 0;
 
         private int CurrentInstance = 0;
@@ -98,7 +103,14 @@ namespace Ryujinx.Graphics.Graphics3d
 
             GalPipelineState State = new GalPipelineState();
 
+            // Framebuffer must be run configured because viewport dimensions may be used in other methods
             SetFrameBuffer(State);
+
+            for (int FbIndex = 0; FbIndex < 8; FbIndex++)
+            {
+                SetFrameBuffer(Vmm, FbIndex);
+            }
+
             SetFrontFace(State);
             SetCullFace(State);
             SetDepth(State);
@@ -108,11 +120,6 @@ namespace Ryujinx.Graphics.Graphics3d
             SetColorMask(State);
             SetPrimitiveRestart(State);
 
-            for (int FbIndex = 0; FbIndex < 8; FbIndex++)
-            {
-                SetFrameBuffer(Vmm, FbIndex);
-            }
-
             SetZeta(Vmm);
 
             SetRenderTargets();
@@ -207,11 +214,11 @@ namespace Ryujinx.Graphics.Graphics3d
             float SX = ReadRegisterFloat(NvGpuEngine3dReg.ViewportNScaleX + FbIndex * 8);
             float SY = ReadRegisterFloat(NvGpuEngine3dReg.ViewportNScaleY + FbIndex * 8);
 
-            int VpX = (int)MathF.Max(0, TX - MathF.Abs(SX));
-            int VpY = (int)MathF.Max(0, TY - MathF.Abs(SY));
+            ViewportX0 = (int)MathF.Max(0, TX - MathF.Abs(SX));
+            ViewportY0 = (int)MathF.Max(0, TY - MathF.Abs(SY));
 
-            int VpW = (int)(TX + MathF.Abs(SX)) - VpX;
-            int VpH = (int)(TY + MathF.Abs(SY)) - VpY;
+            ViewportX1 = (int)(TX + MathF.Abs(SX));
+            ViewportY1 = (int)(TY + MathF.Abs(SY));
 
             GalImageFormat Format = ImageUtils.ConvertSurface((GalSurfaceFormat)SurfFormat);
 
@@ -219,9 +226,7 @@ namespace Ryujinx.Graphics.Graphics3d
 
             Gpu.ResourceManager.SendColorBuffer(Vmm, Key, FbIndex, Image);
 
-            ViewportHeight = VpH;
-
-            Gpu.Renderer.RenderTarget.SetViewport(FbIndex, VpX, VpY, VpW, VpH);
+            Gpu.Renderer.RenderTarget.SetViewport(FbIndex, ViewportX0, ViewportY0, ViewportX1 - ViewportX0, ViewportY1 - ViewportY0);
         }
 
         private void SetFrameBuffer(GalPipelineState State)
@@ -423,38 +428,83 @@ namespace Ryujinx.Graphics.Graphics3d
 
         private void SetScissor(GalPipelineState State)
         {
-            // FIXME: Stubbed, only the first scissor test is valid without a geometry shader loaded. At time of writing geometry shaders are also stubbed.
-            // Once geometry shaders are fixed it should be equal to GalPipelineState.RenderTargetCount when shader loaded, otherwise equal to 1
-            State.ScissorTestCount = 1;
+            int count = 0;
 
-            for (int Index = 0; Index < State.ScissorTestCount; Index++)
+            for (int Index = 0; Index < GalPipelineState.RenderTargetsCount; Index++)
             {
                 State.ScissorTestEnabled[Index] = ReadRegisterBool(NvGpuEngine3dReg.ScissorEnable + Index * 4);
 
                 if (State.ScissorTestEnabled[Index])
                 {
-                    uint ScissorHorizontal        = (uint)ReadRegister(NvGpuEngine3dReg.ScissorHorizontal + Index * 4);
-                    uint ScissorVertical          = (uint)ReadRegister(NvGpuEngine3dReg.ScissorVertical + Index * 4);
+                    uint ScissorHorizontal = (uint)ReadRegister(NvGpuEngine3dReg.ScissorHorizontal + Index * 4);
+                    uint ScissorVertical   = (uint)ReadRegister(NvGpuEngine3dReg.ScissorVertical   + Index * 4);
 
-                    State.ScissorTestX[Index]     = (int)((ScissorHorizontal & 0xFFFF) * State.FlipX);                          // X, lower 16 bits
-                    State.ScissorTestWidth[Index] = (int)((ScissorHorizontal >> 16) * State.FlipX) - State.ScissorTestX[Index]; // Width, right side is upper 16 bits
+                    int left  = (int)(ScissorHorizontal & 0xFFFF); // Left, lower 16 bits
+                    int right = (int)(ScissorHorizontal >> 16);    // Right, upper 16 bits
 
-                    State.ScissorTestY[Index]      = (int)((ScissorVertical & 0xFFFF));                                         // Y, lower 16 bits
-                    State.ScissorTestHeight[Index] = (int)((ScissorVertical >> 16)) - State.ScissorTestY[Index];                // Height, top side is upper 16 bits
+                    int bottom = (int)(ScissorVertical & 0xFFFF); // Bottom, lower 16 bits
+                    int top    = (int)(ScissorVertical >> 16);    // Top, upper 16 bits
 
-                    // Y coordinates may have to be flipped
-                    if ((int)State.FlipY == -1)
+                    int width  = Math.Abs(right - left);
+                    int height = Math.Abs(top   - bottom);
+
+                    // If the scissor test covers the whole possible viewport, i.e. uninititalized, disable scissor test
+                    if ((width > NvGpu.MaxViewportSize && height > NvGpu.MaxViewportSize) || width <= 0 || height <= 0)
                     {
-                        State.ScissorTestY[Index] = ViewportHeight - State.ScissorTestY[Index] - State.ScissorTestHeight[Index];
+                        State.ScissorTestEnabled[Index] = false;
+                        continue;
+                    }
 
-                        // Handle negative viewpont coordinate
-                        if (State.ScissorTestY[Index] < 0)
-                        {
-                            State.ScissorTestY[Index] = 0;
-                        }
+                    // Keep track of how many scissor tests are active.
+                    // If only 1, and it's the first user should apply to all viewports
+                    count++;
+
+                    // Flip X
+                    if (State.FlipX == -1)
+                    {
+                        left  = ViewportX1 - (left  - ViewportX0);
+                        right = ViewportX1 - (right - ViewportX0);
+                    }
+                    
+                    // Ensure X is in the right order
+                    if (left > right)
+                    {
+                        int temp = left;
+                        left     = right;
+                        right    = temp;
+                    }
+
+                    // Flip Y
+                    if (State.FlipY == -1)
+                    {
+                        bottom = ViewportY1 - (bottom - ViewportY0);
+                        top    = ViewportY1 - (top - ViewportY0);
                     }
+
+                    // Ensure Y is in the right order
+                    if (bottom > top)
+                    {
+                        int temp = top;
+                        top      = bottom;
+                        bottom   = temp;
+                    }
+
+                    // Handle out of active viewport dimensions
+                    left   = Math.Clamp(left,   ViewportX0, ViewportX1);
+                    right  = Math.Clamp(right,  ViewportX0, ViewportX1);
+                    top    = Math.Clamp(top,    ViewportY0, ViewportY1);
+                    bottom = Math.Clamp(bottom, ViewportY0, ViewportY1);
+
+                    // Save values to state
+                    State.ScissorTestX[Index] = left;
+                    State.ScissorTestY[Index] = bottom;
+
+                    State.ScissorTestWidth[Index]  = right - left;
+                    State.ScissorTestHeight[Index] = top - bottom;
                 }
             }
+
+            State.ScissorTestCount = count;
         }
 
         private void SetBlending(GalPipelineState State)
@@ -997,6 +1047,9 @@ namespace Ryujinx.Graphics.Graphics3d
                 Gpu.Renderer.Rasterizer.DrawArrays(VertexFirst, VertexCount, PrimType);
             }
 
+            // Reset pipeline for host OpenGL calls
+            Gpu.Renderer.Pipeline.Unbind(State);
+
             //Is the GPU really clearing those registers after draw?
             WriteRegister(NvGpuEngine3dReg.IndexBatchFirst, 0);
             WriteRegister(NvGpuEngine3dReg.IndexBatchCount, 0);

Ryujinx.Graphics/NvGpu.cs

@@ -8,6 +8,8 @@ namespace Ryujinx.Graphics
 {
     public class NvGpu
     {
+        public const int MaxViewportSize = 0x3FFF;
+
         public IGalRenderer Renderer { get; private set; }
 
         public GpuResourceManager ResourceManager { get; private set; }