From 5bd921fa3aa494af76972abf6299bcf44ec74b7f Mon Sep 17 00:00:00 2001 From: fredzio Date: Sat, 19 Dec 2020 20:25:46 +0100 Subject: [PATCH] Restore pre-async handling of absolute actors positionning One of the issue since the introduction of async physics is the quirky handling of scripted moves. Previous attempt to account for them was based on detecting changes in actor position while the physics thread is running. To this end, semantics of Actor::updatePosition() (which is responsible for set the absolute position of an actor in the world) was toned down to merely store the desired position, with the physics system actually responsible for moving the actor. For the cases were complete override of the physics simulation was needed, I introduced Actor::resetPosition(), which actually have same semantics as original updatePosition(). This in turn introduced a loads of new bugs when the weakened semantics broke key assumptions inside the engine (spawning, summoning, teleport, etc). Instead of tracking them down, count on the newly introduced support for object relative movements in the engine (World::moveObjectBy) to register relative movements and restore original handling of absolute positionning. Changes are relatively small: - move resetPosition() content into updatePosition() - call updatePosition() everywhere it was called before - remove all added calls to the now non-existing resetPosition() tldr; ditch last month worth of bug introduction and eradication and redo it properly --- apps/openmw/mwphysics/actor.cpp | 51 +++++++++---------------- apps/openmw/mwphysics/actor.hpp | 8 ++-- apps/openmw/mwphysics/mtphysics.cpp | 4 +- apps/openmw/mwphysics/physicssystem.cpp | 16 +------- apps/openmw/mwphysics/physicssystem.hpp | 1 - apps/openmw/mwworld/worldimp.cpp | 1 - 6 files changed, 23 insertions(+), 58 deletions(-) diff --git a/apps/openmw/mwphysics/actor.cpp b/apps/openmw/mwphysics/actor.cpp index 10fd463fb..015424db5 100644 --- a/apps/openmw/mwphysics/actor.cpp +++ b/apps/openmw/mwphysics/actor.cpp @@ -74,7 +74,7 @@ Actor::Actor(const MWWorld::Ptr& ptr, const Resource::BulletShape* shape, Physic updateRotation(); updateScale(); - resetPosition(); + updatePosition(); addCollisionMask(getCollisionMask()); updateCollisionObjectPosition(); } @@ -119,30 +119,34 @@ int Actor::getCollisionMask() const return collisionMask; } -void Actor::updatePositionUnsafe() +void Actor::updatePosition() { - if (!mWorldPositionChanged && mWorldPosition != mPtr.getRefData().getPosition().asVec3()) - mWorldPositionChanged = true; - mWorldPosition = mPtr.getRefData().getPosition().asVec3(); + std::scoped_lock lock(mPositionMutex); + updateWorldPosition(); + mPreviousPosition = mWorldPosition; + mPosition = mWorldPosition; + mSimulationPosition = mWorldPosition; + mStandingOnPtr = nullptr; + mSkipSimulation = true; } -void Actor::updatePosition() +void Actor::updateWorldPosition() { - std::scoped_lock lock(mPositionMutex); - updatePositionUnsafe(); + if (mWorldPosition != mPtr.getRefData().getPosition().asVec3()) + mWorldPositionChanged = true; + mWorldPosition = mPtr.getRefData().getPosition().asVec3(); } osg::Vec3f Actor::getWorldPosition() const { - std::scoped_lock lock(mPositionMutex); return mWorldPosition; } void Actor::setSimulationPosition(const osg::Vec3f& position) { - if (!mResetSimulation) + if (!mSkipSimulation) mSimulationPosition = position; - mResetSimulation = false; + mSkipSimulation = false; } osg::Vec3f Actor::getSimulationPosition() const @@ -150,8 +154,9 @@ osg::Vec3f Actor::getSimulationPosition() const return mSimulationPosition; } -void Actor::updateCollisionObjectPositionUnsafe() +void Actor::updateCollisionObjectPosition() { + std::scoped_lock lock(mPositionMutex); mShape->setLocalScaling(Misc::Convert::toBullet(mScale)); osg::Vec3f scaledTranslation = mRotation * osg::componentMultiply(mMeshTranslation, mScale); osg::Vec3f newPosition = scaledTranslation + mPosition; @@ -161,12 +166,6 @@ void Actor::updateCollisionObjectPositionUnsafe() mWorldPositionChanged = false; } -void Actor::updateCollisionObjectPosition() -{ - std::scoped_lock lock(mPositionMutex); - updateCollisionObjectPositionUnsafe(); -} - osg::Vec3f Actor::getCollisionObjectPosition() const { std::scoped_lock lock(mPositionMutex); @@ -189,18 +188,6 @@ void Actor::adjustPosition(const osg::Vec3f& offset) mPositionOffset += offset; } -void Actor::resetPosition() -{ - std::scoped_lock lock(mPositionMutex); - updatePositionUnsafe(); - mPreviousPosition = mWorldPosition; - mPosition = mWorldPosition; - mSimulationPosition = mWorldPosition; - mStandingOnPtr = nullptr; - mWorldPositionChanged = false; - mResetSimulation = true; -} - osg::Vec3f Actor::getPosition() const { return mPosition; @@ -214,8 +201,6 @@ osg::Vec3f Actor::getPreviousPosition() const void Actor::updateRotation () { std::scoped_lock lock(mPositionMutex); - if (mRotation == mPtr.getRefData().getBaseNode()->getAttitude()) - return; mRotation = mPtr.getRefData().getBaseNode()->getAttitude(); } @@ -246,7 +231,6 @@ osg::Vec3f Actor::getHalfExtents() const osg::Vec3f Actor::getOriginalHalfExtents() const { - std::scoped_lock lock(mPositionMutex); return mHalfExtents; } @@ -283,7 +267,6 @@ void Actor::setWalkingOnWater(bool walkingOnWater) void Actor::setCanWaterWalk(bool waterWalk) { - std::scoped_lock lock(mPositionMutex); if (waterWalk != mCanWaterWalk) { mCanWaterWalk = waterWalk; diff --git a/apps/openmw/mwphysics/actor.hpp b/apps/openmw/mwphysics/actor.hpp index b26b52d14..18419c2c8 100644 --- a/apps/openmw/mwphysics/actor.hpp +++ b/apps/openmw/mwphysics/actor.hpp @@ -60,7 +60,7 @@ namespace MWPhysics * Set mWorldPosition to the position in the Ptr's RefData. This is used by the physics simulation to account for * when an object is "instantly" moved/teleported as opposed to being moved by the physics simulation. */ - void updatePosition(); + void updateWorldPosition(); osg::Vec3f getWorldPosition() const; /** @@ -93,7 +93,7 @@ namespace MWPhysics * Returns true if the new position is different. */ bool setPosition(const osg::Vec3f& position); - void resetPosition(); + void updatePosition(); void adjustPosition(const osg::Vec3f& offset); osg::Vec3f getPosition() const; @@ -155,8 +155,6 @@ namespace MWPhysics void updateCollisionMask(); void addCollisionMask(int collisionMask); int getCollisionMask() const; - void updateCollisionObjectPositionUnsafe(); - void updatePositionUnsafe(); bool mCanWaterWalk; std::atomic mWalkingOnWater; @@ -180,7 +178,7 @@ namespace MWPhysics osg::Vec3f mPreviousPosition; osg::Vec3f mPositionOffset; bool mWorldPositionChanged; - bool mResetSimulation; + bool mSkipSimulation; btTransform mLocalTransform; mutable std::mutex mPositionMutex; diff --git a/apps/openmw/mwphysics/mtphysics.cpp b/apps/openmw/mwphysics/mtphysics.cpp index bedcaba6a..3774ee734 100644 --- a/apps/openmw/mwphysics/mtphysics.cpp +++ b/apps/openmw/mwphysics/mtphysics.cpp @@ -281,8 +281,8 @@ namespace MWPhysics mActorsFrameData.clear(); for (const auto& [_, actor] : actors) { - actor->resetPosition(); - actor->setSimulationPosition(actor->getWorldPosition()); // resetPosition skip next simulation, now we need to "consume" it + actor->updatePosition(); + actor->setSimulationPosition(actor->getWorldPosition()); // updatePosition skip next simulation, now we need to "consume" it actor->updateCollisionObjectPosition(); mMovedActors.emplace_back(actor->getPtr()); } diff --git a/apps/openmw/mwphysics/physicssystem.cpp b/apps/openmw/mwphysics/physicssystem.cpp index 9c239200b..0b3cdbff5 100644 --- a/apps/openmw/mwphysics/physicssystem.cpp +++ b/apps/openmw/mwphysics/physicssystem.cpp @@ -437,7 +437,6 @@ namespace MWPhysics ActorMap::iterator found = mActors.find(ptr); if (found == mActors.end()) return ptr.getRefData().getPosition().asVec3(); - resetPosition(ptr); return MovementSolver::traceDown(ptr, position, found->second.get(), mCollisionWorld.get(), maxHeight); } @@ -642,17 +641,6 @@ namespace MWPhysics if (foundActor != mActors.end()) { foundActor->second->updatePosition(); - mTaskScheduler->updateSingleAabb(foundActor->second); - return; - } - } - - void PhysicsSystem::resetPosition(const MWWorld::ConstPtr &ptr) - { - ActorMap::iterator foundActor = mActors.find(ptr); - if (foundActor != mActors.end()) - { - foundActor->second->resetPosition(); mTaskScheduler->updateSingleAabb(foundActor->second, true); return; } @@ -694,8 +682,6 @@ namespace MWPhysics if (found != mActors.end()) { bool cmode = found->second->getCollisionMode(); - if (cmode) - resetPosition(found->first); cmode = !cmode; found->second->enableCollisionMode(cmode); // NB: Collision body isn't disabled for vanilla TCL compatibility @@ -942,7 +928,7 @@ namespace MWPhysics void ActorFrameData::updatePosition() { - mActorRaw->updatePosition(); + mActorRaw->updateWorldPosition(); mPosition = mActorRaw->getPosition(); if (mMoveToWaterSurface) { diff --git a/apps/openmw/mwphysics/physicssystem.hpp b/apps/openmw/mwphysics/physicssystem.hpp index 39bf9dd8b..9ebd587f5 100644 --- a/apps/openmw/mwphysics/physicssystem.hpp +++ b/apps/openmw/mwphysics/physicssystem.hpp @@ -143,7 +143,6 @@ namespace MWPhysics void updateScale (const MWWorld::Ptr& ptr); void updateRotation (const MWWorld::Ptr& ptr); void updatePosition (const MWWorld::Ptr& ptr); - void resetPosition(const MWWorld::ConstPtr &ptr); void addHeightField (const float* heights, int x, int y, float triSize, float sqrtVerts, float minH, float maxH, const osg::Object* holdObject); diff --git a/apps/openmw/mwworld/worldimp.cpp b/apps/openmw/mwworld/worldimp.cpp index 6a7e39644..b3a9dbb02 100644 --- a/apps/openmw/mwworld/worldimp.cpp +++ b/apps/openmw/mwworld/worldimp.cpp @@ -1356,7 +1356,6 @@ namespace MWWorld } moveObject(ptr, ptr.getCell(), pos.x(), pos.y(), pos.z()); - mPhysics->resetPosition(ptr); } void World::fixPosition()