search

processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.13k stars 3.23k forks source link

Implemented roll function #6819

Open haroon10725 opened 4 months ago

haroon10725 commented 4 months ago

Implemented roll function #6760

haroon10725 commented 4 months ago

Hey @davepagurek please review it. Thanks.

davepagurek commented 4 months ago

I was taking a look at the example, and it seems like the implementation doesn't quite do what a roll actually is. Here, it seems to do something kind of like tilt(), but where the object is kept centered: image

What I would expect it to do is produce something kind of like how rotate() works in 2D (the image below is just an approximation, made using rotate()): image

It looks like that's because _rotateView only modifies the point the camera is looking at (centerX/Y/Z), leaving the camera position (eyeX/Y/Z) and the up vector in place: https://github.com/processing/p5.js/blob/4c13650450c6caaba795c0e5ebb695a4558cb4be/src/webgl/p5.Camera.js#L1052-L1057

I think the correct output of roll() would leave the center and eye the same, but just rotate the up vector. @SableRaf does that make sense, given how much you've been looking into cameras recently?

If that's true, then I think (but am not certain, definitely test this!) that we could edit _rotateView to also apply the rotation matrix to the up vector:

const rotatedUp = [
  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]
];
SableRaf commented 4 months ago

I think the correct output of roll() would leave the center and eye the same, but just rotate the up vector. @SableRaf does that make sense, given how much you've been looking into cameras recently?

Yes! The camera should rotate around its own forward vector.

A good approach would be to compute the camera rotation using quaternions, then compute the up and forward vectors from the resulting rotation and apply them to the camera. This demo by scudly is a good example of how to do it: https://infinitefunspace.com/p5/fly/

I made a minimal example based on it here: https://editor.p5js.org/SableRaf/sketches/WPYb6PRMu

SableRaf commented 1 month ago

@haroon10725 are you still working on this? Otherwise we'll look if someone else would like to tackle it.

haroon10725 commented 1 month ago

@SableRaf I am busy, you can look if some one can solve it.

davepagurek commented 1 month ago

If anyone else is interested in taking this up, feel free to branch off of this PR so that the work @haroon10725 has put in also gets included! A next step could be to try out the suggestion in https://github.com/processing/p5.js/pull/6819#issuecomment-1960584109.

rohanjulka19 commented 3 weeks ago

Hi, Have added the suggested changes with the existing changes of this PR in a new PR here #7093.