Merge pull request #96032 from lawnjelly/safe_unproject

[3.x] Safe `Camera::unproject_position()`

editor/spatial_editor_gizmos.cpp

@@ -586,7 +586,10 @@ bool EditorSpatialGizmo::intersect_ray(Camera *p_camera, const Point2 &p_point,
 
 		for (int i = 0; i < handles.size(); i++) {
 			Vector3 hpos = t.xform(handles[i]);
-			Vector2 p = p_camera->unproject_position(hpos);
+			Vector2 p;
+			if (!p_camera->safe_unproject_position(hpos, p)) {
+				continue;
+			}
 
 			if (p.distance_to(p_point) < HANDLE_HALF_SIZE) {
 				real_t dp = p_camera->get_transform().origin.distance_to(hpos);
@@ -665,8 +668,12 @@ bool EditorSpatialGizmo::intersect_ray(Camera *p_camera, const Point2 &p_point,
 			Vector3 a = t.xform(vptr[i * 2 + 0]);
 			Vector3 b = t.xform(vptr[i * 2 + 1]);
 			Vector2 s[2];
-			s[0] = p_camera->unproject_position(a);
+			if (!p_camera->safe_unproject_position(a, s[0])) {
-			s[1] = p_camera->unproject_position(b);
+				continue;
+			}
+			if (!p_camera->safe_unproject_position(b, s[1])) {
+				continue;
+			}
 
 			Vector2 p = Geometry::get_closest_point_to_segment_2d(p_point, s);
 

scene/3d/camera.cpp

@@ -465,8 +465,8 @@ Vector<Vector3> Camera::get_near_plane_points() const {
 	return points;
 }
 
-Point2 Camera::unproject_position(const Vector3 &p_pos) const {
+bool Camera::safe_unproject_position(const Vector3 &p_pos, Point2 &r_result) const {
-	ERR_FAIL_COND_V_MSG(!is_inside_tree(), Vector2(), "Camera is not inside scene.");
+	ERR_FAIL_COND_V_MSG(!is_inside_tree(), false, "Camera is not inside scene.");
 
 	Size2 viewport_size = get_viewport()->get_visible_rect().size;
 
@@ -478,19 +478,71 @@ Point2 Camera::unproject_position(const Vector3 &p_pos) const {
 		cm.set_perspective(fov, viewport_size.aspect(), near, far, keep_aspect == KEEP_WIDTH);
 	}
 
+	// These are homogeneous coordinates, as Godot 3 has no Vector4.
+	// The 1.0 will later become w, the perspective divide.
 	Plane p(get_camera_transform().xform_inv(p_pos), 1.0);
 
 	p = cm.xform4(p);
 
-	// Prevent divide by zero.
+	// If p.d is zero, there is a potential divide by zero ahead.
-	// TODO : Investigate, this was causing Nans.
+	// This can occur if the test point is exactly on the focal plane
-	ERR_FAIL_COND_V(p.d == 0, Point2());
+	// with a perspective camera matrix (i.e. behind the near plane).
+
+	// There are two possibilities here:
+	// Either the test point is exactly at the origin, in which case the unprojected
+	// point should theoretically be the center of the viewport, OR
+	// infinity distance from the center of the viewport.
+
+	// We should also handle the case where the test point is CLOSE
+	// to the focal plane.
+	// This can cause returned unprojected results near infinity.
+	// The epsilon chosen here must be small, but still allow for near planes quite close to zero.
+
+	// Here we return false and let the calling routine handle this error condition.
+	if (Math::absf(p.d) < CMP_EPSILON) {
+		// Bodge some kind of result at infinity from the viewport center.
+		r_result = Point2();
+
+		// The viewport size here is irrelevant, we just want a high number
+		// (representing infinity) but not actually close to infinity to prevent
+		// knock on bugs if later maths later does something with these values.
+		// Suffice is for them to be WAY off the main viewport.
+		const float SOME_HIGH_VALUE = 100000.0f;
+		if (p.normal.x > 0) {
+			r_result.x = SOME_HIGH_VALUE;
+		} else if (p.normal.x < 0) {
+			r_result.x = -SOME_HIGH_VALUE;
+		}
+		if (p.normal.y > 0) {
+			r_result.y = SOME_HIGH_VALUE;
+		} else if (p.normal.y < 0) {
+			r_result.y = -SOME_HIGH_VALUE;
+		}
+
+		return false;
+	}
 	p.normal /= p.d;
 
-	Point2 res;
+	r_result.x = (p.normal.x * 0.5 + 0.5) * viewport_size.x;
-	res.x = (p.normal.x * 0.5 + 0.5) * viewport_size.x;
+	r_result.y = (-p.normal.y * 0.5 + 0.5) * viewport_size.y;
-	res.y = (-p.normal.y * 0.5 + 0.5) * viewport_size.y;
 
+	return true;
+}
+
+Point2 Camera::unproject_position(const Vector3 &p_pos) const {
+	ERR_FAIL_COND_V_MSG(!is_inside_tree(), Point2(), "Camera is not inside scene.");
+
+	Point2 res;
+
+	// Unproject can fail if the test point is on the camera matrix focal plane
+	// with a perspective transform.
+	// In this case, the unprojected point is potentially at infinity from the viewport
+	// center.
+	if (!safe_unproject_position(p_pos, res)) {
+#ifdef DEV_ENABLED
+		WARN_PRINT_ONCE("Camera::unproject_position() unprojecting points on the focal plane is unreliable.");
+#endif
+	}
 	return res;
 }
 

scene/3d/camera.h

@@ -173,6 +173,7 @@ public:
 	virtual Vector3 project_ray_origin(const Point2 &p_pos) const;
 	virtual Vector3 project_local_ray_normal(const Point2 &p_pos) const;
 	virtual Point2 unproject_position(const Vector3 &p_pos) const;
+	bool safe_unproject_position(const Vector3 &p_pos, Point2 &r_result) const;
 	bool is_position_behind(const Vector3 &p_pos) const;
 	virtual Vector3 project_position(const Point2 &p_point, float p_z_depth) const;