glgsg: Prevent exceeding max supported vertex attrib stride

See also Moguri/panda3d-gltf#117

panda/src/gles2gsg/gles2gsg.h

@@ -171,6 +171,7 @@ typedef char GLchar;
 #define GL_RG32I 0x823B
 #define GL_RG32UI 0x823C
 #define GL_PROGRAM_BINARY_RETRIEVABLE_HINT 0x8257
+#define GL_MAX_VERTEX_ATTRIB_STRIDE 0x82E5
 #define GL_PROGRAM_BINARY_LENGTH 0x8741
 #define GL_NUM_PROGRAM_BINARY_FORMATS 0x87FE
 #define GL_PROGRAM_BINARY_FORMATS 0x87FF

panda/src/glstuff/glGeomMunger_src.cxx

@@ -220,6 +220,7 @@ munge_format_impl(const GeomVertexFormat *orig,
     // Combine the primary data columns into a single array.
     new_format = new GeomVertexFormat(*format);
     PT(GeomVertexArrayFormat) new_array_format = new GeomVertexArrayFormat;
+    size_t insert_at = 0;
 
     const GeomVertexColumn *column = format->get_vertex_column();
     if (column != nullptr) {
@@ -263,22 +264,34 @@ munge_format_impl(const GeomVertexFormat *orig,
             // This is the first time we've encountered this texcoord name.
             const GeomVertexColumn *texcoord_type = format->get_column(name);
 
+            // Note that we have to add something as a placeholder, even if the
+            // texture coordinates aren't defined.
+            int num_values = 2;
+            int column_alignment = 0;
+            int start = new_array_format->get_total_bytes();
+            start = (start + sizeof(PN_stdfloat) - 1) & ~(sizeof(PN_stdfloat) - 1);
             if (texcoord_type != nullptr) {
-              new_array_format->add_column
-                (name, texcoord_type->get_num_values(), NT_stdfloat, C_texcoord,
-                 -1, texcoord_type->get_column_alignment());
-            } else {
-              // We have to add something as a placeholder, even if the
-              // texture coordinates aren't defined.
-              new_array_format->add_column(name, 2, NT_stdfloat, C_texcoord);
+              column_alignment = texcoord_type->get_column_alignment();
+              num_values = texcoord_type->get_num_values();
             }
+            if (start + num_values * sizeof(PN_stdfloat) > glgsg->get_max_vertex_attrib_stride()) {
+              // We are exceeding the limit for stride reported by the driver.
+              // Start a new array.
+              new_format->insert_array(insert_at++, new_array_format);
+              new_array_format = new GeomVertexArrayFormat;
+              start = 0;
+            }
+            new_array_format->add_column(name, num_values, NT_stdfloat,
+                                         C_texcoord, start, column_alignment);
             new_format->remove_column(name);
           }
         }
       }
     }
 
-    new_format->insert_array(0, new_array_format);
+    if (new_array_format->get_num_columns() > 0) {
+      new_format->insert_array(insert_at, new_array_format);
+    }
     format = GeomVertexFormat::register_format(new_format);
   }
 
@@ -377,6 +390,7 @@ premunge_format_impl(const GeomVertexFormat *orig) {
     // of doing this step at load time than you might be at run time.
     new_format = new GeomVertexFormat(*format);
     PT(GeomVertexArrayFormat) new_array_format = new GeomVertexArrayFormat;
+    size_t insert_at = 0;
 
     const GeomVertexColumn *column = format->get_vertex_column();
     if (column != nullptr) {
@@ -421,15 +435,25 @@ premunge_format_impl(const GeomVertexFormat *orig) {
             // This is the first time we've encountered this texcoord name.
             const GeomVertexColumn *texcoord_type = format->get_column(name);
 
+            // Note that we have to add something as a placeholder, even if the
+            // texture coordinates aren't defined.
+            int num_values = 2;
+            int column_alignment = 0;
+            int start = new_array_format->get_total_bytes();
+            start = (start + sizeof(PN_stdfloat) - 1) & ~(sizeof(PN_stdfloat) - 1);
             if (texcoord_type != nullptr) {
-              new_array_format->add_column
-                (name, texcoord_type->get_num_values(), NT_stdfloat, C_texcoord,
-                 -1, texcoord_type->get_column_alignment());
-            } else {
-              // We have to add something as a placeholder, even if the
-              // texture coordinates aren't defined.
-              new_array_format->add_column(name, 2, NT_stdfloat, C_texcoord);
+              column_alignment = texcoord_type->get_column_alignment();
+              num_values = texcoord_type->get_num_values();
             }
+            if (start + num_values * sizeof(PN_stdfloat) > 2048) {
+              // We are exceeding the limit for stride (and the one that is
+              // guaranteed to be supported).  Start a new array.
+              new_format->insert_array(insert_at++, new_array_format);
+              new_array_format = new GeomVertexArrayFormat;
+              start = 0;
+            }
+            new_array_format->add_column(name, num_values, NT_stdfloat,
+                                         C_texcoord, start, column_alignment);
             new_format->remove_column(name);
           }
         }
@@ -453,7 +477,9 @@ premunge_format_impl(const GeomVertexFormat *orig) {
     }
 
     // Finally, insert the interleaved array first in the format.
-    new_format->insert_array(0, new_array_format);
+    if (new_array_format->get_num_columns() > 0) {
+      new_format->insert_array(insert_at, new_array_format);
+    }
     format = GeomVertexFormat::register_format(new_format);
   }
 

