Browse Source

shaderpipeline: SSBO support, many other improvements

Fixes #1190
rdb 1 year ago
parent
commit
4a91995082

+ 9 - 0
panda/src/display/shaderInputBinding_impls.I

@@ -82,6 +82,15 @@ ShaderTextureBinding(CPT(InternalName) input, Texture::TextureType desired_type)
   _desired_type(desired_type) {
 }
 
+/**
+ *
+ */
+INLINE ShaderBufferBinding::
+ShaderBufferBinding(CPT(InternalName) input, size_t min_size) :
+  _input(std::move(input)),
+  _min_size(min_size) {
+}
+
 /**
  *
  */

+ 58 - 0
panda/src/display/shaderInputBinding_impls.cxx

@@ -315,6 +315,13 @@ make_shader_input(const ShaderType *type, CPT_InternalName name) {
     Texture::TextureType desired_type = image->get_texture_type();
     return new ShaderTextureBinding(std::move(name), desired_type);
   }
+  else if (const ShaderType::StorageBuffer *buffer = type->as_storage_buffer()) {
+    size_t min_size = 0;
+    if (const ShaderType *contained_type = buffer->get_contained_type()) {
+      min_size = contained_type->get_size_bytes();
+    }
+    return new ShaderBufferBinding(std::move(name), min_size);
+  }
   else if (const ShaderType::Matrix *matrix = type->as_matrix()) {
     // For historical reasons, we handle non-arrayed matrices differently,
     // which has the additional feature that they can be passed in as a
@@ -1787,6 +1794,45 @@ fetch_texture_image(const State &state, ResourceId resource_id, ShaderType::Acce
   return tex;
 }
 
+/**
+ * Returns a mask indicating which state changes should cause the parameter to
+ * be respecified.
+ */
+int ShaderBufferBinding::
+get_state_dep() const {
+  // We don't specify D_frame, because we don't (yet) support updating shader
+  // buffers from the CPU.
+  return Shader::D_frame | Shader::D_shader_inputs;
+}
+
+/**
+ * Returns an opaque resource identifier that can later be used to fetch the
+ * nth resource, which is of the given type.
+ */
+ShaderInputBinding::ResourceId ShaderBufferBinding::
+get_resource_id(int index, const ShaderType *type) const {
+  return (ResourceId)_input.p();
+}
+
+/**
+ * Fetches the shader buffer associated with the given resource identifier,
+ * which was previously returned by get_resource_id.
+ */
+PT(ShaderBuffer) ShaderBufferBinding::
+fetch_shader_buffer(const State &state, ResourceId resource_id) const {
+  const InternalName *name = (const InternalName *)resource_id;
+  PT(ShaderBuffer) buffer = state.gsg->get_target_shader_attrib()->get_shader_input_buffer(name);
+#ifndef NDEBUG
+  if (!_shown_error && buffer->get_data_size_bytes() < _min_size) {
+    _shown_error = true;
+    shader_cat.error()
+      << *buffer << " is too small for shader input " << *name
+      << " (expected at least " << _min_size << " bytes)\n";
+  }
+#endif
+  return buffer;
+}
+
 /**
  * Returns a mask indicating which state changes should cause the parameter to
  * be respecified.
@@ -2152,6 +2198,18 @@ fetch_texture_image(const State &state, ResourceId resource_id, ShaderType::Acce
   return state.gsg->get_target_shader_attrib()->get_shader_input_texture_image(name, access, z, n);
 }
 
+/**
+ * Fetches the shader buffer associated with the given resource identifier,
+ * which was previously returned by get_resource_id.
+ */
+PT(ShaderBuffer) ShaderAggregateBinding::
+fetch_shader_buffer(const State &state, ResourceId resource_id) const {
+  // GLSL does not support SSBOs in structs, but they can be in arrays, and we
+  // might add support for another shader language that does support this.
+  const InternalName *name = (const InternalName *)resource_id;
+  return state.gsg->get_target_shader_attrib()->get_shader_input_buffer(name);
+}
+
 /**
  * Unwraps the aggregate type, storing the individual members.
  */

+ 22 - 0
panda/src/display/shaderInputBinding_impls.h

@@ -248,6 +248,25 @@ protected:
   mutable bool _shown_error = false;
 };
 
+/**
+ * Binds a parameter to a generic storage buffer shader input.
+ */
+class EXPCL_PANDA_DISPLAY ShaderBufferBinding : public ShaderInputBinding {
+public:
+  INLINE ShaderBufferBinding(CPT(InternalName) input, size_t min_size = 0);
+
+  virtual int get_state_dep() const override;
+
+  virtual ResourceId get_resource_id(int index, const ShaderType *type) const;
+  virtual PT(ShaderBuffer) fetch_shader_buffer(const State &state,
+                                               ResourceId resource_id) const;
+
+protected:
+  CPT(InternalName) const _input;
+  size_t const _min_size = 0;
+  mutable bool _shown_error = false;
+};
+
 /**
  * This binds a parameter to a generic numeric data shader input.
  */
@@ -327,6 +346,9 @@ public:
                                           ResourceId index,
                                           ShaderType::Access &access,
                                           int &z, int &n) const;
+  virtual PT(ShaderBuffer) fetch_shader_buffer(const State &state,
+                                               ResourceId resource_id) const;
+
 private:
   void r_collect_members(const InternalName *name, const ShaderType *type, size_t offset = 0);
 

+ 4 - 0
panda/src/gles2gsg/gles2gsg.h

@@ -272,6 +272,10 @@ typedef char GLchar;
 #define GL_UNSIGNED_INT_IMAGE_BUFFER 0x9067
 #define GL_UNSIGNED_INT_IMAGE_2D_ARRAY 0x9069
 #define GL_UNSIGNED_INT_IMAGE_CUBE_MAP_ARRAY 0x906A
+#define GL_SHADER_STORAGE_BUFFER 0x90D2
+#define GL_SHADER_STORAGE_BUFFER_BINDING 0x90D3
+#define GL_SHADER_STORAGE_BUFFER_START 0x90D4
+#define GL_SHADER_STORAGE_BUFFER_SIZE 0x90D5
 #define GL_SYNC_GPU_COMMANDS_COMPLETE 0x9117
 #define GL_UNSIGNALED 0x9118
 #define GL_SIGNALED 0x9119

+ 9 - 1
panda/src/glstuff/glGraphicsStateGuardian_src.cxx

