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.08k stars 3.22k forks source link

Should setCamera() also call resetMatrix()? #7056

Closed davepagurek closed 1 month ago

davepagurek commented 1 month ago

Most appropriate sub-area of p5.js?

p5.js version

1.9.3

Web browser and version

All

Operating system

All

Steps to reproduce this

Currently, setting a camera just updates the projection matrix and does not change the model view matrix. I assume that normally this is not an issue because people don't switch cameras in the middle of draw. However, this leads to some confusion when using cameras on framebuffers, which must be applied within draw. If you comment out the resetMatrix call, changing cameras has no effect:

// Double-click to toggle between cameras.

let myBuffer;
let cam1;
let cam2;
let usingCam1 = true;

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

  // Create a p5.Framebuffer object.
  myBuffer = createFramebuffer();

  // Create cameras between begin/end.
  myBuffer.begin();
  // Create the first camera.
  // Keep its default settings.
  cam1 = myBuffer.createCamera();

  // Create the second camera.
  // Place it at the top-left.
  // Point it at the origin.
  cam2 = myBuffer.createCamera();
  cam2.setPosition(400, -400, 800);
  cam2.lookAt(0, 0, 0);
  myBuffer.end();

  describe('A white cube on a gray background. The camera toggles between frontal and aerial views when the user double-clicks.');
}

function draw() {
  // Draw to the p5.Framebuffer object.
  myBuffer.begin();
  if (usingCam1) {
    setCamera(cam1);
  } else {
    setCamera(cam2);
  }
  background(200);
  resetMatrix()
  box();
  myBuffer.end();

  // Display the p5.Framebuffer object.
  image(myBuffer, -50, -50);
}

// Toggle the current camera when the user double-clicks.
function doubleClicked() {
  if (usingCam1 === true) {
    usingCam1 = false;
  } else {
    usingCam1 = true;
  }
}

Live: https://editor.p5js.org/davepagurek/sketches/8uENBvPU3

I think setCamera should also update uMVMatrix, basically also updating the camera position.

nickmcintyre commented 1 month ago

@davepagurek would this approach also affect/reset transformations applied elsewhere in code?

davepagurek commented 1 month ago

I think it would only affect transforms if you're calling setCamera in the middle of draw rather than in setup or in an event handler. But if you were previously calling setCamera mid-draw, arguably the resulting things you draw wouldn't have been in the correct spot anyway since they wouldn't be using the camera's transform yet?

nickmcintyre commented 1 month ago

As long as the reference is clear about where/when to call setCamera() I think we're good. This test case is a little contrived but it surprised me.

davepagurek commented 1 month ago

Right, I guess the rule would be, set the camera before you draw the content?

Also, we have this PR basically ready to start testing as soon as we're done making quick docs releases for the website release: https://github.com/processing/p5.js/pull/6761 I think after this change, since the camera and model transform matrices will not be merged into a single variable, we actually might not need to reset at all, since we'll be able to discern what part came from transforms vs what part came from the camera. So maybe the thing to do is to wait until we can merge and test to see if that refactor fixes this for us? (The reason for the wait is that it's a pretty big change to a core part of the rendering algorithm, which I think warrants a beta testing period before release, which we've been skipping for our expedited docs releases.)

nickmcintyre commented 1 month ago

Agreed that beta testing for this kind of change is a good call.

davepagurek commented 1 month ago

Just put up https://github.com/processing/p5.js/pull/7067 with something that augments https://github.com/processing/p5.js/pull/6761 so that only the view matrix updates when you change cameras. We can play around with that and see if there are any situations that would cause issues