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

A way to reset the current transform #1372

Open
lijon opened this issue Feb 8, 2024 · 34 comments
Open

A way to reset the current transform #1372

lijon opened this issue Feb 8, 2024 · 34 comments
Assignees
Labels
Defer Deferred until after the next major release.

Comments

@lijon
Copy link

lijon commented Feb 8, 2024

See openscad/openscad#4975

It was suggested that I add an issue also here.

In short, I'd like to have a way to reset the current transform:

translate(A) {
  reset_transform() translate(B) cube(C); // translated by B only, because A is "undone" by the reset
  cube(D); // translated by A as usual
}

The use case is that in my box enclosure library I'd like to add support for auxillary parts (such as button extenders, battery hatches, etc), that can be viewed in place in an assembly but also laid out and oriented separatedly for 3d printing.

The question is if this is something that could be implemented in BOSL2? One idea would be to have a $no_transform flag that if set, makes all the BOSL2 transform wrappers do nothing. But that wouldn't work, and the above example can't be implemented, since the script is parsed top-down while the transforms are made bottom-up. The no-transform would have to block all transforms further up the tree.

@lijon
Copy link
Author

lijon commented Feb 8, 2024

There is one way it could be implemented in BOSL2, by rewriting all the transform modules to keep track of the current (inverse) matrix in a $variable: openscad/openscad#4975 (comment)

One could then have a reset_transform() that would apply the inverse matrix to undo all transforms, and reset the $variable to identity.

One might even take a snapshot of the current matrix: $my_snapshot = $inverse_matrix and then pass it as an arg to reset_transform($my_snapshot) later down the chain to restore the current matrix at that point?

@adrianVmariano
Copy link
Collaborator

I think the implementation idea you propose is basically how it would need to be done. All the transform operators would accumulate their effect (or inverse?) in some global like $global_transform and you could then do whatever you wanted with it, e.g. multmatrix(matrix_inverse($global_transform)). It seems like this would be straight forward to do, but would be a lot of work since all transform modules would need to be modified.

I have questions about whether accumulating the transform might have broader applicability as you are thinking with the snapshot idea. But it wouldn't work like you think. It would be multmatrix($my_snapshot*matrix_inverse($global_transform)) because you have to first go back to the global reference and then go to the snapshot reference.

If instead of accumulating a matrix you accumulated a list of transforms then it would be possible to snapshot by index and do something like multmatrix(matrix_inverse(cumprod(select($transform_list,$snapshot_index,-1)))))

@adrianVmariano
Copy link
Collaborator

We'd also have to override multmatrix so that it would add its argument to the chain. Hmmm. Actually I wonder if just doing that would take care of most of the cases. That would make it much easier.

@revarbat
Copy link
Collaborator

revarbat commented Feb 9, 2024

I implemented basically exactly this a couple years ago. It's completely doable, mostly by overriding multmatrix(). I'll dig for my reference file for this.

@adrianVmariano
Copy link
Collaborator

So I took a quick look through transforms.scad and I think the following steps are needed:

  1. Override multmatrix() to set the accumulated variable(s). Not sure if it would be the accumulated total transform or the list of all transforms or both
  2. Modify all modules in transforms.scad that don't use multmatrix() to work by calling the function form and using multmatrix instead of what they currently do.
  3. Override translate, rotate, scale and mirror

@lijon
Copy link
Author

lijon commented Feb 9, 2024

Wouldn’t it be enough to just override the built-in multmatrix, translate, rotate, scale and mirror? All the others are using one of these.

You’re right the snapshot idea wouldn’t work as I thought, but also I’m not sure I see the use case of being able to rewind the transforms to different snapshots. I think it would be enough with a transform_save() that sets $global_matrix=identity, and transform_restore() that would apply $global_matrix and then transform_save(). So one would be able to restore the transform back to the nearest transform_save() up the tree.

Just saving the accumulated inverted transforms would be enough I think, and have better precision than using matrix_inverse()

@revarbat
Copy link
Collaborator

revarbat commented Feb 9, 2024

Here's the (untested) code I posted to the OpenSCAD list back in 2020:

