Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add means for fixing navmap synchronization errors #87959

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

Scony
Copy link
Contributor

@Scony Scony commented Feb 4, 2024

Fixes #85548

This PR is based on my observation that for some valid input geometries, recast produces valid navigation meshes and yet the transformation that is done as part vertex hashing:

const int x = static_cast<int>(Math::floor(p_pos.x / cell_size));
const int y = static_cast<int>(Math::floor(p_pos.y / cell_height));
const int z = static_cast<int>(Math::floor(p_pos.z / cell_size));
is making those navigation meshes invalid. That is because some vertices are conflicting with others after transformation above and thus leading to synchronization errors mentioned in #85548

This PR proposes a map-level option to change the scale of internal rasterizer which allows the engine to perform the align-to-recast-grid transformation using greater precision. With that, in projects where navigation baking leads to synchronization problems, the precision of internal rasterizer can be increased to avoid errors (although potentially sacrificing the ability to connect separate regions).

I've prepared a straightforward demo (godot-navigation-demos.zip - ComplexNavmeshPaths.tscn) application that showcases the problem - all it does is query the server for paths from random point A to random point B each and render them to show potential holes in navigation.

The demo outcome when using current Godot implementation:
2024-02-11-153336_4480x1440_scrot

The demo outcome when the Godot from this PR is used and merge_rasterizer_cell_scale is set to 0.001:
2024-02-11-152839_4480x1440_scrot

@AThousandShips AThousandShips added this to the 4.x milestone Feb 5, 2024
@Scony Scony force-pushed the fix-navi-sync-errors branch 3 times, most recently from f3c2b64 to 0b95162 Compare February 11, 2024 15:13
@Scony Scony force-pushed the fix-navi-sync-errors branch from 0b95162 to f9126d3 Compare February 11, 2024 17:16
@Scony Scony marked this pull request as ready for review February 11, 2024 17:22
@Scony Scony requested review from a team as code owners February 11, 2024 17:22
@Scony Scony force-pushed the fix-navi-sync-errors branch from f9126d3 to 9ea8d4f Compare February 11, 2024 20:06
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Docs look good enough, I just cannot speak on the feature itself.

@Scony Scony requested a review from smix8 February 11, 2024 20:57
@smix8 smix8 modified the milestones: 4.x, 4.3 Feb 11, 2024
@akien-mga akien-mga merged commit 24bd307 into godotengine:master Feb 12, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

"Navigation map synchronization error" caused internally (by recast?)
5 participants