X Tutup
Skip to content

[Navigation 2D] Fix sign of cross product#110815

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
AThousandShips:fix_nav_2d
Sep 23, 2025
Merged

[Navigation 2D] Fix sign of cross product#110815
Repiteo merged 1 commit intogodotengine:masterfrom
AThousandShips:fix_nav_2d

Conversation

@AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Sep 23, 2025

Regression from splitting the servers. The original code effectively took the cross product using Vector3(v0.x, 0, v0.y).cross(Vector3(v1.x, 0, v1.y)).y, but this is not equivalent to the 2D cross product which is equivalent to Vector3(v0.x, v0.y, 0).cross(v1.x, v1.y, 0).z.

The former gives v1.x * v0.y - v0.x * v1.y, but the latter gives v0.x * v1.y - v1.x * v0.y.

Fixed by negating the result in all places, verified against that the original 3D code is the plain cross product.

Also replaced the area formula, using abs instead which was an oversight in the original code.

Added minimal tests for the area formula and also for the closest point method, the latter can be removed if not necessary but wanted to make sure the area formula is correct, verified the area with conventional methods.

Minimal project from smix8 (note that due to #110807 you will need to open the main scene manually, as saving the main scene currently breaks things):
godot-4.5-path-corridorfunnel-bug.zip

Click in the red circle to confirm that the path no longer goes the wrong way

Related (but does not fix the entire issue):

Regression from splitting the servers. Also replaces the method for
getting the triangle area.
@AThousandShips AThousandShips added this to the 4.6 milestone Sep 23, 2025
@AThousandShips AThousandShips requested review from a team as code owners September 23, 2025 13:01
@AThousandShips AThousandShips added bug regression topic:navigation topic:2d cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release labels Sep 23, 2025
Copy link
Contributor

@smix8 smix8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm that this fixes the bug in the 2d corridor funnel path postprocessing. In a few tests I did this now yields the identical paths with all post processing options in both 2d and 3d when using the same navmesh.

(NavigationPolygon.get_navigation_mesh() can be used to convert any 2D navmesh to 3D)

@AThousandShips
Copy link
Member Author

(Updated the MRP to get around #110807)

@Repiteo Repiteo merged commit b7c5fca into godotengine:master Sep 23, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Sep 23, 2025

Thanks!

@AThousandShips AThousandShips deleted the fix_nav_2d branch September 23, 2025 21:41
@AThousandShips
Copy link
Member Author

Thank you!

@Repiteo
Copy link
Contributor

Repiteo commented Sep 30, 2025

Cherry-picked to 4.5

@Repiteo Repiteo removed the cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release label Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

X Tutup