more subtle optimizations

panda/src/dgraph/dataNode.cxx

@@ -91,7 +91,13 @@ void
 register_data_transition(TypeHandle &type_handle, const string &name,
                          TypeHandle derived_from) {
   // Make sure the user gave us a transition type as the base.
-  nassertv(derived_from.is_derived_from(NodeTransition::get_class_type()));
+
+  // We won't do this assertion for now, since it causes the
+  // derivation tree to get rebuilt repeatedly (since this runs at
+  // static init, before the whole tree is set up).  This isn't really
+  // a problem, but it seems messy to me.
+
+  //  nassertv(derived_from.is_derived_from(NodeTransition::get_class_type()));
 
   string actual_name = name + "_" + derived_from.get_name();
   type_handle = TypeRegistry::ptr()->find_type(actual_name);

panda/src/express/Sources.pp

@@ -45,7 +45,7 @@
      buffer.cxx checksumHashGenerator.cxx clockObject.cxx \
      config_express.cxx datagram.cxx datagramGenerator.cxx \
      datagramInputFile.cxx datagramIterator.cxx \
-     datagramOutputFile.cxx datagramSink.cxx error_utils.cxx \
+     datagramOutputFile.cxx datagramSink.cxx dcast.cxx error_utils.cxx \
      get_config_path.cxx \
      hashGeneratorBase.cxx hashVal.cxx indent.cxx \
      memoryInfo.cxx memoryUsage.cxx memoryUsagePointerCounts.cxx \

panda/src/express/config_express.cxx

@@ -99,6 +99,30 @@ get_paranoid_clock() {
   return config_express.GetBool("paranoid-clock", false);
 }
 
+// Set this to true to double-check the test for inheritance of
+// TypeHandles, e.g. via is_of_type().  This has no effect if NDEBUG
+// is defined.
+bool
+get_paranoid_inheritance() {
+  return config_express.GetBool("paranoid-inheritance", true);
+}
+
+// Set this to true to verify that every attempted DCAST operation in
+// fact references the correct type, or false otherwise.  This has no
+// effect if NDEBUG is defined, in which case it is never tested.
+bool
+get_verify_dcast() {
+  static bool got_verify_dcast = false;
+  static bool verify_dcast;
+
+  if (!got_verify_dcast) {
+    verify_dcast = config_express.GetBool("verify-dcast", true);
+    got_verify_dcast = true;
+  }
+
+  return verify_dcast;
+}
+
 const int patchfile_window_size =
         config_express.GetInt("patchfile-window-size", 16);
 

panda/src/express/config_express.h

@@ -33,10 +33,12 @@ NotifyCategoryDecl(express, EXPCL_PANDAEXPRESS, EXPTP_PANDAEXPRESS);
 
 //extern EXPCL_PANDAEXPRESS const bool track_memory_usage;
 
-bool EXPCL_PANDAEXPRESS get_leak_memory();
+EXPCL_PANDAEXPRESS bool get_leak_memory();
-bool EXPCL_PANDAEXPRESS get_never_destruct();
+EXPCL_PANDAEXPRESS bool get_never_destruct();
-bool EXPCL_PANDAEXPRESS get_use_high_res_clock();
+EXPCL_PANDAEXPRESS bool get_use_high_res_clock();
-bool EXPCL_PANDAEXPRESS get_paranoid_clock();
+EXPCL_PANDAEXPRESS bool get_paranoid_clock();
+EXPCL_PANDAEXPRESS bool get_paranoid_inheritance();
+EXPCL_PANDAEXPRESS bool get_verify_dcast();
 
 extern const int patchfile_window_size;
 extern const int patchfile_increment_size;

panda/src/express/dcast.T

@@ -27,6 +27,8 @@ template<class WantType>
 INLINE TypeHandle
 _dcast_get_typehandle(WantType *) {
   TypeHandle handle = WantType::get_class_type();
+
+#ifdef _DEBUG
   if (handle == TypeHandle::none()) {
     // This type handle is unregistered.  Oops!
     WantType::init_type();
@@ -34,6 +36,8 @@ _dcast_get_typehandle(WantType *) {
     express_cat->warning()
       << "Type " << handle << " was unregistered!\n";
   }
+#endif
+
   return handle;
 }
 
@@ -52,23 +56,7 @@ INLINE WantType *
 _dcast(WantType *, TypedObject *ptr) {
 #ifndef NDEBUG
   TypeHandle want_handle = _dcast_get_typehandle((WantType *)0);
- #if defined(_DEBUG) && defined(_WIN32)
+  if (!_dcast_verify(want_handle, sizeof(WantType), ptr)) {
-  if ((ptr == (TypedObject *)NULL) || IsBadWritePtr(ptr,sizeof(TypedObject))) {
- #else
-  if (ptr == (TypedObject *)NULL) {
- #endif
-    express_cat->warning()
-      << "Attempt to cast NULL pointer to " << want_handle << "\n";
-    return (WantType *)NULL;
-  }
-  if (!ptr->is_of_type(want_handle)) {
-    express_cat->error()
-      << "Attempt to cast pointer from " << ptr->get_type()
-      << " to " << want_handle << "\n";
-    if (ptr->get_type() == TypedObject::get_class_type()) {
-      express_cat->error(false)
-        << "Perhaps pointer was inadvertently deleted?\n";
-    }
     return (WantType *)NULL;
   }
 #endif
@@ -89,19 +77,7 @@ INLINE const WantType *
 _dcast(WantType *, const TypedObject *ptr) {
 #ifndef NDEBUG
   TypeHandle want_handle = _dcast_get_typehandle((WantType *)0);
-  if (ptr == (const TypedObject *)NULL) {
+  if (!_dcast_verify(want_handle, sizeof(WantType), ptr)) {
-    express_cat->warning()
-      << "Attempt to cast NULL pointer to " << want_handle << "\n";
-    return (const WantType *)NULL;
-  }
-  if (!ptr->is_of_type(want_handle)) {
-    express_cat->error()
-      << "Attempt to cast pointer from " << ptr->get_type()
-      << " to " << want_handle << "\n";
-    if (ptr->get_type() == TypedObject::get_class_type()) {
-      express_cat->error(false)
-        << "Perhaps pointer was inadvertently deleted?\n";
-    }
     return (const WantType *)NULL;
   }
 #endif

panda/src/express/dcast.cxx

@@ -0,0 +1,63 @@
+// Filename: dcast.cxx
+// Created by:  drose (07Aug01)
+//
+////////////////////////////////////////////////////////////////////
+//
+// PANDA 3D SOFTWARE
+// Copyright (c) 2001, Disney Enterprises, Inc.  All rights reserved
+//
+// All use of this software is subject to the terms of the Panda 3d
+// Software license.  You should have received a copy of this license
+// along with this source code; you will also find a current copy of
+// the license at http://www.panda3d.org/license.txt .
+//
+// To contact the maintainers of this program write to
+// panda3d@yahoogroups.com .
+//
+////////////////////////////////////////////////////////////////////
+
+#include "dcast.h"
+#include "config_express.h"
+
+#ifdef _WIN32
+#include <windows.h>  // for IsBadWritePtr()
+#endif
+
+
+#ifndef NDEBUG
+////////////////////////////////////////////////////////////////////
+//     Function: _dcast_verify
+//  Description: This function performs the actual check that the
+//               indicated TypedObject pointer is of the intended
+//               type.
+////////////////////////////////////////////////////////////////////
+bool
+_dcast_verify(TypeHandle want_handle, size_t want_size, 
+              const TypedObject *ptr) {
+  if (get_verify_dcast()) {
+    if ((ptr == (const TypedObject *)NULL)
+#if defined(_DEBUG) && defined(_WIN32)
+        || IsBadWritePtr(ptr, want_size)
+#endif
+        ) {
+      express_cat->warning()
+        << "Attempt to cast NULL or invalid pointer to " 
+        << want_handle << "\n";
+      return false;
+    }
+    if (!ptr->is_of_type(want_handle)) {
+      express_cat->error()
+        << "Attempt to cast pointer from " << ptr->get_type()
+        << " to " << want_handle << "\n";
+      if (ptr->get_type() == TypedObject::get_class_type()) {
+        express_cat->error(false)
+          << "Perhaps pointer was inadvertently deleted?\n";
+      }
+      return false;
+    }
+  }
+
+  return true;
+}
+#endif  // NDEBUG
+

panda/src/express/dcast.h

@@ -24,18 +24,14 @@
 #include "typeHandle.h"
 #include "typedObject.h"
 
-#if defined(_DEBUG) && defined(_WIN32)
-#include <windows.h>  // for IsBadWritePtr()
-#endif
-
 // The DCAST (downcast) macro is defined as a convenience for
 // downcasting from some TypedObject pointer (or a PointerTo).  It's
 // just a normal C++-style downcast, except it first checks get_type()
 // to make sure the downcasting is safe.  If you compile with NDEBUG,
-// this check is removed.
+// or set verify-dcast to #f, this check is removed.
 
 // DCAST will return NULL if the downcasting is unsafe.  If you'd
-// rather it abort out of the function (ala nassertv/nassertr), then
+// rather it abort out of the function (a la nassertv/nassertr), then
 // see DCAST_INTO_V and DCAST_INTO_R, below.
 
 template<class WantType>
@@ -72,6 +68,12 @@ INLINE WantType *_dcast_ref(WantType *&, TypedObject *ptr);
 template<class WantType>
 INLINE const WantType *_dcast_ref(WantType *&, const TypedObject *ptr);
 
+#ifndef NDEBUG
+// _dcast_verify performs the actual verification.
+EXPCL_PANDAEXPRESS bool
+_dcast_verify(TypeHandle want_handle, size_t want_size, 
+              const TypedObject *ptr);
+#endif  // NDEBUG
 
 #define DCAST_INTO_V(to_pointer, from_pointer) \
   { \

panda/src/express/express_composite1.cxx

@@ -9,6 +9,7 @@
 #include "datagramIterator.cxx"
 #include "datagramOutputFile.cxx"
 #include "datagramSink.cxx"
+#include "dcast.cxx"
 #include "get_config_path.cxx"
 #include "hashGeneratorBase.cxx"
 #include "hashVal.cxx"

panda/src/express/test_types.cxx

@@ -20,8 +20,9 @@
 #include "pointerTo.h"
 #include "pointerToArray.h"
 #include "referenceCount.h"
+#include "dcast.h"
 
-#include <notify.h>
+#include "notify.h"
 
 class ThatThingie : public TypedObject, public ReferenceCount {
 public:
@@ -270,5 +271,13 @@ main() {
 
   nout << "jarray[4] is " << jarray[4] << "\n";
 
+  cerr << "dcast thing_1: " << (void *)thing_1 << "\n";
+  ThisThingie *tt1 = DCAST(ThisThingie, thing_1);
+  cerr << "gives " << (void *)tt1 << "\n";
+
+  cerr << "dcast thing_2: " << (const void *)thing_2 << "\n";
+  const ThisThingie *tt2 = DCAST(ThisThingie, thing_2);
+  cerr << "gives " << (const void *)tt2 << "\n";
+
   return 0;
 }

panda/src/express/typeRegistry.cxx

@@ -370,11 +370,12 @@ get_child_class(TypeHandle child, int index) const {
 ////////////////////////////////////////////////////////////////////
 TypeHandle TypeRegistry::
 get_parent_towards(TypeHandle child, TypeHandle base,
-                   TypedObject *child_object) const {
+                   TypedObject *child_object) {
   const TypeRegistryNode *child_node = look_up(child, child_object);
   const TypeRegistryNode *base_node = look_up(base, NULL);
   nassertr(child_node != (TypeRegistryNode *)NULL && 
            base_node != (TypeRegistryNode *)NULL, TypeHandle::none());
+  freshen_derivations();
   return TypeRegistryNode::get_parent_towards(child_node, base_node);
 }
 
@@ -437,6 +438,12 @@ write(ostream &out) const {
 TypeRegistry *TypeRegistry::
 ptr() {
   if (_global_pointer == NULL) {
+#ifdef NOTIFY_DEBUG
+    if (express_cat->is_spam()) {
+      express_cat->spam()
+        << "Creating global TypeRegistry\n";
+    }
+#endif
     init_global_pointer();
   }
   return _global_pointer;
@@ -468,6 +475,10 @@ TypeRegistry() {
 void TypeRegistry::
 init_global_pointer() {
   _global_pointer = new TypeRegistry;
+
+  // Now that we've created the TypeRegistry, we can assign this
+  // Config variable.
+  TypeRegistryNode::_paranoid_inheritance = get_paranoid_inheritance();
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/express/typeRegistry.h

@@ -68,7 +68,7 @@ PUBLISHED:
   TypeHandle get_child_class(TypeHandle child, int index) const;
 
   TypeHandle get_parent_towards(TypeHandle child, TypeHandle base,
-                                TypedObject *child_object) const;
+                                TypedObject *child_object);
 
   static void reregister_types();
 

panda/src/express/typeRegistryNode.cxx

@@ -20,10 +20,7 @@
 
 #include <algorithm>
 
-// Define this to double-check all the inheritance derivations.
+bool TypeRegistryNode::_paranoid_inheritance;
-#ifndef NDEBUG
-#define PARANOID_INHERITANCE
-#endif
 
 ////////////////////////////////////////////////////////////////////
 //     Function: TypeRegistryNode::Constructor
@@ -51,6 +48,11 @@ is_derived_from(const TypeRegistryNode *child, const TypeRegistryNode *base) {
   // code.  Therefore, we go through some pains to make this function
   // as efficient as possible.
 
+  // (Actually, it appears that the function is not called as often as
+  // I'd first thought, and it wasn't really all that expensive to
+  // begin with.  So much of this complexity is of limited usefulness.
+  // Oh well.)
+
   // First, compare the subtree tops.  If they are the same, then this
   // node and the base node are within the same single-inheritance
   // subtree, and we can use our bitmask trick to determine the
@@ -62,23 +64,25 @@ is_derived_from(const TypeRegistryNode *child, const TypeRegistryNode *base) {
     bool derives = 
       Inherit::is_derived_from(child->_inherit, base->_inherit);
 
-#ifdef PARANOID_INHERITANCE
+#ifndef NDEBUG
-    bool paranoid_derives = check_derived_from(child, base);
+    if (_paranoid_inheritance) {
-    if (derives != paranoid_derives) {
+      bool paranoid_derives = check_derived_from(child, base);
-      express_cat.error()
+      if (derives != paranoid_derives) {
-        << "Inheritance test for " << child->_name 
+        express_cat.error()
-        << " from " << base->_name << " failed!\n"
+          << "Inheritance test for " << child->_name 
-        << "Result: " << derives << " should have been: "
+          << " from " << base->_name << " failed!\n"
-        << paranoid_derives << "\n"
+          << "Result: " << derives << " should have been: "
-        << "Classes are in the same single inheritance subtree, children of "
+          << paranoid_derives << "\n"
-        << child->_inherit._top->_name << "\n"
+          << "Classes are in the same single inheritance subtree, children of "
-        << hex
+          << child->_inherit._top->_name << "\n"
-        << child->_name << " has mask " << child->_inherit._mask
+          << hex
-        << " and bits " << child->_inherit._bits << "\n"
+          << child->_name << " has mask " << child->_inherit._mask
-        << base->_name << " has mask " << base->_inherit._mask
+          << " and bits " << child->_inherit._bits << "\n"
+          << base->_name << " has mask " << base->_inherit._mask
         << " and bits " << base->_inherit._bits << "\n"
-        << dec;
+          << dec;
-      return paranoid_derives;
+        return paranoid_derives;
+      }
     }
 #endif
 
@@ -122,19 +126,21 @@ is_derived_from(const TypeRegistryNode *child, const TypeRegistryNode *base) {
     ++ti;
   }
 
-#ifdef PARANOID_INHERITANCE
+#ifndef NDEBUG
-  bool paranoid_derives = check_derived_from(child, base);
+  if (_paranoid_inheritance) {
-  if (derives != paranoid_derives) {
+    bool paranoid_derives = check_derived_from(child, base);
-    express_cat.error()
+    if (derives != paranoid_derives) {
-      << "Inheritance test for " << child->_name 
+      express_cat.error()
-      << " from " << base->_name << " failed!\n"
+        << "Inheritance test for " << child->_name 
-      << "Result: " << derives << " should have been: "
+        << " from " << base->_name << " failed!\n"
-      << paranoid_derives << "\n"
+        << "Result: " << derives << " should have been: "
-      << child->_name << " is a descendent of "
+        << paranoid_derives << "\n"
-      << child_top->_name << "\n"
+        << child->_name << " is a descendent of "
-      << base->_name << " is a descendent of "
+        << child_top->_name << "\n"
+        << base->_name << " is a descendent of "
       << base_top->_name << "\n";
-    return paranoid_derives;
+      return paranoid_derives;
+    }
   }
 #endif
 
@@ -339,6 +345,12 @@ r_build_subtrees(TypeRegistryNode *top, int bit_count,
 ////////////////////////////////////////////////////////////////////
 TypeRegistryNode::TopInheritance::const_iterator TypeRegistryNode::
 find_top_inherit(const TypeRegistryNode *base) const {
+  // If the need arises, we can make this a binary search to
+  // theoretically save even more time, since the list is already
+  // sorted.  However, the lists do tend to be short, and this
+  // function doesn't get called too awful much, so a linear search is
+  // not as bad as you might think.
+
   TopInheritance::const_iterator ti;
   for (ti = _top_inheritance.begin(); ti != _top_inheritance.end(); ++ti) {
     if ((*ti)._top == base) {

panda/src/express/typeRegistryNode.h

@@ -52,6 +52,8 @@ public:
   Classes _parent_classes;
   Classes _child_classes;
 
+  static bool _paranoid_inheritance;
+
 private:
   typedef int SubtreeMaskType;
 

panda/src/graph/allTransitionsWrapper.cxx

@@ -96,6 +96,8 @@ output(ostream &out) const {
 void AllTransitionsWrapper::
 write(ostream &out, int indent_level) const {
   if (_cache != (NodeTransitionCache *)NULL) {
+    indent(out, indent_level)
+      << "Cache pointer is " << _cache << "\n";
     _cache->write(out, indent_level);
   }
 }

panda/src/graph/nodeTransition.I

@@ -111,37 +111,6 @@ operator >= (const NodeTransition &other) const {
   return compare_to(other) >= 0;
 }
 
-////////////////////////////////////////////////////////////////////
-//     Function: NodeTransition::compare_to
-//       Access: Public
-//  Description: This function works like strcmp(): it compares the
-//               two transitions and returns a number less than zero
-//               if this transition sorts before the other one, equal
-//               to zero if they are equivalent, or greater than zero
-//               if this transition sorts after the other one.
-//
-//               This imposes an arbitrary sorting order across all
-//               transitions, whose sole purpose is to allow grouping
-//               of equivalent transitions together in STL structures
-//               like maps and sets.
-////////////////////////////////////////////////////////////////////
-INLINE_GRAPH int NodeTransition::
-compare_to(const NodeTransition &other) const {
-  TypeHandle my_handle = get_handle();
-  TypeHandle other_handle = other.get_handle();
-
-  if (my_handle != other_handle) {
-    return
-      (my_handle < other_handle) ? -1 : 1;
-
-  } else if (_priority != other._priority) {
-    return _priority - other._priority;
-
-  } else {
-    return internal_compare_to(&other);
-  }
-}
-
 ////////////////////////////////////////////////////////////////////
 //     Function: NodeTransition::generate_hash
 //       Access: Public

panda/src/graph/nodeTransition.cxx

@@ -25,6 +25,42 @@
 
 TypeHandle NodeTransition::_type_handle;
 
+////////////////////////////////////////////////////////////////////
+//     Function: NodeTransition::compare_to
+//       Access: Public
+//  Description: This function works like strcmp(): it compares the
+//               two transitions and returns a number less than zero
+//               if this transition sorts before the other one, equal
+//               to zero if they are equivalent, or greater than zero
+//               if this transition sorts after the other one.
+//
+//               This imposes an arbitrary sorting order across all
+//               transitions, whose sole purpose is to allow grouping
+//               of equivalent transitions together in STL structures
+//               like maps and sets.
+////////////////////////////////////////////////////////////////////
+int NodeTransition::
+compare_to(const NodeTransition &other) const {
+  if (this == &other) {
+    // Same pointer, no comparison necessary.
+    return 0;
+  }
+
+  TypeHandle my_handle = get_handle();
+  TypeHandle other_handle = other.get_handle();
+
+  if (my_handle != other_handle) {
+    return
+      (my_handle < other_handle) ? -1 : 1;
+
+  } else if (_priority != other._priority) {
+    return _priority - other._priority;
+
+  } else {
+    return internal_compare_to(&other);
+  }
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: NodeTransition::get_handle
 //       Access: Public, Virtual

panda/src/graph/nodeTransition.h

@@ -73,7 +73,7 @@ public:
   INLINE_GRAPH bool operator > (const NodeTransition &other) const;
   INLINE_GRAPH bool operator >= (const NodeTransition &other) const;
 
-  INLINE_GRAPH int compare_to(const NodeTransition &other) const;
+  int compare_to(const NodeTransition &other) const;
   INLINE_GRAPH void generate_hash(GraphHashGenerator &hash) const;
 
 PUBLISHED:

panda/src/graph/nodeTransitionCache.cxx

@@ -550,7 +550,9 @@ void NodeTransitionCache::
 write(ostream &out, int indent_level) const {
   Cache::const_iterator ci;
   for (ci = _cache.begin(); ci != _cache.end(); ++ci) {
-    indent(out, indent_level) << (*ci).first << "\n";
+    indent(out, indent_level) 
+      << (*ci).first << ", ptr = "
+      << (const PT(NodeTransition) &)(*ci).second << "\n";
     (*ci).second.write(out, indent_level + 2);
   }
 }

panda/src/sgraphutil/quickRenderTraverser.I

@@ -16,16 +16,3 @@
 //
 ////////////////////////////////////////////////////////////////////
 
-
-////////////////////////////////////////////////////////////////////
-//     Function: QuickRenderTraverser::backward_arc
-//       Access: Public
-//  Description:
-////////////////////////////////////////////////////////////////////
-INLINE void QuickRenderTraverser::
-backward_arc(NodeRelation *arc, NullTransitionWrapper &,
-             NullTransitionWrapper &, NullTransitionWrapper &,
-             const QuickRenderLevelState &) {
-  mark_backward_arc(arc);
-  _arc_chain.pop_back();
-}

panda/src/sgraphutil/quickRenderTraverser.cxx

@@ -31,6 +31,7 @@ TypeHandle QuickRenderTraverser::_type_handle;
 
 #ifndef CPPPARSER
 PStatCollector QuickRenderTraverser::_draw_pcollector("Draw:Quick");
+static PStatCollector _fooby_pcollector("Draw:Quick:Fooby");
 #endif
 
 ////////////////////////////////////////////////////////////////////
@@ -77,51 +78,39 @@ traverse(Node *root, const AllTransitionsWrapper &initial_state) {
   level_state._as_of = UpdateSeq::initial();
   level_state._under_instance = false;
 
-  df_traverse(_root, *this, NullTransitionWrapper(), level_state, _graph_type);
+  const DownRelationPointers &drp =
+    _root->find_connection(_graph_type).get_down();
+
+  // Now visit each of the children in turn.
+  DownRelationPointers::const_iterator drpi;
+  for (drpi = drp.begin(); drpi != drp.end(); ++drpi) {
+    NodeRelation *arc = *drpi;
+    r_traverse(DCAST(RenderRelation, arc), level_state);
+  }
 
   _root = (Node *)NULL;
 }
 
 ////////////////////////////////////////////////////////////////////
-//     Function: QuickRenderTraverser::forward_arc
+//     Function: QuickRenderTraverser::r_traverse
-//       Access: Public
+//       Access: Private
 //  Description:
 ////////////////////////////////////////////////////////////////////
-bool QuickRenderTraverser::
+void QuickRenderTraverser::
-forward_arc(NodeRelation *arc, NullTransitionWrapper &,
+r_traverse(RenderRelation *arc, const QuickRenderLevelState &level_state) {
-            NullTransitionWrapper &, NullTransitionWrapper &,
-            QuickRenderLevelState &level_state) {
   if (arc->has_transition(PruneTransition::get_class_type())) {
-    return false;
+    return;
   }
 
+  QuickRenderLevelState next_level_state(level_state);
   Node *node = arc->get_child();
 
   if (node->get_num_parents(_graph_type) != 1) {
-    /*
+    next_level_state._under_instance = true;
-    const UpRelationPointers &urp = node->find_connection(_graph_type).get_up();
-    int num_parents = urp.size();
-
-    sgraphutil_cat.warning()
-      << "Cannot support instancing via QuickRenderTraverser; "
-      << *node << " has " << num_parents << " parents.\n"
-      << "  parents are: ";
-    nassertr(num_parents > 1, false);
-
-    NodeRelation *parent_arc = urp[0];
-    sgraphutil_cat.warning(false) << *parent_arc->get_parent();
-    for (int i = 1; i < num_parents; i++) {
-      parent_arc = urp[i];
-      sgraphutil_cat.warning(false)
-        << ", " << *parent_arc->get_parent();
-    }
-    sgraphutil_cat.warning(false) << "\n";
-
-    return false;
-    */
-    level_state._under_instance = true;
   }
 
+  _arc_chain.push_back(arc);
+
   if (implicit_app_traversal) {
     node->app_traverse(_arc_chain);
   }
@@ -132,13 +121,10 @@ forward_arc(NodeRelation *arc, NullTransitionWrapper &,
   UpdateSeq now = last_graph_update(_graph_type);
 
   UpdateSeq last_update = arc->get_last_update();
-  if (level_state._as_of < last_update) {
+  if (next_level_state._as_of < last_update) {
-    level_state._as_of = last_update;
+    next_level_state._as_of = last_update;
   }
   
-  mark_forward_arc(arc);
-  _arc_chain.push_back(arc);
-  
   _gsg->_nodes_pcollector.add_level(1);
 
   if (node->is_of_type(GeomNode::get_class_type())) {
@@ -148,26 +134,16 @@ forward_arc(NodeRelation *arc, NullTransitionWrapper &,
     GeomNode *gnode = DCAST(GeomNode, node);
     AllTransitionsWrapper trans;
 
-    if (level_state._under_instance) {
+    if (next_level_state._under_instance) {
       // If we're under an instance node, we have to use the more
       // expensive wrt() operation.
       wrt(node, begin(), end(), (Node *)NULL, trans, _graph_type);
 
     } else {
       // Otherwise, we can use wrt_subtree() to get better caching.
-
-      //Node *top_subtree = 
       wrt_subtree(arc, NULL,
-                  level_state._as_of, now,
+                  next_level_state._as_of, now,
                   trans, _graph_type);
-
-      /*
-      cerr << "top_subtree is " << (void *)top_subtree;
-      if (top_subtree != (Node *)NULL) {
-        cerr << " is " << *top_subtree;
-      }
-      cerr << "\n";
-      */
     }
 
     AllTransitionsWrapper attrib;
@@ -177,5 +153,15 @@ forward_arc(NodeRelation *arc, NullTransitionWrapper &,
     gnode->draw(_gsg);
   }
 
-  return true;
+  const DownRelationPointers &drp =
+    node->find_connection(_graph_type).get_down();
+
+  // Now visit each of the children in turn.
+  DownRelationPointers::const_iterator drpi;
+  for (drpi = drp.begin(); drpi != drp.end(); ++drpi) {
+    NodeRelation *arc = *drpi;
+    r_traverse(DCAST(RenderRelation, arc), next_level_state);
+  }
+
+  _arc_chain.pop_back();
 }

panda/src/sgraphutil/quickRenderTraverser.h

@@ -24,13 +24,12 @@
 #include "quickRenderLevelState.h"
 
 #include "renderTraverser.h"
-#include "traverserVisitor.h"
-#include "nullTransitionWrapper.h"
 #include "allTransitionsWrapper.h"
 #include "pStatCollector.h"
 
 class GraphicsStateGuardian;
 class AllTransitionsWrapper;
+class RenderRelation;
 
 ////////////////////////////////////////////////////////////////////
 //       Class : QuickRenderTraverser
@@ -42,9 +41,7 @@ class AllTransitionsWrapper;
 //               However, it does not support instancing, nor
 //               view-frustum culling.
 ////////////////////////////////////////////////////////////////////
-class EXPCL_PANDA QuickRenderTraverser :
+class EXPCL_PANDA QuickRenderTraverser : public RenderTraverser {
-  public RenderTraverser,
-  public TraverserVisitor<NullTransitionWrapper, QuickRenderLevelState> {
 public:
   QuickRenderTraverser(GraphicsStateGuardian *gsg, TypeHandle graph_type,
                        const ArcChain &arc_chain = ArcChain());
@@ -53,18 +50,8 @@ public:
   virtual void traverse(Node *root,
                         const AllTransitionsWrapper &initial_state);
 
-public:
+private:
-  // These methods, from parent class TraverserVisitor, define the
+  void r_traverse(RenderRelation *arc, const QuickRenderLevelState &level_state);
-  // behavior of the RenderTraverser as it traverses the graph.
-  // Normally you would never call these directly.
-  bool forward_arc(NodeRelation *arc, NullTransitionWrapper &trans,
-                   NullTransitionWrapper &pre, NullTransitionWrapper &post,
-                   QuickRenderLevelState &level_state);
-
-  INLINE void
-  backward_arc(NodeRelation *arc, NullTransitionWrapper &trans,
-               NullTransitionWrapper &pre, NullTransitionWrapper &post,
-               const QuickRenderLevelState &level_state);
 
 private:
   Node *_root;