From ea4aa1645500d672a9ca0b3ec686519c1099950f Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Thu, 20 Oct 2022 12:48:02 +0100
Subject: [PATCH] Force custom display shaders VS explicit output location.
 Closes #2757

* This works around an obtuse GL requirement that explicit locations on one side
  will cause a break in compatibility with the other side, even if dropping the
  location would work otherwise.
---
 renderdoc/data/glsl/glsl_globals.h | 19 ++++++++++++++++++-
 renderdoc/driver/gl/gl_debug.cpp   | 13 ++++++++++---
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/renderdoc/data/glsl/glsl_globals.h b/renderdoc/data/glsl/glsl_globals.h
index 51cfadf61..cb30b3349 100644
--- a/renderdoc/data/glsl/glsl_globals.h
+++ b/renderdoc/data/glsl/glsl_globals.h
@@ -51,8 +51,25 @@
 // drop I/O location specifiers and bindings on GL, we don't use separate programs so I/O variables
 // can be matched by name, and we don't want to require GL_ARB_shading_language_420pack so we can't
 // specify bindings in shaders.
-#define BINDING(b) layout(std140)
+//
+// however due to obtuse GL rules, variables with identical name and declaration do NOT match if
+// only one of them has an explicit location. This is only used in custom shaders, but means we need
+// to match it as while most drivers will handle the fallback as you'd normally expect, not all do
+// and the spec doesn't require it. This does mean custom shaders won't work if they drop the
+// explicit location, but there's no feasible way to support both and explicit location has been
+// standard since this was added.
+
+#ifdef FORCE_IO_LOCATION
+
+#define IO_LOCATION(l) layout(location = l)
+
+#else
+
 #define IO_LOCATION(l)
+
+#endif
+
+#define BINDING(b) layout(std140)
 #define VERTEX_ID gl_VertexID
 #define INSTANCE_ID gl_InstanceID
 
diff --git a/renderdoc/driver/gl/gl_debug.cpp b/renderdoc/driver/gl/gl_debug.cpp
index 74229b097..a45042516 100644
--- a/renderdoc/driver/gl/gl_debug.cpp
+++ b/renderdoc/driver/gl/gl_debug.cpp
@@ -425,6 +425,16 @@ void GLReplay::InitDebugData()
           "#extension GL_ARB_shader_bit_encoding : require\n";
   }
 
+  vs = GenerateGLSLShader(
+      GetEmbeddedResource(glsl_blit_vert), shaderType, glslBaseVer,
+      "#extension GL_ARB_separate_shader_objects : require\n#define FORCE_IO_LOCATION 1");
+
+  // used to combine with custom shaders.
+  // this has to have explicit locations on the output even though we don't normally use that,
+  // because GL doesn't have a fallback to match by name, and custom shaders are expected to have an
+  // explicit location on the input
+  DebugData.texDisplayVertexShader = CreateShader(eGL_VERTEX_SHADER, vs);
+
   vs = GenerateGLSLShader(GetEmbeddedResource(glsl_blit_vert), shaderType, glslBaseVer);
 
   DebugData.fixedcolFragShaderSPIRV = DebugData.quadoverdrawFragShaderSPIRV = 0;
@@ -453,9 +463,6 @@ void GLReplay::InitDebugData()
     }
   }
 
-  // used to combine with custom shaders.
-  DebugData.texDisplayVertexShader = CreateShader(eGL_VERTEX_SHADER, vs);
-
   for(int i = 0; i < 3; i++)
   {
     rdcstr defines = rdcstr("#define SHADER_BASETYPE ") + ToStr(i) + "\n";