From df18f72384fb4d650b8c8838c293c4b8a7966410 Mon Sep 17 00:00:00 2001
From: lawnjelly <lawnjelly@gmail.com>
Date: Thu, 28 Jan 2021 13:48:15 +0000
Subject: [PATCH] BVH fix pairing AABB init and mask checks

Fix bug whereby AABBs were reused from previous items due to use of a pool, resulting in missed collisions.
Also use full mask collision checks for all cases except generic update.
---
 core/math/bvh.h        | 26 +++++++++++++++++---------
 core/math/bvh_pair.inc |  1 +
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/core/math/bvh.h b/core/math/bvh.h
index 3cb58f6c4ae..c6ebeb287e9 100644
--- a/core/math/bvh.h
+++ b/core/math/bvh.h
@@ -90,10 +90,10 @@ public:
 
 	BVHHandle create(T *p_userdata, const AABB &p_aabb = AABB(), int p_subindex = 0, bool p_pairable = false, uint32_t p_pairable_type = 0, uint32_t p_pairable_mask = 1) {
 
-		// not completely sure why, but the order of the pairing callbacks seems to be causing problems in
-		// the render tree unless these are flushed before creating a new object
+		// not sure if absolutely necessary to flush collisions here. It will cost performance to, instead
+		// of waiting for update, so only uncomment this if there are bugs.
 		if (USE_PAIRS) {
-			_check_for_collisions();
+			//_check_for_collisions();
 		}
 
 #ifdef TOOLS_ENABLED
@@ -107,7 +107,15 @@ public:
 		BVHHandle h = tree.item_add(p_userdata, p_aabb, p_subindex, p_pairable, p_pairable_type, p_pairable_mask);
 
 		if (USE_PAIRS) {
-			_add_changed_item(h, p_aabb);
+			// for safety initialize the expanded AABB
+			AABB &expanded_aabb = tree._pairs[h.id()].expanded_aabb;
+			expanded_aabb = p_aabb;
+			expanded_aabb.grow_by(tree._pairing_expansion);
+
+			// force a collision check no matter the AABB
+			_add_changed_item(h, p_aabb, false);
+
+			_check_for_collisions(true);
 		}
 
 		return h;
@@ -170,6 +178,8 @@ public:
 		}
 
 		tree.item_remove(p_handle);
+
+		_check_for_collisions(true);
 	}
 
 	// call e.g. once per frame (this does a trickle optimize)
@@ -192,11 +202,9 @@ public:
 
 		if (USE_PAIRS) {
 
-			// not completely sure why, but the order of the pairing callbacks seems to be causing problems in
-			// the render tree unless these are flushed before creating a new object.. we will do this for set_pairable
-			// to just in case. This may not be required, and may slow things down slightly when there are a lot
-			// of visibility changes in a frame
-			_check_for_collisions();
+			// not sure if absolutely necessary to flush collisions here. It will cost performance to, instead
+			// of waiting for update, so only uncomment this if there are bugs.
+			//_check_for_collisions();
 
 			// when the pairable state changes, we need to force a collision check because newly pairable
 			// items may be in collision, and unpairable items might move out of collision.
diff --git a/core/math/bvh_pair.inc b/core/math/bvh_pair.inc
index 6c5c4eb44f6..ced3022a538 100644
--- a/core/math/bvh_pair.inc
+++ b/core/math/bvh_pair.inc
@@ -14,6 +14,7 @@ struct ItemPairs {
 	void clear() {
 		num_pairs = 0;
 		extended_pairs.reset();
+		expanded_aabb = AABB();
 	}
 
 	AABB expanded_aabb;