Merge pull request #45970 from lawnjelly/bvh_fix_mesh_visible

BVH - fix deferred visible mesh collision check

core/math/bvh.h

@@ -137,6 +137,12 @@ public:
 		erase(h);
 	}
 
+	void force_collision_check(uint32_t p_handle) {
+		BVHHandle h;
+		h.set(p_handle);
+		force_collision_check(h);
+	}
+
 	bool activate(uint32_t p_handle, const AABB &p_aabb, bool p_delay_collision_check = false) {
 		BVHHandle h;
 		h.set(p_handle);
@@ -195,6 +201,23 @@ public:
 		_check_for_collisions(true);
 	}
 
+	// use in conjunction with activate if you have deferred the collision check, and
+	// set pairable has never been called.
+	// (deferred collision checks are a workaround for visual server for historical reasons)
+	void force_collision_check(BVHHandle p_handle) {
+		if (USE_PAIRS) {
+			// the aabb should already be up to date in the BVH
+			AABB aabb;
+			item_get_AABB(p_handle, aabb);
+
+			// add it as changed even if aabb not different
+			_add_changed_item(p_handle, aabb, false);
+
+			// force an immediate full collision check, much like calls to set_pairable
+			_check_for_collisions(true);
+		}
+	}
+
 	// these should be read as set_visible for render trees,
 	// but generically this makes items add or remove from the
 	// tree internally, to speed things up by ignoring inactive items

servers/visual/visual_server_scene.cpp

@@ -132,6 +132,10 @@ void VisualServerScene::SpatialPartitioningScene_BVH::deactivate(SpatialPartitio
 	_bvh.deactivate(p_handle - 1);
 }
 
+void VisualServerScene::SpatialPartitioningScene_BVH::force_collision_check(SpatialPartitionID p_handle) {
+	_bvh.force_collision_check(p_handle - 1);
+}
+
 void VisualServerScene::SpatialPartitioningScene_BVH::update() {
 	_bvh.update();
 }
@@ -782,7 +786,13 @@ void VisualServerScene::instance_set_visible(RID p_instance, bool p_visible) {
 
 	// give the opportunity for the spatial paritioning scene to use a special implementation of visibility
 	// for efficiency (supported in BVH but not octree)
-	if (instance->spatial_partition_id) {
+
+	// slightly bug prone optimization here - we want to avoid doing a collision check twice
+	// once when activating, and once when calling set_pairable. We do this by deferring the collision check.
+	// However, in some cases (notably meshes), set_pairable never gets called. So we want to catch this case
+	// and force a collision check (see later in this function).
+	// This is only done in two stages to maintain compatibility with the octree.
+	if (instance->spatial_partition_id && instance->scenario) {
 		if (p_visible) {
 			instance->scenario->sps->activate(instance->spatial_partition_id, instance->transformed_aabb);
 		} else {
@@ -828,6 +838,11 @@ void VisualServerScene::instance_set_visible(RID p_instance, bool p_visible) {
 
 		} break;
 		default: {
+			// if we haven't called set_pairable, we STILL need to do a collision check
+			// for activated items because we deferred it earlier in the call to activate.
+			if (instance->spatial_partition_id && instance->scenario && p_visible) {
+				instance->scenario->sps->force_collision_check(instance->spatial_partition_id);
+			}
 		}
 	}
 }

servers/visual/visual_server_scene.h

@@ -119,6 +119,7 @@ public:
 		virtual void move(SpatialPartitionID p_handle, const AABB &p_aabb) = 0;
 		virtual void activate(SpatialPartitionID p_handle, const AABB &p_aabb) {}
 		virtual void deactivate(SpatialPartitionID p_handle) {}
+		virtual void force_collision_check(SpatialPartitionID p_handle) {}
 		virtual void update() {}
 		virtual void update_collisions() {}
 		virtual void set_pairable(SpatialPartitionID p_handle, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask) = 0;
@@ -168,6 +169,7 @@ public:
 		void move(SpatialPartitionID p_handle, const AABB &p_aabb);
 		void activate(SpatialPartitionID p_handle, const AABB &p_aabb);
 		void deactivate(SpatialPartitionID p_handle);
+		void force_collision_check(SpatialPartitionID p_handle);
 		void update();
 		void update_collisions();
 		void set_pairable(SpatialPartitionID p_handle, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask);