Currently, we use an `UnrefQueue` which supposedly aims to transfer destruction costs to another thread. The implications of this unusual pattern can not be well understood because some allocators might free resources more efficiently if they are freed by the same thread that allocated them. In addition, `UnrefQueue` complicates the validation of thread safety in our engine. Lastly, our current usage of `UnrefQueue` triggers `ref()`, `unref()` atomic operations as objects are passed into the queue. These operations could be more expensive than the actual destruction.
With this PR we thus remove `UnrefQueue`. We can expect these changes to have a minor impact at most because we free most resources elsewhere in `ResourceSystem::updateCache`.
==16218== Conditional jump or move depends on uninitialised value(s)
==16218== at 0xCDBC27: MWPhysics::Object::commitPositionChange() (object.cpp:68)
==16218== by 0xCE5B64: MWPhysics::PhysicsTaskScheduler::updatePtrAabb(std::shared_ptr<MWPhysics::PtrHolder> const&) (mtphysics.cpp:446)
==16218== by 0xCE5CC3: operator() (mtphysics.cpp:432)
==16218== by 0xCE5CC3: for_each<std::_Rb_tree_const_iterator<std::shared_ptr<MWPhysics::PtrHolder> >, MWPhysics::PhysicsTaskScheduler::updateAabbs()::<lambda(const std::shared_ptr<MWPhysics::PtrHolder>&)> > (stl_algo.h:3820)
==16218== by 0xCE5CC3: MWPhysics::PhysicsTaskScheduler::updateAabbs() (mtphysics.cpp:431)
==16218== by 0xCE5E35: MWPhysics::PhysicsTaskScheduler::afterPreStep() (mtphysics.cpp:586)
==16218== by 0xCE6238: operator() (mtphysics.cpp:533)
==16218== by 0xCE6238: wait<MWPhysics::PhysicsTaskScheduler::doSimulation()::<lambda()> > (barrier.hpp:33)
==16218== by 0xCE6238: MWPhysics::PhysicsTaskScheduler::doSimulation() (mtphysics.cpp:533)
==16218== by 0xCE6377: MWPhysics::PhysicsTaskScheduler::worker() (mtphysics.cpp:464)
==16218== by 0x74523C3: execute_native_thread_routine (thread.cc:82)
==16218== by 0x76FF258: start_thread (in /usr/lib/libpthread-2.33.so)
==16218== by 0x78155E2: clone (in /usr/lib/libc-2.33.so)
==16218==
==16218== Conditional jump or move depends on uninitialised value(s)
==16218== at 0xCDBC30: MWPhysics::Object::commitPositionChange() (object.cpp:73)
==16218== by 0xCE5B64: MWPhysics::PhysicsTaskScheduler::updatePtrAabb(std::shared_ptr<MWPhysics::PtrHolder> const&) (mtphysics.cpp:446)
==16218== by 0xCE5CC3: operator() (mtphysics.cpp:432)
==16218== by 0xCE5CC3: for_each<std::_Rb_tree_const_iterator<std::shared_ptr<MWPhysics::PtrHolder> >, MWPhysics::PhysicsTaskScheduler::updateAabbs()::<lambda(const std::shared_ptr<MWPhysics::PtrHolder>&)> > (stl_algo.h:3820)
==16218== by 0xCE5CC3: MWPhysics::PhysicsTaskScheduler::updateAabbs() (mtphysics.cpp:431)
==16218== by 0xCE5E35: MWPhysics::PhysicsTaskScheduler::afterPreStep() (mtphysics.cpp:586)
==16218== by 0xCE6238: operator() (mtphysics.cpp:533)
==16218== by 0xCE6238: wait<MWPhysics::PhysicsTaskScheduler::doSimulation()::<lambda()> > (barrier.hpp:33)
==16218== by 0xCE6238: MWPhysics::PhysicsTaskScheduler::doSimulation() (mtphysics.cpp:533)
==16218== by 0xCE6377: MWPhysics::PhysicsTaskScheduler::worker() (mtphysics.cpp:464)
==16218== by 0x74523C3: execute_native_thread_routine (thread.cc:82)
==16218== by 0x76FF258: start_thread (in /usr/lib/libpthread-2.33.so)
==16218== by 0x78155E2: clone (in /usr/lib/libc-2.33.so)
==16218==
To reduce amount of computations on the caller side and restrict possible
values.
* verts can't be non-int because it's a number of things.
* worldsize is initially defined as int by ESM::Land::REAL_SIZE.
* Put function to calculate heightfied shift into components to be able to
reuse by other binaries.
This is the value used by vanilla Morrowind engine. At this angle player
character starts sliding down the slope.
This fixes navigation and movement issue in Mournhold, Godsreach.
Use faster aabbTest but without destance filter. To avoid dependency on a
specific constant and correctly handle situations when there is a big
difference between actors sizes.
Simple Physics API modification for Lua
See merge request OpenMW/openmw!1216
(cherry picked from commit d88494c90b501d0832ae0330a0ca81d8b8e5aa50)
02dd055a Save hitObject in castSphere() just like in castRay()
0793d0bf Allow to override collision mask and group for castSphere() as for castRay()
- enchanted arrow explode upon hit the water plane
- non enchanted arrow disappear (or more accurately, they hit nothingness)
- enchanted arrow shot underwater explode immediately
- non enchanted arrow disappear immediately
Also, solve a bug that occured previously and could theoritically still happens where we use the last tested collision position for instead of the last registered hit:
Use the hit position as saved inside Projectile::hit() instead of the last position saved inside the callback.
If a projectile collides with several objects (bottom of the sea and water surface for instance), the last collision tested won't necessarily be the impact position as we have no control over the order in which the tests are performed.
simulation
The purpose of weak_ptr is to avoid performing the simulation on deleted
Actor by promoting it to a shared_ptr via a lock() call. This clutter
the code with a lot of branches, whereas the overwhelmingly common case is for the call to succeed.
Since the simulation is decoupled from the engine state, we can use a shared_ptr instead of a weak_ptr.
This allow us to ignore (ie not handle) the rarer case where the actor is delete from the scene. This means that the simulation
will run for one frame more than before for each actor, whereas the rest of the engine
will be ignorant of this.
PhysicsSystem::traceDown before inserting into mActors.
The latter does nothing until the actor is inserted into mActors.
We can't move the call after the insertion either because then
the actor is part of the simulation, and we'd have a race.
collision handling and castRay() to avoid calling getPtr(). It is a step forward
removing the mutex inside of PtrHolder.
Do the same for DeepestNotMeContactTestResultCallback. It is used
only for not-ranged combat for now, but do it anyway for parity with all
other callback. This way, once the PtrHolder mutex is gone one will not
have to worry about wether it is safe to use the callback in a specific
context.
To avoid use-after-free with projectile / projectile collision, defer deletion of projectile.
Since instead of storing a copy of target Ptr we have a pointer to its collision object,
we can't delete projectiles until after we finished iterating over the loops.
MWWorld::Ptr:
- they are equivalent
- btCollisionObject* is readily available from the simulation, it saves
a call to a mutex
- btCollisionObject* is smaller
- compute the swimming state instead of storing it, it changes as part of the simulation and was not updated, so it was wrong anyway.
- store the swim level in ActorFrameData, it is constant per Actor so no need to compute it inside the simulation
ActActorFrameData structure. It makes it easier to reason about the
simulation (and hopefully simplify it).
Remove atomics from Actor class as a side effect.
Rename mFloatToSurface to mInert to make is explicit what it represent, not what it is used for
Store the Actor rotation (1 Vec2) instead of the whole ESM::Position (2 Vec3)
When we call moveObject(), we might trigger a change of cell for the
actor, which in turn triggers updatePtr(). The erase/emplace
construct invalidate references, whereas extract/insert do not.
The reason is was working before !1075 is because we were always
"refreshing" the reference by a call to getActor().
