BVH add support for visibility (activation)

A major feature lacking in the octree was proper support for setting visibility / activation. This meant that invisible objects were still causing lots of processing in the tree unnecessarily.

This PR adds proper support for activation, items are temporarily removed from the tree and collision detection when inactive.

core/math/bvh.h

@@ -88,7 +88,7 @@ public:
 		unpair_callback_userdata = p_userdata;
 	}
 
-	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) {
+	BVHHandle create(T *p_userdata, bool p_active, 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 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.
@@ -104,7 +104,7 @@ public:
 		}
 #endif
 
-		BVHHandle h = tree.item_add(p_userdata, p_aabb, p_subindex, p_pairable, p_pairable_type, p_pairable_mask);
+		BVHHandle h = tree.item_add(p_userdata, p_active, p_aabb, p_subindex, p_pairable, p_pairable_type, p_pairable_mask);
 
 		if (USE_PAIRS) {
 			// for safety initialize the expanded AABB
@@ -113,9 +113,10 @@ public:
 			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);
+			if (p_active) {
+				_add_changed_item(h, p_aabb, false);
+				_check_for_collisions(true);
+			}
 		}
 
 		return h;
@@ -136,6 +137,18 @@ public:
 		erase(h);
 	}
 
+	bool activate(uint32_t p_handle, const AABB &p_aabb, bool p_delay_collision_check = false) {
+		BVHHandle h;
+		h.set(p_handle);
+		return activate(h, p_aabb, p_delay_collision_check);
+	}
+
+	bool deactivate(uint32_t p_handle) {
+		BVHHandle h;
+		h.set(p_handle);
+		return deactivate(h);
+	}
+
 	void set_pairable(uint32_t p_handle, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask) {
 		BVHHandle h;
 		h.set(p_handle);
@@ -182,6 +195,54 @@ public:
 		_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
+	bool activate(BVHHandle p_handle, const AABB &p_aabb, bool p_delay_collision_check = false) {
+		// sending the aabb here prevents the need for the BVH to maintain
+		// a redundant copy of the aabb.
+		// returns success
+		if (tree.item_activate(p_handle, p_aabb)) {
+			if (USE_PAIRS) {
+
+				// in the special case of the render tree, when setting visibility we are using the combination of
+				// activate then set_pairable. This would case 2 sets of collision checks. For efficiency here we allow
+				// deferring to have a single collision check at the set_pairable call.
+				// Watch for bugs! This may cause bugs if set_pairable is not called.
+				if (!p_delay_collision_check) {
+					_add_changed_item(p_handle, p_aabb, false);
+
+					// force an immediate collision check, much like calls to set_pairable
+					_check_for_collisions(true);
+				}
+			}
+			return true;
+		}
+
+		return false;
+	}
+
+	bool deactivate(BVHHandle p_handle) {
+		// returns success
+		if (tree.item_deactivate(p_handle)) {
+			// call unpair and remove all references to the item
+			// before deleting from the tree
+			if (USE_PAIRS) {
+				_remove_changed_item(p_handle);
+
+				// force check for collisions, much like an erase was called
+				_check_for_collisions(true);
+			}
+			return true;
+		}
+
+		return false;
+	}
+
+	bool get_active(BVHHandle p_handle) const {
+		return tree.item_get_active(p_handle);
+	}
+
 	// call e.g. once per frame (this does a trickle optimize)
 	void update() {
 		tree.update();
@@ -206,21 +267,23 @@ public:
 			// 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.
-			// We cannot depend on waiting for the next update, because that may come much later.
-			AABB aabb;
-			item_get_AABB(p_handle, aabb);
+			if (get_active(p_handle)) {
+				// 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.
+				// We cannot depend on waiting for the next update, because that may come much later.
+				AABB aabb;
+				item_get_AABB(p_handle, aabb);
 
-			// passing false disables the optimization which prevents collision checks if
-			// the aabb hasn't changed
-			_add_changed_item(p_handle, aabb, false);
+				// passing false disables the optimization which prevents collision checks if
+				// the aabb hasn't changed
+				_add_changed_item(p_handle, aabb, false);
 
-			// force an immediate collision check (probably just for this one item)
-			// but it must be a FULL collision check, also checking pairable state and masks.
-			// This is because AABB intersecting objects may have changed pairable state / mask
-			// such that they should no longer be paired. E.g. lights.
-			_check_for_collisions(true);
+				// force an immediate collision check (probably just for this one item)
+				// but it must be a FULL collision check, also checking pairable state and masks.
+				// This is because AABB intersecting objects may have changed pairable state / mask
+				// such that they should no longer be paired. E.g. lights.
+				_check_for_collisions(true);
+			} // only if active
 		}
 	}
 

