From 61097dac7291cc37b8ab1240aebd2f4fd6f8d903 Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Tue, 3 Dec 2013 03:15:25 +0100 Subject: [PATCH] Auto clean up motion cache. Conflicts: indra/llcharacter/llkeyframemotion.cpp indra/llcharacter/llkeyframemotion.h indra/newview/llfloaterbvhpreview.cpp --- indra/llcharacter/llcharacter.cpp | 1 - indra/llcharacter/llheadrotmotion.cpp | 3 + indra/llcharacter/lljoint.h | 2 +- indra/llcharacter/llkeyframemotion.cpp | 168 +++++++++++++++++------ indra/llcharacter/llkeyframemotion.h | 132 ++++++++++++++++-- indra/llcharacter/llmotioncontroller.cpp | 110 +++++++++------ indra/llcharacter/llmotioncontroller.h | 4 +- indra/newview/llfloaterbvhpreview.cpp | 4 + indra/newview/llvoavatar.cpp | 2 +- 9 files changed, 331 insertions(+), 95 deletions(-) diff --git a/indra/llcharacter/llcharacter.cpp b/indra/llcharacter/llcharacter.cpp index 888c20cd4..25b89d2f8 100644 --- a/indra/llcharacter/llcharacter.cpp +++ b/indra/llcharacter/llcharacter.cpp @@ -136,7 +136,6 @@ LLMotion* LLCharacter::findMotion( const LLUUID &id ) //----------------------------------------------------------------------------- // createMotion() -// NOTE: Always assign the result to a LLPointer! //----------------------------------------------------------------------------- LLMotion* LLCharacter::createMotion( const LLUUID &id ) { diff --git a/indra/llcharacter/llheadrotmotion.cpp b/indra/llcharacter/llheadrotmotion.cpp index 0ee378f3b..14017da10 100644 --- a/indra/llcharacter/llheadrotmotion.cpp +++ b/indra/llcharacter/llheadrotmotion.cpp @@ -104,7 +104,10 @@ LLHeadRotMotion::~LLHeadRotMotion() LLMotion::LLMotionInitStatus LLHeadRotMotion::onInitialize(LLCharacter *character) { if (!character) + { + llwarns << "character is NULL." << llendl; return STATUS_FAILURE; + } mCharacter = character; mPelvisJoint = character->getJoint("mPelvis"); diff --git a/indra/llcharacter/lljoint.h b/indra/llcharacter/lljoint.h index fe14c1e44..cb55e6ca7 100644 --- a/indra/llcharacter/lljoint.h +++ b/indra/llcharacter/lljoint.h @@ -41,7 +41,7 @@ #include "lldarray.h" const S32 LL_CHARACTER_MAX_JOINTS_PER_MESH = 15; -const U32 LL_CHARACTER_MAX_JOINTS = 32; // must be divisible by 4! +const U32 LL_CHARACTER_MAX_JOINTS = 32; // must be divisible by 16! const U32 LL_HAND_JOINT_NUM = 31; const U32 LL_FACE_JOINT_NUM = 30; const S32 LL_CHARACTER_MAX_PRIORITY = 7; diff --git a/indra/llcharacter/llkeyframemotion.cpp b/indra/llcharacter/llkeyframemotion.cpp index 98906d3d4..653d3988c 100644 --- a/indra/llcharacter/llkeyframemotion.cpp +++ b/indra/llcharacter/llkeyframemotion.cpp @@ -84,38 +84,55 @@ LLKeyframeMotion::JointMotionList::~JointMotionList() for_each(mJointMotionArray.begin(), mJointMotionArray.end(), DeletePointer()); } -U32 LLKeyframeMotion::JointMotionList::dumpDiagInfo() +//Singu: add parameter 'silent'. +U32 LLKeyframeMotion::JointMotionList::dumpDiagInfo(bool silent) const { S32 total_size = sizeof(JointMotionList); for (U32 i = 0; i < getNumJointMotions(); i++) { - LLKeyframeMotion::JointMotion* joint_motion_p = mJointMotionArray[i]; + LLKeyframeMotion::JointMotion const* joint_motion_p = mJointMotionArray[i]; - llinfos << "\tJoint " << joint_motion_p->mJointName << llendl; + if (!silent) + { + llinfos << "\tJoint " << joint_motion_p->mJointName << llendl; + } if (joint_motion_p->mUsage & LLJointState::SCALE) { - llinfos << "\t" << joint_motion_p->mScaleCurve.mNumKeys << " scale keys at " - << joint_motion_p->mScaleCurve.mNumKeys * sizeof(ScaleKey) << " bytes" << llendl; - + if (!silent) + { + llinfos << "\t" << joint_motion_p->mScaleCurve.mNumKeys << " scale keys at " + << joint_motion_p->mScaleCurve.mNumKeys * sizeof(ScaleKey) << " bytes" << llendl; + } total_size += joint_motion_p->mScaleCurve.mNumKeys * sizeof(ScaleKey); } if (joint_motion_p->mUsage & LLJointState::ROT) { - llinfos << "\t" << joint_motion_p->mRotationCurve.mNumKeys << " rotation keys at " - << joint_motion_p->mRotationCurve.mNumKeys * sizeof(RotationKey) << " bytes" << llendl; - + if (!silent) + { + llinfos << "\t" << joint_motion_p->mRotationCurve.mNumKeys << " rotation keys at " + << joint_motion_p->mRotationCurve.mNumKeys * sizeof(RotationKey) << " bytes" << llendl; + } total_size += joint_motion_p->mRotationCurve.mNumKeys * sizeof(RotationKey); } if (joint_motion_p->mUsage & LLJointState::POS) { - llinfos << "\t" << joint_motion_p->mPositionCurve.mNumKeys << " position keys at " - << joint_motion_p->mPositionCurve.mNumKeys * sizeof(PositionKey) << " bytes" << llendl; - + if (!silent) + { + llinfos << "\t" << joint_motion_p->mPositionCurve.mNumKeys << " position keys at " + << joint_motion_p->mPositionCurve.mNumKeys * sizeof(PositionKey) << " bytes" << llendl; + } total_size += joint_motion_p->mPositionCurve.mNumKeys * sizeof(PositionKey); } } - llinfos << "Size: " << total_size << " bytes" << llendl; + //Singu: Also add memory used by the constraints. + S32 constraints_size = mConstraints.size() * sizeof(constraint_list_t::value_type); + total_size += constraints_size; + if (!silent) + { + llinfos << "\t" << mConstraints.size() << " constraints at " << constraints_size << " bytes" << llendl; + llinfos << "Size: " << total_size << " bytes" << llendl; + } return total_size; } @@ -429,7 +446,6 @@ void LLKeyframeMotion::JointMotion::update(LLJointState* joint_state, F32 time, //----------------------------------------------------------------------------- LLKeyframeMotion::LLKeyframeMotion(const LLUUID &id) : LLMotion(id), - mJointMotionList(NULL), mPelvisp(NULL), mLastSkeletonSerialNum(0), mLastUpdateTime(0.f), @@ -517,6 +533,7 @@ LLMotion::LLMotionInitStatus LLKeyframeMotion::onInitialize(LLCharacter *charact case ASSET_FETCHED: return STATUS_HOLD; case ASSET_FETCH_FAILED: + llwarns << "Trying to initialize a motion that failed to be fetched." << llendl; return STATUS_FAILURE; case ASSET_LOADED: return STATUS_SUCCESS; @@ -526,7 +543,7 @@ LLMotion::LLMotionInitStatus LLKeyframeMotion::onInitialize(LLCharacter *charact break; } - LLKeyframeMotion::JointMotionList* joint_motion_list = LLKeyframeDataCache::getKeyframeData(getID()); + LLKeyframeMotion::JointMotionListPtr joint_motion_list = LLKeyframeDataCache::getKeyframeData(getID()); if(joint_motion_list) { @@ -1227,13 +1244,42 @@ void LLKeyframeMotion::applyConstraint(JointConstraint* constraint, F32 time, U8 } } +// Helper class. +template +struct AIAutoDestruct +{ + T* mPtr; + AIAutoDestruct() : mPtr(NULL) { } + ~AIAutoDestruct() { delete mPtr; } + void add(T* ptr) { mPtr = ptr; } +}; + //----------------------------------------------------------------------------- // deserialize() //----------------------------------------------------------------------------- BOOL LLKeyframeMotion::deserialize(LLDataPacker& dp) { BOOL old_version = FALSE; - mJointMotionList = new LLKeyframeMotion::JointMotionList; + + // + + // First add a new LLKeyframeMotion::JointMotionList to the cache, then assign a pointer + // to that to mJointMotionList. In LLs code the cache is never deleted again. Now it is + // is deleted when the last mJointMotionList pointer is destructed. + // + // It is possible that we get here for an already added animation, because animations can + // be requested multiple times (we get here from LLKeyframeMotion::onLoadComplete) when + // the animation was still downloading from a previous request for another LLMotionController + // object (avatar). In that case we just overwrite the old data while decoding it again. + mJointMotionList = LLKeyframeDataCache::getKeyframeData(getID()); + bool singu_new_joint_motion_list = !mJointMotionList; + if (singu_new_joint_motion_list) + { + // Create a new JointMotionList. + mJointMotionList = LLKeyframeDataCache::createKeyframeData(getID()); + } + + // //------------------------------------------------------------------------- // get base priority @@ -1396,8 +1442,10 @@ BOOL LLKeyframeMotion::deserialize(LLDataPacker& dp) return FALSE; } - mJointMotionList->mJointMotionArray.clear(); - mJointMotionList->mJointMotionArray.reserve(num_motions); + if (singu_new_joint_motion_list) + { + mJointMotionList->mJointMotionArray.reserve(num_motions); + } mJointStates.clear(); mJointStates.reserve(num_motions); @@ -1407,8 +1455,19 @@ BOOL LLKeyframeMotion::deserialize(LLDataPacker& dp) for(U32 i=0; i watcher; + JointMotion* joint_motion = new JointMotion; - mJointMotionList->mJointMotionArray.push_back(joint_motion); + if (singu_new_joint_motion_list) + { + // Pass ownership to mJointMotionList. + mJointMotionList->mJointMotionArray.push_back(joint_motion); + } + else + { + // Just delete this at the end. + watcher.add(joint_motion); + } std::string joint_name; if (!dp.unpackString(joint_name, "joint_name")) @@ -1836,7 +1895,15 @@ BOOL LLKeyframeMotion::deserialize(LLDataPacker& dp) return FALSE; } - mJointMotionList->mConstraints.push_front(constraintp); + AIAutoDestruct watcher; + if (singu_new_joint_motion_list) + { + mJointMotionList->mConstraints.push_front(constraintp); + } + else + { + watcher.add(constraintp); + } constraintp->mJointStateIndices = new S32[constraintp->mChainLength + 1]; // note: mChainLength is size-limited - comes from a byte @@ -1876,15 +1943,12 @@ BOOL LLKeyframeMotion::deserialize(LLDataPacker& dp) if (constraintp->mJointStateIndices[i] < 0 ) { llwarns << "No joint index for constraint " << i << llendl; - delete constraintp; return FALSE; } } } } - // *FIX: support cleanup of old keyframe data - LLKeyframeDataCache::addKeyframeData(getID(), mJointMotionList); mAssetStatus = ASSET_LOADED; setupPose(); @@ -2226,16 +2290,24 @@ void LLKeyframeMotion::onLoadComplete(LLVFS *vfs, //-------------------------------------------------------------------- // LLKeyframeDataCache::dumpDiagInfo() //-------------------------------------------------------------------- -void LLKeyframeDataCache::dumpDiagInfo() +// +// quiet = 0 : print everything in detail. +// 1 : print the UUIDs of all animations in the cache and the total memory usage. +// 2 : only print the total memory usage. +// +void LLKeyframeDataCache::dumpDiagInfo(int quiet) { // keep track of totals U32 total_size = 0; char buf[1024]; /* Flawfinder: ignore */ - llinfos << "-----------------------------------------------------" << llendl; - llinfos << " Global Motion Table (DEBUG only)" << llendl; - llinfos << "-----------------------------------------------------" << llendl; + if (quiet < 2) + { + llinfos << "-----------------------------------------------------" << llendl; + llinfos << " Global Motion Table" << llendl; + llinfos << "-----------------------------------------------------" << llendl; + } // print each loaded mesh, and it's memory usage for (keyframe_data_map_t::iterator map_it = sKeyframeDataMap.begin(); @@ -2243,30 +2315,46 @@ void LLKeyframeDataCache::dumpDiagInfo() { U32 joint_motion_kb; - LLKeyframeMotion::JointMotionList *motion_list_p = map_it->second; + LLKeyframeMotion::JointMotionList const* motion_list_p = map_it->get(); - llinfos << "Motion: " << map_it->first << llendl; + if (quiet < 2) + { + llinfos << "Motion: " << map_it->key() << llendl; + } - joint_motion_kb = motion_list_p->dumpDiagInfo(); - - total_size += joint_motion_kb; + if (motion_list_p) + { + joint_motion_kb = motion_list_p->dumpDiagInfo(quiet); + total_size += joint_motion_kb; + } } - llinfos << "-----------------------------------------------------" << llendl; + if (quiet < 2) + { + llinfos << "-----------------------------------------------------" << llendl; + } llinfos << "Motions\tTotal Size" << llendl; snprintf(buf, sizeof(buf), "%d\t\t%d bytes", (S32)sKeyframeDataMap.size(), total_size ); /* Flawfinder: ignore */ llinfos << buf << llendl; - llinfos << "-----------------------------------------------------" << llendl; + if (quiet < 2) + { + llinfos << "-----------------------------------------------------" << llendl; + } } //-------------------------------------------------------------------- -// LLKeyframeDataCache::addKeyframeData() +// LLKeyframeDataCache::createKeyframeData() //-------------------------------------------------------------------- -void LLKeyframeDataCache::addKeyframeData(const LLUUID& id, LLKeyframeMotion::JointMotionList* joint_motion_listp) +// This function replaces LLKeyframeDataCache::addKeyframeData and was rewritten to fix a memory leak (aka, the usage of AICachedPointer). +LLKeyframeMotion::JointMotionListPtr LLKeyframeDataCache::createKeyframeData(LLUUID const& id) { - sKeyframeDataMap[id] = joint_motion_listp; + std::pair result = + sKeyframeDataMap.insert(AICachedPointer(id, new LLKeyframeMotion::JointMotionList, &sKeyframeDataMap)); + llassert(result.second); // id may not already exist in the cache. + return &*result.first; // Construct and return a JointMotionListPt from a pointer to the actually inserted AICachedPointer. } +// //-------------------------------------------------------------------- // LLKeyframeDataCache::removeKeyframeData() @@ -2276,7 +2364,6 @@ void LLKeyframeDataCache::removeKeyframeData(const LLUUID& id) keyframe_data_map_t::iterator found_data = sKeyframeDataMap.find(id); if (found_data != sKeyframeDataMap.end()) { - delete found_data->second; sKeyframeDataMap.erase(found_data); } } @@ -2284,14 +2371,14 @@ void LLKeyframeDataCache::removeKeyframeData(const LLUUID& id) //-------------------------------------------------------------------- // LLKeyframeDataCache::getKeyframeData() //-------------------------------------------------------------------- -LLKeyframeMotion::JointMotionList* LLKeyframeDataCache::getKeyframeData(const LLUUID& id) +LLKeyframeMotion::JointMotionListPtr LLKeyframeDataCache::getKeyframeData(const LLUUID& id) { keyframe_data_map_t::iterator found_data = sKeyframeDataMap.find(id); if (found_data == sKeyframeDataMap.end()) { return NULL; } - return found_data->second; + return &*found_data; // Construct and return a JointMotionListPt from a pointer to the found AICachedPointer. } //-------------------------------------------------------------------- @@ -2307,7 +2394,6 @@ LLKeyframeDataCache::~LLKeyframeDataCache() //----------------------------------------------------------------------------- void LLKeyframeDataCache::clear() { - for_each(sKeyframeDataMap.begin(), sKeyframeDataMap.end(), DeletePairedPointer()); sKeyframeDataMap.clear(); } diff --git a/indra/llcharacter/llkeyframemotion.h b/indra/llcharacter/llkeyframemotion.h index 50d9d0504..55be6df09 100644 --- a/indra/llcharacter/llkeyframemotion.h +++ b/indra/llcharacter/llkeyframemotion.h @@ -5,6 +5,7 @@ * $LicenseInfo:firstyear=2001&license=viewergpl$ * * Copyright (c) 2001-2009, Linden Research, Inc. + * AICachedPointer and AICachedPointPtr copyright (c) 2013, Aleric Inglewood. * * Second Life Viewer Source Code * The source code in this file ("Source Code") is provided by Linden Lab @@ -48,6 +49,7 @@ #include "v3dmath.h" #include "v3math.h" #include "llbvhconsts.h" +#include class LLKeyframeDataCache; class LLVFS; @@ -59,6 +61,112 @@ class LLDataPacker; const S32 KEYFRAME_MOTION_VERSION = 1; const S32 KEYFRAME_MOTION_SUBVERSION = 0; +//----------------------------------------------------------------------------- +// + +template +class AICachedPointer; + +template +void intrusive_ptr_add_ref(AICachedPointer const* p); + +template +void intrusive_ptr_release(AICachedPointer const* p); + +template +class AICachedPointer +{ + public: + typedef std::set > container_type; + + private: + KEY mKey; // The unique key. + LLPointer mData; // The actual data pointer. + container_type* mCache; // Pointer to the underlaying cache. + mutable int mRefCount; // Number of AICachedPointerPtr's pointing to this object. + + public: + // Construct a NULL pointer. This is needed when adding a new entry to a std::set, it is always first default constructed. + AICachedPointer(void) : mCache(NULL) { } + + // Copy constructor. This is needed to replace the std::set inserted instance with its actual value. + AICachedPointer(AICachedPointer const& cptr) : mKey(cptr.mKey), mData(cptr.mData), mCache(cptr.mCache), mRefCount(0) { } + + // Construct a AICachedPointer that points to 'ptr' with key 'key'. + AICachedPointer(KEY const& key, T* ptr, container_type* cache) : mKey(key), mData(ptr), mCache(cache), mRefCount(-1) { } + + // Construct a temporary NULL pointer that can be used in a search for a key. + AICachedPointer(KEY const& key) : mKey(key), mCache(NULL) { } + + // Accessors for key and data. + KEY const& key(void) const { return mKey; } + T const* get(void) const { return mData.get(); } + T* get(void) { return mData.get(); } + + // Order only by key. + friend bool operator<(AICachedPointer const& cp1, AICachedPointer const& cp2) { return cp1.mKey < cp2.mKey; } + + private: + friend void intrusive_ptr_add_ref<>(AICachedPointer const* p); + friend void intrusive_ptr_release<>(AICachedPointer const* p); + + private: + AICachedPointer& operator=(AICachedPointer const&); +}; + +template +void intrusive_ptr_add_ref(AICachedPointer const* p) +{ + llassert(p->mCache); + if (p->mCache) + { + p->mRefCount++; + } +} + +template +void intrusive_ptr_release(AICachedPointer const* p) +{ + llassert(p->mCache); + if (p->mCache) + { + if (--p->mRefCount == 0) + { + p->mCache->erase(p->mKey); + } + } +} + +template +class AICachedPointerPtr +{ + private: + boost::intrusive_ptr const> mPtr; + static int sCnt; + + public: + AICachedPointerPtr(void) { ++sCnt; } + AICachedPointerPtr(AICachedPointerPtr const& cpp) : mPtr(cpp.mPtr) { ++sCnt; } + AICachedPointerPtr(AICachedPointer const* cp) : mPtr(cp) { ++sCnt; } + ~AICachedPointerPtr() { --sCnt; } + + typedef boost::intrusive_ptr const> const AICachedPointerPtr::* const bool_type; + operator bool_type() const { return mPtr ? &AICachedPointerPtr::mPtr : NULL; } + + T const* operator->() const { return mPtr->get(); } + T* operator->() { return const_cast&>(*mPtr).get(); } + T const& operator*() const { return *mPtr->get(); } + T& operator*() { return *const_cast&>(*mPtr).get(); } + + AICachedPointerPtr& operator=(AICachedPointerPtr const& cpp) { mPtr = cpp.mPtr; return *this; } +}; + +template +int AICachedPointerPtr::sCnt; + +// +//----------------------------------------------------------------------------- + //----------------------------------------------------------------------------- // class LLKeyframeMotion //----------------------------------------------------------------------------- @@ -158,7 +266,7 @@ public: U32 getFileSize(); BOOL serialize(LLDataPacker& dp) const; BOOL deserialize(LLDataPacker& dp); - BOOL isLoaded() { return mJointMotionList != NULL; } + BOOL isLoaded() { return !!mJointMotionList; } // setters for modifying a keyframe animation @@ -416,19 +524,22 @@ public: public: JointMotionList(); ~JointMotionList(); - U32 dumpDiagInfo(); + U32 dumpDiagInfo(bool silent = false) const; JointMotion* getJointMotion(U32 index) const { llassert(index < mJointMotionArray.size()); return mJointMotionArray[index]; } U32 getNumJointMotions() const { return mJointMotionArray.size(); } }; + // Singu: Type of a pointer to the cached pointer (in LLKeyframeDataCache::sKeyframeDataMap) to a JointMotionList object. + typedef AICachedPointerPtr JointMotionListPtr; + protected: static LLVFS* sVFS; //------------------------------------------------------------------------- // Member Data //------------------------------------------------------------------------- - JointMotionList* mJointMotionList; + JointMotionListPtr mJointMotionList; // singu: automatically clean up cache entry when destructed. std::vector > mJointStates; LLJoint* mPelvisp; LLCharacter* mCharacter; @@ -442,21 +553,24 @@ protected: class LLKeyframeDataCache { -public: - // *FIX: implement this as an actual singleton member of LLKeyframeMotion +private: + friend class LLKeyframeMotion; LLKeyframeDataCache(){}; ~LLKeyframeDataCache(); - typedef std::map keyframe_data_map_t; +public: + typedef AICachedPointer::container_type keyframe_data_map_t; // singu: add automatic cache cleanup. static keyframe_data_map_t sKeyframeDataMap; - static void addKeyframeData(const LLUUID& id, LLKeyframeMotion::JointMotionList*); - static LLKeyframeMotion::JointMotionList* getKeyframeData(const LLUUID& id); + // + static LLKeyframeMotion::JointMotionListPtr createKeyframeData(LLUUID const& id); // id may not exist. + static LLKeyframeMotion::JointMotionListPtr getKeyframeData(LLUUID const& id); // id may or may not exists. Returns a NULL pointer when it doesn't exist. + // static void removeKeyframeData(const LLUUID& id); //print out diagnostic info - static void dumpDiagInfo(); + static void dumpDiagInfo(int quiet = 0); // singu: added param 'quiet'. static void clear(); }; diff --git a/indra/llcharacter/llmotioncontroller.cpp b/indra/llcharacter/llmotioncontroller.cpp index 35dcc1057..b5d60205c 100644 --- a/indra/llcharacter/llmotioncontroller.cpp +++ b/indra/llcharacter/llmotioncontroller.cpp @@ -168,7 +168,10 @@ void LLMotionController::deleteAllMotions() mLoadingMotions.clear(); mLoadedMotions.clear(); mActiveMotions.clear(); - + // + for_each(mDeprecatedMotions.begin(), mDeprecatedMotions.end(), DeletePointer()); + mDeprecatedMotions.clear(); + // for_each(mAllMotions.begin(), mAllMotions.end(), DeletePairedPointer()); mAllMotions.clear(); } @@ -178,26 +181,19 @@ void LLMotionController::deleteAllMotions() //----------------------------------------------------------------------------- void LLMotionController::purgeExcessMotions() { - if (mLoadedMotions.size() > MAX_MOTION_INSTANCES) + // + // The old code attempted to remove non-active motions from mDeprecatedMotions, + // but that is nonsense: there are no motions in mDeprecatedMotions that are not active. + if (mLoadedMotions.size() <= MAX_MOTION_INSTANCES) { - // clean up deprecated motions - for (motion_set_t::iterator deprecated_motion_it = mDeprecatedMotions.begin(); - deprecated_motion_it != mDeprecatedMotions.end(); ) - { - motion_set_t::iterator cur_iter = deprecated_motion_it++; - LLMotion* cur_motionp = *cur_iter; - if (!isMotionActive(cur_motionp)) - { - // Motion is deprecated so we know it's not cannonical, - // we can safely remove the instance - removeMotionInstance(cur_motionp); // modifies mDeprecatedMotions - mDeprecatedMotions.erase(cur_iter); - } - } + // Speed up, no need to create motions_to_kill. + return; } + // std::set motions_to_kill; - if (mLoadedMotions.size() > MAX_MOTION_INSTANCES) + + if (1) // Singu: leave indentation alone... { // too many motions active this frame, kill all blenders mPoseBlender.clearBlenders(); @@ -308,24 +304,44 @@ BOOL LLMotionController::registerMotion( const LLUUID& id, LLMotionConstructor c void LLMotionController::removeMotion( const LLUUID& id) { LLMotion* motionp = findMotion(id); - mAllMotions.erase(id); - removeMotionInstance(motionp); + // + // If a motion is erased from mAllMotions, it must be deleted. + if (motionp) + { + mAllMotions.erase(id); + removeMotionInstance(motionp); + delete motionp; + } + // } // removes instance of a motion from all runtime structures, but does // not erase entry by ID, as this could be a duplicate instance -// use removeMotion(id) to remove all references to a given motion by id. +// use removeMotion(id) to remove a reference to a given motion by id +// (that will not remove (active) deprecated motions). void LLMotionController::removeMotionInstance(LLMotion* motionp) { if (motionp) { llassert(findMotion(motionp->getID()) != motionp); - if (motionp->isActive()) - motionp->deactivate(); mLoadingMotions.erase(motionp); mLoadedMotions.erase(motionp); mActiveMotions.remove(motionp); - delete motionp; + // + // Deactivation moved here. Only delete motionp when it is being removed from mDeprecatedMotions. + if (motionp->isActive()) + { + motionp->deactivate(); + // If a motion is deactivated, it must be removed from mDeprecatedMotions if there. + motion_set_t::iterator found_it = mDeprecatedMotions.find(motionp); + if (found_it != mDeprecatedMotions.end()) + { + mDeprecatedMotions.erase(found_it); + // If a motion is erased from mDeprecatedMotions, it must be deleted. + delete motionp; + } + } + // } } @@ -393,6 +409,7 @@ BOOL LLMotionController::startMotion(const LLUUID &id, F32 start_offset) if (motion && !mPaused && motion->canDeprecate() + && motion->isActive() // singu: do not deprecate motions that are not active. && motion->getFadeWeight() > 0.01f // not LOD-ed out && (motion->isBlending() || motion->getStopTime() != 0.f)) { @@ -782,11 +799,10 @@ void LLMotionController::updateLoadingMotions() llinfos << "Motion " << motionp->getID() << " init failed." << llendl; sRegistry.markBad(motionp->getID()); mLoadingMotions.erase(curiter); - motion_set_t::iterator found_it = mDeprecatedMotions.find(motionp); - if (found_it != mDeprecatedMotions.end()) - { - mDeprecatedMotions.erase(found_it); - } + // Singu note: a motion in mLoadingMotions will not be in mActiveMotions + // and therefore not be in mDeprecatedMotions. So, we don't have to + // check for it's existence there. + llassert(mDeprecatedMotions.find(motionp) == mDeprecatedMotions.end()); mAllMotions.erase(motionp->getID()); delete motionp; } @@ -970,18 +986,16 @@ BOOL LLMotionController::activateMotionInstance(LLMotion *motion, F32 time) //----------------------------------------------------------------------------- BOOL LLMotionController::deactivateMotionInstance(LLMotion *motion) { - motion->deactivate(); - motion_set_t::iterator found_it = mDeprecatedMotions.find(motion); if (found_it != mDeprecatedMotions.end()) { // deprecated motions need to be completely excised - removeMotionInstance(motion); - mDeprecatedMotions.erase(found_it); + removeMotionInstance(motion); // singu note: this deactivates motion and removes it from mDeprecatedMotions. } else { // for motions that we are keeping, simply remove from active queue + motion->deactivate(); // singu note: moved here from the top of the function. mActiveMotions.remove(motion); } @@ -1049,11 +1063,26 @@ void LLMotionController::dumpMotions() state_string += std::string("L"); if (std::find(mActiveMotions.begin(), mActiveMotions.end(), motion)!=mActiveMotions.end()) state_string += std::string("A"); - if (mDeprecatedMotions.find(motion) != mDeprecatedMotions.end()) - state_string += std::string("D"); + llassert(mDeprecatedMotions.find(motion) == mDeprecatedMotions.end()); // singu: it's impossible that a motion is in mAllMotions and mDeprecatedMotions at the same time. llinfos << gAnimLibrary.animationName(id) << " " << state_string << llendl; - } + // + // Also dump the deprecated motions. + for (motion_set_t::iterator iter = mDeprecatedMotions.begin(); + iter != mDeprecatedMotions.end(); ++iter) + { + std::string state_string; + LLMotion* motion = *iter; + LLUUID id = motion->getID(); + llassert(mLoadingMotions.find(motion) == mLoadingMotions.end()); + if (mLoadedMotions.find(motion) != mLoadedMotions.end()) + state_string += std::string("L"); + if (std::find(mActiveMotions.begin(), mActiveMotions.end(), motion)!=mActiveMotions.end()) + state_string += std::string("A"); + state_string += "D"; + llinfos << gAnimLibrary.animationName(id) << " " << state_string << llendl; + } + // } //----------------------------------------------------------------------------- @@ -1061,11 +1090,11 @@ void LLMotionController::dumpMotions() //----------------------------------------------------------------------------- void LLMotionController::deactivateAllMotions() { - for (motion_map_t::iterator iter = mAllMotions.begin(); - iter != mAllMotions.end(); iter++) + // Singu note: this must run over mActiveMotions: other motions are not active, + // and running over mAllMotions will miss the ones in mDeprecatedMotions. + for (motion_list_t::iterator iter = mActiveMotions.begin(); iter != mActiveMotions.end(); ++iter) { - LLMotion* motionp = iter->second; - deactivateMotionInstance(motionp); + deactivateMotionInstance(*iter); } } @@ -1086,8 +1115,7 @@ void LLMotionController::flushAllMotions() active_motions.push_back(std::make_pair(motionp->getID(),dtime)); motionp->deactivate(); // don't call deactivateMotionInstance() because we are going to reactivate it } - mActiveMotions.clear(); - + // delete all motion instances deleteAllMotions(); diff --git a/indra/llcharacter/llmotioncontroller.h b/indra/llcharacter/llmotioncontroller.h index 2de13aa36..7e0cc7ad7 100644 --- a/indra/llcharacter/llmotioncontroller.h +++ b/indra/llcharacter/llmotioncontroller.h @@ -115,7 +115,6 @@ public: // unregisters a motion with the controller // (actually just forwards call to motion registry) - // returns true if successfull void removeMotion( const LLUUID& id ); // start motion @@ -205,10 +204,13 @@ protected: // Life cycle of an animation: // // Animations are instantiated and immediately put in the mAllMotions map for their entire lifetime. +// Singu note: that is not true, they are moved to mDeprecatedMotions (often) for the last part of their lifetime. // If the animations depend on any asset data, the appropriate data is fetched from the data server, // and the animation is put on the mLoadingMotions list. // Once an animations is loaded, it will be initialized and put on the mLoadedMotions list. // Any animation that is currently playing also sits in the mActiveMotions list. +// Singu note: animations are only put in mDeprecatedMotions if and while they are playing, +// therefore animations in mDeprecatedMotions will be (must be) active and in mActiveMotions. typedef std::map motion_map_t; motion_map_t mAllMotions; diff --git a/indra/newview/llfloaterbvhpreview.cpp b/indra/newview/llfloaterbvhpreview.cpp index 58e27b914..251397fa6 100644 --- a/indra/newview/llfloaterbvhpreview.cpp +++ b/indra/newview/llfloaterbvhpreview.cpp @@ -375,6 +375,10 @@ BOOL LLFloaterBvhPreview::postBuild() } } + if (motionp && mInWorld) + { + gAgentAvatarp->removeMotion(mMotionID); + } //setEnabled(FALSE); mMotionID.setNull(); mAnimPreview = NULL; diff --git a/indra/newview/llvoavatar.cpp b/indra/newview/llvoavatar.cpp index 3b15311e7..aec11e199 100644 --- a/indra/newview/llvoavatar.cpp +++ b/indra/newview/llvoavatar.cpp @@ -3884,7 +3884,7 @@ BOOL LLVOAvatar::updateCharacter(LLAgent &agent) getOffObject(); // //Singu note: this appears to be a safety catch: - // when getParent() is NULL and we're note playing ANIM_AGENT_SIT_GROUND_CONSTRAINED then we aren't sitting! + // when getParent() is NULL and we're not playing ANIM_AGENT_SIT_GROUND_CONSTRAINED then we aren't sitting! // The previous call existed in an attempt to fix this inconsistent state by standing up from an object. // However, since getParent() is NULL that function would crash! // Since we never got crash reports regarding to this, that apparently never happened, except, I discovered