improve ability of flattenStrong() to unify Geoms

panda/src/egg/eggGroupNode.cxx

@@ -1069,26 +1069,44 @@ rebuild_vertex_pool(EggVertexPool *vertex_pool, bool recurse) {
 //               polygons; otherwise, each polygon is considered
 //               individually.
 //
+//               If allow_per_primitive is false, S_per_face will
+//               treated like S_per_vertex: normals and colors will
+//               always be assigned to the vertices.  In this case,
+//               there will never be per-primitive colors or normals
+//               after this call returns.  On the other hand, if
+//               allow_per_primitive is true, then S_per_face means
+//               that normals and colors should be assigned to the
+//               primitives, and removed from the vertices, as
+//               described above.
+//
 //               This may create redundant vertices in the vertex
 //               pool, so it may be a good idea to follow this up with
 //               remove_unused_vertices().
 ////////////////////////////////////////////////////////////////////
 void EggGroupNode::
-unify_attributes(bool use_connected_shading, bool recurse) {
+unify_attributes(bool use_connected_shading, bool allow_per_primitive,
+		 bool recurse) {
   Children::iterator ci;
   for (ci = _children.begin(); ci != _children.end(); ++ci) {
     EggNode *child = *ci;
 
     if (child->is_of_type(EggPrimitive::get_class_type())) {
       EggPrimitive *prim = DCAST(EggPrimitive, child);
+      EggPrimitive::Shading shading = prim->get_shading();
       if (use_connected_shading) {
-        prim->unify_attributes(prim->get_connected_shading());
-      } else {
-        prim->unify_attributes(prim->get_shading());
+        shading = prim->get_connected_shading();
+      }
+
+      if (!allow_per_primitive && shading == EggPrimitive::S_per_face) {
+	shading = EggPrimitive::S_per_vertex;
       }
+
+      prim->unify_attributes(shading);
+
     } else if (child->is_of_type(EggGroupNode::get_class_type())) {
       if (recurse) {
-        DCAST(EggGroupNode, child)->unify_attributes(use_connected_shading, recurse);
+        DCAST(EggGroupNode, child)->unify_attributes
+	  (use_connected_shading, allow_per_primitive, recurse);
       }
     }
   }

panda/src/egg/eggGroupNode.h

@@ -150,7 +150,8 @@ PUBLISHED:
   void clear_connected_shading();
   void get_connected_shading();
   void rebuild_vertex_pool(EggVertexPool *vertex_pool, bool recurse);
-  void unify_attributes(bool use_connected_shading, bool recurse);
+  void unify_attributes(bool use_connected_shading, bool allow_per_primitive,
+			bool recurse);
   void apply_last_attribute(bool recurse);
   void apply_first_attribute(bool recurse);
   void post_apply_flat_attribute(bool recurse);

panda/src/egg2pg/config_egg2pg.cxx

@@ -82,6 +82,20 @@ ConfigVariableBool egg_rigid_geometry
           "geometry has to be vertex-animated, but there will tend to be "
           "more separate pieces."));
 
+ConfigVariableBool egg_flat_shading
+("egg-flat-shading", false,
+ PRC_DESC("Set this true to allow the egg loader to create geometry with the "
+	  "ShadeModelAttrib::M_flat attribute set.  It will do this only "
+	  "for geometry that has per-polygon normals and/or colors.  This "
+	  "allows the egg loader to avoid duplicating vertices when they "
+	  "are shared between connect polygons with different normals or "
+	  "colors, but it prevents the flat-shaded geometry from being "
+	  "combined with any adjacent smooth-shaded geometry (for instance, "
+	  "as the result of a flatten_strong operation).  It is false by "
+	  "default, since flat-shaded geometry is rare; but you may wish "
+	  "to set it true if your scene largely or entirely consists of "
+	  "flat-shaded polygons."));
+
 ConfigVariableBool egg_load_old_curves
 ("egg-load-old-curves", true,
  PRC_DESC("When this is true, a <NurbsCurve> entry appearing in an egg file "

panda/src/egg2pg/config_egg2pg.h

@@ -45,10 +45,10 @@ extern EXPCL_PANDAEGG ConfigVariableBool egg_unify;
 extern EXPCL_PANDAEGG ConfigVariableDouble egg_flatten_radius;
 extern EXPCL_PANDAEGG ConfigVariableBool egg_combine_geoms;
 extern EXPCL_PANDAEGG ConfigVariableBool egg_rigid_geometry;
+extern EXPCL_PANDAEGG ConfigVariableBool egg_flat_shading;
 extern EXPCL_PANDAEGG ConfigVariableBool egg_load_old_curves;
 extern EXPCL_PANDAEGG ConfigVariableBool egg_load_classic_nurbs_curves;
 extern EXPCL_PANDAEGG ConfigVariableBool egg_accept_errors;
-extern EXPCL_PANDAEGG ConfigVariableBool egg_accept_errors;
 extern EXPCL_PANDAEGG ConfigVariableBool egg_suppress_hidden;
 extern EXPCL_PANDAEGG ConfigVariableEnum<EggRenderMode::AlphaMode> egg_alpha_mode;
 

panda/src/egg2pg/eggLoader.cxx

@@ -173,7 +173,7 @@ build_graph() {
   _data->clear_connected_shading();
   _data->remove_unused_vertices(true);
   _data->get_connected_shading();
-  _data->unify_attributes(true, true);
+  _data->unify_attributes(true, egg_flat_shading, true);
 
   // Now we need to get the connected shading again, since in unifying
   // the attributes we may have made vertices suddenly become

panda/src/egg2pg/eggRenderState.cxx

@@ -344,7 +344,8 @@ fill_state(EggPrimitive *egg_prim) {
   }
 
   _flat_shaded = 
-    (egg_prim->get_connected_shading() == EggPrimitive::S_per_face);
+    (egg_flat_shading &&
+     egg_prim->get_connected_shading() == EggPrimitive::S_per_face);
 
   if (_flat_shaded) {
     add_attrib(ShadeModelAttrib::make(ShadeModelAttrib::M_flat));

panda/src/gobj/geomVertexData.cxx

@@ -199,6 +199,53 @@ set_name(const string &name) {
   _morphs_pcollector = PStatCollector(_char_pcollector, "Morphs");
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexData::set_format
+//       Access: Published
+//  Description: Changes the format of the vertex data.  If the data
+//               is not empty, this will implicitly change every row
+//               to match the new format.
+//
+//               Don't call this in a downstream thread unless you
+//               don't mind it blowing away other changes you might
+//               have recently made in an upstream thread.
+////////////////////////////////////////////////////////////////////
+void GeomVertexData::
+set_format(const GeomVertexFormat *format) {
+  nassertv(format->is_registered());
+  if (format == _format) {
+    // Trivially no-op.
+    return;
+  }
+
+  CDWriter cdata(_cycler, true);
+
+  // Put the current data aside, so we can copy it back in below.
+  CPT(GeomVertexData) orig_data = new GeomVertexData(*this);
+
+  // Assign the new format.  This means clearing out all of our
+  // current arrays and replacing them with new, empty arrays.
+  _format = format;
+
+  UsageHint usage_hint = cdata->_usage_hint;
+  cdata->_arrays.clear();
+  int num_arrays = _format->get_num_arrays();
+  for (int i = 0; i < num_arrays; i++) {
+    PT(GeomVertexArrayData) array = new GeomVertexArrayData
+      (_format->get_array(i), usage_hint);
+    cdata->_arrays.push_back(array);
+  }
+
+  // Now copy the original data back in.  This will automatically
+  // convert it to the new format.
+  copy_from(orig_data, false);
+
+  clear_cache_stage();
+  cdata->_modified = Geom::get_next_modified();
+  cdata->_animated_vertices.clear();
+}
+
+
 ////////////////////////////////////////////////////////////////////
 //     Function: GeomVertexData::set_usage_hint
 //       Access: Published

panda/src/gobj/geomVertexData.h

@@ -90,6 +90,8 @@ PUBLISHED:
   void set_name(const string &name);
 
   INLINE const GeomVertexFormat *get_format() const;
+  void set_format(const GeomVertexFormat *format);
+
   INLINE UsageHint get_usage_hint() const;
   void set_usage_hint(UsageHint usage_hint);
 

panda/src/gobj/geomVertexFormat.cxx

@@ -101,6 +101,7 @@ GeomVertexFormat::
 //               CPU-animation data elements removed.
 //
 //               This may only be called after the format has been
+//               registered.  The return value will have been already
 //               registered.
 ////////////////////////////////////////////////////////////////////
 CPT(GeomVertexFormat) GeomVertexFormat::
@@ -134,6 +135,61 @@ get_post_animated_format() const {
   return _post_animated_format;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: GeomVertexFormat::get_union_format
+//       Access: Published
+//  Description: Returns a new GeomVertexFormat that includes all of
+//               the columns defined in either this GeomVertexFormat
+//               or the other one.  If any column is defined in both
+//               formats with different sizes (for instance, texcoord2
+//               vs. texcoord3), the new format will include the
+//               larger of the two definitions.
+//
+//               This may only be called after both source formats
+//               have been registered.  The return value will also
+//               have been already registered.
+////////////////////////////////////////////////////////////////////
+CPT(GeomVertexFormat) GeomVertexFormat::
+get_union_format(const GeomVertexFormat *other) const {
+  nassertr(is_registered() && other->is_registered(), NULL);
+
+  PT(GeomVertexArrayFormat) new_array = new GeomVertexArrayFormat;
+
+  // First, ensure that it has all of the columns in the first format.
+  Arrays::const_iterator ai;
+  for (ai = _arrays.begin(); ai != _arrays.end(); ++ai) {
+    GeomVertexArrayFormat *array_format = (*ai);
+    int num_columns = array_format->get_num_columns();
+    for (int i = 0; i < num_columns; ++i) {
+      const GeomVertexColumn *column = array_format->get_column(i);
+      const GeomVertexColumn *other_column = other->get_column(column->get_name());
+      if (other_column != (GeomVertexColumn *)NULL &&
+	  other_column->get_total_bytes() > column->get_total_bytes()) {
+	// The other column is larger.  Keep it.
+	new_array->add_column(*other_column);
+      } else {
+	// Our column is larger.  Keep it.
+	new_array->add_column(*column);
+      }
+    }
+  }
+
+  // Now pick up any remaining columns in the second format.
+  for (ai = other->_arrays.begin(); ai != other->_arrays.end(); ++ai) {
+    GeomVertexArrayFormat *other_array_format = (*ai);
+    int num_columns = other_array_format->get_num_columns();
+    for (int i = 0; i < num_columns; ++i) {
+      const GeomVertexColumn *other_column = other_array_format->get_column(i);
+      if (!new_array->has_column(other_column->get_name())) {
+	new_array->add_column(*other_column);
+      }
+    }
+  }
+
+  // Finally, create a format for the thing.
+  return GeomVertexFormat::register_format(new_array);
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: GeomVertexFormat::modify_array
 //       Access: Published

panda/src/gobj/geomVertexFormat.h

@@ -78,6 +78,7 @@ PUBLISHED:
   INLINE void set_animation(const GeomVertexAnimationSpec &animation);
 
   CPT(GeomVertexFormat) get_post_animated_format() const;
+  CPT(GeomVertexFormat) get_union_format(const GeomVertexFormat *other) const;
 
   INLINE int get_num_arrays() const;
   INLINE const GeomVertexArrayFormat *get_array(int array) const;

panda/src/pgraph/geomTransformer.cxx

@@ -404,7 +404,9 @@ collect_vertex_data(Geom *geom, int collect_bits) {
   if ((collect_bits & SceneGraphReducer::CVD_name) != 0) {
     key._name = vdata->get_name();
   }
-  key._format = format;
+  if ((collect_bits & SceneGraphReducer::CVD_format) != 0) {
+    key._format = format;
+  }
   key._usage_hint = vdata->get_usage_hint();
 
   AlreadyCollected::const_iterator ai;
@@ -424,9 +426,12 @@ collect_vertex_data(Geom *geom, int collect_bits) {
   PT(GeomVertexData) new_data;
   if (ni != _new_collected_data.end()) {
     new_data = (*ni).second;
+
   } else {
+    // We haven't encountered a compatible GeomVertexData before.
+    // Create a new one.
     new_data = new GeomVertexData(vdata->get_name(), format, 
-                                    vdata->get_usage_hint());
+				  vdata->get_usage_hint());
     _new_collected_data[key] = new_data;
     ++num_created;
   }
@@ -437,13 +442,22 @@ collect_vertex_data(Geom *geom, int collect_bits) {
     // Whoa, hold the phone!  Too many vertices going into this one
     // GeomVertexData object; we'd better start over.
     new_data = new GeomVertexData(vdata->get_name(), format, 
-                                    vdata->get_usage_hint());
+				  vdata->get_usage_hint());
     _new_collected_data[key] = new_data;
     offset = 0;
     new_num_vertices = vdata->get_num_rows();
     ++num_created;
   }
 
+  if (new_data->get_format() != format) {
+    // We're combining two GeomVertexDatas of different formats.
+    // Therefore, we need to expand the format to include the union of
+    // both sets of columns.
+    CPT(GeomVertexFormat) new_format = format->get_union_format(new_data->get_format());
+    new_data->set_format(new_format);
+    nassertr(offset == new_data->get_num_rows(), 0);
+  }
+
   new_data->set_num_rows(new_num_vertices);
 
   for (int i = 0; i < vdata->get_num_arrays(); ++i) {

panda/src/pgraph/nodePath.cxx

@@ -5555,7 +5555,7 @@ flatten_strong() {
   gr.apply_attribs(node());
   int num_removed = gr.flatten(node(), ~0);
 
-  gr.collect_vertex_data(node());
+  gr.collect_vertex_data(node(), ~SceneGraphReducer::CVD_format);
   gr.unify(node());
 
   return num_removed;

panda/src/pgraph/sceneGraphReducer.I

@@ -114,7 +114,9 @@ apply_attribs(PandaNode *node, const AccumulatedAttribs &attribs,
 //               have compatible formats at this node and below into a
 //               single, unified block (or at least multiple larger
 //               blocks).  This is intended to reduce rendering
-//               overhead incurred by switching vertex buffers.
+//               overhead incurred by switching vertex buffers.  It
+//               can also make a subsequent call to unify() much more
+//               effective than it would have been otherwise.
 //
 //               The set of bits passed in collect_bits indicates
 //               which properties are used to differentiate
@@ -147,7 +149,10 @@ make_nonindexed(PandaNode *root, int nonindexed_bits) {
 //     Function: SceneGraphReducer::unify
 //       Access: Published
 //  Description: Calls unify() on every GeomNode at this level and
-//               below.
+//               below.  This attempts to reduce the total number of
+//               individual Geoms and GeomPrimitives by combining
+//               these objects wherever possible.  See
+//               GeomNode::unify().
 ////////////////////////////////////////////////////////////////////
 INLINE void SceneGraphReducer::
 unify(PandaNode *root) {

panda/src/pgraph/sceneGraphReducer.h

@@ -87,6 +87,12 @@ PUBLISHED:
     // If set, only those GeomVertexDatas within the same node might
     // be collected together.
     CVD_one_node_only  = 0x010,
+
+    // If set, two GeomVertexDatas with different formats will not be
+    // collected together.  If not set, GeomVertexDatas of different
+    // formats may be combined by expanding all GeomVertexDatas to the
+    // union of all defined columns.
+    CVD_format         = 0x020,
   };
 
   enum MakeNonindexed {