shaderpipeline: Assorted bug fixes

panda/src/glstuff/glGraphicsStateGuardian_src.cxx

@@ -8672,14 +8672,19 @@ do_issue_shader() {
         // If it's a different type of shader, make sure to unbind the old.
         _current_shader_context->unbind();
       }
-      context->bind(alpha_test_mode);
+      if (!context->bind(alpha_test_mode)) {
+        shader = nullptr;
+        context = nullptr;
+      }
       _current_shader = shader;
     }
 
-    context->set_display_region(_current_display_region);
+    if (context != nullptr) {
+      context->set_display_region(_current_display_region);
 
-    // Bind the shader storage buffers.
-    context->update_shader_buffer_bindings(_current_shader_context);
+      // Bind the shader storage buffers.
+      context->update_shader_buffer_bindings(_current_shader_context);
+    }
     _current_shader_context = context;
   }
 

panda/src/glstuff/glShaderContext_src.cxx

@@ -161,7 +161,7 @@ CLP(ShaderContext)(CLP(GraphicsStateGuardian) *glgsg, Shader *s) : ShaderContext
   GLuint program = 0;
   if (!_inject_alpha_test) {
     program = _glgsg->_glCreateProgram();
-    _programs[0] = program;
+    _program = program;
   }
 
   // Compile all the modules now, except for the fragment module if we will be
@@ -208,14 +208,14 @@ valid() {
   if (_shader->get_error_flag()) {
     return false;
   }
-  return true;//(_glsl_program != 0);
+  return _program != 0 || _inject_alpha_test;
 }
 
 /**
  * This function is to be called to enable a new shader.  It also initializes
  * all of the shader's input parameters.
  */