core/math/bvh_logic.inc

@@ -5,6 +5,10 @@ void _logic_item_remove_and_reinsert(uint32_t p_ref_id) {
 	// get the reference
 	ItemRef &ref = _refs[p_ref_id];
 
+	// no need to optimize inactive items
+	if (!ref.is_active())
+		return;
+
 	// special case of debug draw
 	if (ref.item_id == BVHCommon::INVALID)
 		return;

core/math/bvh_public.inc

@@ -1,5 +1,5 @@
 public:
-BVHHandle item_add(T *p_userdata, const AABB &p_aabb, int32_t p_subindex, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask, bool p_invisible = false) {
+BVHHandle item_add(T *p_userdata, bool p_active, const AABB &p_aabb, int32_t p_subindex, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask, bool p_invisible = false) {
 #ifdef BVH_VERBOSE_TREE
 	VERBOSE_PRINT("\nitem_add BEFORE");
 	_debug_recursive_print_tree(0);
@@ -60,15 +60,19 @@ BVHHandle item_add(T *p_userdata, const AABB &p_aabb, int32_t p_subindex, bool p
 	create_root_node(_current_tree);
 
 	// we must choose where to add to tree
-	ref->tnode_id = _logic_choose_item_add_node(_root_node_id[_current_tree], abb);
+	if (p_active) {
+		ref->tnode_id = _logic_choose_item_add_node(_root_node_id[_current_tree], abb);
 
-	bool refit = _node_add_item(ref->tnode_id, ref_id, abb);
+		bool refit = _node_add_item(ref->tnode_id, ref_id, abb);
 
-	if (refit) {
-		// only need to refit from the parent
-		const TNode &add_node = _nodes[ref->tnode_id];
-		if (add_node.parent_id != BVHCommon::INVALID)
-			refit_upward_and_balance(add_node.parent_id);
+		if (refit) {
+			// only need to refit from the parent
+			const TNode &add_node = _nodes[ref->tnode_id];
+			if (add_node.parent_id != BVHCommon::INVALID)
+				refit_upward_and_balance(add_node.parent_id);
+		}
+	} else {
+		ref->set_inactive();
 	}
 
 #ifdef BVH_VERBOSE
@@ -100,11 +104,14 @@ void _debug_print_refs() {
 bool item_move(BVHHandle p_handle, const AABB &p_aabb) {
 	uint32_t ref_id = p_handle.id();
 
-	BVH_ABB abb;
-	abb.from(p_aabb);
-
 	// get the reference
 	ItemRef &ref = _refs[ref_id];
+	if (!ref.is_active()) {
+		return false;
+	}
+
+	BVH_ABB abb;
+	abb.from(p_aabb);
 
 	BVH_ASSERT(ref.tnode_id != BVHCommon::INVALID);
 	TNode &tnode = _nodes[ref.tnode_id];
@@ -117,7 +124,14 @@ bool item_move(BVHHandle p_handle, const AABB &p_aabb) {
 		// for accurate collision detection
 		TLeaf &leaf = _node_get_leaf(tnode);
 
-		leaf.get_aabb(ref.item_id) = abb;
+		BVH_ABB &leaf_abb = leaf.get_aabb(ref.item_id);
+
+		// no change?
+		if (leaf_abb == abb) {
+			return false;
+		}
+
+		leaf_abb = abb;
 		_integrity_check_all();
 
 		return true;
@@ -168,8 +182,10 @@ void item_remove(BVHHandle p_handle) {
 	_extra[ref_id_moved_back].active_ref_id = active_ref_id;
 	////////////////////////////////////////
 
-	// remove the item from the node
-	node_remove_item(ref_id);
+	// remove the item from the node (only if active)
+	if (_refs[ref_id].is_active()) {
+		node_remove_item(ref_id);
+	}
 
 	// remove the item reference
 	_refs.free(ref_id);
@@ -186,6 +202,54 @@ void item_remove(BVHHandle p_handle) {
 #endif
 }
 
+// returns success
+bool item_activate(BVHHandle p_handle, const AABB &p_aabb) {
+	uint32_t ref_id = p_handle.id();
+	ItemRef &ref = _refs[ref_id];
+	if (ref.is_active()) {
+		// noop
+		return false;
+	}
+
+	// add to tree
+	BVH_ABB abb;
+	abb.from(p_aabb);
+
+	_current_tree = _handle_get_tree_id(p_handle);
+
+	// we must choose where to add to tree
+	ref.tnode_id = _logic_choose_item_add_node(_root_node_id[_current_tree], abb);
+	_node_add_item(ref.tnode_id, ref_id, abb);
+
+	refit_upward_and_balance(ref.tnode_id);
+
+	return true;
+}
+
+// returns success
+bool item_deactivate(BVHHandle p_handle) {
+	uint32_t ref_id = p_handle.id();
+	ItemRef &ref = _refs[ref_id];
+	if (!ref.is_active()) {
+		// noop
+		return false;
+	}
+
+	// remove from tree
+	BVH_ABB abb;
+	node_remove_item(ref_id, &abb);
+
+	// mark as inactive
+	ref.set_inactive();
+	return true;
+}
+
+bool item_get_active(BVHHandle p_handle) const {
+	uint32_t ref_id = p_handle.id();
+	const ItemRef &ref = _refs[ref_id];
+	return ref.is_active();
+}
+
 // during collision testing, we want to set the mask and whether pairable for the item testing from
 void item_fill_cullparams(BVHHandle p_handle, CullParams &r_params) const {
 	uint32_t ref_id = p_handle.id();
@@ -226,7 +290,10 @@ void item_set_pairable(const BVHHandle &p_handle, bool p_pairable, uint32_t p_pa
 	ex.pairable_type = p_pairable_type;
 	ex.pairable_mask = p_pairable_mask;
 
-	if ((ex.pairable != 0) != p_pairable) {
+	bool active = ref.is_active();
+	bool pairable_changed = (ex.pairable != 0) != p_pairable;
+
+	if (active && pairable_changed) {
 		// record abb
 		TNode &tnode = _nodes[ref.tnode_id];
 		TLeaf &leaf = _node_get_leaf(tnode);
@@ -238,6 +305,8 @@ void item_set_pairable(const BVHHandle &p_handle, bool p_pairable, uint32_t p_pa
 		// remove from old tree
 		node_remove_item(ref_id);
 
+		// we must set the pairable AFTER getting the current tree
+		// because the pairable status determines which tree
 		ex.pairable = p_pairable;
 
 		// add to new tree
@@ -255,6 +324,9 @@ void item_set_pairable(const BVHHandle &p_handle, bool p_pairable, uint32_t p_pa
 			if (add_node.parent_id != BVHCommon::INVALID)
 				refit_upward_and_balance(add_node.parent_id);
 		}
+	} else {
+		// always keep this up to date
+		ex.pairable = p_pairable;
 	}
 }
 

core/math/bvh_structs.inc

@@ -3,6 +3,12 @@ public:
 struct ItemRef {
 	uint32_t tnode_id; // -1 is invalid
 	uint32_t item_id; // in the leaf
+
+	bool is_active() const { return tnode_id != BVHCommon::INACTIVE; }
+	void set_inactive() {
+		tnode_id = BVHCommon::INACTIVE;
+		item_id = BVHCommon::INACTIVE;
+	}
 };
 
 // extra info kept in separate parallel list to the references,

core/math/bvh_tree.h

@@ -73,7 +73,11 @@
 
 // really just a namespace
 struct BVHCommon {
+	// these could possibly also be the same constant,
+	// although this may be useful for debugging.
+	// or use zero for invalid and +1 based indices.
 	static const uint32_t INVALID = (0xffffffff);
+	static const uint32_t INACTIVE = (0xfffffffe);
 };
 
 // really a handle, can be anything

servers/physics/broad_phase_bvh.cpp

@@ -34,7 +34,7 @@
 
 BroadPhaseSW::ID BroadPhaseBVH::create(CollisionObjectSW *p_object, int p_subindex, const AABB &p_aabb) {
 
-	ID oid = bvh.create(p_object, p_aabb, p_subindex, false, 1 << p_object->get_type(), 0);
+	ID oid = bvh.create(p_object, true, p_aabb, p_subindex, false, 1 << p_object->get_type(), 0);
 	return oid + 1;
 }
 

servers/visual/visual_server_scene.cpp

@@ -105,7 +105,12 @@ void VisualServerScene::camera_set_use_vertical_aspect(RID p_camera, bool p_enab
 
 /* SPATIAL PARTITIONING */
 VisualServerScene::SpatialPartitionID VisualServerScene::SpatialPartitioningScene_BVH::create(Instance *p_userdata, const AABB &p_aabb, int p_subindex, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask) {
-	return _bvh.create(p_userdata, p_aabb, p_subindex, p_pairable, p_pairable_type, p_pairable_mask) + 1;
+#if defined(DEBUG_ENABLED) && defined(TOOLS_ENABLED)
+	// we are relying on this instance to be valid in order to pass
+	// the visible flag to the bvh.
+	CRASH_COND(!p_userdata);
+#endif
+	return _bvh.create(p_userdata, p_userdata->visible, p_aabb, p_subindex, p_pairable, p_pairable_type, p_pairable_mask) + 1;
 }
 
 void VisualServerScene::SpatialPartitioningScene_BVH::erase(SpatialPartitionID p_handle) {
@@ -116,6 +121,17 @@ void VisualServerScene::SpatialPartitioningScene_BVH::move(SpatialPartitionID p_
 	_bvh.move(p_handle - 1, p_aabb);
 }
 
+void VisualServerScene::SpatialPartitioningScene_BVH::activate(SpatialPartitionID p_handle, const AABB &p_aabb) {
+	// be very careful here, we are deferring the collision check, expecting a set_pairable to be called
+	// immediately after.
+	// see the notes in the BVH function.
+	_bvh.activate(p_handle - 1, p_aabb, true);
+}
+
+void VisualServerScene::SpatialPartitioningScene_BVH::deactivate(SpatialPartitionID p_handle) {
+	_bvh.deactivate(p_handle - 1);
+}
+
 void VisualServerScene::SpatialPartitioningScene_BVH::update() {
 	_bvh.update();
 }
@@ -764,6 +780,16 @@ void VisualServerScene::instance_set_visible(RID p_instance, bool p_visible) {
 
 	instance->visible = 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) {
+		if (p_visible) {
+			instance->scenario->sps->activate(instance->spatial_partition_id, instance->transformed_aabb);
+		} else {
+			instance->scenario->sps->deactivate(instance->spatial_partition_id);
+		}
+	}
+
 	// when showing or hiding geometry, lights must be kept up to date to show / hide shadows
 	if ((1 << instance->base_type) & VS::INSTANCE_GEOMETRY_MASK) {
 		InstanceGeometryData *geom = static_cast<InstanceGeometryData *>(instance->base_data);

servers/visual/visual_server_scene.h

@@ -117,6 +117,8 @@ public:
 		virtual SpatialPartitionID create(Instance *p_userdata, const AABB &p_aabb = AABB(), int p_subindex = 0, bool p_pairable = false, uint32_t p_pairable_type = 0, uint32_t pairable_mask = 1) = 0;
 		virtual void erase(SpatialPartitionID p_handle) = 0;
 		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 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;
@@ -164,6 +166,8 @@ public:
 		SpatialPartitionID create(Instance *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);
 		void erase(SpatialPartitionID p_handle);
 		void move(SpatialPartitionID p_handle, const AABB &p_aabb);
+		void activate(SpatialPartitionID p_handle, const AABB &p_aabb);
+		void deactivate(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);