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

Add loop subdivision algorithm #1750

Merged
merged 14 commits into from
Nov 23, 2022
Merged

Conversation

HyeonseoNam
Copy link
Contributor

Hi, I added an loop subdivision algorithm as the need for the advanced subdivision method mentioned in #1004.

I tried to implement the Catmull-Clark method as you mentioned in #1557, but I found that this method is suitable for quad mesh and requires additional triangulation. Also, loop subdivision is more preferred in triangle mesh (these are disccussed in https://gamedev.net/forums/topic/684378-does-catmull-clark-subdivision-work-for-meshes-consisting-of-only-triangles/5322569/). So I implemented loop method instead of catmull.

My implementation is based on this reference: p70 in http://multires.caltech.edu/pubs/sig00notes.pdf/

  1. Calculate odd vertices: Assign a new odd vertex on each edge and calculate the value for the boundary case and the interior case.
  2. Calculate even vertices: Calculated new even vertices from the existing vertices and their adjacent vertices.
  3. Compose new faces.

@mikedh
Copy link
Owner

mikedh commented Nov 16, 2022

This looks awesome thanks for the PR! Good catch with catmul looking lousy on triangle meshes, this seems like a much better solution and it's sweet you were able to vectorize it! I'll try to test and merge later this week, but my initial notes are:

  • don't worry about the formatting, I can make a pass pretty easily.
  • I think I'll rename Trimesh.loop -> Trimesh.subdivide_loop to match the other functions.
  • is the multibody flag necessary since it's checking graph.neighbors inside? Or does it get wacky for multiple bodies if you don't split?

@HyeonseoNam
Copy link
Contributor Author

I'm very happy that I can contribute to this thankful trimesh :)

I also thought it is quite strange about the name of function, loop. subdivide_loop seems to be much more appropriate.

Yes, since graph.neighbors just gives all neighbors.
In the algorithm, to calculate even vertices, summation should be performed by each vertex's neighbors for same body, but graph.neighbors gives all neighbors from all shared bodies, then, it is quite ambiguous to classify where each neighbor belongs to.
Calculating odd vertices(before calculating evens) is in the same context. An edge should be shared with 1 (boundary case) or 2 (interior case) faces for same body. If more than 2 (e.g., grouping.group_rows(edges, require_count=4)), it is also ambiguous to figure out whether each face is in the same body or not.

So I handled this case (more than 2 faces share one edge) as an error, and made it run for each splitted body. This has possible to be slow if there are many bodies, so I think there will be a better way.

@HyeonseoNam
Copy link
Contributor Author

HyeonseoNam commented Nov 17, 2022

Oh, if split and concatenate are performed at the beginning of the code for all cases, the multibody flag seems to be unnecessary. If you want to me to make changes, I'll send a commit (to remove multibody flag), or refer to it when you reflect code !

mesh = trimesh.load("trimesh/models/cycloidal.ply")
# sub = mesh.loop(iterations=1, multibody=False) # will return error
# sub = mesh.loop(iterations=1, multibody=True) # will use for loop

# <-- replace multibody flag with
splited_meshes = mesh.split(only_watertight=False)
if len(splited_meshes) > 1: 
    mesh = trimesh.util.concatenate(splited_meshes)
# --> 
sub = mesh.loop(iterations=1, multibody=False) # no for loop, no error

In addition, I put some rendered results. Maybe it will be helpful for reference when people use.

target_mesh = trimesh.load("trimesh/models/cycloidal.ply")
image

target_mesh = trimesh.creation.cone(1, 1.5)
image

a custom mesh
image

@mikedh mikedh changed the base branch from main to feat/subd November 23, 2022 18:49
@mikedh mikedh merged commit 9889c78 into mikedh:feat/subd Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants