Prepare an axis variable for angleBetween() and return a signed value

[inaridarkfox4231]

Increasing Access

I wanted to do it while I could, so I'm proposing it now.

First, it's quite natural that angleBetween() returns values ​​between 0 and PI. It's not a bug.

It's called a bug in the issue(#3973), but it's not a bug because the direction of two vectors in 3D space has no meaning. Both Three.js and Processing return values ​​from 0 to PI. I expected that too. Most people expect a value between 0 and PI to be returned.
People new to p5.js would be surprised to see a function in Processing that was treated as angleBetween(). It should be fixed again.

However, I am reluctant to neglect the behavior of returning a signed value that the person who raised this issue expects from angleBetween().

So I prepared a compromise.

Prepare a new variable named axis in angleBetween(). This is like the normal vector of the plane. When viewed from this direction, for example, if we apply a method such as v1.angleBetween(v2, axis), we will behave to return a signed value like the one currently used.

For example if the axis is createVector(0,0,1) the behavior will match the current one. The sign is reversed for createVector(0,0,-1). In other words, when viewed from the axis, we can see the projection of v1 and v2 onto the plane, and consider moving them so that they overlap clockwise within the PI rotation.

Returns values ​​from 0 to PI if axis is not defined or is not a vector.

Now, to get the traditional signed behavior, just specify createVector(0,0,1) for the new argument. It may be inconvenient, but the flexibility is improved by being able to specify other axes. The majority of people expect it to return values ​​between 0 and PI, so I think this change is reasonable.

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build Process
• Unit Testing
• Internalization
• Friendly Errors
• Other (specify if possible)

Feature enhancement details

implementation

Change is easy. All we have to do is to replace the calculation that has been used to obtain the z component with the calculation that obtains the inner product with the axis. This calculation is not performed if axis is not defined.

angleBetween(v, axis) {
  const dotmagmag = this.dot(v) / (this.mag() * v.mag());
  // Mathematically speaking: the dotmagmag variable will be between -1 and 1
  // inclusive. Practically though it could be slightly outside this range due
  // to floating-point rounding issues. This can make Math.acos return NaN.
  //
  // Solution: we'll clamp the value to the -1,1 range
  let angle;
  angle = Math.acos(Math.min(1, Math.max(-1, dotmagmag)));
  // If axis is defined, returns a signed value taking into account
  // the orientation with respect to axis.
  if (axis !== undefined && axis instanceof p5.Vector) {
    angle *= Math.sign(this.cross(v).dot(axis) || 1);
  }
  if (this.isPInst) {
    angle = this._fromRadians(angle);
  }
  return angle;
}

static angleBetween(v1, v2, axis) {
  return v1.angleBetween(v2, axis);
}

All I have to do is rewrite the references and prepare a unit test for the new axis variables. Eliminate some unit tests returning negative values with no axis variables.

example

angleBetween_improve

function setup() {
  createCanvas(400, 400);
  v=createVector(1,0,0);
  w=createVector(0,1,0);
  u=createVector(0,0,1);
  console.log(v.angleBetween(w, u));
  console.log(v.angleBetween(u, w));
  console.log(w.angleBetween(v, u));
  console.log(w.angleBetween(u, v));
  console.log(u.angleBetween(v, w));
  console.log(u.angleBetween(w, v));
  console.log("-----------------------------------")
  console.log(p5.Vector.angleBetween(v, w, u));
  console.log(p5.Vector.angleBetween(v, u, w));
  console.log(p5.Vector.angleBetween(w, v, u));
  console.log(p5.Vector.angleBetween(w, u, v));
  console.log(p5.Vector.angleBetween(u, v, w));
  console.log(p5.Vector.angleBetween(u, w, v));
}

result:

[davepagurek]

I think it makes sense to use the absolute angle as the default. I too was surprised that the range of angleBetween was different from other languages and Processing, and the extension into 3D is ambiguous enough that supplying an axis seems more useful.

Before going ahead with this change, I want to avoid going back and forth too much on this in the future, so I wonder if we can decide on how to decide this sort of thing. Also maybe @limzykenneth as a math steward you have opinions here?

Some thoughts:

• Consistency with other libraries is a good point, but p5 intentionally does some things differently too
• This arguably aligns better with learning resources: I've seen the sign of the angle expressed as $n \cdot (a \times b)$ before, and have seen the formula $\theta = \cos^{-1}(a \cdot b)$ in textbooks too, so there'd be consistency now if you saw this definition in a textbook and then looked up the range of acos to tell you the range of angleBetween. That's just my experience though, I'm not sure if that's how it's taught in general.
• I can also see the argument that 2D is the most common mode and in 2D mode, it's fairly easy to wrap the result in abs() if you want that. It's mostly for 3D mode where the current sign is fairly arbitrary. I could see consistency between 2D and 3D being a reason to make it slightly less convenient to get a signed angle in 2D mode, does that sound right to everyone?

[davepagurek]

