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

Rework NavigationMeshGenerator #70724

Closed

Conversation

smix8
Copy link
Contributor

@smix8 smix8 commented Dec 30, 2022

Supersedes #64682 and #70174

For context, the NavigationMeshGenerator singleton is responsible for navigation mesh baking and source geometry parsing from the SceneTree. SourceGeometryData is navigation mesh baking relevant data parsed from SceneTree nodes, e.g. meshes or collision objects.

What is new?

  • Adds 2D navigation mesh baking.
  • Adds server support for multi-threaded navigation mesh baking.
  • Adds addon and module support for navigation mesh baking.
  • Adds SourceGeometryParsers to customize collection of source geometry data for baking.
  • Adds SourceGeometryData to cache and reuse existing source geometry data for baking.
  • Reworks NavigationMeshGenerator.
  • Reworks NavigationPolygon and NavigationMesh.
  • Reworks NavigationRegion2D and NavigationRegion3D.

Fixed Issues

Implemented proposals

New 2D navigation mesh baking

The navigation mesh baking for 2D is accessible through NavigationRegion2D.

The NavigationRegion2D has new buttons in the Editor for baking or clearing the current NavigationPolygon.
The NavigationRegion2D also has a new function bake_navigation_polygon() to bake the navigation polygon with scripts.

navigationregion2dbakemenu

The 2D navigation mesh baking workflow is in general kept very similar to 3D but there are few key differences.

Collision shapes are always considered obstructions in 2D. Since a 2D canvas is just a flat, infinite surface on the xy axis 2D baking requires a fillrule for the polygons as well as at least one outline that defines the bounding area that should be baked.

The fillrule controls how inner outlines are understood by the baking e.g. if they are turned into holes. The most common fillrules are EvenOdd or NonZero.

  • EvenOdd is closest to the old NavigationPolygon outline behavior minus all the old polygon overlap errors. This fillrule turns inner outlines that are even numbered into obstacles (holes) and odd numbered into traversable surfaces.
  • NonZero allows to draw outlines more or less unconcerned about overlap issues. All outlines are merged and turned into a traversable surface.

fillrules

The other fillrules Positive or Negative should be rarely considered. They work with winding order of outlines and are only relevant in some very niche uses when creating polygons procedural.

navigationpolygonbakesettings

Another, more advanced way to bake navigation meshes for 2D is available for scripts when using the NavigationMeshGenerator singleton directly.

The NavigationMeshGenerator has dedicated 2D functions for parsing, baking or doing both at once.

  • parse_and_bake_2d() can be used to parse source geometry and bake a NavigationPolygon in one go.
  • parse_2d_source_geometry_data() can be used to only parse source geometry to a reuseable and serializable NavigationMeshSourceGeometryData2D resource without the baking step (see description below).
  • bake_2d_from_source_geometry_data() can be used to bake a NavigationPolygon from already parsed data e.g. to avoid runtime performance issues with (redundant) parsing.

The source geometry parsing is implemented with dedicated parsers for each supported node type. Currently the following node parsers are added:

  • StaticBody2D and all CollisionShape2D
  • Polygon2D
  • TileMap
  • MeshInstance2D (only 2D meshes that use ARRAY_FLAG_USE_2D_VERTICES)
  • MultiMesh2D (not working placeholder as it currently uses not supported mesh formats )

TileMap navigation mesh baking

A NavigationRegion2D can bake a navigation mesh fo the entire TileMap by merging the navigation polygons and collision polygons from all used TileMap cells on the first Tilemap layer.

217048075-9cd9c4c5-5dfa-4c76-bf86-ea64331caeca

The TileMap is a special case for 2D navigation mesh baking as it is a node that combines internally both traversable surfaces as well as obstructions.

  1. TileMaps when baked parse all their used cells with a navigation polygon as traversable surface and merge them together.
  2. TileMaps then add all their used cells with collision polygons as obstruction surfaces and merge them together.
  3. The resulting difference between the traversable and obstruction surfaces is then shrunk by agent size.

