tbuser / thingiview.js

Javascript 3D Model Viewer
http://replimat.com/thingiview/examples
GNU Lesser General Public License v3.0
399 stars 110 forks source link

A Thing that doesn't load right #4

Open tbuser opened 13 years ago

tbuser commented 13 years ago

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

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

martymcguire commented 13 years ago

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 commented 13 years ago

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

cbiffle commented 13 years ago

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 commented 13 years ago

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 commented 13 years ago

More things reported by Thingiverse users:

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

cbiffle commented 13 years ago

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 commented 13 years ago

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 commented 13 years ago

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 commented 13 years ago

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

martymcguire commented 13 years ago

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 commented 13 years ago

Another one that seems to fail with flipped normals:

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

tbuser commented 13 years ago

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 commented 13 years ago

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 commented 13 years ago

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 commented 12 years ago

A new thing that doesn't load right.

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