If we agree and go ahead with this, it would be great to also add an example showing how to get a signed angle in 2D mode.

[inaridarkfox4231]

If we agree and go ahead with this, it would be great to also add an example showing how to get a signed angle in 2D mode.

I will add such an example to the reference.

[inaridarkfox4231]

Before going ahead with this change, I want to avoid going back and forth too much on this in the future

I thought about simply removing the sign, but that would lead to the possibility that the specifications would be changed again by people who use them like they do now, so I took this approach. The person who proposed the specification change didn't provide a concrete application example, so I don't know who would need the current signed angleBetween().

Consistency with other libraries is a good point, but p5 intentionally does some things differently too

So, I think it will be a method that finds the angle that is slightly different from other libraries. Three.js does not adopt orbitControl() that can rotate infinitely in the vertical direction. There is a difference.

I found stack overflow with the following:

Three.js Vector.angleTo: How to also get the bigger value

This is a Three.js example, but the questioner is probably looking at the vector from the y-axis direction. However, it is impossible to define the direction with two vectors in 3D space, so it is unreasonable.

I don't want to adopt the abs method because I'm thinking about the possibility that the specification will be changed to return a value from 0 to TAU like this questioner. In that case, the abs method cannot be used. There is probably little demand for this kind of thing, so I think there is no choice but to respond individually like this questioner.

If he use my own method, he only need to prepare one vector in the y-axis direction, and he don't need to calculate the orientation. I don't think it's possible to make it any simpler than this.

[inaridarkfox4231]

In Three.js it seems to return PI/2 if the magnitude is 0...

angleTo( v ) {

const denominator = Math.sqrt( this.lengthSq() * v.lengthSq() );

if ( denominator === 0 ) return Math.PI / 2;

const theta = this.dot( v ) / denominator;

// clamp, to handle numerical problems

return Math.acos( MathUtils.clamp( theta, - 1, 1 ) );

}

p5.js doesn't do this kind of validation...but might need it.
Processing seems to be returning 0.

static public float angleBetween(PVector v1, PVector v2) {

  // We get NaN if we pass in a zero vector which can cause problems
  // Zero seems like a reasonable angle between a (0,0,0) vector and something else
  if (v1.x == 0 && v1.y == 0 && v1.z == 0 ) return 0.0f;
  if (v2.x == 0 && v2.y == 0 && v2.z == 0 ) return 0.0f;

  double dot = v1.x * v2.x + v1.y * v2.y + v1.z * v2.z;
  double v1mag = Math.sqrt(v1.x * v1.x + v1.y * v1.y + v1.z * v1.z);
  double v2mag = Math.sqrt(v2.x * v2.x + v2.y * v2.y + v2.z * v2.z);
  // This should be a number between -1 and 1, since it's "normalized"
  double amt = dot / (v1mag * v2mag);
  // But if it's not due to rounding error, then we need to fix it
  // https://github.com/processing/processing/issues/379
  // Otherwise if outside the range, acos() will return NaN
  // http://www.cppreference.com/wiki/c/math/acos
  if (amt <= -1) {
    return PConstants.PI;
  } else if (amt >= 1) {
    // https://github.com/processing/processing/issues/474
    return 0;
  }
  return (float) Math.acos(amt);
}

No matter what the value is, basically this function applies to vectors of positive length, it seems that there is no problem...

[inaridarkfox4231]

In p5.js it's supposed to return NaN... I think it needs some value because NaN is kind of bad.
I would like to add that to the specification change as well. I think that it is good to follow Processing and set it to 0.

[inaridarkfox4231]

However, it is troublesome to prepare a vector, so I would like to be able to specify it directly as an argument like v.angleBetween(w,0,0,1). Because I may want to use the current specification.

[inaridarkfox4231]

Another reason is that the current specification will become a hotbed for bugs.

As you can see from the specification, this specification does not add a sign if the plane formed by the two vectors is parallel to the z-axis.

Therefore, you don't have to wrap it in absolute value symbols, and bugs go unnoticed. It was actually overlooked the other day. If I was in the same position, I might overlook it. That's what it means to need extra processing, not whether it's easy or not.

[limzykenneth]

There seems to be a few things going on here so I'll group them into points and my take on it.

1. Returning absolute angles instead of signed angles

Although I would like behaviours to match the behaviours of other libraries and tools as well (Processing, Three.js, Unity) as @davepagurek mentioned it is possible for us to implement something different where practical. In this case there's mainly 2 reasons that I see why we couldn't fully do this switch. First (pedantic) reason is that the previous switch was done before 1.0.0 major release so breaking change was allowed but now we couldn't really mae the change without it breaking many sketches out there expecting this behaviour. Second, is the aforementioned expected behaviour, the negative values are very likely going to be used as part of the rotate() method to rotate a vector towards another vector (possibly lerped as well), returning absolute angle will mean those sketches will have vectors rotating in a completely different direction than they should.

2. Passing an axis argument to get signed result