panda/src/glstuff/glGraphicsStateGuardian_src.I

@@ -181,6 +181,19 @@ has_fixed_function_pipeline() const {
 #endif
 }
 
+/**
+ *
+ */
+INLINE int CLP(GraphicsStateGuardian)::
+get_max_vertex_attrib_stride() const {
+#ifdef OPENGLES_1
+  // Best guess.
+  return 2048;
+#else
+  return _max_vertex_attrib_stride;
+#endif
+}
+
 /**
  * Calls glFinish() if the config variable gl-finish is set True.
  */

panda/src/glstuff/glGraphicsStateGuardian_src.cxx

@@ -3434,6 +3434,31 @@ reset() {
   }
 #endif
 
+#ifndef OPENGLES_1
+#ifdef OPENGLES
+  if (is_at_least_gles_version(3, 1))
+#else
+  if (is_at_least_gl_version(4, 4))
+#endif
+  {
+    _max_vertex_attrib_stride = -1;
+    glGetIntegerv(GL_MAX_VERTEX_ATTRIB_STRIDE, &_max_vertex_attrib_stride);
+
+    if (_max_vertex_attrib_stride < 0) {
+      GLCAT.warning()
+        << "Failed to query GL_MAX_VERTEX_ATTRIB_STRIDE.\n";
+      _max_vertex_attrib_stride = INT_MAX;
+    }
+    else if (GLCAT.is_debug()) {
+      GLCAT.debug()
+        << "max vertex attrib stride = " << _max_vertex_attrib_stride << "\n";
+    }
+  }
+  else {
+    _max_vertex_attrib_stride = INT_MAX;
+  }
+#endif  // !OPENGLES_1
+
   _current_vbuffer_index = 0;
   _current_ibuffer_index = 0;
   _current_vao_index = 0;
@@ -6483,6 +6508,16 @@ setup_array_data(const unsigned char *&client_pointer,
     return (client_pointer != nullptr);
   }
 
+#ifndef OPENGLES_1
+  int stride = array_reader->get_array_format()->get_stride();
+  if (stride > _max_vertex_attrib_stride) {
+    GLCAT.error()
+      << "Vertex array stride " << stride << " exceeds supported maximum "
+      << _max_vertex_attrib_stride << "!\n";
+    return false;
+  }
+#endif
+
   // Prepare the buffer object and bind it.
   CLP(VertexBufferContext) *gvbc = DCAST(CLP(VertexBufferContext),
     array_reader->prepare_now(get_prepared_objects(), this));

panda/src/glstuff/glGraphicsStateGuardian_src.h

@@ -428,6 +428,8 @@ public:
   INLINE int get_gl_version_minor() const;
   INLINE bool has_fixed_function_pipeline() const;
 
+  INLINE int get_max_vertex_attrib_stride() const;
+
   virtual void set_state_and_transform(const RenderState *state,
                                        const TransformState *transform);
 
@@ -727,6 +729,7 @@ protected:
   bool _use_vertex_attrib_binding;
   CPT(GeomVertexFormat) _current_vertex_format;
   const GeomVertexColumn *_vertex_attrib_columns[32];
+  int _max_vertex_attrib_stride = INT_MAX;
 
   GLuint _current_sbuffer_index;
   pvector<GLuint> _current_sbuffer_base;