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

@@ -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.