Skip to content
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

end rounding/chamfer for path_sweep() and path_sweep2d() #1384

Open
lijon opened this issue Feb 18, 2024 · 18 comments
Open

end rounding/chamfer for path_sweep() and path_sweep2d() #1384

lijon opened this issue Feb 18, 2024 · 18 comments
Assignees

Comments

@lijon
Copy link

lijon commented Feb 18, 2024

Is your feature request related to a problem? Please describe.
path_sweep(2d) is great to extrude complex curved shapes. But while the extruded shape (polygon) can easily be rounded, there's no way I can think of to round the ends.

Screenshot 2024-02-18 at 11 41 28

Describe the solution you'd like
It would be great to have start and end rounding/chamfers like in offset_sweep().

Describe alternatives you've considered

  • Using skin() and manually calculating the shape and position of each segment along the path. Very tedious, and maybe how this feature would be implemented inside path_sweep()?
  • extruding a rounding mask along the shape polygon. But I'm not sure how to attach it at the correct position and rotation.

Example Code

function a(h) = arc(points=[[-20,0],[0,h],[20,0]],n=24);
s1 = round_corners(flatten([
    a(1), // bottom
    move([0,5],reverse(a(3))) // top
]),r=1);

p1 = xrot(90,path3d(arc(points=[[-40,0],[0,5],[40,-10]],n=24)));

//!region(s1);
//!stroke(p1);

path_sweep(s1,p1);
@adrianVmariano
Copy link
Collaborator

Note that path_sweep will give you the transform list so that you can do something like this. If you have low standard for your rounding you could do it by applying a scale factor to the transform list and calling sweep with the modified transform list. If you want to do it right you'll need to use a series of offset() calculations on your profile. Also you'll need to sample your path so that there are a bunch of tightly sampled points at each end to adequately define the rounding. Or you'd need a superfine sampling everywhere. I am not seeing a good way to do this automatically. In your example above, the entire rounding looks to me like it should fit into one sample point, for example. So anyways, to do it this way, you'd generate your path list of the unrounded part by applying the transforms to the profile. You'd generate offset profiles for the rounded parts and apply the transforms to those and then invoke probably vnf_vertex_array, ideally, if you are able to keep the point count the same. (Otherwise you'd need to use skin.)

I suppose s simpler scheme might be to assume the rounding is short relative to curvature of the path. Then in that case you might assume that the profiles are centered on the vector defined by the end face. Then the end rounding could be computed independently from the path sweep transformations. This would probably look wrong if twist was nonzero but would be simpler to do generically.

Another option to make the shape you're constructing would be to make a rounded shape based on your profile using offset_sweep (which does all the complex stuff with offsets) and then use vnf_bend to curve it. Also for rounding a single polyhedron minkowski() might be feasible.

@lijon
Copy link
Author

lijon commented Feb 20, 2024

Note that path_sweep will give you the transform list so that you can do something like this. If you have low standard for your rounding you could do it by applying a scale factor to the transform list and calling sweep with the modified transform list. If you want to do it right you'll need to use a series of offset() calculations on your profile. Also you'll need to sample your path so that there are a bunch of tightly sampled points at each end to adequately define the rounding. Or you'd need a superfine sampling everywhere. I am not seeing a good way to do this automatically. In your example above, the entire rounding looks to me like it should fit into one sample point, for example. So anyways, to do it this way, you'd generate your path list of the unrounded part by applying the transforms to the profile. You'd generate offset profiles for the rounded parts and apply the transforms to those and then invoke probably vnf_vertex_array, ideally, if you are able to keep the point count the same. (Otherwise you'd need to use skin.)

Which all sounds very complicated and the reason I'm making this feature request :) My hope was that if it could be done in offset_sweep(), it could be done here as well. (Or alternatively, add the ability to provide a path for offset_sweep)

I suppose s simpler scheme might be to assume the rounding is short relative to curvature of the path. Then in that case you might assume that the profiles are centered on the vector defined by the end face. Then the end rounding could be computed independently from the path sweep transformations. This would probably look wrong if twist was nonzero but would be simpler to do generically.

That sounds like a good idea to me! The common case is to just get rid of the sharp corners.

Another option to make the shape you're constructing would be to make a rounded shape based on your profile using offset_sweep (which does all the complex stuff with offsets) and then use vnf_bend to curve it. Also for rounding a single polyhedron minkowski() might be feasible.

Yes, I suppose in this case two vnf_bends (one around X and one around Y) would do it, but this arc path was just an example and not necessarily exactly the shape one would want. I'm looking for a more generic way to round the end face edges when using path_sweep().

