shaderpipeline: Prefer loop unrolling in GLSL < 400 and Cg

This prevents requiring a dynamic indexing cap for lighting shaders with const loop count

panda/src/shaderpipeline/shaderCompilerGlslang.cxx

@@ -364,8 +364,7 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
     return nullptr;
   }
 
-  // Special validation for features in GLSL 330 that are not in GLSL 150.
-  if (glsl_version == 150 && !postprocess_glsl150(stream)) {
+  if (glsl_version < 400 && !postprocess_glsl(stream, glsl_version)) {
     return nullptr;
   }
 
@@ -700,40 +699,50 @@ preprocess_glsl(vector_uchar &code, int &glsl_version, const Filename &source_fi
  * available in GLSL 330 but not in GLSL 150.
  */
 bool ShaderCompilerGlslang::
-postprocess_glsl150(ShaderModuleSpirV::InstructionStream &stream) {
+postprocess_glsl(ShaderModuleSpirV::InstructionStream &stream, int version) {
   bool has_bit_encoding = false;
+  bool has_gpu_shader5 = false;
   bool has_explicit_location = false;
 
   for (ShaderModuleSpirV::Instruction op : stream) {
     if (op.opcode == spv::OpSource) {
       // Set this back to 150.
       if (op.nargs >= 2) {
-        op.args[1] = 150;
+        op.args[1] = version;
       }
     }
     else if (op.opcode == spv::OpSourceExtension) {
-      if (strcmp((const char *)op.args, "GL_ARB_shader_bit_encoding") == 0 ||
-          strcmp((const char *)op.args, "GL_ARB_gpu_shader5") == 0) {
+      if (strcmp((const char *)op.args, "GL_ARB_shader_bit_encoding") == 0) {
         has_bit_encoding = true;
       }
+      else if (strcmp((const char *)op.args, "GL_ARB_gpu_shader5") == 0) {
+        has_gpu_shader5 = true;
+      }
       else if (strcmp((const char *)op.args, "GL_ARB_explicit_attrib_location") == 0) {
         has_explicit_location = true;
       }
     }
     else if (op.opcode == spv::OpDecorate && op.nargs >= 2 &&
              (spv::Decoration)op.args[1] == spv::DecorationLocation &&
-             !has_explicit_location) {
+             !has_explicit_location && version < 330) {
       shader_cat.error()
         << "Explicit location assignments require #version 330 or "
            "#extension GL_ARB_explicit_attrib_location\n";
       return false;
     }
-    else if (op.opcode == spv::OpBitcast && !has_bit_encoding) {
+    else if (op.opcode == spv::OpBitcast && !has_bit_encoding && !has_gpu_shader5 && version < 330) {
       shader_cat.error()
         << "floatBitsToInt, floatBitsToUint, intBitsToFloat, uintBitsToFloat"
            " require #version 330 or #extension GL_ARB_shader_bit_encoding.\n";
       return false;
     }
+    else if (op.opcode == spv::OpLoopMerge && version < 400 && !has_gpu_shader5) {
+      // If DontUnroll wasn't specified, and GLSL 330 is used, then we prefer
+      // unrolling the loop so we don't get dynamic indexing.
+      if ((op.args[2] & (spv::LoopControlUnrollMask | spv::LoopControlDontUnrollMask)) == 0) {
+        op.args[2] |= spv::LoopControlUnrollMask;
+      }
+    }
   }
 
   return true;
@@ -771,6 +780,12 @@ postprocess_cg(ShaderModuleSpirV::InstructionStream &stream) {
       }
       break;
 
+    case spv::OpLoopMerge:
+      if ((op.args[2] & (spv::LoopControlUnrollMask | spv::LoopControlDontUnrollMask)) == 0) {
+        op.args[2] |= spv::LoopControlUnrollMask;
+      }
+      break;
+
     default:
       break;
     }

panda/src/shaderpipeline/shaderCompilerGlslang.h

@@ -39,7 +39,8 @@ private:
                               const Filename &source_filename,
                               pset<Filename> &once_files,
                               BamCacheRecord *record = nullptr);
-  static bool postprocess_glsl150(ShaderModuleSpirV::InstructionStream &stream);
+  static bool postprocess_glsl(ShaderModuleSpirV::InstructionStream &stream,
+                               int version);
   static bool postprocess_cg(ShaderModuleSpirV::InstructionStream &stream);
 
 public:

tests/shaderpipeline/test_glsl_caps.py

@@ -531,6 +531,71 @@ def test_glsl_caps_dynamic_indexing():
     }
     """) == 0
 
+    # Such a simple loop MUST be unrolled!
+    assert compile_and_get_caps(Stage.FRAGMENT, """
+    #version 330
+
+    #define COUNT 3
+    struct LightSource {
+      sampler2D shadowMap;
+    };
+    uniform LightSource source[COUNT];
+
+    out vec4 p3d_FragColor;
+
+    void main() {
+        vec4 result = vec4(0);
+        for (int i = 0; i < COUNT; ++i) {
+            result += texture(source[i].shadowMap, vec2(0.0));
+        }
+        p3d_FragColor = result;
+    }
+    """) == 0
+
+    # This one need not be unrolled.
+    assert compile_and_get_caps(Stage.FRAGMENT, """
+    #version 400
+
+    #define COUNT 3
+    struct LightSource {
+      sampler2D shadowMap;
+    };
+    uniform LightSource source[COUNT];
+
+    out vec4 p3d_FragColor;
+
+    void main() {
+        vec4 result = vec4(0);
+        for (int i = 0; i < COUNT; ++i) {
+            result += texture(source[i].shadowMap, vec2(0.0));
+        }
+        p3d_FragColor = result;
+    }
+    """) == Shader.C_dynamic_indexing
+
+    # This one need not be unrolled either.
+    assert compile_and_get_caps(Stage.FRAGMENT, """
+    #version 330
+
+    #extension GL_ARB_gpu_shader5 : require
+
+    #define COUNT 3
+    struct LightSource {
+      sampler2D shadowMap;
+    };
+    uniform LightSource source[COUNT];
+
+    out vec4 p3d_FragColor;
+
+    void main() {
+        vec4 result = vec4(0);
+        for (int i = 0; i < COUNT; ++i) {
+            result += texture(source[i].shadowMap, vec2(0.0));
+        }
+        p3d_FragColor = result;
+    }
+    """) == Shader.C_dynamic_indexing
+
 
 def test_glsl_caps_atomic_counters():
     assert compile_and_get_caps(Stage.VERTEX, """