Skip to content
This repository has been archived by the owner on Sep 28, 2019. It is now read-only.

Increased support for quad faces and ft's #2

Closed
wants to merge 5 commits into from
Closed

Conversation

paulmelnikow
Copy link

@paulmelnikow paulmelnikow commented Sep 8, 2018

This adds some initial support for quad faces, as well as textures and normals for quad meshes. These features are supported:

  1. Reading a quad mesh's faces, normals, and texture coordinates from an OBJ. f4, fn4, and ft4 are set in addition to f, fn, and ft which hold the triangulated faces as before.
  2. Writing a mesh with quad faces to OBJ. write_obj prioritizes f4, fn4, and ft4 over the triangulated faces.
  3. keep_vertices operates correctly on either f and f4, though you need to del the one you don't want updated.

This approach worked for my application, though it is probably not the right thing to do in a legacy context. (2) and (3) would yield new behavior in certain workflows, for example reading a quad mesh, mutating f, and then writing to OBJ, or reading a quad mesh and then using keep_vertices.

Ideally we'd provide the legacy behavior by default, while also providing an upgrade path that gives first-class support to quad meshes. One way to do that would be a separate QuadMesh class with mostly shared mixins. By using QuadMesh you indicate you want to act on quads, and then loading operations and all manipulations would happen on the quads, while the triangulated faces would not be loaded. The same could be accomplished by passing a new parameter like with_quads=True to the Mesh constructor. QuadMesh is nice because it'll be a bit easier to troubleshoot down the line. When you log the mesh you'll see that you've got the wrong thing. Though that could be handled in __repr__ just as easily. I guess it depends whether we want to hang different mixins on quad and triangle classes, or share them all.

The mesh loading code works, though it's not covered by any tests. The existing mesh-loading tests are disabled because of missing assets.

@paulmelnikow
Copy link
Author

I'll start implementing the conservative thing: with_quads=True.

@paulmelnikow paulmelnikow changed the title Support quad faces and ft's Increased support for quad faces and ft's Sep 9, 2018
@paulmelnikow
Copy link
Author

paulmelnikow commented Sep 9, 2018

After doing that, one thing I realize is that for casual or interactive use of meshes, it would be more helpful to load quads when quads are present and tris otherwise. So, perhaps instead of with_quads=True, the parameter should be triangulate, where triangulate=True provides the legacy behavior, and triangulate=False gives you whatever is there.

In either case, a mesh is frozen into one mode or the other at load time, with triangulate() and detriangulate() methods returning new instances.

Possibly down the line, the default for triangulate could be flipped, meaning you'd need to provide triangulate=True to get the legacy behavior.

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

Successfully merging this pull request may close these issues.

1 participant