This full TileMap cell merge is required as it is the only reliably way to get working navigation meshes that are fitted for an agent size. It yields the best possible result for both navigation performance and quality by removing all those little TileMap cell edge seams that can cause problems and cost a lot of performance with both pathfinding and navigation map updates.

NavigationGeometryParsers

A NavigationGeometryParser is the magic ingredient that turns information from a Node into something useful for navigation mesh baking.

2D and 3D version of the geometry parsers are available as NavigationGeometryParser2D and NavigationGeometryParser3D respectively.

NavigationGeometryParser classes make it possibility for modules and addons to extend the navigation mesh baking by contributing their own source geometry.

Godot by default already provides parsers for all commonly used nodes in navigation mesh baking. Addons, modules or other customizations can extended the NavigationGeometryParsers class and registered on the NavigationMeshGenerator singleton. This allows them to contribute scene geometry from custom nodes to the navigation mesh baking process.

Modules can use the parses_node() and parse_geometry() directly while scripts can override the underscore _parses_node() and _parse_geometry() functions. Since navigation mesh baking requires specific mesh and polygon data helper functions are available to facilitate the conversions.

extends NavigationGeometryParser2D
class_name CustomNavigationGeometryParser2D

func _init():
    var custom_parser : NavigationGeometryParser2D = CustomNavigationGeometryParser2D.new()
    NavigationMeshGenerator.register_geometry_parser_2d(custom_parser)

func _parses_node(node : Node):
    # return true if this node wants to add baking geometry
    return node is CustomNode2D

func _parse_geometry(node: Node, navigation_polygon : NavigationPolygon, source_geometry : NavigationMeshSourceGeometryData2D):
    var traversable_outline : PackedVector2Array = PackedVector2Array([])
    var obstruction_outline : PackedVector2Array = PackedVector2Array([])
    
    for outline_position in traversable_polygon_outline:
        traversable_outline.push_back(outline_position)
    for outline_position in obstruction_polygon_outline:
        obstruction_outline.push_back(outline_position)
    
    source_geometry.add_traversable_outline(traversable_outline)
    source_geometry.add_obstruction_outline(obstruction_outline)
extends NavigationGeometryParser3D
class_name CustomNavigationGeometryParser3D

func _init():
    var custom_parser : NavigationGeometryParser3D = CustomNavigationGeometryParser3D.new()
    NavigationMeshGenerator.register_geometry_parser_3d(custom_parser)

func _parses_node(node : Node):
    # return true if this node wants to add baking geometry
    return node is CustomNode3D

func _parse_geometry(node: Node, navigation_mesh : NavigationMesh, source_geometry : NavigationMeshSourceGeometryData3D):
    var mesh : Mesh = PlaneMesh.new()
    mesh.size = Vector2(10.0,10.0)
    var mesh_xfrom : Transform3D = node.global_transform
    source_geometry.add_mesh(mesh, mesh_xfrom)
    
    var mesh_array : Array = mesh.get_mesh_arrays()
    mesh_xfrom.origin.x += 5.0
    mesh_xfrom.origin.z += 5.0
    source_geometry.add_mesh_array(mesh_array, mesh_xfrom)
    
    mesh_xfrom.origin.x -= 10.0
    mesh_xfrom.origin.z -= 10.0
    var faces : PackedVector3Array = mesh.get_faces()
    source_geometry.add_faces(faces, mesh_xfrom)

NavigationMeshSourceGeometryData

NavigationMeshSourceGeometryData is the resulting data of a parsing operation done with the NavigationMeshGenerator used for navigation mesh baking.

The advantage of having this data available in a resources is that it can be stored and loaded from disk. This helps to avoid runtime performance issues with parsing operations on larger scenes. NavigationMeshSourceGeometryData makes it possibility to split the source geometry parsing process from the navigation mesh baking process. It also makes it possible to reuse the same source data to bake multiple meshes with different parameters.

Previously both parsing and baking were forced into the same function and frame which resulted in unavoidable runtime stutter.
Since parsing requires reading from the SceneTree (which is not thread-safe) this always caused problems when baking with threads. With already parsed source geometry the baking can now happen safely in a background thread.

Reworked NavigationMeshGenerator

The NavigationMeshGenerator singleton is now its own module and server, independent from the "navigation" pathfinding module and NavigationServer.

The NavigationMeshGenerator now also has a dummy NavigationMeshGenerator implementation so minimalist Godot can be build without including the module.

A NavigationMeshGeneratorManager is added to the engine to make generator implementation selectable in ProjectSettings.
This allows for other implementations of the generator for games that need a different navigation mesh baking than the default one.

The NavigationMeshGenerator now has full build-in multithreading for the navigation mesh baking and parsing using a WorkerThreadPool. For platforms or situations that have problems using threads this can be toggled with options in the ProjectSettings.

navigationmeshgenerator_settings

Previously every single NavigationRegion3D node was creating its own thread for baking its own navigation mesh. Not only added this stutter due to thread creation and waiting but region threads also parsed the SceneTree in unsafe moments resulting in random thread-related bugs. NavigationRegion3D also parsed and baked in one which created another case of unsolvable frame rate issues with (re)baking at runtime.

@Calinou Calinou added this to the 4.0 milestone Dec 30, 2022
@smix8 smix8 force-pushed the navigationmeshgenerator_rework_4.x branch 7 times, most recently from 44ccfc4 to acd10b8 Compare January 7, 2023 20:55
@smix8 smix8 marked this pull request as ready for review January 8, 2023 02:56
@smix8 smix8 requested review from a team as code owners January 8, 2023 02:56
doc/classes/NavigationMeshGeneratorManager.xml Outdated Show resolved Hide resolved
doc/classes/NavigationRegion3D.xml Outdated Show resolved Hide resolved
doc/classes/ProjectSettings.xml Show resolved Hide resolved
main/main.cpp Show resolved Hide resolved
main/main.cpp Outdated Show resolved Hide resolved
scene/3d/navigation_geometry_parser_3d.h Outdated Show resolved Hide resolved
scene/resources/navigation_mesh_source_geometry_data.h Outdated Show resolved Hide resolved
servers/navigation/navigation_mesh_generator.cpp Outdated Show resolved Hide resolved
tests/test_main.cpp Show resolved Hide resolved
@DarkKilauea
Copy link
Contributor

Generally I'm onboard with this approach, but I think there is a little confusion around having both the bake and parse_source_geometry_data paths that I'd like to see unified. There really should be only one way to get geometry data into the NavigationMeshGenerator for processing into a navigation mesh.

@smix8 smix8 force-pushed the navigationmeshgenerator_rework_4.x branch 9 times, most recently from f6f74a7 to 6301049 Compare January 9, 2023 20:52
@smix8 smix8 force-pushed the navigationmeshgenerator_rework_4.x branch 3 times, most recently from b54a82d to 51003a9 Compare April 19, 2023 08:04
Copy link
Contributor

@MewPurPur MewPurPur left a comment

Choose a reason for hiding this comment

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

Took a look at the descriptions at the top.

doc/classes/NavigationGeometryParser2D.xml Show resolved Hide resolved
doc/classes/NavigationGeometryParser2D.xml Outdated Show resolved Hide resolved
doc/classes/NavigationMesh.xml Outdated Show resolved Hide resolved
doc/classes/NavigationMesh.xml Outdated Show resolved Hide resolved
doc/classes/NavigationMesh.xml Outdated Show resolved Hide resolved
doc/classes/NavigationMeshGenerator.xml Outdated Show resolved Hide resolved
doc/classes/NavigationMeshGenerator.xml Outdated Show resolved Hide resolved
doc/classes/NavigationMeshGenerator.xml Outdated Show resolved Hide resolved
@smix8 smix8 force-pushed the navigationmeshgenerator_rework_4.x branch 2 times, most recently from 86dadbb to b13fa0d Compare April 26, 2023 12:26
Reworks NavigationMeshGenerator and navigation mesh parse and bake process. Adds navigation mesh baking for 2D.
@smix8 smix8 force-pushed the navigationmeshgenerator_rework_4.x branch from b13fa0d to 38699a8 Compare April 29, 2023 16:58
@filipinyo
Copy link

Any info on when can we expect this?

@smix8
Copy link
Contributor Author

smix8 commented Jun 1, 2023

This pr is now broken due to more recent changes in core related to SceneTree and threading.
There is no easy or fast fix. A bulk of the pr needs to be redone from scratch. No ETA.

Relintai added a commit to Relintai/pandemonium_engine that referenced this pull request Jun 4, 2023
…ator

Reworks NavigationMeshGenerator and navigation mesh parse and bake process. Adds navigation mesh baking for 2D.
- smix8
godotengine/godot#70724
godotengine/godot@38699a8
@groud
Copy link
Member

groud commented Jun 16, 2023

EvenOdd is closest to the old NavigationPolygon outline behavior minus all the old polygon overlap errors. This fillrule turns inner outlines that are even numbered into obstacles (holes) and odd numbered into traversable surfaces.

Is this mode really needed ? It looks kind of hard to use and not really useful. Unless i misunderstood something.

From the original post and the code, it seems like some properties to NavigationPolygon do not really belong there. Things like agent_radius, collision_mask, etc... should be decided at bake time, if I am not mistaking ? I feel it should be more like "bake profiles" than properties of the polygon themselves ?

Regarding NavigationGeometryParsers, I think the idea is good, but it does not really look like the Godot-way to do the things. Instead of a custom class to extend, it would be a lot easier to simply allow 2D or 3D node to expose a _parse_navigation_geometry virtual method if they want to, instead of relying on another class.

For TileMap mesh baking, the approach looks quite good to me. As I mentioned in chat, I am unsure whether if, like, being able to define "walkable areas" make a lot of sense when you can define "non-walkable areas" using collision shapes. It might be a bit confusing to users IMO. However, if we want to keep it, I believe that, by default, we should probably make it that the default shape on tiles is the tile's shape. That would avoid having to define the shape for all tiles.

The NavigationMeshGenerator singleton is now its own module and server, independent from the "navigation" pathfinding module and NavigationServer. The NavigationMeshGenerator now also has a dummy NavigationMeshGenerator implementation so minimalist Godot can be build without including the module

What is the point of this ? Is it to reduce the executable size ? Because, in terms of features, it makes it a bit weird to have navigation without the possibility to bake things at some point.
Or maybe is it mainly to make it swappable ?

@KoB-Kirito
Copy link

I need tilemap navigation mesh baking in my life 🙇‍♂️
But what do you mean with on the first layer? That's one of the main problems I have currently, I have dozens of layers. Most are painted on the ground, which I have to mark as navigation, so collisions are marked as well in some locations. It would be nice if it could "merge" all layers, just take all collisions together, and bake a navigation mesh from that.

@smix8
Copy link
Contributor Author

smix8 commented Aug 19, 2023

Superseded by #80796.

@AThousandShips AThousandShips removed this from the 4.x milestone Aug 20, 2023
@Zylann
Copy link
Contributor

Zylann commented Mar 23, 2024

Sorry to post this after all this time, but I keep coming back to this problem, and was waiting on this PR. Now it was closed, but I found that the one superseeding it only about 2D. So it seems everything raised about 3D is back to nobody working on it? Or something else was done since?

@smix8
Copy link
Contributor Author

smix8 commented Apr 1, 2024

@Zylann
I removed this from the PR because as you see from the history the PR was always too big and never got reviewed (as is tradition with navigation PRs).

The general idea is not abandoned but I simply had no time to get back to it.

We already talked in godot dev chat but can continue talk in godot dev navigation channel if you want because I see you made a lot of comments and "issues" on different repos in the past days regarding this and other navigation problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment