shaderpipeline: Improvements to glslang front-end, preprocessing

* Validate #version before handing off to glslang
* Properly set target to SPIR-V for OpenGL
* Sneakily support GLSL 150 by upgrading to 330 and checking that no 330-specific features are used
* Fix dynamic indexing problem by feeding shader through glslang link pass (no idea why this is needed)
* Replace #pragma include with GL_GOOGLE_include_directive equivalent

panda/src/glstuff/glGraphicsStateGuardian_src.cxx

@@ -159,11 +159,7 @@ null_glBlendColor(GLclampf, GLclampf, GLclampf, GLclampf) {
 // drawing GUIs and such.
 static const string default_vshader =
 #ifndef OPENGLES
-#ifdef __APPLE__ // Apple's GL 3.2 contexts require at least GLSL 1.50.
   "#version 150\n"
-#else
-  "#version 130\n"
-#endif
   "in vec4 p3d_Vertex;\n"
   "in vec4 p3d_Color;\n"
   "in vec2 p3d_MultiTexCoord0;\n"
@@ -188,11 +184,7 @@ static const string default_vshader =
 #ifndef OPENGLES
 // This version of the shader is used if vertices-float64 is enabled.
 static const string default_vshader_fp64 =
-#ifdef __APPLE__
   "#version 150\n"
-#else
-  "#version 130\n"
-#endif
   "#extension GL_ARB_vertex_attrib_64bit : require\n"
   "#extension GL_ARB_gpu_shader_fp64 : require\n"
   "in dvec3 p3d_Vertex;\n"
@@ -229,11 +221,7 @@ static const string default_vshader_fp64_gl41 =
 
 static const string default_fshader =
 #ifndef OPENGLES
-#ifdef __APPLE__  // Apple's GL 3.2 contexts require at least GLSL 1.50.
   "#version 150\n"
-#else
-  "#version 130\n"
-#endif
   "in vec2 texcoord;\n"
   "in vec4 color;\n"
   "out vec4 p3d_FragColor;\n"

panda/src/shaderpipeline/shaderCompilerGlslang.cxx

@@ -12,7 +12,6 @@
  */
 
 #include "shaderCompilerGlslang.h"
-#include "shaderModuleSpirV.h"
 #include "config_shaderpipeline.h"
 #include "virtualFile.h"
 
@@ -281,6 +280,18 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
     return nullptr;
   }
 
+  bool is_cg = false;
+  bool add_include_directive = false;
+  int glsl_version = 110;
+
+  // Is this a Cg shader or a GLSL shader?
+  if (code.size() >= 5 && strncmp((const char *)&code[0], "//Cg\n", 5) == 0) {
+    is_cg = true;
+  }
+  else if (!preprocess_glsl(code, glsl_version, add_include_directive)) {
+    return nullptr;
+  }
+
   static bool is_initialized = false;
   if (!is_initialized) {
     ShInitialize();
@@ -291,15 +302,14 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
   const int length = (int)code.size();
   const char *fname = filename.c_str();
 
-  EShMessages messages = (EShMessages)(EShMsgDefault | EShMsgDebugInfo);
+  EShMessages messages = (EShMessages)(EShMsgDefault | EShMsgSpvRules);
 
   glslang::TShader *shader = new glslang::TShader((EShLanguage)stage);
   shader->setStringsWithLengthsAndNames(&string, &length, &fname, 1);
   shader->setEntryPoint("main");
 
   // If it's marked as a Cg shader, we compile it with the HLSL front-end.
-  bool is_hlsl = false;
-  if (code.size() >= 5 && strncmp((const char *)&code[0], "//Cg\n", 5) == 0) {
+  if (is_cg) {
     shader->setEnvInput(glslang::EShSource::EShSourceHlsl, (EShLanguage)stage, glslang::EShClient::EShClientOpenGL, 120);
     switch (stage) {
     case ShaderModule::Stage::vertex:
@@ -311,6 +321,10 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
     case ShaderModule::Stage::fragment:
       shader->setSourceEntryPoint("fshader");
       break;
+    default:
+      shader_cat.error()
+        << "Cg does not support " << stage << " shaders.\n";
+      return nullptr;
     }
 
     shader->setPreamble(
@@ -319,18 +333,19 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
       "#define shadow2D(s, tc) (float4(tex2D(s, tc) > tc.z))\n"
       "#define shadow2DProj(s, tc) (float4(tex2Dproj(s, tc) > tc.z / tc.w))\n"
     );
-    is_hlsl = true;
     messages = (EShMessages)(messages | EShMsgHlslDX9Compatible | EShMsgHlslLegalization);
+  } else {
+    shader->setEnvInput(glslang::EShSource::EShSourceGlsl, (EShLanguage)stage, glslang::EShClient::EShClientOpenGL, 450);
+
+    if (add_include_directive) {
+      shader->setPreamble(
+        "#extension GL_GOOGLE_include_directive : require\n"
+      );
+    }
   }
-  //shader->setEnvInput(glslang::EShSource::EShSourceGlsl, (EShLanguage)stage, glslang::EShClient::EShClientVulkan, 430);
   shader->setEnvClient(glslang::EShClient::EShClientOpenGL, glslang::EShTargetOpenGL_450);
   shader->setEnvTarget(glslang::EShTargetSpv, glslang::EShTargetSpv_1_0);
 
-  // Have the compilers assign bindings to everything--even if we will end up
-  // changing them, it's useful if there are already location assignments to
-  // overwrite so that we only have to modify instructions and not insert new
-  // ones.
-
   // This will squelch the warnings about missing bindings and locations, since
   // we can assign those ourselves.
   shader->setAutoMapBindings(true);
@@ -344,13 +359,24 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
     return nullptr;
   }
 
-  glslang::TIntermediate *ir = shader->getIntermediate();
+  // I don't know why we need to pretend to link it into a program.  One would
+  // think one can just do shader->getIntermediate() and convert that to SPIR-V,
+  // but that generates shaders that are ever-so-subtly wrong.
+  glslang::TProgram program;
+  program.addShader(shader);
+
+  if (!program.link(messages)) {
+    std::cerr << "failed to link " << filename << "\n";
+    return nullptr;
+  }
+
+  glslang::TIntermediate *ir = program.getIntermediate((EShLanguage)stage);
   if (!ir) {
     std::cerr << "failed to obtain IR " << filename << ":\n";
     return nullptr;
   }
 
-  std::vector<unsigned int> spirv;
+  ShaderModuleSpirV::InstructionStream stream;
   std::string warningsErrors;
   spv::SpvBuildLogger logger;
   glslang::SpvOptions spvOptions;
@@ -359,7 +385,16 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
   spvOptions.optimizeSize = false;
   spvOptions.disassemble = false;
   spvOptions.validate = true;
-  glslang::GlslangToSpv(*ir, spirv, &logger, &spvOptions);
+  glslang::GlslangToSpv(*ir, stream, &logger, &spvOptions);
+
+  if (!stream.validate_header()) {
+    return nullptr;
+  }
+
+  // Special validation for features in GLSL 330 that are not in GLSL 150.
+  if (glsl_version == 150 && !postprocess_glsl150(stream)) {
+    return nullptr;
+  }
 
   printf("%s", logger.getAllMessages().c_str());
   /*if (Options & EOptionOutputHexadecimal) {
@@ -367,18 +402,182 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
       glslang::OutputSpvBin(spirv, GetBinaryName((EShLanguage)stage));
   }*/
 
+  // Run it through the optimizer.
   spvtools::Optimizer opt(SPV_ENV_UNIVERSAL_1_0);
   opt.SetMessageConsumer(log_message);
   opt.RegisterPerformancePasses();
 
-  if (is_hlsl) {
+  if (is_cg) {
     opt.RegisterLegalizationPasses();
   }
 
   std::vector<uint32_t> optimized;
-  if (!opt.Run(spirv.data(), spirv.size(), &optimized)) {
+  if (!opt.Run(stream.get_data(), stream.get_data_size(), &optimized)) {
     return nullptr;
   }
 
-  return new ShaderModuleSpirV(stage, optimized.data(), optimized.size());
+  return new ShaderModuleSpirV(stage, std::move(optimized));
+}
+
+/**
+ * Do some very basic preprocessing of the GLSL shader to extract the GLSL
+ * version and fix the use of #pragma include (which glslang doesn't support,
+ * but we historically did).
+ * Returns false if any errors occurred.
+ */
+bool ShaderCompilerGlslang::
+preprocess_glsl(vector_uchar &code, int &glsl_version, bool &uses_pragma_include) {
+  glsl_version = 110;
+
+  // Make sure it ends with a newline.  This makes parsing easier.
+  if (!code.empty() && code.back() != (unsigned char)'\n') {
+    code.push_back((unsigned char)'\n');
+  }
+
+  char *p = (char *)&code[0];
+  char *end = (char *)&code[0] + code.size();
+  bool has_code = false;
+
+  while (p < end) {
+    if (isspace(*p)) {
+      ++p;
+      continue;
+    }
+    if (*p == '/') {
+      // Check for comment, so that we don't pick up commented-out preprocessor
+      // directives.
+      ++p;
+      if (*p == '/') {
+        // Skip till end of line.
+        do { ++p; } while (*p != '\n');
+      }
+      else if (*p == '*') {
+        // Skip till */
+        do { ++p; } while ((p + 1) < end && (p[0] != '*' || p[1] != '/'));
+      }
+    }
+    else if (*p == '#') {
+      // Check for a preprocessor directive.
+      do { ++p; } while (isspace(*p) && *p != '\n');
+      char *directive = p;
+      do { ++p; } while (!isspace(*p));
+      size_t directive_size = p - directive;
+      do { ++p; } while (isspace(*p) && *p != '\n');
+
+      if (directive_size == 7 && strncmp(directive, "version", directive_size) == 0) {
+        glsl_version = strtol(p, &p, 10);
+
+        if (!isspace(*p)) {
+          shader_cat.error()
+            << "Invalid version number in #version directive\n";
+          return false;
+        }
+
+        if (glsl_version == 150) {
+          // glslang doesn't support 150, but it's almost identical to 330,
+          // and we have many shaders written in 150, so sneakily change
+          // the version number.  We'll do some extra validation later.
+          p[-3] = '3';
+          p[-2] = '3';
+          p[-1] = '0';
+        }
+      }
+      else if (directive_size == 6 && strncmp(directive, "pragma", directive_size) == 0) {
+        if (strncmp(p, "include", 7) == 0 && !isalnum(p[7]) && p[7] != '_') {
+          // Turn this into a normal include directive (by replacing the word
+          // "pragma" with spaces) and enable the GL_GOOGLE_include_directive
+          // extension using the preamble.
+          memset(directive, ' ', directive_size);
+          uses_pragma_include = true;
+          has_code = true;
+
+          static bool warned = false;
+          if (!warned) {
+            warned = true;
+            shader_cat.warning()
+              << "#pragma include is deprecated, use the "
+                 "GL_GOOGLE_include_directive extension instead.\n";
+          }
+        }
+      }
+      else {
+        has_code = true;
+      }
+
+      // Skip the rest of the line.
+      while (*p != '\n') { ++p; }
+    }
+    else {
+      has_code = true;
+    }
+
+    ++p;
+  }
+
+  if (!has_code) {
+    shader_cat.error()
+      << "Shader contains no code\n";
+    return false;
+  }
+
+  if (glsl_version < 330 && glsl_version != 150) {
+    if (glsl_version == 100 || glsl_version == 110 || glsl_version == 120 ||
+        glsl_version == 130 || glsl_version == 140 || glsl_version == 300) {
+      shader_cat.warning()
+        << "Support for GLSL " << glsl_version << " is deprecated.  Some "
+           "features may not work.  Minimum supported version is GLSL 330.\n";
+      return true;
+    }
+    else {
+      shader_cat.error()
+        << "Invalid GLSL version " << glsl_version << ".\n";
+      return false;
+    }
+  }
+
+  return true;
+}
+
+/**
+ * Validates that the given SPIR-V shader does not use features that are
+ * available in GLSL 330 but not in GLSL 150.
+ */
+bool ShaderCompilerGlslang::
+postprocess_glsl150(ShaderModuleSpirV::InstructionStream &stream) {
+  bool has_bit_encoding = false;
+  bool has_explicit_location = false;
+
+  for (ShaderModuleSpirV::Instruction op : stream) {
+    if (op.opcode == SpvOpSource) {
+      // Set this back to 150.
+      if (op.nargs >= 2) {
+        op.args[1] = 150;
+      }
+    }
+    else if (op.opcode == SpvOpSourceExtension) {
+      if (strcmp((const char *)op.args, "GL_ARB_shader_bit_encoding") == 0 ||
+          strcmp((const char *)op.args, "GL_ARB_gpu_shader5") == 0) {
+        has_bit_encoding = true;
+      }
+      else if (strcmp((const char *)op.args, "GL_ARB_explicit_attrib_location") == 0) {
+        has_explicit_location = true;
+      }
+    }
+    else if (op.opcode == SpvOpDecorate && op.nargs >= 2 &&
+             (SpvDecoration)op.args[1] == SpvDecorationLocation &&
+             !has_explicit_location) {
+      shader_cat.error()
+        << "Explicit location assignments require #version 330 or "
+           "#extension GL_ARB_explicit_attrib_location\n";
+      return false;
+    }
+    else if (op.opcode == SpvOpBitcast && !has_bit_encoding) {
+      shader_cat.error()
+        << "floatBitsToInt, floatBitsToUint, intBitsToFloat, uintBitsToFloat"
+           " require #version 330 or #extension GL_ARB_shader_bit_encoding.\n";
+      return false;
+    }
+  }
+
+  return true;
 }

panda/src/shaderpipeline/shaderCompilerGlslang.h

@@ -17,6 +17,7 @@
 #include "pandabase.h"
 
 #include "shaderCompiler.h"
+#include "shaderModuleSpirV.h"
 
 /**
  * ShaderCompiler implementation that uses the libglslang library to compile
@@ -32,6 +33,10 @@ public:
                                        const std::string &filename = "created-shader",
                                        BamCacheRecord *record = nullptr) const override;
 
+private:
+  static bool preprocess_glsl(vector_uchar &code, int &glsl_version, bool &uses_pragma_include);
+  static bool postprocess_glsl150(ShaderModuleSpirV::InstructionStream &stream);
+
 public:
   static TypeHandle get_class_type() {
     return _type_handle;

panda/src/shaderpipeline/shaderModule.h

@@ -77,7 +77,6 @@ public:
   const Variable &get_parameter(size_t i) const;
   int find_parameter(CPT_InternalName name) const;
 
-public:
   typedef pmap<CPT_InternalName, Variable *> VariablesByName;
 
   virtual bool link_inputs(const ShaderModule *previous);

panda/src/shaderpipeline/shaderModuleSpirV.I

@@ -79,8 +79,22 @@ InstructionIterator(uint32_t *words) : _words(words) {
 INLINE ShaderModuleSpirV::InstructionStream::
 InstructionStream(const uint32_t *words, size_t num_words) :
   _words(words, words + num_words) {
-  // We must have at least a valid header.
-  nassertv(num_words >= 5);
+}
+
+/**
+ * Initializes the instruction stream from an existing module.
+ */
+INLINE ShaderModuleSpirV::InstructionStream::
+InstructionStream(std::vector<uint32_t> words) :
+  _words(std::move(words)) {
+}
+
+/**
+ * Direct cast to the underlying vector.
+ */
+INLINE ShaderModuleSpirV::InstructionStream::
+operator std::vector<uint32_t> & () {
+  return _words;
 }
 
 /**
@@ -208,6 +222,14 @@ get_data_size() const {
   return _words.size();
 }
 
+/**
+ * Returns the number of ids allocated.
+ */
+INLINE uint32_t ShaderModuleSpirV::InstructionStream::
+get_id_bound() const {
+  return _words[3];
+}
+
 /**
  * Allocates a new identifier.
  */

panda/src/shaderpipeline/shaderModuleSpirV.cxx

@@ -27,17 +27,23 @@ TypeHandle ShaderModuleSpirV::_type_handle;
  * - Strips debugging information from the module.
  */
 ShaderModuleSpirV::
-ShaderModuleSpirV(Stage stage, const uint32_t *words, size_t size) :
+ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words) :
   ShaderModule(stage),
-  _instructions(words, size)
+  _instructions(std::move(words))
 {
-  Definitions defs;
-  if (!parse(defs)) {
-    shader_cat.error()
-      << "Failed to parse SPIR-V shader code.\n";
+  if (!_instructions.validate_header()) {
     return;
   }
 
+  Definitions defs(_instructions.get_id_bound());
+  for (Instruction op : _instructions) {
+    if (!parse_instruction(defs, op.opcode, op.args, op.nargs)) {
+      shader_cat.error()
+        << "Failed to parse SPIR-V shader code.\n";
+      return;
+    }
+  }
+
   // Check if there is a $Global uniform block.  This is generated by the HLSL
   // front-end of glslang.  If so, unwrap it back down to individual uniforms.
   for (uint32_t id = 0; id < defs.size(); ++id) {
@@ -162,42 +168,30 @@ remap_parameter_locations(pmap<int, int> &locations) {
 }
 
 /**
- * Parses the SPIR-V file, extracting the definitions into the given vector.
+ * Validates the header of the instruction stream.
  */
-bool ShaderModuleSpirV::
-parse(Definitions &defs) {
-  if (get_data_size() < 5) {
+bool ShaderModuleSpirV::InstructionStream::
+validate_header() const {
+  if (_words.size() < 5) {
     shader_cat.error()
       << "Invalid SPIR-V file: too short.\n";
     return false;
   }
 
   // Validate the header.
-  const uint32_t *words = (const uint32_t *)get_data();
+  const uint32_t *words = (const uint32_t *)&_words[0];
   if (*words++ != SpvMagicNumber) {
     shader_cat.error()
       << "Invalid SPIR-V file: wrong magic number.\n";
     return false;
   }
 
-  ++words; // version
-  ++words; // generator
-  uint32_t bound = *words++;
-  ++words; // schema (reserved)
-
-  defs = Definitions(bound);
-
-  for (Instruction op : _instructions) {
-    if (!parse_instruction(defs, op.opcode, op.args, op.nargs)) {
-      return false;
-    }
-  }
-
   return true;
 }
 
 /**
- * Parses the instruction with the given SPIR-V opcode and arguments.
+ * Parses the instruction with the given SPIR-V opcode and arguments.  Any
+ * encountered definitions are stored in the given definitions vector.
  */
 bool ShaderModuleSpirV::
 parse_instruction(Definitions &defs, SpvOp opcode, const uint32_t *args, size_t nargs) {

panda/src/shaderpipeline/shaderModuleSpirV.h

@@ -26,7 +26,7 @@ class ShaderType;
  */
 class EXPCL_PANDA_SHADERPIPELINE ShaderModuleSpirV final : public ShaderModule {
 public:
-  ShaderModuleSpirV(Stage stage, const uint32_t *words, size_t size);
+  ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words);
   virtual ~ShaderModuleSpirV();
 
   virtual PT(CopyOnWriteObject) make_cow_copy() override;
@@ -39,7 +39,6 @@ public:
 
   virtual std::string get_ir() const override;
 
-protected:
   class InstructionStream;
 
   struct Instruction {
@@ -72,7 +71,13 @@ protected:
   public:
     typedef InstructionIterator iterator;
 
-    INLINE InstructionStream(const uint32_t *words, size_t size);
+    InstructionStream() = default;
+    INLINE InstructionStream(const uint32_t *words, size_t num_words);
+    INLINE InstructionStream(std::vector<uint32_t> words);
+
+    bool validate_header() const;
+
+    INLINE operator std::vector<uint32_t> & ();
 
     InstructionStream strip() const;
 
@@ -87,14 +92,18 @@ protected:
     INLINE const uint32_t *get_data() const;
     INLINE size_t get_data_size() const;
 
+    INLINE uint32_t get_id_bound() const;
     INLINE uint32_t allocate_id();
 
   private:
-    pvector<uint32_t> _words;
+    // We're not using a pvector since glslang/spirv-opt are working with
+    // std::vector<uint32_t> and so we can avoid some unnecessary copies.
+    std::vector<uint32_t> _words;
   };
 
   InstructionStream _instructions;
 
+protected:
   enum DefinitionType {
     DT_none,
     DT_type,