Change the orientation of generated NavigationMeshes so they face up #23818
Conversation
needleful
changed the title
Now the faces of a navigation mesh point towards the specified "up" vector, as intended. Fixes godotengine#23817
|
If the problem is just visual representation I'd suggest changing of debug mesh generation only. |
|
Sorry, I don't know what you mean. I'm only seeing one mesh being referred to in a NavigationMeshInstance, so I'm not sure if you mean a "debug mesh" that I haven't noticed that's only used for viewing, or only changing the orientation if the engine is built targeting debug, or something else. Imported navmeshes are rendered as expected, so I wouldn't want to change how all of them are rendered, just the meshes baked by the editor. |
|
The display of navmeshes is different from actual navmesh data. If you
change navmesh triangles you might disrupt actual graph.
Editor debug code uses special debug mesh with debug material applied to
display navmesh data. The actual winding order
is not important for navmeshes but actual node order is. I think you could
just disable culling for debug geometry material to
avoid messing with mesh data. Look at Ref<Mesh>
NavigationMesh::get_debug_mesh() and
get_tree()->get_debug_navigation_material() for reference.
…On Wed, Nov 21, 2018 at 9:37 AM needelful ***@***.***> wrote:
Sorry, I don't know what you mean. I'm only seeing one mesh being referred
to in a NavigationMeshInstance, so I'm not sure if you mean a "debug mesh"
that I haven't noticed that's only used for viewing, or only changing the
orientation if the engine is built targeting debug, or something else.
Imported navmeshes are rendered as expected, so I wouldn't want to change
how all of them are rendered, just the meshes baked by the editor.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#23818 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAX07R-48kzNx5gakomPqBQX9zwq-nPks5uxPS-gaJpZM4YoaAJ>
.
|
|
basically just set material's cull_mode to CULL_DISABLED. This way it will disable cull mode and navmeshes will work as intended. |
|
Sorry for irregular responses, I'm traveling right now. I'll change the commit so it just changes the material. |
This reverts commit db80dd4.
|
After some thinking, I'd argue we flip the faces, since this is a bug in the mesh generation, rather than the rendering. I'd rather not change how all navmeshes are rendered just to make the generated meshes look good. |
|
TL;DR
That depends on how you use navmesh graph. If you flip triangles you might
get wrong results. Also navmesh is not visual structure 0 it is graph
structure for pathfinding and structural world wareness, not visual
representation. When you generate navmesh using Recast in Godot editor, you
get optimized navmesh graph. If you import pre-made "navmesh" from Blender
you get
not so optimized structure which does not pass optimizations and filtering
of Recast (basically just uses geometry data as is). BUT if you generate
data, flipping of triangles to look the same is pointless waste of
resources. What you see in editor is DEBUG mesh generated from navmesh.
What you do is needlessly modify data to make DEBUG mesh look good for you.
Is there any point? More logical step would be change DEBUG mesh generation
code, but that is pointless again, as you can disable culling in DEBUG mesh
and completely ignore navmesh data winding order. Single line patch. So you
will accept both modelled "navmeshes" and generated by Recast for DEBUG
display regardless of winding order. This is how it supposed to be done.
More complex answer follows:
I work on navigation_pr and that branch uses Recast/Detour to the fullest.
That means area costs and ledges. When you flip triangles like you do, you
will mess-up costs of navigation and neighbor relationships in navmesh
generated by Recast/Detour. Which will lead to incorrect navigation, making
it from corner case to common problem. As for simple navigation which
barely uses navmesh data this is not obvious, for complicated scenes this
will lead to navigation errors due to incorrect polygon connections.
navigation_pr also uses this navmesh debug code to display navmesh in
editor and during debug, so changing material to non-culled will solve
problem of display. Also another issue is if you have modelled your navmesh
on ceiling and floor using cube as source, so your winding order will
prevent part of navmesh from being displayed. So culling of DEBUG navmesh
material should be disabled regardless, as it is in the way.
…On Fri, Nov 23, 2018 at 9:00 AM needelful ***@***.***> wrote:
After some thinking, I'd argue we flip the faces, since this is a bug in
the mesh generation, rather than the rendering. I'd rather not change how
all navmeshes are rendered just to make the generated meshes look good.
I made a brute-force test to see if there were any differences between the
regular and flipped navmeshes: navmesh-test.zip
<https://github.com/godotengine/godot/files/2609781/navmesh-test.zip>. It
generated several thousand paths using the two navmeshes, and they always
generated the exact same paths, so I feel confident that the polygon
orientation doesn't affect the navigation, since it doesn't change the
edges or vertices.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#23818 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAX08oVoEX2jnN57cNH0eZEZ-M-mdw8ks5ux48AgaJpZM4YoaAJ>
.
|
|
Ok, fair enough. You know more about the navmeshes and Recast than I do, so I'll take your word that there's behavior depending on the order of the edges. I just didn't want to be masking over a bug, if this fixed it. Even if it were a small bug, that's not a pattern I want to fall into. |
needleful
force-pushed
the
navmesh-orientation
branch
from
f21d093 to
dddab72
Compare
|
After trying to get this working, I've discovered that |
|
I still feel like changing the mesh generation, even if what I had wasn't the solution, is preferable to changing the material. It's possible some users will want to know what direction the face of a navmesh is oriented, for example to align their navigation agent as it goes up walls/on ceilings/etc. With generated navmeshes being oriented the wrong way, that would be harder to do, especially if they mix generated and hand-made navmeshes. While ignoring the orientation fixes the basic uses, it only hides potential problems for more advanced cases. |
needleful
force-pushed
the
navmesh-orientation
branch
from
dddab72 to
8439ace
Compare
|
I think this at least deserves configuration parameter, if you want to go
this route.
Recast uses "up" vector value when generates navmeshes I think you could
research there for what is good.
As I guess importer code does nothing. If everything is right, the code
which converts Recast data to Godot NavigationMesh data might be buggy,
But double check is needed. For complex navigation you would not use
imported navmeshes so please check that Recast gets correct parameters
when navmesh is generated and that these data are correctly converted to
Godot data. Just blindly changing winding order might lead to future
problems,
so this have to be justified, as manipulation of graph data is last thing
you want to do. Recast is used in engines like Unity and UE4 and others, so
you can be
sure of its correct behavior, so a bug is somewhere in Godot code while
either supplying data to Recast or while converting the result. You should
not be afraid of touching core code in 3D as it does have much less
exposure to the public and can have a lot of bugs unlike 2D code, and
fixing them is great way to move forward.
…On Sat, Nov 24, 2018 at 7:40 AM needelful ***@***.***> wrote:
I still feel like changing the mesh generation, even if what I had wasn't
the solution, is preferable to changing the material. It's possible some
users will want to know what direction the face of a navmesh is oriented,
for example to align their navigation agent as it goes up walls/on
ceilings/etc. With generated navmeshes being oriented the wrong way, that
would be harder to do, especially if they mix generated and hand-made
navmeshes. While ignoring the orientation fixes the basic uses, it only
hides potential problems for more advanced cases.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#23818 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAX0wkyUHthMfcAOhfvn2lhcw3R-vb5ks5uyM2ggaJpZM4YoaAJ>
.
|
|
I'll look into it more. I wasn't previously aware that winding order was used for anything, so I'll experiment and see what the best way to go about it is. It seems the issue is with how the navmesh is converted from Recast's format to Godot's, so I bet there's a way to preserve any relevant information about the graph while fixing the mesh. |
|
I'll close this pull request, and come back when I've got a way to fix the meshes while preserving the graph. |
Now the faces of a navigation mesh point towards the specified "up" vector.
Fixes #23817
This should not (and from my testing, doesn't) affect the path-finding behavior of a navmesh, only its appearance in the editor, but feel free to double-check.
I think (and I only looked at this for like an hour so I'm no expert) this bug was because the ReCast library orients the faces of meshes counter-clockwise, while Godot orients them clockwise. This was taken into account when inputting meshes to ReCast, but apparently forgotten about when getting the generated navmesh.