Bug in Vector.lerp() implementation

[xuanquang1999]

There seems to be a bug with the implementation of Vector.lerp() function

$.Vector.lerp= function(v,u,t){return new $.Vector(v.x * (1-t) + u.x * t,
                                                   v.y = v.y * (1-t) + u.y * t,
                                                   v.z = v.z * (1-t) + u.z * t);}

The function change the value of y and z coordinate of the first param vector, which don't seems to be intended (it should not modify any value of the first param vector)

[quinton-ashley]

Is the solution to just to remove v.y = and v.z = ?

Could you also please show an example of correct output for this function?

[GoToLoop]

p5js' lerp() reference: https://p5js.org/reference/#/p5.Vector/lerp

My Processing Pjs-based implementation:

1. https://Gist.GitHub.com/GoToLoop/acbf106aa784820aff23#file-pvector-js-L4-L5
2. https://Gist.GitHub.com/GoToLoop/acbf106aa784820aff23#file-pvector-js-L53-L54
3. https://Gist.GitHub.com/GoToLoop/acbf106aa784820aff23#file-pvector-js-L183-L197

[quinton-ashley]

My solution to this was just to do this, ha! This is technically faster than what @LingDong- had before as well, creating new objects takes longer than just calling the function on the first vector inputted.

Vector.add = (v, u) => v.add(u);
Vector.rem = (v, u) => v.rem(u);
Vector.sub = (v, u) => v.sub(u);
Vector.mult = (v, u) => v.mult(u);
Vector.div = (v, u) => v.div(u);
Vector.dist = (v, u) => v.dist(u);
Vector.cross = (v, u) => v.cross(u);
Vector.lerp = (v, u, t) => v.lerp(u, t);
Vector.equals = (v, u, epsilon) => v.equals(u, epsilon);

The lerp implementation in the Vector class looks correct to me.

https://github.com/quinton-ashley/q5js

[GoToLoop]

, creating new objects takes longer than just calling the function on the first vector inputted.

Static methods aren't supposed to mutate its passed arguments! 🙅

Take for example this implementation of static method PVector.add(): 👓 https://Gist.GitHub.com/GoToLoop/acbf106aa784820aff23#file-pvector-js-L56-L58

PVector.add = (v1, v2, t) =>
  t? t.set(v1.x + v2.x, v1.y + v2.y, v1.z + v2.z)
   : new PVector(v1.x + v2.x, v1.y + v2.y, v1.z + v2.z);

Neither parameters v1 & v2 are ever mutated there. 🔒

Instead, either a new PVector is created or the method relies on parameter t to store the result.

BtW, file PVector.js is a close implementation of class PVector from Processing Java, which p5js more or less follows. ☕

Actually, p5js' implementation of its static method p5.Vector.add() also has a 3rd parameter that serves the same purpose as Java Mode's PVector.add(): 😻 https://GitHub.com/processing/p5.js/blob/v1.5.0/src/math/p5.Vector.js#L2070-L2084

[quinton-ashley]

ahh! okay I understand. I'll have to revert those changes

On Sun, Feb 12, 2023 at 22:40 GoToLoop @.***> wrote:

, creating new objects takes longer than just calling the function on the first vector inputted.

Static methods aren't supposed to mutate its passed arguments! 🙅

Take for example this implementation of static method PVector.add(): 👓

https://Gist.GitHub.com/GoToLoop/acbf106aa784820aff23#file-pvector-js-L56-L58

PVector.add = (v1, v2, t) =>

t? t.set(v1.x + v2.x, v1.y + v2.y, v1.z + v2.z)

: new PVector(v1.x + v2.x, v1.y + v2.y, v1.z + v2.z);

Neither parameters v1 & v2 are ever mutated there. 🔒

Instead, either a new PVector is created or the method relies on parameter t to store the result.

BtW, file PVector.js https://gist.github.com/GoToLoop/acbf106aa784820aff23#file-pvector-js is a close implementation of class PVector from Processing Java, which p5js more or less follows. ☕

Actually, p5js' implementation of its p5.Vector.add() also has a 3rd parameter that serves the same purpose as Java Mode's PVector.add(). 😻

— Reply to this email directly, view it on GitHub https://github.com/LingDong-/q5xjs/issues/21#issuecomment-1427290787, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEY2RQ3AGN2CWWUZ5GZRVV3WXGUJPANCNFSM6AAAAAAUUZKB5M . You are receiving this because you commented.Message ID: @.***>

