Merge pull request #51274 from lawnjelly/portals_recursive_loop

Portals - fix recursive loop looking out from internal rooms

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

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;
 };

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;

servers/visual/portals/portal_tracer.cpp

@@ -338,7 +338,7 @@ void PortalTracer::trace_pvs(int p_source_room_id, const LocalVector<Plane> &p_p
 	}
 }
 
-void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int p_room_id, const LocalVector<Plane> &p_planes) {
+void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int p_room_id, const LocalVector<Plane> &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);

servers/visual/portals/portal_tracer.h

@@ -108,10 +108,11 @@ public:
 	int trace_globals(const LocalVector<Plane> &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<Plane> &p_planes);
+	void trace_recursive(const TraceParams &p_params, int p_depth, int p_room_id, const LocalVector<Plane> &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.