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, If ortho() is used without arguments, drawing may fail in some cases #6621

Closed
1 of 17 tasks
inaridarkfox4231 opened this issue Dec 10, 2023 · 15 comments · Fixed by #6639
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. Run ortho() in webgl mode on a 400x400 canvas.
  2. execute box(100).
  3. box is disappear.

Snippet:

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

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

1.8.0

おーそ2

1.9.0

オーソ1

The cause is that the default settings of the camera have been changed due to #6216.
The inconsistency occurs because the default settings for ortho() are still the same as before.
I would like to wait for the solution from someone who is interested.

@diyaayay
Copy link
Contributor

Hey, I would like to work on this issue.

@inaridarkfox4231
Copy link
Contributor Author

I think it will be easier for the reviewer to help if you can provide a concrete plan on how to fix it.

@diyaayay
Copy link
Contributor

@inaridarkfox4231 I will post it as soon as possible.

@inaridarkfox4231
Copy link
Contributor Author

OK. But I'm not a reviewer...sorry.

@inaridarkfox4231
Copy link
Contributor Author

I think I didn't explain it enough, so I'll add more.

The cause is that the far value of ortho() depends on the canvas size. I think you should correct this part.
ortho
The default value of eyeZ is determined here. It 's 800.
_computeCameraDefaultSettings
This value is too large, causing the camera to be too far away.

Changing the default value of ortho() will affect unit tests.
ortho() with no parameters specified (sets default)
So we'll have to fix this as well.

Or there are other solutions. Eliminate this unit test and clearly state in the reference, "Do not use ortho() without arguments." This will prevent users from using ortho() with no arguments, which will prevent the bug from occurring. It would also be a good idea to provide a friendly error. Of course, you need to clearly state in the reference that eyeZ is 800. Otherwise, the user will not be able to decide how to set far value.

It is up to the contributor to decide how to resolve it. My work is just to present ideas.

@diyaayay
Copy link
Contributor

diyaayay commented Dec 14, 2023

@inaridarkfox4231 I tried changing the default far value and increasing it by a factor of 2.
far = Math.max(this._renderer.width, this._renderer.height)*2;

And these are the results:
Before:
Screenshot 2023-12-14 113843

After far = Math.max(this._renderer.width, this._renderer.height)*2; :

Screenshot 2023-12-14 114026

Do you think this could be a possible solution to keep using ortho without arguments?

@inaridarkfox4231
Copy link
Contributor Author

I think it's a good idea. I am not authorized to review. Please wait for a reviewer to find it.

@inaridarkfox4231
Copy link
Contributor Author

I think that method will probably fail when the canvas size is small (100x100, for example), so I think another method is better.

@inaridarkfox4231 inaridarkfox4231 changed the title If ortho() is used without arguments, drawing will fail Due to #6216, If ortho() is used without arguments, drawing will fail Dec 14, 2023
@inaridarkfox4231
Copy link
Contributor Author

Currently, the eyeZ value is fixed at 800 when no camera is specified. Therefore, you could consider specifying far value much larger than 800, such as 1600 or 8000.

Of course, if we specify it with the camera function, we can just set the far value accordingly, so there is no problem. The problem here is what happens when we don't use it. In that case, eyeZ will be 800, and drawing may fail in some cases. The purpose is to avoid that.

@inaridarkfox4231 inaridarkfox4231 changed the title Due to #6216, If ortho() is used without arguments, drawing will fail Due to #6216, If ortho() is used without arguments, drawing may fail in some cases Dec 14, 2023
@diyaayay
Copy link
Contributor

diyaayay commented Dec 14, 2023

I tried adjusting for more values, and it turns out far = Math.max(this._renderer.width, this._renderer.height)*8; works for as small as canvas size(100x100), since the eyeZ value is 800. For the safer side, instead of *8, we can consider an even larger factor as it would cater to the canvas smaller than (100x100), which I'm not quite clear about if we can settle onto a certain low limit of canvas size. For the safer side, I tried far = Math.max(this._renderer.width, this._renderer.height)*10; which works for canvas sizes as small as (80x80). Do you think settling onto a factor value, that shows the drawing for smaller canvas up to a certain limit can be the solution?

@diyaayay
Copy link
Contributor

I will also look into changing the eyeZ value as suggested by @inaridarkfox4231.

@inaridarkfox4231
Copy link
Contributor Author

I think any value is fine. I'll leave the rest to the reviewer.

@inaridarkfox4231
Copy link
Contributor Author

inaridarkfox4231 commented Dec 15, 2023

The easiest solution I can think of is to add 800 to the current value.

    if (far === undefined)
      far = Math.max(this._renderer.width, this._renderer.height) + 800;

We have long been drawing sketches in situations where eyeZ depends on the canvas size. For 100x100, we may have been written as box(50), and for 1000x1000, box(500), and so on. Therefore, it is correct that it depends on the size, but currently eyeZ is fixed at 800. So, the simplest solution is to cancel that amount by adding it up.

I think we can multiply it by 8 or 10, but if they want a large value, they should specify it specifically. That's the user's job.

@diyaayay
Copy link
Contributor

diyaayay commented Dec 15, 2023

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

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

snipfinal1

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

function draw() {
  background(220);
  box(50);
}

snipdinal2

Changes Made:
if (far === undefined) far = Math.max(this._renderer.width, this._renderer.height) + 800;

@davepagurek What do you think?

@davepagurek
Copy link
Contributor

I think shifting the far plane by 800 works, thanks for the suggestion @inaridarkfox4231 and for testing it out @diyaayay!

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