[quinton-ashley]

Done, thank you! I still need to fix lerp though.

[GoToLoop]

Have you looked at my PVector.lerp() implementation? Although it's indeed a little complex b/c the whole thing is divided in 3 parts:

1. The lerp() calculation itself.
2. The static implementation which invokes the instance implementation.
3. The instance implementation which may invoke its static counterpart when arguments is 3.

[quinton-ashley]

Not yet and clearly this is an area of the project I know very little about 😅

Do you think you'd be able to make the fix and do a pull request to my repo? https://github.com/quinton-ashley/q5.js

[GoToLoop]

• I haven't created any of those vector methods. I've just refactored an existing code.
• What I can do is refactor again file "PVector.js" to modern JS and make sure it fits in q5.js.
• Just noticed the whole vector implementation in q5.js is memory intensive!
• Every time we instantiate a new vector in q5.js all of its methods are re-created!
• B/c its methods are defined inside the vector constructor itself instead of its prototype{}!
• ES6 classes automatically place methods inside their prototype{} btW.
• Anyways, I'm gonna leave you w/ a shorter version w/ just the needed parts for the lerp() implementation, which can be copied & pasted on any browser's console for test purposes:

const
  lerp = (start, stop, amt = 0) => +start + amt*(stop - start),

  argsErr = (mtd, len, min) => {
    throw `Too few args passed to ${mtd}() [${len} < ${min}].`;
  };

function PVector(x, y, z) {
  this.x = x || 0, this.y = y || 0, this.z = z || 0;
}

PVector.lerp = (v1, v2, amt, t) => (t && t.set(v1) || v1.copy()).lerp(v2, amt);

PVector.prototype.lerp = function (a, b, c, d) {
  let x, y, z, amt;
  const len = arguments.length;

  if (len < 2)  argsErr('lerp', len, 2);

  if (len === 2) { // given vector and amt
    ({x, y, z} = a), amt = b;

  } else if (len === 3) { // given vector 1, vector 2 and amt
    return PVector.lerp(a, b, c);

  } else { // given x, y, z and amt
    [x, y, z, amt] = arguments;
  }

  return this.set(lerp(this.x, x, amt),
                  lerp(this.y, y, amt),
                  lerp(this.z, z, amt));
};

PVector.prototype.copy = function () {
  return new PVector(this.x, this.y, this.z);
};

PVector.prototype.set = function (v, y, z) {
  if (y != void 0)  this.x = +v, this.y = +y, z != void 0 && (this.z = +z);
  else this.set(v[0] || v.x || 0, v[1] || v.y || 0, v[2] || v.z);
  return this;
};

[quinton-ashley]

Just noticed the whole vector implementation in q5.js is memory intensive! Every time we instantiate a new vector in q5.js all of its methods are re-created!

That's true, I will change it to use an ES6 class!

[GoToLoop]

That's true, I will change it to use an ES6 class!

Actually I'm doing it right now. 😆 Just starting though: 👲

"use strict";

function Q5(scope, parent) {
  const $ = this;

  $.createVector = (x, y, z) => new $.Vector(x, y, z);

  Q5.Vector = $.Vector = class Vector {
    constructor(x, y, z) {
      this.x = x || 0, this.y = y || 0, this.z = z || 0;
      this._deleteMagCache();
    }

    _deleteMagCache() {
      this._magSq = this._mag = null;
    }

    _calcMagCache() {
      if (this._magSq == null)
        this._magSq = (this._mag = Math.hypot(this.x, this.y, this.z)) ** 2;
    }
  };
}

[quinton-ashley]

Just finished and I tested it this time! I moved the Vector class to the bottom of the file and now it is only created once and uses ES6 classes.

https://github.com/quinton-ashley/q5.js/blob/main/q5.js

[GoToLoop]

It doesn't seem correct to me! How is such an independent class now able to use Q5 instance functions like $.cos(), $.sin(), $.tan(), $.atan2()?

[quinton-ashley]

ah! good catch

[quinton-ashley]

Okay I think it's good now: https://github.com/quinton-ashley/q5.js/blob/39c3662c454519c932c29544384cd3bf42dd75f3/q5.js#L372