From 0973daefa1a509a8b08c936384251f1fee475587 Mon Sep 17 00:00:00 2001 From: BaronKiko Date: Sat, 2 Mar 2019 10:50:21 +0000 Subject: [PATCH] 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 | 1 + Ryujinx.Graphics/Gal/OpenGL/OGLExtension.cs | 36 +++++- Ryujinx.Graphics/Gal/OpenGL/OGLPipeline.cs | 85 +++++++------ .../Gal/OpenGL/OGLRenderTarget.cs | 3 - Ryujinx.Graphics/Graphics3d/NvGpuEngine3d.cs | 117 +++++++++++++----- Ryujinx.Graphics/NvGpu.cs | 2 + 6 files changed, 171 insertions(+), 73 deletions(-) diff --git a/Ryujinx.Graphics/Gal/IGalPipeline.cs b/Ryujinx.Graphics/Gal/IGalPipeline.cs index cba4e7dc..cb470fb4 100644 --- a/Ryujinx.Graphics/Gal/IGalPipeline.cs +++ b/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); diff --git a/Ryujinx.Graphics/Gal/OpenGL/OGLExtension.cs b/Ryujinx.Graphics/Gal/OpenGL/OGLExtension.cs index 52b3d0ce..eb06f83c 100644 --- a/Ryujinx.Graphics/Gal/OpenGL/OGLExtension.cs +++ b/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 s_EnhancedLayouts = new Lazy(() => HasExtension("GL_ARB_enhanced_layouts")); private static Lazy s_TextureMirrorClamp = new Lazy(() => HasExtension("GL_EXT_texture_mirror_clamp")); private static Lazy s_ViewportArray = new Lazy(() => HasExtension("GL_ARB_viewport_array")); private static Lazy s_NvidiaDriver = new Lazy(() => 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 s_EnhancedLayoutsRequired = new Lazy(() => HasExtensionRequired(OGLExtension.EnhancedLayouts, "GL_ARB_enhanced_layouts")); + private static Lazy s_TextureMirrorClampRequired = new Lazy(() => HasExtensionRequired(OGLExtension.TextureMirrorClamp, "GL_EXT_texture_mirror_clamp")); + private static Lazy s_ViewportArrayRequired = new Lazy(() => 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; + } + } } -} \ No newline at end of file +} diff --git a/Ryujinx.Graphics/Gal/OpenGL/OGLPipeline.cs b/Ryujinx.Graphics/Gal/OpenGL/OGLPipeline.cs index 6a928603..96d42e02 100644 --- a/Ryujinx.Graphics/Gal/OpenGL/OGLPipeline.cs +++ b/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) + if (New.ScissorTestEnabled[Index]) { - GL.Enable(EnableCap.ScissorTest); - } - else - { - 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); diff --git a/Ryujinx.Graphics/Gal/OpenGL/OGLRenderTarget.cs b/Ryujinx.Graphics/Gal/OpenGL/OGLRenderTarget.cs index 8dd3b37f..53cfd4a6 100644 --- a/Ryujinx.Graphics/Gal/OpenGL/OGLRenderTarget.cs +++ b/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, diff --git a/Ryujinx.Graphics/Graphics3d/NvGpuEngine3d.cs b/Ryujinx.Graphics/Graphics3d/NvGpuEngine3d.cs index eb6289fb..370fe1fa 100644 --- a/Ryujinx.Graphics/Graphics3d/NvGpuEngine3d.cs +++ b/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]; - - // Handle negative viewpont coordinate - if (State.ScissorTestY[Index] < 0) - { - State.ScissorTestY[Index] = 0; - } + State.ScissorTestEnabled[Index] = false; + continue; } + + // 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); diff --git a/Ryujinx.Graphics/NvGpu.cs b/Ryujinx.Graphics/NvGpu.cs index 3fd91f74..4669c899 100644 --- a/Ryujinx.Graphics/NvGpu.cs +++ b/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; }