From 728d1ec504647f512aeb46d3eb7b15d262d94246 Mon Sep 17 00:00:00 2001 From: Porteries Tristan Date: Mon, 19 Oct 2015 16:03:40 +0200 Subject: BGE: Fix T46381 : last action frame not updated. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It fix T46381. Normally BL_Action::Update (manage action time, end, loop…) should be called the same number of times as BL_Action::UpdateIPO (update action position, scale ect… in the game object). But the bug report shows that UpdateIPO is called one less time than Update. To fix it i revert the commit 362b25b38287cb75e4d22b30bdbc7f47e8eb3fdf and implement a mutex in BL_Action::Update. Example file : {F245823} Reviewers: lordloki, kupoman, campbellbarton, youle, moguri, sybren Reviewed By: youle, moguri, sybren Maniphest Tasks: T39928, T46381 Differential Revision: https://developer.blender.org/D1562 --- source/gameengine/Ketsji/BL_Action.cpp | 27 ++++++++++++++++++++++----- source/gameengine/Ketsji/BL_Action.h | 9 +++++---- source/gameengine/Ketsji/BL_ActionManager.cpp | 10 ---------- source/gameengine/Ketsji/BL_ActionManager.h | 5 ----- source/gameengine/Ketsji/KX_GameObject.cpp | 8 +++----- source/gameengine/Ketsji/KX_GameObject.h | 6 ------ source/gameengine/Ketsji/KX_KetsjiEngine.cpp | 6 ++++++ source/gameengine/Ketsji/KX_Scene.cpp | 4 ---- 8 files changed, 36 insertions(+), 39 deletions(-) (limited to 'source/gameengine/Ketsji') diff --git a/source/gameengine/Ketsji/BL_Action.cpp b/source/gameengine/Ketsji/BL_Action.cpp index 507476dbd0e..4d5b6a0e139 100644 --- a/source/gameengine/Ketsji/BL_Action.cpp +++ b/source/gameengine/Ketsji/BL_Action.cpp @@ -56,6 +56,15 @@ extern "C" { #include "BKE_library.h" #include "BKE_global.h" +#include "BLI_threads.h" // for lock + +/* Lock to solve animation thread issues. + * A spin lock is better than a mutex in case of short wait + * because spin lock stop the thread by a loop contrary to mutex + * which switch all memory, process. + */ +static SpinLock BL_ActionLock; + BL_Action::BL_Action(class KX_GameObject* gameobj) : m_action(NULL), @@ -501,15 +510,23 @@ void BL_Action::Update(float curtime) } } - // This isn't thread-safe, so we move it into it's own function for now - //m_obj->UpdateIPO(m_localtime, m_ipo_flags & ACT_IPOFLAG_CHILD); + BLI_spin_lock(&BL_ActionLock); + /* This function is not thread safe because of recursive scene graph transform + * updates on children. e.g: If an object and one of its children is animated, + * the both can write transform at the same time. A thread lock avoid problems. */ + m_obj->UpdateIPO(m_localtime, m_ipo_flags & ACT_IPOFLAG_CHILD); + BLI_spin_unlock(&BL_ActionLock); if (m_done) ClearControllerList(); } -void BL_Action::UpdateIPOs() +void BL_Action::InitLock() +{ + BLI_spin_init(&BL_ActionLock); +} + +void BL_Action::EndLock() { - if (!m_done) - m_obj->UpdateIPO(m_localtime, m_ipo_flags & ACT_IPOFLAG_CHILD); + BLI_spin_end(&BL_ActionLock); } diff --git a/source/gameengine/Ketsji/BL_Action.h b/source/gameengine/Ketsji/BL_Action.h index 7a404416758..f6124b032aa 100644 --- a/source/gameengine/Ketsji/BL_Action.h +++ b/source/gameengine/Ketsji/BL_Action.h @@ -101,10 +101,6 @@ public: * Update the action's frame, etc. */ void Update(float curtime); - /** - * Update object IPOs (note: not thread-safe!) - */ - void UpdateIPOs(); // Accessors float GetFrame(); @@ -140,6 +136,11 @@ public: ACT_IPOFLAG_CHILD = 8, }; + /// Initialize a lock for animation thread issues. + static void InitLock(); + /// Finalize a lock for animation thread issues. + static void EndLock(); + #ifdef WITH_CXX_GUARDEDALLOC MEM_CXX_CLASS_ALLOC_FUNCS("GE:BL_Action") #endif diff --git a/source/gameengine/Ketsji/BL_ActionManager.cpp b/source/gameengine/Ketsji/BL_ActionManager.cpp index 4249db55b45..491be035d66 100644 --- a/source/gameengine/Ketsji/BL_ActionManager.cpp +++ b/source/gameengine/Ketsji/BL_ActionManager.cpp @@ -162,13 +162,3 @@ void BL_ActionManager::Update(float curtime) } } } - -void BL_ActionManager::UpdateIPOs() -{ - BL_ActionMap::iterator it; - for (it = m_layers.begin(); it != m_layers.end(); ++it) - { - if (!it->second->IsDone()) - it->second->UpdateIPOs(); - } -} diff --git a/source/gameengine/Ketsji/BL_ActionManager.h b/source/gameengine/Ketsji/BL_ActionManager.h index 1292938d8f4..69c6d611df0 100644 --- a/source/gameengine/Ketsji/BL_ActionManager.h +++ b/source/gameengine/Ketsji/BL_ActionManager.h @@ -124,11 +124,6 @@ public: */ void Update(float); - /** - * Update object IPOs (note: not thread-safe!) - */ - void UpdateIPOs(); - #ifdef WITH_CXX_GUARDEDALLOC MEM_CXX_CLASS_ALLOC_FUNCS("GE:BL_ActionManager") #endif diff --git a/source/gameengine/Ketsji/KX_GameObject.cpp b/source/gameengine/Ketsji/KX_GameObject.cpp index d7a94f0601c..1dbcf14af89 100644 --- a/source/gameengine/Ketsji/KX_GameObject.cpp +++ b/source/gameengine/Ketsji/KX_GameObject.cpp @@ -481,11 +481,6 @@ void KX_GameObject::UpdateActionManager(float curtime) GetActionManager()->Update(curtime); } -void KX_GameObject::UpdateActionIPOs() -{ - GetActionManager()->UpdateIPOs(); -} - float KX_GameObject::GetActionFrame(short layer) { return GetActionManager()->GetActionFrame(layer); @@ -949,6 +944,9 @@ void KX_GameObject::InitIPO(bool ipo_as_force, void KX_GameObject::UpdateIPO(float curframetime, bool recurse) { + /* This function shouldn't call BL_Action::Update, not even indirectly, + * as it will cause deadlock due to the lock in BL_Action::Update. */ + // just the 'normal' update procedure. GetSGNode()->SetSimulatedTime(curframetime,recurse); GetSGNode()->UpdateWorldData(curframetime); diff --git a/source/gameengine/Ketsji/KX_GameObject.h b/source/gameengine/Ketsji/KX_GameObject.h index abd109a5e52..c2c455dab6a 100644 --- a/source/gameengine/Ketsji/KX_GameObject.h +++ b/source/gameengine/Ketsji/KX_GameObject.h @@ -327,12 +327,6 @@ public: */ void UpdateActionManager(float curtime); - /** - * Have the action manager update IPOs - * note: not thread-safe! - */ - void UpdateActionIPOs(); - /********************************* * End Animation API *********************************/ diff --git a/source/gameengine/Ketsji/KX_KetsjiEngine.cpp b/source/gameengine/Ketsji/KX_KetsjiEngine.cpp index 9d6fae6f555..c6e5f734c16 100644 --- a/source/gameengine/Ketsji/KX_KetsjiEngine.cpp +++ b/source/gameengine/Ketsji/KX_KetsjiEngine.cpp @@ -75,6 +75,8 @@ #include "KX_NavMeshObject.h" +#include "BL_Action.h" // For managing action lock. + #define DEFAULT_LOGIC_TIC_RATE 60.0 //#define DEFAULT_PHYSICS_TIC_RATE 60.0 @@ -181,6 +183,8 @@ KX_KetsjiEngine::KX_KetsjiEngine(KX_ISystem* system) #endif m_taskscheduler = BLI_task_scheduler_create(TASK_SCHEDULER_AUTO_THREADS); + + BL_Action::InitLock(); } @@ -200,6 +204,8 @@ KX_KetsjiEngine::~KX_KetsjiEngine() if (m_taskscheduler) BLI_task_scheduler_free(m_taskscheduler); + + BL_Action::EndLock(); } diff --git a/source/gameengine/Ketsji/KX_Scene.cpp b/source/gameengine/Ketsji/KX_Scene.cpp index c9c63371713..16d1fdd6ea2 100644 --- a/source/gameengine/Ketsji/KX_Scene.cpp +++ b/source/gameengine/Ketsji/KX_Scene.cpp @@ -1689,10 +1689,6 @@ void KX_Scene::UpdateAnimations(double curtime) BLI_task_pool_work_and_wait(pool); BLI_task_pool_free(pool); - - for (int i=0; iGetCount(); ++i) { - ((KX_GameObject*)m_animatedlist->GetValue(i))->UpdateActionIPOs(); - } } void KX_Scene::LogicUpdateFrame(double curtime, bool frame) -- cgit v1.2.3