search

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
247 stars 68 forks source link

Mirroring #140

Closed jimfoltz closed 10 years ago

jimfoltz commented 10 years ago

In some cases when a ComponentInstance has been flipped in SketchUp, the order of the exported vertices no longer follow the right hand rule for the facet normal. This may be an issue for some stl importers.

Image 1: Instance in SketchUp with no mirroring. The vertices are written to the STL file using the right-hand-rule.

2014-06_88

In Netfabb at least, the facet is reported as OK.

2014-06_91

Image 2: Shows a Instance in SketchUp which has been Flipped along the Red axis. The vertices are written to the STL file using the left hand rule.

2014-06_89

In NetFabb, the back of the facet is displayed.

2014-06_90

I think this means NetFabb is using vertex order to determine the facet normal rather than using the normal vector given in the STL file for the facet.

All this boils down to checking and manipulating vertex order on export to ensure vertex order and normals follow the right hand rule.

jimfoltz commented 10 years ago

I believe I have a fix to export correct vertex windings for flipped instances.

jimfoltz commented 10 years ago

Whoops - this is a duplicate of #129

thomthom commented 10 years ago

Closing this as a dupe an leave #129 open. Did you have a pull request?

jimfoltz commented 10 years ago

I did not create a pull request - not sure what branch to file it for. This bug is in milestone 2.2, but it seems unlikely to get released anytime soon. This fix should go in the current stable branch as a bug-fix update, and the 2.2 branch would now need rebased on top of the new stable release.

It seems like bug fixes shouldn't get assigned to a milestone, and that milestones should be just for new features rather than bug fixing.

With permanent version branches, I am not sure how to get the fix into both branches. Copy and paste? I fear this might create merge problems in the future. I believe the correct way would be to rebase the 2.2 branch on top of the release branch once the bug fix branch is merged. However, rebasing does not work well on a shared repo because it changes history.

This makes the purpose of the 2.2 branch an integration branch, which is created and recreated (merging in feature branches and testing) until 2.2 is ready and able to cleanly merge into the master (release) branch.

thomthom commented 10 years ago

I agree that this issue should be fixed in 2.1.

Why rebase? Why not just merge 2.1 into 2.2? That's what I do when I maintain my own plugins - for instance Vertex Tools. I apply a bug fix to the released 1.x branch, then merge that into the unreleased 2.0 branch.