Merge pull request #45576 from lawnjelly/bvh_visibility

BVH add support for visibility (activation)

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);
-
-			// 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);
+			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);
+
+				// 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);