I think this proposal make sense and would be a way to solve the issue, but it doesn't make much sense when working with 2D vectors, especially it is 2D vectors that would need the signed directionality the most (ie. use case above). Complicating things is that as we had investigated before in #5821 there is no trivial way in p5.js of differentiating between actual 2D and 3D vectors, so it would be difficult to separate out the behaviours that way as well.

3. Behaviour when vector magnitude is 0

This depends on what perspective we want angleBetween() to have. Three.js take the perspective that uses orthoganality which posits that any two vectors whose dot product is 0 are orthoganal, whereas for returning NaN is using perpendicularity (ie. more geometric) which posits that an vector with no magnitude has no direction and so cannot have an angle between value, this is the result Wolfram Ahpha gives. (at least the above from what I can find and understand). I can't find reference to it but I would be interested to know how game engines like Unity deal with these cases.

[inaridarkfox4231]

Returning absolute angles instead of signed angles

The first suggestion in #4048 was to have a boolean and return a signed value only if it is false. but it has since been removed.
It has the following description:

I realized that the next release is a minor release so it is okay to break backwards compatibility on a function like this.

In addition to this, the reason is that it is easy to modify because it is just surrounded by abs(). But in practice this is unacceptable as it can lead to bugs for the reasons mentioned above.

Even during that specification change(#4048), there must have been some sketches where the result of angleBetween() suddenly had a negative sign. This is because it is a function that is also used in 3D. I can't imagine what a sketch using rotate() would look like, but I suspect it could be constructed without the angles being signed.
There is no big change due to this specification change. Just write v.angleBetween(w,0,0,1) instead of v.angleBetween(w).

In the first place, in the final specification change of orbitControl() that became a problem this time(#6150), I naturally assumed that it was a value between 0 and PI when calculating the angle between two vectors. It might have been easier to understand if this had a sign, but it is not possible to give this angle an appropriate sign with the current specifications. Because the spec is tied to the z-axis. If you write your code properly, you don't need such a sign. Even in 2D.

Even in Processing, there is no distinction between vectors in 2D and 3D, so I don't know what the problem is.

Behaviour when vector magnitude is 0

If it's better to leave it as NaN, so be it. I think anything is fine, but if we don't have to change it, it's better.

Note that #4048 had a breaking change, but it seems that the reference was not updated. Looking at the reference now, there was no mention of the possibility or condition of returning a signed angle. This is not good, so when we change the specifications this time, I would like to describe it in as much detail as possible in an easy-to-understand manner.

One last thing. As you can see from #3973, the person who proposed the current specification seems to have had a certain idea about 3D as well. But it never happened. As of now, there are no updates on his GitHub. In the end, I still don't know why He was so particular about the current specifications. If this specification change is realized, I think that it will be possible to contribute to some extent with respect to that 3D proposal.

[limzykenneth]

Sorry I wasn't clear before but to clarify changes made in #4048 in response to #3973 was done back in 2019 and released in version 0.8.0, as per semver major version 0.x.x are not considered stable and as such may have API breaking changes, whereas we are currently at 1.x.x semver version and there is a certain level of guarantee that the API will be backwards compatible with all other 1.x.x versions (ie. code written for 1.0.0 should still work when run with 1.6.0 of the library), the main exception to this rule would be behaviour considered a bug, which I don't think in this case would be.

That is why breaking changes were made in #4048 and not entirely advisible in this case.

I realized that the next release is a minor release so it is okay to break backwards compatibility on a function like this.

This is in reference to the above. At that time, unofficially we decided during 0.x.x versions we will limit breaking changes to semver minor releases and keep semver patch release backwards compatible. It has no relation to 1.x.x and beyond where we try to stick to semver in terms of version compatibility.

If you have a look at this example and toggle the commented in/out lines using angleBetween you'll be able to see the issue I'm referring to in 2D case: https://editor.p5js.org/limzykenneth/sketches/3QANn4pHU. While it is possible to provide the extra axis arguments as per your proposal, it will still be a breaking change and also I don't think it is particularly user friendly in 2D. It is possible to create this sketch without signed angles but the main point is more about breaking existing sketch that relies on this behaviour than alternative implementations for the sketch.

[davepagurek]

@inaridarkfox4231 maybe we could think of a way to not change the default but still allow the behaviour you're describing? e.g. defaulting to the z axis to preserve the existing behaviour, or putting this version of the method under a different name?

[inaridarkfox4231]

I understand well why the specification of angleBetween() cannot be changed.

For example, a contributor familiar with Processing or Three.js is working on a specification change. At that time, suppose he use angleBetween(), like myself, expecting a value from 0 to PI to be returned. As a result, a fatal bug occurs and many people are in trouble, but the possibility cannot be ruled out.

It turned out well that p5.js is such a library.

I would like to create a pull request. The content is to add something like the following to the reference: "A negatively signed angle may be returned.", "A negatively signed angle may be returned in the following cases."
Also add some examples.