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

Using Voxel Centers for Marching Cubes #12

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

davidstutz
Copy link

Unfortunately, I cannot create a pull request for a new branch.

This pull request includes the changes of #11 and additionally changes the marching cubes algorithm to use the voxel centers as reference points. This shifts the resulting mesh slightly.

@pmneila: I suggest not merging this pull request into master; you can create a new branch if you like, otherwise this pull request is just for people interested in the changes and you can leave it open.

Thanks.

@pmneila
Copy link
Owner

pmneila commented Nov 23, 2017

This is an interesting PR. I would like to adapt the API to allow the user which kind of reference points they want (the original style or the voxel centers). I have to think about it.

By the way, your commit 34ce2be is adding _mcubes.cpp to the repository. This is an automatically generated file and it should not be tracked by git unless you have a very good reason for that. Is there a reason for that? Otherwise, could you remove it?

@davidstutz
Copy link
Author

No adding _mcubes.cpp was an error (I probably automatically added all .cpp files). Sorry for that. I will make a new PR in the near future.

Regarding the API, I think it would be very nice to let the user switch. For my research, I actually used at least two different variants (voxel centers and voxel corners). But I simply duplicated the marching cubes functions - quick and dirty before the deadline.

@davidstutz
Copy link
Author

@pmneila: Removed _cubes.cpp.

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