Cull improvements: don't compute bounds when not necessary

panda/src/grutil/frameRateMeter.cxx

@@ -21,6 +21,7 @@
 #include "depthTestAttrib.h"
 #include "depthWriteAttrib.h"
 #include "pStatTimer.h"
+#include "omniBoundingVolume.h"
 #include <stdio.h>  // For sprintf/snprintf
 
 PStatCollector FrameRateMeter::_show_fps_pcollector("*:Show fps");
@@ -39,6 +40,10 @@ FrameRateMeter(const string &name) :
 
   set_cull_callback();
 
+  // Don't do frustum culling, as the text will always be in view.
+  set_bounds(new OmniBoundingVolume());
+  set_final(true);
+
   Thread *current_thread = Thread::get_current_thread();
 
   _show_milliseconds = frame_rate_meter_milliseconds;
@@ -103,6 +108,11 @@ setup_window(GraphicsOutput *window) {
   _root.set_material_off(1);
   _root.set_two_sided(1, 1);
 
+  // If we don't set this explicitly, Panda will cause it to be rendered
+  // in a back-to-front cull bin, which will cause the bounding volume
+  // to be computed unnecessarily.  Saves a little bit of overhead.
+  _root.set_bin("unsorted", 0);
+
   // Create a display region that covers the entire window.
   _display_region = _window->make_mono_display_region();
   _display_region->set_sort(frame_rate_meter_layer_sort);
@@ -188,7 +198,6 @@ cull_callback(CullTraverser *trav, CullTraverserData &data) {
   if (aspect_ratio != _last_aspect_ratio) {
     _aspect_ratio_transform = TransformState::make_scale(LVecBase3(aspect_ratio, 1, 1));
     _last_aspect_ratio = aspect_ratio;
-    cerr << aspect_ratio << "\n";
   }
   data._net_transform = data._net_transform->compose(_aspect_ratio_transform);
 

panda/src/pgraph/cullTraverserData.I

@@ -15,7 +15,7 @@
 ////////////////////////////////////////////////////////////////////
 //     Function: CullTraverserData::Constructor
 //       Access: Public
-//  Description: 
+//  Description:
 ////////////////////////////////////////////////////////////////////
 INLINE CullTraverserData::
 CullTraverserData(const NodePath &start,
@@ -29,16 +29,18 @@ CullTraverserData(const NodePath &start,
   _state(state),
   _view_frustum(view_frustum),
   _cull_planes(CullPlanes::make_empty()),
-  _draw_mask(DrawMask::all_on())
+  _draw_mask(DrawMask::all_on()),
+  _portal_depth(0)
 {
-  _node_reader.check_bounds();
-  _portal_depth = 0;
+  // Only update the bounding volume if we're going to end up needing it.
+  bool check_bounds = (view_frustum != (GeometricBoundingVolume *)NULL);
+  _node_reader.check_cached(check_bounds);
 }
 
 ////////////////////////////////////////////////////////////////////
 //     Function: CullTraverserData::Copy Constructor
 //       Access: Public
-//  Description: 
+//  Description:
 ////////////////////////////////////////////////////////////////////
 INLINE CullTraverserData::
 CullTraverserData(const CullTraverserData &copy) :
@@ -56,7 +58,7 @@ CullTraverserData(const CullTraverserData &copy) :
 ////////////////////////////////////////////////////////////////////
 //     Function: CullTraverserData::Copy Assignment Operator
 //       Access: Public
-//  Description: 
+//  Description:
 ////////////////////////////////////////////////////////////////////
 INLINE void CullTraverserData::
 operator = (const CullTraverserData &copy) {
@@ -84,16 +86,19 @@ CullTraverserData(const CullTraverserData &parent, PandaNode *child) :
   _state(parent._state),
   _view_frustum(parent._view_frustum),
   _cull_planes(parent._cull_planes),
-  _draw_mask(parent._draw_mask)
+  _draw_mask(parent._draw_mask),
+  _portal_depth(parent._portal_depth)
 {
-  _node_reader.check_bounds();
-  _portal_depth = parent._portal_depth;
+  // Only update the bounding volume if we're going to end up needing it.
+  bool check_bounds = !_cull_planes->is_empty() ||
+                    (_view_frustum != (GeometricBoundingVolume *)NULL);
+  _node_reader.check_cached(check_bounds);
 }
 
 ////////////////////////////////////////////////////////////////////
 //     Function: CullTraverserData::Destructor
 //       Access: Public
-//  Description: 
+//  Description:
 ////////////////////////////////////////////////////////////////////
 INLINE CullTraverserData::
 ~CullTraverserData() {

panda/src/pgraph/cullTraverserData.cxx

@@ -50,7 +50,7 @@ void CullTraverserData::
 apply_transform_and_state(CullTraverser *trav) {
   CPT(RenderState) node_state = _node_reader.get_state();
 
-  if (trav->has_tag_state_key() && 
+  if (trav->has_tag_state_key() &&
       _node_reader.has_tag(trav->get_tag_state_key())) {
     // Here's a node that has been tagged with the special key for our
     // current camera.  This indicates some special state transition
@@ -74,8 +74,8 @@ apply_transform_and_state(CullTraverser *trav) {
 //               data.  This also evaluates billboards, etc.
 ////////////////////////////////////////////////////////////////////
 void CullTraverserData::
-apply_transform_and_state(CullTraverser *trav, 
-                          CPT(TransformState) node_transform, 
+apply_transform_and_state(CullTraverser *trav,
+                          CPT(TransformState) node_transform,
                           CPT(RenderState) node_state,
                           CPT(RenderEffects) node_effects,
                           const RenderAttrib *off_clip_planes) {
@@ -98,7 +98,7 @@ apply_transform_and_state(CullTraverser *trav,
         _cull_planes = CullPlanes::make_empty();
 
       } else {
-        CPT(TransformState) inv_transform = 
+        CPT(TransformState) inv_transform =
           node_transform->invert_compose(TransformState::make_identity());
 
         // Copy the bounding volumes for the frustums so we can
@@ -116,7 +116,7 @@ apply_transform_and_state(CullTraverser *trav,
   _state = _state->compose(node_state);
 
   if (clip_plane_cull) {
-    _cull_planes = _cull_planes->apply_state(trav, this, 
+    _cull_planes = _cull_planes->apply_state(trav, this,
                                              DCAST(ClipPlaneAttrib, node_state->get_attrib(ClipPlaneAttrib::get_class_slot())),
                                              DCAST(ClipPlaneAttrib, off_clip_planes),
                                              DCAST(OccluderEffect, node_effects->get_effect(OccluderEffect::get_class_type())));
@@ -130,19 +130,18 @@ apply_transform_and_state(CullTraverser *trav,
 ////////////////////////////////////////////////////////////////////
 bool CullTraverserData::
 is_in_view_impl() {
-  CPT(BoundingVolume) node_volume = _node_reader.get_bounds();
-  nassertr(node_volume->is_of_type(GeometricBoundingVolume::get_class_type()), false);
-  const GeometricBoundingVolume *node_gbv =
-    DCAST(GeometricBoundingVolume, node_volume);
+  const GeometricBoundingVolume *node_gbv = NULL;
 
   if (_view_frustum != (GeometricBoundingVolume *)NULL) {
+    DCAST_INTO_R(node_gbv, _node_reader.get_bounds(), false)
+
     int result = _view_frustum->contains(node_gbv);
-    
+
     if (pgraph_cat.is_spam()) {
       pgraph_cat.spam()
         << _node_path << " cull result = " << hex << result << dec << "\n";
     }
-    
+
     if (result == BoundingVolume::IF_no_intersection) {
       // No intersection at all.  Cull.
 #ifdef NDEBUG
@@ -151,7 +150,7 @@ is_in_view_impl() {
       if (!fake_view_frustum_cull) {
         return false;
       }
-      
+
       // If we have fake view-frustum culling enabled, instead of
       // actually culling an object we simply force it to be drawn in
       // red wireframe.
@@ -179,10 +178,14 @@ is_in_view_impl() {
   }
 
   if (!_cull_planes->is_empty()) {
+    if (node_gbv == (const GeometricBoundingVolume *)NULL) {
+      DCAST_INTO_R(node_gbv, _node_reader.get_bounds(), false)
+    }
+
     // Also cull against the current clip planes.
     int result;
     _cull_planes = _cull_planes->do_cull(result, _state, node_gbv);
-    
+
     if (pgraph_cat.is_spam()) {
       pgraph_cat.spam()
         << _node_path << " cull planes cull result = " << hex
@@ -194,7 +197,7 @@ is_in_view_impl() {
       // Even though the node may be partially within the clip planes,
       // do no more culling against them below this node.
       _cull_planes = CullPlanes::make_empty();
-      
+
       if (pgraph_cat.is_spam()) {
         pgraph_cat.spam()
           << _node_path << " is_final, cull planes disabled, state:\n";

panda/src/pgraph/pandaNode.I

@@ -632,14 +632,14 @@ get_internal_vertices(Thread *current_thread) const {
 //     Function: PandaNode::is_bounds_stale
 //       Access: Published
 //  Description: Returns true if the bounding volume of this node is
-//                stale and will be implicitly recomputed at the next
-//                call to get_bounds(), or false if it is fresh and
-//                need not be recomputed.
+//               stale and will be implicitly recomputed at the next
+//               call to get_bounds(), or false if it is fresh and
+//               need not be recomputed.
 ////////////////////////////////////////////////////////////////////
 bool PandaNode::
 is_bounds_stale() const {
   CDReader cdata(_cycler);
-  return (cdata->_last_update != cdata->_next_update);
+  return (cdata->_last_bounds_update != cdata->_next_update);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -726,14 +726,13 @@ mark_bounds_stale(int pipeline_stage, Thread *current_thread) const {
   bool is_stale_bounds;
   {
     CDStageReader cdata(_cycler, pipeline_stage, current_thread);
-    is_stale_bounds = (cdata->_last_update != cdata->_next_update);
+    is_stale_bounds = (cdata->_last_bounds_update != cdata->_next_update);
   }
   if (!is_stale_bounds) {
     ((PandaNode *)this)->force_bounds_stale(pipeline_stage, current_thread);
   }
 }
 
-
 ////////////////////////////////////////////////////////////////////
 //     Function: PandaNode::mark_internal_bounds_stale
 //       Access: Protected
@@ -907,7 +906,7 @@ operator < (const DownConnection &other) const {
 ////////////////////////////////////////////////////////////////////
 //     Function: PandaNode::DownConnection::get_child
 //       Access: Public
-//  Description: 
+//  Description:
 ////////////////////////////////////////////////////////////////////
 INLINE PandaNode *PandaNode::DownConnection::
 get_child() const {
@@ -927,7 +926,7 @@ set_child(PandaNode *child) {
 ////////////////////////////////////////////////////////////////////
 //     Function: PandaNode::DownConnection::get_sort
 //       Access: Public
-//  Description: 
+//  Description:
 ////////////////////////////////////////////////////////////////////
 INLINE int PandaNode::DownConnection::
 get_sort() const {
@@ -963,7 +962,7 @@ operator < (const UpConnection &other) const {
 ////////////////////////////////////////////////////////////////////
 //     Function: PandaNode::UpConnection::get_parent
 //       Access: Public
-//  Description: 
+//  Description:
 ////////////////////////////////////////////////////////////////////
 INLINE PandaNode *PandaNode::UpConnection::
 get_parent() const {
@@ -1945,7 +1944,7 @@ get_parents() const {
 ////////////////////////////////////////////////////////////////////
 //     Function: PandaNode::BamReaderAuxDataDown::Constructor
 //       Access: Public
-//  Description: 
+//  Description:
 ////////////////////////////////////////////////////////////////////
 INLINE PandaNode::BamReaderAuxDataDown::
 BamReaderAuxDataDown() :

panda/src/pgraph/pandaNode.h

@@ -616,6 +616,11 @@ private:
     // When _last_update != _next_update, this cache is stale.
     UpdateSeq _last_update, _next_update;
 
+    // We don't always update the bounding volume and number of
+    // nested vertices.  This indicates the last time they were changed.
+    // It is never higher than _last_update.
+    UpdateSeq _last_bounds_update;
+
   public:
     // This section stores the links to other nodes above and below
     // this node in the graph.
@@ -669,7 +674,8 @@ private:
   typedef CycleDataStageWriter<CData> CDStageWriter;
 
   int do_find_child(PandaNode *node, const Down *down) const;
-  CDStageWriter update_bounds(int pipeline_stage, CDLockedStageReader &cdata);
+  CDStageWriter update_cached(bool update_bounds, int pipeline_stage,
+                              CDLockedStageReader &cdata);
 
   static DrawMask _overall_bit;
 
@@ -823,7 +829,7 @@ public:
 
   INLINE void release();
 
-  void check_bounds() const;
+  void check_cached(bool update_bounds) const;
 
   INLINE void compose_draw_mask(DrawMask &running_draw_mask) const;
   INLINE bool compare_draw_mask(DrawMask running_draw_mask,

panda/src/pgraph/sceneGraphReducer.cxx

@@ -117,8 +117,8 @@ flatten(PandaNode *root, int combine_siblings_bits) {
       num_pass_nodes += r_flatten(root, child_node, combine_siblings_bits);
     }
 
-    if (combine_siblings_bits != 0 && 
-        root->get_num_children() >= 2 && 
+    if (combine_siblings_bits != 0 &&
+        root->get_num_children() >= 2 &&
         root->safe_to_combine_children()) {
       num_pass_nodes += flatten_siblings(root, combine_siblings_bits);
     }
@@ -381,7 +381,7 @@ r_apply_attribs(PandaNode *node, const AccumulatedAttribs &attribs,
             pgraph_cat.spam()
               << "Duplicated " << *child_node << "\n";
           }
-          
+
           new_node->copy_children(child_node);
           node->replace_child(child_node, new_node);
           child_node = new_node;
@@ -416,10 +416,17 @@ r_flatten(PandaNode *grandparent_node, PandaNode *parent_node,
           int combine_siblings_bits) {
   if (pgraph_cat.is_spam()) {
     pgraph_cat.spam()
-      << "SceneGraphReducer::r_flatten(" << *grandparent_node << ", " 
+      << "SceneGraphReducer::r_flatten(" << *grandparent_node << ", "
       << *parent_node << ", " << hex << combine_siblings_bits << dec
       << ")\n";
   }
+
+  if ((combine_siblings_bits & (CS_geom_node | CS_other | CS_recurse)) != 0) {
+    // Unset CS_within_radius, since we're going to flatten everything
+    // anyway.  This avoids needlessly calculating the bounding volume.
+    combine_siblings_bits &= ~CS_within_radius;
+  }
+
   int num_nodes = 0;
 
   if (!parent_node->safe_to_flatten_below()) {
@@ -428,7 +435,7 @@ r_flatten(PandaNode *grandparent_node, PandaNode *parent_node,
         << "Not traversing further; " << *parent_node
         << " doesn't allow flattening below itself.\n";
     }
-    
+
   } else {
     if ((combine_siblings_bits & CS_within_radius) != 0) {
       CPT(BoundingVolume) bv = parent_node->get_bounds();
@@ -462,16 +469,16 @@ r_flatten(PandaNode *grandparent_node, PandaNode *parent_node,
         num_nodes += r_flatten(parent_node, child_node, combine_siblings_bits);
       }
     }
-    
+
     // Now that the above loop has removed some children, the child
     // list saved above is no longer accurate, so hereafter we must
     // ask the node for its real child list.
-    
+
     // If we have CS_recurse set, then we flatten siblings before
     // trying to flatten children.  Otherwise, we flatten children
     // first, and then flatten siblings, which avoids overly
     // enthusiastic flattening.
-    if ((combine_siblings_bits & CS_recurse) != 0 && 
+    if ((combine_siblings_bits & CS_recurse) != 0 &&
         parent_node->get_num_children() >= 2 &&
         parent_node->safe_to_combine_children()) {
       num_nodes += flatten_siblings(parent_node, combine_siblings_bits);
@@ -482,11 +489,11 @@ r_flatten(PandaNode *grandparent_node, PandaNode *parent_node,
       // out.
       PT(PandaNode) child_node = parent_node->get_child(0);
       int child_sort = parent_node->get_child_sort(0);
-      
+
       if (consider_child(grandparent_node, parent_node, child_node)) {
         // Ok, do it.
         parent_node->remove_child(child_node);
-        
+
         if (do_flatten_child(grandparent_node, parent_node, child_node)) {
           // Done!
           num_nodes++;
@@ -498,7 +505,7 @@ r_flatten(PandaNode *grandparent_node, PandaNode *parent_node,
     }
 
     if ((combine_siblings_bits & CS_recurse) == 0 &&
-        (combine_siblings_bits & ~CS_recurse) != 0 && 
+        (combine_siblings_bits & ~CS_recurse) != 0 &&
         parent_node->get_num_children() >= 2 &&
         parent_node->safe_to_combine_children()) {
       num_nodes += flatten_siblings(parent_node, combine_siblings_bits);
@@ -617,7 +624,7 @@ flatten_siblings(PandaNode *parent_node, int combine_siblings_bits) {
           ++ai2;
 
           if (consider_siblings(parent_node, child1, child2)) {
-            PT(PandaNode) new_node = 
+            PT(PandaNode) new_node =
               do_flatten_siblings(parent_node, child1, child2);
             if (new_node != (PandaNode *)NULL) {
               // We successfully collapsed a node.
@@ -644,7 +651,7 @@ flatten_siblings(PandaNode *parent_node, int combine_siblings_bits) {
 //               node may be removed, false if it should be kept.
 ////////////////////////////////////////////////////////////////////
 bool SceneGraphReducer::
-consider_child(PandaNode *grandparent_node, PandaNode *parent_node, 
+consider_child(PandaNode *grandparent_node, PandaNode *parent_node,
                PandaNode *child_node) {
   if (!parent_node->safe_to_combine() || !child_node->safe_to_combine()) {
     // One or both nodes cannot be safely combined with another node;
@@ -696,7 +703,7 @@ consider_siblings(PandaNode *parent_node, PandaNode *child1,
 //               successfully collapsed, false if we chickened out.
 ////////////////////////////////////////////////////////////////////
 bool SceneGraphReducer::
-do_flatten_child(PandaNode *grandparent_node, PandaNode *parent_node, 
+do_flatten_child(PandaNode *grandparent_node, PandaNode *parent_node,
                  PandaNode *child_node) {
   if (pgraph_cat.is_spam()) {
     pgraph_cat.spam()
@@ -707,7 +714,7 @@ do_flatten_child(PandaNode *grandparent_node, PandaNode *parent_node,
   if (new_parent == (PandaNode *)NULL) {
     if (pgraph_cat.is_spam()) {
       pgraph_cat.spam()
-        << "Decided not to collapse " << *parent_node 
+        << "Decided not to collapse " << *parent_node
         << " and " << *child_node << "\n";
     }
     return false;
@@ -820,7 +827,7 @@ r_remove_column(PandaNode *node, const InternalName *column,
       ++num_changed;
     }
   }
-    
+
   PandaNode::Children children = node->get_children();
   int num_children = children.get_num_children();
   for (int i = 0; i < num_children; ++i) {
@@ -845,7 +852,7 @@ r_make_compatible_state(PandaNode *node, GeomTransformer &transformer) {
       ++num_changed;
     }
   }
-  
+
   PandaNode::Children children = node->get_children();
   int num_children = children.get_num_children();
   for (int i = 0; i < num_children; ++i) {
@@ -890,7 +897,7 @@ r_collect_vertex_data(PandaNode *node, int collect_bits,
     PandaNode::Children children = node->get_children();
     int num_children = children.get_num_children();
     for (int i = 0; i < num_children; ++i) {
-      num_adjusted += 
+      num_adjusted +=
         r_collect_vertex_data(children.get_child(i), collect_bits, new_transformer, format_only);
     }
 
@@ -902,7 +909,7 @@ r_collect_vertex_data(PandaNode *node, int collect_bits,
     if (node->is_geom_node()) {
       num_adjusted += transformer.collect_vertex_data(DCAST(GeomNode, node), collect_bits, format_only);
     }
-    
+
     PandaNode::Children children = node->get_children();
     int num_children = children.get_num_children();
     for (int i = 0; i < num_children; ++i) {
@@ -943,7 +950,7 @@ r_make_nonindexed(PandaNode *node, int nonindexed_bits) {
           geom->get_usage_hint() != Geom::UH_static) {
         this_geom_bits |= MN_avoid_dynamic;
       }
-      
+
       if ((nonindexed_bits & this_geom_bits) == 0) {
         // The geom meets the user's qualifications for making
         // nonindexed, so do it.
@@ -956,10 +963,10 @@ r_make_nonindexed(PandaNode *node, int nonindexed_bits) {
   PandaNode::Children children = node->get_children();
   int num_children = children.get_num_children();
   for (int i = 0; i < num_children; ++i) {
-    num_changed += 
+    num_changed +=
       r_make_nonindexed(children.get_child(i), nonindexed_bits);
   }
-    
+
   return num_changed;
 }