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.49k stars 3.29k forks source link

Default camera view changed in v1.9? #6694

Open komatebe opened 8 months ago

komatebe commented 8 months ago

Most appropriate sub-area of p5.js?

p5.js version

1.9

Web browser and version

All

Operating System

All

Steps to reproduce this

Has the default camera view in WEBGL mode changed in the latest 1.9 version of p5js? p5js I have noticed a significant difference, and it seems that any sketches created before version 1.9 would require reprogramming to work correctly.

Steps:

  1. Use this code in v1.9
  2. Use this code in v1.8
  3. Spot the difference

Snippet:


function setup() {
  createCanvas(400, 400, WEBGL);
    noStroke();
}

function draw() {
  background(0);
  for(z=0;z<300;z+=50){
    push();
    translate(0,0,z);
    torus(50+z/5,5);
    pop();
  }
}
welcome[bot] commented 8 months ago

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

davepagurek commented 8 months ago

This was an intentional change which is hopefully minor for new sketches. The change was to move from a fixed field of view, which requires the camera position to change when the canvas size changed, to a variable field of view, which left the camera position in place. There are some instructions here on how to call perspective() with a fixed field of view again: https://p5js.org/reference/#/p5/perspective

Everything at z=0 should look the same as before though!

inaridarkfox4231 commented 8 months ago

Please also refer to #6611. The cause is #6216.

inaridarkfox4231 commented 8 months ago

@iadarshrawat We're talking about how it looks, not whether it works. If you don't understand what's being discussed, don't participate in the discussion.

inaridarkfox4231 commented 8 months ago

Simply put, the default value of eyeZ is now 800. As a result, the appearance changes when sketching a small canvas. If you specify camera settings concretely, the problem will not occur, so that is the only countermeasure you can take now.

As you can see from #6611, many examples of reference page were broken. I have modified most of them accordingly using the method I just described.

inaridarkfox4231 commented 8 months ago

Also, isn't the example backwards? This is what happened in my environment. Well, the environment doesn't matter...

ver1.9.0

ver190

ver1.8.0

ver180

In any case, if you want to achieve the desired look with the current specifications, please specify the camera settings specifically.

komatebe commented 8 months ago

Hi guys, thank you for the clarification! Your detailed explanation was very helpful. I don't mind changes as long as they are intentional 😁. Perhaps adding a small footnote on the camera documentation page about this change would be a good idea? This change looks like a good improvement. Also, @inaridarkfox4231 you are right: I have been using flipped examples. Sorry for the confusion.

inaridarkfox4231 commented 8 months ago

I don't care about the example order as it is a trivial matter.

Perhaps adding a small footnote on the camera documentation page about this change would be a good idea? This change looks like a good improvement.

In #6620, I added descriptions to clarify that eyeZ is 800 in several places in the camera reference. If this is not enough, I will consider adding more, but please carefully review the content first.

But the problem is, they won't know that unless they look at the reference page. They may not even realize that it's because the camera's default settings have changed.

Since changing the camera defaults is obviously a destructive specification change, I personally think it would be good if such contents were notified in an appropriate manner when updating the version. That will be a challenge for the future. It is also important that the person making the change has this awareness. I think the problem is that it was missing this time, personally.

If you're on Twitter, you can also tweet to spread the word that the camera default has been changed.

It would be also a good idea to have it brought up in a p5 study session (p5勉強会).

komatebe commented 8 months ago

This change is presented as a fix. But it changed the way how all previous p5 WebGL sketches work. This should have been mentioned with bold letters along with suggestions to either redo sketches or use your perspective fix. The only thing I can do now is to tweet. And I will.

But I am unsure about adding:

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

...to fix the camera example #6620.

It makes the example work but also sends the wrong message that using the camera requires setting the perspective, too. I prefer to keep it simple. The code has been changed, the decision has been made, and examples should be rewritten.

I don't remember how the example looked before, but something like this:

camera(0, 0, 300 + sin(frameCount * 0.03) * 200, 0, 0, 0, 0, 1, 0); 

...is enough to make it work again. As soon as someone fixes the strokeWeight too (but you know that already). 🤯 This just got more complicated than I thought...