@lijon
Copy link
Author

lijon commented Feb 20, 2024

(This is one case where I'm a bit envious of other CAD programs where you could just select the edges and click a button to get them chamfered!)

@lijon
Copy link
Author

lijon commented Feb 20, 2024

Here's a try with path-extruding a rounding mask around the end faces. It uses the normals at the endpoints of the path for correct rotation:

function a(h) = arc(points=[[-20,0],[0,h],[20,0]],n=24);
s1 = round_path(flatten([
    a(2), // bottom
    move([0,6],reverse(a(4))) // top
]),1);

p1 = xrot(90,path3d(arc(points=[[-40,0],[0,5],[40,-10]],n=36)));

module round_sweep_ends(shape,path,r) {
    for(i=[0,len(path)-1])
        move(path[i])
            rot(from=RIGHT,to=path_normals(path)[i])
                tag("remove")
                    mirror([0,0,i==0?0:1]) path_sweep(mask2d_roundover(r=r,$fn=36,excess=1),shape,closed=true,spin=90);

}

recolor("white") diff()
path_sweep(s1,p1) {
    round_sweep_ends(s1,p1,r=1);
}

It works for this particular case:
Screenshot 2024-02-20 at 15 34 58

But only if p1 is on the XZ plane, etc. The hardcoded spin=90 and rot(from=RIGHT) is probably not the correct way to do it.. Perhaps this method could be improved to be more generalized? Either built into path_sweep(), or as a separate module used as a child. In that case, maybe path_sweep() could export its shape and path via $variables to be picked up by round_sweep_ends().

@adrianVmariano
Copy link
Collaborator

It sort of works, but it's a hack job. It you study the underside where it is concave, you'll see that the roundover is too large and creates a lip rather than blending smoothly with the shape. (It's easier to see if you make the curve sharper or the roundover bigger.)

If you want to approach rounding with a mask then the mask has to take into account the curvature of the path all the way to the base of the mask. In the context of polyhedra I always think of rounding in a positive rather than negative fashion because I find it to be much simpler---construct one object rather than two objects---so I would approach this by creating a rounded cap to apply to the end. You could actually do this with offset_sweep. It will look OK if the height of the rounded cap is small relative to the curvature of the shape at the end. But it's not the fully correct answer because the cap doesn't respect the curvature or twist.

Note also that you only need one vnf_bend to do this, not two. You still sweep the curved profile, just linearly using offset_sweep which can put roundovers on things. But the vnf_bend approach is certainly not generic.

@lijon
Copy link
Author

lijon commented Feb 21, 2024

It sort of works, but it's a hack job. It you study the underside where it is concave, you'll see that the roundover is too large and creates a lip rather than blending smoothly with the shape. (It's easier to see if you make the curve sharper or the roundover bigger.)

I see!

If you want to approach rounding with a mask then the mask has to take into account the curvature of the path all the way to the base of the mask. In the context of polyhedra I always think of rounding in a positive rather than negative fashion because I find it to be much simpler---construct one object rather than two objects---so I would approach this by creating a rounded cap to apply to the end. You could actually do this with offset_sweep. It will look OK if the height of the rounded cap is small relative to the curvature of the shape at the end. But it's not the fully correct answer because the cap doesn't respect the curvature or twist.

So to make that generic, one would shorten the path at the ends, and put two offset_sweeps() there instead.

But to do it correctly, what would be the best approach? Maybe:

  • Get the list of transforms from path_sweep()
  • Process this into a list of 3D-transformed shape paths, where we call offset() on a number of the start and end segments, inserting slices if needed.
  • Feed this list of paths into skin()

Note also that you only need one vnf_bend to do this, not two. You still sweep the curved profile, just linearly using offset_sweep which can put roundovers on things. But the vnf_bend approach is certainly not generic.

That's true!

@adrianVmariano
Copy link
Collaborator

The strategy you describe is the harder version of what I suggested, namely adding caps created with offset_sweep. And it doesn't work any better, unless you devise some very clever nonlinear interpolation method for inserting slices. Though suppose if you use the transformation interpolation code to interpolate between the transformations it might produce a better result. But I think the centerpoint will still travel linearly rather than following the curve.

For a truly ideal result you need the user, who knows what the curve is, to subsample it at the ends.

You would use vnf_vertex_array() rather than skin.

@lijon
Copy link
Author

lijon commented Feb 21, 2024

The strategy you describe is the harder version of what I suggested, namely adding caps created with offset_sweep. And it doesn't work any better, unless you devise some very clever nonlinear interpolation method for inserting slices. Though suppose if you use the transformation interpolation code to interpolate between the transformations it might produce a better result. But I think the centerpoint will still travel linearly rather than following the curve.

For a truly ideal result you need the user, who knows what the curve is, to subsample it at the ends.

Yes the idea was to interpolate between points of the path as well as between the transforms of those points.
Regarding subsampling at the ends, I've no idea how that works, but isn't offset_sweep doing something like this already?

You would use vnf_vertex_array() rather than skin.

👍

@adrianVmariano
Copy link
Collaborator

Interpolating between transforms is nontrivial, but there is code to do it. Yes, offset_sweep resamples at the ends. It has a huge advantage: the "curve" is linear which means that linear sampling is exactly correct. That's probably not the case for path_sweep.

There is an issue I hadn't thought of which is that the normals are only correct at the path. This can lead to a mismatch (step) using the simple applied-endcap scheme. In this example the twist makes everything more extreme:

image

The lip results from the small excess length I added to ensure overlap in the union, without which I got CGAL errors. Without that excess there is no lip, but the joint is not tangent. This suggests that to make this work robustly it can't be done by unioning on endcaps, but that instead the polyhedron needs to be joined. Maybe need an option to offset_sweep to omit the end face(s). That would give a result that isn't always tangent, and might look funny in some cases, but at least should always be valid.

It also occurs to me that a complication with the fully correct approach is handling added points in the offset. (Subtracted points can be handled but not added ones.) You could use skin to align the extra points, but it might make a poor choice for how to connect the added points and it might be slow(er). To do it robustly it would be necessary to use the faces feature of offset and actually construct the polyhedron from that---basically copy the code from offset_sweep. This would then be joined (without the bottom face) to the polyhedron produced by the sweep operation.

@adrianVmariano
Copy link
Collaborator

I have a method to do this where the incorrect approach is correctly implement so that there is no step. The code looks like this:

T=path_sweep(s1,p1,atype=typ,twist=44,transforms=true,scale=1);
end = apply(last(T),offset_sweep(s1,top=os_circle(r=1),steps=8,caps=[false,true]));
vnf_polyhedron(vnf_join([sweep(clockwise_polygon(s1),T,caps=[true,false]), end]));

I'm not sure where to go from here. I could add a simple version of this to path_sweep (accept end rounding simple radii only). However, adding this to path_sweep seems problematic because Revar doesn't want rounding.scad to be in the core, but skin.scad is in the core.

image

There's a rounded end on a twisted sweep, similar to the failure shown above. No step this time. Basically everything has to be done in polyhedron land by generating polyhedra with path_sweep and offset_sweep and joining them into one. Note that above code won't run until my offset_sweep mods are merged, because offset_sweep couldn't produce output with no end faces.

@lijon
Copy link
Author

lijon commented Feb 27, 2024

Nice! Would love to see this merged, whichever file it ends up in :)
But please allow to pass the os_profile so it's not limited to round only, but also chamfer etc.

In the meanwhile I tried your suggestion of appending a short offset_sweep() segment at the ends:

module extend_with_chamfer(shape,w=1) {
    z = abs(w);
    T = select($transforms,[0,-1]);
    a = [-1,1];
    for(i=[0,1])
        multmatrix(T[i]) up(z/2 * a[i]) offset_sweep(shape,h=z,bottom=os_chamfer(w),orient=[0,0,-a[i]]);
}

which works quite well (if one is accepting that the rounded/chamfered segment is linear):

Screenshot 2024-02-27 at 23 24 14

But then one would need to shorten the extrude path in equal amount so the full shape keeps its length..

@adrianVmariano
Copy link
Collaborator

Note that my above solution is the same as yours. It lengthens the path. User will have to make the path shorter if necessary. And it creates the rounding as a linear extension, not properly following the curvature.

Adding the offset_sweep as you do above may work---see if it passes F6---with the previous caveat that it can create a lip or poor join in various situations. (To pass F6 I had to use the extra parameter, which is what creates the lip.)

The concern about passing os_profile is that it's very complicated and adds a zillion options to path_sweep.

@lijon
Copy link
Author

lijon commented Feb 28, 2024

Note that my above solution is the same as yours. It lengthens the path. User will have to make the path shorter if necessary. And it creates the rounding as a linear extension, not properly following the curvature.

Right. But your version is probably better because it ensures the added segment is joined in the VNF itself, while my method gives the risk that the segment is not fully merged with the rest.

Regarding path length, I think it would make sense that when adding end treatments it would automatically shorten the path as much as needed. The docs should be clear that the end treatments are linearly following the tangent of the last path segments, which is not an issue as long as the rounding is short compared to the thickness of the profile slices.

Adding the offset_sweep as you do above may work---see if it passes F6---with the previous caveat that it can create a lip or poor join in various situations. (To pass F6 I had to use the extra parameter, which is what creates the lip.)

It does pass F6 (render) without any extra for me.

The concern about passing os_profile is that it's very complicated and adds a zillion options to path_sweep.

I was thinking just path_sweep(..., start=os_circle(2), end=os_chamfer(1.5)) and it would just pass them on to offset_sweep? An alternative would be rounding1, rounding2, chamfer1, chamfer2 as in cyl().

@adrianVmariano
Copy link
Collaborator

The changes were merged so you can try my code that does the single polyhedron.

I'm not sure about shortening vs lengthening. Shortening means you may cut off part of the specified path and then add a completely different extension of that path. I wonder if it would be workable to pass a transform list to offset_sweep. This is sort of complicated because either the list needs to match the sampling that offset_sweep will use, or offset_sweep has to do transform resampling. I guess, the biggest complication would be if new samples need to be added in the middle (straight) section of the offset_sweep.

I was thinking that support would be more like rounding, chamfer and teardrop than allowing full os_profiles. The docs for path_sweep are already overwhelming long and adding the additional explanation of those profiles there and that whole system of machinery is even more complication.

I think in order to do this, Revar needs to agree to move rounding.scad into the core. We haven't managed to connect to discuss this possibility yet.

@adrianVmariano
Copy link
Collaborator

Revar has agreed that rounding.scad can join the core. But he favored (along with you) the idea that we cut back the path by the rounding radius. This turns out be a major complication and may require a substantial reworking of how data flows through the code. Specifically, the problem has to do with applied twist. In interpolating between frames we need to correctly interpolate twist, which needs to be done based on rotation angle, but the interpolation of other rotation between adjacent slices should be linear, because they are connected linearly in the model. That means that the rotation needs to be partitioned into its twist portion and non-twist portion, which I am not sure is possible after the fact. So that means the code needs to be rewritten to keep twist separate from the rest of the transformation.

@adrianVmariano
Copy link
Collaborator

It occurs to me that the interpolated profile will not be congruent to the profile specified for the sweep. Not sure if that's a concern or not.

@lijon
Copy link
Author

lijon commented Mar 21, 2024

Perhaps an option to trim the path or not? I don't think I fully understand the implications and difficulties, but I guess the trimmed path would be used only for the actual generated faces (and then combined with the rounded/chamfered end caps), while the whole untrimmed path would be used for the transforms. So it would do everything as if the path was not trimmed, except it would leave out the last segment on both ends, and instead add the rounded end caps there, using the same transforms as the original end segments would have.

It occurs to me that the interpolated profile will not be congruent to the profile specified for the sweep. Not sure if that's a concern or not.

How do you mean?

@adrianVmariano
Copy link
Collaborator

Well, option to trim is like worst of both worlds from an implementation perspective.

So how does trimming work? You have requested a radius r rounding. So you remove length r from the path. There are two cases, either the last segment has length>r or it does not. Consider the first case. We need to remove a partial segment. The last section of the path_sweep is defined by the two final profiles and it is constructed by linear interpolation between those profiles. We therefore should perform a linear interpolation (lerp) operation between the profiles in the proportion of r divided by the length of the last segment. This linear interpolation will produce a distorted profile---it will be different than the original profile except in the special case where the two profiles are both perpendicular to the last path segment. In this special case, the cap will be built on the original user-given profile and it will have its flat surface coplanar with the original terminal face of the path_sweep. But in ithe general case, neither of those things is true. The cap's top surface won't be in the same plane as the original surface, and the cap will be based on a profile that is different from the user specified profile---the interpolation between two.

So what happens if the final segment length of the path is less than r? In that case you have to cut back many segments. Actually now that I think this through carefully, it seems like linear interpolation is the correct approach even in this situation, so the complexity may be less than I thought. The twist and scale will have been computed at each path sample point and connected linearly when making the sweep. So the main point of confusion really is just that adding rounding changes the location of the final surface, which is arguably unexpected. This change can be quite large if the profile is big and the curvature is large. It's unclear to me if having the rounding based on the interpolated profile causes any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants