From 31940c784a6c33de1c9e59004a8982b348ee3c36 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Sat, 24 Aug 2024 14:22:07 +0100 Subject: [PATCH] Safe `Camera::unproject_position()` `unproject_position()` can fail in some circumstances, and this needs to be conveyed to calling code. --- editor/spatial_editor_gizmos.cpp | 13 ++++-- scene/3d/camera.cpp | 68 ++++++++++++++++++++++++++++---- scene/3d/camera.h | 1 + 3 files changed, 71 insertions(+), 11 deletions(-) diff --git a/editor/spatial_editor_gizmos.cpp b/editor/spatial_editor_gizmos.cpp index 4546c0742cd..a4b7f25d435 100644 --- a/editor/spatial_editor_gizmos.cpp +++ b/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); - s[1] = p_camera->unproject_position(b); + if (!p_camera->safe_unproject_position(a, s[0])) { + continue; + } + if (!p_camera->safe_unproject_position(b, s[1])) { + continue; + } Vector2 p = Geometry::get_closest_point_to_segment_2d(p_point, s); diff --git a/scene/3d/camera.cpp b/scene/3d/camera.cpp index e85b1eb50be..218e3dcb3be 100644 --- a/scene/3d/camera.cpp +++ b/scene/3d/camera.cpp @@ -465,8 +465,8 @@ Vector Camera::get_near_plane_points() const { return points; } -Point2 Camera::unproject_position(const Vector3 &p_pos) const { - ERR_FAIL_COND_V_MSG(!is_inside_tree(), Vector2(), "Camera is not inside scene."); +bool Camera::safe_unproject_position(const Vector3 &p_pos, Point2 &r_result) const { + 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. - // TODO : Investigate, this was causing Nans. - ERR_FAIL_COND_V(p.d == 0, Point2()); + // If p.d is zero, there is a potential divide by zero ahead. + // This can occur if the test point is exactly on the focal plane + // 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; - res.x = (p.normal.x * 0.5 + 0.5) * viewport_size.x; - res.y = (-p.normal.y * 0.5 + 0.5) * viewport_size.y; + r_result.x = (p.normal.x * 0.5 + 0.5) * viewport_size.x; + r_result.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; } diff --git a/scene/3d/camera.h b/scene/3d/camera.h index e090185eeae..afa95b046bb 100644 --- a/scene/3d/camera.h +++ b/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;