BelfrySCAD / BOSL2

The Belfry OpenScad Library, v2.0. An OpenSCAD library of shapes, masks, and manipulators to make working with OpenSCAD easier. BETA
https://github.com/BelfrySCAD/BOSL2/wiki
BSD 2-Clause "Simplified" License
938 stars 111 forks source link

[BUG] path_sweep() weird anchors #1383

Closed lijon closed 5 months ago

lijon commented 7 months ago

Code To Reproduce Bug

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);

//typ = "hull";
typ = "intersect";
path_sweep(s1,p1,atype=typ) {
    show_anchors();

    rainbow([LEFT,RIGHT,TOP,BOT,FRONT,BACK])
        position($item) sphere(2);

}

Expected behavior I would wish to get usable anchors with correct normals, so that children could be positioned for example flush to the ends or sides.

Screenshots For example with "intersect" the RIGHT anchor is nowhere near the actual end of the shape, and none of the anchors are centered on their sides. And there's a bunch of extra anchor arrows drawn:

Screenshot 2024-02-18 at 11 29 29

Here's how "hull" looks, also a bit weird and not very usable.

Screenshot 2024-02-18 at 11 21 53
adrianVmariano commented 7 months ago

The anchor creating for path_sweep treats it as a generic polyhedron. Creating intelligent anchors for an arbitrary shape that we know nothing about is hard to do and yes, the anchors are often not great, especially if the shape isn't convex. If you have an algorithm in mind that you think will be better than the existing algorithms, you can propose it, but otherwise, we don't know how to improve this. You can probably get at least some of the anchors where you want them by fiddling with the centerpoint.

Now for path_sweep we do have some additional information, so I could probably put a named anchor at the centroid of the two end faces in the case that closed=false. Things get rather more complicated if we want more than that, though, as it's not clear how to find other points on the end faces that are natural choices. I suppose anchors could be placed relative to the profile in its native position and then the transformed anchor locations used.

Note that with current anchors, normals are always correct, defined as the normal of the face of the anchor point, or if on a corner or edge, the average normal of the faces that meet at the anchor point.

Also not sure what you mean about "a bunch of extra anchor arrows drawn". There should always be 27, not counting any named anchors.

adrianVmariano commented 7 months ago

It occurs to me that it's not even obvious how to define the size of a roundover for this situation given that the length of the path traced out by the profile can be enormously different for different points on the profile.

lijon commented 7 months ago

It occurs to me that it's not even obvious how to define the size of a roundover for this situation given that the length of the path traced out by the profile can be enormously different for different points on the profile.

I assume this comment was meant for my other issue #1384 ?

The anchor creating for path_sweep treats it as a generic polyhedron. Creating intelligent anchors for an arbitrary shape that we know nothing about is hard to do and yes, the anchors are often not great, especially if the shape isn't convex. If you have an algorithm in mind that you think will be better than the existing algorithms, you can propose it, but otherwise, we don't know how to improve this. You can probably get at least some of the anchors where you want them by fiddling with the centerpoint.

I certainly don't have an algorithm in mind, and I'm not even sure how the algorithm for generic polyhedrons work. But it would be great to be able to position and rotate children along any point of path, especially the ends.

Now for path_sweep we do have some additional information, so I could probably put a named anchor at the centroid of the two end faces in the case that closed=false. Things get rather more complicated if we want more than that, though, as it's not clear how to find other points on the end faces that are natural choices. I suppose anchors could be placed relative to the profile in its native position and then the transformed anchor locations used.

Having anchors at the end faces would be great! For example, how to position a copy of the 2d polygon profile so it fits exactly at the end faces?

Note that with current anchors, normals are always correct, defined as the normal of the face of the anchor point, or if on a corner or edge, the average normal of the faces that meet at the anchor point.

Also not sure what you mean about "a bunch of extra anchor arrows drawn". There should always be 27, not counting any named anchors.

Ok. I'm confused why there was no anchor at the end face, and the top left edge has double anchors for "hull"? And that in "intersect" there was no anchor near the right end, but I understand now that those anchors are just drawn out from the center point until it intersects the outer surface?

