danini-the-panini / mittsu

3D Graphics Library for Ruby.
https://github.com/danini-the-panini/mittsu
MIT License
508 stars 33 forks source link

OBJ loader clones all vertices for every material #123

Open Floppy opened 6 months ago

Floppy commented 6 months ago

When loading a multi-material OBJ file, such as male02.obj, each time a new material is encountered, the existing mesh is finished, a copy made of the vertices, and then a new mesh started. This means that connectivity information becomes more vague, as an loaded object may have multiple meshes, each with separate (albeit coincident) vertices, and a separate list of faces.

This is probably fine for rendering, so it may be a low priority issue, but in my use case where I'm trying to convert files, it means that I've lost some information, and when I output a mesh there are lots and lots of unused vertices.

I suspect the right way to solve it is for the loader to use geometry groups, as described in https://threejs.org/docs/index.html?q=buffergeom#api/en/core/BufferGeometry.groups, rather than separate meshes, but I don't know what the state of those is in Mittsu currently.

Floppy commented 5 months ago

I'm having a read through the OBJ specification, because some of my files weren't loading and I wasn't sure why. I think there's some more problems with the OBJ loader than I initially thought here.

For instance, the o object name line is purely for naming. In the spec it says:

Polygonal and free-form geometry statement. Optional statement; it is not processed by any Wavefront programs. It specifies a user-defined object name for the elements defined after this statement. object_name is the user-defined object name. There is no default

In the Mittsu loader code though, an o line starts a new object and resets the vertex list, which I don't think should ever happen, as all vertices are numbered sequentially across the entire file. So I think the loader is a bit wrong. I think that's not shown up because the test file doesn't have an o partway down, unlike the ones I'm loading.

@danini-the-panini are there any reasons for the loader doing some of this? It's entirely possible I've misunderstood something. If not, and I'm looking at it right, I'm happy to address the o behaviour and the vertex cloning all in one big fix.

danini-the-panini commented 5 months ago

yeah, i think it was to name the object that the o refers to, it has to create a new object

Floppy commented 5 months ago

Right, that makes sense. I was looking at the THREE.js loader, and it handles it by naming the original object in the case where the first o is encountered later on. I'll have a look.

Floppy commented 5 months ago

OK, I've fixed the o problem with a pretty minimal change in #126. I'll just see if I can do the group thing as well.

Floppy commented 5 months ago

Not fixing the main face group issue in #126 - needs a bigger bit of work and the PR is good to go to resolve the object problem. Sorry for derailing this issue with another one that turned out not to be related.