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

Due to #6216, some of the reference examples related to cameras are broken. #6611

Closed
1 of 17 tasks
inaridarkfox4231 opened this issue Dec 7, 2023 · 15 comments · Fixed by #6620
Closed
1 of 17 tasks

Comments

@inaridarkfox4231
Copy link
Contributor

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)

p5.js version

1.9.0

Web browser and version

Chrome

Operating System

Windows11

Steps to reproduce this

Steps:

  1. use orbitControl() on reference page of ortho().
  2. Then the object flies off somewhere.

https://p5js.org/reference/#/p5/ortho

Snippet:

//drag the mouse to look around!
//there's no vanishing point
function setup() {
  createCanvas(100, 100, WEBGL);
  ortho(-width / 2, width / 2, height / 2, -height / 2, 0, 500);
  describe(
    'two 3D boxes move back and forth along same plane, rotating as mouse is dragged.'
  );
}
function draw() {
  background(200);
  orbitControl();
  normalMaterial();

  translate(0,0,500);
  rotateX(0.2);
  rotateY(-0.2);
  push();
  translate(-15, 0, sin(frameCount / 30) * 65);
  box(30);
  pop();
  push();
  translate(15, 0, sin(frameCount / 30 + PI) * 65);
  box(30);
  pop();
}
2023-12-07.10-14-19.mp4

The cause is due to translate(). This causes the cube to be drawn far up in the sky, so when you rotate the camera around the viewpoint using orbitControl(), the object disappears from view.

In the current specifications, simply calling ortho() does not change the viewpoint position as it is initially set. In this case, it will remain at 800.

ortho800

Value of eye without ortho():

pers800

So, for example, by setting the far value to 1000, you can prevent the object from disappearing.

2023-12-07.10-17-16.mp4

However, I can understand why they would want to do translate(0, 0, 500). I'm just guessing since I'm not involved.
Since it is a parallel projection, I think this indicates that the appearance does not change even if the drawing position moves up or down. Probably.
However, it is also true that because of this, orbitControl() actually behaves like a bug.
Therefore, I think some kind of modification is necessary, such as cutting orbitControl(). But I don't know what kind of fix it will be.

As suggested here, I am not suggesting that you should stop using translate() and set far to 1000. The problem is the unnatural behavior of orbitControl().
However, since there is no problem with orbitControl(), I think that some other modification is necessary.

In addition, it seems that a similar problem occurs with perspective().
https://p5js.org/reference/#/p5/perspective

I don't understand why the eyeZ default is uniformly 800. Previously, the value should have been based on the canvas size.
Looks like it was changed in #6216. I don't understand why this was allowed. It feels unnatural to always have 800. I don't understand.

After that, it seems that a change was made to #6549 to add this translate(0,0,500). However, because of this, orbitControl() has become a bug. Was this the right solution to do? I don't know.

@inaridarkfox4231
Copy link
Contributor Author

Also, since eyeZ has become 800, the behavior of the default ortho() has become strange.

function setup() {
  createCanvas(400, 400, WEBGL);
  ortho();
  const cam = this._renderer._curCamera;
  console.log(cam.cameraFar);
  console.log(cam.eyeZ);
}

function draw() {
  orbitControl();
  background(220);
  box(100);
}

400800

The reference page may have been able to do something with a band-aid solution, but should the ordinary user be asked to do the same? I think some kind of response is necessary. This is a bug no matter how you look at it.

@inaridarkfox4231
Copy link
Contributor Author

The same goes for frustum().
https://p5js.org/reference/#/p5.Camera/frustum

The same goes for the first example of slerp(). This example was created taking into account that the viewpoint position is the previous version's position, so the behavior has changed significantly.
https://p5js.org/reference/#/p5.Camera/slerp

And nowhere in the reference is it stated that eyeZ defaults to 800 regardless of the canvas size. Why...

@inaridarkfox4231 inaridarkfox4231 changed the title Object disappears when using orbitControl() on reference page of ortho() Object disappears when using orbitControl() on reference page of ortho(), perspective(), frustum() Dec 7, 2023
@inaridarkfox4231
Copy link
Contributor Author

In summary, there are two points:

For example... for "perspective" example can be modify as:

// drag the mouse to look around!

let cam;

function setup() {
  createCanvas(100, 100, WEBGL);
  // create a camera
  cam = createCamera();
  cam.camera(0, 0, 50*sqrt(3), 0, 0, 0, 0, 1, 0);
  // give it a perspective projection
  cam.perspective(PI / 3.0, width / height, 0.1, 500);
}

function draw() {
  background(200);
  orbitControl();
  normalMaterial();

  rotateX(-0.3);
  rotateY(-0.2);

  push();
  translate(-15, 0, sin(frameCount / 30) * 35);
  box(30);
  pop();
  push();
  translate(15, 0, sin(frameCount / 30 + PI) * 35);
  box(30);
  pop();
}

@davepagurek
Copy link
Contributor

davepagurek commented Dec 7, 2023

And nowhere in the reference is it stated that eyeZ defaults to 800 regardless of the canvas size. Why...

