Implemented new make_consistent_state routine to unify ColorAttribs

panda/src/pgraph/colorAttrib.cxx

@@ -26,6 +26,8 @@
 #include "datagramIterator.h"
 
 TypeHandle ColorAttrib::_type_handle;
+CPT(RenderAttrib) ColorAttrib::_off;
+CPT(RenderAttrib) ColorAttrib::_vertex;
 
 ////////////////////////////////////////////////////////////////////
 //     Function: ColorAttrib::make_vertex
@@ -36,8 +38,12 @@ TypeHandle ColorAttrib::_type_handle;
 ////////////////////////////////////////////////////////////////////
 CPT(RenderAttrib) ColorAttrib::
 make_vertex() {
+  if (_vertex != 0) {
+    return _vertex;
+  }
   ColorAttrib *attrib = new ColorAttrib(T_vertex);
-  return return_new(attrib);
+  _vertex = return_new(attrib);
+  return _vertex;
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -61,8 +67,12 @@ make_flat(const Colorf &color) {
 ////////////////////////////////////////////////////////////////////
 CPT(RenderAttrib) ColorAttrib::
 make_off() {
+  if (_off != 0) {
+    return _off;
+  }
   ColorAttrib *attrib = new ColorAttrib(T_off);
-  return return_new(attrib);
+  _off = return_new(attrib);
+  return _off;
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/pgraph/colorAttrib.h

@@ -63,6 +63,8 @@ private:
 private:
   Type _type;
   Colorf _color;
+  static CPT(RenderAttrib) _off;
+  static CPT(RenderAttrib) _vertex;
 
 public:
   static void register_with_read_factory();

panda/src/pgraph/geomNode.cxx

@@ -137,13 +137,15 @@ apply_attribs_to_vertices(const AccumulatedAttribs &attribs, int attrib_types,
       bool any_changed = false;
       
       if ((attrib_types & SceneGraphReducer::TT_color) != 0) {
-        if (geom_attribs._color != (const RenderAttrib *)NULL) {
-          const ColorAttrib *ca = DCAST(ColorAttrib, geom_attribs._color);
-          if (ca->get_color_type() == ColorAttrib::T_flat) {
-            if (transformer.set_color(new_geom, ca->get_color())) {
-              entry._state = entry._state->add_attrib(ColorAttrib::make_vertex());
-              any_changed = true;
-            }
+        const RenderAttrib *ra = geom_attribs._color;
+        if (ra == (const RenderAttrib *)NULL) {
+          ra = ColorAttrib::make_off();
+        }
+        const ColorAttrib *ca = DCAST(ColorAttrib, ra);
+        entry._state = entry._state->add_attrib(ca);
+        if (ca->get_color_type() != ColorAttrib::T_vertex) {
+          if (transformer.remove_column(new_geom, InternalName::get_color())) {
+            any_changed = true;
           }
         }
       }

panda/src/pgraph/geomTransformer.cxx

@@ -478,41 +478,115 @@ remove_column(GeomNode *node, const InternalName *column) {
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: GeomTransformer::apply_colors
+//     Function: GeomTransformer::make_compatible_state
 //       Access: Public
-//  Description: Checks if the GeomNode has differing ColorAttribs.
-//               If so, all the colors for all the Geoms are pushed
-//               down into the vertices, and the differing
-//               ColorAttribs are removed.
+//  Description: Checks if the different geoms in the GeomNode have
+//               different RenderStates.  If so, tries to make the 
+//               RenderStates the same.  It does this by
+//               canonicalizing the ColorAttribs, and in the future,
+//               possibly other attribs.
+//
+//               This implementation is not very smart yet.  It 
+//               unnecessarily canonicalizes ColorAttribs even if 
+//               this will not yield compatible RenderStates.  A
+//               better algorithm would:
+//
+//               - each geom already starts with an original
+//                 RenderState.  In addition to this, calculate for
+//                 each geom a canonical RenderState.
+//
+//               - maintain a table mapping canonical RenderState
+//                 to a list of geoms.
+//
+//               - for each group of geoms with the same 
+//                 canonical renderstate, see if they already have
+//                 matching RenderStates.
+//
+//               - If they have differing RenderStates, then
+//                 actually canonicalize the geoms.
+//               
 ////////////////////////////////////////////////////////////////////
 bool GeomTransformer::
-apply_colors(GeomNode *node) {
+make_compatible_state(GeomNode *node) {
   if (node->get_num_geoms() < 2) {
     return false;
   }
   
-  bool need_apply = false;
-
+  bool has_incompatible = false;
+  
   GeomNode::CDWriter cdata(node->_cycler);
   GeomNode::GeomList::iterator gi;
   GeomNode::GeomList &geoms = *(cdata->modify_geoms());
+
+  // For each geom, calculate a canonicalized RenderState, and 
+  // classify all the geoms according to that.
   
-  const RenderAttrib *first = geoms[0]._state->get_attrib(ColorAttrib::get_class_type());
-  for (gi = geoms.begin(); gi != geoms.end(); ++gi) {
-    GeomNode::GeomEntry &entry = (*gi);
-    if (entry._state->get_attrib(ColorAttrib::get_class_type()) != first) {
-      need_apply = true;
-      break;
+  typedef pmap <CPT(RenderState), pvector<int>> StateTable;
+  StateTable state_table;
+  
+  for (int i=0; i<geoms.size(); i++) {
+    GeomNode::GeomEntry &entry = geoms[i];
+    CPT(RenderState) canon = entry._state->add_attrib(ColorAttrib::make_vertex());
+    state_table[canon].push_back(i);
+  }
+
+  // For each group of geoms, check for mismatch.
+  
+  bool any_changed = false;
+  StateTable::iterator si;
+  for (si = state_table.begin(); si != state_table.end(); si++) {
+    
+    // If the geoms in the group already have the same RenderStates,
+    // then nothing needs to be done to this group.
+    
+    pvector<int> &indices = (*si).second;
+    bool mismatch = false;
+    for (int i=1; i<indices.size(); i++) {
+      if (geoms[indices[i]]._state != geoms[indices[0]]._state) {
+        mismatch = true;
+        break;
+      }
+    }
+    if (!mismatch) {
+      continue;
+    }
+    
+    // The geoms do not have the same RenderState, but they could,
+    // since their canonicalized states are the same.  Canonicalize them.
+    
+    const RenderState *canon_state = (*si).first;
+    for (int i=0; i<indices.size(); i++) {
+      GeomNode::GeomEntry &entry = geoms[indices[i]];
+      const RenderAttrib *ra = entry._state->get_attrib(ColorAttrib::get_class_type());
+      if (ra == (RenderAttrib *)NULL) {
+        ra = ColorAttrib::make_off();
+      }
+      const ColorAttrib *ca = DCAST(ColorAttrib, ra);
+      if (ca->get_color_type() == ColorAttrib::T_vertex) {
+        // All we need to do is ensure that the geom has a color column.
+        if (!entry._geom.get_read_pointer()->get_vertex_data()->has_column(InternalName::get_color())) {
+          PT(Geom) new_geom = entry._geom.get_read_pointer()->make_copy();
+          if (set_color(new_geom, Colorf(1,1,1,1))) {
+            entry._geom = new_geom;
+            any_changed = true;
+          }
+        }
+      } else {
+        Colorf c(1,1,1,1);
+        if (ca->get_color_type() == ColorAttrib::T_flat) {
+          c = ca->get_color();
+        }
+        PT(Geom) new_geom = entry._geom.get_read_pointer()->make_copy();
+        if (set_color(new_geom, c)) {
+          entry._geom = new_geom;
+          any_changed = true;
+        }
+      }
+      entry._state = canon_state;
     }
   }
   
-  if (!need_apply) {
-    return false;
-  }
-
-  // NOT IMPLEMENTED YET.  DOESNT DO ANYTHING.
-  
-  return true;
+  return any_changed;
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/pgraph/geomTransformer.h

@@ -75,7 +75,7 @@ public:
   bool remove_column(Geom *geom, const InternalName *column);
   bool remove_column(GeomNode *node, const InternalName *column);
 
-  bool apply_colors(GeomNode *node);
+  bool make_compatible_state(GeomNode *node);
 
   bool reverse_normals(Geom *geom);
   bool doubleside(GeomNode *node);

panda/src/pgraph/nodePath.cxx

@@ -5862,6 +5862,7 @@ flatten_medium() {
   int num_removed = gr.flatten(node(), 0);
 
   if (flatten_geoms) {
+    gr.make_compatible_state(node());
     gr.collect_vertex_data(node());
     gr.unify(node(), true);
   }
@@ -5894,6 +5895,7 @@ flatten_strong() {
   int num_removed = gr.flatten(node(), ~0);
 
   if (flatten_geoms) {
+    gr.make_compatible_state(node());
     gr.collect_vertex_data(node(), ~(SceneGraphReducer::CVD_format | SceneGraphReducer::CVD_name | SceneGraphReducer::CVD_animation_type));
     gr.unify(node(), false);
   }

panda/src/pgraph/sceneGraphReducer.cxx

@@ -31,7 +31,7 @@
 PStatCollector SceneGraphReducer::_flatten_collector("*:Flatten:flatten");
 PStatCollector SceneGraphReducer::_apply_collector("*:Flatten:apply");
 PStatCollector SceneGraphReducer::_remove_column_collector("*:Flatten:remove column");
-PStatCollector SceneGraphReducer::_apply_colors_collector("*:Flatten:apply colors");
+PStatCollector SceneGraphReducer::_compatible_state_collector("*:Flatten:compatible colors");
 PStatCollector SceneGraphReducer::_collect_collector("*:Flatten:collect");
 PStatCollector SceneGraphReducer::_make_nonindexed_collector("*:Flatten:make nonindexed");
 PStatCollector SceneGraphReducer::_unify_collector("*:Flatten:unify");
@@ -149,7 +149,7 @@ remove_column(PandaNode *root, const InternalName *column) {
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: SceneGraphReducer::apply_colors
+//     Function: SceneGraphReducer::make_compatible_state
 //       Access: Published
 //  Description: Searches for GeomNodes that contain multiple Geoms
 //               that differ only in their ColorAttribs.  If such a
@@ -158,9 +158,9 @@ remove_column(PandaNode *root, const InternalName *column) {
 //               the geoms to be unified later.
 ////////////////////////////////////////////////////////////////////
 int SceneGraphReducer::
-apply_colors(PandaNode *root) {
-  PStatTimer timer(_apply_colors_collector);
-  return r_apply_colors(root, _transformer);
+make_compatible_state(PandaNode *root) {
+  PStatTimer timer(_compatible_state_collector);
+  return r_make_compatible_state(root, _transformer);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -758,16 +758,16 @@ r_remove_column(PandaNode *node, const InternalName *column,
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: SceneGraphReducer::r_apply_colors
+//     Function: SceneGraphReducer::r_make_compatible_state
 //       Access: Private
-//  Description: The recursive implementation of apply_colors().
+//  Description: The recursive implementation of make_compatible_state().
 ////////////////////////////////////////////////////////////////////
 int SceneGraphReducer::
-r_apply_colors(PandaNode *node, GeomTransformer &transformer) {
+r_make_compatible_state(PandaNode *node, GeomTransformer &transformer) {
   int num_changed = 0;
 
   if (node->is_geom_node()) {
-    if (transformer.apply_colors(DCAST(GeomNode, node))) {
+    if (transformer.make_compatible_state(DCAST(GeomNode, node))) {
       ++num_changed;
     }
   }
@@ -776,7 +776,7 @@ r_apply_colors(PandaNode *node, GeomTransformer &transformer) {
   int num_children = children.get_num_children();
   for (int i = 0; i < num_children; ++i) {
     num_changed +=
-      r_apply_colors(children.get_child(i), transformer);
+      r_make_compatible_state(children.get_child(i), transformer);
   }
 
   return num_changed;

panda/src/pgraph/sceneGraphReducer.h

@@ -138,7 +138,7 @@ PUBLISHED:
 
   int remove_column(PandaNode *root, const InternalName *column);
 
-  int apply_colors(PandaNode *root);
+  int make_compatible_state(PandaNode *root);
   
   INLINE int make_compatible_format(PandaNode *root, int collect_bits = ~0);
   void decompose(PandaNode *root);
@@ -177,7 +177,7 @@ protected:
   int r_remove_column(PandaNode *node, const InternalName *column,
                       GeomTransformer &transformer);
 
-  int r_apply_colors(PandaNode *node, GeomTransformer &transformer);
+  int r_make_compatible_state(PandaNode *node, GeomTransformer &transformer);
 
   int r_collect_vertex_data(PandaNode *node, int collect_bits,
                             GeomTransformer &transformer, bool format_only);
@@ -195,7 +195,7 @@ private:
   static PStatCollector _flatten_collector;
   static PStatCollector _apply_collector;
   static PStatCollector _remove_column_collector;
-  static PStatCollector _apply_colors_collector;
+  static PStatCollector _compatible_state_collector;
   static PStatCollector _collect_collector;
   static PStatCollector _make_nonindexed_collector;
   static PStatCollector _unify_collector;