[physx] fixed errors during async loading

Author: PanosK92

source/runtime/Physics/PhysicsWorld.cpp

@@ -53,7 +53,7 @@ namespace spartan
 {
     namespace
     {
-        mutex scene_mutex;
+        recursive_mutex physx_mutex;
     }
 
     namespace settings
@@ -95,6 +95,7 @@ namespace spartan
             PxRaycastBuffer hit;
             PxQueryFilterData filter_data(PxQueryFlag::eDYNAMIC); // only pick dynamic bodies - static/kinematic can be moved as per usual from the editor
             PxScene* scene = static_cast<PxScene*>(PhysicsWorld::GetScene());
+            lock_guard<recursive_mutex> lock(PhysicsWorld::GetMutex());
             if (scene->raycast(origin, direction, 1000.0f, hit, PxHitFlag::eDEFAULT, filter_data) && hit.hasBlock)
             {
                 PxRigidActor* actor = hit.block.actor;
@@ -142,6 +143,7 @@ namespace spartan
         {
             if (picked_body && joint)
             {
+                lock_guard<recursive_mutex> lock(PhysicsWorld::GetMutex());
                 joint->release();
                 joint = nullptr;
 
@@ -178,6 +180,7 @@ namespace spartan
             PxVec3 target = origin + direction * pick_distance;
 
             // move dummy actor to target
+            lock_guard<recursive_mutex> lock(PhysicsWorld::GetMutex());
             dummy_actor->setKinematicTarget(PxTransform(target));
         }
     }
@@ -319,6 +322,7 @@ namespace spartan
                 while (accumulated_time >= fixed_time_step)
                 {
                     // simulate one fixed time step
+                    lock_guard<recursive_mutex> lock(physx_mutex);
                     scene->simulate(fixed_time_step);
                     scene->fetchResults(true); // block
                     accumulated_time -= fixed_time_step;
@@ -345,6 +349,8 @@ namespace spartan
         // debug visualization (editor only, skip during play)
         if (cvar_physics.GetValueAs<bool>() && !Engine::IsFlagSet(EngineMode::Playing))
         {
+            lock_guard<recursive_mutex> lock(physx_mutex);
+
             // run a near-zero step so physx populates the render buffer (physx requires dt > 0)
             scene->simulate(numeric_limits<float>::min());
             scene->fetchResults(true);
@@ -369,7 +375,7 @@ namespace spartan
     {
         if (actor && scene && !actor->getScene())
         {
-            lock_guard<mutex> lock(scene_mutex);
+            lock_guard<recursive_mutex> lock(physx_mutex);
             scene->addActor(*actor);
         }
     }
@@ -378,7 +384,7 @@ namespace spartan
     {
         if (actor && scene && actor->getScene() == scene)
         {
-            lock_guard<mutex> lock(scene_mutex);
+            lock_guard<recursive_mutex> lock(physx_mutex);
             scene->removeActor(*actor);
         }
     }
@@ -400,6 +406,11 @@ namespace spartan
     {
         return static_cast<void*>(physics);
     }
+
+    recursive_mutex& PhysicsWorld::GetMutex()
+    {
+        return physx_mutex;
+    }
 
     float PhysicsWorld::GetInterpolationAlpha()
     {
@@ -428,7 +439,7 @@ namespace spartan
         PxRaycastBuffer hit;
         PxQueryFilterData filter_data(PxQueryFlag::eSTATIC);
 
-        lock_guard<mutex> lock(scene_mutex);
+        lock_guard<recursive_mutex> lock(physx_mutex);
         if (scene->raycast(px_origin, px_direction, max_distance, hit, PxHitFlag::eDEFAULT, filter_data) && hit.hasBlock)
         {
             hit_position = Vector3(hit.block.position.x, hit.block.position.y, hit.block.position.z);

source/runtime/Physics/PhysicsWorld.h

@@ -22,6 +22,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #pragma once
 
 //= INCLUDES ===============
+#include <mutex>
 #include <vector>
 #include "../Math/Vector3.h"
 //==========================
@@ -58,5 +59,11 @@ namespace spartan
 
         // cast a ray against static geometry and return the closest hit position + the entity that was hit
         static bool RaycastStatic(const math::Vector3& origin, const math::Vector3& direction, float max_distance, math::Vector3& hit_position, Entity*& hit_entity);
+
+        // global physx scene lock, used to serialize all reads and writes that touch
+        // PxScene/PxRigidActor/PxShape state, async scene loading runs Physics::Create on
+        // worker threads which races with the main thread, recursive so the same thread
+        // can re-enter under nested helpers like PhysicsWorld::AddActor
+        static std::recursive_mutex& GetMutex();
     };
 }

source/runtime/Rendering/Renderer_Passes_Lighting.cpp

@@ -425,20 +425,19 @@ namespace spartan
 
     void Renderer::Pass_ReSTIR_PathTracing(RHI_CommandList* cmd_list)
     {
-        const uint32_t min_rt_dimension = 64;
         if (Window::IsMinimized())
-        {
             return;
-        }
 
         RHI_Texture* tex_gi     = GetRenderTarget(Renderer_RenderTarget::restir_output);
         RHI_Texture* reservoir0 = GetRenderTarget(Renderer_RenderTarget::restir_reservoir0);
-        SP_ASSERT(tex_gi != nullptr);
 
+        // restir resources are gated by cvar_restir_pt, nothing to do when they are not allocated
+        if (!tex_gi)
+            return;
+
+        const uint32_t min_rt_dimension = 64;
         if (tex_gi->GetWidth() < min_rt_dimension || tex_gi->GetHeight() < min_rt_dimension)
-        {
             return;
-        }
 
         if (!cvar_restir_pt.GetValueAs<bool>() || !RHI_Device::IsSupportedRayTracing() || !reservoir0)
         {
@@ -453,9 +452,7 @@ namespace spartan
 
         RHI_AccelerationStructure* tlas = GetTopLevelAccelerationStructure();
         if (!tlas)
-        {
             return;
-        }
 
         RHI_Shader* shader_rgen = GetShader(Renderer_Shader::restir_pt_ray_generation_r);
         RHI_Shader* shader_miss = GetShader(Renderer_Shader::restir_pt_ray_miss_r);
@@ -498,21 +495,21 @@ namespace spartan
 
     void Renderer::Pass_ReSTIR_Denoising(RHI_CommandList* cmd_list)
     {
+        if (Window::IsMinimized())
+            return;
+
         RHI_Texture* tex_gi_raw      = GetRenderTarget(Renderer_RenderTarget::restir_output);
         RHI_Texture* tex_gi_denoised = GetRenderTarget(Renderer_RenderTarget::restir_denoised);
         RHI_Texture* tex_gi_history  = GetRenderTarget(Renderer_RenderTarget::restir_denoised_history);
         RHI_Texture* tex_gi_ping     = GetRenderTarget(Renderer_RenderTarget::restir_denoised_ping);
-        SP_ASSERT(tex_gi_raw && tex_gi_denoised && tex_gi_history && tex_gi_ping);
 
-        const uint32_t min_rt_dimension = 64;
-        if (Window::IsMinimized())
-        {
+        // restir resources are gated by cvar_restir_pt, nothing to do when they are not allocated
+        if (!tex_gi_raw || !tex_gi_denoised || !tex_gi_history || !tex_gi_ping)
             return;
-        }
+
+        const uint32_t min_rt_dimension = 64;
         if (tex_gi_raw->GetWidth() < min_rt_dimension || tex_gi_raw->GetHeight() < min_rt_dimension)
-        {
             return;
-        }
 
         if (!cvar_restir_pt.GetValueAs<bool>() || !RHI_Device::IsSupportedRayTracing())
         {
@@ -835,7 +832,6 @@ namespace spartan
         RHI_Texture* tex_light_specular   = GetRenderTarget(Renderer_RenderTarget::light_specular);
         RHI_Texture* tex_light_volumetric = GetRenderTarget(Renderer_RenderTarget::light_volumetric);
         RHI_Texture* tex_gi               = GetRenderTarget(Renderer_RenderTarget::restir_denoised);
-        SP_ASSERT(tex_gi != nullptr);
 
         cmd_list->InsertBarrier(tex_out, RHI_BarrierType::EnsureReadThenWrite);
 

source/runtime/World/Components/Physics.cpp

@@ -225,6 +225,9 @@ namespace spartan
 
     void Physics::Remove()
     {
+        // serialize physx writes, async scene loading runs this on worker threads in parallel
+        lock_guard<recursive_mutex> physx_lock(PhysicsWorld::GetMutex());
+
         // release controller if it exists
         // skip if controller_manager was already released during shutdown
         if (m_controller && controller_manager)
@@ -267,22 +270,24 @@ namespace spartan
 
     void Physics::PreTick()
     {
-        // deferred creation after loading (renderable component needs to be available first)
-        if (m_needs_creation)
-        {
-            m_needs_creation = false;
-            Create();
-        }
-
         // during world load worker threads are busy inside pxphysics/pxscene creating actors and
         // shapes, the editor sync path below writes to pxrigidactor on the main thread which physx
         // flags as concurrent api access and corrupts the internal pruner aabb tree, the entities
         // were just placed at the right pose so this sync would be a no-op anyway
+        // we also defer Create until loading completes, this avoids racing with workers that are
+        // still populating sibling components like Render whose mesh data we read for the shape
         if (ProgressTracker::IsLoading())
         {
             return;
         }
 
+        // deferred creation after loading (renderable component needs to be available first)
+        if (m_needs_creation)
+        {
+            m_needs_creation = false;
+            Create();
+        }
+
         // sync physics transforms to entities before other components (like camera) tick
         // this ensures child entities have up-to-date parent transforms when they compute matrices
         const bool is_playing  = Engine::IsFlagSet(EngineMode::Playing);
@@ -1192,6 +1197,7 @@ namespace spartan
         // get the actor used by the controller to avoid returning itself
         PxRigidActor* actor_to_ignore = controller->getActor();
 
+        lock_guard<recursive_mutex> physx_lock(PhysicsWorld::GetMutex());
         if (scene->raycast(ray_start, ray_dir, ray_length, hit, PxHitFlag::eDEFAULT, filter_data))
         {
             for (PxU32 i = 0; i < hit.nbTouches; ++i)
@@ -1511,6 +1517,10 @@ namespace spartan
             return;
         }
 
+        // car::set_chassis below mutates shapes on car::body which is already attached to the
+        // scene, async load runs this on worker threads so serialize against other physx writes
+        lock_guard<recursive_mutex> physx_lock(PhysicsWorld::GetMutex());
+
         // collect all entities with renderables in the hierarchy
         vector<Entity*> mesh_entities;
         mesh_entities.push_back(chassis_entity);
@@ -1713,6 +1723,9 @@ namespace spartan
         // recalculate and update body height based on actual wheel radius
         if (car::body)
         {
+            // car::body is in the scene, async load reaches this from car prefab workers
+            lock_guard<recursive_mutex> physx_lock(PhysicsWorld::GetMutex());
+
             // calculate correct body height using actual spring stiffness
             float front_mass_per_wheel = car::cfg.mass * 0.40f * 0.5f;
             float front_omega = 2.0f * math::pi * car::tuning::spec.front_spring_freq;
@@ -2551,6 +2564,11 @@ namespace spartan
 
     void Physics::Create()
     {
+        // serialize the entire physx setup, the car prefab path calls this on loader workers
+        // in parallel with the main thread's PreTick, so without this lock physx flags concurrent
+        // scene writes (addActor, setGlobalPose, setSimulationFilterData) and corrupts the scene
+        lock_guard<recursive_mutex> physx_lock(PhysicsWorld::GetMutex());
+
         // clear previous state
         Remove();