SketchUp / sketchup-stl

A SketchUp Ruby Extension that adds STL (STereoLithography) file format import and export.
http://extensions.sketchup.com/content/sketchup-stl
MIT License
248 stars 68 forks source link

fixes #59 is_solid? now works with entities collection #64

Closed jimfoltz closed 11 years ago

jimfoltz commented 11 years ago

Conflicts: src/sketchup-stl/importer.rb

scottlininger commented 11 years ago

Looks good to me. Any comments from the peanut gallery before I merge it?

jimfoltz commented 11 years ago

actually, is_solid? can never return a positive the way it is written...

thomthom commented 11 years ago

Anyone had time to test this? (I've been too busy with deadlines.)

thomthom commented 11 years ago

I noticed one thing:

When importing geometry when there is existing geometry, so that the imported geometry ends up in a group, then the view doesn't properly zoom around the imported geometry.

thomthom commented 11 years ago

I tried to have quick look at the zoom code - but wasn't able to work out what was going on.

Anyone else had a chance?

jimfoltz commented 11 years ago

When importing geometry when there is existing geometry, so that the imported geometry ends up in a group, then the view doesn't properly zoom around the imported geometry.

No idea, it works for me.

thomthom commented 11 years ago

@jimfoltz Do you not get the same behaviour I get: http://youtu.be/pUwRZdz2sRc

jimfoltz commented 11 years ago

Sorry Tom, you're right. Pushed an update.

thomthom commented 11 years ago

That looks to be working now. :+1:

But can yo push a latest change - it's just a minor one that doesn't affect functionality, but I found it read easier - at line 98: model.active_view.zoom(container.entities.to_a) to model.active_view.zoom(entities.to_a)

?

thomthom commented 11 years ago

Never mind - I'll just merge it and use GitHub's edit feature to fix it.