Fix camera tilt function to prevent orientation flipping #7598

Forchapeatl · 2025-03-04T12:03:05Z

Resolves #7377

Changes:

Fixed an issue where p5.Camera.tilt() did not update the up vector.
Prevents rapid orientation flipping when tilting too far.
Ensures the up vector remains perpendicular by recalculating it dynamically.

Screenshots of the change:

before

AwesomeScreenshot-3_13_2025.9_54_32AM.mp4

after

AwesomeScreenshot-3_13_2025.9_59_50AM.mp4

snippet

let cam;

function setup() {
  angleMode(DEGREES);
  createCanvas(400, 400, WEBGL);
  cam = createCamera();
  cam.setPosition(0,0,0);
}

function draw() {
  background(220);
  let d = 800;

  drawBox(0, d, 0);
}

function drawBox(x, y, z) {
  push();
  stroke("red");
  strokeWeight(10);
  translate(x, y, z);
  line(0, 0, 0, 0, 0, 200);
  stroke("black");
  strokeWeight(1); 
  rotateY(45);
  box(100);
  pop();
}

function mouseMoved(event) {  
  cam.tilt((mouseY-pmouseY)/2);
  
  // Hold down control key to set cam.up correctly:
  if (event.ctrlKey) {
    let forward = createVector(
      cam.centerX - cam.eyeX,
      cam.centerY - cam.eyeY,
      cam.centerZ - cam.eyeZ
    );
    let up = createVector(cam.upX, cam.upY, cam.upZ);
    let right = p5.Vector.cross(forward, up);
    up = p5.Vector.cross(right, forward).normalize();
    cam.camera(
      cam.eyeX, cam.eyeY, cam.eyeZ,
      cam.centerX, cam.centerY, cam.centerZ,
      up.x, up.y, up.z
    );
  }
  
  console.log(cam.upX, cam.upY, cam.upZ);
}

PR Checklist

npm run lint passes
Inline reference is included / updated
Unit tests are included / updated

davepagurek

Thanks for taking this on, I think your approach looks good! Two things before merging:

Since the pan/tilt/etc functions have been a tad buggy in the past, do you think we can add a unit test to check the result of the up vector after a tilt?
Like your other recent PR, I assume this applies to p5 2.0 as well. Once this one is ready, could you also make a second PR into the dev-2.0 branch?

davepagurek · 2025-03-07T12:38:37Z

src/webgl/p5.Camera.js

@@ -2448,17 +2448,33 @@ p5.Camera = class Camera {
    rotatedCenter[1] += this.eyeY;
    rotatedCenter[2] += this.eyeZ;

+    // Compute new up vector to prevent flipping
+    let forward = createVector(


The global createVector function isn't available in instance mode, can we import the vector class directly and do new Vector(...) here instead?

Thank you for the review . I’ll refactor the code to avoid createVector while still keeping it clean and readable

Forchapeatl · 2025-03-13T09:04:23Z

Since the pan/tilt/etc functions have been a tad buggy in the past, do you think we can add a unit test to check the result of the up vector after a tilt?

Yes @davepagurek . I the unit tests have been added. It verifies the correctness of the up vector by computing the expected values after tilt

davepagurek

Looks great, thanks @Forchapeatl! I'll merge this in, then could you also copy the changes into a PR for dev-2.0?

franolichdesign · 2025-03-15T21:55:50Z

I'm glad that the workaround I suggested when I originally reported the tilt bug proved to be of use. To be honest, I assumed that there would be a more efficient way of recalculating the up vector which is why I didn't attempt to apply the fix myself - that and the fact that I do not have a good understanding of the p5 library internals and do not know how to go about applying a fix anyway. Thanks so much Forchapeatl for bothering!

However I have noticed one small inefficiency with the fix as it stands in the codebase. There are a couple of unnecessary calls to Vector.normalize() on intermediate vectors. Only the final version of the up vector needs to be normalized since only the directions of the intermediate cross products matter and these are unaffected by the magnitudes of the input vectors.

franolichdesign · 2025-03-16T15:24:17Z

Just spotted another small inefficiency and even a potential problem with the accepted fix. Currently:

    const rotatedCenter = [
      centerX * rotation.mat4[0] + centerY * rotation.mat4[4] + centerZ * rotation.mat4[8],
      centerX * rotation.mat4[1] + centerY * rotation.mat4[5] + centerZ * rotation.mat4[9],
      centerX * rotation.mat4[2] + centerY * rotation.mat4[6] + centerZ * rotation.mat4[10]
    ];

    rotatedCenter[0] += this.eyeX;
    rotatedCenter[1] += this.eyeY;
    rotatedCenter[2] += this.eyeZ;

    let forward = createVector(
      rotatedCenter[0] - this.eyeX,
      rotatedCenter[1] - this.eyeY,
      rotatedCenter[2] - this.eyeZ
    ).normalize();

    let up = createVector(this.upX, this.upY, this.upZ);
    let right = p5.Vector.cross(forward, up).normalize(); 
    up = p5.Vector.cross(right, forward).normalize(); 

    this.camera(
      this.eyeX, this.eyeY, this.eyeZ,
      rotatedCenter[0], rotatedCenter[1], rotatedCenter[2],
      this.up.x, this.up.y, this.up.z
    );

    this.upX = up.x;
    this.upY = up.y;
    this.upZ = up.z;

This means that:

eye is added to rotatedCenter then eye is subtracted from rotatedCenter to define forward ie there is an extra unnecessary calculation.
this.up is updated after it is used in the call to this.camera() ie the new value for for this.up is not used immediately to update the camera's orientation which could be problematic in some cases.

Would it not be better to have:

    let forward = createVector(
      centerX * rotation.mat4[0] + centerY * rotation.mat4[4] + centerZ * rotation.mat4[8],
      centerX * rotation.mat4[1] + centerY * rotation.mat4[5] + centerZ * rotation.mat4[9],
      centerX * rotation.mat4[2] + centerY * rotation.mat4[6] + centerZ * rotation.mat4[10]
    );

    let up = createVector(this.upX, this.upY, this.upZ);
    let right = p5.Vector.cross(forward, up); 
    up = p5.Vector.cross(right, forward).normalize(); 

    this.upX = up.x;
    this.upY = up.y;
    this.upZ = up.z;

    this.camera(
      this.eyeX, this.eyeY, this.eyeZ,
      forward.x + this.eyeX, 
      forward.y + this.eyeY, 
      forward.z + this.eyeZ,
      this.up.x, this.up.y, this.up.z
    );

EDIT - It looks like the code I've quoted from Camera.js is not the final version as it still includes calls to createVector(). I'm still finding my way around GitHub so please forgive me if my comments above have already been addressed.

Forchapeatl · 2025-03-16T18:54:09Z

Thank you for pointing out those inefficiencies! You're absolutely right about the unnecessary addition/subtraction of the eye coordinates and the delayed up vector update. I've refactored the code to address both of these issues. The optimized version looks like this.

  _rotateView(a, x, y, z) {
    let centerX = this.centerX;
    let centerY = this.centerY;
    let centerZ = this.centerZ;
    // move center by eye position such that rotation happens around eye position
    centerX -= this.eyeX;
    centerY -= this.eyeY;
    centerZ -= this.eyeZ;
    const rotation = p5.Matrix.identity(this._renderer._pInst);
    rotation.rotate(this._renderer._pInst._toRadians(a), x, y, z);

    // Apply the rotation matrix to the center vector
    /* eslint-disable max-len */
    const rotatedCenter = [
      centerX * rotation.mat4[0] + centerY * rotation.mat4[4] + centerZ * rotation.mat4[8],

    /* eslint-enable max-len */

    // Translate the rotated center back to world coordinates
    rotatedCenter[0] += this.eyeX;
    rotatedCenter[1] += this.eyeY;
    rotatedCenter[2] += this.eyeZ;

    // Rotate the up vector to keep the correct camera orientation
    /* eslint-disable max-len */
    const upX = this.upX * rotation.mat4[0] + this.upY * rotation.mat4[4] + this.upZ * rotation.mat4[8];
    const upY = this.upX * rotation.mat4[1] + this.upY * rotation.mat4[5] + this.upZ * rotation.mat4[9];
    const upZ = this.upX * rotation.mat4[2] + this.upY * rotation.mat4[6] + this.upZ * rotation.mat4[10];
    /* eslint-enable max-len */

    this.camera(
      this.eyeX,
      this.eyeY,
      this.eyeZ,
      rotatedCenter[0],
      rotatedCenter[1],
      rotatedCenter[2],
      upX,
      upY,
      upZ
    );
  }

I was also able to avoid the createVector() call for the forward vector, as I'm now just working directly with the individual X, Y, and Z components.

Forchapeatl · 2025-03-16T18:54:15Z

@franolichdesign Would you like to summit the fix on dev2.0 on this . I will guide you.

franolichdesign · 2025-03-16T21:55:21Z

Unless I'm mistaken (which is quite possible!) I think some new issues have been introduced in the refactored code:

The definition of of rotatedCenter is missing a couple of coordinates.
this.upX etc are not being updated.
The updated up vector should be normalized to avoid cumulative rounding errors.

Forchapeatl · 2025-03-16T23:03:38Z

Unless I'm mistaken (which is quite possible!) I think some new issues have been introduced in the refactored code:
1. The definition of of `rotatedCenter` is missing a couple of coordinates.

2. `this.upX` etc are not being updated.

3. The updated `up` vector should be normalized to avoid cumulative rounding errors.

@franolichdesign , Please tell me your optimized approach . I am having a hard time visualizing this comment

Forchapeatl · 2025-03-16T23:07:53Z

@franolichdesign or please maybe help me with a test snippet where this refactored code fails.

davepagurek · 2025-03-17T13:59:54Z

The definition of of rotatedCenter is missing a couple of coordinates.

Thanks for taking a look into this more! I haven't taken a close look at that implementation since it was unchanged in this PR, what bits are missing?

this.upX etc are not being updated.

I think the call to this.camera(...) updates this.upX/Y/Z if you pass in new values, so I think they'll continue to be updated as long as the new values are correct.

The updated up vector should be normalized to avoid cumulative rounding errors.

This is a good point, and worth updating in a follow-up PR if you (or @Forchapeatl) are interested in taking that on!

franolichdesign · 2025-03-17T19:33:23Z

@Forchapeatl Thank you so much for the kind offer to guide me through the process for submitting a fix. Unfortunately, I'm currently not able to set up my old laptop to test any code changes before submitting a fix (I desperately need to get a new computer!)

My suggested alterations to the fix, including your nice optimization for calculating the up vector, would give:

  _rotateView(a, x, y, z) {
    let centerX = this.centerX;
    let centerY = this.centerY;
    let centerZ = this.centerZ;
    // move center by eye position such that rotation happens around eye position
    centerX -= this.eyeX;
    centerY -= this.eyeY;
    centerZ -= this.eyeZ;
    const rotation = p5.Matrix.identity(this._renderer._pInst);
    rotation.rotate(this._renderer._pInst._toRadians(a), x, y, z);

    // Apply the rotation matrix to the center vector
    /* eslint-disable max-len */
    const rotatedCenter = [
      centerX * rotation.mat4[0] + centerY * rotation.mat4[4] + centerZ * rotation.mat4[8],
      centerX * rotation.mat4[1] + centerY * rotation.mat4[5] + centerZ * rotation.mat4[9],
      centerX * rotation.mat4[2] + centerY * rotation.mat4[6] + centerZ * rotation.mat4[10]
    ];
    /* eslint-enable max-len */

    // Translate the rotated center back to world coordinates
    rotatedCenter[0] += this.eyeX;
    rotatedCenter[1] += this.eyeY;
    rotatedCenter[2] += this.eyeZ;

    // Rotate the up vector to keep the correct camera orientation
    /* eslint-disable max-len */
    let up = new p5.Vector(
      this.upX * rotation.mat4[0] + this.upY * rotation.mat4[4] + this.upZ * rotation.mat4[8],
      this.upX * rotation.mat4[1] + this.upY * rotation.mat4[5] + this.upZ * rotation.mat4[9],
      this.upX * rotation.mat4[2] + this.upY * rotation.mat4[6] + this.upZ * rotation.mat4[10]
    );
    /* eslint-enable max-len */
    up.normalize();

    this.camera(
      this.eyeX,
      this.eyeY,
      this.eyeZ,
      rotatedCenter[0],
      rotatedCenter[1],
      rotatedCenter[2],
      up.x,
      up.y,
      up.z
    );
  }

I hope that my use of the p5.Vector() constructor is correct and would work in instance mode.

Forchapeatl · 2025-03-17T22:06:37Z

Thank you @franolichdesign

franolichdesign · 2025-03-19T02:19:15Z

@Forchapeatl Do you have time to test and submit the latest changes? If not, how do we highlight these proposed changes so that another contributor can find and submit them? Should I raise another issue including the new code?

Forchapeatl · 2025-03-19T09:23:06Z

Should I raise another issue with the new code?

Yes , please do , If no one takes it up , then I will do it during the weekend. Thank you for the reminder @franolichdesign

Forchapeatl added 4 commits March 4, 2025 13:02

Fix camera tilt function to prevent orientation flipping

fcae974

Fixed indentation error

daf9642

fixed lint

2128bfc

fixed lint

961dc63

Forchapeatl changed the base branch from main to dev-2.0 March 4, 2025 12:13

Forchapeatl closed this Mar 4, 2025

Forchapeatl reopened this Mar 4, 2025

Forchapeatl changed the base branch from dev-2.0 to main March 4, 2025 12:23

davepagurek reviewed Mar 7, 2025

View reviewed changes

removed create vector error

1ac8e43

Forchapeatl added 3 commits March 13, 2025 10:38

added test on up vector tilt

9d2ba57

removed trailing spaces

870804c

Merge branch 'processing:main' into patch-9

b6cb7a7

davepagurek approved these changes Mar 15, 2025

View reviewed changes

davepagurek merged commit 3783dd8 into processing:main Mar 15, 2025
2 checks passed

This was referenced Mar 29, 2025

Normalize up vector of Camera's rotateView to avoid round / cummulative errors #7680

Open

Normalized up vector of P5Camera tilt (Follow up update of PR #7598) #7681

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix camera tilt function to prevent orientation flipping #7598

Fix camera tilt function to prevent orientation flipping #7598

Forchapeatl commented Mar 4, 2025 •

edited

Loading

davepagurek left a comment

davepagurek Mar 7, 2025

Forchapeatl Mar 13, 2025

Forchapeatl commented Mar 13, 2025 •

edited

Loading

davepagurek left a comment

franolichdesign commented Mar 15, 2025 •

edited

Loading

franolichdesign commented Mar 16, 2025 •

edited

Loading

Forchapeatl commented Mar 16, 2025

Forchapeatl commented Mar 16, 2025 •

edited

Loading

franolichdesign commented Mar 16, 2025 •

edited

Loading

Forchapeatl commented Mar 16, 2025

Forchapeatl commented Mar 16, 2025

davepagurek commented Mar 17, 2025

franolichdesign commented Mar 17, 2025 •

edited

Loading

Forchapeatl commented Mar 17, 2025

franolichdesign commented Mar 19, 2025 •

edited

Loading

Forchapeatl commented Mar 19, 2025

Fix camera tilt function to prevent orientation flipping #7598

Fix camera tilt function to prevent orientation flipping #7598

Conversation

Forchapeatl commented Mar 4, 2025 • edited Loading

PR Checklist

davepagurek left a comment

Choose a reason for hiding this comment

davepagurek Mar 7, 2025

Choose a reason for hiding this comment

Forchapeatl Mar 13, 2025

Choose a reason for hiding this comment

Forchapeatl commented Mar 13, 2025 • edited Loading

davepagurek left a comment

Choose a reason for hiding this comment

franolichdesign commented Mar 15, 2025 • edited Loading

franolichdesign commented Mar 16, 2025 • edited Loading

Forchapeatl commented Mar 16, 2025

Forchapeatl commented Mar 16, 2025 • edited Loading

franolichdesign commented Mar 16, 2025 • edited Loading

Forchapeatl commented Mar 16, 2025

Forchapeatl commented Mar 16, 2025

davepagurek commented Mar 17, 2025

franolichdesign commented Mar 17, 2025 • edited Loading

Forchapeatl commented Mar 17, 2025

franolichdesign commented Mar 19, 2025 • edited Loading

Forchapeatl commented Mar 19, 2025

Forchapeatl commented Mar 4, 2025 •

edited

Loading

Forchapeatl commented Mar 13, 2025 •

edited

Loading

franolichdesign commented Mar 15, 2025 •

edited

Loading

franolichdesign commented Mar 16, 2025 •

edited

Loading

Forchapeatl commented Mar 16, 2025 •

edited

Loading

franolichdesign commented Mar 16, 2025 •

edited

Loading

franolichdesign commented Mar 17, 2025 •

edited

Loading

franolichdesign commented Mar 19, 2025 •

edited

Loading