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

Rotation with different axis does not work #106

Closed
JulianBroudy opened this issue Mar 21, 2020 · 6 comments
Closed

Rotation with different axis does not work #106

JulianBroudy opened this issue Mar 21, 2020 · 6 comments
Labels

Comments

@JulianBroudy
Copy link

Hello,

I am trying to migrate my code from the old Pt to this new library (Pts) and I am having some trouble.
More specifically, with the Pt's rotate2D or the Geom's rotate2D functions.

Good to know: before I illustrate the problem, I would like to add that since I was using the standalone file, in order to eliminate some possibilities I copied the standalone version that is currently public on the old repo and replaced it with my working pt.min.js and the Canvas didn't even load.
Conclusion: my version is different also from the currently released old one.
Here is a link to a gist with my version: pt.min.js.

Sample to replicate: (Tested in my own environment as well as in a demo editor)

(function() {

    /* Because I would like to have some more properties that 
       I will be using (not showing the usage here) */
    function MyPoint(thePoint, brightness) {
        this.thePoint = thePoint;
        this.brightness = brightness;
        this.direction = Math.random() < 0.5 ? -1 : 1;
    }

    var space = new CanvasSpace("#pt").setup({
        bgcolor: "#99eeff",
        retina: true,
        resize: true
    });

    var form = space.getForm();
    var mainPath = [new Pt(), new Pt(-Util.randomInt(20), 0)];
    let points;
    let pts;

    space.add({
        start: (bound) => {
            if (!points) {
                points = [];
                pts = [];
                for (let i = 0; i < 100; i++) {
                    var point = new Pt({
                        x: Util.randomInt(20) * i,
                        y: Util.randomInt(20) * i
                        /* The next line is my attemp to add 3rd coordinate */
                        // ,z: Util.randomInt(20) * i 
                    });

                    pts.push(point);
                    points[i] = new MyPoint(point, 0.7);
                }
            }
        },

        animate: (time, ftime) => {

            /* The following for loop and its alternative "Geom.rot..." work both 
               with and without the z coordinate, and it gives the same result: points rotate.*/

            // for (let i = 0; i < 100; i++) {
                // points[i].thePoint.rotate2D(Const.one_degree / 12, space.center);
            // }
            /* For's alternative */
            // Geom.rotate2D(pts,Const.one_degree, space.center);



            /* The following for loop and its alternative "Geom.rot..." does not create the 
               expected result whether I include the z coordinate or not! */

            for (let i = 0; i < 100; i++) {
                points[i].thePoint.rotate2D(Const.one_degree / 12, space.center, Const.xy);
                    /*OR*/
                //     points[i].thePoint.rotate2D(Const.one_degree / 12, space.center, Const.xz);
                    /*OR*/
                //     points[i].thePoint.rotate2D(Const.one_degree / 12, space.center, Const.yz);
            }
            // Geom.rotate2D(pts,Const.one_degree, space.center,Const.xy);
                /*OR*/
            // Geom.rotate2D(pts,Const.one_degree, space.center,Const.xz);
                /*OR*/
            // Geom.rotate2D(pts,Const.one_degree, space.center,Const.yz);


            let perpends = pts.map((p) => [p, Line.perpendicularFromPt(mainPath, p)]);

            form.strokeOnly("#123", 1).lines(perpends);
            form.fillOnly("#123").points(pts, 5, "circle");
            form.point(space.pointer, 10);
        }
    });

    space.bindMouse().bindTouch().play();

})();

To be clear, in the previous code, I am only trying to rotate it in 1 plane out of the 3 possibilities.

Expected result is what I have now with the old code and old pt.min.js:

Rotation on 2 planes at once
2planes

Rotation on YZ plane
yz

Here is my current code with the old library and the pt.min.js I mentioned at the beginning.

I think the Const.xy don't get registered in the new library, possible?

Thank you.

@williamngan
Copy link
Owner

Hi @JulianBroudy , thanks for you detailed bug report. I tested your code and I believe there's a bug in Pts here:

https://github.com/williamngan/pts/blob/master/src/Num.ts#L368
(The $take function returns a new point and should update the original point)

I'll try to patch it in the next few days. Thank you again!

@JulianBroudy
Copy link
Author

@williamngan hi,
Did you get a chance to take a look at that bug?

williamngan added a commit that referenced this issue Apr 27, 2020
@williamngan
Copy link
Owner

Hi @JulianBroudy - Sorry about the delay. I've been so busy these days!

The bug should be fixed and patched in 0.9.2. I tested with your sample code and it seemed to work. Please give it a try and see if it works for you.

Thank you for reporting this bug and thanks for upgrading from Pt to Pts :D

@JulianBroudy
Copy link
Author

Hi,
Thank you for that.

Does the Demo Editor include the fixed version?
If so, ignore the rest of the post because I tested it there.

I am getting some weird behavior, I am not sure if I am missing something maybe you could see it right away?

When enabling the following line:

            // Geom.rotate2D(pts,Const.one_degree, space.center,Const.yz);

(with Z point enabled) I instantly get this result:

image

Also, enabling only XZ plane seems to rotate points until they reach this stage and it stops there:

image

What am I doing wrong?
P.S., This can be replicated by using the same code I posted above in a Demo Editor and uncommenting the Z-coordinate. The results are same whether rotating using point.rotate2D() or Geom.rotate2D().

@williamngan
Copy link
Owner

williamngan commented May 4, 2020

Hi again @JulianBroudy - sorry it's my fault. I forgot about the array index here (8093b23)

It should be fixed now (version 0.9.4). I didn't test your code but I wrote some unit tests and it's working as expected. See if it works for you.

Thanks!

@JulianBroudy
Copy link
Author

Hi,
I apologize for going MIA.
I took a brief look, looks like everything checks out.
Thank you very much.

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

No branches or pull requests

2 participants