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

WEBGL stroke weight misbehaves after call to perspective() #4214

Closed
4 of 5 tasks
jwdunn1 opened this issue Dec 28, 2019 · 10 comments
Closed
4 of 5 tasks

WEBGL stroke weight misbehaves after call to perspective() #4214

jwdunn1 opened this issue Dec 28, 2019 · 10 comments

Comments

@jwdunn1
Copy link
Contributor

jwdunn1 commented Dec 28, 2019

After perspective() is called, stroke weights become unexpectedly thin near the camera while objects far away are stroked too thickly. I noticed this on another sketch I was building with a roving camera. For reporting purposes, I've created a minimal sketch to illustrate the problem.

Nature of issue?

  • Found a bug

Most appropriate sub-area of p5.js?

  • Core/Environment/Rendering
  • WebGL

Which platform were you using when you encountered this?

  • Mobile/Tablet (touch devices)
  • Desktop/Laptop

Details about the bug:

  • p5.js version: v0.10.2
  • Web browser and version: Chrome 79.0.3945.88, Firefox 71.0, Safari 12.1.1
  • Operating System: Windows 7 Pro, macOS 10.13.6, Android 7
  • Steps to reproduce this:
  1. Run this repro sketch https://editor.p5js.org/jwdunn1/sketches/VGEuB8mTP
  2. Note the stroke weights - these are rendered correctly at this point. Stroke weight is set to 3 for emphasis.
  3. Now, click on the sketch canvas to invoke perspective()
  4. Note the change in stroke weights (items closer to the camera become too thin while those further away become too thick) This is not the expected behavior.
    The stroke weights work correctly with perspective() in v0.9.0 and earlier.

Sketch code is below:

function setup() {
  createCanvas(400, 400, WEBGL);
  strokeWeight(3);
}

function draw() {
  background(220);
  rotateY(PI/8);
  rotateZ(PI/8);
  translate(0,0,300);
  for(let i=0; i<12; i++){
    translate(0,0,-100);
    box();
  }
}
function mousePressed(){
  perspective(PI/3, width/height, 1, 10000);
}
@stalgiag
Copy link
Contributor

stalgiag commented Dec 29, 2019

Thanks for the well-written issue @jwdunn1 !

This is known behavior. Currently the stroke shader doesn't track the amount of perspective that is applied to the sketch. There are only two states, default perspective and custom perspective. Custom perspectives always use orthographic projection on the stroke. In the example you linked, after clicking, the stroke doesn't get larger further from the camera but instead stays the exact same size while the box itself gets smaller.

This behavior begins with this PR which I filed. The PR was a way of fixing broken stroking in custom perspectives. As a blunt solution, stroke perspective was given a simple on or off state. This was done to avoid calculating the mix of perspective to orthographic projection in a user's custom perspective.

That said, if you (or anyone) has interest in adding a calculation of the ortho-to-perspective interpolation of a custom perspective then I'd be happy to review.

@jwdunn1
Copy link
Contributor Author

jwdunn1 commented Dec 31, 2019

Thank you for the information. As an experiment (without knowing the deeper ramifications), I removed the else clause introduced by the PR in p5.Shader.js. With this change, my sketch (which uses a roving camera: EasyCam) now operates as expected with a custom perspective (or ortho) and with correctly scaling and rotating strokes. Do you see a problem with this approach? Could this solution work in the general case?

This is the code change I include at the beginning of my sketch to override the v0.10.2 _setMatrixUniforms method:

p5.Shader.prototype._setMatrixUniforms = function() {
  this.setUniform('uProjectionMatrix', this._renderer.uPMatrix.mat4);
  if (this.isStrokeShader()) this.setUniform('uPerspective', 1);
  this.setUniform('uModelViewMatrix', this._renderer.uMVMatrix.mat4);
  this.setUniform('uViewMatrix', this._renderer._curCamera.cameraMatrix.mat4);
  if (this.uniforms.uNormalMatrix) {
    this._renderer.uNMatrix.inverseTranspose(this._renderer.uMVMatrix);
    this.setUniform('uNormalMatrix', this._renderer.uNMatrix.mat3);
  }
};

Examples with this 'fix':
https://www.openprocessing.org/sketch/811370
https://www.openprocessing.org/sketch/817106

@stalgiag
Copy link
Contributor

@jwdunn1 I don't see a problem with that solution for your sketches. Unfortunately, it isn't a general solution because it is possible for a user to make a sketch that has a projection matrix that breaks this.