-void CLP(ShaderContext)::
+bool CLP(ShaderContext)::
 bind(RenderAttrib::PandaCompareFunc alpha_test_mode) {
   /*if (!_validated) {
     _glgsg->_glValidateProgram(_glsl_program);
@@ -227,13 +227,17 @@ bind(RenderAttrib::PandaCompareFunc alpha_test_mode) {
     alpha_test_mode = RenderAttrib::M_none;
   }
 
-  GLuint program = _programs[alpha_test_mode];
+  GLuint program = _linked_programs[alpha_test_mode];
   if (program != 0) {
     if (!_shader->get_error_flag()) {
       _glgsg->_glUseProgram(program);
+    } else {
+      return false;
     }
   } else {
-    compile_for(alpha_test_mode);
+    if (!compile_for(alpha_test_mode)) {
+      return false;
+    }
   }
 
   _alpha_test_mode = alpha_test_mode;
@@ -245,6 +249,7 @@ bind(RenderAttrib::PandaCompareFunc alpha_test_mode) {
   }
 
   _glgsg->report_my_gl_errors();
+  return true;
 }
 
 /**
@@ -280,7 +285,7 @@ compile_for(RenderAttrib::PandaCompareFunc alpha_test_mode) {
     _shader->_error_flag = true;
     return false;
   }
-  _programs[alpha_test_mode] = program;
+  _linked_programs[alpha_test_mode] = program;
   _alpha_test_ref_locations[alpha_test_mode] = -1;
 
   // Bind the program, so that we can call glUniform1i for the textures.
@@ -965,10 +970,16 @@ reflect_program(GLuint program, SparseArray &active_locations) {
 
   struct StackItem {
     std::string name;
-    GLint loc;
-    GLint next_loc;
     int num_elements; // 0 means not an array
     ShaderType::Struct type;
+
+    const ShaderType *make_type() const {
+      const ShaderType *new_type = ShaderType::register_type(std::move(type));
+      if (num_elements > 0) {
+        new_type = ShaderType::register_type(ShaderType::Array(new_type, num_elements));
+      }
+      return new_type;
+    }
   };
 
   pdeque<StackItem> struct_stack;
@@ -999,45 +1010,28 @@ reflect_program(GLuint program, SparseArray &active_locations) {
       if (sizes[i] > struct_stack[i].num_elements) {
         struct_stack[i].num_elements = sizes[i];
       }
-      if (sizes[i] > 1) {
-        // Ignore all but the first struct element for determining the type.
-        skip = true;
-      }
       ++i;
     }
-    if (skip) {
-      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();
-      const ShaderType *type = ShaderType::register_type(std::move(item.type));
-      if (item.num_elements > 0) {
-        type = ShaderType::register_type(ShaderType::Array(type, item.num_elements));
-      }
+      const ShaderType *type = item.make_type();
 
       if (struct_stack.empty()) {
-        // Add struct as top-level
+        // Don't record location, the driver may omit members of individual
+        // structs in a struct array, we'll re-query them later
         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) {
+        _shader->add_parameter(std::move(name), type);
+      } else {
         // Add as nested struct member
-        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;
+        struct_stack.back().type.merge_member_by_name(item.name, type);
       }
     }
     // 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, loc, sizes[struct_stack.size()], {}});
+      struct_stack.push_back({parts[struct_stack.size()], sizes[struct_stack.size()], {}});
     }
 
     const ShaderType *type = get_param_type(param.type);
@@ -1046,50 +1040,38 @@ reflect_program(GLuint program, SparseArray &active_locations) {
     }
 
     if (struct_stack.empty()) {
-      // Add as top-level.
+      // Add as top-level.  Recording the location here saves a
+      // glGetUniformLocation call later on.
       assert(parts.size() == 1);
       CPT(InternalName) name = InternalName::make(parts[0]);
-      //_locations[name] = loc;
+      _locations[name] = loc;
       _shader->add_parameter(std::move(name), type, loc);
-    }
-    else if (struct_stack.back().num_elements <= 1) {
+    } else {
       // Add as struct member
       assert(parts.size() > 1);
-      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;
+      struct_stack.back().type.merge_member_by_name(parts.back(), type);
     }
   }
 
   while (!struct_stack.empty()) {
     StackItem item = std::move(struct_stack.back());
     struct_stack.pop_back();
-    const ShaderType *type = ShaderType::register_type(std::move(item.type));
-    if (item.num_elements > 0) {
-      type = ShaderType::register_type(ShaderType::Array(type, item.num_elements));
-    }
+    const ShaderType *type = item.make_type();
 
     if (struct_stack.empty()) {
-      // Add struct as top-level
+      // Add struct as top-level.
+      // Don't record location, the driver may omit members of individual
+      // structs in a struct array, we'll re-query them later
       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) {
+      _shader->add_parameter(std::move(name), type);
+    } else {
       // Add as nested struct member
-      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;
+      struct_stack.back().type.merge_member_by_name(item.name, type);
     }
   }
+
+  _matrix_cache = pvector<LMatrix4>(_shader->_matrix_cache_desc.size(), LMatrix4::ident_mat());
+  _matrix_cache_deps = _shader->_matrix_cache_deps;
 }
 
 /**
@@ -1729,15 +1711,15 @@ release_resources() {
   if (!_glgsg) {
     return;
   }
-  if (_programs[0] != 0) {
-    _glgsg->_glDeleteProgram(_programs[0]);
-    _programs[0] = 0;
+  if (_program != 0) {
+    _glgsg->_glDeleteProgram(_program);
+    _program = 0;
   }
   if (_inject_alpha_test) {
-    for (int i = 1; i < RenderAttrib::M_always; ++i) {
-      if (_programs[i] != 0) {
-        _glgsg->_glDeleteProgram(_programs[i]);
-        _programs[i] = 0;
+    for (int i = 0; i < RenderAttrib::M_always; ++i) {
+      if (_linked_programs[i] != 0) {
+        _glgsg->_glDeleteProgram(_linked_programs[i]);
+        _linked_programs[i] = 0;
       }
     }
   }
@@ -2922,14 +2904,14 @@ create_shader(GLuint program, const ShaderModule *module,
  */
 GLuint CLP(ShaderContext)::
 compile_and_link(RenderAttrib::PandaCompareFunc alpha_test_mode) {
-  GLuint program;
-  if (_inject_alpha_test) {
-    program = _glgsg->_glCreateProgram();
-  } else {
-    program = _programs[0];
-  }
+  GLuint program = _program;
   if (program == 0) {
-    return 0;
+    program = _glgsg->_glCreateProgram();
+    if (UNLIKELY(program == 0)) {
+      GLCAT.error()
+        << "Failed to create program object\n";
+      return 0;
+    }
   }
 
   if (_glgsg->_use_object_labels) {

panda/src/glstuff/glShaderContext_src.h

@@ -53,7 +53,7 @@ public:
   ALLOC_DELETED_CHAIN(CLP(ShaderContext));
 
   bool valid(void);
-  void bind(RenderAttrib::PandaCompareFunc alpha_test_mode);
+  bool bind(RenderAttrib::PandaCompareFunc alpha_test_mode);
   void unbind();
 
   bool compile_for(RenderAttrib::PandaCompareFunc alpha_test_mode);
@@ -102,7 +102,8 @@ private:
 private:
   bool _validated = false;
   bool _inject_alpha_test = false;
-  GLuint _programs[RenderAttrib::M_always] {0u};
+  GLuint _program = 0;
+  GLuint _linked_programs[RenderAttrib::M_always] {0u};
   RenderAttrib::PandaCompareFunc _alpha_test_mode = RenderAttrib::M_none;
   GLint _alpha_test_ref_locations[RenderAttrib::M_always];
 

panda/src/gobj/shaderType.I

@@ -154,6 +154,19 @@ get_member(size_t i) const {
   return _members[i];
 }
 
+/**
+ * Returns true if this struct has a member with the given name.
+ */
+INLINE bool ShaderType::Struct::
+has_member(const std::string &name) const {
+  for (size_t i = 0; i < _members.size(); ++i) {
+    if (_members[i].name == name) {
+      return true;
+    }
+  }
+  return false;
+}
+
 /**
  * Constructs an array type from a base type and number of elements.
  */

panda/src/gobj/shaderType.cxx

@@ -60,6 +60,14 @@ replace_type(const ShaderType *a, const ShaderType *b) const {
   return (this == a) ? b : this;
 }
 
+/**
+ * Returns a new type that can contain both this type and the other type.
+ */
+const ShaderType *ShaderType::
+merge(const ShaderType *other) const {
+  return (this == other) ? this : nullptr;
+}
+
 /**
  *
  */
@@ -316,6 +324,30 @@ replace_scalar_type(ScalarType a, ScalarType b) const {
   }
 }
 
+/**
+ * Returns a new type that can contain both this type and the other type.
+ */
+const ShaderType *ShaderType::Vector::
+merge(const ShaderType *other) const {
+  if (this == other) {
+    return this;
+  }
+  const ShaderType::Vector *other_vec = other->as_vector();
+  if (other_vec == nullptr) {
+    return nullptr;
+  }
+
+  if (_scalar_type != other_vec->_scalar_type) {
+    return nullptr;
+  }
+
+  if (other_vec->_num_components > _num_components) {
+    return other;
+  } else {
+    return this;
+  }
+}
+
 /**
  * Returns the number of in/out locations taken up by in/out variables having
  * this type.
@@ -542,6 +574,52 @@ add_member(const ShaderType *type, std::string name, uint32_t offset) {
   _members.insert(it, std::move(member));
 }
 
+/**
+ * If a member with the given name already exists, merges the types.
+ * Otherwise, simply adds it.
+ */
+void ShaderType::Struct::
+merge_member_by_name(std::string name, const ShaderType *type) {
+  bool found = false;
+  uint32_t min_offset = 0;
+
+  for (Member &member : _members) {
+    if (found) {
+      if (member.offset > min_offset) {
+        member.offset = min_offset;
+        uint32_t alignment = type->get_align_bytes();
+        if (alignment > 0) {
+          member.offset += alignment - ((member.offset + (alignment - 1)) % alignment) - 1;
+        }
+      } else {
+        return;
+      }
+    }
+    else if (member.name == name) {
+      if (member.type == type) {
+        return;
+      }
+      const ShaderType *merged_type = member.type->merge(type);
+      if (merged_type == type) {
+        return;
+      }
+      uint32_t new_size = merged_type->get_size_bytes();
+      uint32_t alignment = merged_type->get_align_bytes();
+      if (alignment > 0) {
+        member.offset += alignment - ((member.offset + (alignment - 1)) % alignment) - 1;
+      }
+      member.type = merged_type;
+      // Shift the rest down.
+      min_offset = member.offset + new_size;
+      found = true;
+    }
+  }
+
+  if (!found) {
+    add_member(type, name);
+  }
+}
+
 /**
  * Returns true if this type is or contains any opaque type.
  */
@@ -621,6 +699,61 @@ replace_type(const ShaderType *a, const ShaderType *b) const {
   }
 }
 
+/**
+ * Returns a new type that can contain both this type and the other type.
+ */
+const ShaderType *ShaderType::Struct::
+merge(const ShaderType *other) const {
+  if (this == other) {
+    return this;
+  }
+  const ShaderType::Struct *other_struct = other->as_struct();
+  if (other_struct == nullptr) {
+    return nullptr;
+  }
+  const auto &other_members = other_struct->_members;
+
+  ShaderType::Struct new_type;
+  size_t ti = 0;
+  size_t oi = 0;
+  while (ti < _members.size() && oi < other_members.size()) {
+    const Member &this_member = _members[ti];
+    const Member &other_member = other_members[oi];
+
+    if (this_member.name == other_member.name) {
+      const ShaderType *merged = this_member.type->merge(other_member.type);
+      if (merged == nullptr) {
+        return nullptr;
+      }
+      new_type.add_member(merged, this_member.name);
+      ++ti;
+      ++oi;
+      continue;
+    }
+
+    if (!has_member(other_member.name)) {
+      new_type.add_member(other_member.type, other_member.name);
+      ++oi;
+    } else {
+      new_type.add_member(this_member.type, this_member.name);
+      ++ti;
+    }
+  }
+
+  while (ti < _members.size()) {
+    const Member &this_member = _members[ti];
+    new_type.merge_member_by_name(this_member.name, this_member.type);
+    ++ti;
+  }
+  while (oi < other_members.size()) {
+    const Member &other_member = other_members[oi];
+    new_type.merge_member_by_name(other_member.name, other_member.type);
+    ++oi;
+  }
+
+  return ShaderType::register_type(std::move(new_type));
+}
+
 /**
  *
  */
@@ -849,6 +982,30 @@ replace_type(const ShaderType *a, const ShaderType *b) const {
   }
 }
 
