[p5.js 2.0+ Bug Report]: computeNormals(SMOOTH) destroys uv attribute

[inaridarkfox4231]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• WebGPU
• p5.strands
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

2.3.3

Web browser and version

Chrome

Operating system

Windows

Steps to reproduce this

Steps:

1. create cylinder with buildGeometry().
2. use computeNormals(SMOOTH).
3. add texture and display this geometry by directional lighting.
4. texture broken.

Snippet:

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

  noStroke();
  const geom=buildGeometry(()=>{
    cylinder(160,400,24,24,0,0);
  });
  geom.computeNormals(SMOOTH);

  const gr = createGraphics(600,600);
  gr.textSize(200);
  gr.background('red');
  gr.fill(255);
  gr.textAlign(CENTER,CENTER);
  gr.text('龍',300,300);
	
  draw=()=>{
    orbitControl();
    background(0);
    lights();
    texture(gr);
    model(geom);
  }
}

computeNormals(FLAT)

computeNormals(SMOOTH)

As shown, using computeNormals() makes it impossible to combine texture-based rendering with lighting.

The point that "using computeNormals() is the problem" is valid. In this example, it is entirely correct.

But what if you want to deform the geometry you have created? For example, you might vary the radius based on the angle from the center to shape the outer edge into a star.
In that case, since calculating the normals directly is difficult, you have to rely on computeNormals(), which leads to this kind of problem.

"the right way is Giving up on rendering using lights" is a valid point. That way, the UVs wouldn't break, and rendering could proceed normally. However, the right to make that decision belongs to the user. Currently, the library holds that right.

Another issue—visible in the console at the bottom left—is the frequent occurrence of errors during computeNormals(); however, as this warrants separate discussion, I will not address it here.

It is possible that this has already been discussed, or that I am simply unaware of a way to resolve it within the current specifications. In that case, I will withdraw this issue. Regardless, I felt it was necessary to raise the point, so I am submitting it here.

[davepagurek]

This has been talked about a bit (with @wagedu on Discord, not on Github yet I think: https://discord.com/channels/836700474425475088/836702229951283281/1194353544627769444), this is definitely something we want to resolve. There are a few issues related to this:

1. SMOOTH ignores all other vertex properties than position/normal
2. FLAT does not actually always look flat (e.g. on the cylinder you show) because it doesn't disconnect triangles sharing vertices
3. There's no in-between to have a few sharp edges

My initial thoughts on how to resolve the first two issues:

• For SMOOTH, continue to calculate normals based on position-deduplicated vertices as we do now, but then rather than actually combining the vertices in the p5.Geometry, update the normals of the initial vertices from the deduplicated geometry so that other vertex properties are preserved and the vertex count remains the same
• Rename FLAT to something like KEEP_CONNECTIONS (deprecate but keep around FLAT for compatibility)
• Make a new SEGMENTED that duplicated shared vertices to make actually flat shading. (This would need to duplicate other vertex properties too to not break anything!)

For (3), I don't have a concrete idea yet -- possibly you could add an optional options object after the smoothing type where you can pass in vertices to remain sharp, essentially whose vertex normals we don't touch? Let me know if you or anyone else have thoughts.

[Srinidhi444]

Hey @davepagurek, I’d like to work on this issue. I’m new to the org and to contributing to open source, so I’d really appreciate any guidance or feedback along the way.

I spent some time going through the issue and the relevant p5.Geometry code, and this is how I currently understand it:

A vertex can have multiple attributes such as position, normal, color, UVs, etc., and p5.Geometry stores these attributes separately. This means two rendering vertices can have the same position but different UVs, for example at a texture seam.

For the first issue with SMOOTH, I understand that the current implementation deduplicates vertices based only on their position. This means vertices that share a position but have different UVs or other attributes can get merged, which breaks the original vertex/attribute relationships.

I agree with the approach you suggested: use a temporary position-based mapping/deduplicated representation only for calculating the smooth normals, and then map those calculated normals back to the original vertices. This would allow us to share the normal calculation for vertices at the same position while preserving the original vertices, UVs, and other attributes.

For the second issue, my understanding is that the current FLAT behavior is really preserving the existing vertex connections rather than guaranteeing true flat shading. So renaming it to something like KEEP_CONNECTIONS, while keeping FLAT as a deprecated alias for backwards compatibility, makes sense to me.

Then for the actual flat/segmented behavior, we would need to treat the shared vertices of different faces as separate vertices and duplicate all associated per-vertex attributes (UVs, colors, custom attributes, etc.) so that we don't break their alignment.

I haven't looked deeply into the sharp-edge/in-between option yet, so I don't have a concrete proposal for that part.

Because these seem like separate changes, I'd like to start with the SMOOTH fix first, and potentially handle the renaming and SEGMENTED behavior in separate PRs.

I'd appreciate your thoughts on whether my understanding is correct. Any feedback on the approach would be really helpful.