shaderpipeline: Assign unique names to structs with SPIRV-Cross

panda/src/glstuff/glShaderContext_src.cxx

@@ -636,8 +636,8 @@ r_collect_uniforms(GLuint program,
       std::string qualname(name);
       qualname += "." + member.name;
 
-      // SPIRV-Cross names struct members _m0, _m1, etc. in declaration order.
-      sprintf(sym_buffer, "%s._m%u", sym, i);
+      // We have named struct members m0, m1, etc. in declaration order.
+      sprintf(sym_buffer, "%s.m%u", sym, i);
       r_collect_uniforms(program, param, calls, member.type, qualname.c_str(), sym_buffer,
                          cur_location, active_locations, resource_index, cur_binding,
                          offset + member.offset);
@@ -2725,26 +2725,6 @@ create_shader(GLuint program, const ShaderModule *module, size_t mi,
 
       ShaderModuleSpirV::InstructionStream stream = spv->_instructions;
 
-      // It's really important that we don't have any member name decorations
-      // if we're going to end up remapping the locations, because we rely on
-      // spirv-cross generating the standard _m0, _m2, etc. names.
-      if ((!options.es && options.version < 430) ||
-          (options.es && options.version < 310)) {
-        ShaderModuleSpirV::InstructionIterator it = stream.begin();
-        while (it != stream.end()) {
-          ShaderModuleSpirV::Instruction op = *it;
-          if (op.opcode == spv::OpMemberName) {
-            it = stream.erase(it);
-            continue;
-          }
-          else if (op.opcode == spv::OpFunction || op.is_annotation()) {
-            // There are no more debug instructions after this point.
-            break;
-          }
-          ++it;
-        }
-      }
-
       if (stage != ShaderModule::Stage::FRAGMENT) {
         alpha_test_mode = RenderAttrib::M_none;
       }
@@ -2811,9 +2791,10 @@ create_shader(GLuint program, const ShaderModule *module, size_t mi,
         if (sc == spv::StorageClassUniformConstant) {
           auto it = id_to_location.find(id);
           if (it != id_to_location.end()) {
-            sprintf(buf, "p%u", it->second);
+            int location = it->second;
+            sprintf(buf, "p%u", location);
             compiler.set_name(id, buf);
-            compiler.set_decoration(id, spv::DecorationLocation, it->second);
+            compiler.set_decoration(id, spv::DecorationLocation, location);
 
             // Older versions of OpenGL (ES) do not support explicit uniform
             // locations, and we need to query the locations later.
@@ -2854,6 +2835,23 @@ create_shader(GLuint program, const ShaderModule *module, size_t mi,
         }
       }
 
+      // For all uniform constant structs, we need to ensure we have procedural
+      // names like _m0, _m1, _m2, etc.  Furthermore, we need to assign each
+      // struct a name that is guaranteed to be the same between stages, since
+      // some drivers will complain if the struct name is different for the
+      // same uniform between different stages.
+      for (auto &item : spv->_uniform_struct_types) {
+        std::ostringstream str;
+        item.second->output_signature(str);
+        compiler.set_name(item.first, str.str());
+
+        char buf[32];
+        for (size_t i = 0; i < item.second->get_num_members(); ++i) {
+          sprintf(buf, "m%d", (int)i);
+          compiler.set_member_name(item.first, i, buf);
+        }
+      }
+
       // Add bindings for the shader storage buffers.
       for (size_t binding = 0; binding < binding_ids.size(); ++binding) {
         uint32_t id = binding_ids[binding];

panda/src/gobj/shaderType.cxx

@@ -13,10 +13,17 @@
 
 #include "shaderType.h"
 
-const char *texture_type_suffixes[] = {
+static const char *texture_type_suffixes[] = {
   "1D", "2D", "3D", "2DArray", "Cube", "Buffer", "CubeArray", "1DArray",
 };
 
+static const char *texture_type_signatures[] = {
+  "1", "2", "3", "A2", "C", "B", "AC", "A1",
+};
+
+static const char scalar_signatures[] = "?fdiub";
+static const char access_signatures[] = "nrwx";
+
 ShaderType::Registry *ShaderType::_registered_types = nullptr;
 TypeHandle ShaderType::_type_handle;
 TypeHandle ShaderType::Void::_type_handle;
@@ -176,6 +183,14 @@ output(std::ostream &out) const {
   out << "void";
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::Void::
+output_signature(std::ostream &out) const {
+  out << 'V';
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -237,6 +252,14 @@ output(std::ostream &out) const {
   out << _scalar_type;
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::Scalar::
+output_signature(std::ostream &out) const {
+  out << scalar_signatures[_scalar_type];
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -365,6 +388,14 @@ output(std::ostream &out) const {
   out << _scalar_type << _num_components;
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::Vector::
+output_signature(std::ostream &out) const {
+  out << scalar_signatures[_scalar_type] << _num_components;
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -467,6 +498,14 @@ output(std::ostream &out) const {
   out << _scalar_type << _num_rows << "x" << _num_columns;
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::Matrix::
+output_signature(std::ostream &out) const {
+  out << scalar_signatures[_scalar_type] << _num_rows << _num_columns;
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -769,6 +808,18 @@ output(std::ostream &out) const {
   out << '}';
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::Struct::
+output_signature(std::ostream &out) const {
+  out << 'S';
+  for (const Member &member : _members) {
+    member.type->output_signature(out);
+  }
+  out << '_';
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -1019,6 +1070,16 @@ output(std::ostream &out) const {
   out << ']';
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::Array::
+output_signature(std::ostream &out) const {
+  _element_type->output_signature(out);
+  out << 'A';
+  out << _num_elements;
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -1143,6 +1204,14 @@ output(std::ostream &out) const {
   out << "image" << texture_type_suffixes[_texture_type];
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::Image::
+output_signature(std::ostream &out) const {
+  out << 'I' << scalar_signatures[_sampled_type] << texture_type_signatures[_texture_type] << access_signatures[(size_t)_access];
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -1208,6 +1277,14 @@ output(std::ostream &out) const {
   out << "sampler";
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::Sampler::
+output_signature(std::ostream &out) const {
+  out << 's';
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -1244,6 +1321,18 @@ output(std::ostream &out) const {
   }
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::SampledImage::
+output_signature(std::ostream &out) const {
+  out << 't';
+  if (_shadow) {
+    out << 's';
+  }
+  out << texture_type_signatures[_texture_type];
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.
@@ -1328,6 +1417,16 @@ output(std::ostream &out) const {
   }
 }
 
+/**
+ * Outputs a signature that compactly but uniquely identifies this type.
+ */
+void ShaderType::StorageBuffer::
+output_signature(std::ostream &out) const {
+  out << 'B';
+  _contained_type->output_signature(out);
+  out << access_signatures[(size_t)_access];
+}
+
 /**
  * Private implementation of compare_to, only called for types with the same
  * TypeHandle.

panda/src/gobj/shaderType.h

@@ -34,6 +34,7 @@ public:
   virtual int compare_to_impl(const ShaderType &other) const=0;
 
   virtual void output(std::ostream &out) const=0;
+  virtual void output_signature(std::ostream &out) const=0;
 
   virtual uint32_t get_align_bytes() const { return 1; }
   virtual uint32_t get_size_bytes() const { return 0; }
@@ -156,6 +157,7 @@ INLINE std::ostream &operator << (std::ostream &out, const ShaderType &stype) {
 class EXPCL_PANDA_GOBJ ShaderType::Void final : public ShaderType {
 public:
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
 
 private:
   virtual int compare_to_impl(const ShaderType &other) const override;
@@ -193,6 +195,7 @@ public:
   const Scalar *as_scalar() const override { return this; }
 
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
 
 private:
   virtual int compare_to_impl(const ShaderType &other) const override;
@@ -242,6 +245,7 @@ public:
   const Vector *as_vector() const override { return this; }
 
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
 
 private:
   virtual int compare_to_impl(const ShaderType &other) const override;
@@ -291,6 +295,7 @@ public:
   const Matrix *as_matrix() const override { return this; }
 
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
 
 private:
   virtual int compare_to_impl(const ShaderType &other) const override;
@@ -336,6 +341,7 @@ public:
   void merge_member_by_name(std::string name, const ShaderType *type);
 
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;
 
   virtual uint32_t get_align_bytes() const override;
@@ -403,6 +409,7 @@ public:
   virtual const ShaderType *merge(const ShaderType *other) const override;
 
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;
 
   uint32_t get_stride_bytes() const;
@@ -466,6 +473,7 @@ public:
   INLINE bool is_writable() const;
 
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;
 
   virtual bool contains_scalar_type(ScalarType type) const override;
@@ -510,6 +518,7 @@ private:
 
 public:
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;
 
   const Sampler *as_sampler() const override { return this; }
@@ -544,6 +553,7 @@ public:
   INLINE bool is_shadow() const;
 
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;
 
   virtual bool contains_scalar_type(ScalarType type) const override;
@@ -585,6 +595,7 @@ public:
   INLINE Access get_access() const;
 
   virtual void output(std::ostream &out) const override;
+  virtual void output_signature(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;
 
   virtual bool contains_scalar_type(ScalarType type) const override;

panda/src/shaderpipeline/shaderModuleSpirV.cxx

@@ -240,6 +240,12 @@ ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words, BamCacheRecord *reco
             break;
           }
         }
+        else if (def._type->is_aggregate_type()) {
+          // Store all the uniform struct types while we have them, as a
+          // convenience for the GL back-end, which may need them.
+          db.collect_nested_structs(_uniform_struct_types, def._type_id);
+        }
+
         if (def.is_dynamically_indexed() &&
             (sampled_image_type != nullptr || def._type->contains_opaque_type())) {
           _used_caps |= C_dynamic_indexing;

panda/src/shaderpipeline/shaderModuleSpirV.h

@@ -123,6 +123,8 @@ public:
 
   InstructionStream _instructions;
 
+  pmap<uint32_t, const ShaderType::Struct *> _uniform_struct_types;
+
 private:
   void remap_locations(spv::StorageClass storage_class, const pmap<int, int> &locations);
   void strip();

panda/src/shaderpipeline/spirVResultDatabase.cxx

@@ -1236,3 +1236,27 @@ mark_used(uint32_t id) {
     nassertv(!_defs[id].is_variable());
   }
 }
+
+/**
+ * For a given type id, recursively collects all struct types nested therein
+ * and writes them to the given map.
+ */
+void SpirVResultDatabase::
+collect_nested_structs(pmap<uint32_t, const ShaderType::Struct *> &result, uint32_t id) const {
+  const Definition &type_def = get_definition(id);
+  if (type_def._type == nullptr) {
+    return;
+  }
+  const ShaderType::Struct *struct_type = type_def._type->as_struct();
+  if (struct_type != nullptr) {
+    result[id] = struct_type;
+  }
+  for (const MemberDefinition &def : type_def._members) {
+    if (def._type_id != 0) {
+      collect_nested_structs(result, def._type_id);
+    }
+  }
+  if (type_def._type_id != 0 && type_def._type_id != id) {
+    collect_nested_structs(result, type_def._type_id);
+  }
+}

panda/src/shaderpipeline/spirVResultDatabase.h

@@ -136,6 +136,8 @@ public:
 
   void mark_used(uint32_t id);
 
+  void collect_nested_structs(pmap<uint32_t, const ShaderType::Struct *> &result, uint32_t id) const;
+
 private:
   Definitions _defs;