@@ -1810,6 +1810,7 @@ reset() {
       Shader::C_atomic_counters |
       Shader::C_image_load_store |
       Shader::C_image_query_size |
+      Shader::C_storage_buffer |
       Shader::C_compute_shader;
 
     if (has_extension("GL_OES_geometry_shader")) {
@@ -2070,12 +2071,15 @@ reset() {
 
     _glGetProgramInterfaceiv = (PFNGLGETPROGRAMINTERFACEIVPROC)
        get_extension_func("glGetProgramInterfaceiv");
+    _glGetProgramResourceIndex = (PFNGLGETPROGRAMRESOURCEINDEXPROC)
+       get_extension_func("glGetProgramResourceIndex");
     _glGetProgramResourceName = (PFNGLGETPROGRAMRESOURCENAMEPROC)
        get_extension_func("glGetProgramResourceName");
     _glGetProgramResourceiv = (PFNGLGETPROGRAMRESOURCEIVPROC)
        get_extension_func("glGetProgramResourceiv");
 
     if (_glGetProgramInterfaceiv != nullptr &&
+        _glGetProgramResourceIndex != nullptr &&
         _glGetProgramResourceName != nullptr &&
         _glGetProgramResourceiv != nullptr) {
       _supports_program_interface_query = true;
@@ -2431,6 +2435,10 @@ reset() {
     _glShaderStorageBlockBinding = (PFNGLSHADERSTORAGEBLOCKBINDINGPROC)
        get_extension_func("glShaderStorageBlockBinding");
   } else
+#else
+  if (is_at_least_gles_version(3, 1)) {
+    _supports_shader_buffers = _supports_program_interface_query;
+  } else
 #endif
   {
     _supports_shader_buffers = false;
@@ -7500,7 +7508,7 @@ setup_primitive(const unsigned char *&client_pointer,
   return true;
 }
 
-#ifndef OPENGLES
+#ifndef OPENGLES_1
 /**
  * Creates a new retained-mode representation of the given data, and returns a
  * newly-allocated BufferContext pointer to reference it.  It is the

+ 2 - 1
panda/src/glstuff/glGraphicsStateGuardian_src.h

@@ -407,7 +407,7 @@ public:
                        const GeomPrimitivePipelineReader *reader,
                        bool force);
 
-#ifndef OPENGLES
+#ifndef OPENGLES_1
   virtual BufferContext *prepare_shader_buffer(ShaderBuffer *data);
   void apply_shader_buffer(GLuint base, ShaderBuffer *buffer);
   virtual void release_shader_buffer(BufferContext *bc);
@@ -1137,6 +1137,7 @@ public:
 
 #ifndef OPENGLES_1
   PFNGLGETPROGRAMINTERFACEIVPROC _glGetProgramInterfaceiv;
+  PFNGLGETPROGRAMRESOURCEINDEXPROC _glGetProgramResourceIndex;
   PFNGLGETPROGRAMRESOURCENAMEPROC _glGetProgramResourceName;
   PFNGLGETPROGRAMRESOURCEIVPROC _glGetProgramResourceiv;
 

+ 313 - 109
panda/src/glstuff/glShaderContext_src.cxx

@@ -58,16 +58,28 @@ CLP(ShaderContext)(CLP(GraphicsStateGuardian) *glgsg, Shader *s) : ShaderContext
   // Ignoring any locations that may have already been set, we assign a new
   // unique location to each parameter.
   LocationMap locations;
+  LocationMap ssbo_bindings;
   GLint next_location = 0;
+  GLint next_ssbo_binding = 0;
   for (const Shader::Parameter &param : _shader->_parameters) {
-    locations[param._name] = next_location;
-    next_location += param._type->get_num_parameter_locations();
+    GLint num_locations = 0;
+    GLint num_ssbo_bindings = 0;
+    r_count_locations_bindings(param._type, num_locations, num_ssbo_bindings);
+
+    if (num_locations > 0) {
+      locations[param._name] = next_location;
+      next_location += num_locations;
+    }
+    if (num_ssbo_bindings > 0) {
+      ssbo_bindings[param._name] = next_ssbo_binding;
+      next_ssbo_binding += num_ssbo_bindings;
+    }
   }
 
   // We compile and analyze the shader here, instead of in shader.cxx, to
   // avoid gobj getting a dependency on GL stuff.
-  bool needs_query_locations = false;
-  if (!compile_and_link(locations, needs_query_locations)) {
+  bool remap_locations = false;
+  if (!compile_and_link(locations, remap_locations, ssbo_bindings)) {
     release_resources();
     s->_error_flag = true;
     return;
@@ -80,10 +92,12 @@ CLP(ShaderContext)(CLP(GraphicsStateGuardian) *glgsg, Shader *s) : ShaderContext
   // yet, so we need to perform reflection on the shader.
   SparseArray active_locations;
   if (_is_legacy) {
-    reflect_program();
-    needs_query_locations = true;
+    locations.clear();
+    ssbo_bindings.clear();
+
+    reflect_program(active_locations, locations, ssbo_bindings);
   }
-  else if (!needs_query_locations) {
+  else if (!remap_locations) {
     // We still need to query which uniform locations are actually in use,
     // because the GL driver may have optimized some out.
     GLint num_active_uniforms = 0;
@@ -114,8 +128,27 @@ CLP(ShaderContext)(CLP(GraphicsStateGuardian) *glgsg, Shader *s) : ShaderContext
     block._dep = param._binding->get_state_dep();
     block._bindings.push_back({param._binding, 0});
 
-    int chosen_location = locations[param._name];
-    int actual_location = needs_query_locations ? -1 : chosen_location;
+    // We chose a location earlier, but if remap_locations was set to true,
+    // we have to map this to the actual location chosen by the driver.
+    int chosen_location = -1;
+    int actual_location = -1;
+    {
+      auto it = locations.find(param._name);
+      if (it != locations.end()) {
+        chosen_location = it->second;
+        if (!remap_locations) {
+          actual_location = it->second;
+        }
+      }
+    }
+
+    int ssbo_binding = -1;
+    {
+      auto it = ssbo_bindings.find(param._name);
+      if (it != ssbo_bindings.end()) {
+        ssbo_binding = it->second;
+      }
+    }
 
     // Though the code is written to take advantage of UBOs, we're not using
     // UBOs yet, so we instead have the parameters copied to a scratch space.
@@ -123,9 +156,13 @@ CLP(ShaderContext)(CLP(GraphicsStateGuardian) *glgsg, Shader *s) : ShaderContext
     int resource_index = 0;
     std::string name = param._name->get_name();
     char sym_buffer[16];
-    sprintf(sym_buffer, "p%d", chosen_location);
+    sym_buffer[0] = 0;
+    if (remap_locations && chosen_location >= 0) {
+      sprintf(sym_buffer, "p%d", chosen_location);
+    }
     r_collect_uniforms(param, block, param._type, name.c_str(), sym_buffer,
-                       actual_location, active_locations, resource_index);
+                       actual_location, active_locations, resource_index,
+                       ssbo_binding);
 
     if (!block._matrices.empty() || !block._vectors.empty()) {
       _uniform_data_deps |= block._dep;
@@ -175,6 +212,52 @@ CLP(ShaderContext)(CLP(GraphicsStateGuardian) *glgsg, Shader *s) : ShaderContext
   }
 }
 
+/**
+ * Counts the number of uniform locations and shader storage buffer bindings,
+ * which are separate because OpenGL doesn't use locations for SSBOs (unlike
+ * textures).
+ */
+void CLP(ShaderContext)::
+r_count_locations_bindings(const ShaderType *type,
+                           GLint &num_locations, GLint &num_ssbo_bindings) {
+
+  if (const ShaderType::Array *array_type = type->as_array()) {
+    GLint element_locs = 0, element_binds = 0;
+    r_count_locations_bindings(array_type->get_element_type(), element_locs, element_binds);
+    num_locations += element_locs * array_type->get_num_elements();
+    num_ssbo_bindings += element_binds * array_type->get_num_elements();
+    return;
+  }
+  if (const ShaderType::Struct *struct_type = type->as_struct()) {
+    for (uint32_t i = 0; i < struct_type->get_num_members(); ++i) {
+      const ShaderType::Struct::Member &member = struct_type->get_member(i);
+
+      r_count_locations_bindings(member.type, num_locations, num_ssbo_bindings);
+    }
+    return;
+  }
+
+  ShaderType::ScalarType scalar_type;
+  uint32_t num_elements;
+  uint32_t num_rows;
+  uint32_t num_cols;
+  if (type->as_scalar_type(scalar_type, num_elements, num_rows, num_cols)) {
+    num_locations += num_elements;
+    return;
+  }
+
+  if (type->as_sampled_image() != nullptr ||
+      type->as_image() != nullptr) {
+    ++num_locations;
+    return;
+  }
+
+  if (type->as_storage_buffer()) {
+    ++num_ssbo_bindings;
+    return;
+  }
+}
+
 /**
  * For UBO emulation, expand the individual uniforms within aggregate types and
  * make a list of individual glUniform calls, also querying their location from
@@ -184,21 +267,25 @@ CLP(ShaderContext)(CLP(GraphicsStateGuardian) *glgsg, Shader *s) : ShaderContext
 void CLP(ShaderContext)::
 r_collect_uniforms(const Shader::Parameter &param, UniformBlock &block,
                    const ShaderType *type, const char *name, const char *sym,
-                   int location, const SparseArray &active_locations,
-                   int &resource_index, size_t offset) {
+                   int &cur_location, const SparseArray &active_locations,
+                   int &resource_index, int &ssbo_binding, size_t offset) {
 
   ShaderType::ScalarType scalar_type;
   uint32_t num_elements;
   uint32_t num_rows;
   uint32_t num_cols;
   if (type->as_scalar_type(scalar_type, num_elements, num_rows, num_cols)) {
+    int location = cur_location;
     if (location < 0) {
       location = _glgsg->_glGetUniformLocation(_glsl_program, _is_legacy ? name : sym);
       if (location < 0) {
         return;
       }
-    } else if (!active_locations.get_bit(location)) {
-      return;
+    } else {
+      cur_location += num_elements;
+      if (!active_locations.get_bit(location)) {
+        return;
+      }
     }
     if (GLCAT.is_debug()) {
       GLCAT.debug()
@@ -299,16 +386,14 @@ r_collect_uniforms(const Shader::Parameter &param, UniformBlock &block,
     char *name_buffer = (char *)alloca(strlen(name) + 14);
     char *sym_buffer = (char *)alloca(strlen(sym) + 14);
     const ShaderType *element_type = array_type->get_element_type();
-    int num_locations = element_type->get_num_parameter_locations();
     size_t stride = (size_t)array_type->get_stride_bytes();
 
     for (uint32_t i = 0; i < array_type->get_num_elements(); ++i) {
       sprintf(name_buffer, "%s[%u]", name, i);
       sprintf(sym_buffer, "%s[%u]", sym, i);
-      r_collect_uniforms(param, block, element_type, name_buffer, sym_buffer, location, active_locations, resource_index, offset);
-      if (location >= 0) {
-        location += num_locations;
-      }
+      r_collect_uniforms(param, block, element_type, name_buffer, sym_buffer,
+                         cur_location, active_locations, resource_index, ssbo_binding,
+                         offset);
       offset += stride;
     }
     return;
@@ -324,30 +409,59 @@ r_collect_uniforms(const Shader::Parameter &param, UniformBlock &block,
 
       // SPIRV-Cross names struct members _m0, _m1, etc. in declaration order.
       sprintf(sym_buffer, "%s._m%u", sym, i);
-      r_collect_uniforms(param, block, member.type, qualname.c_str(), sym_buffer, location, active_locations, resource_index, offset + member.offset);
+      r_collect_uniforms(param, block, member.type, qualname.c_str(), sym_buffer,
+                         cur_location, active_locations, resource_index, ssbo_binding,
+                         offset + member.offset);
+    }
+    return;
+  }
 
-      if (location >= 0) {
-        location += member.type->get_num_parameter_locations();
-      }
+  if (type->as_storage_buffer() != nullptr) {
+    // These are an exception, they do not have locations but bindings.
+    GLint binding = ssbo_binding;
+    if (binding < 0) {
+      // We have to look this one up.
+      GLuint index = _glgsg->_glGetProgramResourceIndex(_glsl_program, GL_SHADER_STORAGE_BLOCK, name);
+      const GLenum props[] = {GL_BUFFER_BINDING};
+      _glgsg->_glGetProgramResourceiv(_glsl_program, GL_SHADER_STORAGE_BLOCK, index, 1, props, 1, nullptr, &binding);
+    } else {
+      ++ssbo_binding;
+    }
+
+    if (GLCAT.is_debug()) {
+      GLCAT.debug()
+        << "Storage block " << name << " with type " << *type
+        << " is bound at binding " << binding << "\n";
     }
+
+    StorageBlock block;
+    block._binding = param._binding;
+    block._resource_id = param._binding->get_resource_id(resource_index++, type);
+    block._binding_index = binding++;
+    _storage_blocks.push_back(std::move(block));
     return;
   }
 
+  int location = cur_location;
   if (location < 0) {
     location = _glgsg->_glGetUniformLocation(_glsl_program, _is_legacy ? name : sym);
     if (location < 0) {
       return;
     }
-  } else if (!active_locations.get_bit(location)) {
-    return;
+  } else {
+    ++cur_location;
+    if (!active_locations.get_bit(location)) {
+      return;
+    }
   }
+
   if (GLCAT.is_debug()) {
     GLCAT.debug()
       << "Active uniform " << name << " with type " << *type
       << " is bound to location " << location << "\n";
   }
 
-  if (const ::ShaderType::SampledImage *sampler = type->as_sampled_image()) {
+  if (const ShaderType::SampledImage *sampler = type->as_sampled_image()) {
     TextureUnit unit;
     unit._binding = param._binding;
     unit._resource_id = param._binding->get_resource_id(resource_index++, type);
@@ -356,7 +470,7 @@ r_collect_uniforms(const Shader::Parameter &param, UniformBlock &block,
     _glgsg->_glUniform1i(location, (GLint)_texture_units.size());
     _texture_units.push_back(std::move(unit));
   }
-  else if (const ::ShaderType::Image *image = type->as_image()) {
+  else if (const ShaderType::Image *image = type->as_image()) {
     ImageUnit unit;
     unit._binding = param._binding;
     unit._resource_id = param._binding->get_resource_id(resource_index++, type);
@@ -373,10 +487,13 @@ r_collect_uniforms(const Shader::Parameter &param, UniformBlock &block,
 
 /**
  * Analyzes the uniforms, attributes, etc. of a shader that was not already
- * reflected.
+ * reflected.  Also sets active_locations to all ranges of uniforms that are
+ * active, and anything that is known about top-level uniform names (that are
+ * not in a struct or array) is also already filled into the maps.  The rest
+ * will have to be queried later.
  */
 void CLP(ShaderContext)::
-reflect_program() {
+reflect_program(SparseArray &active_locations, LocationMap &locations, LocationMap &ssbo_bindings) {
   // Process the vertex attributes first.
   GLint param_count = 0;
   GLint name_buflen = 0;
@@ -426,7 +543,9 @@ reflect_program() {
     }
   }
 
-#ifndef OPENGLES
+  _shader->_parameters.clear();
+
+#ifndef OPENGLES_1
   // Get the used shader storage blocks.
   if (_glgsg->_supports_shader_buffers) {
     GLint block_count = 0, block_maxlength = 0;
@@ -437,7 +556,17 @@ reflect_program() {
     block_maxlength = max(64, block_maxlength);
     char *block_name_cstr = (char *)alloca(block_maxlength);
 
+#ifndef OPENGLES
     BitArray bindings;
+#endif
+
+    // OpenGL exposes SSBO arrays as individual members named name[0][1],
+    // name[0][2], etc. with potential gaps for unused SSBOs.  We have to
+    // untangle this.
+    struct SSBO {
+      pvector<uint32_t> _array_sizes;
+    };
+    pmap<CPT_InternalName, SSBO> ssbos;
 
     for (int i = 0; i < block_count; ++i) {
       block_name_cstr[0] = 0;
@@ -447,12 +576,14 @@ reflect_program() {
       GLint values[2];
       _glgsg->_glGetProgramResourceiv(_glsl_program, GL_SHADER_STORAGE_BLOCK, i, 2, props, 2, nullptr, values);
 
+#ifndef OPENGLES
       if (bindings.get_bit(values[0])) {
         // Binding index already in use, assign a different one.
         values[0] = bindings.get_lowest_off_bit();
         _glgsg->_glShaderStorageBlockBinding(_glsl_program, i, values[0]);
       }
       bindings.set_bit(values[0]);
+#endif
 
       if (GLCAT.is_debug()) {
         GLCAT.debug()
@@ -461,11 +592,49 @@ reflect_program() {
           << values[0] << "\n";
       }
 
-      StorageBlock block;
-      block._name = InternalName::make(block_name_cstr);
-      block._binding_index = values[0];
-      block._min_size = (GLuint)values[1];
-      _storage_blocks.push_back(block);
+      // Parse the array elements off the end of the name.
+      char *p = strchr(block_name_cstr, '[');
+      if (p != nullptr) {
+        // It's an array, this is a bit annoying.
+        CPT_InternalName name = InternalName::make(std::string(block_name_cstr, p - block_name_cstr));
+        SSBO &ssbo = ssbos[name];
+        size_t i = 0;
+        do {
+          ++p;
+          uint32_t count = strtoul(p, &p, 10) + 1;
+          if (*p == ']') {
+            if (i >= ssbo._array_sizes.size()) {
+              ssbo._array_sizes.resize(i + 1, 0);
+            }
+            if (count > ssbo._array_sizes[i]) {
+              ssbo._array_sizes[i] = count;
+            }
+            ++i;
+            ++p;
+          }
+        } while (*p == '[');
+      } else {
+        // Simple case.  We can already jot down the binding, so we don't have
+        // to query it later.
+        CPT(InternalName) name = InternalName::make(std::string(block_name_cstr));
+        ssbo_bindings[name] = values[0];
+        ssbos[std::move(name)];
+      }
+    }
+
+    for (auto &item : ssbos) {
+      const InternalName *name = item.first.p();
+      SSBO &ssbo = item.second;
+
+      //TODO: write code to actually query the block variables.
+      const ShaderType *struct_type = ShaderType::register_type(ShaderType::Struct());
+      const ShaderType *type = ShaderType::register_type(ShaderType::StorageBuffer(struct_type, ShaderType::Access::read_write));
+
+      std::reverse(ssbo._array_sizes.begin(), ssbo._array_sizes.end());
+      for (uint32_t count : ssbo._array_sizes) {
+        type = ShaderType::register_type(ShaderType::Array(type, count));
+      }
+      _shader->add_parameter(name, type);
     }
   }
 #endif
@@ -475,8 +644,6 @@ reflect_program() {
   param_count = 0;
   _glgsg->_glGetProgramiv(_glsl_program, GL_ACTIVE_UNIFORMS, &param_count);
 
-  _shader->_parameters.clear();
-
   // First gather the uniforms and sort them by location.
   struct Uniform {
     std::string name;
@@ -518,6 +685,8 @@ reflect_program() {
       continue;
     }
 
+    active_locations.set_range(loc, param_size);
+
     uniforms.insert({loc, {std::string(name_buffer), param_size, param_type}});
   }
 
@@ -575,7 +744,9 @@ reflect_program() {
 
       if (struct_stack.empty()) {
         // Add struct as top-level
-        _shader->add_parameter(InternalName::make(item.name), type, item.loc);
+        CPT(InternalName) name = InternalName::make(item.name);
+        locations[name] = item.loc;
+        _shader->add_parameter(std::move(name), type, item.loc);
       }
       else if (struct_stack.back().num_elements <= 1) {
         // Add as nested struct member
@@ -596,9 +767,11 @@ reflect_program() {
     }
 
     if (struct_stack.empty()) {
-      // Add as top-level
+      // Add as top-level.
       assert(parts.size() == 1);
-      _shader->add_parameter(InternalName::make(parts[0]), type, loc);
+      CPT(InternalName) name = InternalName::make(parts[0]);
+      locations[name] = loc;
+      _shader->add_parameter(std::move(name), type, loc);
     }
     else if (struct_stack.back().num_elements <= 1) {
       // Add as struct member
@@ -621,7 +794,9 @@ reflect_program() {
 
     if (struct_stack.empty()) {
       // Add struct as top-level
-      _shader->add_parameter(InternalName::make(item.name), type, item.loc);
+      CPT(InternalName) name = InternalName::make(item.name);
+      locations[name] = item.loc;
+      _shader->add_parameter(std::move(name), type, item.loc);
     }
     else if (struct_stack.back().num_elements <= 1) {
       // Add as nested struct member
@@ -1993,24 +2168,15 @@ update_shader_texture_bindings(ShaderContext *prev) {
  */
 void CLP(ShaderContext)::
 update_shader_buffer_bindings(ShaderContext *prev) {
-#ifndef OPENGLES
   // Update the shader storage buffer bindings.
-  const ShaderAttrib *attrib = _glgsg->_target_shader;
-
-  for (size_t i = 0; i < _storage_blocks.size(); ++i) {
-    StorageBlock &block = _storage_blocks[i];
+  ShaderInputBinding::State state;
+  state.gsg = _glgsg;
+  state.matrix_cache = &_matrix_cache[0];
 
-    ShaderBuffer *buffer = attrib->get_shader_input_buffer(block._name);
-#ifndef NDEBUG
-    if (buffer->get_data_size_bytes() < block._min_size) {
-      GLCAT.error()
-        << "cannot bind " << *buffer << " to shader because it is too small"
-           " (expected at least " << block._min_size << " bytes)\n";
-    }
-#endif
+  for (const StorageBlock &block : _storage_blocks) {
+    PT(ShaderBuffer) buffer = block._binding->fetch_shader_buffer(state, block._resource_id);
     _glgsg->apply_shader_buffer(block._binding_index, buffer);
   }
-#endif
 }
 
 /**
@@ -2154,7 +2320,8 @@ report_program_errors(GLuint program, bool fatal) {
  */
 bool CLP(ShaderContext)::
 attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
-              const LocationMap &locations, bool &needs_query_locations) {
+              const LocationMap &locations, bool &remap_locations,
+              const LocationMap &ssbo_bindings) {
   ShaderModule::Stage stage = module->get_stage();
 
   GLuint handle = 0;
@@ -2207,11 +2374,23 @@ attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
     ShaderModuleSpirV *spv = (ShaderModuleSpirV *)module;
 
     pmap<uint32_t, int> id_to_location;
-    for (size_t pi = 0; pi < spv->get_num_parameters(); ++pi) {
-      const ShaderModule::Variable &var = spv->get_parameter(pi);
-      auto it = locations.find(var.name);
-      if (it != locations.end()) {
-        id_to_location[var.id] = it->second;
+    pvector<uint32_t> ssbo_binding_ids;
+    for (size_t pi = 0; pi < module->get_num_parameters(); ++pi) {
+      const ShaderModule::Variable &var = module->get_parameter(pi);
+      {
+        auto it = locations.find(var.name);
+        if (it != locations.end()) {
+          id_to_location[var.id] = it->second;
+        }
+      }
+      {
+        auto it = ssbo_bindings.find(var.name);
+        if (it != ssbo_bindings.end()) {
+          if (it->second >= (int)ssbo_binding_ids.size()) {
+            ssbo_binding_ids.resize(it->second + 1, 0);
+          }
+          ssbo_binding_ids[it->second] = var.id;
+        }
       }
     }
 
@@ -2230,33 +2409,21 @@ attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
 
       if (!id_to_location.empty()) {
         transformer.assign_locations(id_to_location);
-      }
 
-      ShaderModuleSpirV::InstructionStream stream = transformer.get_result();
-
-      if (_glgsg->_gl_vendor == "NVIDIA Corporation" && !id_to_location.empty()) {
-        // Sigh... NVIDIA driver gives an error if the SPIR-V ID doesn't match
-        // for variables with overlapping locations if the OpName is stripped.
-        // We'll have to just insert OpNames for every parameter.
-        // https://forums.developer.nvidia.com/t/gl-arb-gl-spirv-bug-duplicate-location-link-error-if-opname-is-stripped-from-spir-v-shader/128491
-        // Bug was found with 446.14 drivers on Windows 10 64-bit.
-        ShaderModuleSpirV::InstructionIterator it = stream.begin_annotations();
-        for (ShaderModuleSpirV::Instruction op : spv->_instructions) {
-          if (op.opcode == spv::OpVariable &&
-              (spv::StorageClass)op.args[2] == spv::StorageClassUniformConstant) {
-            uint32_t var_id = op.args[1];
-            auto lit = id_to_location.find(var_id);
-            if (lit != id_to_location.end()) {
-              uint32_t args[4] = {var_id, 0, 0, 0};
-              int len = sprintf((char *)(args + 1), "p%d", lit->second);
-              nassertr(len > 0 && len < 12, false);
-              it = stream.insert(it, spv::OpName, args, len / 4 + 2);
-              ++it;
-            }
-          }
+        if (_glgsg->_gl_vendor == "NVIDIA Corporation") {
+          // Sigh... NVIDIA driver gives an error if the SPIR-V ID doesn't match
+          // for variables with overlapping locations if the OpName is stripped.
+          // We'll have to just insert OpNames for every parameter.
+          // https://forums.developer.nvidia.com/t/gl-arb-gl-spirv-bug-duplicate-location-link-error-if-opname-is-stripped-from-spir-v-shader/128491
+          // Bug was found with 446.14 drivers on Windows 10 64-bit.
+          transformer.assign_procedural_names("p", id_to_location);
         }
       }
+      if (!ssbo_binding_ids.empty()) {
+        transformer.bind_descriptor_set(0, ssbo_binding_ids);
+      }
 
+      ShaderModuleSpirV::InstructionStream stream = transformer.get_result();
       _glgsg->_glShaderBinary(1, &handle, GL_SHADER_BINARY_FORMAT_SPIR_V_ARB,
                               (const char *)stream.get_data(),
                               stream.get_data_size() * sizeof(uint32_t));
@@ -2274,9 +2441,8 @@ attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
           << "Transpiling SPIR-V " << stage << " shader "
           << module->get_source_filename() << "\n";
       }
-      spirv_cross::CompilerGLSL compiler(std::vector<uint32_t>(spv->get_data(), spv->get_data() + spv->get_data_size()));
-      spirv_cross::CompilerGLSL::Options options;
 
+      spirv_cross::CompilerGLSL::Options options;
       options.version = _glgsg->_glsl_version;
 #ifdef OPENGLES
       options.es = true;
@@ -2288,12 +2454,42 @@ attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
 #endif
       options.vertex.support_nonzero_base_instance = false;
       options.enable_420pack_extension = false;
-      compiler.set_common_options(options);
+
+      // We want to use explicit bindings for SSBOs, which requires 420 or
+      // the 420pack extension.  Presumably we have it if we support SSBOs.
+      if (!ssbo_binding_ids.empty() && !options.es && options.version < 420) {
+        options.enable_420pack_extension = true;
+      }
 
       if (options.version < 130) {
         _emulate_float_attribs = true;
       }
 
+      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;
+        }
+      }
+
+      spirv_cross::CompilerGLSL compiler(std::move(stream._words));
+      compiler.set_common_options(options);
+
       // At this time, SPIRV-Cross doesn't always add these automatically.
       uint64_t used_caps = module->get_used_capabilities();
 #ifndef OPENGLES
@@ -2311,6 +2507,9 @@ attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
         if (options.version < 400 && (used_caps & Shader::C_dynamic_indexing) != 0) {
           compiler.require_extension("GL_ARB_gpu_shader5");
         }
+        if (options.version < 430 && (used_caps & Shader::C_storage_buffer) != 0) {
+          compiler.require_extension("GL_ARB_shader_storage_buffer_object");
+        }
       }
       else
 #endif
@@ -2336,26 +2535,19 @@ attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
 
         char buf[1024];
         if (sc == spv::StorageClassUniformConstant) {
-          CPT(InternalName) name;
-          for (size_t i = 0; i < spv->get_num_parameters(); ++i) {
-            const ShaderModule::Variable &var = spv->get_parameter(i);
-            if (var.id == id) {
-              auto it = locations.find(var.name);
-              if (it != locations.end()) {
-                sprintf(buf, "p%u", it->second);
-                compiler.set_name(id, buf);
-                compiler.set_decoration(id, spv::DecorationLocation, it->second);
-              }
-              break;
+          auto it = id_to_location.find(id);
+          if (it != id_to_location.end()) {
+            sprintf(buf, "p%u", it->second);
+            compiler.set_name(id, buf);
+            compiler.set_decoration(id, spv::DecorationLocation, it->second);
+
+            // Older versions of OpenGL (ES) do not support explicit uniform
+            // locations, and we need to query the locations later.
+            if ((!options.es && options.version < 430) ||
+                (options.es && options.version < 310)) {
+              remap_locations = true;
             }
           }
-
-          // Older versions of OpenGL (ES) do not support explicit uniform
-          // locations, and we need to query the locations later.
-          if ((!options.es && options.version < 430) ||
-              (options.es && options.version < 310)) {
-            needs_query_locations = true;
-          }
         }
         else if (sc == spv::StorageClassInput) {
           if (stage == ShaderModule::Stage::vertex) {
@@ -2388,6 +2580,14 @@ attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
         }
       }
 
+      // Add bindings for the shader storage buffers.
+      for (size_t binding = 0; binding < ssbo_binding_ids.size(); ++binding) {
+        uint32_t id = ssbo_binding_ids[binding];
+        if (id > 0) {
+          compiler.set_decoration(id, spv::DecorationBinding, binding);
+        }
+      }
+
       // Optimize out unused variables.
       compiler.set_enabled_interface_variables(compiler.get_active_interface_variables());
 
@@ -2436,10 +2636,14 @@ attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &consts,
 }
 
 /**
- * This subroutine compiles a GLSL shader.
+ * This subroutine compiles a GLSL shader.  The given locations and SSBO
+ * bindings will be assigned.  If that is not possible, it will instead
+ * generate names for the uniforms based on the given locations, and
+ * remap_locations will be set to true.
  */
 bool CLP(ShaderContext)::
-compile_and_link(const LocationMap &locations, bool &needs_query_locations) {
+compile_and_link(const LocationMap &locations, bool &remap_locations,
+                 const LocationMap &ssbo_bindings) {
   _modules.clear();
   _glsl_program = _glgsg->_glCreateProgram();
   if (!_glsl_program) {
@@ -2478,7 +2682,7 @@ compile_and_link(const LocationMap &locations, bool &needs_query_locations) {
   bool valid = true;
   for (Shader::LinkedModule &linked_module : _shader->_modules) {
     valid &= attach_shader(linked_module._module.get_read_pointer(), linked_module._consts,
-                           locations, needs_query_locations);
+                           locations, remap_locations, ssbo_bindings);
   }
 
   if (!valid) {

+ 17 - 14
panda/src/glstuff/glShaderContext_src.h

@@ -31,6 +31,7 @@ class CLP(GraphicsStateGuardian);
 class EXPCL_GL CLP(ShaderContext) final : public ShaderContext {
 private:
   struct UniformBlock;
+  typedef pmap<const InternalName *, GLint> LocationMap;
 
 public:
   friend class CLP(GraphicsStateGuardian);
@@ -39,13 +40,18 @@ public:
   ~CLP(ShaderContext)();
   ALLOC_DELETED_CHAIN(CLP(ShaderContext));
 
+  static void r_count_locations_bindings(const ShaderType *type,
+                                         GLint &num_locations,
+                                         GLint &num_ssbo_bindings);
+
   void r_collect_uniforms(const Shader::Parameter &param, UniformBlock &block,
                           const ShaderType *type, const char *name,
-                          const char *sym, int location,
+                          const char *sym, int &location,
                           const SparseArray &active_locations,
-                          int &resource_index, size_t offset = 0);
+                          int &resource_index, int &ssbo_binding,
+                          size_t offset = 0);
 
-  void reflect_program();
+  void reflect_program(SparseArray &active_locations, LocationMap &locations, LocationMap &ssbo_bindings);
   void reflect_attribute(int i, char *name_buf, GLsizei name_buflen);
   void reflect_uniform_block(int i, const char *block_name,
                              char *name_buffer, GLsizei name_buflen);
@@ -135,7 +141,7 @@ private:
   TextureUnits _texture_units;
 
   struct ImageUnit {
-    ShaderInputBinding *_binding;
+    PT(ShaderInputBinding) _binding;
     ShaderInputBinding::ResourceId _resource_id;
     CLP(TextureContext) *_gtc = nullptr;
     ShaderType::Access _access;
@@ -147,28 +153,25 @@ private:
   BitMask32 _enabled_attribs;
   GLint _color_attrib_index;
 
-#ifndef OPENGLES
   struct StorageBlock {
-    CPT(InternalName) _name;
-    GLuint _binding_index;
-    GLuint _min_size;
+    PT(ShaderInputBinding) _binding;
+    ShaderInputBinding::ResourceId _resource_id;
+    GLint _binding_index;
   };
   typedef pvector<StorageBlock> StorageBlocks;
   StorageBlocks _storage_blocks;
-  BitArray _used_storage_bindings;
-#endif
 
   CLP(GraphicsStateGuardian) *_glgsg;
 
   bool _uses_standard_vertex_arrays;
 
-  typedef pmap<const InternalName *, GLint> LocationMap;
-
   void report_shader_errors(const Module &module, bool fatal);
   void report_program_errors(GLuint program, bool fatal);
   bool attach_shader(const ShaderModule *module, Shader::ModuleSpecConstants &spec_consts,
-                     const LocationMap &locations, bool &needs_query_locations);
-  bool compile_and_link(const LocationMap &locations, bool &needs_query_locations);
+                     const LocationMap &locations, bool &remap_locations,
+                     const LocationMap &ssbo_bindings);
+  bool compile_and_link(const LocationMap &locations, bool &remap_locations,
+                        const LocationMap &ssbo_bindings);
   void release_resources();
 
 public:

+ 12 - 8
panda/src/gobj/shader.cxx

@@ -78,7 +78,7 @@ Shader::
  * parameter.  Always returns false.
  */
 bool Shader::
-report_parameter_error(const InternalName *name, const ::ShaderType *type, const char *msg) {
+report_parameter_error(const InternalName *name, const ShaderType *type, const char *msg) {
   Filename fn = get_filename();
   shader_cat.error()
     << fn << ": " << *type << ' ' << *name << ": " << msg << "\n";
@@ -530,13 +530,17 @@ link() {
 }
 
 void Shader::
-add_parameter(const InternalName *name, const ::ShaderType *type, int location) {
-  Shader::Parameter param;
-  param._name = name;
-  param._type = type;
-  param._binding = ShaderInputBinding::make(_language, name, type);
-  param._location = location;
-  _parameters.push_back(std::move(param));
+add_parameter(const InternalName *name, const ShaderType *type, int location) {
+  {
+    Shader::Parameter param;
+    param._name = name;
+    param._type = type;
+    param._binding = ShaderInputBinding::make(_language, name, type);
+    param._location = location;
+    _parameters.push_back(std::move(param));
+  }
+
+  Shader::Parameter &param = _parameters.back();
   if (param._binding != nullptr) {
     param._binding->setup(this);
   } else {

+ 9 - 0
panda/src/gobj/shaderInputBinding.cxx

@@ -86,6 +86,15 @@ fetch_texture_image(const State &state, ResourceId resource_id, ShaderType::Acce
   return nullptr;
 }
 
+/**
+ * Fetches the shader buffer associated with the given resource identifier,
+ * which was previously returned by get_resource_id.
+ */
+PT(ShaderBuffer) ShaderInputBinding::
+fetch_shader_buffer(const State &state, ResourceId resource_id) const {
+  return nullptr;
+}
+
 /**
  * Registers a factory function to create a binding.
  */

+ 4 - 0
panda/src/gobj/shaderInputBinding.h

@@ -19,6 +19,8 @@
 #include "internalName.h"
 #include "shaderEnums.h"
 #include "small_vector.h"
+#include "texture.h"
+#include "shaderBuffer.h"
 
 class GraphicsStateGuardian;
 class LMatrix4f;
@@ -63,6 +65,8 @@ public:
                                           ResourceId resource_id,
                                           ShaderType::Access &access,
                                           int &z, int &n) const;
+  virtual PT(ShaderBuffer) fetch_shader_buffer(const State &state,
+                                               ResourceId resource_id) const;
 
   // All the binders are defined in display, we provide this mechanism so that
   // we don't get a dependency on display here.

+ 56 - 0
panda/src/gobj/shaderType.cxx

@@ -51,6 +51,15 @@ unwrap_array(const ShaderType *&element_type, uint32_t &num_elements) const {
   return false;
 }
 
+/**
+ * Returns a new type with all occurrences of the given type recursively
+ * replaced with the second type.
+ */
+const ShaderType *ShaderType::
+replace_type(const ShaderType *a, const ShaderType *b) const {
+  return (this == a) ? b : this;
+}
+
 /**
  *
  */
@@ -582,6 +591,36 @@ replace_scalar_type(ScalarType a, ScalarType b) const {
   }
 }
 
+/**
+ * Returns a new type with all occurrences of the given type recursively
+ * replaced with the second type.
+ */
+const ShaderType *ShaderType::Struct::
+replace_type(const ShaderType *a, const ShaderType *b) const {
+  if (this == a) {
+    return b;
+  }
+  bool any_changed = false;
+  bool recompute_offsets = a->get_size_bytes() != b->get_size_bytes();
+  ShaderType::Struct copy;
+  for (const Member &member : _members) {
+    const ShaderType *type = member.type->replace_type(a, b);
+    if (type != member.type) {
+      any_changed = true;
+    }
+    if (recompute_offsets) {
+      copy.add_member(type, member.name);
+    } else {
+      copy.add_member(type, member.name, member.offset);
+    }
+  }
+  if (any_changed) {
+    return ShaderType::register_type(std::move(copy));
+  } else {
+    return this;
+  }
+}
+
 /**
  *
  */
@@ -806,6 +845,23 @@ replace_scalar_type(ScalarType a, ScalarType b) const {
   }
 }
 
+/**
+ * Returns a new type with all occurrences of the given type recursively
+ * replaced with the second type.
+ */
+const ShaderType *ShaderType::Array::
+replace_type(const ShaderType *a, const ShaderType *b) const {
+  if (this == a) {
+    return b;
+  }
+  const ShaderType *element_type = _element_type->replace_type(a, b);
+  if (_element_type != element_type) {
+    return ShaderType::register_type(ShaderType::Array(element_type, _num_elements));
+  } else {
+    return this;
+  }
+}
+
 /**
  *
  */

+ 3 - 0
panda/src/gobj/shaderType.h

@@ -99,6 +99,7 @@ public:
                               uint32_t &num_rows,
                               uint32_t &num_columns) const { return false; }
   virtual const ShaderType *replace_scalar_type(ScalarType a, ScalarType b) const { return this; }
+  virtual const ShaderType *replace_type(const ShaderType *a, const ShaderType *b) const;
 
   INLINE static constexpr uint32_t get_scalar_size_bytes(ScalarType scalar_type);
 
@@ -343,6 +344,7 @@ public:
   virtual bool contains_opaque_type() const override;
   virtual bool contains_scalar_type(ScalarType type) const override;
   virtual const ShaderType *replace_scalar_type(ScalarType a, ScalarType b) const override;
+  virtual const ShaderType *replace_type(const ShaderType *a, const ShaderType *b) const override;
   const Struct *as_struct() const override { return this; }
 
 PUBLISHED:
@@ -393,6 +395,7 @@ public:
   virtual bool as_scalar_type(ScalarType &type, uint32_t &num_elements,
                               uint32_t &num_rows, uint32_t &num_columns) const override;
   virtual const ShaderType *replace_scalar_type(ScalarType a, ScalarType b) const override;
+  virtual const ShaderType *replace_type(const ShaderType *a, const ShaderType *b) const override;
 
   virtual void output(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;

+ 14 - 4
panda/src/shaderpipeline/shaderModuleSpirV.cxx

@@ -148,6 +148,11 @@ ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words, BamCacheRecord *reco
   // Add in location decorations for any inputs that are missing it.
   transformer.assign_locations(stage);
 
+  // Get rid of uniform locations and bindings.  The numbering rules are
+  // different for each back-end, so we regenerate these later.
+  transformer.strip_uniform_locations();
+  transformer.strip_bindings();
+
   // Identify the inputs, outputs and uniform parameters.
   for (uint32_t id = 0; id < transformer.get_id_bound(); ++id) {
     const Definition &def = db.get_definition(id);
@@ -246,11 +251,16 @@ ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words, BamCacheRecord *reco
         // name of the struct type.
         const Definition &type_pointer_def = db.get_definition(def._type_id);
         nassertd(type_pointer_def.is_pointer_type()) continue;
-        const Definition &type_def = db.get_definition(type_pointer_def._type_id);
-        nassertd(type_def.is_type()) continue;
-        nassertd(!type_def._name.empty()) continue;
 
-        var.name = InternalName::make(type_def._name);
+        // This may be an array of structs.  Unwrap any array layers.
+        const Definition *type_def = &db.get_definition(type_pointer_def._type_id);
+        while (type_def->_type->as_array() != nullptr) {
+          type_def = &db.get_definition(type_def->_type_id);
+        }
+        nassertd(type_def->is_type()) continue;
+        nassertd(!type_def->_name.empty()) continue;
+
+        var.name = InternalName::make(type_def->_name);
         _parameters.push_back(std::move(var));
 
         _used_caps |= C_storage_buffer;

+ 71 - 30
panda/src/shaderpipeline/spirVResultDatabase.cxx

@@ -604,6 +604,10 @@ parse_instruction(spv::Op opcode, const uint32_t *args, uint32_t nargs, uint32_t
       _defs[args[0]]._flags |= DF_buffer_block;
       break;
 
+    case spv::DecorationBlock:
+      _defs[args[0]]._flags |= DF_block;
+      break;
+
     case spv::DecorationBuiltIn:
       _defs[args[0]]._builtin = (spv::BuiltIn)args[2];
       break;
@@ -945,9 +949,43 @@ record_pointer_type(uint32_t id, spv::StorageClass storage_class, uint32_t type_
 
   const Definition &type_def = get_definition(type_id);
   nassertv(type_def._dtype == DT_type || type_def._dtype == DT_pointer_type);
+  const ShaderType *type = type_def._type;
+
+  // Now that we know the access type, we can determine whether a struct is
+  // actually a uniform block or storage block.
+  if (storage_class == spv::StorageClassUniform ||
+      storage_class == spv::StorageClassStorageBuffer) {
+    // Unwrap arrays to get to the underlying block type.
+    uint32_t block_type_id = type_id;
+    while (_defs[block_type_id]._type->as_array() != nullptr) {
+      block_type_id = _defs[block_type_id]._type_id;
+    }
+
+    const Definition &block_type_def = get_definition(block_type_id);
+    if (block_type_def._type->as_struct() != nullptr) {
+      // Check if this was pointing to an SSBO.
+      if (block_type_def._flags & (DF_buffer_block | DF_block)) {
+        if (block_type_def._flags & DF_buffer_block) {
+          // Map old SPIR-V way of declaring SSBO to new way.
+          storage_class = spv::StorageClassStorageBuffer;
+        }
+        if (storage_class == spv::StorageClassStorageBuffer) {
+          ShaderType::Access access = ShaderType::Access::read_write;
+          if (block_type_def._flags & DF_non_writable) {
+            access = (access & ShaderType::Access::read_only);
+          }
+          if (block_type_def._flags & DF_non_readable) {
+            access = (access & ShaderType::Access::write_only);
+          }
+          const ShaderType *new_type = ShaderType::register_type(ShaderType::StorageBuffer(block_type_def._type, access));
+          type = type->replace_type(block_type_def._type, new_type);
+        }
+      }
+    }
+  }
 
   def._dtype = DT_pointer_type;
-  def._type = type_def._type;
+  def._type = type;
   def._storage_class = storage_class;
   def._type_id = type_id;
 }
@@ -975,44 +1013,30 @@ record_variable(uint32_t id, uint32_t pointer_type_id, spv::StorageClass storage
     return;
   }
 
-  // In older versions of SPIR-V, an SSBO was defined using BufferBlock.
+  // If we've determined that the pointer was supposed to be StorageBuffer
+  // instead of Uniform, change that here as well.
   if (storage_class == spv::StorageClassUniform &&
-      (type_def._flags & DF_buffer_block) != 0) {
+      pointer_type_def._storage_class == spv::StorageClassStorageBuffer) {
     storage_class = spv::StorageClassStorageBuffer;
   }
 
   def._dtype = DT_variable;
-  def._type = type_def._type;
+  def._type = pointer_type_def._type;
   def._type_id = pointer_type_id;
   def._storage_class = storage_class;
   def._origin_id = id;
   def._function_id = function_id;
 
-  if (storage_class == spv::StorageClassStorageBuffer) {
-    // Inherit readonly/writeonly from the variable but also from the struct.
-    int flags = def._flags | type_def._flags;
-    ShaderType::Access access = ShaderType::Access::read_write;
-    if (flags & DF_non_writable) {
-      access = (access & ShaderType::Access::read_only);
+  if (def._flags & (DF_non_writable | DF_non_readable)) {
+    // If an image variable, SSBO or array thereof has the readonly/writeonly
+    // qualifiers, then we'll inject those back into the type.
+    const ShaderType *unwrapped_type = def._type;
+    while (const ShaderType::Array *array_type = unwrapped_type->as_array()) {
+      unwrapped_type = array_type->get_element_type();
     }
-    if (flags & DF_non_readable) {
-      access = (access & ShaderType::Access::write_only);
-    }
-    def._type = ShaderType::register_type(ShaderType::StorageBuffer(def._type, access));
 
-    if (shader_cat.is_debug()) {
-      std::ostream &out = shader_cat.debug()
-        << "Defined buffer " << id;
-      if (!def._name.empty()) {
-        out << ": " << def._name;
-      }
-      out << " with type " << *def._type << "\n";
-    }
-  }
-  else if (def._flags & (DF_non_writable | DF_non_readable)) {
-    // If an image variable has the readonly/writeonly qualifiers, then we'll
-    // inject those back into the type.
-    if (const ShaderType::Image *image = def._type->as_image()) {
+    const ShaderType *new_type = unwrapped_type;
+    if (const ShaderType::Image *image = unwrapped_type->as_image()) {
       ShaderType::Access access = image->get_access();
       if (def._flags & DF_non_writable) {
         access = (access & ShaderType::Access::read_only);
@@ -1021,18 +1045,35 @@ record_variable(uint32_t id, uint32_t pointer_type_id, spv::StorageClass storage
         access = (access & ShaderType::Access::write_only);
       }
       if (access != image->get_access()) {
-        def._type = ShaderType::register_type(ShaderType::Image(
+        new_type = ShaderType::register_type(ShaderType::Image(
           image->get_texture_type(),
           image->get_sampled_type(),
           access));
       }
     }
+    else if (const ShaderType::StorageBuffer *buffer = unwrapped_type->as_storage_buffer()) {
+      ShaderType::Access access = buffer->get_access();
+      if (def._flags & DF_non_writable) {
+        access = (access & ShaderType::Access::read_only);
+      }
+      if (def._flags & DF_non_readable) {
+        access = (access & ShaderType::Access::write_only);
+      }
+      if (access != buffer->get_access()) {
+        new_type = ShaderType::register_type(ShaderType::StorageBuffer(
+          buffer->get_contained_type(), access));
+      }
+    }
+
+    if (new_type != unwrapped_type) {
+      def._type = def._type->replace_type(unwrapped_type, new_type);
+    }
   }
 
 #ifndef NDEBUG
-  if (storage_class == spv::StorageClassUniformConstant && shader_cat.is_debug()) {
+  if (shader_cat.is_debug()) {
     std::ostream &out = shader_cat.debug()
-      << "Defined uniform " << id;
+      << "Defined variable " << id;
 
     if (!def._name.empty()) {
       out << ": " << def._name;

+ 7 - 5
panda/src/shaderpipeline/spirVResultDatabase.h

@@ -38,16 +38,18 @@ private:
     // Set for arrays that are indexed with a non-const index.
     DF_dynamically_indexed = 32,
 
-    // Has the "buffer block" decoration (older versions of SPIR-V).
+    // Has the "buffer block" decoration (older versions of SPIR-V),
+    // or the "block" decoration on newer versions.
     DF_buffer_block = 64,
+    DF_block = 128,
 
     // If both of these are set, no access is permitted (size queries only)
-    DF_non_writable = 128, // readonly
-    DF_non_readable = 256, // writeonly
+    DF_non_writable = 256, // readonly
+    DF_non_readable = 512, // writeonly
 
-    DF_relaxed_precision = 512,
+    DF_relaxed_precision = 1024,
 
-    DF_null_constant = 1024,
+    DF_null_constant = 2048,
   };
 
 public:

+ 81 - 0
panda/src/shaderpipeline/spirVTransformer.cxx

@@ -279,6 +279,87 @@ assign_locations(pmap<uint32_t, int> remap) {
   }
 }
 
+/**
+ * Assigns procedural names consisting of a prefix followed by an index.
+ */
+void SpirVTransformer::
+assign_procedural_names(const char *prefix, const pmap<uint32_t, int> &suffixes) {
+  // Remove existing names matching theses ids.
+  auto it = _preamble.begin() + 5;
+  while (it != _preamble.end()) {
+    spv::Op opcode = (spv::Op)(*it & spv::OpCodeMask);
+    uint32_t wcount = *it >> spv::WordCountShift;
+    nassertd(wcount > 0) break;
+
+    if (opcode == spv::OpName && wcount >= 2) {
+      if (suffixes.count(*(it + 1))) {
+        it = _preamble.erase(it, it + wcount);
+        continue;
+      }
+    }
+
+    std::advance(it, wcount);
+  }
+
+  // Insert names before the annotations block.
+  uint32_t *words = (uint32_t *)alloca(sizeof(uint32_t) + strlen(prefix) + 32);
+  for (auto it = suffixes.begin(); it != suffixes.end(); ++it) {
+    words[1] = it->first;
+    int len = sprintf((char *)(words + 2), "%s%d", prefix, it->second);
+    uint32_t num_words = len / 4 + 3;
+    words[0] = spv::OpName | (num_words << spv::WordCountShift);
+    _preamble.insert(_preamble.end(), words, words + num_words);
+  }
+}
+
+/**
+ * Removes location decorations from uniforms.
+ */
+void SpirVTransformer::
+strip_uniform_locations() {
+  auto it = _annotations.begin();
+  while (it != _annotations.end()) {
+    spv::Op opcode = (spv::Op)(*it & spv::OpCodeMask);
+    uint32_t wcount = *it >> spv::WordCountShift;
+    nassertd(wcount > 0) break;
+
+    if (opcode == spv::OpDecorate && wcount >= 3 && *(it + 2) == spv::DecorationLocation) {
+      Definition &def = _db.modify_definition(*(it + 1));
+      if (def._storage_class == spv::StorageClassUniformConstant) {
+        it = _annotations.erase(it, it + wcount);
+        def._location = -1;
+        continue;
+      }
+    }
+
+    std::advance(it, wcount);
+  }
+}
+
+/**
+ * Removes all binding and descriptor set decorations.
+ */
+void SpirVTransformer::
+strip_bindings() {
+  auto it = _annotations.begin();
+  while (it != _annotations.end()) {
+    spv::Op opcode = (spv::Op)(*it & spv::OpCodeMask);
+    uint32_t wcount = *it >> spv::WordCountShift;
+    nassertd(wcount > 0) break;
+
+    if (opcode == spv::OpDecorate && wcount >= 3) {
+      spv::Decoration decoration = (spv::Decoration)*(it + 2);
+      if (decoration == spv::DecorationBinding ||
+          decoration == spv::DecorationDescriptorSet) {
+        it = _annotations.erase(it, it + wcount);
+        continue;
+      }
+    }
+
+    std::advance(it, wcount);
+  }
+}
+
 /**
  * Assign descriptor bindings for a descriptor set based on the given ids.
  * To create gaps in the descriptor set, entries in ids may be 0.

+ 3 - 0
panda/src/shaderpipeline/spirVTransformer.h

@@ -44,6 +44,9 @@ public:
 
   void assign_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();
+  void strip_bindings();
   void bind_descriptor_set(uint32_t set, const pvector<uint32_t> &ids);
 
 private:

+ 26 - 2
tests/display/test_glsl_shader.py

@@ -435,7 +435,6 @@ def test_glsl_uimage(gsg):
 
 
 def test_glsl_ssbo(gsg):
-    return
     from struct import pack
     num1 = pack('<i', 1234567)
     num2 = pack('<i', -1234567)
@@ -466,8 +465,33 @@ def test_glsl_ssbo(gsg):
                   version=430)
 
 
+def test_glsl_ssbo_array(gsg):
+    from struct import pack
+    dummy = pack('<i', 999999)
+    num1 = pack('<i', 1234567)
+    num2 = pack('<i', -1234567)
+    unused = core.ShaderBuffer("unused", dummy, core.GeomEnums.UH_static)
+    buffer1 = core.ShaderBuffer("buffer1", num1, core.GeomEnums.UH_static)
+    buffer2 = core.ShaderBuffer("buffer2", num2, core.GeomEnums.UH_static)
+
+    preamble = """
+    struct InsideStruct {
+        int value;
+    };
+    layout(std430, binding=0) buffer test {
+        readonly InsideStruct inside[1];
+    } test_ns[3][1];
+    """
+    code = """
+    assert(test_ns[1][0].inside[0].value == 1234567);
+    assert(test_ns[2][0].inside[0].value == -1234567);
+    """
+    run_glsl_test(gsg, code, preamble,
+                  {'test[0][0]': unused, 'test[1][0]': buffer1, 'test[2][0]': buffer2},
+                  version=430)
+
+
 def test_glsl_ssbo_runtime_length(gsg):
-    return
     from struct import pack
     nums = pack('<ii', 1234, 5678)
     ssbo = core.ShaderBuffer("ssbo", nums, core.GeomEnums.UH_static)