+/**
+ * Returns a new type that can contain both this type and the other type.
+ */
+const ShaderType *ShaderType::Array::
+merge(const ShaderType *other) const {
+  if (this == other) {
+    return this;
+  }
+  const ShaderType::Array *other_array = other->as_array();
+  if (other_array == nullptr) {
+    return nullptr;
+  }
+
+  if (other_array->_element_type == _element_type) {
+    return (other_array->_num_elements > _num_elements) ? other_array : this;
+  }
+
+  const ShaderType *merged = _element_type->merge(other_array->_element_type);
+  if (merged == nullptr) {
+    return nullptr;
+  }
+  return ShaderType::register_type(ShaderType::Array(merged, std::max(_num_elements, other_array->_num_elements)));
+}
+
 /**
  *
  */

panda/src/gobj/shaderType.h

@@ -99,6 +99,7 @@ public:
                               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;
+  virtual const ShaderType *merge(const ShaderType *other) const;
 
   INLINE static constexpr uint32_t get_scalar_size_bytes(ScalarType scalar_type);
 
@@ -234,6 +235,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 *merge(const ShaderType *other) const override;
 
   virtual int get_num_interface_locations() const override;
 
@@ -327,9 +329,12 @@ public:
 
   INLINE size_t get_num_members() const;
   INLINE const Member &get_member(size_t i) const;
