shaderpipeline: Remove last bits of uniform location handling from front-end

Also do a better job of dealing with the void struct member we insert as placeholders in the legacy GLSL pipeline

panda/src/display/shaderInputBinding_impls.cxx

@@ -244,7 +244,9 @@ check_light_struct_member(const string &name, const ShaderType *type) {
   uint32_t num_rows = 1;
   uint32_t min_cols = 3;
   uint32_t max_cols = 4;
-  if (name == "color") {
+  if (name.empty()) {
+    return type == ShaderType::void_type;
+  } else if (name == "color") {
   } else if (name == "specular") {
   } else if (name == "ambient") {
   } else if (name == "diffuse") {
@@ -786,6 +788,9 @@ make_fog(const ShaderType *type) {
   bool success = true;
   for (size_t i = 0; i < struct_type->get_num_members(); ++i) {
     const ShaderType::Struct::Member &member = struct_type->get_member(i);
+    if (member.type == ShaderType::void_type) {
+      continue;
+    }
 
     CPT(InternalName) fqname = InternalName::make(member.name);
     if (member.name == "color") {
@@ -866,6 +871,9 @@ make_material(const ShaderType *type) {
   bool success = true;
   for (size_t i = 0; i < struct_type->get_num_members(); ++i) {
     const ShaderType::Struct::Member &member = struct_type->get_member(i);
+    if (member.type == ShaderType::void_type) {
+      continue;
+    }
 
     CPT(InternalName) fqname = InternalName::make(member.name);
     if (member.name == "baseColor") {
@@ -1317,6 +1325,9 @@ ShaderLightStructBinding(const ShaderType *type, const InternalName *input) {
 
   for (size_t i = 0; i < struct_type->get_num_members(); ++i) {
     const ShaderType::Struct::Member &member = struct_type->get_member(i);
+    if (member.type == ShaderType::void_type) {
+      continue;
+    }
 
     PT(InternalName) fqname = InternalName::make("light")->append(member.name);
     if (member.name == "color") {

panda/src/glstuff/glShaderContext_src.cxx

@@ -247,7 +247,8 @@ r_count_locations_bindings(const ShaderType *type,
   }
 
   if (type->as_sampled_image() != nullptr ||
-      type->as_image() != nullptr) {
+      type->as_image() != nullptr ||
+      type == ShaderType::void_type) {
     ++num_locations;
     return;
   }
@@ -415,6 +416,10 @@ r_collect_uniforms(const Shader::Parameter &param, UniformBlock &block,
     }
     return;
   }
+  if (type == ShaderType::void_type) {
+    // We use this as a placeholder to advance the location by one.
+    ++cur_location;
+  }
 
   if (type->as_storage_buffer() != nullptr) {
     // These are an exception, they do not have locations but bindings.
@@ -693,6 +698,7 @@ reflect_program(SparseArray &active_locations, LocationMap &locations, LocationM
   struct StackItem {
     std::string name;
     GLint loc;
+    GLint next_loc;
     int num_elements; // 0 means not an array
     ShaderType::Struct type;
   };
@@ -717,6 +723,7 @@ reflect_program(SparseArray &active_locations, LocationMap &locations, LocationM
       }
     }
 
+    // Count the number of parts in the name that match the current stack.
     size_t i = 0;
     bool skip = false;
     while (i < struct_stack.size() && i < parts.size() - 1 &&
@@ -734,6 +741,7 @@ reflect_program(SparseArray &active_locations, LocationMap &locations, LocationM
       continue;
     }
 
+    // Pop everything else from the end of the stack.
     while (i < struct_stack.size()) {
       StackItem item = std::move(struct_stack.back());
       struct_stack.pop_back();
@@ -750,15 +758,18 @@ reflect_program(SparseArray &active_locations, LocationMap &locations, LocationM
       }
       else if (struct_stack.back().num_elements <= 1) {
         // Add as nested struct member
-        while (item.loc > struct_stack.back().loc + struct_stack.back().type.get_num_parameter_locations()) {
+        while (item.loc > struct_stack.back().next_loc) {
           // Add a dummy member
           struct_stack.back().type.add_member(ShaderType::void_type, "");
+          struct_stack.back().next_loc++;
         }
         struct_stack.back().type.add_member(type, item.name);
+        struct_stack.back().next_loc = item.next_loc;
       }
     }
+    // Push the remaining parts (except the last) onto the stack.
     while (struct_stack.size() < parts.size() - 1) {
-      struct_stack.push_back({parts[struct_stack.size()], loc, sizes[struct_stack.size()], {}});
+      struct_stack.push_back({parts[struct_stack.size()], loc, loc, sizes[struct_stack.size()], {}});
     }
 
     const ShaderType *type = get_param_type(param.type);
@@ -776,11 +787,13 @@ reflect_program(SparseArray &active_locations, LocationMap &locations, LocationM
     else if (struct_stack.back().num_elements <= 1) {
       // Add as struct member
       assert(parts.size() > 1);
-      while (loc > struct_stack.back().loc + struct_stack.back().type.get_num_parameter_locations()) {
+      while (loc > struct_stack.back().next_loc) {
         // Add a dummy member
         struct_stack.back().type.add_member(ShaderType::void_type, "");
+        struct_stack.back().next_loc++;
       }
       struct_stack.back().type.add_member(type, parts.back());
+      struct_stack.back().next_loc += param.size;
     }
   }
 
@@ -800,11 +813,13 @@ reflect_program(SparseArray &active_locations, LocationMap &locations, LocationM
     }
     else if (struct_stack.back().num_elements <= 1) {
       // Add as nested struct member
-      while (item.loc > struct_stack.back().loc + struct_stack.back().type.get_num_parameter_locations()) {
+      while (item.loc > struct_stack.back().next_loc) {
         // Add a dummy member
         struct_stack.back().type.add_member(ShaderType::void_type, "");
+        struct_stack.back().next_loc++;
       }
       struct_stack.back().type.add_member(type, item.name);
+      struct_stack.back().next_loc = item.next_loc;
     }
   }
 }

panda/src/gobj/shaderModule.cxx

@@ -68,14 +68,6 @@ void ShaderModule::
 remap_input_locations(const pmap<int, int> &locations) {
 }
 
-/**
- * Remaps parameters with a given location to a given other location.  Locations
- * not included in the map remain untouched.
- */
-void ShaderModule::
-remap_parameter_locations(const pmap<int, int> &locations) {
-}
-
 /**
  *
  */

panda/src/gobj/shaderModule.h

@@ -89,7 +89,6 @@ public:
 
   virtual bool link_inputs(const ShaderModule *previous, pmap<int, int> &remap) const;
   virtual void remap_input_locations(const pmap<int, int> &remap);
-  virtual void remap_parameter_locations(const pmap<int, int> &remap);
 
 PUBLISHED:
   MAKE_PROPERTY(stage, get_stage);

panda/src/gobj/shaderType.cxx

@@ -702,19 +702,6 @@ get_num_interface_locations() const {
   return total;
 }
 
-/**
- * Returns the number of uniform locations taken up by uniform variables having
- * this type.
- */
-int ShaderType::Struct::
-get_num_parameter_locations() const {
-  int total = 0;
-  for (const Member &member : _members) {
-    total += member.type->get_num_parameter_locations();
-  }
-  return total;
-}
-
 /**
  * Returns the number of resources (samplers, etc.) in this type.
  */
@@ -929,15 +916,6 @@ get_num_interface_locations() const {
   return _element_type->get_num_interface_locations() * _num_elements;
 }
 
-/**
- * Returns the number of uniform locations taken up by uniform variables having
- * this type.
- */
-int ShaderType::Array::
-get_num_parameter_locations() const {
-  return _element_type->get_num_parameter_locations() * _num_elements;
-}
-
 /**
  * Returns the number of resources (samplers, etc.) in this type.
  */

panda/src/gobj/shaderType.h

@@ -38,7 +38,6 @@ public:
   virtual uint32_t get_align_bytes() const { return 1; }
   virtual uint32_t get_size_bytes() const { return 0; }
   virtual int get_num_interface_locations() const { return 1; }
-  virtual int get_num_parameter_locations() const { return 1; }
   virtual int get_num_resources() const { return 0; }
 
   enum ScalarType {
@@ -337,7 +336,6 @@ public:
   virtual uint32_t get_align_bytes() const override;
   virtual uint32_t get_size_bytes() const override;
   virtual int get_num_interface_locations() const override;
-  virtual int get_num_parameter_locations() const override;
   virtual int get_num_resources() const override;
 
   bool is_aggregate_type() const override { return true; }
@@ -404,7 +402,6 @@ public:
   virtual uint32_t get_align_bytes() const override;
   virtual uint32_t get_size_bytes() const override;
   virtual int get_num_interface_locations() const override;
-  virtual int get_num_parameter_locations() const override;
   virtual int get_num_resources() const override;
 
   bool is_aggregate_type() const override { return true; }

panda/src/shaderpipeline/shaderModuleSpirV.cxx

@@ -146,7 +146,7 @@ ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words, BamCacheRecord *reco
   transformer.run(SpirVRemoveUnusedVariablesPass());
 
   // Add in location decorations for any inputs that are missing it.
-  transformer.assign_locations(stage);
+  transformer.assign_interface_locations(stage);
 
   // Get rid of uniform locations and bindings.  The numbering rules are
   // different for each back-end, so we regenerate these later.
@@ -647,25 +647,6 @@ remap_input_locations(const pmap<int, int> &locations) {
   }
 }
 
-/**
- * Remaps parameters with a given location to a given other location.  Locations
- * not included in the map remain untouched.
- */
-void ShaderModuleSpirV::
-remap_parameter_locations(const pmap<int, int> &locations) {
-  remap_locations(spv::StorageClassUniformConstant, locations);
-
-  // If we extracted out the parameters, replace the locations there as well.
-  for (Variable &parameter : _parameters) {
-    if (parameter.has_location()) {
-      pmap<int, int>::const_iterator it = locations.find(parameter.get_location());
-      if (it != locations.end()) {
-        parameter._location = it->second;
-      }
-    }
-  }
-}
-
 /**
  * Validates the header of the instruction stream.
  */

panda/src/shaderpipeline/shaderModuleSpirV.h

@@ -46,7 +46,6 @@ public:
 
   virtual bool link_inputs(const ShaderModule *previous, pmap<int, int> &remap) const override;
   virtual void remap_input_locations(const pmap<int, int> &remap) override;
-  virtual void remap_parameter_locations(const pmap<int, int> &remap) override;
 
   virtual std::string get_ir() const override;
 

panda/src/shaderpipeline/spirVTransformer.cxx

@@ -136,11 +136,11 @@ get_result() const {
 }
 
 /**
- * Assigns location decorations to all input, output and uniform variables that
- * do not have a location decoration yet.
+ * Assigns location decorations to all input and output variables that do not
+ * have a location decoration yet.  Does not touch uniform constants.
  */
 void SpirVTransformer::
-assign_locations(ShaderModule::Stage stage) {
+assign_interface_locations(ShaderModule::Stage stage) {
   // Determine which locations have already been assigned.
   bool has_unassigned_locations = false;
   BitArray input_locations;
@@ -153,8 +153,7 @@ assign_locations(ShaderModule::Stage stage) {
       if (!def.has_location()) {
         if (!def.is_builtin() &&
             (def._storage_class == spv::StorageClassInput ||
-             def._storage_class == spv::StorageClassOutput ||
-             def._storage_class == spv::StorageClassUniformConstant)) {
+             def._storage_class == spv::StorageClassOutput)) {
           // A non-built-in variable definition without a location.
           has_unassigned_locations = true;
         }
@@ -165,9 +164,6 @@ assign_locations(ShaderModule::Stage stage) {
       else if (def._storage_class == spv::StorageClassOutput) {
         output_locations.set_range(def._location, def._type ? def._type->get_num_interface_locations() : 1);
       }
-      /*else if (def._storage_class == spv::StorageClassUniformConstant) {
-        uniform_locations.set_range(def._location, def._type ? def._type->get_num_parameter_locations() : 1);
-      }*/
     }
   }
 
@@ -213,17 +209,6 @@ assign_locations(ShaderModule::Stage stage) {
 
         sc_str = "output";
       }
-      /*else if (def._storage_class == spv::StorageClassUniformConstant) {
-        num_locations = def._type->get_num_parameter_locations();
-        if (num_locations == 0) {
-          continue;
-        }
-
-        location = uniform_locations.find_off_range(num_locations);
-        uniform_locations.set_range(location, num_locations);
-
-        sc_str = "uniform";
-      }*/
       else {
         continue;
       }

panda/src/shaderpipeline/spirVTransformer.h

@@ -42,7 +42,7 @@ public:
   INLINE uint32_t get_id_bound() const;
   INLINE const SpirVResultDatabase &get_db() const;
 
-  void assign_locations(ShaderModule::Stage stage);
+  void assign_interface_locations(ShaderModule::Stage stage);
   void assign_locations(pmap<uint32_t, int> locations);
   void assign_procedural_names(const char *prefix, const pmap<uint32_t, int> &suffixes);
   void strip_uniform_locations();