Fix closest edge and face check in NavigationServer3D.map_get_closest_point_to_segment

[oshman99]

If the line segment was not intersecting a face, Geometry3D::get_closest_points_between_segments was used. It calculated distance between the line segment and face's edges, instead of the distance between the line segment and face itself.
Since If the line segment is not itersecting a face, closest point to this face will be either beggining or end of this line segment, Face3::get_closest_point_to was used.

• Fixes NavigationServer3D.map_get_closest_point_to_segment only returns correct result if segment intersects nav mesh #93138
  

[smix8]

Thanks.

On a different note the NavigationServer3D documentation of that function should be improved to clarify what the collision parameter does.

[AThousandShips]

When you've fixed the comment please squash your commits into one, see here

And please also update the commit message to match the PR title to make it a bit clearer

[oshman99]

Sorry, my bad. Will follow pull-request guidelines properly next time.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉

[kleonc]

Since If the line segment is not itersecting a face, closest point to this face will be either beggining or end of this line segment,

This is incorrect statement (and hence this PR is incorrect as well). Counterexample: consider a face [(0, 0, 0), (1, 0, 0), (0, 1, 0)] and a segment [(0, 0, 1), (1, 2, 0)]. Then:

• Segment's endpoint (0, 0, 1) distance to the face is 1.0, closest on-face point is (0, 0, 0).
• Segment's endpoint (1, 2, 0) distance to the face is sqrt(2) (approx 1.414), closest on-face point is (0, 1, 0).
• Distance of on-segment point (5 / 11, 10 / 11, 6 / 11) to on-face point (3 / 11, 8 / 11, 0) (it's on face's edge) is 2 / sqrt(11) (approx 0.603).

[oshman99]

@kleonc you're right, completely missed this, thanks! I still would like to try to implement a proper fix with your correction in mind.
@AThousandShips what would be the correct way of handeling this? Should a bug be reopened and a new PR created?

[AThousandShips]

New PR yes, unsure about a report

[akien-mga]

A bug report could be good so we make sure to track this for the 4.3 release, and ensure that we can either merge a fix up PR, or revert this one if it's incorrect, to revisit later.

[kleonc]

I still would like to try to implement a proper fix with your correction in mind.

@oshman99 So any update on this / are you planning to open a PR? If you are not able to then please say so (in such case I'll address this myself). 🙂

[oshman99]

@kleonc yeah you can go ahead and address this. Wanted to tackle it today, but life got in the way. If you don't mind, can you tag me in your PR, when you create one?

[kleonc]

@kleonc yeah you can go ahead and address this. Wanted to tackle it today, but life got in the way. If you don't mind, can you tag me in your PR, when you create one?

@oshman99 Oh, I've asked kinda preemptively (seeing no update and just wanting to ensure it will be fixed for 4.3.stable). Aka haven't looked into this myself yet, maybe I'll do so tomorrow/Tuesday. So if you still want to open PR yourself feel free to do so (sorry if I made you not open a PR today etc.!); a simple fix ensuring correctness of the result would be fine for now (there's very likely a more optimal way to do the calculations as we're dealing with convex polygons / triangle fans and currently we're considering/checking each triangle separately).
And sure I'll tag you if I'll end up opening a PR for this.

[oshman99]

@oshman99 Oh, I've asked kinda preemptively (seeing no update and just wanting to ensure it will be fixed for 4.3.stable). Aka haven't looked into this myself yet, maybe I'll do so tomorrow/Tuesday. So if you still want to open PR yourself feel free to do so (sorry if I made you not open a PR today etc.!); a simple fix ensuring correctness of the result would be fine for now (there's very likely a more optimal way to do the calculations as we're dealing with convex polygons / triangle fans and currently we're considering/checking each triangle separately). And sure I'll tag you if I'll end up opening a PR for this.

@kleonc oh, hahaha, sorry for causing confusion then. There is MOST DEFINITELY a more optimal way of doing things. My first instinct was to basically add a third shortest distance check, that would compare the current shortest distance with the distance to the edge (basically, a copy-paste of the previous implementation). It works fine, but that's a pretty naive solution. I'll create a PR today and ping you and @smix8 . Feel free to comment on any issues or suggest better ways of doing this :)