search

joostn / OpenJsCad

3D solid CAD using only Javascript
315 stars 128 forks source link

Behavior of extrudeInPlane() has been changed recently #69

Closed joostn closed 5 years ago

joostn commented 9 years ago

Some of my designs are broken due to a change in this commit (line 6229): https://github.com/joostn/OpenJsCad/commit/6ccaba0950043e3c19aa5a6175f01e6aaad09e2d

extrudeInPlane used to work symmetrically around the plane, but now it extrudes in one direction. It's not documented so it seems a mistake? Shall we change it back to the previous behavior?

This would mean adding .translate([0, 0, -depth / 2]) here in extrudeInOrthonormalBasis():

var extruded = this.extrude({ offset: [0, 0, depth] }).translate([0, 0, -depth / 2]);

bebbi commented 9 years ago

My apologies for the mistake, this has slipped in from what should have been a separate commit/pull request. In my designs I found it easier as I can place the bottom side and don't have to calc the mid point. Also I oriented myself against user experience of sketchup (pull square->cube) I prefer it but I'll make a pr to reverse the change if you like the other approach better - sorry again

On 21.07.2015, at 10:15, joostn notifications@github.com wrote:

Some of my designs are broken due to a change in this commit (line 6229): 6ccaba0

extrudeInPlane used to work symmetrically around the plane, but now it extrudes in one direction. It's not documented so it seems a mistake? Shall we change it back to the previous behavior?

This would mean adding .translate([0, 0, -depth / 2]) here in extrudeInOrthonormalBasis():

var extruded = this.extrude({ offset: [0, 0, depth] }).translate([0, 0, -depth / 2]);

— Reply to this email directly or view it on GitHub.

joostn commented 9 years ago

Well yeah maybe we should keep it like this, I find myself doing most extrusions on top of a plane too.

joostn commented 9 years ago

I'll make PR to update the docs.