preserve geometry order when flattening

panda/src/egg2pg/load_egg_file.cxx

@@ -52,7 +52,7 @@ load_from_loader(EggLoader &loader) {
     egg2pg_cat.info() << "Flattened " << num_reduced << " nodes.\n";
     if (egg_unify) {
       gr.collect_vertex_data(loader._root);
-      gr.unify(loader._root);
+      gr.unify(loader._root, true);
       if (egg2pg_cat.is_debug()) {
         egg2pg_cat.debug() << "Unified.\n";
       }

panda/src/gobj/geom.I

@@ -209,12 +209,15 @@ rotate() const {
 //               triangle fans.
 //
 //               max_indices represents the maximum number of indices
-//               that will be put in any one GeomPrimitive.
+//               that will be put in any one GeomPrimitive.  If
+//               preserve_order is true, then the primitives will not
+//               be reordered during the operation, even if this
+//               results in a suboptimal result.
 ////////////////////////////////////////////////////////////////////
 INLINE PT(Geom) Geom::
-unify(int max_indices) const {
+unify(int max_indices, bool preserve_order) const {
   PT(Geom) new_geom = make_copy();
-  new_geom->unify_in_place(max_indices);
+  new_geom->unify_in_place(max_indices, preserve_order);
   return new_geom;
 }
 

panda/src/gobj/geom.cxx

@@ -612,14 +612,17 @@ rotate_in_place() {
 //               triangle fans.
 //
 //               max_indices represents the maximum number of indices
-//               that will be put in any one GeomPrimitive.
+//               that will be put in any one GeomPrimitive.  If
+//               preserve_order is true, then the primitives will not
+//               be reordered during the operation, even if this
+//               results in a suboptimal result.
 //
 //               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 Geom::
-unify_in_place(int max_indices) {
+unify_in_place(int max_indices, bool preserve_order) {
   Thread *current_thread = Thread::get_current_thread();
   if (get_num_primitives() <= 1) {
     // If we don't have more than one primitive to start with, no need
@@ -633,10 +636,36 @@ unify_in_place(int max_indices) {
 
   NewPrims new_prims;
 
+  bool keep_different_types = preserve_triangle_strips && !preserve_order;
+
   Primitives::const_iterator pi;
   for (pi = cdata->_primitives.begin(); pi != cdata->_primitives.end(); ++pi) {
     CPT(GeomPrimitive) primitive = (*pi).get_read_pointer();
     NewPrims::iterator npi = new_prims.find(primitive->get_type());
+    if (npi == new_prims.end()) {
+      // This is the first primitive of this type.
+      if (!keep_different_types && !new_prims.empty()) {
+        // Actually, since we aren't trying to keep the different
+        // types of primitives, we should try to combine this type and
+        // the other type by decomposing them both (into triangles,
+        // segments, or whatever).
+
+        // First, decompose the incoming one.
+        primitive = primitive->decompose();
+        npi = new_prims.find(primitive->get_type());
+        if (npi == new_prims.end()) {
+          // That didn't help, so decompose the one already in the
+          // table.
+          nassertv(new_prims.size() == 1);
+          npi = new_prims.begin();
+          CPT(GeomPrimitive) np = (*npi).second->decompose();
+          new_prims.clear();
+          new_prims.insert(NewPrims::value_type(np->get_type(), np->make_copy()));
+          npi = new_prims.find(primitive->get_type());
+        }
+      }
+    }
+
     if (npi == new_prims.end()) {
       // This is the first primitive of this type.  Just store it.
       new_prims.insert(NewPrims::value_type(primitive->get_type(), primitive->make_copy()));
@@ -649,23 +678,14 @@ unify_in_place(int max_indices) {
   }
 
   // Now, we have one or more primitives, but only one of each type.
-  if (!preserve_triangle_strips && new_prims.size() > 1) {
-    // More than one different type.  Recombine them into a single
-    // primitive by decomposing.
-    PT(GeomPrimitive) new_prim;
-    NewPrims::iterator npi;
-    for (npi = new_prims.begin(); npi != new_prims.end(); ++npi) {
-      CPT(GeomPrimitive) prim = (*npi).second->decompose();
-      if (new_prim.is_null()) {
-        new_prim = prim->make_copy();
-      } else {
-        combine_primitives(new_prim, prim, current_thread);
-      }
-    }
-
-    new_prims.clear();
-    new_prims.insert(NewPrims::value_type(new_prim->get_type(), new_prim));
+#ifndef NDEBUG
+  if (!keep_different_types && new_prims.size() > 1) {
+    // This shouldn't be possible, because we decompose as we go, in
+    // the loop above.  (We have to decompose as we go to preserve the
+    // ordering of the primitives.)
+    nassertv(false);
   }
+#endif
 
   // Finally, iterate through the remaining primitives, and copy them
   // to the output list.

panda/src/gobj/geom.h

@@ -101,13 +101,13 @@ PUBLISHED:
   INLINE PT(Geom) doubleside() const;
   INLINE PT(Geom) reverse() const;
   INLINE PT(Geom) rotate() const;
-  INLINE PT(Geom) unify(int max_indices) const;
+  INLINE PT(Geom) unify(int max_indices, bool preserve_order) const;
 
   void decompose_in_place();
   void doubleside_in_place();
   void reverse_in_place();
   void rotate_in_place();
-  void unify_in_place(int max_indices);
+  void unify_in_place(int max_indices, bool preserve_order);
 
   virtual bool copy_primitives_from(const Geom *other);
 

panda/src/grutil/rigidBodyCombiner.cxx

@@ -109,7 +109,7 @@ collect() {
   SceneGraphReducer gr;
   gr.apply_attribs(_internal_root);
   gr.collect_vertex_data(_internal_root, ~(SceneGraphReducer::CVD_format | SceneGraphReducer::CVD_name | SceneGraphReducer::CVD_animation_type));
-  gr.unify(_internal_root);
+  gr.unify(_internal_root, false);
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/pgraph/geomNode.cxx

@@ -527,7 +527,10 @@ check_valid() const {
 //               into triangles.  See also Geom::unify().
 //
 //               max_indices represents the maximum number of indices
-//               that will be put in any one GeomPrimitive.
+//               that will be put in any one GeomPrimitive.  If
+//               preserve_order is true, then the primitives will not
+//               be reordered during the operation, even if this
+//               results in a suboptimal result.
 //
 //               In order for this to be successful, the primitives
 //               must reference the same GeomVertexData, have the same
@@ -535,7 +538,7 @@ check_valid() const {
 //               models.
 ////////////////////////////////////////////////////////////////////
 void GeomNode::
-unify(int max_indices) {
+unify(int max_indices, bool preserve_order) {
   bool any_changed = false;
 
   Thread *current_thread = Thread::get_current_thread();
@@ -553,8 +556,10 @@ unify(int max_indices) {
       const GeomEntry &old_entry = (*gi);
       
       bool unified = false;
-      GeomList::iterator gj;
-      for (gj = new_geoms->begin(); gj != new_geoms->end() && !unified; ++gj) {
+
+      // Go from back to front, to minimize damage to the primitive ordering.
+      GeomList::reverse_iterator gj;
+      for (gj = new_geoms->rbegin(); gj != new_geoms->rend() && !unified; ++gj) {
         GeomEntry &new_entry = (*gj);
         if (old_entry._state == new_entry._state) {
           // Both states match, so try to combine the primitives.
@@ -566,6 +571,12 @@ unify(int max_indices) {
             any_changed = true;
           }
         }
+
+        if (preserve_order) {
+          // If we're insisting on preserving the order, we can only
+          // attempt to merge with the tail of the list.
+          break;
+        }
       }
       
       if (!unified) {
@@ -584,7 +595,7 @@ unify(int max_indices) {
       GeomEntry &entry = (*wgi);
       nassertv(entry._geom.test_ref_count_integrity());
       PT(Geom) geom = entry._geom.get_write_pointer();
-      geom->unify_in_place(max_indices);
+      geom->unify_in_place(max_indices, preserve_order);
     }
   }
   CLOSE_ITERATE_CURRENT_AND_UPSTREAM(_cycler);

panda/src/pgraph/geomNode.h

@@ -79,7 +79,7 @@ PUBLISHED:
   INLINE void remove_all_geoms();
   bool check_valid() const;
 
-  void unify(int max_indices);
+  void unify(int max_indices, bool preserve_order);
 
   void write_geoms(ostream &out, int indent_level) const;
   void write_verbose(ostream &out, int indent_level) const;

panda/src/pgraph/nodePath.cxx

@@ -5779,7 +5779,7 @@ analyze() const {
 //       Access: Published
 //  Description: Lightly flattens out the hierarchy below this node by
 //               applying transforms, colors, and texture matrices
-//               from the arcs onto the vertices, but does not remove
+//               from the nodes onto the vertices, but does not remove
 //               any nodes.
 //
 //               This can result in improved rendering performance
@@ -5787,16 +5787,16 @@ analyze() const {
 //               resulting scene graph, but the number of nodes will
 //               remain the same.
 //
-//               Particularly, any NodePaths that reference nodes
+//               In particular, any NodePaths that reference nodes
 //               within this hierarchy will not be damaged.  However,
 //               since this operation will remove transforms from the
-//               scene graph, it may be dangerous to apply to arcs
+//               scene graph, it may be dangerous to apply to nodes
 //               where you expect to dynamically modify the transform,
 //               or where you expect the geometry to remain in a
 //               particular local coordinate system.
 //
 //               The return value is always 0, since flatten_light
-//               does not remove any arcs.
+//               does not remove any nodes.
 ////////////////////////////////////////////////////////////////////
 int NodePath::
 flatten_light() {
@@ -5812,24 +5812,16 @@ flatten_light() {
 //       Access: Published
 //  Description: A more thorough flattening than flatten_light(), this
 //               first applies all the transforms, colors, and texture
-//               matrices from the arcs onto the vertices, and then
+//               matrices from the nodes onto the vertices, and then
 //               removes unneeded grouping nodes--nodes that have
 //               exactly one child, for instance, but have no special
 //               properties in themselves.
 //
-//               This results in improved perforamance over
+//               This results in improved performance over
 //               flatten_light() because the number of nodes in the
 //               scene graph is reduced.
 //
-//               If max_children is specified, it represents the
-//               maximum number of children a node is allowed to have
-//               and still be flattened.  Normally, this is 1; we
-//               don't typically want to flatten a node that has
-//               multiple children.  However, sometimes this may be
-//               desirable; set this parameter to control the limit.
-//               If this is set to -1, there is no limit.
-//
-//               The return value is the number of arcs removed.
+//               The return value is the number of nodes removed.
 ////////////////////////////////////////////////////////////////////
 int NodePath::
 flatten_medium() {
@@ -5839,7 +5831,7 @@ flatten_medium() {
   int num_removed = gr.flatten(node(), 0);
 
   gr.collect_vertex_data(node());
-  gr.unify(node());
+  gr.unify(node(), true);
 
   return num_removed;
 }
@@ -5869,7 +5861,7 @@ flatten_strong() {
   int num_removed = gr.flatten(node(), ~0);
 
   gr.collect_vertex_data(node(), ~(SceneGraphReducer::CVD_format | SceneGraphReducer::CVD_name | SceneGraphReducer::CVD_animation_type));
-  gr.unify(node());
+  gr.unify(node(), false);
 
   return num_removed;
 }

panda/src/pgraph/sceneGraphReducer.cxx

@@ -154,14 +154,14 @@ remove_column(PandaNode *root, const InternalName *column) {
 //               GeomNode::unify().
 ////////////////////////////////////////////////////////////////////
 void SceneGraphReducer::
-unify(PandaNode *root) {
+unify(PandaNode *root, bool preserve_order) {
   PStatTimer timer(_unify_collector);
 
   int max_indices = max_collect_indices;
   if (_gsg != (GraphicsStateGuardianBase *)NULL) {
     max_indices = min(max_indices, _gsg->get_max_vertices_per_primitive());
   }
-  r_unify(root, max_indices);
+  r_unify(root, max_indices, preserve_order);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -831,16 +831,16 @@ r_make_nonindexed(PandaNode *node, int nonindexed_bits) {
 //  Description: The recursive implementation of unify().
 ////////////////////////////////////////////////////////////////////
 void SceneGraphReducer::
-r_unify(PandaNode *node, int max_indices) {
+r_unify(PandaNode *node, int max_indices, bool preserve_order) {
   if (node->is_geom_node()) {
     GeomNode *geom_node = DCAST(GeomNode, node);
-    geom_node->unify(max_indices);
+    geom_node->unify(max_indices, preserve_order);
   }
 
   PandaNode::Children children = node->get_children();
   int num_children = children.get_num_children();
   for (int i = 0; i < num_children; ++i) {
-    r_unify(children.get_child(i), max_indices);
+    r_unify(children.get_child(i), max_indices, preserve_order);
   }
   Thread::consider_yield();
 }

panda/src/pgraph/sceneGraphReducer.h

@@ -140,7 +140,7 @@ PUBLISHED:
 
   INLINE int collect_vertex_data(PandaNode *root, int collect_bits = ~0);
   INLINE int make_nonindexed(PandaNode *root, int nonindexed_bits = ~0);
-  void unify(PandaNode *root);
+  void unify(PandaNode *root, bool preserve_order);
 
   INLINE void premunge(PandaNode *root, const RenderState *initial_state);
 
@@ -175,7 +175,7 @@ protected:
   int r_collect_vertex_data(PandaNode *node, int collect_bits,
                             GeomTransformer &transformer);
   int r_make_nonindexed(PandaNode *node, int collect_bits);
-  void r_unify(PandaNode *node, int max_indices);
+  void r_unify(PandaNode *node, int max_indices, bool preserve_order);
 
   void r_premunge(PandaNode *node, const RenderState *state);
 

panda/src/text/textNode.cxx

@@ -339,7 +339,7 @@ generate() {
     gr.apply_attribs(root);
     gr.flatten(root, ~SceneGraphReducer::CS_within_radius);
     gr.collect_vertex_data(root);
-    gr.unify(root);
+    gr.unify(root, false);
   }
 
   // Now deal with the decorations.