godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
86.73k stars 19.4k forks source link

MeshDataTool.get_edge_faces giving incorrect faces. #91536

Open bluelightzero opened 2 months ago

bluelightzero commented 2 months ago

Tested versions

v4.2.2.stable.official [15073afe3]

System information

Godot v4.2.2.stable - Fedora Linux 40 (Workstation Edition) - Wayland - Vulkan (Forward+) - dedicated AMD Radeon RX 5700 XT (RADV NAVI10) () - AMD Ryzen 7 3700X 8-Core Processor (16 Threads)

Issue description

MeshDataTool.get_edge_faces() incorrectly reports 1 face (An open edge) on some edges that should share a face.

Steps to reproduce

func _ready():
    var mesh_instance = $default.find_child("Cube") as MeshInstance3D;
    var mesh = mesh_instance.mesh as ArrayMesh;

    var mdt = MeshDataTool.new()
    mdt.create_from_surface(mesh, 0);

    var totals = [0, 0, 0];
    for i in range(mdt.get_edge_count()):
        totals[mdt.get_edge_faces(i).size()] += 1;
    print("Open edges: ", totals[1]);
    print("Shared edges: ", totals[2]);

Output I get:

Open edges: 24
Shared edges: 6

Output I expect (Cube is triangulated during import):

Open edges: 0
Shared edges: 18

It should not be possible for a cube to have open edges, but for some reason it does.

Minimal reproduction project (MRP)

BlenderImportFaceEdges.zip

bluelightzero commented 2 months ago

I just tested with built in BoxMesh:

var mesh_instance = $MeshInstance3D;
var box_mesh = mesh_instance.mesh as BoxMesh;

var surface_tool := SurfaceTool.new()
surface_tool.create_from(box_mesh, 0)
var array_mesh := surface_tool.commit()

var mdt = MeshDataTool.new()
mdt.create_from_surface(array_mesh, 0);

var totals = [0, 0, 0];
for i in range(mdt.get_edge_count()):
    totals[mdt.get_edge_faces(i).size()] += 1;
print("Open edges: ", totals[1]);
print("Shared edges: ", totals[2]);

The same problem happens:

Open edges: 24
Shared edges: 6

I don't think blender has anything to do with it.

BlenderImportFaceEdges.zip

bluelightzero commented 2 months ago

After playing around with a few meshes, I don't actually think this is a bug. Maybe there could be some extra documentation to make this clearer though.

The fix is simple: Shade the model smooth.

It treats edges as unique if there are any properties at all that are not shared by the vertices. The mesh I was working with was this one:

image

Because the mesh is flat, most of the time it recognises that the flat shaded vertices are identical and treats them as such, but I guess sometimes they are slightly different (Moving the mesh around is enough to causes changes) and splits them up during import.

I have solved my problem, but I can imagine this might stump a lot of people trying to write code to work with meshes.

huwpascoe commented 2 months ago

Open edges are going to occur at any boundary between patches of triangles. A sharp normal, a UV map seam, or even random breaks if there's triangle strips involved.

image

This cube for example, might have 5 common edges and 14 seams. There's many ways it can be rearranged though...

Processing any mesh data, from blender or otherwise, I've found it easiest to treat each face as completely individual, then worry about re-reducing shared vertices later.

lawnjelly commented 2 months ago

This is a common misunderstanding, which should eventually be added to the docs. A cube with sharp edges has 24 open edges, and 6 shared edges.

24 is 6*4 (for each corner of each face) 6 is for the edge between two triangles forming each face.

Edges are not shared if the normals are different. Godot vertices cannot have multiple normals, or multiple UVs. Where the position is the same and other attributes differ, the vertex is duplicated (sometimes on import).

This is in effect a duplicate of #75279.