lijon commented 7 months ago

Maybe we could have a module to position and rotate children at any point along a path? That could then be used along the extrude path:

function lerp_index(v,x) = let(i=floor(x), a = v[i], b = v[min(i+1,len(v)-1)], f = x-i) lerp(a,b,f);
module pos_along(path,f) {
    i = (len(path)-1)*f;
    a = path_normals(path);
    pos = lerp_index(path,i);
    ang = lerp_index(a,i);
    move(pos) rot(from=DOWN,to=ang) children();
}

Now this only works along the center in this case:

Screenshot 2024-02-20 at 16 45 20

But perhaps the module could implement an attachable() with the anchors of the 2D profile polygon, and apply the position and rotation to get into that position? One could then do something like pos_along(shape, path, 0.5) attach(TOP+RIGHT) children() etc..

(PS. lerp_index() is another function that might be worth including in BOSL2?)

(PS2. another problem: how to chamfer the edges of that hole? :)

adrianVmariano commented 7 months ago

You have reinvented path_copies(), I think.

Because the hole is in a cylindrical ring it can actually be rounded and probably chamfered using bent_cutout_mask().

Have you read the "VNF Attachables" section of the "Attachments Tutorial"?

adrianVmariano commented 7 months ago

Interesting complication with end face anchors: if they point out then you can't just attach the 2d profile polygon at the "start" anchor because it will get flipped around. lerp_index() seems very close to lookup() so not sure about that.

lijon commented 7 months ago

You have reinvented path_copies(), I think.

Oh, will give that a try!

Because the hole is in a cylindrical ring it can actually be rounded and probably chamfered using bent_cutout_mask().

However it's bent on both axis so it's rather a spheric surface.

Would it be possible to project a path onto the surface of a VNF and get the resulting 3D path?

Interesting complication with end face anchors: if they point out then you can't just attach the 2d profile polygon at the "start" anchor because it will get flipped around.

I'm not sure what this means, or why.

lerp_index() seems very close to lookup() so not sure about that.

It's not the same, lookup() wants a table of [key, value] pairs. To get the same functionality, one would first need to process the list like [for(i = [0:len(list)-1]) [i, list[i]]]. lerp_index() is simply indexing an array with fractional index. I find it very useful and have the same function in many other languages.

adrianVmariano commented 7 months ago

Ah, yeah, I forgot about the gentle curve the other way. You can probably get an OK result with bent_cutout_mask if you make the thickness of the mask thick enough so it doesn't cut a groove on the inside.

Yes, that projection operation is possible in principle. There can be failures like a path that isn't connected if the VNF is concave.

image

Above is the result of making anchors point out and then using attach to place the polygon on each end. Note that on the start end, the polygon is flipped. The triangular protrusion is on the wrong side. It would work if I had the "start" anchor pointing toward the interior of the object---backwards.

Yes, lookup isn't exactly the same thing.

lijon commented 7 months ago

Yes, that projection operation is possible in principle. There can be failures like a path that isn't connected if the VNF is concave.