This seems to work:

$local_matrix = undef;

function _wc_transpose(m) = [
    for (c=[0:3]) [
        for (r=[0:3]) m[r][c]
    ]
];

function _wc_minor(m,r,c) = [
    for (rr=[0:1:len(m)-1])
    if (rr != r) [
        for (cc=[0:1:len(m[rr])-1])
        if (cc != c) 
        m[rr][cc]
    ]
];

function _wc_sum(v,_total=0,_i=0) =
    _i>=len(v) ? _total :
    _wc_sum(v, _total+v[_i], _i+1);

function _wc_determinant(m) =
    len(m)==2? m[0][0] * m[1][1] - m[0][1] * m[1][0] :
    _wc_sum([
        for (c = [0:1:len(m[0])-1])
        ((c%2==0)? 1 : -1) * m[0][c] *
        _wc_determinant(_wc_minor(m,0,c))
    ]);

function _wc_inverse(m) =
    let(
        det = _wc_determinant(m),
        inv = _wc_transpose([
            for (r = [0:3]) [
                for (c = [0:3])
                ((r+c) % 2 == 0? 1 : -1) *
                _wc_determinant(_wc_minor(m,r,c))
            ]
        ]) / det
    ) inv;


function _wc_ident() = [
	[1, 0, 0, 0],
	[0, 1, 0, 0],
	[0, 0, 1, 0],
	[0, 0, 0, 1]
];

function _wc_translate(v) = [
    [1, 0, 0, v.x],
    [0, 1, 0, v.y],
    [0, 0, 1, v.z],
    [0 ,0, 0,   1]
];

function _wc_scale(v) = [
    [v.x,   0,   0, 0],
    [  0, v.y,   0, 0],
    [  0,   0, v.z, 0],
    [  0,   0,   0, 1]
];

function _wc_rotx(ang) = [
    [1,        0,         0,   0],
    [0, cos(ang), -sin(ang),   0],
    [0, sin(ang),  cos(ang),   0],
    [0,        0,         0,   1]
];

function _wc_roty(ang) = [
    [ cos(ang), 0, sin(ang),   0],
    [        0, 1,        0,   0],
    [-sin(ang), 0, cos(ang),   0],
    [        0, 0,        0,   1]
];

function _wc_rotz(ang) = [
    [cos(ang), -sin(ang), 0, 0],
    [sin(ang),  cos(ang), 0, 0],
    [       0,         0, 1, 0],
    [       0,         0, 0, 1]
];

function _wc_rot_by_axis(u, ang) =
    ang==0? _wc_ident() :
    let(
        u = u/norm(u),
        c = cos(ang),
        c2 = 1-c,
        s = sin(ang)
    ) [
        [u.x*u.x*c2+c    , u.x*u.y*c2-u.z*s, u.x*u.z*c2+u.y*s, 0],
        [u.y*u.x*c2+u.z*s, u.y*u.y*c2+c    , u.y*u.z*c2-u.x*s, 0],
        [u.z*u.x*c2-u.y*s, u.z*u.y*c2+u.x*s, u.z*u.z*c2+c    , 0],
        [               0,                0,                0, 1]
    ];

function _wc_vec_angle(v1,v2) =
    let(
        n0 = norm(v1),
        n1 = norm(v2)
    )
    assert(n0>0 && n1>0, "Zero length vector.")
    let (
        c1 = (v1*v2)/(n0*n1),
        c2 = min(max(c1,-1),1) // Correct for FP rounding errors.
    ) acos(c2);
 
function _wc_to3d(v) = [for (i=[0:2]) i<len(v)? v[i] : 0];

function _wc_vec_axis(v1,v2) =
        let(
          eps = 1e-6,
          w1 = _wc_to3d(v1/norm(v1)),
          w2 = _wc_to3d(v2/norm(v2)),
          w3 = (norm(w1-w2) > eps && norm(w1+w2) > eps) ? w2 
               : (norm([abs(w2.x),abs(w2.y),abs(w2.z)]-[0,0,1]) > eps)? [0,0,1] 
               : [1,0,0],
          x = cross(w1,w3)
        ) x/norm(x);