Cheers :-)

inaridarkfox4231 commented 8 months ago

You can see the original example by going to the reference page now.

It makes the example work but also sends the wrong message that using the camera requires setting the perspective, too. I prefer to keep it simple. The code has been changed, the decision has been made, and examples should be rewritten.

Unfortunately, with the current specifications, it is not enough to change just the camera to get intended behavior. Because just setting the camera doesn't change the fov. In the previous spec, fov was always fixed at PI/3, but in the current spec, eyeZ is 800, so it is set accordingly. What happens then is that the smaller the canvas, the smaller the fov.

fov

This resulted in the appearance of some reference example being unnecessarily enlarged. Specifically, things like debugMode. debugMode In the debugMode example, only the camera is set, so the situation just described occurs. Therefore, I changed the fov. I can't think of any other way. How would you fix it? As of now, the reference page examples are still broken. Why not check it out for yourself first?

The expression "wrong message" is wrong. With the current specs, if you only change the camera, you will fall into the fov trap. This was not the case before. This is because most people set their camera settings depending on the canvas size. However, that is now a thing of the past. The only way to avoid this is to establish the use of perspective() as the new standard.

To convey this, I've modified the example accordingly. Using perspective() is no longer unavoidable. It is true that it is inconvenient. But there's nothing I can do.

Frankly, I hate this change. I think it is extremely unnatural to have to revise so many examples. All we can do now is better understand camera specs than ever before.

p5.js webgl is no longer suitable for beginners.

inaridarkfox4231 commented 8 months ago

I don't remember how the example looked before, but something like this:

camera(0, 0, 300 + sin(frameCount * 0.03) * 200, 0, 0, 0, 0, 1, 0);

...is enough to make it work again. As soon as someone fixes the strokeWeight too (but you know that already). 🤯 This just got more complicated than I thought...

Which example is this about? I won't know unless you post the link. It's that easy, right? I can't debate, so I can't comment for this.

komatebe commented 8 months ago

6620

inaridarkfox4231 commented 8 months ago

I also tried to spread the word by tweeting. However, the people who responded only gave fav. He won't RT. In other words, it wouldn't spread, so it was no good. It's no good unless someone with a lot of followers who RT does it. 作例の修正が終わりました。 レファレンスの書き換えなどやることが多そうです...

komatebe commented 8 months ago

p5.js webgl is no longer suitable for beginners.

Yup. That is the new reality. I'll see if I can help somehow... Regarding spreading the info - I'll do my best.

inaridarkfox4231 commented 8 months ago

It's not a big problem, and this specification won't change in the future, so I think you can close this issue now.

davepagurek commented 8 months ago

p5.js webgl is no longer suitable for beginners.

Is this just for when you use a custom camera? Examples like the one for rotateX() still work without setting custom perspective -- I don't think it hurts having it, but I don't think it's necessary for all cases.

In the debugMode example, only the camera is set, so the situation just described occurs. Therefore, I changed the fov. I can't think of any other way. How would you fix it? As of now, the reference page examples are still broken. Why not check it out for yourself first?

For this case, a possible simple change is just to change the camera position in the existing call:

image

I think the perspective call also works here, if that helps keep our examples working. But I don't think it's necessary that users need custom perspective when just changing the camera location. That's the main reason why for now we're sticking with the change.

Probably the main case where you might want to edit the FOV is when you have a specific scene that you want to fill the frame, and where you aren't already adapting the scene to the frame size. I could see it being useful to add a camera function that takes in just a field of view, and it grabs the current aspect ratio for you. (This would avoid also moving the camera closer to/farther from the subject when the aspect ratio changes, leaving the camera position to you to manually change. The automatic distance changing was the cause of https://github.com/processing/p5.js/issues/6203 originally and the need to shake things up, for context.) Would that help with some of your concerns?

Garima3110 commented 8 months ago

I could see it being useful to add a camera function that takes in just a field of view, and it grabs the current aspect ratio for you

Well , i agree to this.

