From 5f14d9c48f250ff982a1369b1dce1bb8150bdefc Mon Sep 17 00:00:00 2001 From: Sam Edwards Date: Thu, 8 Mar 2018 14:27:04 -0700 Subject: [PATCH] char: Move JointVertexTransform::_matrix to CharacterJoint The rationale is that CharacterJoint should contain all of the joint state information, and JointVertexTransform should be pretty much devoid of state so that we don't have to worry about synchronizing it. JointVertexTransform::_matrix was just a cached/precomputed matrix that transforms from original vertex positions to animated vertex positions, so moving it to CharacterJoint doesn't change any engine functionality. This also removes the useless lock on recomputing that matrix. It was useless because it was computing from shared state in CharacterJoint that wasn't properly synchronized, but this would have to be fixed by making CharacterJoint pipeline-cycled - a lock won't do. --- panda/src/char/characterJoint.cxx | 18 ++++++++++----- panda/src/char/characterJoint.h | 6 +++++ panda/src/char/jointVertexTransform.I | 10 --------- panda/src/char/jointVertexTransform.cxx | 30 +++++-------------------- panda/src/char/jointVertexTransform.h | 7 ------ 5 files changed, 23 insertions(+), 48 deletions(-) diff --git a/panda/src/char/characterJoint.cxx b/panda/src/char/characterJoint.cxx index 9efb002349..6ce5b0be88 100644 --- a/panda/src/char/characterJoint.cxx +++ b/panda/src/char/characterJoint.cxx @@ -39,7 +39,8 @@ CharacterJoint(const CharacterJoint ©) : MovingPartMatrix(copy), _character(NULL), _net_transform(copy._net_transform), - _initial_net_transform_inverse(copy._initial_net_transform_inverse) + _initial_net_transform_inverse(copy._initial_net_transform_inverse), + _skinning_matrix(copy._skinning_matrix) { // We don't copy the sets of transform nodes. } @@ -60,9 +61,12 @@ CharacterJoint(Character *character, // update_internals() to get our _net_transform set properly. update_internals(root, parent, true, false, current_thread); - // And then compute its inverse. This is needed for JointVertexTransform, - // during animation. + // And then compute its inverse. This is needed to track changes in + // _net_transform as the joint moves, so we can recompute _skinning_matrix, + // which maps vertices from their initial positions to their animated + // positions. _initial_net_transform_inverse = invert(_net_transform); + _skinning_matrix = LMatrix4::ident_mat(); } /** @@ -141,11 +145,13 @@ update_internals(PartBundle *root, PartGroup *parent, bool self_changed, } } - // Also tell our related JointVertexTransforms that they now need to - // recompute themselves. + // Recompute the transform used by any vertices animated by this joint. + _skinning_matrix = _initial_net_transform_inverse * _net_transform; + + // Also tell our related JointVertexTransforms that we've changed their + // underlying matrix. VertexTransforms::iterator vti; for (vti = _vertex_transforms.begin(); vti != _vertex_transforms.end(); ++vti) { - (*vti)->_matrix_stale = true; (*vti)->mark_modified(current_thread); } } diff --git a/panda/src/char/characterJoint.h b/panda/src/char/characterJoint.h index 759d175c7e..5da83def66 100644 --- a/panda/src/char/characterJoint.h +++ b/panda/src/char/characterJoint.h @@ -108,6 +108,12 @@ public: LMatrix4 _net_transform; LMatrix4 _initial_net_transform_inverse; + // This is the product of the above; the matrix that gets applied to a + // vertex (whose coordinates are in the coordinate space of the character + // in its neutral pose) to transform it from its neutral position to its + // animated position. + LMatrix4 _skinning_matrix; + public: virtual TypeHandle get_type() const { return get_class_type(); diff --git a/panda/src/char/jointVertexTransform.I b/panda/src/char/jointVertexTransform.I index a5a2a2e8cf..4973ae6d60 100644 --- a/panda/src/char/jointVertexTransform.I +++ b/panda/src/char/jointVertexTransform.I @@ -18,13 +18,3 @@ INLINE const CharacterJoint *JointVertexTransform:: get_joint() const { return _joint; } - -/** - * Recomputes _matrix if it needs it. - */ -INLINE void JointVertexTransform:: -check_matrix() const { - if (_matrix_stale) { - ((JointVertexTransform *)this)->compute_matrix(); - } -} diff --git a/panda/src/char/jointVertexTransform.cxx b/panda/src/char/jointVertexTransform.cxx index cf943669a7..5212aa6cd9 100644 --- a/panda/src/char/jointVertexTransform.cxx +++ b/panda/src/char/jointVertexTransform.cxx @@ -24,8 +24,7 @@ TypeHandle JointVertexTransform::_type_handle; * Constructs an invalid object; used only by the bam loader. */ JointVertexTransform:: -JointVertexTransform() : - _matrix_stale(true) +JointVertexTransform() { } @@ -35,8 +34,7 @@ JointVertexTransform() : */ JointVertexTransform:: JointVertexTransform(CharacterJoint *joint) : - _joint(joint), - _matrix_stale(true) + _joint(joint) { // Tell the joint that we need to be informed when it moves. _joint->_vertex_transforms.insert(this); @@ -57,8 +55,7 @@ JointVertexTransform:: */ void JointVertexTransform:: get_matrix(LMatrix4 &matrix) const { - check_matrix(); - matrix = _matrix; + matrix = _joint->_skinning_matrix; } /** @@ -69,8 +66,7 @@ get_matrix(LMatrix4 &matrix) const { */ void JointVertexTransform:: mult_matrix(LMatrix4 &result, const LMatrix4 &previous) const { - check_matrix(); - result.multiply(_matrix, previous); + result.multiply(_joint->_skinning_matrix, previous); } /** @@ -80,9 +76,7 @@ mult_matrix(LMatrix4 &result, const LMatrix4 &previous) const { */ void JointVertexTransform:: accumulate_matrix(LMatrix4 &accum, PN_stdfloat weight) const { - check_matrix(); - - accum.accumulate(_matrix, weight); + accum.accumulate(_joint->_skinning_matrix, weight); } /** @@ -93,19 +87,6 @@ output(ostream &out) const { out << _joint->get_name(); } -/** - * Recomputes _matrix if it needs it. Uses locking. - */ -void JointVertexTransform:: -compute_matrix() { - LightMutexHolder holder(_lock); - if (_matrix_stale) { - _matrix = _joint->_initial_net_transform_inverse * _joint->_net_transform; - _matrix_stale = false; - } -} - - /** * Tells the BamReader how to create objects of type JointVertexTransform. */ @@ -165,6 +146,5 @@ fillin(DatagramIterator &scan, BamReader *manager) { VertexTransform::fillin(scan, manager); manager->read_pointer(scan); - _matrix_stale = true; mark_modified(Thread::get_current_thread()); } diff --git a/panda/src/char/jointVertexTransform.h b/panda/src/char/jointVertexTransform.h index 39b531de8d..1be097eba6 100644 --- a/panda/src/char/jointVertexTransform.h +++ b/panda/src/char/jointVertexTransform.h @@ -47,15 +47,8 @@ PUBLISHED: virtual void output(ostream &out) const; private: - INLINE void check_matrix() const; - void compute_matrix(); - PT(CharacterJoint) _joint; - LMatrix4 _matrix; - bool _matrix_stale; - LightMutex _lock; - public: static void register_with_read_factory(); virtual void write_datagram(BamWriter *manager, Datagram &dg);