Merge pull request #1669 from ReinUsesLisp/fixup-gs

gl_shader_decompiler: Guard out of bound geometry shader input reads

4 files changed, 24 insertions, 15 deletions

diff --git a/src/video_core/renderer_opengl/gl_shader_cache.cpp b/src/video_core/renderer_opengl/gl_shader_cache.cpp
index d7fb50a85..a85a7c0c5 100644
--- a/src/video_core/renderer_opengl/gl_shader_cache.cpp
+++ b/src/video_core/renderer_opengl/gl_shader_cache.cpp
@@ -121,12 +121,16 @@ GLint CachedShader::GetUniformLocation(const GLShader::SamplerEntry& sampler) {
 }
 
 GLuint CachedShader::LazyGeometryProgram(OGLProgram& target_program,
-                                         const std::string& glsl_topology,
+                                         const std::string& glsl_topology, u32 max_vertices,
                                          const std::string& debug_name) {
     if (target_program.handle != 0) {
         return target_program.handle;
     }
-    const std::string source{geometry_programs.code + "layout (" + glsl_topology + ") in;\n"};
+    std::string source = "#version 430 core\n";
+    source += "layout (" + glsl_topology + ") in;\n";
+    source += "#define MAX_VERTEX_INPUT " + std::to_string(max_vertices) + '\n';
+    source += geometry_programs.code;
+
     OGLShader shader;
     shader.Create(source.c_str(), GL_GEOMETRY_SHADER);
     target_program.Create(true, shader.handle);
diff --git a/src/video_core/renderer_opengl/gl_shader_cache.h b/src/video_core/renderer_opengl/gl_shader_cache.h
index 016ab18f9..ffbf21831 100644
--- a/src/video_core/renderer_opengl/gl_shader_cache.h
+++ b/src/video_core/renderer_opengl/gl_shader_cache.h
@@ -48,22 +48,23 @@ public:
         }
         switch (primitive_mode) {
         case GL_POINTS:
-            return LazyGeometryProgram(geometry_programs.points, "points", "ShaderPoints");
+            return LazyGeometryProgram(geometry_programs.points, "points", 1, "ShaderPoints");
         case GL_LINES:
         case GL_LINE_STRIP:
-            return LazyGeometryProgram(geometry_programs.lines, "lines", "ShaderLines");
+            return LazyGeometryProgram(geometry_programs.lines, "lines", 2, "ShaderLines");
         case GL_LINES_ADJACENCY:
         case GL_LINE_STRIP_ADJACENCY:
-            return LazyGeometryProgram(geometry_programs.lines_adjacency, "lines_adjacency",
+            return LazyGeometryProgram(geometry_programs.lines_adjacency, "lines_adjacency", 4,
                                        "ShaderLinesAdjacency");
         case GL_TRIANGLES:
         case GL_TRIANGLE_STRIP:
         case GL_TRIANGLE_FAN:
-            return LazyGeometryProgram(geometry_programs.triangles, "triangles", "ShaderTriangles");
+            return LazyGeometryProgram(geometry_programs.triangles, "triangles", 3,
+                                       "ShaderTriangles");
         case GL_TRIANGLES_ADJACENCY:
         case GL_TRIANGLE_STRIP_ADJACENCY:
             return LazyGeometryProgram(geometry_programs.triangles_adjacency, "triangles_adjacency",
-                                       "ShaderLines");
+                                       6, "ShaderTrianglesAdjacency");
         default:
             UNREACHABLE_MSG("Unknown primitive mode.");
         }
@@ -78,7 +79,7 @@ public:
 private:
     /// Generates a geometry shader or returns one that already exists.
     GLuint LazyGeometryProgram(OGLProgram& target_program, const std::string& glsl_topology,
-                               const std::string& debug_name);
+                               u32 max_vertices, const std::string& debug_name);
 
     VAddr addr;
     Maxwell::ShaderProgram program_type;
diff --git a/src/video_core/renderer_opengl/gl_shader_decompiler.cpp b/src/video_core/renderer_opengl/gl_shader_decompiler.cpp
index f6a879a7b..5fde22ad4 100644
--- a/src/video_core/renderer_opengl/gl_shader_decompiler.cpp
+++ b/src/video_core/renderer_opengl/gl_shader_decompiler.cpp
@@ -494,10 +494,10 @@ public:
             // instruction for now.
             if (stage == Maxwell3D::Regs::ShaderStage::Geometry) {
                 // TODO(Rodrigo): nouveau sets some attributes after setting emitting a geometry
-                // shader. These instructions use a dirty register as buffer index. To avoid some
-                // drivers from complaining for the out of boundary writes, guard them.
-                const std::string buf_index{"min(" + GetRegisterAsInteger(buf_reg) + ", " +
-                                            std::to_string(MAX_GEOMETRY_BUFFERS - 1) + ')'};
+                // shader. These instructions use a dirty register as buffer index, to avoid some
+                // drivers from complaining about out of boundary writes, guard them.
+                const std::string buf_index{"((" + GetRegisterAsInteger(buf_reg) + ") % " +
+                                            std::to_string(MAX_GEOMETRY_BUFFERS) + ')'};
                 shader.AddLine("amem[" + buf_index + "][" +
                                std::to_string(static_cast<u32>(attribute)) + ']' +
                                GetSwizzle(elem) + " = " + src + ';');
@@ -811,7 +811,11 @@ private:
                                   std::optional<Register> vertex = {}) {
         auto GeometryPass = [&](const std::string& name) {
             if (stage == Maxwell3D::Regs::ShaderStage::Geometry && vertex) {
-                return "gs_" + name + '[' + GetRegisterAsInteger(*vertex, 0, false) + ']';
+                // TODO(Rodrigo): Guard geometry inputs against out of bound reads. Some games set
+                // an 0x80000000 index for those and the shader fails to build. Find out why this
+                // happens and what's its intent.
+                return "gs_" + name + '[' + GetRegisterAsInteger(*vertex, 0, false) +
+                       " % MAX_VERTEX_INPUT]";
             }
             return name;
         };
diff --git a/src/video_core/renderer_opengl/gl_shader_gen.cpp b/src/video_core/renderer_opengl/gl_shader_gen.cpp
index 9d17edd63..eea090e52 100644
--- a/src/video_core/renderer_opengl/gl_shader_gen.cpp
+++ b/src/video_core/renderer_opengl/gl_shader_gen.cpp
@@ -82,8 +82,8 @@ void main() {
 }
 
 ProgramResult GenerateGeometryShader(const ShaderSetup& setup) {
-    std::string out = "#version 430 core\n";
-    out += "#extension GL_ARB_separate_shader_objects : enable\n\n";
+    // Version is intentionally skipped in shader generation, it's added by the lazy compilation.
+    std::string out = "#extension GL_ARB_separate_shader_objects : enable\n\n";
     out += Decompiler::GetCommonDeclarations();
     out += "bool exec_geometry();\n";