Compute STL normals #70

Merged
merged 1 commit into from Dec 2, 2012

3 participants

@bgamari

This fixes Issue #40. It seems, however, that the expression for the face normals given in this issue is off by a negative sign (comparing against normals computed by meshlab)

@colah
Owner

It isn't entirely clear this is the right course of action.

STL normals are ignored by basically all readers, or, at the very least, recomputed if you give a 0 vector. Why do extra work?

With regards to tabs/spaces, everything in ImplicitCAD is tabs right now, not just that file. Is there a good reason to switch to spaces? While it is trivial to run a sed script over it, merges of work done before hand would be a pain. And I kind of like tabs for indentation, spaces for alignment...

@bgamari

@colah, the untabify patch was simply because I saw that the rest of the file was using spaces. I assumed this was an inconsistency that was in need of fixing. It seems I resolved it in the wrong direction. That being said, I personally think that mixing tabs and spaces is a pretty bad idea. This explains why the code is often so mangled in my text editor (emacs). I have given up on using tabs in source code and now just use spaces for indentation (four spaces) and alignment everywhere. I've found my life has been simplified as a result.

As far as computing the normals, I don't see any reason not to do it. It's a pretty simple calculation and as Wikipedia says,

 in order to be entirely portable, a file should both provide the facet normal and order the vertices appropriately.
@matthewSorensen

If we are going to support stl with normal vectors (and I explicitly didn't add that when I rewrote all of the stl export), you'll also need to patch the binary stl implementation.

@bgamari

I've rebased this patch on top of master, added support to the binary STL implementation, and verified that both match meshlab's normals. I think this should be now ready.

@colah
Owner

Great!

@colah colah merged commit c0d7b41 into colah:master Dec 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment