From 597e892b982c03c907d45515e4694c2fce7077ec Mon Sep 17 00:00:00 2001
From: David Rose <david@ddrose.com>
Date: Mon, 11 Mar 2002 19:28:28 +0000
Subject: [PATCH] optimize TransformState inverse operation a bit more

---
 panda/src/pgraph/cullTraverserData.I |  5 ++-
 panda/src/pgraph/qpcullTraverser.cxx | 10 ++++++
 panda/src/pgraph/transformState.cxx  | 47 +++++++++++++++++-----------
 panda/src/pgraph/transformState.h    |  1 +
 4 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/panda/src/pgraph/cullTraverserData.I b/panda/src/pgraph/cullTraverserData.I
index cd8ec4b709..8628c68007 100644
--- a/panda/src/pgraph/cullTraverserData.I
+++ b/panda/src/pgraph/cullTraverserData.I
@@ -90,8 +90,11 @@ INLINE CullTraverserData::
 INLINE bool CullTraverserData::
 is_in_view(PandaNode *node) {
   if (_view_frustum == (GeometricBoundingVolume *)NULL) {
-    // If we don't have a frustum, we're always in.
+    // If the transform is valid, but we don't have a frustum, it's
+    // always in.
     return true;
   }
+
+  // Otherwise, compare the bounding volume to the frustum.
   return is_in_view_impl(node);
 }
diff --git a/panda/src/pgraph/qpcullTraverser.cxx b/panda/src/pgraph/qpcullTraverser.cxx
index bd63ca3631..41baaeb033 100644
--- a/panda/src/pgraph/qpcullTraverser.cxx
+++ b/panda/src/pgraph/qpcullTraverser.cxx
@@ -149,6 +149,11 @@ r_traverse(PandaNode *node, const CullTraverserData &data) {
   // optimization, we should tag nodes with these properties as
   // being "fancy", and skip this processing for non-fancy nodes.
 
+  if (node->get_transform()->is_invalid()) {
+    // If the transform is invalid, forget it.
+    return;
+  }
+
   if (next_data.is_in_view(node)) {
     next_data.apply_transform_and_state(node);
 
@@ -265,6 +270,11 @@ r_get_decals(PandaNode *node, const CullTraverserData &data,
              CullableObject *decals) {
   CullTraverserData next_data(data);
 
+  if (node->get_transform()->is_invalid()) {
+    // If the transform is invalid, forget it.
+    return decals;
+  }
+
   if (next_data.is_in_view(node)) {
     next_data.apply_transform_and_state(node);
 
diff --git a/panda/src/pgraph/transformState.cxx b/panda/src/pgraph/transformState.cxx
index cbdb6151a1..b72cad2cff 100644
--- a/panda/src/pgraph/transformState.cxx
+++ b/panda/src/pgraph/transformState.cxx
@@ -40,6 +40,7 @@ TransformState() {
   _saved_entry = _states.end();
   _self_compose = (TransformState *)NULL;
   _flags = F_is_identity | F_singular_known;
+  _inv_mat = (LMatrix4f *)NULL;
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -70,6 +71,11 @@ operator = (const TransformState &) {
 ////////////////////////////////////////////////////////////////////
 TransformState::
 ~TransformState() {
+  // Free the inverse matrix computation, if it has been stored.
+  if (_inv_mat != (LMatrix4f *)NULL) {
+    delete _inv_mat;
+  }
+
   // Remove the deleted TransformState object from the global pool.
   if (_saved_entry != _states.end()) {
     _states.erase(_saved_entry);
@@ -584,19 +590,22 @@ do_invert_compose(const TransformState *other) const {
   nassertr((_flags & F_is_invalid) == 0, this);
   nassertr((other->_flags & F_is_invalid) == 0, other);
 
+  if (is_singular()) {
+    return make_invalid();
+  }
+
   // We should do this operation componentwise if both transforms were
   // given componentwise.
 
-  // Perhaps we should cache the result of the inverse matrix
-  // operation separately, as a further optimization.
+  // Now that is_singular() has returned true, we can assume that
+  // _inv_mat has been allocated and filled in.
+  nassertr(_inv_mat != (LMatrix4f *)NULL, make_invalid());
 
-  LMatrix4f new_mat;
-  bool invertible = new_mat.invert_from(get_mat());
-  if (!invertible) {
-    return make_invalid();
+  if (other->is_identity()) {
+    return make_mat(*_inv_mat);
+  } else {
+    return make_mat(other->get_mat() * (*_inv_mat));
   }
-  new_mat = other->get_mat() * new_mat;
-  return make_mat(new_mat);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -608,19 +617,21 @@ do_invert_compose(const TransformState *other) const {
 void TransformState::
 calc_singular() {
   nassertv((_flags & F_is_invalid) == 0);
-  bool singular = false;
 
-  if (has_components()) {
-    // The matrix is singular if any component of its scale is 0.
-    singular = (_scale[0] == 0.0f || _scale[1] == 0.0f || _scale[2] == 0.0f);
-  } else {
-    // The matrix is singular if its determinant is zero.
-    const LMatrix4f &mat = get_mat();
-    singular = (mat.get_upper_3().determinant() == 0.0f);
-  }
+  // We determine if a matrix is singular by attempting to invert it
+  // (and we save the result of this invert operation for a subsequent
+  // do_invert_compose() call, which is almost certain to be made if
+  // someone is asking whether we're singular).
 
-  if (singular) {
+  // This should be NULL if no one has called calc_singular() yet.
+  nassertv(_inv_mat == (LMatrix4f *)NULL);
+  _inv_mat = new LMatrix4f;
+  bool inverted = _inv_mat->invert_from(get_mat());
+
+  if (!inverted) {
     _flags |= F_is_singular;
+    delete _inv_mat;
+    _inv_mat = (LMatrix4f *)NULL;
   }
   _flags |= F_singular_known;
 }
diff --git a/panda/src/pgraph/transformState.h b/panda/src/pgraph/transformState.h
index 3fd115198b..c5dfdaa8bf 100644
--- a/panda/src/pgraph/transformState.h
+++ b/panda/src/pgraph/transformState.h
@@ -157,6 +157,7 @@ private:
   };
   LVecBase3f _pos, _hpr, _scale;
   LMatrix4f _mat;
+  LMatrix4f *_inv_mat;
   
   short _flags;