Here is an example that shows this.. If you comment out your prototype override you can see that the ortho is more legible than the incorrect perspective.

This is a situation where a fix introduces another problem. The thought with introducing this behavior was that orthographic stroke projection was a safer assumption for custom perspective, but perhaps it is better to assume perspective projection. It is difficult to say which problem will present itself more, or be more obtrusive, but I am open to arguments either way.

The only true fix, as I said in my earlier comment, would be a calculation of where a custom perspective is on an interpolation between orthographic and perspective, but I don't know how to do that easily.

@davepagurek davepagurek moved this to Bugs with No Solution Yet in p5.js WebGL Projects Oct 27, 2023
@Garima3110
Copy link
Member

Garima3110 commented Jan 3, 2024

That said, if you (or anyone) has interest in adding a calculation of the ortho-to-perspective interpolation of a custom perspective then I'd be happy to review.

I have come up with an approach to calculate the interpolation value. Please let me know if I am going in the right direction or not ..!
I think we can use the ratio of the object's distance from the camera to the max distance (maybe using the bounding box approach) ,
And then using the Math.min() function to get the min of the two values , i.e 1 and the calculated ratio.

@davepagurek @stalgiag , I would love to have your suggestions on this ..
Thankyou..!

@davepagurek
Copy link
Contributor

Could you describe the bounding box method you're mentioning @Garima3110? Also, in case it's useful, p5.Camera has a cameraFar property describing the maximum z value that will get rendered.

Another option we have is just to add a way to manually set whether or not lines should have perspective, e.g. linePerspective(true) or linePerspective(false). Someone on discord was recently asking whether or not they could turn off perspective on lines while using the default perspective camera. Maybe it's easier to keep the current defaults but let users pick for themselves if they want?

@Garima3110
Copy link
Member

Garima3110 commented Jan 8, 2024

Could you describe the bounding box method you're mentioning @Garima3110?

I thought maybe we could do something like getting the dimensions of the bounding box, and then finding the max of all these dimensions (length ,breadth ,height) to get the max distance for perspective scaling.
Do we have something like computeBoundingBox() in p5 , actually saw this in threeJS
image
If its not available in p5, I would love to add that feature too.!

@Garima3110
Copy link
Member

Another option we have is just to add a way to manually set whether or not lines should have perspective, e.g. linePerspective(true) or linePerspective(false). Someone on discord was recently asking whether or not they could turn off perspective on lines while using the default perspective camera. Maybe it's easier to keep the current defaults but let users pick for themselves if they want?

This too sounds good, I'll just look into its implementation and get back to you soon..
Thankyou @davepagurek for your suggestions..!

@davepagurek
Copy link
Contributor

I thought maybe we could do something like getting the dimensions of the bounding box, and then finding the max of all these dimensions (length ,breadth ,height) to get the max distance for perspective scaling.
Do we have something like computeBoundingBox() in p5 , actually saw this in threeJS

I don't think we currently do. This seems like a useful feature for other reasons like physics simulations or shape packing, but I'm imagining how it might get used for line perspective. If the perspective changes per object, that probably isn't what we want, as all the objects in the scene would potentially have different line perspectives, which might look inconsistent. If we base it on the bounding box of everything in the scene, then on a single frame things look consistent, but as objects move over time, the bounding box will change, and the line perspective will change accordingly, and then lines might look inconsistent over time, which is also not ideal. So whatever we do, I think it would need to be (by default anyway) consistent across all objects in the scene, and consistent over time as the objects move.

Basing it on camera settings might do that, but also it might just be easier to let users pick for themselves.

@Garima3110
Copy link
Member

Another option we have is just to add a way to manually set whether or not lines should have perspective, e.g. linePerspective(true) or linePerspective(false). Someone on discord was recently asking whether or not they could turn off perspective on lines while using the default perspective camera. Maybe it's easier to keep the current defaults but let users pick for themselves if they want?

Thanks for your insights on this @davepagurek ! I'll just start working on this approach.

Garima3110 added a commit to Garima3110/p5.js that referenced this issue Jan 12, 2024
@Garima3110 Garima3110 mentioned this issue Jan 13, 2024
3 tasks
davepagurek added a commit that referenced this issue Mar 12, 2024
@jwdunn1
Copy link
Contributor Author

jwdunn1 commented Mar 28, 2024

Thank you, this appears to be fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: DONE! 🎉
Development

No branches or pull requests

4 participants