Since, the default settings should be designed to work well for a broad range of users and scenarios, providing an intuitive and user-friendly experience for most cases, it recognizes that users may have diverse needs and preferences, and a one-size-fits-all approach might not be ideal. So, I think :The proposed addition of a camera function by @davepagurek that will allow independent customization of the field of view (FOV) (utilising the the current aspect ratio) is seen as a thoughtful solution. This means users can have the option to fine-tune the perspective based on their artistic or scene-specific requirements, without complicating the default settings for those who find them suitable.

inaridarkfox4231 commented 8 months ago

@davepagurek The rotateX() example looks almost the same as when ortho() is declared. This is caused by eyeZ being 800. Nevertheless ortho() is not declared. This may lead beginners to misunderstand that the default setting for projection mode is ortho(). So I fixed it. There was no need to do this with the previous specifications, but I can't help it. You are the one who caused this change in the example in the first place. Some of this was fixed in #6549 before the release of p5.1.9.0, but it was a very crude fix that negatively affected the behavior of orbitControl(). I fixed that. Various other examples were also affected, but you overlooked it. 1.9.0 was released as is. This kind of correction work should originally be done by those of you who changed the specifications. I did that instead of you. I also received a review. It's already over. If there is a problem with the fixed examples, all you have to do is to fix it again. I don't know what the problem is. That's all from me.

inaridarkfox4231 commented 8 months ago

However, with the current specifications, actually calling ortho() will only cause a bug. This was also overlooked during 1.9.0. (Of course I know it has been fixed.)

ortho333

compare

ortho2

ortho3

inaridarkfox4231 commented 8 months ago

@Garima3110 You were in charge of #6549. Why didn't you check the behavior of orbitControl()? The perspective() example is still broken. perspective However, I fixed it in #6620, so it will be fixed in the next version. Is the contributor's job to copy and paste the code suggested by the reviewer? I would appreciate it if you could work more carefully. However, the reviewer also neglected to check the behavior, so it's not only your fault.

If you didn't need orbitControl(), you might have had the option of eliminating that line from that code.

davepagurek commented 8 months ago

I'm mostly interested in seeing what we can do to improve the situation going forward, in two areas. The main one is your sense that p5.js webgl is no longer suitable for beginners. This is something we care a lot about so I want to make sure concerns are addressed. I was trying to get a sense of whether that was just because the versions of some functions with no parameters broke in 1.9.0 (understandably making things harder, but also will be fixed again in the next release) or if there was something more fundamental that needs to be changed that we haven't addressed yet in the fixes that will be released soon. Let me know if there are scenarios like that!

The other area is in prevention of regressions. I don't like to blame individual contributors for not having remembered to check everything--as a reviewer, I certainly don't always remember everything, so I'd prefer to instead look at systems we can improve to help prevent those issues in the future. So far, we're trying to add visual tests to better notice regressions, and we're creating beta builds to help us find issues before they go out to users. I hope these processes can grow and make it easier to make changes confidently. If anyone has more suggestions, feel free to let us know!

inaridarkfox4231 commented 8 months ago

@komatebe You started this discussion, so it's up to you to decide whether to end it. I don't care anymore.

inaridarkfox4231 commented 8 months ago

The people who are disadvantaged by inappropriate reference examples are the people who need those reference examples. The people who are disadvantaged by unnatural default settings are the people who need appropriate default settings. Neither of these apply to me, so it doesn't matter to me.

inaridarkfox4231 commented 8 months ago

In other words, the person who needs the reference example should decide how to properly edit the reference example. There is no meaning in arbitrarily determining it by ours. That'll just end up with nonsence results anyway.

inaridarkfox4231 commented 7 months ago

There was an omission in fixing reference example. I think it's better to deal with it.

6790

lightFalloff() It's really affecting in many ways.

sudhanshuv1 commented 7 months ago

There was an omission in fixing reference example. I think it's better to deal with it. #6790 lightFalloff() It's really affecting in many ways.

Was it needed to include ortho() in that example though? ortho() only makes the objects appear at the same distance from the camera if one of them is far away. In that example both spheres are at the same distance from the camera so probably it isn't necessary to use otho(). Also, including ortho() in the code blacks out the entire canvas and renders nothing except the background color and I'm still trying to ascertain the reason behind that.