I believe it says in here https://p5js.org/reference/#/p5/perspective, but putting this elsewhere too would be helpful.

I don't understand why the eyeZ default is uniformly 800. Previously, the value should have been based on the canvas size. Looks like it was changed in #6216. I don't understand why this was allowed. It feels unnatural to always have 800. I don't understand.

This was to address the issue that the camera position was updating inconsistently on window resize, since the camera distance from its target was, in part, defined by the aspect ratio of the window. Using a fixed distance removes the need to change the distance when the camera changes, and the perspective changes that happen when resizing a window. As you've noted, affects the near/far planes used, and existing behaviour.

I think to address the issues, we would either need to:

  • Revert the change and add also handle updating the camera position whenever the window aspect ratio changes. This was initially not implemented because it gets complicated, but it is still an option if anyone is willing to work on it!
  • Adjust the examples and ortho() and frustum() defaults to accommodate the default camera distance, and improve documentation

@davepagurek
Copy link
Contributor

For orbit control flying away: I think that's because it orbits around the origin, and the translate() in the examples messes that up. maybe we need to remove the translation in the examples and change the near and far planes instead?

@inaridarkfox4231
Copy link
Contributor Author

inaridarkfox4231 commented Dec 7, 2023

For orbit control flying away: I think that's because it orbits around the origin, and the translate() in the examples messes that up. maybe we need to remove the translation in the examples and change the near and far planes instead?

I agree. You can prevent objects from disappearing by increasing the far clip of the camera, so there is no need to use the translate() function. orbitControl() will also work correctly (but need to check, of course).

However, for reference examples such as pan(), I think it is better to move the camera eye position closer. If the viewpoint is (0,0,800) on a 100x100 canvas, the movement will be almost parallel, and it will be difficult to understand the behavior of shaking the camera left and right.

Another point I forgot to mention earlier is that the default far clip for ortho() has not been changed, so the object disappears when you run ortho() by default.
eyeZ is larger than camera's far clip

@inaridarkfox4231
Copy link
Contributor Author

I just noticed that the example for the camera() function is also broken.

https://p5js.org/reference/#/p5/camera

かめら=====

The default values ​​listed here are wrong, right? It should be fixed at 800. I ran it and confirmed it.
It seems that the cause is that cameraNear is 80. Some modification will be necessary.

I have no objection to setting eyeZ's default to 800 across the board. However, I think we should have paid a little more attention to each area. Changing the default value is a breaking change. I think so.

@inaridarkfox4231
Copy link
Contributor Author

I just took a quick look at it, and I see that most of the camera function examples are broken...I wonder why they didn't look into it properly.
https://p5js.org/reference/#/p5.Camera

In my opinion, resizing the camera is not a feature we always want. Therefore, shouldn't the impact on users who use camera ordinary be minimized? But I don't know what is the best thing to do. Because I haven't participated in the camera resizing discussion. Because I'm not interested.

@davepagurek
Copy link
Contributor

Right, looks like a lot of the interactivity of the examples were missed.

I have no objection to setting eyeZ's default to 800 across the board. However, I think we should have paid a little more attention to each area. Changing the default value is a breaking change. I think so.

Yeah, it definitely is a behaviour change. It's always a balance between fixes that are perfectly backwards compatible, and fixes that are practical for contributors to implement. It's not always obvious ahead of time what the right balance is unfortunately, so we won't always get it right in retrospect.

In my opinion, resizing the camera is not a feature we always want. Therefore, shouldn't the impact on users who use camera ordinary be minimized? But I don't know what is the best thing to do. Because I haven't participated in the camera resizing discussion. Because I'm not interested.

Minimizing that impact makes sense, but also I think users generally want the camera view to not be stretched, so some amount of updating needs to happen. I've seen artists on Discord discussing how best to make their art responsive, so they need the resizing ability, and also need the view to be the same after resize + reload (so whatever position changes are imposed on load also have to happen on resize) and had to implement custom cameras before. None of you are wrong, all of it matters, but unfortunately it's a tough one to get fully correct.

It might mean that the best case scenario is to revert + try to find someone to invest the time into dealing with all the edge cases on resize when using fixed FoV rather than fixed distance.

@inaridarkfox4231
Copy link
Contributor Author

I will summarize the problems.

broken examples

  • p5.Camera, p5.Camera/pan
    Because the camera is too far away, the behavior is mostly parallel movement. It needs to make it easier to see that it is swinging from side to side by moving the camera closer.
  • p5.Camera/tilt
    It has almost the same problem.
  • p5.Camera/perspective, p5.Camera/ortho, p5.Camera/frustum
  • perspective, ortho, frustum
    OrbitControl() does not work properly because of the translate() function. It needs to avoid using the translate() function. It will also need to make adjustments such as adjusting the far clip so that the object does not disappear.
  • p5.Camera/camera, camera
    The object disappears because the near clip is 80. Clips need to be adjusted.
  • setCamera, createCamera
    It might not be a bug, but the camera is too close or the object is zoomed in too much somehow.
    This may also be affected by the clip.
  • p5.Camera/setPosition
    This may also be affected by the clip. Object disappear.
  • p5.Camera/set
    This example also looks strange because it was created based on the previous version's perspective settings.
  • p5.Camera/slerp
    The first example has the same problem as pan(). It needs to move the camera closer.
  • debugMode, noDebugMode
    This also requires adjusting perspective(). The default appearance is strange.

Most of the problems with perspective() seem to be resolved by applying the previous settings as they are. Because it is designed with that in mind.

Problem with ortho() default settings

For example, if you run ortho() with no arguments on a 400x400 canvas, the object will disappear because the camera is too far away.

400800

Measures such as adjusting the default value of farClip are required.

@inaridarkfox4231 inaridarkfox4231 changed the title Object disappears when using orbitControl() on reference page of ortho(), perspective(), frustum() Due to #6216, some of the reference examples related to cameras are broken. Dec 8, 2023
@inaridarkfox4231
Copy link
Contributor Author

inaridarkfox4231 commented Dec 8, 2023

Probably most examples can be solved by applying one or both of the following: Applies to the global camera or default camera.

  camera(0, 0, 50*sqrt(3), 0, 0, 0, 0, 1, 0);
  perspective(PI/3, 1, 5*sqrt(3), 500*sqrt(3));

Because it was designed with that in mind. In any case, the current camera default settings are unnatural when drawing on a 100x100 canvas, so we have no choice but to return to the previous state.

current state:

  camera(0, 0, 800, 0, 0, 0, 0, 1, 0);
  perspective(PI/25.165, 1, 80, 8000);

@inaridarkfox4231
Copy link
Contributor Author

I would like to express my thoughts on camera resizing.
text3D_1.9.0

2023-12-09.09-49-40.mp4

A canvas in webgl is like a TV screen. Changing the size of the canvas while a program is running is like watching a show on TV and then watching it continue on your mobile phone. The screen size changes, but the appearance remains the same.
Therefore, it makes sense to configure the camera settings that do not depend on the size of the canvas. If the model data has a height of 2, use a smaller number, for example. This sketch also uses concrete numbers.

However, this is something users should be aware of when creating 3D sketches that are frequently resized. It's not something the p5.js library forces them to do. The p5.js library can only help with common requirements when resizing camera. I don't know what that is, but...

If resizing is not performed, the default settings for traditional cameras (settings that depend on canvas size) make sense. This is because the strength of p5.js is that even webgl can be treated like 2D. You can write code intuitively.

Above all, this specification change (80,800,8000system) may break the previous sketch. In fact, the reference example is broken. The reference example also serves as a model for beginners, so I think it's a problem if it breaks.

However, the broken example was never fixed by the 1.9.0 update, and the camera's default settings were never changed. That's why I'm opening this issue now.

@inaridarkfox4231
Copy link
Contributor Author

As for pull requests...
As mentioned above, if there is no problem with going back to camera settings that depend on the 100x100 size, I will work on it. Of course, I'll eliminate the translate() function, etc., and simply add the camera settings to the conventional example. Also, there is much lack of description about the 80,800,8000 camera system, so I will fill in that as appropriate.

If there's a better way to do it, I'll leave it up to the person who wants to do it.

Alternatively, if you want to restore the camera's previous default settings and hold off on resize system, I would like to leave this to someone else, too. This includes changes to the unit test, but I don't want to be responsible for it. Once the specifications have changed, this will be a destructive change, so I would like to leave it to someone else. I don't want to end up with something like what happened with angleBetween().

@davepagurek
Copy link
Contributor

As mentioned #6611 (comment), if there is no problem with going back to camera settings that depend on the 100x100 size, I will work on it. Of course, I'll eliminate the translate() function, etc., and simply add the camera settings to the conventional example. Also, there is much lack of description about the 80,800,8000 camera system, so I will fill in that as appropriate.

Just to confirm, this just involves specifying the camera location with camera(...) for each example, right? That sounds like a good way to get these examples into a working state, feel free to make that change!

@inaridarkfox4231
Copy link
Contributor Author

OK. I got it
I will describe what I will be working on by pull request.
Returns the appearance of all camera-related reference examples to their previous state. That is, it restores the camera settings depending on the canvas size of 100x100. Also add some notes about the new 80,800,8000 camera system.

I'll leave it to others to modify the default settings of ortho(). Namely, to fix the following bugs:

400800

Because it's outside the scope of this issue. Also, since it involves changing the unit test, I don't want to be responsible for it. The reason I'm working on this issue is because a bug in the reference example involves orbitControl(). I didn't want contributor to get the wrong idea that the bug was caused by ortbiControl(), so I decided to fix it in advance. Therefore, I want to do as little as possible other than fixing example. I don't want to take responsibility for this situation because I didn't cause it.

If you prefer a fixed field of view, follow these steps:
perspective

I still have my doubts about requiring everyone who draws small-sized sketches to go through this tedious process, but I can't help it.

I'll create a pull request.

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.

2 participants