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

A Thing that doesn't load right #4

Open
tbuser opened this issue Feb 22, 2011 · 15 comments
Open

A Thing that doesn't load right #4

tbuser opened this issue Feb 22, 2011 · 15 comments
Labels

Comments

@tbuser
Copy link
Owner

tbuser commented Feb 22, 2011

There's something wrong with this stl http://www.thingiverse.com/thing:6620

Also: Note to self - add some kind of testing framework.

@martymcguire
Copy link
Contributor

Folks have been reporting more objects that don't work:

I wonder if some of these are related to normals facing the wrong way? I'm not really familiar with what format or ordering THREE.js is looking for.

@tbuser
Copy link
Owner Author

tbuser commented Feb 22, 2011

Here's another one http://www.thingiverse.com/thing:6624

@cbiffle
Copy link
Contributor

cbiffle commented Feb 24, 2011

http://www.thingiverse.com/thingiview:24758 and http://www.thingiverse.com/thing:6620 are both fixed by the pull request I sent you: the object's name (first and last line of the STL) contains a '-' character that isn't matched by \w. The ASCII STL parser must be more permissive in the first line.

http://www.thingiverse.com/thing:5984 begins with the word 'solid', which tricks the parser into assuming it's an ASCII STL file -- which it isn't, it's binary. This is arguably a bogus file -- the Wikipedia STL page even calls this out as a silly thing to do -- but the parser could be made smarter about this, I suppose.

http://www.thingiverse.com/thingiview:44119 and http://www.thingiverse.com/thingiview:44101 both have bogus normals that you're rendering correctly.

@cbiffle
Copy link
Contributor

cbiffle commented Feb 24, 2011

Aaaand http://www.thingiverse.com/thing:6624 also has broken normals. Even though thingiloader recomputes the normals, the STL file is internally consistent: it's not just that 'facet normal' is wrong, but the vertex ordering/winding is wrong in the same way. If I pull it into Blender, remove doubles, and recalculate normals, it displays fine.

I'm not sure that Thingiviewer can be reasonably expected to render this file correctly, given that you're doing exactly what it says.

@martymcguire
Copy link
Contributor

More things reported by Thingiverse users:

I suspect these share the same issues as the other files above. :)

@cbiffle
Copy link
Contributor

cbiffle commented Feb 28, 2011

Except for bogus normals (which always show up as transparent surfaces), the issues above are fixed. :-)

http://www.thingiverse.com/thing:6278 is a neat one -- apparently the ASCII parser needs to handle DOS-style line endings!

Here's my diagnostic procedure.

  1. I downloaded OpenX_Carriage.stl.
  2. I ran file OpenX_Carriage.stl, which said ASCII text, with CRLF line terminators. This was already a bad sign, but maybe file was lying? So...
  3. I ran hexdump -C OpenX_Carriage.stl | less. Sure enough, where the ASCII parser expects \n, this file contained \r\n.

On http://www.thingiverse.com/thing:4398,

  1. File reported UNIX line endings, so that's good.
  2. less showed that the solid had no name, but couldn't indicate whether solid was followed by a single space, as required by the de-facto "standard."
  3. hexdump -C top_backright.stl | less showed that it is not. This file will not parse correctly using the original ASCII parser or my fixed version.

This latter file is arguably broken, but Pleasant3D (among other tools) handles it fine.

My open pull request fixes both these issues. Marty, feel free to pull from my tree if you're in a hurry.

@martymcguire
Copy link
Contributor

Thanks for taking an in-depth look, cliff! Awesome. I've got a stack of things to do today, but I think I'll go ahead and merge this in if I get a chance.

I got another report from the contact form:

http://www.thingiverse.com/thingiview:45832

This one looks like it has flipped normals.

@tbuser
Copy link
Owner Author

tbuser commented Feb 28, 2011

I've got a fix for the flipped normals but it's causing some rendering artifacts that I'm not sure I like or know what I can do about. Basically you just need to set object.doubleSided = true; in loadObjectGeometry()

@cbiffle
Copy link
Contributor

cbiffle commented Mar 1, 2011

I'm kind of amazed that these objects print correctly, much less render, with broken normals.

@martymcguire
Copy link
Contributor

Here's one that fails in a new way:

http://www.thingiverse.com/thing:913

It simply doesn't seem to show up anywhere, and none of the camera angle options reveal the object. I haven't had time to dig into logging to see where it might be dying.

@martymcguire
Copy link
Contributor

Another one that seems to fail with flipped normals:

http://www.thingiverse.com/thingiview:47031

@tbuser
Copy link
Owner Author

tbuser commented Mar 9, 2011

The latest commit I just pushed fixes the majority of the the problems reported here (although not all)

The inverted normals now render, however by using doubleSided = true it results in the lighting being reversed, so there's probably a better way around that.

I'm wondering if we should open a separate issue on github for each bad model reported so that it's easier to keep track of? Here are the models that still don't work that were reported in this thread:

http://www.thingiverse.com/thingiview:41363

http://www.thingiverse.com/thingiview:42823

http://www.thingiverse.com/thingiview:47031

http://www.thingiverse.com/thing:913

btw, Thanks cbiffle!

@martymcguire
Copy link
Contributor

I'm running into some weird issues deploying these new changes. First, on some models I am seeing the weird lighting, as you suggested might happen.

More importantly - for a lot of models I simply get no geometry at all - the console reports that 0 vertices were parsed. I'll need to set up a better test bed to dig further, but I was wondering if that issue rang any bells.

@tbuser
Copy link
Owner Author

tbuser commented Mar 9, 2011

Are you sure you have the latest changes I made to cbiffle's work? He had forgotten to change all the places where it parses models into an array and I was getting that too. (in the php parser and obj parsing) Are you perhaps caching the results somewhere? If so, it will need to be changed because the parsed array is no longer 3 arrays. The empty normals array has been removed.

@martymcguire
Copy link
Contributor

A new thing that doesn't load right.

http://www.thingiverse.com/thingiview:149426

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

No branches or pull requests

3 participants