Merge pull request #87713 from lawnjelly/portal_include_in_bound

[3.x] Portals - include in bound and special cases in start room

doc/classes/Portal.xml

@@ -23,6 +23,9 @@
 		</method>
 	</methods>
 	<members>
+		<member name="include_in_bound" type="bool" setter="set_include_in_bound" getter="get_include_in_bound" default="false">
+			When a manual bound has not been explicitly specified for a [Room], the convex hull bound will be estimated from the geometry of the objects within the room. This setting determines whether the portal geometry is included in this estimate of the room bound.
+		</member>
 		<member name="linked_room" type="NodePath" setter="set_linked_room" getter="get_linked_room" default="NodePath(&quot;&quot;)">
 			This is a shortcut for setting the linked [Room] in the name of the [Portal] (the name is used during conversion).
 		</member>

scene/3d/portal.cpp

@@ -46,6 +46,7 @@ Portal::Portal() {
 
 	_settings_active = true;
 	_settings_two_way = true;
+	_settings_include_in_bound = false;
 	_internal = false;
 	_linkedroom_ID[0] = -1;
 	_linkedroom_ID[1] = -1;
@@ -696,8 +697,12 @@ void Portal::_bind_methods() {
 
 	ClassDB::bind_method(D_METHOD("set_point", "index", "position"), &Portal::set_point);
 
+	ClassDB::bind_method(D_METHOD("set_include_in_bound", "p_enabled"), &Portal::set_include_in_bound);
+	ClassDB::bind_method(D_METHOD("get_include_in_bound"), &Portal::get_include_in_bound);
+
 	ADD_PROPERTY(PropertyInfo(Variant::BOOL, "portal_active"), "set_portal_active", "get_portal_active");
 	ADD_PROPERTY(PropertyInfo(Variant::BOOL, "two_way"), "set_two_way", "is_two_way");
+	ADD_PROPERTY(PropertyInfo(Variant::BOOL, "include_in_bound"), "set_include_in_bound", "get_include_in_bound");
 	ADD_PROPERTY(PropertyInfo(Variant::NODE_PATH, "linked_room", PROPERTY_HINT_NODE_PATH_VALID_TYPES, "Room"), "set_linked_room", "get_linked_room");
 	ADD_PROPERTY(PropertyInfo(Variant::BOOL, "use_default_margin"), "set_use_default_margin", "get_use_default_margin");
 	ADD_PROPERTY(PropertyInfo(Variant::REAL, "portal_margin", PROPERTY_HINT_RANGE, "0.0,10.0,0.01"), "set_portal_margin", "get_portal_margin");

scene/3d/portal.h

@@ -87,6 +87,9 @@ public:
 	// primarily for the gizmo
 	void set_point(int p_idx, const Vector2 &p_point);
 
+	void set_include_in_bound(bool p_enabled) { _settings_include_in_bound = p_enabled; }
+	bool get_include_in_bound() const { return _settings_include_in_bound; }
+
 	String get_configuration_warning() const;
 
 	Portal();

scene/3d/room_manager.cpp

@@ -1538,6 +1538,11 @@ bool RoomManager::_convert_room_hull_final(Room *p_room, const LocalVector<Porta
 		int portal_id = p_room->_portals[n];
 		Portal *portal = p_portals[portal_id];
 
+		// User can select whether to include in bound.
+		if (!portal->get_include_in_bound()) {
+			continue;
+		}
+
 		// don't add portals to the world bound that are internal to this room!
 		if (portal->is_portal_internal(p_room->_room_ID)) {
 			continue;

servers/visual/portals/portal_tracer.cpp

@@ -130,6 +130,7 @@ void PortalTracer::trace(PortalRenderer &p_portal_renderer, const Vector3 &p_pos
 
 		TraceParams params;
 		params.use_pvs = p_portal_renderer.get_pvs().is_loaded();
+		params.start_room_id = p_start_room_id;
 
 		// create bitfield
 		if (params.use_pvs) {
@@ -345,16 +346,16 @@ 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, int p_from_external_room_id) {
-	// prevent too much depth
+	// Prevent too much depth.
 	if (p_depth > _depth_limit) {
 		WARN_PRINT_ONCE("Portal Depth Limit reached (seeing through too many portals)");
 		return;
 	}
 
-	// get the room
+	// Get the room.
 	const VSRoom &room = _portal_renderer->get_room(p_room_id);
 
-	// set up the occlusion culler as a one off
+	// Set up the occlusion culler as a one off.
 	_occlusion_culler.prepare(*_portal_renderer, room, _trace_start_point, p_planes, &_near_and_far_planes[0]);
 
 	cull_statics(room, p_planes);
@@ -365,13 +366,13 @@ void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int
 	for (int p = 0; p < num_portals; p++) {
 		const VSPortal &portal = _portal_renderer->get_portal(room._portal_ids[p]);
 
-		// portals can be switched on and off at runtime, like opening and closing a door
+		// Portals can be switched on and off at runtime, like opening and closing a door.
 		if (!portal._active) {
 			continue;
 		}
 
-		// everything depends on whether the portal is incoming or outgoing.
+		// Everything depends on whether the portal is incoming or outgoing.
-		// if incoming we reverse the logic.
+		// If incoming we reverse the logic.
 		int outgoing = 1;
 
 		int room_a_id = portal._linkedroom_ID[0];
@@ -380,67 +381,83 @@ void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int
 			DEV_ASSERT(portal._linkedroom_ID[1] == p_room_id);
 		}
 
-		// trace through this portal to the next room
+		// Trace through this portal to the next room.
 		int linked_room_id = portal._linkedroom_ID[outgoing];
 
-		// cull by PVS
+		// Cull by PVS.
 		if (p_params.use_pvs && (!p_params.decompressed_room_pvs[linked_room_id])) {
 			continue;
 		}
 
-		// cull by portal angle to camera.
+		// Cull by portal angle to camera.
 
-		// much better way of culling portals by direction to camera...
+		// Much better way of culling portals by direction to camera...
 		// instead of using dot product with a varying view direction, we simply find which side of the portal
-		// plane the camera is on! If it is behind, the portal can be seen through, if in front, it can't
+		// plane the camera is on! If it is behind, the portal can be seen through, if in front, it can't.
+
+		// There is one exception to this, for the first source room. If we are in front of any portal in the first
+		// source room, we will render EVERYTHING through it into the next room. This can happen due
+		// to precision errors, or inaccuracy in setting up the portal planes relative to the room bounds -
+		// in which case we can end up IN FRONT of a portal in the same room.
+
+		bool start_room_in_front_portal_exception = false;
+
+		/////////////////////////////////////////////
 		real_t dist_cam = portal._plane.distance_to(_trace_start_point);
 
 		if (!outgoing) {
 			dist_cam = -dist_cam;
 		}
 
+		// If the camera is IN FRONT of the portal plane...
 		if (dist_cam >= 0.0) {
-			continue;
+			if ((p_room_id != p_params.start_room_id) || portal._internal) {
+				continue;
+			} else {
+				start_room_in_front_portal_exception = true;
+			}
 		}
+		/////////////////////////////////////////////
 
-		// is it culled by the planes?
+		// While clipping to the planes we maintain a list of partial planes, so we can add them to the
-		VSPortal::ClipResult overall_res = VSPortal::ClipResult::CLIP_INSIDE;
+		// recursive next iteration of planes to check.
-
-		// while clipping to the planes we maintain a list of partial planes, so we can add them to the
-		// recursive next iteration of planes to check
 		static LocalVector<int> partial_planes;
 		partial_planes.clear();
 
-		// for portals, we want to ignore the near clipping plane, as we might be right on the edge of a doorway
+		// Is it culled by the planes?
-		// and still want to look through the portal.
+		VSPortal::ClipResult overall_res = VSPortal::ClipResult::CLIP_INSIDE;
-		// so earlier we have set it that the first plane (ASSUMING that plane zero is the near clipping plane)
+		if (!start_room_in_front_portal_exception) {
-		// starts from the camera position, and NOT the actual near clipping plane.
+			// For portals, we want to ignore the near clipping plane, as we might be right on the edge of a doorway
-		// if we need quite a distant near plane, we may need a different strategy.
+			// and still want to look through the portal.
-		for (uint32_t l = 0; l < p_planes.size(); l++) {
+			// So earlier we have set it that the first plane (ASSUMING that plane zero is the near clipping plane)
-			VSPortal::ClipResult res = portal.clip_with_plane(p_planes[l]);
+			// starts from the camera position, and NOT the actual near clipping plane.
+			// If we need quite a distant near plane, we may need a different strategy.
+			for (uint32_t l = 0; l < p_planes.size(); l++) {
+				VSPortal::ClipResult res = portal.clip_with_plane(p_planes[l]);
 
-			switch (res) {
+				switch (res) {
-				case VSPortal::ClipResult::CLIP_OUTSIDE: {
+					case VSPortal::ClipResult::CLIP_OUTSIDE: {
-					overall_res = res;
+						overall_res = res;
-				} break;
+					} break;
-				case VSPortal::ClipResult::CLIP_PARTIAL: {
+					case VSPortal::ClipResult::CLIP_PARTIAL: {
-					// if the portal intersects one of the planes, we should take this plane into account
+						// If the portal intersects one of the planes, we should take this plane into account
-					// in the next call of this recursive trace, because it can be used to cull out more objects
+						// in the next call of this recursive trace, because it can be used to cull out more objects.
-					overall_res = res;
+						overall_res = res;
-					partial_planes.push_back(l);
+						partial_planes.push_back(l);
-				} break;
+					} break;
-				default: // suppress warning
+					default: // Suppress warning.
+						break;
+				}
+
+				// If the portal was totally outside the 'frustum' then we can ignore it.
+				if (overall_res == VSPortal::ClipResult::CLIP_OUTSIDE)
 					break;
 			}
 
-			// if the portal was totally outside the 'frustum' then we can ignore it
+			// This portal is culled.
-			if (overall_res == VSPortal::ClipResult::CLIP_OUTSIDE)
+			if (overall_res == VSPortal::ClipResult::CLIP_OUTSIDE) {
-				break;
+				continue;
-		}
+			}
-
-		// this portal is culled
-		if (overall_res == VSPortal::ClipResult::CLIP_OUTSIDE) {
-			continue;
 		}
 
 		// Don't allow portals from internal to external room to be followed
@@ -467,64 +484,70 @@ void PortalTracer::trace_recursive(const TraceParams &p_params, int p_depth, int
 			}
 		}
 
-		// occlusion culling of portals
+		// Occlusion culling of portals.
 		if (_occlusion_culler.cull_sphere(portal._pt_center, portal._bounding_sphere_radius)) {
 			continue;
 		}
 
-		// hopefully the portal actually leads somewhere...
+		// Hopefully the portal actually leads somewhere...
 		if (linked_room_id != -1) {
-			// we need some new planes
+			// We need some new planes.
 			unsigned int pool_mem = _planes_pool.request();
 
-			// if the planes pool is not empty, we got some planes, and can recurse
+			// If the planes pool is not empty, we got some planes, and can recurse.
 			if (pool_mem != (unsigned int)-1) {
-				// get a new vector of planes from the pool
+				// Get a new vector of planes from the pool.
 				LocalVector<Plane> &new_planes = _planes_pool.get(pool_mem);
 
-				// makes sure there are none left over (as the pool may not clear them)
+				// Makes sure there are none left over (as the pool may not clear them).
 				new_planes.clear();
 
-				// if portal is totally inside the planes, don't copy the old planes ..
+				if (!start_room_in_front_portal_exception) {
-				// i.e. we can now cull using the portal and forget about the rest of the frustum (yay)
+					// If portal is totally inside the planes, don't copy the old planes ..
-				// note that this loses the far clipping plane .. but that shouldn't be important usually?
+					// i.e. we can now cull using the portal and forget about the rest of the frustum (yay).
-				// (maybe we might need to account for this in future .. look for issues)
+					// Note that this loses the far clipping plane .. but that shouldn't be important usually?
-				if (overall_res != VSPortal::ClipResult::CLIP_INSIDE) {
+					// (maybe we might need to account for this in future .. look for issues)
-					// if it WASN'T totally inside the existing frustum, we also need to add any existing planes
+					if (overall_res != VSPortal::ClipResult::CLIP_INSIDE) {
-					// that cut the portal.
+						// If it WASN'T totally inside the existing frustum, we also need to add any existing planes
-					for (uint32_t n = 0; n < partial_planes.size(); n++) {
+						// that cut the portal.
-						new_planes.push_back(p_planes[partial_planes[n]]);
+						for (uint32_t n = 0; n < partial_planes.size(); n++) {
+							new_planes.push_back(p_planes[partial_planes[n]]);
+						}
 					}
+
+					// We will always add the portals planes. This could probably be optimized, as some
+					// portal planes may be culled out by partial planes... NYI
+					portal.add_planes(_trace_start_point, new_planes, outgoing != 0);
+
+					// Always add the far plane. It is likely the portal is inside the far plane,
+					// but it is still needed in future for culling portals and objects.
+					// Note that there is a small possibility of far plane being added twice here
+					// in some situations, but I don't think it should be a problem.
+					// The fake near plane BTW is almost never added (otherwise it would prematurely
+					// break traversal through the portals), so near clipping must be done
+					// explicitly on objects.
+					new_planes.push_back(_near_and_far_planes[1]);
+				} else {
+					// start_room_in_front_portal_exception
+					// Copy the existing planes and reuse when tracing into the next room.
+					new_planes = p_planes;
 				}
 
-				// we will always add the portals planes. This could probably be optimized, as some
+				// Go and do the whole lot again in the next room...
-				// portal planes may be culled out by partial planes... NYI
-				portal.add_planes(_trace_start_point, new_planes, outgoing != 0);
-
-				// always add the far plane. It is likely the portal is inside the far plane,
-				// but it is still needed in future for culling portals and objects.
-				// note that there is a small possibility of far plane being added twice here
-				// in some situations, but I don't think it should be a problem.
-				// The fake near plane BTW is almost never added (otherwise it would prematurely
-				// break traversal through the portals), so near clipping must be done
-				// explicitly on objects.
-				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, p_from_external_room_id);
 
-				// no longer need these planes, return them to the pool
+				// We no longer need these planes, return them to the pool.
 				_planes_pool.free(pool_mem);
 
 			} // pool mem allocated
 			else {
-				// planes pool is empty!
+				// Planes pool is empty!
-				// This will happen if the view goes through shedloads of portals
+				// This will happen if the view goes through shedloads of portals.
 				// The solution is either to increase the plane pool size, or not build levels
 				// with views through multiple portals. Looking through multiple portals is likely to be
 				// slow anyway because of the number of planes to test.
 				WARN_PRINT_ONCE("planes pool is empty");
-				// note we also have a depth check at the top of this function. Which will probably get hit
+				// Note we also have a depth check at the top of this function. Which will probably get hit
 				// before the pool gets empty.
 			}
 

servers/visual/portals/portal_tracer.h

@@ -66,6 +66,7 @@ public:
 	};
 
 	struct TraceParams {
+		int start_room_id;
 		bool use_pvs;
 		uint8_t *decompressed_room_pvs;
 	};