xcyl(), ycyl(), zcyl() pass through all parameters to cyl()

[amatulic]

Changes to xcyl(), ycyl(), and zcyl() in shapes3d.scad:

• Now xcyl(), ycyl(), and zcyl() can have textures like cyl(). All three modules now take all the same arguments as cyl() except for orient.
• For compatibility with how these modules worked previously, orient is used to orient the cylinder.
• All three modules now have function counterparts, like cyl().
• Documentation shortened, now explains that these are alias shortcuts for cyl() and refers the reader to the cyl() documentation for usage.

The cuboid() module calls xcyl(), ycyl(), and zcyl(). Tested all examples in cuboid() as well as the original examples in xcyl(), ycyl(), and zcyl().

[amatulic]

All pass-through arguments are now named, for robustness in case the arg list for cyl() ever changes.

Also, the original function version of cyl() had width and height in two different places in the argument list. Fixed.

Re-tested examples in cuboid(), xcyl(), ycycl(), and zcyl(), as well as examples in cyl().

[adrianVmariano]

The docs for xcyl (and I assume others) should state that orient= is not accepted as a difference compared to cyl(). Also another difference compared to cyl(orient=RIGHT) is how the spin parameter behaves. Also you have unnecessary use of "would be" language when talking about how anchors behave. Another way to understand the change here is that the tutorial says anchoring is first, spin second, and orient last. But these modules make it so orient is first.

Lastly, and more trouble, maybe, I think the function forms need to accept anchor= and spin= and should call reorient, which should have the same args as attachable. I'll admit I've never used this feature, but Revar set things up that way so that even the functional forms can be anchored, oriented (though not in this case) and spun.

[amatulic]

I was afraid that nothing is ever simple.

So in order to have function versions of x/y/zcyl(), I would have to do that weird callback technique that is used in squircle() and other places?

[adrianVmariano]

I think you just need to wrap the return value in a reorient() call. Anchoring is very simple here, so nothing more complex should be necessary.

[amatulic]

cyl() has a function form, and the xcyl() function just calls the cyl() function, which already takes anchor, spin, and orient. So why would a call to reorient() be needed?

[adrianVmariano]

Because the whole point of xcyl() is that its anchoring is different. If you just pass anchor, spin and orient then you'll get the cyl() anchoring, where the TOP anchor is on a circular face. The point is that vnf=xcyl(...., anchor=TOP) is supposed to make a cylinder whose round edge is on the X axis. But vnf=cyl(..., anchor=TOP) makes a vnf whose top circle is on the XY plane.

[amatulic]

I don't understand what is expected here.
Consider this, comparing the function with the module.

ydistribute(50) {
    // function form
    vnf_polyhedron(xcyl(l=35, r1=15, r2=5)){
        attach(TOP) anchor_arrow();
        attach(RIGHT) anchor_arrow();
    }
    // module form
    xcyl(l=35, r1=15, r2=5) {
        attach(TOP) anchor_arrow();
        attach(RIGHT) anchor_arrow();
    }
}

It produces this. The function version is in front.

If I add anchor=TOP to both of them, the module anchors on the top right edge of its bounding box and the function anchors on the small circular face, and the anchor arrows don't move with the module. This behavior is unchanged in the module form of xcyl() because I changed nothing except texture parameter pass-through. Honestly the module seems messed up.

I don't see how fiddling with reorient() is going to fix anything. Even if I removed the function versions so it's like it was, the module versions don't seem correct in their behavior.

[adrianVmariano]

If what you're showing up there is the new code then you've evidently introduced a bug in the anchoring. But first of all, some clarification. Anchors are used in two different ways. They are used to position the object, e.g. when you give anchor=RIGHT at object instantiation. They are also used to position children relative to an object, e.g. when you write attach(RIGHT,BOT) something();

The functional form of an object produces a VNF and has no children, so the only way that anchors play a role is in the first fashion, by changing the position of the object. So you cannot test this by creating anchor arrows. When you invoke vnf_polyhedron and create anchor arrows you are now using vnf_polyhedron() to do the anchoring, not xcyl(). And vnf_polyhedron doesn't get any information from xcyl() about where the anchors are; it has to figure it out just from the VNF.

I would also say that if you move objects around it's going to be harder to tell where they were originally anchored, so harder to understand if the anchoring is doing the right thing. So here's a cylinder anchored to its edge:

cyl(d=10,h=25,anchor=RIGHT+FWD+TOP);

If you get the VNF without passing any anchor argument than you get the default center-anchored cylinder:

vnf_polyhedron(cyl(d=10,h=25));

If you pass the anchor= parameter to the functional form then the returned VNF adjusts to reflect the anchoring. (And also orient and spin.)

vnf_polyhedron(cyl(d=10,h=25,anchor=RIGHT+FWD+TOP));

So the test for whether the function is doing the right thing is that if you write a module invocation and then wrap it with vnf_polyhedron, the result is precisely the same.

Now I'm not sure what is going on in your module example above. Looks like a bug in your implementation, because I cannot duplicate it.

xcyl(l=35, r1=15, r2=5, anchor=TOP) {
        attach(TOP) anchor_arrow();
        attach(RIGHT) anchor_arrow();
    }

produces

Perhaps you wrote the code so that the module version calls the function version and you passed the anchor parameter? That would produce this bug, because then it gets anchored twice, once by the module and once by the function. The invocation in the module that is inside attachable() needs to be the unanchored centered version of the shape. I don't see any reason to have the module call the function, just go straight to cyl and pass orient.

[amatulic]

I didn't change the way the xcyl() module works; all I did was make the parameter list complete. It still calls cyl().

Original version:

module xcyl(
    h, r, d, r1, r2, d1, d2, l, 
    chamfer, chamfer1, chamfer2,
    chamfang, chamfang1, chamfang2,
    rounding, rounding1, rounding2,
    circum=false, realign=false, from_end=false, length, height,
    anchor=CENTER, spin=0, orient=UP
) {
    r1 = get_radius(r1=r1, r=r, d1=d1, d=d, dflt=1);
    r2 = get_radius(r1=r2, r=r, d1=d2, d=d, dflt=1);
    l = one_defined([l,h,length,height],"l,h,length,height",1);
    attachable(anchor,spin,orient, r1=r1, r2=r2, l=l, axis=RIGHT) {
        cyl(
            l=l, r1=r1, r2=r2,
            chamfer=chamfer, chamfer1=chamfer1, chamfer2=chamfer2,
            chamfang=chamfang, chamfang1=chamfang1, chamfang2=chamfang2,
            rounding=rounding, rounding1=rounding1, rounding2=rounding2,
            circum=circum, realign=realign, from_end=from_end,
            anchor=CENTER, orient=RIGHT
        );
        children();
    }
}

New version, the only difference is the parameter list, made more complete with the order the same as cyl(). I also removed the orient parameter and hard-coded it in the attachable() call.

module xcyl(
    h, r, center,
    l, r1, r2,
    d, d1, d2,
    length, height,
    chamfer, chamfer1, chamfer2,
    chamfang, chamfang1, chamfang2,
    rounding, rounding1, rounding2,
    circum=false, realign=false, shift=[0,0],
    teardrop=false, clip_angle,
    from_end, from_end1, from_end2,
    texture, tex_size=[5,5], tex_reps, tex_counts,
    tex_inset=false, tex_rot=0,
    tex_scale, tex_depth, tex_samples,
    tex_taper, style, tex_style,
    extra, extra1, extra2, 
    anchor=CENTER, spin=0
) {
    r1 = get_radius(r1=r1, r=r, d1=d1, d=d, dflt=1);
    r2 = get_radius(r1=r2, r=r, d1=d2, d=d, dflt=1);
    l = one_defined([l,h,length,height],"l,h,length,height",1);
    attachable(anchor,spin,orient=UP, r1=r1, r2=r2, l=l, axis=RIGHT) {
        cyl(
            center=center,
            l=l, r1=r1, r2=r2,
            chamfer=chamfer, chamfer1=chamfer1, chamfer2=chamfer2,
            chamfang=chamfang, chamfang1=chamfang1, chamfang2=chamfang2,
            rounding=rounding, rounding1=rounding1, rounding2=rounding2,
            circum=circum, realign=realign, shift=shift,
            teardrop=teardrop, clip_angle=clip_angle,
            from_end=from_end, from_end1=from_end1, from_end2=from_end2,
            texture=texture, tex_size=tex_size, tex_reps=tex_reps, tex_counts=tex_counts,
            tex_inset=tex_inset, tex_rot=tex_rot,
            tex_scale=tex_scale, tex_depth=tex_depth, tex_samples=tex_samples,
            tex_taper=tex_taper, style=style, tex_style=tex_style,
            extra=extra, extra1=extra1, extra2=extra2, 
            anchor=anchor, spin=spin, orient=RIGHT
        );
        children();
    }
}

And yet, if I pass anchor=TOP into the new one, it behaves differently.

[amatulic]

Oh. Hmm. In the original, whatever the user sets for anchor is always overridden by anchor=CENTER in the call to cyl(). I'm passing in the user setting for anchor. Overriding it fixed it. The user value for anchor gets passed into attachable() instead.

[adrianVmariano]

Oh. Hmm. In the original, whatever the user sets for anchor is always overridden by anchor=CENTER in the call to cyl(). I'm passing in the user setting for anchor. Overriding it fixed it. The user value for anchor gets passed into attachable() instead.

Yes, that's exactly the bug I though you had created and described in my previous message. The attachable() call will handle object positioning based on the anchor, spin and usually orient that are passed to attachable. The object as created as the child to attachable must be the native position object, created with orient=UP, anchor=CENTER and spin=0. Otherwise, you'll apply anchoring transformations twice. In this case, of course, orient needs to be RIGHT. But you also propagate the user value for spin. That's also going to cause problems if the user sets spin, with the shape getting spun two times. I think the error there will be subtle, because one of the spins will be around the cylinder axis, so it will change how the facets are aligned.

[amatulic]

OK, it's working for the module with anchor and spin. I noticed the internal function _find_anchor() has a debugging echo of 'cp' in it. It took me a while to track down where that was coming from.

The function version of xcyl still isn't working though. I am calling reorient as
reorient(anchor, spin, UP, vnf=vnf, p=vnf)
passing the user's values for anchor and spin, and vnf is the output of cyl(). I don't know why I have to pass vnf twice but I get an error if I don't.

For this example:

   vnf_polyhedron(xcyl(l=35, r1=15, r2=5, anchor=TOP)){
        attach(TOP) anchor_arrow();
        attach(RIGHT) anchor_arrow();
    }

I get the anchor on the very top of the object, not the top center of the curve like I do with the module version.

I'm inclined to think that's correct for a top anchor of a sideways cone. But it's different from the module:

[adrianVmariano]

Apparently I need to repeat myself. If you anchor a child using vnf_polyhedron the anchors have NOTHING TO DO with the anchors that were computed in the code that made the object. The VNF anchoring code looks at the VNF and tries to fabricate some non-idiotic anchors. It works sometimes. The chance that they are the same anchors produced by the code that compute the object is basically zero unless the object is a cube or axis oriented cylinder. So drawing those anchors is only a potential source of confusion here.

With one exception, reorient should be receiving precisely the same parameters as attachable in the module version so that it does the same anchoring (conoid). That exception is that attachable gets the object to act on as a child, but reorient gets the object to act on as an input VNF in the p= parameter.

Instead of giving the same attachable parameters (radii, length, axis) you seem to be giving it a vnf for the attachment parameters, so you will get VNF anchoring instead of conoid anchoring, which of course will not agree.

[amatulic]

Ah, there we go. Giving reorent() the same parameters as attachable() did the trick. I'll fix up ycyl and zcyl, and commit another change. The next change will include attachments.scad with the echo commented out and a newline on all the assert() messages.

[adrianVmariano]

Is this ready to be merged?

[amatulic]

Wait a bit... It occurred to me that I should test various combinations of anchor with spin and compare the module to the function. I also want to test cuboid() with textures, if applicable, because cuboid() uses the x/y/zcyl() modules.

[adrianVmariano]

Ok. Use a very small $fn value so you can see rotation around the axis.