From a814dda2aeac3e00aef8925d5d1394035ba85056 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Sat, 13 Feb 2021 18:53:32 +0000 Subject: [PATCH] BVH - fix deferred visible mesh collision check When making items visible from the visual server, the collision check is deferred to prevent two identical collision checks when set_pairable is called shortly after. It turns out that for some items (especially meshes), set_pairable is never called. This PR detects this occurrence and forces a collision check at the end of the routine. --- core/math/bvh.h | 23 +++++++++++++++++++++++ servers/visual/visual_server_scene.cpp | 17 ++++++++++++++++- servers/visual/visual_server_scene.h | 2 ++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/core/math/bvh.h b/core/math/bvh.h index 381901d5498..26decf2bb4c 100644 --- a/core/math/bvh.h +++ b/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 diff --git a/servers/visual/visual_server_scene.cpp b/servers/visual/visual_server_scene.cpp index c4c2a318699..780ff782207 100644 --- a/servers/visual/visual_server_scene.cpp +++ b/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); + } } } } diff --git a/servers/visual/visual_server_scene.h b/servers/visual/visual_server_scene.h index 6c4798b609c..6642d0db867 100644 --- a/servers/visual/visual_server_scene.h +++ b/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);