+  INLINE bool has_member(const std::string &name) const;
   void add_member(const ShaderType *type, std::string name);
   void add_member(const ShaderType *type, std::string name, uint32_t offset);
 
+  void merge_member_by_name(std::string name, const ShaderType *type);
+
   virtual void output(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;
 
@@ -343,6 +348,7 @@ public:
   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;
+  virtual const ShaderType *merge(const ShaderType *other) const override;
   const Struct *as_struct() const override { return this; }
 
 PUBLISHED:
@@ -394,6 +400,7 @@ public:
                               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 const ShaderType *merge(const ShaderType *other) const override;
 
   virtual void output(std::ostream &out) const override;
   virtual int compare_to_impl(const ShaderType &other) const override;

panda/src/shaderpipeline/spirVInjectAlphaTestPass.cxx

@@ -18,10 +18,10 @@
  * Return true to keep the instruction, false to omit it.
  */
 bool SpirVInjectAlphaTestPass::
-transform_entry_point(spv::ExecutionModel model, uint32_t id, const char *name, const uint32_t *interface, uint16_t size) {
+transform_entry_point(spv::ExecutionModel model, uint32_t id, const char *name, const uint32_t *var_ids, uint16_t num_vars) {
   if (model == spv::ExecutionModelFragment) {
-    for (size_t i = 0; i < size; ++i) {
-      uint32_t var_id = interface[i];
+    for (size_t i = 0; i < num_vars; ++i) {
+      uint32_t var_id = var_ids[i];
       if (_db.get_definition(var_id)._location == 0) {
         _entry_points[id] = var_id;
         break;

panda/src/shaderpipeline/spirVInjectAlphaTestPass.h

@@ -35,7 +35,7 @@ public:
 
   SpirVInjectAlphaTestPass(Mode mode, int location = -1) : _mode(mode), _location(location) {}
 
-  virtual bool transform_entry_point(spv::ExecutionModel model, uint32_t id, const char *name, const uint32_t *interface, uint16_t size);
+  virtual bool transform_entry_point(spv::ExecutionModel model, uint32_t id, const char *name, const uint32_t *var_ids, uint16_t num_vars);
   virtual bool begin_function(Instruction op);
   virtual bool transform_function_op(Instruction op);
   virtual void end_function(uint32_t function_id);

panda/src/shaderpipeline/spirVTransformPass.cxx

@@ -201,7 +201,7 @@ preprocess() {
  * Return true to keep the instruction, false to omit it.
  */
 bool SpirVTransformPass::
-transform_entry_point(spv::ExecutionModel model, uint32_t id, const char *name, const uint32_t *interface, uint16_t size) {
+transform_entry_point(spv::ExecutionModel model, uint32_t id, const char *name, const uint32_t *var_ids, uint16_t num_vars) {
   return true;
 }
 

panda/src/shaderpipeline/spirVTransformPass.h

@@ -39,7 +39,7 @@ public:
   void process_functions(std::vector<uint32_t> &instructions);
 
   virtual void preprocess();
-  virtual bool transform_entry_point(spv::ExecutionModel model, uint32_t id, const char *name, const uint32_t *interface, uint16_t size);
+  virtual bool transform_entry_point(spv::ExecutionModel model, uint32_t id, const char *name, const uint32_t *var_ids, uint16_t num_vars);
   virtual bool transform_debug_op(Instruction op);
   virtual bool transform_annotation_op(Instruction op);
   virtual bool transform_definition_op(Instruction op);

tests/display/conftest.py

@@ -579,7 +579,7 @@ def env(request):
         # disrupts the desktop as little as possible
         props.minimized = True
         props.foreground = False
-        props.z_order = WindowProperties.Z_bottom
+        props.z_order = core.WindowProperties.Z_bottom
         buffer = engine.make_output(
             pipe,
             'buffer',