diff --git a/panda/src/pgraph/pandaNode.I b/panda/src/pgraph/pandaNode.I index bf961413c5..a79fb75a5c 100644 --- a/panda/src/pgraph/pandaNode.I +++ b/panda/src/pgraph/pandaNode.I @@ -326,7 +326,13 @@ clear_transform(Thread *current_thread) { INLINE CPT(TransformState) PandaNode:: get_prev_transform(Thread *current_thread) const { CDReader cdata(_cycler, current_thread); - return cdata->_prev_transform.p(); + if (_prev_transform_valid == _reset_prev_transform_seq) { + return cdata->_prev_transform.p(); + } else { + // If these values are different, someone called reset_prev_transform(), + // and we haven't changed our transform since then. + return cdata->_transform.p(); + } } /** @@ -334,10 +340,19 @@ get_prev_transform(Thread *current_thread) const { * indicates its _prev_transform is different from its _transform value (in * pipeline stage 0). In this case, the node will be visited by * reset_prev_transform(). + * + * @deprecated Simply check prev_transform != transform instead. */ INLINE bool PandaNode:: has_dirty_prev_transform() const { - return _dirty_prev_transform; + CDStageReader cdata(_cycler, 0); + if (_prev_transform_valid == _reset_prev_transform_seq) { + return cdata->_prev_transform != cdata->_transform; + } else { + // If these values are different, someone called reset_prev_transform(), + // and we haven't changed our transform since then. + return false; + } } /** @@ -701,34 +716,6 @@ verify_child_no_cycles(PandaNode *child_node) { return true; } -/** - * Sets the dirty_prev_transform flag, and adds the node to the - * _dirty_prev_transforms chain. Assumes _dirty_prev_transforms._lock is - * already held. - */ -INLINE void PandaNode:: -do_set_dirty_prev_transform() { - nassertv(_dirty_prev_transforms._lock.debug_is_locked()); - if (!_dirty_prev_transform) { - LinkedListNode::insert_before(&_dirty_prev_transforms); - _dirty_prev_transform = true; - } -} - -/** - * Clears the dirty_prev_transform flag, and removes the node from the - * _dirty_prev_transforms chain. Assumes _dirty_prev_transforms._lock is - * already held. - */ -INLINE void PandaNode:: -do_clear_dirty_prev_transform() { - nassertv(_dirty_prev_transforms._lock.debug_is_locked()); - if (_dirty_prev_transform) { - LinkedListNode::remove_from_list(); - _dirty_prev_transform = false; - } -} - /** * */ @@ -1513,7 +1500,11 @@ get_transform() const { */ const TransformState *PandaNodePipelineReader:: get_prev_transform() const { - return _cdata->_prev_transform; + if (_node->_prev_transform_valid == PandaNode::_reset_prev_transform_seq) { + return _cdata->_prev_transform; + } else { + return _cdata->_transform; + } } /** diff --git a/panda/src/pgraph/pandaNode.cxx b/panda/src/pgraph/pandaNode.cxx index 31dee668bd..00f0d76143 100644 --- a/panda/src/pgraph/pandaNode.cxx +++ b/panda/src/pgraph/pandaNode.cxx @@ -42,10 +42,9 @@ TypeHandle PandaNode::BamReaderAuxDataDown::_type_handle; PandaNode::SceneRootFunc *PandaNode::_scene_root_func; -PandaNodeChain PandaNode::_dirty_prev_transforms("_dirty_prev_transforms"); +UpdateSeq PandaNode::_reset_prev_transform_seq; DrawMask PandaNode::_overall_bit = DrawMask::bit(31); -PStatCollector PandaNode::_reset_prev_pcollector("App:Collisions:Reset"); PStatCollector PandaNode::_update_bounds_pcollector("*:Bounds"); TypeHandle PandaNode::_type_handle; @@ -78,7 +77,7 @@ PandaNode:: PandaNode(const string &name) : Namable(name), _paths_lock("PandaNode::_paths_lock"), - _dirty_prev_transform(false) + _prev_transform_valid(_reset_prev_transform_seq) { if (pgraph_cat.is_debug()) { pgraph_cat.debug() @@ -100,12 +99,6 @@ PandaNode:: << "Destructing " << (void *)this << ", " << get_name() << "\n"; } - if (_dirty_prev_transform) { - // Need to have this held before we grab any other locks. - LightMutexHolder holder(_dirty_prev_transforms._lock); - do_clear_dirty_prev_transform(); - } - // We shouldn't have any parents left by the time we destruct, or there's a // refcount fault somewhere. @@ -133,7 +126,7 @@ PandaNode(const PandaNode ©) : TypedWritableReferenceCount(copy), Namable(copy), _paths_lock("PandaNode::_paths_lock"), - _dirty_prev_transform(false), + _prev_transform_valid(_reset_prev_transform_seq), _python_tag_data(copy._python_tag_data), _unexpected_change_flags(0) { @@ -145,18 +138,16 @@ PandaNode(const PandaNode ©) : MemoryUsage::update_type(this, this); #endif - // Need to have this held before we grab any other locks. - LightMutexHolder holder(_dirty_prev_transforms._lock); - // Copy the other node's state. { CDReader copy_cdata(copy._cycler); CDWriter cdata(_cycler, true); cdata->_state = copy_cdata->_state; cdata->_transform = copy_cdata->_transform; - cdata->_prev_transform = copy_cdata->_prev_transform; - if (cdata->_transform != cdata->_prev_transform) { - do_set_dirty_prev_transform(); + if (copy._prev_transform_valid == _reset_prev_transform_seq) { + cdata->_prev_transform = copy_cdata->_prev_transform; + } else { + cdata->_prev_transform = copy_cdata->_transform; } cdata->_effects = copy_cdata->_effects; @@ -1061,24 +1052,23 @@ void PandaNode:: set_transform(const TransformState *transform, Thread *current_thread) { nassertv(!transform->is_invalid()); - // Need to have this held before we grab any other locks. - LightMutexHolder holder(_dirty_prev_transforms._lock); - // Apply this operation to the current stage as well as to all upstream // stages. bool any_changed = false; OPEN_ITERATE_CURRENT_AND_UPSTREAM(_cycler, current_thread) { CDStageWriter cdata(_cycler, pipeline_stage, current_thread); if (cdata->_transform != transform) { + if (pipeline_stage == 0) { + // Back up the previous transform. + if (_prev_transform_valid != _reset_prev_transform_seq) { + cdata->_prev_transform = std::move(cdata->_transform); + _prev_transform_valid = _reset_prev_transform_seq; + } + } + cdata->_transform = transform; cdata->set_fancy_bit(FB_transform, !transform->is_identity()); any_changed = true; - - if (pipeline_stage == 0) { - if (cdata->_transform != cdata->_prev_transform) { - do_set_dirty_prev_transform(); - } - } } } CLOSE_ITERATE_CURRENT_AND_UPSTREAM(_cycler); @@ -1099,20 +1089,13 @@ void PandaNode:: set_prev_transform(const TransformState *transform, Thread *current_thread) { nassertv(!transform->is_invalid()); - // Need to have this held before we grab any other locks. - LightMutexHolder holder(_dirty_prev_transforms._lock); - // Apply this operation to the current stage as well as to all upstream // stages. OPEN_ITERATE_CURRENT_AND_UPSTREAM(_cycler, current_thread) { CDStageWriter cdata(_cycler, pipeline_stage, current_thread); cdata->_prev_transform = transform; if (pipeline_stage == 0) { - if (cdata->_transform != cdata->_prev_transform) { - do_set_dirty_prev_transform(); - } else { - do_clear_dirty_prev_transform(); - } + _prev_transform_valid = _reset_prev_transform_seq; } } CLOSE_ITERATE_CURRENT_AND_UPSTREAM(_cycler); @@ -1126,10 +1109,6 @@ set_prev_transform(const TransformState *transform, Thread *current_thread) { */ void PandaNode:: reset_prev_transform(Thread *current_thread) { - // Need to have this held before we grab any other locks. - LightMutexHolder holder(_dirty_prev_transforms._lock); - do_clear_dirty_prev_transform(); - // Apply this operation to the current stage as well as to all upstream // stages. @@ -1138,41 +1117,22 @@ reset_prev_transform(Thread *current_thread) { cdata->_prev_transform = cdata->_transform; } CLOSE_ITERATE_CURRENT_AND_UPSTREAM(_cycler); - mark_bam_modified(); } /** - * Visits all nodes in the world with the _dirty_prev_transform flag--that is, - * all nodes whose _prev_transform is different from the _transform in - * pipeline stage 0--and resets the _prev_transform to be the same as - * _transform. + * Makes sure that all nodes reset their prev_transform value to be the same as + * their transform value. This should be called at the start of each frame. */ void PandaNode:: reset_all_prev_transform(Thread *current_thread) { nassertv(current_thread->get_pipeline_stage() == 0); - PStatTimer timer(_reset_prev_pcollector, current_thread); - LightMutexHolder holder(_dirty_prev_transforms._lock); - - LinkedListNode *list_node = _dirty_prev_transforms._next; - while (list_node != &_dirty_prev_transforms) { - PandaNode *panda_node = (PandaNode *)list_node; - nassertv(panda_node->_dirty_prev_transform); - panda_node->_dirty_prev_transform = false; - - CDStageWriter cdata(panda_node->_cycler, 0, current_thread); - cdata->_prev_transform = cdata->_transform; - - list_node = panda_node->_next; -#ifndef NDEBUG - panda_node->_prev = nullptr; - panda_node->_next = nullptr; -#endif // NDEBUG - panda_node->mark_bam_modified(); - } - - _dirty_prev_transforms._prev = &_dirty_prev_transforms; - _dirty_prev_transforms._next = &_dirty_prev_transforms; + // Rather than keeping a linked list of all nodes that have changed their + // transform, we simply increment this counter. All the nodes compare this + // value to their own _prev_transform_valid value, and if it's different, + // they should disregard their _prev_transform field and assume it's the same + // as their _transform. + ++_reset_prev_transform_seq; } /** @@ -1345,9 +1305,6 @@ copy_all_properties(PandaNode *other) { return; } - // Need to have this held before we grab any other locks. - LightMutexHolder holder(_dirty_prev_transforms._lock); - bool any_transform_changed = false; bool any_state_changed = false; bool any_draw_mask_changed = false; @@ -1368,7 +1325,11 @@ copy_all_properties(PandaNode *other) { } cdataw->_transform = cdatar->_transform; - cdataw->_prev_transform = cdatar->_prev_transform; + if (other->_prev_transform_valid == _reset_prev_transform_seq) { + cdataw->_prev_transform = cdatar->_prev_transform; + } else { + cdataw->_prev_transform = cdatar->_transform; + } cdataw->_state = cdatar->_state; cdataw->_effects = cdatar->_effects; cdataw->_draw_control_mask = cdatar->_draw_control_mask; @@ -1390,9 +1351,7 @@ copy_all_properties(PandaNode *other) { (cdatar->_fancy_bits & change_bits); if (pipeline_stage == 0) { - if (cdataw->_transform != cdataw->_prev_transform) { - do_set_dirty_prev_transform(); - } + _prev_transform_valid = _reset_prev_transform_seq; } } CLOSE_ITERATE_CURRENT_AND_UPSTREAM(_cycler); diff --git a/panda/src/pgraph/pandaNode.h b/panda/src/pgraph/pandaNode.h index d078839b7e..76449859ae 100644 --- a/panda/src/pgraph/pandaNode.h +++ b/panda/src/pgraph/pandaNode.h @@ -63,7 +63,7 @@ class GraphicsStateGuardianBase; * properties. */ class EXPCL_PANDA_PGRAPH PandaNode : public TypedWritableReferenceCount, - public Namable, public LinkedListNode { + public Namable { PUBLISHED: explicit PandaNode(const std::string &name); virtual ~PandaNode(); @@ -449,9 +449,6 @@ private: void fix_path_lengths(int pipeline_stage, Thread *current_thread); void r_list_descendants(std::ostream &out, int indent_level) const; - INLINE void do_set_dirty_prev_transform(); - INLINE void do_clear_dirty_prev_transform(); - public: // This must be declared public so that VC6 will allow the nested CData // class to access it. @@ -540,8 +537,10 @@ private: Paths _paths; LightReMutex _paths_lock; - bool _dirty_prev_transform; - static PandaNodeChain _dirty_prev_transforms; + // This is not part of CData because we only care about modifications to the + // transform in the App stage. + UpdateSeq _prev_transform_valid; + static UpdateSeq _reset_prev_transform_seq; // This is used to maintain a table of keyed data on each node, for the // user's purposes. @@ -713,7 +712,6 @@ private: static DrawMask _overall_bit; - static PStatCollector _reset_prev_pcollector; static PStatCollector _update_bounds_pcollector; PUBLISHED: