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

implementation of p5.Camera set() function #6238

Closed
1 of 17 tasks
inaridarkfox4231 opened this issue Jun 25, 2023 · 9 comments · Fixed by #6243
Closed
1 of 17 tasks

implementation of p5.Camera set() function #6238

inaridarkfox4231 opened this issue Jun 25, 2023 · 9 comments · Fixed by #6243

Comments

@inaridarkfox4231
Copy link
Contributor

Increasing Access

I would like to have a function like the p5.Vector set function for the camera, so I would like to propose an implementation.

This function copies information such as the eye and center of the argument camera, cameraMatrix, projMatrix to the target camera. If the target is active, update both uMV and uP. Other properties related to projection mode are not copied. So this function is supposed to be used between cameras with the same projection mode.

set(cam) {
  // Copies information about the camera's view.
  // At this time, if this camera is active, uMVMatrix will be updated
  // with this cameraMatrix.
  this.camera(
    cam.eyeX, cam.eyeY, cam.eyeZ,
    cam.centerX, cam.centerY, cam.centerZ,
    cam.upX, cam.upY, cam.upZ
  );
  // Copy the camera's projection matrix.
  for (let i = 0; i < 16; i++) {
    this.projMatrix.mat4[i] = cam.projMatrix.mat4[i];
  }
  // If this camera is active, update uPMatrix.
  if (this._isActive()) {
    for (let i = 0; i < 16; i++) {
      this._renderer.uPMatrix.mat4[i] = this.projMatrix.mat4[i];
    }
  }
}

For example, after moving the camera with orbitControl(), this function is useful when you want to return to the initial state with some interaction.

let cam, initialCam;

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

  // Set the initial state to initialCamera and set it to the camera
  // used for drawing. Then set cam to be the active camera.
  cam = createCamera();
  initialCam = createCamera();
  initialCam.camera(100, 100, 100, 0, 0, 0, 0, 0, -1);
  cam.set(initialCam);

  setCamera(cam);
}

function draw() {
  orbitControl();
  background(255);
  box(50);
  translate(0, 0, -25);
  plane(100);
}

function doubleClicked(){
  // Double-click to return the camera to its initial position.
  cam.set(initialCam);
}
2023-06-25.11-18-08.mp4

In the case of setCamera(), even if you return to the original camera, the camera will move, so the starting point is not fixed, which is inconvenient. There may be other solutions, but I felt most comfortable using the equivalent of p5.Vector's set().

Also, p5.Camera's copy() is private and not open to users, so the specifications for copying camera data are poor, so I felt that it was necessary to improve it.

However, what we want to do here is not make the copy() function available to the user. I'll leave that up to those who are interested. I think this function set() is useful, so I would like to implement it.

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 request details

The details are as follows:

  /**
 * Copies information about the argument camera's view and projection to
 * the target camera. If the target camera is active, it will be reflected
 * on the screen.
 * This function expects both arguments and targets to have the same projection
 * mode. For example, if one is perspective, ortho, frustum, the other must also be
 * perspective, ortho, frustum in each case.
 *
 * @method set
 * @param {p5.Camera} cam source camera
 *
 * @example
 * <div>
 * <code>
 * let cam, initialCam;
 *
 * function setup() {
 *   createCanvas(100, 100, WEBGL);
 *   strokeWeight(3);
 *
 *   // Set the initial state to initialCamera and set it to the camera
 *   // used for drawing. Then set cam to be the active camera.
 *   cam = createCamera();
 *   initialCam = createCamera();
 *   initialCam.camera(100, 100, 100, 0, 0, 0, 0, 0, -1);
 *   cam.set(initialCam);
 *
 *   setCamera(cam);
 * }
 *
 * function draw() {
 *   orbitControl();
 *   background(255);
 *   box(50);
 *   translate(0, 0, -25);
 *   plane(100);
 * }
 *
 * function doubleClicked(){
 *   // Double-click to return the camera to its initial position.
 *   cam.set(initialCam);
 * }
 * </code>
 * </div>
 * @alt
 * Set up the camera and draw a plane and a box.
 * Operate the camera using orbitControl().
 * Double-click to return the camera to its initial state.
 */
  set(cam) {
    // Copies information about the camera's view.
    // At this time, if this camera is active, uMVMatrix will be updated
    // with this cameraMatrix.
    this.camera(
      cam.eyeX, cam.eyeY, cam.eyeZ,
      cam.centerX, cam.centerY, cam.centerZ,
      cam.upX, cam.upY, cam.upZ
    );
    // Copy the camera's projection matrix.
    for (let i = 0; i < 16; i++) {
      this.projMatrix.mat4[i] = cam.projMatrix.mat4[i];
    }
    // If this camera is active, update uPMatrix.
    if (this._isActive()) {
      for (let i = 0; i < 16; i++) {
        this._renderer.uPMatrix.mat4[i] = this.projMatrix.mat4[i];
      }
    }
  }
@asukaminato0721
Copy link
Contributor

    // Copy the camera's projection matrix.
    for (let i = 0; i < 16; i++) {
      this.projMatrix.mat4[i] = cam.projMatrix.mat4[i];
    }

just use

    // Copy the camera's projection matrix.
    this.projMatrix.mat4 = cam.projMatrix.mat4.slice();

ref https://stackoverflow.com/questions/7486085/copy-array-by-value

@inaridarkfox4231
Copy link
Contributor Author

inaridarkfox4231 commented Jun 25, 2023

Perhaps making a copy of the camera and replacing it may accomplish my purpose. However, since the copy() function is private, I think it is still necessary to create a function.

I want to make sure that it's okay, so I'm going to think about it a little more.

@inaridarkfox4231
Copy link
Contributor Author

The motivation for creation is to move the following sketch.

2023-06-26.02-01-15.mp4

This camera's function slerp() creates a view that combines the views of the two cameras. At that time, copy the state at that time to the camera that is the starting point with set(). By interpolating this camera state and the initial camera state, you can smoothly return to the original state from any state.

let cam, lastCam, initialCam;
let countForReset = 30;

// This sample uses orbitControl() to move the camera.
// Double-clicking the canvas restores the camera to its initial state.
function setup() {
  createCanvas(100, 100, WEBGL);

  cam = createCamera();
  lastCam = createCamera();
  initialCam = createCamera();
  
  setCamera(cam);
}

function draw() {
  if (countForReset < 30) {
    countForReset++;
    cam.slerp(lastCam, initialCam, countForReset / 30);
  } else {
    orbitControl();
  }

  background(255);
  strokeWeight(3);
  plane(100);
  translate(0, 0, 50);
  box(20);
}
// A double-click sets countForReset to 0 and initiates a reset.
function doubleClicked() {
  if (countForReset === 30) {
    countForReset = 0;
    lastCam.set(cam);
  }
}

@inaridarkfox4231
Copy link
Contributor Author

I would like to do something like this:

set(cam) {
  // Copy cam and set its properties to the target camera.
  const copyCam = cam.copy();
  Object.keys(copyCam).forEach((keyName) => {
    this[keyName] = copyCam[keyName];
  });

  // If the target camera is active, update uMVMatrix and uPMatrix.
  if (this._isActive()) {
    this._renderer.uMVMatrix.mat4 = this.cameraMatrix.mat4.slice();
    this._renderer.uPMatrix.mat4 = this.projMatrix.mat4.slice();
  }
}

So this function can be used for any camera regardless of the projection mode.
When implementing the interpolation feature introduced in the comments above, the projection mode must match. Rather, we basically assume that the projection matrices match. However, for ortho, the projection matrix is ​​slightly tampered with in orbitControl(), so if there is a difference of that degree, it will be possible to interpolate.

@davepagurek
Copy link
Contributor

I think your motivating example of interpolating between camera states would be great to be able to support! Do you think it could be accomplished by just making copy() public? Because I could support that change too. Otherwise set() makes sense to me.

I think your implementation looks good too, it's nice that it relies on copy() instead of duplicating logic. I'd maybe just double check that Object.keys(copyCam) doesn't accidentally return anything extra -- I think it should be fine, but I'm not 100% sure if Babel adds anything when it transpiles our ES6.

@inaridarkfox4231
Copy link
Contributor Author

It would be interesting to make copy() public, but I don't think it's necessary for my purpose. My goal is to make it as easy as possible for users to understand how to use the camera interpolation function (Right now I call it slerp(), likened to that of vectors. It's not a simple linear interpolation, so I thought it would be more appropriate than lerp().).

Processing using copy() inevitably increases the number of variables and makes it difficult to understand, so instead of making copy() public, I chose to set the state and interpolate between multiple states. Ultimately, I would like to post examples at a reference so that users can easily implement them. I don't know if it will work...

Regarding Bavel, I was originally thinking of doing something like this:

set(cam) {
  this.cameraFOV = cam.cameraFOV;
  this.aspectRatio = cam.aspectRatio;
  this.eyeX = cam.eyeX;
  this.eyeY = cam.eyeY;
  this.eyeZ = cam.eyeZ;
  this.centerX = cam.centerX;
  this.centerY = cam.centerY;
  this.centerZ = cam.centerZ;
  this.upX = cam.upX;
  this.upY = cam.upY;
  this.upZ = cam.upZ;
  this.cameraNear = cam.cameraNear;
  this.cameraFar = cam.cameraFar;

  this.cameraType = cam.cameraType;

  this.cameraMatrix = cam.cameraMatrix.copy();
  this.projMatrix = cam.projMatrix.copy();
  
  // If the target camera is active, update uMVMatrix and uPMatrix.
  if (this._isActive()) {
    this._renderer.uMVMatrix.mat4 = this.cameraMatrix.mat4.slice();
    this._renderer.uPMatrix.mat4 = this.projMatrix.mat4.slice();
  }
}

In fact, We don't need to copy information about renderer and default values, so I think this is enough. The short way of writing it is easy to understand, but it can have unintended consequences, so I'd like to do it this way if possible, but I'm not sure which is better...
During this time, someone else modified my code by using forEach, so I was a little nervous, so I wrote this code. But personally, even if it is a little redundant, I want to write code that is less likely to cause problems.
Even if it's true that Object.keys() returns unintended values, if I don't use Object.keys(), it doesn't matter. So if possible, I would like to do this.
Since it is not necessary to create a new camera object, I think it is also good in terms of load.

@asukaminato0721
Copy link
Contributor

if worried about Object.keys

Object.entries + hasOwnProperty

const o = {
    1: 2, 3: 4
}
const o1 = {}
Object.entries(o)
    .filter(([k, v]) => o.hasOwnProperty(k))
    .forEach(([k, v]) => o1[k] = v)
console.log(o1)

ref

@inaridarkfox4231
Copy link
Contributor Author

inaridarkfox4231 commented Jun 27, 2023

If use copy() as public, I think it will be the following code.

function doubleClicked() {
  if (countForReset === 30) {
    countForReset = 0;
    const copyCam = cam.copy();
    lastCam = copyCam;
  }
}

However, rather than creating a new camera object, I thought that set() would be better because the same object could be reused and the code would be easier to read (copy() is also convenient, so making it public makes sense I think...)

I got advice, I think I'll rewrite it like this:

set(cam) {
  ['eyeX', 'eyeY', 'eyeZ',
   'centerX', 'centerY', 'centerZ',
   'upX', 'upY', 'upZ',
   'cameraFOV', 'aspectRatio', 'cameraNear', 'cameraFar', 'cameraType']
    .forEach((keyName) => { this[keyName] = cam[keyName]; }
  );

  this.cameraMatrix = cam.cameraMatrix.copy();
  this.projMatrix = cam.projMatrix.copy();
  
  // If the target camera is active, update uMVMatrix and uPMatrix.
  if (this._isActive()) {
    this._renderer.uMVMatrix.mat4 = this.cameraMatrix.mat4.slice();
    this._renderer.uPMatrix.mat4 = this.projMatrix.mat4.slice();
  }
}

I could rely on ES6, but I thought it would be easier to read if I explicitly stated the parameters I wanted to copy.
I thought it would be incomplete to copy only the matrix and not the 'eye' or 'aspect', so I thought I would copy them as well.
Because the matrix is ​​created with those data, it is inconsistent. I didn't need to copy other parameters, so I decided to do it this way.

@davepagurek
Copy link
Contributor

Makes sense, I think that implementation of iterating over keys works!

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

Successfully merging a pull request may close this issue.

3 participants