[Bf-blender-cvs] [728d1ec] master: BGE: Fix T46381 : last action frame not updated.
Porteries Tristan
noreply at git.blender.org
Mon Oct 19 16:03:42 CEST 2015
Commit: 728d1ec504647f512aeb46d3eb7b15d262d94246
Author: Porteries Tristan
Date: Mon Oct 19 16:03:40 2015 +0200
Branches: master
https://developer.blender.org/rB728d1ec504647f512aeb46d3eb7b15d262d94246
BGE: Fix T46381 : last action frame not updated.
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
===================================================================
M source/gameengine/Ketsji/BL_Action.cpp
M source/gameengine/Ketsji/BL_Action.h
M source/gameengine/Ketsji/BL_ActionManager.cpp
M source/gameengine/Ketsji/BL_ActionManager.h
M source/gameengine/Ketsji/KX_GameObject.cpp
M source/gameengine/Ketsji/KX_GameObject.h
M source/gameengine/Ketsji/KX_KetsjiEngine.cpp
M source/gameengine/Ketsji/KX_Scene.cpp
===================================================================
diff --git a/source/gameengine/Ketsji/BL_Action.cpp b/source/gameengine/Ketsji/BL_Action.cpp
index 507476d..4d5b6a0 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 7a40441..f6124b0 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 4249db5..491be03 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 1292938..69c6d61 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 d7a94f0..1dbcf14 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 abd109a..c2c455d 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 9d6fae6..c6e5f73 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 c9c6337..16d1fdd 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; i<m_animatedlist->GetCount(); ++i) {
- ((KX_GameObject*)m_animatedlist->GetValue(i))->UpdateActionIPOs();
- }
}
void KX_Scene::LogicUpdateFrame(double curtime, bool frame)
More information about the Bf-blender-cvs
mailing list