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

Small bug with triangle() orientation in webgl #6262

Closed
2 of 17 tasks
inaridarkfox4231 opened this issue Jul 10, 2023 · 0 comments · Fixed by #6263
Closed
2 of 17 tasks

Small bug with triangle() orientation in webgl #6262

inaridarkfox4231 opened this issue Jul 10, 2023 · 0 comments · Fixed by #6263

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.6.0

Web browser and version

Microsoft Edge

Operating System

Windows11

Steps to reproduce this

Steps:

  1. Prepare pointLight()
  2. Specify the vertices of triangle() in order for clockwise and counterclockwise, and draw
  3. Normals match and lighting is applied in both cases

Snippet:

triangle()_debug

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

  noStroke();
  pointLight(color("lime"), 0, 0, 50);

  // clockwise orientation. normal: (0, 0, 1)
  quad(-200, -200, 0, -200, 0, 0, -200, 0);
  // counter-clockwise orientation. normal: (0, 0, -1)
  quad(200, -200, 0, -200, 0, 0, 200, 0);

  // clockwise orientation. normal: (0, 0, 1)
  triangle(0, 0, 0, 200, -200, 200);
  // counter-clockwise orientation. normal: (0, 0, 1), the same.
  triangle(0, 0, 0, 200, 200, 200);
}

output:

triquad

I previously fixed the triangle() bug in webgl in #5977. However, I had some misunderstandings.
First, regarding beginShape() to endShape(), the direction of the normal line is the front direction regardless of the order in which the vertices are specified.

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

  noStroke();
  pointLight(color("lime"), 0, 0, 50);

  beginShape(TRIANGLES);
  vertex(0, 0);
  vertex(0, 200);
  vertex(-200, 200);
  vertex(0, 0);
  vertex(0, 200);
  vertex(200, 200);
  endShape();
}

beenrrriei

This is also true for 1.4.0. Therefore, it was not suitable for comparison. quad() should be compared. As you can see from the code introduced above, quad() correctly calculates the normals for the specified order of vertices.
But triangle() does not. Regardless of the specified order of vertices, the normal is always toward front.

In fact, in previous versions, the normals were always toward opposite to front regardless of the order in which the vertices were specified. I wasn't aware of this.
You can see this by running the above code on 1.4.0 and 1.5.0.

previour

It's convenient to have the lights brighter regardless of the order specified, but it's not consistent to quad(), so it might be better to fix it.

Fixing it is easy. This is because the (3,3) component is always 1 in the matrix internally used to draw triangles used to temporarily change the uMVMatrix. Therefore, the result of the normal calculation is always (0,0,1).

triangle

    const mult = new p5.Matrix([
      x2 - x1, y2 - y1, 0, 0, // the resulting unit X-axis
      x3 - x1, y3 - y1, 0, 0, // the resulting unit Y-axis

      0, 0, 1, 0,             // the resulting unit Z-axis (unchanged)

      x1, y1, 0, 1            // the resulting origin
    ]).mult(this.uMVMatrix);

Change this to reflect the order in which vertices are specified.
This computes the z component of the cross product of the vectors.

    const orientation = Math.sign(x1*y2-x2*y1 + x2*y3-x3*y2 + x3*y1-x1*y3);
    const mult = new p5.Matrix([
      x2 - x1, y2 - y1, 0, 0, // the resulting unit X-axis
      x3 - x1, y3 - y1, 0, 0, // the resulting unit Y-axis

      0, 0, orientation, 0,   // the resulting unit Z-axis (unchanged)

      x1, y1, 0, 1            // the resulting origin
    ]).mult(this.uMVMatrix);

    this.uMVMatrix = mult;

Now the normal directions will be calculated according to the specified order.
true

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.

1 participant