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

The number of vertices imported from obj file is not correct #75279

Open
huangjiaminhhh opened this issue Mar 24, 2023 · 6 comments
Open

The number of vertices imported from obj file is not correct #75279

huangjiaminhhh opened this issue Mar 24, 2023 · 6 comments

Comments

@huangjiaminhhh
Copy link

Godot version

3.5.stable.custom_build.991bb6ac7

System information

Macos Monterey

Issue description

The mesh imported from 138.obj should have 5023 vertices but has 5118 in godot.
image
image

Steps to reproduce

Run uploaded project.

Minimal reproduction project

test.zip

@MGilleronFJ
Copy link

See also #71085

@lawnjelly
Copy link
Member

lawnjelly commented Mar 26, 2023

I haven't fully examined the obj file, but from a quick look, this is likely a misunderstanding of how Godot stores vertices. 3D modelling packages often store vertex data in separate streams, vertex position, vertex normal and vertex uv. This is what is shown in the obj file (if you open it up, you will see lines that start with:

v - position
vn - normal
vt - uv

The actual vertex in Godot contains all of these together in the same stream, and must be unique per vertex. This is defined in the faces in the obj format:

f 4290/4323/4290 4304/4340/4304 4291/4320/4291

This combines a position, normal and uv to form a unique vert. It is the unique vertices that are measured in Godot, not the source streams. This is common in most cases for indexed rendering on GPUs, you supply a single index for a unique vertex, rather than an index for each component.

See:
https://en.wikipedia.org/wiki/Wavefront_.obj_file

This could possibly be added to documentation somewhere as it comes up periodically, but the misunderstanding is about the file format, rather than Godot, so is not really unique to Godot engine, just a gotcha when dealing with model formats. 🤔

Added a documentation tag, which we could do if a contributor gets around to it, perhaps junior job. Otherwise this could just be closed, unless there is actually an import error occurring.

@dougVanny
Copy link

dougVanny commented Jan 31, 2024

I believe I have just stumbled upon this bug. I am manipulating meshes down to the vertex level, meaning that I require a precise control over the amount of vertices and triangles. Whenever importing my obj into godot, however, I noticed that it was joining vertices by default, without the option to disable that.

image
image

However, when imported into Godot and consulting the amount of vertices and indexes in the mesh, the values I get are 11 vertices and 24 indexes (24/3 = 8 faces). Looking into, it is clear that Godot is joining vertices when they are in the same position

To make sure, I checked the .OBJ file and it clearly contains a total of 24 vertex entries, so this is not an issue with the .OBJ exported by blender: TriangulatedPlane.obj.txt

Investigating, I found this old Godot documentation that mentions the existence of a "Weld Vertices" option when importing meshes. No such option exists within 4.2 however, which leads me to believe that this is always defaulting to true. This is how I am currently importing my OBJ file. Interestingly, the documentation for the "Optimize Mesh" parameter is: "Unused parameter. This currently has no effect."
image

@lawnjelly I would suggest labeling this issue as a bug, as I believe that importing the raw mesh as it is should be an option, specially for someone who is going with an OBJ file rather than a GLTF or a FBX. For now my workaround will be writing a simple custom importer that will build and import the mesh the intended way

@lawnjelly
Copy link
Member

During the import it probably identifies unique vertices (looks like it does this with surf_tool->index() in resource_importer_obj.cpp as part of the indexing step).

As far as Godot is concerned for rendering (in normal circumstances), there is no point in repeating them twice in the vertex stream, as indices can be used instead. This makes the vertex stream smaller and rendering faster, and less potential for GPU artifacts. So 11 unique verts is expected (7 round the side of large piece, 1 in centre, and 3 in the small triangle).

I agree that importing files with minimal modification is sometimes desired (see e.g. my PR #86339 where the ability to not optimize indices is exposed for this very reason).

Importing obj using duplicated vertices sounds like a feature request, I can add a label. As you say, in this particular case, you can also write gdscript to import obj, it is not super complicated (although there are a couple of gotchas with negative indices if I remember right).

It would not be super complex to add to core, but has to be balanced against how many people would use this versus confusion for the other 99% of users. But perhaps it could use the existing optimize flag or something like that.

@dougVanny
Copy link

Importing obj using duplicated vertices sounds like a feature request,

For clarification, this is less about duplicating the vertices, and more about following what the source file is doing. When making this mesh on blender, I had to go out of my way to make sure the vertices were split. Had I not done that, the mesh would have the minimum amount of vertices.

I don't think that having an option to duplicate vertices so that faces are all disconnected from one another makes sense, I just think that, while it is cool that Godot has the option to optimize the mesh, it should be a responsibility of the 3D modeling software, so Godot should have the option to not do any optimization. By just cutting faces on blender, it will generate a mesh with minimal vertices, but once I put on the effort to separate them, Godot undoing that work feels silly.
image

I have already made my custom importer and am not blocked by this issue, but I will add though that the OBJ format does not translate perfectly to the way how Godot stores meshes. The way how you can reuse vertices on different faces, but with different normals and UVs would require that vertex to be duplicated, which in the end of the day would result in a slightly different mesh form the one described in the file. Regardless of that, however, I guess that blender and possibly other softwares will not export OBJs like that, or at least I hope so, so I think that my point about sticking as close as possible to the imported file still makes sense.

@lawnjelly
Copy link
Member

lawnjelly commented Feb 17, 2024

For clarification, this is less about duplicating the vertices, and more about following what the source file is doing.

To be clear, although modelling packages and some file formats may store vertex attributes in separate (non-correlated) streams, Godot doesn't do this by design. GPUs don't naturally work in this way - there is usually one index for the entire vertex (all attributes, position, uv, color, normal etc), no matter how many streams. Afaik modelling packages that store in this way need to modify the data again prior to rendering (to produce unique vertices, either as a single, or multiple coherent streams). (very old school OpenGL immediate mode exposed this kind of thing, but it likely did translation under the hood. I'm also not familiar with Vulkan / DX12, I'm assuming also that they do not allow independent indexing for attributes, defer to the 4.x guys here.)

If you want to build a modelling package in Godot, you would be responsible for storing these streams in your user code, then rebuilding the actual Godot meshes after each change (likely an expensive step), prior to rendering.

It is conceivable that Godot could have the ability to store such source streams in core, but this would be a major proposal, and unlikely to be implemented without significant demand, as it complicates the codebase / bug surface / maintenance requirements.

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

No branches or pull requests

6 participants