From 2c88517a7baa623ef20e3d95df824b3cc47101cd Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Thu, 5 Aug 2021 12:20:08 +0100 Subject: [PATCH] Portals - fix recursive loop looking out from internal rooms In some situations looking out from an internal room it was possible to look back into the portal into the internal room. This PR fixes this by keeping a single item 'stack' record of the last external room, and preventing recursing into this room. This also makes tracing significantly more efficient out of internal rooms, as there is no need to trace the external room multiple times. --- servers/visual/portals/portal_pvs_builder.cpp | 9 +++++- servers/visual/portals/portal_pvs_builder.h | 3 +- servers/visual/portals/portal_renderer.cpp | 2 +- servers/visual/portals/portal_tracer.cpp | 28 +++++++++++++++++-- servers/visual/portals/portal_tracer.h | 3 +- 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/servers/visual/portals/portal_pvs_builder.cpp b/servers/visual/portals/portal_pvs_builder.cpp index a8b89614b1c..24030da2a69 100644 --- a/servers/visual/portals/portal_pvs_builder.cpp +++ b/servers/visual/portals/portal_pvs_builder.cpp @@ -233,9 +233,10 @@ void PVSBuilder::save_pvs(String p_filename) { #endif -void PVSBuilder::calculate_pvs(PortalRenderer &p_portal_renderer, String p_filename) { +void PVSBuilder::calculate_pvs(PortalRenderer &p_portal_renderer, String p_filename, int p_depth_limit) { _portal_renderer = &p_portal_renderer; _pvs = &p_portal_renderer.get_pvs(); + _depth_limit = p_depth_limit; // attempt to load from file rather than create each time #ifdef GODOT_PVS_SUPPORT_SAVE_FILE @@ -330,6 +331,12 @@ void PVSBuilder::trace_rooms_recursive(int p_depth, int p_source_room_id, int p_ return; } + // prevent too much depth + if (p_depth > _depth_limit) { + WARN_PRINT_ONCE("Portal Depth Limit reached (seeing through too many portals)"); + return; + } + logd(p_depth, "trace_rooms_recursive room " + itos(p_room_id)); // get the room diff --git a/servers/visual/portals/portal_pvs_builder.h b/servers/visual/portals/portal_pvs_builder.h index 05050cbc28f..d93615102b8 100644 --- a/servers/visual/portals/portal_pvs_builder.h +++ b/servers/visual/portals/portal_pvs_builder.h @@ -46,7 +46,7 @@ class PVSBuilder { }; public: - void calculate_pvs(PortalRenderer &p_portal_renderer, String p_filename); + void calculate_pvs(PortalRenderer &p_portal_renderer, String p_filename, int p_depth_limit); private: #ifdef GODOT_PVS_SUPPORT_SAVE_FILE @@ -64,6 +64,7 @@ private: PortalRenderer *_portal_renderer = nullptr; PVS *_pvs = nullptr; + int _depth_limit = 16; static bool _log_active; }; diff --git a/servers/visual/portals/portal_renderer.cpp b/servers/visual/portals/portal_renderer.cpp index fcf4a983502..22111805b83 100644 --- a/servers/visual/portals/portal_renderer.cpp +++ b/servers/visual/portals/portal_renderer.cpp @@ -684,7 +684,7 @@ void PortalRenderer::rooms_finalize(bool p_generate_pvs, bool p_cull_using_pvs, // calculate PVS if (p_generate_pvs) { PVSBuilder pvs; - pvs.calculate_pvs(*this, p_pvs_filename); + pvs.calculate_pvs(*this, p_pvs_filename, _tracer.get_depth_limit()); _cull_using_pvs = p_cull_using_pvs; // hard code to on for test } else { _cull_using_pvs = false; diff --git a/servers/visual/portals/portal_tracer.cpp b/servers/visual/portals/portal_tracer.cpp index 18a76d20b1a..f00dfb0a54b 100644 --- a/servers/visual/portals/portal_tracer.cpp +++ b/servers/visual/portals/portal_tracer.cpp @@ -338,7 +338,7 @@ void PortalTracer::trace_pvs(int p_source_room_id, const LocalVector &p_p } } -void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int p_room_id, const LocalVector &p_planes) { +void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int p_room_id, const LocalVector &p_planes, int p_from_external_room_id) { // prevent too much depth if (p_depth > _depth_limit) { WARN_PRINT_ONCE("Portal Depth Limit reached (seeing through too many portals)"); @@ -434,6 +434,30 @@ void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int continue; } + // Don't allow portals from internal to external room to be followed + // if the external room has already been processed in this trace stack. This prevents + // unneeded processing, and also prevents recursive feedback where you + // see into internal room -> external room and back into the same internal room + // via the same portal. + if (portal._internal && (linked_room_id != -1)) { + if (outgoing) { + if (linked_room_id == p_from_external_room_id) { + continue; + } + } else { + // We are entering an internal portal from an external room. + // set the external room id, so we can recognise this when we are + // later exiting the internal rooms. + // Note that as we can only store 1 previous external room, this system + // won't work completely correctly when you have 2 levels of internal room + // and you can see from roomgroup a -> b -> c. However this should just result + // in a little slower culling for that particular view, and hopefully will not break + // with recursive loop looking through the same portal multiple times. (don't think this + // is possible in this scenario). + p_from_external_room_id = p_room_id; + } + } + // hopefully the portal actually leads somewhere... if (linked_room_id != -1) { // we need some new planes @@ -473,7 +497,7 @@ void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int new_planes.push_back(_near_and_far_planes[1]); // go and do the whole lot again in the next room - trace_recursive(p_params, p_depth + 1, linked_room_id, new_planes); + trace_recursive(p_params, p_depth + 1, linked_room_id, new_planes, p_from_external_room_id); // no longer need these planes, return them to the pool _planes_pool.free(pool_mem); diff --git a/servers/visual/portals/portal_tracer.h b/servers/visual/portals/portal_tracer.h index 12861a93211..5670606ec75 100644 --- a/servers/visual/portals/portal_tracer.h +++ b/servers/visual/portals/portal_tracer.h @@ -108,10 +108,11 @@ public: int trace_globals(const LocalVector &p_planes, VSInstance **p_result_array, int first_result, int p_result_max, uint32_t p_mask, bool p_override_camera); void set_depth_limit(int p_limit) { _depth_limit = p_limit; } + int get_depth_limit() const { return _depth_limit; } private: // main tracing function is recursive - void trace_recursive(const TraceParams &p_params, int p_depth, int p_room_id, const LocalVector &p_planes); + void trace_recursive(const TraceParams &p_params, int p_depth, int p_room_id, const LocalVector &p_planes, int p_from_external_room_id = -1); // use pvs to cull instead of dynamically using portals // this is a faster trace but less accurate. Only possible if PVS has been generated.