diff --git a/core/math/bvh.h b/core/math/bvh.h index b4acaad8dd0..a1ec0201477 100644 --- a/core/math/bvh.h +++ b/core/math/bvh.h @@ -175,6 +175,11 @@ public: #endif } + // this can be called more frequently than per frame if necessary + void update_collisions() { + _check_for_collisions(); + } + // prefer calling this directly as type safe void set_pairable(const BVHHandle &p_handle, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask) { tree.item_set_pairable(p_handle, p_pairable, p_pairable_type, p_pairable_mask); @@ -191,7 +196,10 @@ public: _add_changed_item(p_handle, aabb, false); // force an immediate collision check (probably just for this one item) - _check_for_collisions(); + // 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); } } @@ -275,7 +283,12 @@ public: private: // do this after moving etc. - void _check_for_collisions() { + void _check_for_collisions(bool p_full_check = false) { + if (!changed_items.size()) { + // noop + return; + } + AABB bb; typename BVHTREE_CLASS::CullParams params; @@ -297,7 +310,7 @@ private: // find all the existing paired aabbs that are no longer // paired, and send callbacks - _find_leavers(h, abb); + _find_leavers(h, abb, p_full_check); uint32_t changed_item_ref_id = h.id(); @@ -369,13 +382,31 @@ private: } // returns true if unpair - bool _find_leavers_process_pair(typename BVHTREE_CLASS::ItemPairs &p_pairs_from, const BVH_ABB &p_abb_from, BVHHandle p_from, BVHHandle p_to) { + bool _find_leavers_process_pair(typename BVHTREE_CLASS::ItemPairs &p_pairs_from, const BVH_ABB &p_abb_from, BVHHandle p_from, BVHHandle p_to, bool p_full_check) { BVH_ABB abb_to; tree.item_get_ABB(p_to, abb_to); // do they overlap? - if (p_abb_from.intersects(abb_to)) - return false; + if (p_abb_from.intersects(abb_to)) { + // the full check for pairable / non pairable and mask changes is extra expense + // this need not be done in most cases (for speed) except in the case where set_pairable is called + // where the masks etc of the objects in question may have changed + if (!p_full_check) { + return false; + } + const typename BVHTREE_CLASS::ItemExtra &exa = _get_extra(p_from); + const typename BVHTREE_CLASS::ItemExtra &exb = _get_extra(p_to); + + // one of the two must be pairable to still pair + // if neither are pairable, we always unpair + if (exa.pairable || exb.pairable) { + // the masks must still be compatible to pair + // i.e. if there is a hit between the two, then they should stay paired + if (tree._cull_pairing_mask_test_hit(exa.pairable_mask, exa.pairable_type, exb.pairable_mask, exb.pairable_type)) { + return false; + } + } + } _unpair(p_from, p_to); return true; @@ -383,18 +414,15 @@ private: // find all the existing paired aabbs that are no longer // paired, and send callbacks - void _find_leavers(BVHHandle p_handle, const BVH_ABB &expanded_abb_from) { + void _find_leavers(BVHHandle p_handle, const BVH_ABB &expanded_abb_from, bool p_full_check) { typename BVHTREE_CLASS::ItemPairs &p_from = tree._pairs[p_handle.id()]; - // opportunity to de-extend pairs, before removing leavers - p_from.update(); - BVH_ABB abb_from = expanded_abb_from; // remove from pairing list for every partner for (unsigned int n = 0; n < p_from.extended_pairs.size(); n++) { BVHHandle h_to = p_from.extended_pairs[n].handle; - if (_find_leavers_process_pair(p_from, abb_from, p_handle, h_to)) { + if (_find_leavers_process_pair(p_from, abb_from, p_handle, h_to, p_full_check)) { // we need to keep the counter n up to date if we deleted a pair // as the number of items in p_from.extended_pairs will have decreased by 1 // and we don't want to miss an item @@ -465,11 +493,12 @@ private: void _add_changed_item(BVHHandle p_handle, const AABB &aabb, bool p_check_aabb = true) { - // only if uses pairing - // no .. non pairable items seem to be able to pair with pairable + // Note that non pairable items can pair with pairable, + // so all types must be added to the list // aabb check with expanded aabb. This greatly decreases processing // at the cost of slightly less accurate pairing checks + // Note this pairing AABB is separate from the AABB in the actual tree AABB &expanded_aabb = tree._pairs[p_handle.id()].expanded_aabb; // passing p_check_aabb false disables the optimization which prevents collision checks if @@ -478,6 +507,13 @@ private: if (p_check_aabb && expanded_aabb.encloses(aabb)) return; + // ALWAYS update the new expanded aabb, even if already changed once + // this tick, because it is vital that the AABB is kept up to date + expanded_aabb = aabb; + expanded_aabb.grow_by(tree._pairing_expansion); + + // this code is to ensure that changed items only appear once on the updated list + // collision checking them multiple times is not needed, and repeats the same thing uint32_t &last_updated_tick = tree._extra[p_handle.id()].last_updated_tick; if (last_updated_tick == _tick) @@ -486,12 +522,7 @@ private: // mark as on list last_updated_tick = _tick; - // opportunity to de-extend pairs (before collision detection, which will delete then recreate pairs) - - // new expanded aabb - expanded_aabb = aabb; - expanded_aabb.grow_by(tree._pairing_expansion); - + // add to the list changed_items.push_back(p_handle); } diff --git a/core/math/bvh_cull.inc b/core/math/bvh_cull.inc index dcbdb222850..3083df550e4 100644 --- a/core/math/bvh_cull.inc +++ b/core/math/bvh_cull.inc @@ -133,6 +133,22 @@ bool _cull_hits_full(const CullParams &p) { return (int)_cull_hits.size() >= p.result_max; } +// write this logic once for use in all routines +// double check this as a possible source of bugs in future. +bool _cull_pairing_mask_test_hit(uint32_t p_maskA, uint32_t p_typeA, uint32_t p_maskB, uint32_t p_typeB) const { + // double check this as a possible source of bugs in future. + bool A_match_B = p_maskA & p_typeB; + + if (!A_match_B) { + bool B_match_A = p_maskB & p_typeA; + if (!B_match_A) { + return false; + } + } + + return true; +} + void _cull_hit(uint32_t p_ref_id, CullParams &p) { // take into account masks etc @@ -141,13 +157,8 @@ void _cull_hit(uint32_t p_ref_id, CullParams &p) { if (USE_PAIRS) { const ItemExtra &ex = _extra[p_ref_id]; - // double check this as a possible source of bugs in future. - bool from_match_to = p.mask & ex.pairable_type; - - if (!from_match_to) { - bool to_match_from = ex.pairable_mask & p.pairable_type; - if (!to_match_from) - return; + if (!_cull_pairing_mask_test_hit(p.mask, p.pairable_type, ex.pairable_mask, ex.pairable_type)) { + return; } } diff --git a/core/math/bvh_pair.inc b/core/math/bvh_pair.inc index ce79857f088..6c5c4eb44f6 100644 --- a/core/math/bvh_pair.inc +++ b/core/math/bvh_pair.inc @@ -58,7 +58,4 @@ struct ItemPairs { return userdata; } - - void update() { - } }; diff --git a/servers/visual/visual_server_raster.cpp b/servers/visual/visual_server_raster.cpp index 2cbf9ac537e..f863d82c691 100644 --- a/servers/visual/visual_server_raster.cpp +++ b/servers/visual/visual_server_raster.cpp @@ -104,6 +104,7 @@ void VisualServerRaster::draw(bool p_swap_buffers, double frame_step) { VSG::rasterizer->begin_frame(frame_step); VSG::scene->update_dirty_instances(); //update scene stuff + VSG::scene->update_scenarios(); // render tree collision detection (pairing) VSG::viewport->draw_viewports(); VSG::scene->render_probes(); diff --git a/servers/visual/visual_server_scene.cpp b/servers/visual/visual_server_scene.cpp index 7c4b201400a..fd88132d821 100644 --- a/servers/visual/visual_server_scene.cpp +++ b/servers/visual/visual_server_scene.cpp @@ -120,6 +120,10 @@ void VisualServerScene::SpatialPartitioningScene_BVH::update() { _bvh.update(); } +void VisualServerScene::SpatialPartitioningScene_BVH::update_collisions() { + _bvh.update_collisions(); +} + void VisualServerScene::SpatialPartitioningScene_BVH::set_pairable(SpatialPartitionID p_handle, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask) { _bvh.set_pairable(p_handle - 1, p_pairable, p_pairable_type, p_pairable_mask); } @@ -191,7 +195,7 @@ void VisualServerScene::SpatialPartitioningScene_Octree::set_balance(float p_bal VisualServerScene::Scenario::Scenario() { debug = VS::SCENARIO_DEBUG_DISABLED; - bool use_bvh_or_octree = GLOBAL_DEF("rendering/quality/spatial_partitioning/use_bvh", true); + bool use_bvh_or_octree = GLOBAL_GET("rendering/quality/spatial_partitioning/use_bvh"); if (use_bvh_or_octree) { sps = memnew(SpatialPartitioningScene_BVH); @@ -3574,24 +3578,47 @@ void VisualServerScene::_update_dirty_instance(Instance *p_instance) { p_instance->update_materials = false; } +void VisualServerScene::update_scenarios() { + // go through all scenarios and update BVH in each + if (!_use_bvh) { + return; + } + + List owned; + scenario_owner.get_owned_list(&owned); + + for (List::Element *E = owned.front(); E; E = E->next()) { + RID rid = E->get(); + Scenario *scenario = scenario_owner.get(rid); + + scenario->sps->update(); + } +} + void VisualServerScene::update_dirty_instances() { VSG::storage->update_dirty_resources(); + // only define this if you run into problems with missed pairing collisions for debugging +//#define GODOT_RENDER_SPS_EXTRA_COLLISION_CHECKS +#ifdef GODOT_RENDER_SPS_EXTRA_COLLISION_CHECKS // this is just to get access to scenario so we can update the spatial partitioning scheme Scenario *scenario = nullptr; if (_instance_update_list.first()) { scenario = _instance_update_list.first()->self()->scenario; } +#endif while (_instance_update_list.first()) { _update_dirty_instance(_instance_update_list.first()->self()); } +#ifdef GODOT_RENDER_SPS_EXTRA_COLLISION_CHECKS if (scenario) { - scenario->sps->update(); + scenario->sps->update_collisions(); } +#endif } bool VisualServerScene::free(RID p_rid) { @@ -3652,7 +3679,7 @@ VisualServerScene::VisualServerScene() { render_pass = 1; singleton = this; - _use_bvh = false; + _use_bvh = GLOBAL_DEF("rendering/quality/spatial_partitioning/use_bvh", true); } VisualServerScene::~VisualServerScene() { diff --git a/servers/visual/visual_server_scene.h b/servers/visual/visual_server_scene.h index a33e9a4e97d..f2133566b93 100644 --- a/servers/visual/visual_server_scene.h +++ b/servers/visual/visual_server_scene.h @@ -118,6 +118,7 @@ public: virtual void erase(SpatialPartitionID p_handle) = 0; virtual void move(SpatialPartitionID p_handle, const AABB &p_aabb) = 0; 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; virtual int cull_convex(const Vector &p_convex, Instance **p_result_array, int p_result_max, uint32_t p_mask = 0xFFFFFFFF) = 0; virtual int cull_aabb(const AABB &p_aabb, Instance **p_result_array, int p_result_max, int *p_subindex_array = nullptr, uint32_t p_mask = 0xFFFFFFFF) = 0; @@ -164,6 +165,7 @@ public: void erase(SpatialPartitionID p_handle); void move(SpatialPartitionID p_handle, const AABB &p_aabb); void update(); + void update_collisions(); void set_pairable(SpatialPartitionID p_handle, bool p_pairable, uint32_t p_pairable_type, uint32_t p_pairable_mask); int cull_convex(const Vector &p_convex, Instance **p_result_array, int p_result_max, uint32_t p_mask = 0xFFFFFFFF); int cull_aabb(const AABB &p_aabb, Instance **p_result_array, int p_result_max, int *p_subindex_array = nullptr, uint32_t p_mask = 0xFFFFFFFF); @@ -552,6 +554,7 @@ public: void render_camera(RID p_camera, RID p_scenario, Size2 p_viewport_size, RID p_shadow_atlas); void render_camera(Ref &p_interface, ARVRInterface::Eyes p_eye, RID p_camera, RID p_scenario, Size2 p_viewport_size, RID p_shadow_atlas); void update_dirty_instances(); + void update_scenarios(); //probes struct GIProbeDataHeader {