Is there a function for this already? I think it could be useful. Something like path3d = project(path, vnf, direction) (or maybe direction isn't needed)

image

Above is the result of making anchors point out and then using attach to place the polygon on each end. Note that on the start end, the polygon is flipped. The triangular protrusion is on the wrong side. It would work if I had the "start" anchor pointing toward the interior of the object---backwards.

Ah, I see. Wouldn't an orient=DOWN on that end flip the polygon correctly? I think it makes more sense that the anchors point outward, and that the user need to understand that when attaching the same shape polygon as was used for extruding, it naturally must be flipped/mirrored at one end.

adrianVmariano commented 7 months ago

No, there is no projection function written. It's possible in principle, but difficult in practice. I would think you would take the vnf, path, and projection direction. Not sure if you return all connected components or just the closest one? Probably have to return all because closest feels like it might not always be well defined. Feel free to write it. :)

The closest existing code to that kind of projection is in join_prism(), but it's limited to a prism, not an arbitrary VNF.

I think the answer is basically anchor("start",TOP) to position the polygon correctly. So this seems like a working addition.

I'm not sure about adding other anchors that are along the path. There's not an obvious, clean way to do that. Anchor names all need to be text strings computed when the object is made, not when the anchor is used.

adrianVmariano commented 7 months ago

Another question is whether there should be two anchors at each end, one at the origin of the profile and one at the centroid of the profile.

lijon commented 7 months ago

No, there is no projection function written. It's possible in principle, but difficult in practice. I would think you would take the vnf, path, and projection direction. Not sure if you return all connected components or just the closest one? Probably have to return all because closest feels like it might not always be well defined. Feel free to write it. :)

The closest existing code to that kind of projection is in join_prism(), but it's limited to a prism, not an arbitrary VNF.

I see. Unfortunately that's a bit beyond my skills I'm afraid. :) My idea was to return a 3D path of the projection on the closest surface of the VNF, but it could be an option to project it "through" the VNF to get multiple paths, or if there's an easy way to split them afterwards.

I think the answer is basically anchor("start",TOP) to position the polygon correctly. So this seems like a working addition.

I'm not sure about adding other anchors that are along the path. There's not an obvious, clean way to do that. Anchor names all need to be text strings computed when the object is made, not when the anchor is used. Another question is whether there should be two anchors at each end, one at the origin of the profile and one at the centroid of the profile.

The idea was that pos_along() would implement an attachable() forwarding the anchors of the profile, just translated and rotated into place, so whatever anchors available from the profile would be available here as well (including at the ends).

adrianVmariano commented 7 months ago

So the idea is that pos_along() provides anchors to one slice of the swept object. I wonder if there's a reasonable way to do that which doesn't require separate calls. Like path_sweep can provide its transformation list as a $ variable. But it's not clear what would happen next---how it might be used. I mean, it could just be used directly:

path_sweep(....) { matrix_mult($transform[33]) { child object here}}

lijon commented 7 months ago

Yes, pos_along() would take the position as a factor between 0.0 and 1.0, or optionally an actual position (using closest_point or something), to get the corresponding index into $transform and apply it with multmatrix(), which would make the child objects positioned and rotated to the slice origin. From there the user can do position() etc to use the anchors of that slice, if pos_along() also forwards the attachable() stuff like $parent_geom and $parent_size.

So one could do something like pos_along(f=0.5) position(TOP) sphere() to put a sphere at the top of the slice at the center of the swept path. Note that pos_along() should ideally interpolate the transform so we can reach positions in between points of the swept path as well.

adrianVmariano commented 7 months ago

"start" and "end" anchors are now available in path_sweep, as well as $transforms and $scales.

lijon commented 7 months ago

Great! Works fine.

Here's a proof of concept of my pos_along_anchors idea:

include <BOSL2/std.scad>

$fn = 24;

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

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

path_sweep(s1,p1) {
    sweep_pos_anchors(s1, 0.5) attach(TOP) recolor("red") cyl(d1=5,d2=0,h=8,anchor=BOTTOM,orient=BACK);

    sweep_pos_anchors(s1, 0.25) show_anchors();
}

function lerp_index(v,x) = let(i=floor(x), a = v[i], b = v[min(i+1,len(v)-1)], f = x-i) lerp(a,b,f);

module sweep_pos(f) {
    assert(is_def($transforms), "must be called from path_sweep()");
    i = (len($transforms)-1)*f;
    t = lerp_index($transforms, i);
    multmatrix(t) children();
}

module region_anchors(r, cp="centroid", atype="hull") {
    assert(in_list(atype, _ANCHOR_TYPES), "Anchor type must be \"hull\" or \"intersect\"");
    r = force_region(r);
    dummy = assert(is_region(r), "Input is not a region");
    attachable("origin", 0, two_d=true, region=r,extent=atype=="hull", cp=cp) {
      union() {}
      children();
    }
}

module sweep_pos_anchors(profile, f) {
    region_anchors(profile) sweep_pos(f) children();
}
Screenshot 2024-02-27 at 16 00 48

It seems to work fine, but one annoying thing is the need for orient=BACK on the cone child. Any idea how to fix this so that attach() works as expected with default UP orientation of children? I would like the anchors behave like the shown 2D anchors.

adrianVmariano commented 7 months ago

Problem is that the 2d anchors mean things anchor in 2d. That's why orientations are wrong.

module path_sweep_slice(ind){
  multmatrix($transforms[10])
  attachable("origin",0,region=force_region(s1),h=.001){
    union(){}
    children();
  }
}
path_sweep(s1,p1,atype=typ,twist=0){
  path_sweep_slice(10) {
    //attach(LEFT) cyl(d=2,h=10,anchor=BOT);
    //attach(RIGHT) cyl(d=3,h=5,anchor=BOT);
    //attach(RIGHT+TOP) cyl(d=1,h=5,anchor=BOT);
    show_anchors();
  }
  attach("end")anchor_arrow();
}

The above works. Problem is you can't set h=0 to create 3d anchors to a zero height extrusion, so the code needs to be examined to fix that to make this work properly.

Another thing: it's invalid math to lerp between transform matrices. The error is probably small if there's not much rotation between them, but generally not a valid practice.

More seriously, there's a nasty complication having to do with scaling. If scaling is used, the surface isn't parallel to the path, and all the anchor orientations will be wrong. I'm not sure of an easy way to fix that one. It may be easiest to write an entire new pair of attachable methods in order to address this whole situation.

lijon commented 7 months ago

Problem is that the 2d anchors mean things anchor in 2d. That's why orientations are wrong.

module path_sweep_slice(ind){
  multmatrix($transforms[10])
  attachable("origin",0,region=force_region(s1),h=.001){
    union(){}
    children();
  }
}
path_sweep(s1,p1,atype=typ,twist=0){
  path_sweep_slice(10) {
    //attach(LEFT) cyl(d=2,h=10,anchor=BOT);
    //attach(RIGHT) cyl(d=3,h=5,anchor=BOT);
    //attach(RIGHT+TOP) cyl(d=1,h=5,anchor=BOT);
    show_anchors();
  }
  attach("end")anchor_arrow();
}

The above works. Problem is you can't set h=0 to create 3d anchors to a zero height extrusion, so the code needs to be examined to fix that to make this work properly.

Great! It would be nice if it accepted h=0, and then only used the "middle slice" of the anchors (so it would look like the 2D anchors).

One confusing thing is that I need to use attach(BACK) to position this red cone, instead of TOP which would be more intuitive. Is there a way to remap it so it follows the path_sweep() normal?

Screenshot 2024-02-27 at 22 02 44

Another thing: it's invalid math to lerp between transform matrices. The error is probably small if there's not much rotation between them, but generally not a valid practice.

I tried with lots of twist and a stronger curve on p1, and fewer slices (to make the step change bigger) and it looked fine here. But maybe the error is very subtle?

More seriously, there's a nasty complication having to do with scaling. If scaling is used, the surface isn't parallel to the path, and all the anchor orientations will be wrong. I'm not sure of an easy way to fix that one. It may be easiest to write an entire new pair of attachable methods in order to address this whole situation.

I hadn't tried path_sweep(scale=...). Now when I do, the anchors are correctly positioned, but they are also scaled! I thought scaling wouldn't be included in $transforms (since you made a separate $scales):

Screenshot 2024-02-27 at 22 16 36
adrianVmariano commented 7 months ago

A simple example of catastrophic failure of lerping a transformation is to lerp between identity and xrot(180). Right answer is xrot(90). But lerp will give you something rather different.

Maybe you have the base shape rotated differently. I have it like this:

image

in which case the LEFT and RIGHT anchors are going to be aligned with the path_sweep normal. The BACK and FWD anchors will be the other normals. The TOP and BOT anchors will point along the shape inside of it. What remapping of anchors are you suggesting? There is no natural "top" for a path_swept object. Consider the case of something with twist=1200. One could imagine trying to remap TOP and BOT to be the same as FWD and BACK, like happens in 2d.

The reason I pass $scale is because you need it so you can remove the scale from $transforms in order to find the anchors at the ends. And it looks like a similar situation applies here. Maybe it makes sense to instead (or additionally?) provide the unscaled transforms.

Note that in the case of scaled sweeps, the anchors are correctly positioned but their direction is incorrect. It will be normal to the path instead of normal to the swept surface. This complication is basically the blocker at this point for implementing this.

I'm not sure what's gained by going to extra effort to discard the diagonal anchors. Maybe they are sometimes useful? If not....don't use them. Remapping TOP and BOT does mean you can't use them at the end slices.

lijon commented 7 months ago

Oh, yes my base shape is:

Screenshot 2024-02-27 at 23 56 50

So from the POV of the base shape, BACK makes sense of course. But when looking at the sweep it's a bit confusing. I pass normal=UP to path_sweep(), and my sweep path is a path on the XZ plane, so for me it would make sense that TOP is at the top of that path:

Screenshot 2024-02-28 at 00 01 59

Regarding interpolating between transforms. It sounds like it should be possible by decomposing it first: https://stackoverflow.com/questions/3093455/3d-geometry-how-to-interpolate-a-matrix

adrianVmariano commented 7 months ago

See rot_decode and rot_resample.

The problem with saying "this direction was weird" for your case is that if you reorganize things to "make sense" for your case then they won't make sense in some other case and also won't be understandable because an arbitrary modification was applied to rationalize everything for your case. Could be quite common that normals point outwards, for example.

In any case, as I have said, the scale thing is the killer problem for doing this, not the transform interpolation or any other matter. Probably needs an entirely new anchor type written that takes some kind of scale information and the pair of slices at each end, or maybe the transforms for each end. Maybe it's possible to adapt what's there already? But it's decidedly nontrivial.

lijon commented 7 months ago

See rot_decode and rot_resample.

Great! So it should be possible to make a generic lerp_transform(t1, t2, x) that does the right thing? This would also be useful for animations.

The problem with saying "this direction was weird" for your case is that if you reorganize things to "make sense" for your case then they won't make sense in some other case and also won't be understandable because an arbitrary modification was applied to rationalize everything for your case. Could be quite common that normals point outwards, for example.

Understood.

In any case, as I have said, the scale thing is the killer problem for doing this, not the transform interpolation or any other matter. Probably needs an entirely new anchor type written that takes some kind of scale information and the pair of slices at each end, or maybe the transforms for each end. Maybe it's possible to adapt what's there already? But it's decidedly nontrivial.

Would it be an alternative to have an intermediate implementation that works for all the cases where one does not use scaling?

adrianVmariano commented 7 months ago

No, the transform interpolation only works for rotations. I struggled to find a more generic way to interpolate between transformations and was not successful. I wrote code to do eigenvalues in OpenSCAD, thinking that matrix powers would then enable this capability, but it turns out a bunch of common transforms aren't diagonalizable, so it doesn't work for those cases.

The existing method doesn't even work for scaling. It also happens that the "natural" interpolation for scaling may not be what people want---it is exponential rather than linear.

You're suggesting that it gives an error if scale is not 1? Intermediate implementations have a tendency to become the permanent implementations.

lijon commented 7 months ago

Perhaps https://www.euclideanspace.com/maths/geometry/rotations/conversions/matrixToQuaternion or http://callumhay.blogspot.com/2010/10/decomposing-affine-transforms.html could help?

however in this particular case there’s no reason to decompose it, since path_sweep is producing the transforms and could as well save them in separate arrays $sweep_translations, $sweep_rotations, $sweep_scales that can easily be interpolated.

Scales should not be applied to the children, only to the region used to produce the anchors.

adrianVmariano commented 7 months ago

It's trivial to separate translation from other parts of the transform, and since the current code supplies the scales you can back out the unscaled transforms. Note that a rotation and translation combined is actually just a rotation about a point other than the origin. The linked code only handles rotations.