module translate(v) {
    multmat(_wc_translate(v)) children();
}


module scale(v) {
    multmat(_wc_scale(v)) children();
}


module rotate(a=0, v) {
    mat = is_undef(v)? (
        is_list(a)? (
            _wc_rotx(len(a)>=1? assert(is_num(a.x)) a.x : 0) *
            _wc_roty(len(a)>=2? assert(is_num(a.y)) a.y : 0) *
            _wc_rotz(len(a)>=3? assert(is_num(a.z)) a.z : 0)
        ) : (
            assert(is_num(a)) _wc_rotz(a)
        )
    ) : (
        assert(is_num(a))
        assert(is_list(v))
        let( v = [ for (i=[0:2]) v[i] ] )
        _wc_rot_by_axis(v, a)
    );
    multmat(mat) children();
}


module multmat(mat) {
    $local_matrix = is_undef($local_matrix)? mat : ($local_matrix * mat);
    multmatrix(mat) children();
}


// Returns the local transformation matrix.
function local_matrix() =
    is_undef($local_matrix)? _wc_ident() :
    $local_matrix;


// Returns the local translation vector [X,Y,Z]
function local_translation() =
    is_undef($local_matrix)? [0,0,0] :
    let(
        vec = [0, 0, 0, 1],
        lvec = $local_matrix * vec
    ) _wc_to3d(lvec);


// If mat is undef, resets to the world reference frame.
// If given a matrix in mat, resets to that reference frame.
module reference_frame(mat) {
    if (is_undef(mat)) {
        multmat(_wc_inverse($local_matrix)) children();
    } else {
        multmat(_wc_inverse(mat)) children();
    }
}

translate([30,40,50]) {
    scale([2,3,4]) {
        rotate(30,v=[1,1,0]) {
            cylinder(d1=5,d2=0,h=5);
            reference_frame() {
                cylinder(d1=5,d2=0,h=5);  // This gets rendered in the world reference frame
            }
        }
    }
}

You should be able to also store local reference frames in the middle there, in $special_vars and pass those to reference_frame() to access arbitrary reference frames. You just have to make sure that your code never calls multmatrix(), but multmat() instead.

  • Revar

@revarbat
Copy link
Collaborator

revarbat commented Feb 9, 2024

I think that code could be implemented much more simply with modern BOSL2.

@lijon
Copy link
Author

lijon commented Feb 9, 2024

Ok, it seems this actually works and covers all the cases? :)

reset_transform.scad:

use <BOSL2/builtins.scad>
include <BOSL2/std.scad>

$_matrix = IDENT;

module translate(v) {
    $_matrix = $_matrix * translate(v);
    _translate(v) children();
}

module rotate(a,v) {
    $_matrix = $_matrix * rot(a=a,v=v);
    _rotate(a,v) children();
}

module scale(v) {
    $_matrix = $_matrix * scale(v);
    _scale(v) children();
}

module multmatrix(m) {
    $_matrix = $_matrix * m;
    _multmatrix(m) children();
}

module save_transform() {
    $_matrix = IDENT;
    children();
}

module reset_transform() {
    _multmatrix(matrix_inverse($_matrix)) save_transform() children();
}

Example:

include <reset_transform.scad>

move([10,5,20]) scale(0.5) rot([0,45,45]) multmatrix(zrot(45)) {
        cube([10,15,20],anchor=CENTER);
        
        reset_transform() cube(5,anchor=CENTER); // this cube is back at global [0,0,0]
}

For now I've just included this in my own library, there's not even a need to rewrite anything in BOSL2! (But I think it would be a good thing to have there)

@adrianVmariano
Copy link
Collaborator

Yeah, that code from Revar seems complicated.

I was thinking you override multmatrix only to do $global = matrix * $global_transform and then just make sure all transform operators use multmatrix. But it does seem like it's true that you need to override scale, translate and so on, and that would cover the other methods. Seems a little less elegant to me, and less efficient (which probably doesn't matter) but it would work.

Saving accumulated inverse transforms means calling matrix_inverse for every transformation. That is not going to be more accurate than calling it one time. And it's also less efficient, since it means you do a bunch of inverses that you usually never need.

Saving the list of transforms would make it possible to do things in the parent frame, for example. I don't have a use case for that in mind, but it vaguely seems like it could be useful.

@adrianVmariano
Copy link
Collaborator

Looking at the code a little more, I see that the scale() function actually multiplies three matrices together, so the efficiency I was thinking about is missing. This may explain why I found it so advantageous in terms of run time in the screw code to hand-multiply the transformations.

@adrianVmariano
Copy link
Collaborator

A possible complication with overriding the transform operators is that the error handling and error messages will potentially change.

@revarbat
Copy link
Collaborator

It would be very minor work to refactor all of the modules in transforms.scad to use multmatrix(). Add thin overrides for translate(), scale(), rotate(), and mirror(). Then all the tracking can be done by a multmatrix() override, that calls the builtin via _multmatrix().

@lijon
Copy link
Author

lijon commented Feb 10, 2024

A possible complication with overriding the transform operators is that the error handling and error messages will potentially change.

I haven't tried now, but wouldn't any errors just be handled by the built-ins as usual? It would just insert the _wrappers in the stack trace.

@lijon
Copy link
Author

lijon commented Feb 10, 2024

It would be very minor work to refactor all of the modules in transforms.scad to use multmatrix(). Add thin overrides for translate(), scale(), rotate(), and mirror(). Then all the tracking can be done by a multmatrix() override, that calls the builtin via _multmatrix().

If one doesn't want to refactor transforms.scad, all that is needed (AFAICT) is to override the openscad built-ins: https://github.com/lijon/jl_scad/blob/main/reset_transform.scad Since any other module will eventually call one of them. (I see now that I forgot about mirror! will add that as well.)

@adrianVmariano
Copy link
Collaborator

Revar, what do you mean by "thin overrides for translate, scale, ..."? Would you have them call multmatrix? Because unless you do that you need to include tracking there as well. I'm not sure it's worth the trouble to go through and rewrite the modules to use multmatrix(), since you have to override the other built-in transform modules anyway.

Errors will not be handled by builtins because the builtins run after the calls to the function form that update the tracking. So errors will be handled by those functions.

@lijon
Copy link
Author

lijon commented Feb 10, 2024

Errors will not be handled by builtins because the builtins run after the calls to the function form that update the tracking. So errors will be handled by those functions.

Right, unless it just passes on the arguments to the built-ins, but it has to do the matrix transform and multiplication so errors like passing wrong data type will be asserted in those functions.

@adrianVmariano
Copy link
Collaborator

Not sure what you mean at first. If it just passes on the arguments to the built-in then that means it's never using the arguments, which isn't very useful. It would be possible to check arg validity and only update the transform matrix if args are valid and then invoke the built-in so the built-in can display errors.

Another observation is that probably calls to compute transform matrices should be into affine3d functions instead of the functions in transforms.scad, which check for and handle various cases not handled by the built-ins.

@lijon
Copy link
Author

lijon commented Feb 10, 2024

Yes, good points, @adrianVmariano!

@lijon
Copy link
Author

lijon commented Feb 10, 2024

However there's no affine3d_rot() it seems?

@adrianVmariano
Copy link
Collaborator

No, that one's a bit more complex. I think there are two cases?

I noticed with regard to error handling, that bogus parameters to the built-ins are just warnings, not errors. So it's actually critical to do this right. And probably to understand what it actually does when you give it bogus parameters---that is, what transform gets executed.

@lijon
Copy link
Author

lijon commented Feb 10, 2024

No, that one's a bit more complex. I think there are two cases?

I noticed with regard to error handling, that bogus parameters to the built-ins are just warnings, not errors. So it's actually critical to do this right. And probably to understand what it actually does when you give it bogus parameters---that is, what transform gets executed.

Perhaps it's enough to say that transform-tracking only works for valid parameters?

@adrianVmariano
Copy link
Collaborator

Yeah, that's probably sufficient. I always run with "stop on first warning" anyway so to me warnings and errors are not different. And really everyone should be doing that.

It remains to answer the question of whether to store the transform list or just a transform. The idea of setting the accumulated transform to identity doesn't seem great, because it is a side effect and invalidates subsequent calls to the global reset---or changes what they do, I guess. Imagine that a module does that: the children might then try to reset the transform and the wrong thing would happen. With the list you can "save" a position by storing its index in the transform list. Or you can save by storing the current transform in a list of saved transforms.

@lijon
Copy link
Author

lijon commented Feb 10, 2024

It remains to answer the question of whether to store the transform list or just a transform. The idea of setting the accumulated transform to identity doesn't seem great, because it is a side effect and invalidates subsequent calls to the global reset---or changes what they do, I guess. Imagine that a module does that: the children might then try to reset the transform and the wrong thing would happen. With the list you can "save" a position by storing its index in the transform list. Or you can save by storing the current transform in a list of saved transforms.

In my mind, reset_transform() rewinds back to the nearest save_transform() up the call tree, which I think makes sense and would be useful as is. (Maybe it shouldn't be named "reset", but "restore"?)

But yes, even more useful would be able to $a = save_transform() and then restore_transform($a) or restore_transform() for global. Each save would add the current matrix to a list and return the index, restore would unwind each element in the list back to that index.

@adrianVmariano
Copy link
Collaborator

I understood your concept, but was just concerned about the practical issue of a module that resets the matrix and breaks behavior for children. Now maybe that situation never arises because a module that uses the reset can't have children that also need a reset? I don't know.

I was thinking about maintaining the list of transforms meaning that saving a transform would just be an index, and you could do things like "up two levels", or attachable() could set checkpoints so you could say "parent_frame()".
But I have absolutely no use case for any of this stuff.

@lijon
Copy link
Author

lijon commented Feb 11, 2024

I understood your concept, but was just concerned about the practical issue of a module that resets the matrix and breaks behavior for children. Now maybe that situation never arises because a module that uses the reset can't have children that also need a reset? I don't know.

Maybe this is what would be expected? Think about it like this: a parent does reset_transform() on its child to reset it back to whatever frame of reference the parent itself was made in. So in my use case, an auxillary part is detached from its position in the assembly and reset back to [0,0,0] so we can easily apply a transform for print layout. Now if the top level code wants to show two such assemblies next to each other, it can do move([100,0]) save_transform() on the second one, which will then get to [100,0,0] instead of [0,0,0] when doing its reset_transform().

Perhaps "save transform" should rather be called "set transform tracking origin"...

I was thinking about maintaining the list of transforms meaning that saving a transform would just be an index, and you could do things like "up two levels", or attachable() could set checkpoints so you could say "parent_frame()". But I have absolutely no use case for any of this stuff.

The question is what exactly is "up two levels" and what would a child know about where that is? The number of child-levels (or worse, number of applied transforms!) might not always be obvious, and might change depending on conditional expressions etc. I think it would be more useful to explicitly save such an index at a specific call stack depth and use it later down the stack.

@adrianVmariano
Copy link
Collaborator

Does it suffice to have a single save point? In your example above you don't have multiple nested resetting, which was the situation I was concerned about. You just have differing save points on different branches.

For the idea about "up two" to make sense it would need to be done at the attachables. This could also be done by having attachable store a (separate) list of transforms instead of an index into a master list. So "up 1" would be the parent from, and "up 2" would be the grandparent frame, regardless of the number of intervening transform operators.

@lijon
Copy link
Author

lijon commented Feb 11, 2024

Here's an example with multiple nested save_transform(), which for me behaves as expected:

include <jl_scad/reset_transform.scad>

$do_reset = true;

module maybe_reset()
    if($do_reset) reset_transform() children();
    else children();

module foo() {
    move([10,5,20]) scale(0.5) rot([0,45,45]) multmatrix(zrot(15)) {
            cube([10,15,20],anchor=CENTER)
            position(TOP)
            maybe_reset()
            cube(5,anchor=BOT);
    }
}

module bar() {
    foo();
    move([10,0]) save_transform() foo();
}

module zoo() {
    bar();
    zrot(180) move([0,10]) save_transform() bar();
}

let($do_reset=true) recolor("blue") zoo();
move([0,30]) let($do_reset=false) recolor("orange") zoo();
Screenshot 2024-02-11 at 22 38 31

@adrianVmariano
Copy link
Collaborator

I don't think that matches the pattern that I was thinking about, which is that a module that you use, written by someone else, which is a black box to you, modifies the state by calling save_transform(). Meanwhile, you are trying to use it in a child of that module to restore to a state that was saved above the level of the offending module. Instead, you restore to the state saved by the module. Baffled confusion ensues.

@lijon
Copy link
Author

lijon commented Feb 11, 2024

I don't think that matches the pattern that I was thinking about, which is that a module that you use, written by someone else, which is a black box to you, modifies the state by calling save_transform(). Meanwhile, you are trying to use it in a child of that module to restore to a state that was saved above the level of the offending module. Instead, you restore to the state saved by the module. Baffled confusion ensues.

Ok, I think I now understand your point! Even if I can't think up of an actual such use case right now. But then, we also don't know how many levels of attachables there is in that black box module, so I think it would make most sense to have save_transform return a token (probably an index to the list) that can be used to restore the transform later down the call stack, something like:

let($token = save_transform())
  black_box_module()
    reset_transform($token)
      some_children();

where the arg to reset_transform defaults to $global_matrix_token.

@adrianVmariano
Copy link
Collaborator

The idea of storing a stack of transforms was unrelated to the concern I had about the global side effect problem. But allow me to point out: we actually do know how many levels of attachable are in the black box module---1. Because the children are children of attachable, not children of the object. I mean, unless the black box modules nested calls to attachable, which would be strange.

Note that we can address the side effect problem by having the "token" be simply the current transformation. The default would be the identity. That's why we don't need the list to address this issue. The code would simply do $saved_transform * matrix_inverse($current_transform) to obtain the desired transform. This model isn't totally safe from nefarious black box modules (it could rewrite $token in your above example) but you can choose a different variable name to avoid collisions.

@lijon
Copy link
Author

lijon commented Feb 12, 2024

The idea of storing a stack of transforms was unrelated to the concern I had about the global side effect problem.

Hmm, ok then I'm not sure I understand exactly what you mean.

But allow me to point out: we actually do know how many levels of attachable are in the black box module---1. Because the children are children of attachable, not children of the object. I mean, unless the black box modules nested calls to attachable, which would be strange.

You mean like in my other issue #1374 where I use attachable to make a group of other attachable(s)?

Note that we can address the side effect problem by having the "token" be simply the current transformation. The default would be the identity. That's why we don't need the list to address this issue. The code would simply do $saved_transform * matrix_inverse($current_transform) to obtain the desired transform. This model isn't totally safe from nefarious black box modules (it could rewrite $token in your above example) but you can choose a different variable name to avoid collisions.

Sounds good, though I haven't thought it through so can't confirm it would actually work :)

I wonder if it would make sense to use a stack, and do push_matrix() and pop_matrix() instead?

@adrianVmariano
Copy link
Collaborator

A stack has the same problem that it is based on side-effects and if a module runs a push then it can screw up code at a lower level. I don't think push/pop is the easiest scheme to use anyway----it seems easier to keep track of places where a variable assignment occurs.

@adrianVmariano adrianVmariano added the Defer Deferred until after the next major release. label May 5, 2024
@adrianVmariano
Copy link
Collaborator

An idea has been proposed to use this for a simplified API to join_prism() so I was looking at it again. It appears that for multmatrix() there's a fundamental ambiguity if you provide a 3x3 matrix. If the child is 2D it's treated as a 3x3 affine matrix. If the child is 3D it's treated as a 3x3 transform missing its affine part, so effectively a 4x4 matrix.

There's no way to distinguish these cases for propagating the transformation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Defer Deferred until after the next major release.
Projects
None yet
Development

No branches or pull requests

3 participants