From b9c9f30ce052b5fd4b3b2b742b320c873a1be484 Mon Sep 17 00:00:00 2001 From: Diren D Bharwani Date: Sat, 4 Mar 2023 14:31:07 +0800 Subject: [PATCH] Fixed heap corruption caused by out-of-sync collider active states --- Assets/Scenes/SS_Playground.shade | 35 +++++-------------- .../Inspector/SHEditorComponentView.hpp | 2 -- .../src/Physics/Collision/SHCollider.cpp | 4 +-- .../Physics/Collision/SHCollisionSpace.cpp | 27 ++++++++------ .../src/Physics/Dynamics/SHContactManager.cpp | 13 ++----- .../src/Physics/Dynamics/SHPhysicsWorld.cpp | 2 -- .../Routines/SHPhysicsDebugDrawRoutine.cpp | 6 ++-- .../Routines/SHPhysicsPreUpdateRoutine.cpp | 22 ++++++++++++ 8 files changed, 56 insertions(+), 55 deletions(-) diff --git a/Assets/Scenes/SS_Playground.shade b/Assets/Scenes/SS_Playground.shade index cdb63d23..9acf4216 100644 --- a/Assets/Scenes/SS_Playground.shade +++ b/Assets/Scenes/SS_Playground.shade @@ -41,17 +41,17 @@ - EID: 3 Name: Default IsActive: true - NumberOfChildren: 1 + NumberOfChildren: 0 Components: Transform Component: - Translate: {x: -0.597273231, y: 5, z: 0} - Rotate: {x: -0, y: 0, z: 0.490080804} + Translate: {x: 0.141888797, y: 5, z: 0} + Rotate: {x: -0, y: 0, z: 0.490080774} Scale: {x: 0.999999702, y: 0.999999702, z: 1} IsActive: true RigidBody Component: - Type: Static + Type: Dynamic Auto Mass: false - Mass: .inf + Mass: 1 Drag: 0.00999999978 Angular Drag: 0.00999999978 Use Gravity: true @@ -88,26 +88,9 @@ NumberOfChildren: 0 Components: Transform Component: - Translate: {x: 0, y: -2, z: 0} - Rotate: {x: 0, y: 0, z: 0} - Scale: {x: 1, y: 1, z: 1} - IsActive: true - RigidBody Component: - Type: Dynamic - Auto Mass: false - Mass: 1 - Drag: 0.00999999978 - Angular Drag: 0.00999999978 - Use Gravity: true - Gravity Scale: 1 - Interpolate: true - Sleeping Enabled: true - Freeze Position X: false - Freeze Position Y: false - Freeze Position Z: false - Freeze Rotation X: false - Freeze Rotation Y: false - Freeze Rotation Z: false + Translate: {x: 0.34412086, y: 3.23541069, z: 0} + Rotate: {x: 0, y: 0, z: 0.490080804} + Scale: {x: 0.999999702, y: 0.999999702, z: 1} IsActive: true Collider Component: Colliders: @@ -120,5 +103,5 @@ Density: 1 Position Offset: {x: 0, y: 0, z: 0} Rotation Offset: {x: 0, y: 0, z: 0} - IsActive: true + IsActive: false Scripts: ~ \ No newline at end of file diff --git a/SHADE_Engine/src/Editor/EditorWindow/Inspector/SHEditorComponentView.hpp b/SHADE_Engine/src/Editor/EditorWindow/Inspector/SHEditorComponentView.hpp index 04008c7b..d313bdea 100644 --- a/SHADE_Engine/src/Editor/EditorWindow/Inspector/SHEditorComponentView.hpp +++ b/SHADE_Engine/src/Editor/EditorWindow/Inspector/SHEditorComponentView.hpp @@ -432,8 +432,6 @@ namespace SHADE // TODO: Handle differences between composite & hull collider if (ImGui::BeginMenu("Add Collider")) { - int newColl = -1; - if (ImGui::Selectable("Box Collider")) { auto* compositeCollider = reinterpret_cast(component->GetCollider()); diff --git a/SHADE_Engine/src/Physics/Collision/SHCollider.cpp b/SHADE_Engine/src/Physics/Collision/SHCollider.cpp index b50413b0..2f1de000 100644 --- a/SHADE_Engine/src/Physics/Collision/SHCollider.cpp +++ b/SHADE_Engine/src/Physics/Collision/SHCollider.cpp @@ -204,7 +204,7 @@ namespace SHADE for (auto* shape : shapes) { - if (PREV_STATE) // Previously inactive + if (!PREV_STATE) // Previously inactive broadphase->Insert(shape->id, shape->ComputeAABB()); else // Previously active broadphase->Remove(shape->id); @@ -300,7 +300,7 @@ namespace SHADE }; // Remove from broadphase - if (broadphase) + if (IsActive() && broadphase) broadphase->Remove((*shape)->id); shapeLibrary->DestroyShape(*shape); diff --git a/SHADE_Engine/src/Physics/Collision/SHCollisionSpace.cpp b/SHADE_Engine/src/Physics/Collision/SHCollisionSpace.cpp index 8be6786c..c0fb3724 100644 --- a/SHADE_Engine/src/Physics/Collision/SHCollisionSpace.cpp +++ b/SHADE_Engine/src/Physics/Collision/SHCollisionSpace.cpp @@ -49,11 +49,14 @@ namespace SHADE return; } - collider->broadphase = &broadphase; + if (collider->IsActive()) + { + collider->broadphase = &broadphase; - // Add all existing shapes to the broadphase - for (const auto* shape : collider->shapes) - broadphase.Insert(shape->id, shape->ComputeAABB()); + // Attempt to add all existing shapes to the broadphase + for (const auto* shape : collider->shapes) + broadphase.Insert(shape->id, shape->ComputeAABB()); + } } void SHCollisionSpace::RemoveCollider(SHCollider* collider) noexcept @@ -64,13 +67,17 @@ namespace SHADE if (NUM_SHAPES == 0) return; - for (uint32_t i = 0; i < NUM_SHAPES; ++i) - broadphase.Remove(collider->shapes[i]->id); - - if (contactManager) + // If collider is inactive, skip removals since they were already removed + if (collider->IsActive()) { - contactManager->RemoveInvalidatedTrigger(collider->entityID); - contactManager->RemoveInvalidatedManifold(collider->entityID); + for (uint32_t i = 0; i < NUM_SHAPES; ++i) + broadphase.Remove(collider->shapes[i]->id); + + if (contactManager) + { + contactManager->RemoveInvalidatedTrigger(collider->entityID); + contactManager->RemoveInvalidatedManifold(collider->entityID); + } } /* diff --git a/SHADE_Engine/src/Physics/Dynamics/SHContactManager.cpp b/SHADE_Engine/src/Physics/Dynamics/SHContactManager.cpp index c9b8dc5f..940c500c 100644 --- a/SHADE_Engine/src/Physics/Dynamics/SHContactManager.cpp +++ b/SHADE_Engine/src/Physics/Dynamics/SHContactManager.cpp @@ -252,22 +252,13 @@ namespace SHADE { // New states start at invalid. In the first frame of collision, move to enter. // If it already in enter, move to stay - if (state == SHCollisionState::ENTER) - state = SHCollisionState::STAY; - - if (state == SHCollisionState::INVALID) - state = SHCollisionState::ENTER; + state = state == SHCollisionState::INVALID ? SHCollisionState::ENTER : SHCollisionState::STAY; } else { // If already exited and still not colliding, the collision has expired. // Invalid states are removed in the next frame - if (state == SHCollisionState::EXIT) - state = SHCollisionState::INVALID; - - // If previously colliding, move to exit. - if (state == SHCollisionState::ENTER || state == SHCollisionState::STAY) - state = SHCollisionState::EXIT; + state = state == SHCollisionState::EXIT ? SHCollisionState::INVALID : SHCollisionState::EXIT; } } diff --git a/SHADE_Engine/src/Physics/Dynamics/SHPhysicsWorld.cpp b/SHADE_Engine/src/Physics/Dynamics/SHPhysicsWorld.cpp index b4e67688..db70579c 100644 --- a/SHADE_Engine/src/Physics/Dynamics/SHPhysicsWorld.cpp +++ b/SHADE_Engine/src/Physics/Dynamics/SHPhysicsWorld.cpp @@ -86,8 +86,6 @@ namespace SHADE void SHPhysicsWorld::Step(float dt) { - - /* * Detect Collisions */ diff --git a/SHADE_Engine/src/Physics/System/Routines/SHPhysicsDebugDrawRoutine.cpp b/SHADE_Engine/src/Physics/System/Routines/SHPhysicsDebugDrawRoutine.cpp index fb2750cc..3fab2440 100644 --- a/SHADE_Engine/src/Physics/System/Routines/SHPhysicsDebugDrawRoutine.cpp +++ b/SHADE_Engine/src/Physics/System/Routines/SHPhysicsDebugDrawRoutine.cpp @@ -57,7 +57,8 @@ namespace SHADE for (const auto& COLLIDER_COMPONENT : COLLIDER_COMPONENT_DENSE) { const auto* COLLIDER = COLLIDER_COMPONENT.GetCollider(); - drawCollider(debugDrawSystem, *COLLIDER); + if (COLLIDER->IsActive()) + drawCollider(debugDrawSystem, *COLLIDER); } } else if (!physicsDebugDrawSystem->collidersToDraw.empty()) @@ -65,7 +66,8 @@ namespace SHADE for (const auto EID : physicsDebugDrawSystem->collidersToDraw) { const auto* COLLIDER = SHComponentManager::GetComponent(EID)->GetCollider(); - drawCollider(debugDrawSystem, *COLLIDER); + if (COLLIDER->IsActive()) + drawCollider(debugDrawSystem, *COLLIDER); } } diff --git a/SHADE_Engine/src/Physics/System/Routines/SHPhysicsPreUpdateRoutine.cpp b/SHADE_Engine/src/Physics/System/Routines/SHPhysicsPreUpdateRoutine.cpp index 3f2de93f..b68406c0 100644 --- a/SHADE_Engine/src/Physics/System/Routines/SHPhysicsPreUpdateRoutine.cpp +++ b/SHADE_Engine/src/Physics/System/Routines/SHPhysicsPreUpdateRoutine.cpp @@ -52,8 +52,19 @@ namespace SHADE const bool RIGIDBODY_ACTIVE = physicsObject.rigidBody->IsActive(); if (IS_ACTIVE != RIGIDBODY_ACTIVE) + { physicsObject.rigidBody->SetIsActive(IS_ACTIVE); + // If previously active, we remove it from the world. + // If previously inactive, we add it back to the world. + // This removes the need to check for inactive objects in the world every frame to remove invalid contacts. + + if (IS_ACTIVE) // Previously inactive + physicsSystem->physicsWorld->AddRigidBody(physicsObject.rigidBody); + else // Previously active + physicsSystem->physicsWorld->RemoveRigidBody(physicsObject.rigidBody); + } + if (UPDATE_TRANSFORM) { const SHVec3& WORLD_POS = TRANSFORM_COMPONENT->GetWorldPosition(); @@ -71,8 +82,19 @@ namespace SHADE const bool COLLIDER_ACTIVE = physicsObject.collider->IsActive(); if (IS_ACTIVE != COLLIDER_ACTIVE) + { physicsObject.collider->SetIsActive(IS_ACTIVE); + // If previously active, we remove it from the world. + // If previously inactive, we add it back to the world. + // This removes the need to check for inactive objects in the world every frame to remove invalid contacts. + + if (IS_ACTIVE) // Previously inactive + physicsSystem->collisionSpace->AddCollider(physicsObject.collider); + else // Previously active + physicsSystem->collisionSpace->RemoveCollider(physicsObject.collider); + } + if (UPDATE_TRANSFORM) { const SHVec3& WORLD_POS = TRANSFORM_COMPONENT->GetWorldPosition();