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

implement p5.Camera.slerp(), 3x3Matrix functions and minor spec-change of orbitControl() #6251

Merged
merged 27 commits into from Jul 6, 2023
Merged

implement p5.Camera.slerp(), 3x3Matrix functions and minor spec-change of orbitControl() #6251

merged 27 commits into from Jul 6, 2023

Conversation

inaridarkfox4231
Copy link
Contributor

@inaridarkfox4231 inaridarkfox4231 commented Jul 3, 2023

Resolves #6247

Introduces a new function, slerp(), to p5.Camera. It interpolates the views of the two camera arguments.

Interpolation is the so-called quaternion slerp() and is well known. However, since p5.js does not implement quaternions, it does interpolation directly using orthogonal matrix, which is effectively equivalent to quaternions.

However, when the so-called between-angle (corresponding to the difference of the orthonormal basis) is very small, linear interpolation is used instead. This is to avoid problems by classifying cases including that case because the unit matrix cannot take the axis. This is similar to quaternion interpolation, so I used it as a reference.

Besides that, I made a minor fix regarding orbitControl().

First, only uPMatrix is ​​updated when scaling ortho(), but this does not update the camera's projMatrix. This causes problems when interpolating between ortho() cameras, so I rewrote it to update projMatrix as well.

Also, mouseWheelY is used for the mouse wheel interaction, but since this value depends on the model, there is a concern that using the direct value may cause a significant difference of behavior depends on machine. Therefore, we changed the specification to use only the sign.

The main changes are as above. Here is an example:

Example

p5.Camera.slerp() for PR

2023-07-03.23-03-54.mp4

The main usage is to smoothly switch between multiple cameras, or smoothly restore the camera state changed by orbitControl().

PR Checklist

The value of mouseWheel is machine-dependent, so I'll just use only the sign to remove that effect. Also, I replaced with 'cam' where it could be replaced with 'cam'.
In addition, in the current specification, the projMatrix of the camera is not updated when scaling with ortho(), so after updating it, I rewrote it to update uPMatrix.
Implement slerp(). It interpolates the views of the two cameras. For example, by interpolating between straight front and straight right, you can make it look diagonally to the right.
shorten code for calculate eyeDist
@davepagurek davepagurek self-requested a review July 3, 2023 20:18
@inaridarkfox4231
Copy link
Contributor Author

inaridarkfox4231 commented Jul 4, 2023

Introduced a new implementation. As a result, when pan(), tilt(), _orbit(), and _orbitFree() move the camera, slerp() now gives the same result.

function setup(){
  createCanvas(100, 100, WEBGL);

  const cam = createCamera();
  const cam0 = createCamera();
  cam0.camera(20, 30, 40, 0, 2, 4, 0, 1, 8);
  const cam1 = createCamera();
  cam1.camera(100, 100, 100, 0, 0, 0, 0, 0, -1);

  cam.slerp(cam0, cam1, 0);
  console.log(confirmcameraMatricesAreTheSame(cam, cam0));
  cam.slerp(cam0, cam1, 1);
  console.log(confirmcameraMatricesAreTheSame(cam, cam1));

  const cam2 = createCamera();
  const cam3 = cam2.copy();
  cam3.pan(PI*0.9);
  const cam4 = cam2.copy();
  cam4.pan(PI*0.3);

  cam.slerp(cam2, cam3, 1/3);
  console.log(confirmcameraMatricesAreTheSame(cam, cam4));
  
  const cam5 = createCamera();
  const cam6 = cam5.copy();
  cam6.tilt(PI*0.4);
  const cam7 = cam5.copy();
  cam7.tilt(PI*0.1);

  cam.slerp(cam5, cam6, 0.25);
  console.log(confirmcameraMatricesAreTheSame(cam, cam7));

  const cam8 = createCamera();
  const cam9 = cam8.copy();
  cam9._orbit(PI*0.8, 0, 0);
  const cam10 = cam8.copy();
  cam10._orbit(PI*0.3, 0, 0);

  cam.slerp(cam8, cam9, 3/8);
  console.log(confirmcameraMatricesAreTheSame(cam, cam10));

  const cam11 = createCamera();
  const cam12 = cam11.copy();
  cam12._orbit(0, PI*0.77, 0);
  const cam13 = cam11.copy();
  cam13._orbit(0, PI*0.22, 0);

  cam.slerp(cam11, cam12, 2/7);
  console.log(confirmcameraMatricesAreTheSame(cam, cam13));

  const cam14 = createCamera();
  const cam15 = cam14.copy();
  cam15._orbitFree(PI*0.27, PI*0.9, 0);
  const cam16 = cam14.copy();
  cam16._orbitFree(PI*0.12, PI*0.4, 0);

  cam.slerp(cam14, cam15, 4/9);
  console.log(confirmcameraMatricesAreTheSame(cam, cam16));
}