inaridarkfox4231 commented 7 months ago

Please also refer to #6611. The cause is #6216. I only mentioned it because the cause is the same. There is no deep meaning. Ask @davepagurek if that's the correct way to fix it. I'm just an ordinary person who has nothing to do with it.

I'm irrelevant.

sudhanshuv1 commented 7 months ago

Please also refer to #6611. The cause is #6216. I only mentioned it because the cause is the same. There is no deep meaning. Ask @davepagurek if that's the correct way to fix it. I'm just an ordinary person who has nothing to do with it.

I'm irrelevant.

Okay thanks I will. :)

davepagurek commented 7 months ago

Also, including ortho() in the code blacks out the entire canvas and renders nothing except the background color and I'm still trying to ascertain the reason behind that.

For why it makes nothing render: the change causing this is an update to the default distance from the camera to the center of the scene. In theory that wouldn't make a difference for a perfect ortho camera, but WebGL/OpenGL ortho cameras generally have a near and far clipping plane (the distance between those planes affects how precisely we can tell when one object is in front of another object): image

Because the camera distance changed, the elements in that scene are now outside of the clipping planes, and don't render.

We actually updated the ortho function (the fix will be released in 1.9.1 soon) so that the default near and far planes work better with the default camera positioning, but ortho isn't integral to that example anyway, so it doesn't hurt to use perspective there too.

sudhanshuv1 commented 7 months ago

Also, including ortho() in the code blacks out the entire canvas and renders nothing except the background color and I'm still trying to ascertain the reason behind that.

For why it makes nothing render: the change causing this is an update to the default distance from the camera to the center of the scene. In theory that wouldn't make a difference for a perfect ortho camera, but WebGL/OpenGL ortho cameras generally have a near and far clipping plane (the distance between those planes affects how precisely we can tell when one object is in front of another object): image

Because the camera distance changed, the elements in that scene are now outside of the clipping planes, and don't render.

We actually updated the ortho function (the fix will be released in 1.9.1 soon) so that the default near and far planes work better with the default camera positioning, but ortho isn't integral to that example anyway, so it doesn't hurt to use perspective there too.

Thanks, I understood it now :)

nickmcintyre commented 7 months ago

I noticed this change over the weekend while tinkering with another project. To echo @komatebe's point about the need for reprogramming, the ray casting example is now broken. Most of the other examples on the p5.js website seem fine except for orbit control.

It's not immediately clear to me that the camera change is unfriendly to beginners–just different. As a quick test, does anybody have a fix for the ray casting example?

nickmcintyre commented 7 months ago

@inaridarkfox4231 I realize I'm pretty late to the discussion, but I wanted to respond to your comment:

In other words, the person who needs the reference example should decide how to properly edit the reference example. There is no meaning in arbitrarily determining it by ours. That'll just end up with nonsence results anyway.

It can be hard to maintain empathy with beginners once you've gained a certain level of expertise (see curse of knowledge). Here's part of a discussion with Tega Brain from her book Code as a Creative Medium:

... There have been times when teaching a class for the first time where I've received some unexpectedly good results and reviews, especially for material slightly outside my skill set. Then once I've taught it a few more times and I know the material really well, I think it gets harder to empathize. After those first couple of times I often feel like I'm actually getting worse at teaching it, which is really not intuitive.

We all started somewhere. Beginners generally don't know what they don't know. It's up to more experienced contributors to create an environment (APIs, docs, etc.) that supports them. If you're not sure how a contribution may impact beginners, ask around. You can also code a few practical examples and imagine explaining them to someone who is encountering the feature for the first time.

davepagurek commented 7 months ago

As a quick test, does anybody have a fix for the ray casting example?

@nickmcintyre I think it works after this small change:

- eyeZ = height / 2 / tan((30 * PI) / 180); // The default distance the camera is away from the origin.
+ eyeZ = 800 // The default distance the camera is away from the origin.

I'll take a pass later through the examples directory to update that + the orbitControl one, thanks for pointing it out!

Edit: PR is up here: https://github.com/processing/p5.js-website/pull/1506