-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
New parameter "v" for rotate_extrude #5110
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't look into the actual parameters yet, just commented on some surrounding code.
src/geometry/GeometryEvaluator.cc
Outdated
|
||
bool flip_faces = (min_x >= 0 && node.angle > 0 && node.angle != 360) || (min_x < 0 && (node.angle < 0 || node.angle == 360)); | ||
double fact=(node.v[2]/node.angle)*(180.0/G_PI); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
G_PI: That's the first time I've seen that. No idea where that's coming from, but it feels wrong.
I think the cleanest way to achieve this prior to C++20 is:
#define _USE_MATH_DEFINES
#include <cmath>
translate([80,-40,0]) | ||
difference(){ | ||
cylinder(r=5,h=20); | ||
rotate_extrude(angle=180*5,v=[0,0,20]) | ||
translate([5,0]) circle(2,$fn=20); | ||
} | ||
|
||
translate([80,0,0]) | ||
rotate_extrude(angle=180*5,v=[10,-10,30],method="linear") | ||
translate([5,0]) circle(4,$fn=20); | ||
|
||
translate([80,40,0]) | ||
rotate_extrude(angle=270,v=[0,0,40]) | ||
translate([5,0]) circle(4,$fn=20); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is getting too crowded, let's start a new file for this new feature.
src/geometry/GeometryEvaluator.cc
Outdated
@@ -1330,6 +1334,60 @@ static std::unique_ptr<Geometry> rotatePolygon(const RotateExtrudeNode& node, co | |||
return builder.build(); | |||
} | |||
|
|||
static std::shared_ptr<const Geometry> rotatePolygon(const RotateExtrudeNode& node, const Polygon2d& poly){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #5096 - I suggest pulling out all the rotate_extrude tools into a new file, as this file is getting a bit hard to reason about.
I understand the desire for introducing vertical offset to rotate_extrude, to allow for much simpler creation of threads and springs etc. I am starting to think that these sort of cases ( I think the existing |
One benefit I can think of is that if we know the shear in advance, we can tessellate slightly better. |
The parameter "v" in rotate_extrude, similar to linear_extrude defines an extra linear direction of extruion which makes it easy to design helical screws and other fancy stuff.
An extra parameter method, which defaults to "center" defines the per-point elevation depending on the rotated angle. With "center" the center of gravity is calculated on the 2 objects, wherease for setting " linear" steepnees decreases 1/x from the v axis outwards
"center" looks better whereas "linear" is more natural.