function confirmcameraMatricesAreTheSame(cam0, cam1, threshold = 0.0001){
  const m0 = cam0.cameraMatrix.mat4;
  const m1 = cam1.cameraMatrix.mat4;
  let errorSum = 0;
  for(i=0;i<16;i++){
    errorSum += abs(m0[i] - m1[i]);
    if(abs(m0[i] - m1[i]) > threshold){
      return false;
    }
  }
  console.log("error: " + errorSum);
  return true;
}

result:

error: 0
true
error: 0
true
error: 0
true
error: 0
true
error: 3.74861372287934e-7
true
error: 4.196068630335503e-7
true
error: 5.949606247668271e-7
true

This result was not possible in previous implementations due to pan() and tilt() moving the viewpoint. It's clearly improving.
Based on this result, I would like to add a unit test.

@inaridarkfox4231
Copy link
Contributor Author

It may be a little difficult to understand, but you can see that it matches the movement in the case of pan(). (slerpOld() is old slerp())

2023-07-04.10-38-41.mp4

Unrelated, but set() is also useful in this case. Since there was no camera function to set a specific state for the same camera, we had no choice but to use push() ~ pop() in the past, but using set() makes it very easy to return to the prepared state.

Until now, we used linear interpolation of the center to determine the viewpoint and the center, but this method does not result in a clean interpolation when the center moves.
So I've improved the code so that if at least one of eye and center doesn't move, it's an interpolation that doesn't move that.
In addition, we have improved the speed by not creating a new vector when only that component is needed.
As a result, there is little change in performance.
@inaridarkfox4231
Copy link
Contributor Author

I want to make unit tests carefully, so I'll work on it after I get home...

@inaridarkfox4231
Copy link
Contributor Author

inaridarkfox4231 commented Jul 4, 2023

Here are some examples.
p5.Camera.slerp() RISING

let cam, cam0, cam1, farCam;
let isFar = false;
function setup(){
  createCanvas(600, 600, WEBGL);
  pixelDensity(1);

  farCam = createCamera();
  farCam.camera(200, 200, 500, 0, 0, 250, 0, 0, -1);

  cam0 = createCamera();
  cam0.camera(100, 0, 0, -100, 0, 0, 0, 0, -1);
  cam1 = createCamera();
  cam1.camera(0, 100, 100, 0, -100, 100, 0, 0, -1);
  cam = createCamera();
}

function draw(){
  if (isFar) {
    setCamera(farCam);
  } else {
    setCamera(cam);
  }

  noStroke();
  strokeWeight(2);
  lights();
  fill("red");

  background(255);
  cam.slerp(cam0, cam1, (frameCount % 360) / 120);
  box(30);
  translate(0, 0, 100);
  box(30);
  translate(0, 0, 100);
  box(30);
  translate(0, 0, 100);
  box(30);
  translate(0, 0, 100);
  box(30);
  translate(0, 0, -300);

  if (isFar) {
    stroke(0);
    line(
      cam.eyeX, cam.eyeY, cam.eyeZ,
      cam.centerX,cam.centerY, cam.centerZ
    );
    noStroke();
    push();
    fill("blue");
    translate(cam.eyeX, cam.eyeY, cam.eyeZ);
    sphere(8); // small: eye
    pop();
    push();
    fill("green");
    translate(cam.centerX, cam.centerY, cam.centerZ);
    sphere(24); // big: center
    pop();
  }
}

function doubleClicked(){
  isFar = !isFar;
}
2023-07-04.21-30-29.mp4

First, prepare one camera, raise it as it is, and then rotate it by 90 degrees. By interpolating with these two, you can make the field of view rise. In this way, we can achieve rotary motion without using trigonometric functions.

The other concerns pan() and tilt().
p5.Camera.slerp() PAN_TILT

let cam, cam0, cam1;

function setup(){
  createCanvas(400, 400, WEBGL);
  pixelDensity(1);

  cam0 = createCamera();
  cam1 = createCamera();
  cam1.pan(PI/7);
  // cam1.tilt(PI/7);
  cam = createCamera();

  noStroke();
}

function draw(){
  background(255);
  
  cam.slerp(cam0, cam1, (frameCount % 420) / 30);

  lights();
  fill("skyblue");
  ambientMaterial("skyblue");
  push();
  rotateX(frameCount*0.08);
  box(80);
  pop();
  translate(0, 0, 680);
  push();
  rotateY(frameCount*0.06);
  box(80);
  pop();
  translate(0, 0, -680);
  translate(340, 0, 340);
  push();
  rotateZ(frameCount*0.08);
  box(80);
  pop();
  translate(-680, 0, 0);
  push();
  rotateZ(frameCount*0.08);
  box(80);
  pop();
}
2023-07-04.21-34-21.mp4

After setting the direction of rotation with pan() and tilt(), by executing slerp() outside the [0,1] range, you can use the camera to rotate infinitely.

In addition, the second example regarding pan() and tilt() will be strange if it is executed with slerp() before the specification change above. So I'm glad I made the change.

I'll try to make just one unit test. It is the test about when amt is 0 or 1.
@inaridarkfox4231
Copy link
Contributor Author

I'll try to make just one unit test. It is the test about when amt is 0 or 1.

@inaridarkfox4231
Copy link
Contributor Author

It seems to work fine, so I'll add more.

It is troublesome to write each cameraMatrix comparison method, so I decided to prepare it globally.
@inaridarkfox4231
Copy link
Contributor Author

It is troublesome to write each cameraMatrix comparison method, so I decided to prepare it globally.
I don't know if it will work, so I'll just make one for now.

add indent and semicolon
@inaridarkfox4231
Copy link
Contributor Author

It went well. In the case of _orbit() and _orbitFree(), the matrices are not exactly the same, so I thought it would be useful to have this.

Added unit test for tilt(),_orbit(),_orbitFree()
I forgot to mention that PI/2 rotations are problematic and should not be used.
@inaridarkfox4231
Copy link
Contributor Author

I forgot to mention that _orbit() shouldn't use for vertical PI/2 rotations. _orbitFree() would be fine, but _orbit() is no good...

@inaridarkfox4231
Copy link
Contributor Author

PI/2 rotation destroys the up vector, so it cannot be used. excuse me... I have confirmed that 0.49 and 0.51 are fine.
All checks by the method have been completed for now.
Also, add tests for when the viewpoint is fixed, when the center is fixed, and when ortho() is used.

Added tests for fixed view, fixed center, and ortho()
use strictEqual
use expect().to.be.closeTo()
@inaridarkfox4231
Copy link
Contributor Author

Since this function is only for interpolating the view, it is assumed that all the projection matrices match. However, for ortho(), orbitControl() changes the projection matrix a bit, so it is possible to interpolate if there is a difference of that degree. But that's where it gets tricky because it usually doesn't work. I can't do what I can't, so I think this is fine.

Added explanation about ortho()
@inaridarkfox4231
Copy link
Contributor Author

I added that it is permissible for the projection matrix to change with orbitControl(). If there are any problems, I will rewrite.
It's difficult because the matrix is ​​not open to the user, so I can't say something like "It doesn't matter if the components here and here are different"...

@inaridarkfox4231
Copy link
Contributor Author

That's all for the changes, so please leave a review.

Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on this, it looks like it took a lot of calculations, and the version you arrived on that handles both fixed view and fixed center works really well!

I left some comments about possibly using p5 math objects like vectors and matrices to clean up the code. It looks like that would involve augmenting p5.Matrix in order to do it though, what do you think? If you think it's a good idea but it looks like it'll take time, I'm also not opposed to doing that in a separate PR so that it doesn't feel like it's rushed to get it in in time for 1.7.0.

}

// Calculate the distance between eye and center for each camera.
const dist0 = Math.hypot(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth making a p5.Vector out of the eye and center points for both cameras, which would allow us to use .mag(), .sub(), .lerp(), etc below

// Create the inverse matrix of mat0 by transposing mat0,
// and multiply it to mat1 from the right.
// This matrix represents the difference between the two.
const m = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could make this out of p5.Matrix to make the transpose + multiply clearer? It looks like we support mat3s there if you do new p5.Matrix('mat3', [...]). I'm not certain all the operations work on mat3s, so if they don't, this is also fine

// Obtain the front vector and up vector by linear interpolation
// and normalize them.
const lerpedFront = new p5.Vector(
(1 - amt) * m0[2] + amt * m1[2],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do end up using p5.Matrix, maybe we could make functions to get a column as a p5.Vector? Or maybe least as an array to handle both 3x3 and 4x4 matrices, which one can then put into a vector? so then this line could be something like:

const lerpedFront = createVector(...m0.column(2))
  .lerp(...m1.column(2), amt)
  .normalize();
const lerpedUp = createVector(...m0.column(1))
  .lerp(...m1.column(1), amt)
  .normalize();

// https://github.com/mrdoob/three.js/blob/dev/src/math/Quaternion.js#L294
let a, b, c, sinTheta;
let invOneMinusCosTheta = 1 / (1 - cosTheta);
if (m[0] > m[4] && m[0] > m[8]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a diagonal() method on a matrix could be helpful too, so it's easier to tell which indices you're using?

// Calculate the trace and from it the cos value of the angle.
// An orthogonal matrix is just an orthonormal basis. If this is not the identity
// matrix, it is a centered orthonormal basis plus some angle of rotation about
// some axis. That's the angle. Letting this be theta, trace becomes 1+2cos(theta).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this might not be obvious to readers, maybe we can drop in a link to this: https://en.wikipedia.org/wiki/Rotation_matrix#Determining_the_angle

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learned for the first time that there is a website that has a specific shape of the procession.
Ok, I'll add it.

// Calculates the axis vector and the angle of the difference orthogonal matrix.
// The axis vector is what I explained earlier in the comments.
// similar calculation is here:
// https://github.com/mrdoob/three.js/blob/dev/src/math/Quaternion.js#L294
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the permalink URL to this line instead? (Obtained through the three dot menu on github when you select a line, I think it just uses the commit hash in the URL instead of the branch, in case this file changes on the dev branch in the future.) https://github.com/mrdoob/three.js/blob/883249620049d1632e8791732808fefd1a98c871/src/math/Quaternion.js#L294

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll fix that.

// Multiplying mat0 by the first matrix yields mat1, but by creating a state
// in the middle of that matrix, you can obtain a matrix that is
// an intermediate state between mat0 and mat1.
const angle = amt * Math.atan2(sinTheta, cosTheta);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see what you mean about quaternions maybe making the implementation more difficult, since you mostly need the angle in order to then pick a different angle based on amt. Just for my understanding, I suppose you'd want to convert the quaternion to axis/angle, update the angle, then make a new quaternion from that, then apply that to m0, and this current implementation avoids unnecessary work in making the intermediate states?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with quaternions in the first place, so I'm not sure. Orthogonal matrices are easier to work with, so I've thought about implementing them in that direction. It's very easy because we can get the difference just by taking the transpose and multiplying it. I thought it would be easier to implement this way because it avoids the extra effort of converting to quaternions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I used the word quaternion was because I thought that it was doing something like that in the atmosphere while researching various things. I'm not sure how it would actually be implemented. However, I got the impression that it would take a lot of effort to convert between them.
It doesn't bother me that p5.js doesn't implement quaternions either. If we can handle it with matrices, I thought it would be simpler unless there was a big problem.

];

// Multiply this to mat0 from left to get the interpolated front vector.
const newFrontX = result[0] * m0[2] + result[1] * m0[5] + result[2] * m0[8];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe add a multiplyVec3 to p5.Matrix to use here (again only if we use p5.Matrix)

add link
@inaridarkfox4231
Copy link
Contributor Author

The implementation that gets the components instead of creating a new vector is for performance, but if it's easier to understand, I'll rewrite it.
Regarding matrices, I didn't do anything difficult, so I didn't feel the need to prepare methods. Because there are no other functions that work with 3x3 matrices. But I will do it if necessary.

@inaridarkfox4231
Copy link
Contributor Author

I would add two suggestions regarding 3x3 matrix.

  • createSubMatrix3x3(): A function that creates and returns a 3x3 matrix from the top left 3x3 component of a 4x4 matrix. This process was originally planned as a method. Since this process is being done in two places, I would like to make it a method. For the cameraMatrix, this 3x3 part represents the local coordinate system and has an important meaning, so I thought it would be useful when dealing with functions related to the camera in the future.
  • copy(): This is an extension of functionality. Currently, copy() only supports 4x4, so I'm thinking of making it compatible with 3x3. I think it's more reasonable than making copy3x3(). By this function, copying the matrix of cam0, transposing it, and multiplying it from the right. This simplifies the process of creating m.

I would like to implement the above two.

@davepagurek
Copy link
Contributor

Sounds good to me!

Added methods for 3x3 matrices.
In addition, change the specification so that copy() can be used with 3x3
New specifications for transpose3x3 when there are no arguments
remove trailing comma
Make the argument of copy obtained by slice instead of array
add mat3 method to original copy() specification
Direct assignment seems to be safer, so let's rewrite it.
@inaridarkfox4231
Copy link
Contributor Author

Changed transpose3x3 to return itself transposed if there is no argument.
Also, since there was no assignment to the diagonal component, I added it.

test for unit test of p5.Matrix 3x3.
copy() for 3x3Matrix.
@inaridarkfox4231
Copy link
Contributor Author

inaridarkfox4231 commented Jul 6, 2023

Let's make a unit test for a 3x3 matrix. If it doesn't work, I give up for this.

@inaridarkfox4231
Copy link
Contributor Author

This seems fine, so I'll add more.

@davepagurek
Copy link
Contributor

The matrix methods are looking good!

add other tests for 3x3Matrix
fix indent
fix transpose to transpose3x3
@inaridarkfox4231
Copy link
Contributor Author

Unit testing is over. Rewrite the method using matrix operations.

@inaridarkfox4231
Copy link
Contributor Author

About multiplyVec3(), I add a method that takes a target as an argument.
This eliminates the need to create a new vector when assigning results to an existing vector.

Changed to take target as an argument.
If there is no target, prepare a copy of the vector to be multiplied.
@inaridarkfox4231
Copy link
Contributor Author

If there is no target, prepare a copy of the vector to be multiplied.
This has the advantage that the created vector and the original vector have the same information, such as whether or not they were created with createVector().

If there is a target, set result and return that.
@inaridarkfox4231 inaridarkfox4231 changed the title implementation of p5.Camera.slerp() and minor specification change of orbitControl() implement p5.Camera.slerp(), 3x3Matrix functions and minor spec-change of orbitControl() Jul 6, 2023
@inaridarkfox4231
Copy link
Contributor Author

Change title to "implement p5.Camera.slerp(), 3x3Matrix functions and minor spec-change of orbitControl()".
I thought I should mention that I made functions for 3x3Matrix.

rewrite with vectors and 3x3Matrix functions
do not use lerp
@inaridarkfox4231
Copy link
Contributor Author

Requested rewrite completed.
It passes all the unit tests I prepared. I don't think there is a problem.
Also, I have confirmed the operation of some samples that I created and all of the examples used in the reference of this method.
Please leave a review.

Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updates look great! thanks for adding all the new matrix methods!

@davepagurek davepagurek merged commit f2a0c5b into processing:main Jul 6, 2023
5 checks passed
@inaridarkfox4231 inaridarkfox4231 deleted the p5_Camera_slerp() branch July 6, 2023 22:04
@inaridarkfox4231
Copy link
Contributor Author

Thanks for review and merge!
thanks very much!!

@Qianqianye
Copy link
Contributor

Great work @inaridarkfox4231! Thanks @davepagurek for reviewing!

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

Successfully merging this pull request may close these issues.

implementation of p5.Camera.slerp(), and minor specification change of orbitControl()
3 participants