feat(webgl): add SEGMENTED mode for true flat shading

[Srinidhi444]

Description

Fixes #9285

This PR introduces a new SEGMENTED mode for computeNormals() to support true flat shading.

Unlike KEEP_CONNECTIONS, which averages normals across connected vertices, SEGMENTED assigns independent face normals to each triangle by selectively duplicating shared vertices.

Related to #9205.

Changes

• Added the SEGMENTED constant.
• Implemented selective vertex duplication, preserving original vertex indices and appending duplicates only when vertices are shared between triangles.
• Updated face indices to reference the duplicated vertices and assigned normalized face normals.
• Preserved existing stroke edge connectivity by keeping original vertex indices unchanged.
• Preserved per-vertex attributes during duplication:
  • UV coordinates
  • Vertex colors
  • Vertex stroke colors
  • Vertex tangents
  • Custom vertex attributes, including multi-component attributes

Tests

Added and updated unit tests covering:

• Selective vertex duplication and correct face normals.
• Updated triangle indices.
• UV and vertex color preservation.
• Stroke edge connectivity and vertex stroke colors.
• Vertex tangent preservation.
• Custom vertex attributes with one and three components.
• Preservation of unused vertices.
• Avoiding unnecessary vertex duplication.
• Repeated calls to computeNormals(SEGMENTED).

Validation: npx vitest run test/unit/webgl/p5.Geometry.js — all tests passed.

[Srinidhi444]

Hey @davepagurek, I've updated the SEGMENTED implementation based on your suggestions.
Instead of rebuilding the entire vertex array, it now preserves the original vertex indices and only appends duplicates when vertices are shared between triangles. This keeps the existing stroke edges intact.

I've also handled the associated per-vertex attributes, including UVs, colors, tangents, and custom vertex attributes, and added the relevant unit tests. All tests are passing.
Let me know if there's anything I've missed or if you'd suggest any improvements. Thanks!

[bhabishnu]

Hey @Srinidhi444, found one thing while testing this with a small loadModel() case.

uvs can be stored as [u, v] pairs, but the new SEGMENTED path slices this.uvs as if it’s always flat. That gives duplicated vertices the wrong UVs and visibly skews the texture. Flattening before slicing fixed it locally, and the existing tests still passed.

A pair-style UV / tiny OBJ regression test should cover it.

Also noticed SEGMENTED is missing from the computeNormals() param docs.

@davepagurek does flattening at this point sound like the right direction?

[Srinidhi444]

Hey @bhabishnu, yeah, I checked it and I think you're right about the [u, v] pair issue. I'm also aware that the documentation needs updating. I was focusing on getting the core logic right and the tests passing first.

I'll address both issues and push the fixes soon. Really appreciate you catching this and taking the time to test it. Thanks!

[bhabishnu]

Thanks, I’m glad it was useful

[Srinidhi444]

Hey @davepagurek, I've addressed the UV handling issue in SEGMENTED mode.

• Fixed the handling of nested UV pairs returned by loadModel() by flattening them before duplicating vertex attributes.
• Added a regression test to ensure UV coordinates remain correctly aligned with duplicated vertices.
• Updated the computeNormals() documentation to include SEGMENTED and explain its behavior.

I also reproduced the issue using an OBJ model and verified the fix. The unit tests are passing.

Please let me know if anything else needs to be changed!

[p5-bot]

Continuous Release

CDN link

• https://raw.esm.sh/pr/p5@20463e1/lib/p5.min.js

Published Packages

• https://pkg.pr.new/p5@20463e1

Commit hash: 20463e1

Previous deployments

3e8db92

• https://raw.esm.sh/pr/p5@3e8db92/lib/p5.min.js
• https://pkg.pr.new/p5@3e8db92

---

This is an automated message.

[davepagurek]

Thanks everyone, this is looking good! @Srinidhi444 it would be great to add a visual test or two in here (maybe with and without strokes?) to confirm visually how it looks.

uvs can be stored as [u, v] pairs, but the new SEGMENTED path slices this.uvs as if it’s always flat. That gives duplicated vertices the wrong UVs and visibly skews the texture. Flattening before slicing fixed it locally, and the existing tests still passed.

@bhabishnu good catch! it would be good to understand in what circumstances it's in this format. I see that we collapse them here before passing it into a buffer:

p5.js/src/core/p5.Renderer3D.js

Lines 302 to 304 in 24a68d5

	new RenderBuffer(2, 'uvs', 'uvBuffer', 'aTexCoord', this, arr =>
	arr.flat()
	),

...but it's definitely more to manage internally if it can be in either format. It looks like some 3D primitives in our codebase use nested uvs and others use flat uvs. I wonder how feasible it'd be, in a follow-up issue, to have a clear separation of user inputs from internal representation so that we can e.g. always use flat uvs internally without having to handle multiple types everywhere. I'm not sure if there's a clear point where we could convert them though, since you can make a new p5.Geometry and set uvs manually without us knowing when it's done being set up... but if anyone has ideas, feel free to raise an issue with a proposal!

[Srinidhi444]

Thanks @davepagurek! I'll add visual tests for SEGMENTED both with and without strokes to verify the flat shading and ensure the existing edges render correctly.

Regarding UVs, I agree that having a consistent internal representation would simplify things across the codebase. Once I've finished this PR, I might raise a follow-up issue and explore the problem in more depth to see how we could approach it.

Thanks everyone, this is looking good! @Srinidhi444 it would be great to add a visual test or two in here (maybe with and without strokes?) to confirm visually how it looks.

uvs can be stored as [u, v] pairs, but the new SEGMENTED path slices this.uvs as if it’s always flat. That gives duplicated vertices the wrong UVs and visibly skews the texture. Flattening before slicing fixed it locally, and the existing tests still passed.

@bhabishnu good catch! it would be good to understand in what circumstances it's in this format. I see that we collapse them here before passing it into a buffer:

p5.js/src/core/p5.Renderer3D.js

Lines 302 to 304 in 24a68d5

	new RenderBuffer(2, 'uvs', 'uvBuffer', 'aTexCoord', this, arr =>
	arr.flat()
	),

...but it's definitely more to manage internally if it can be in either format. It looks like some 3D primitives in our codebase use nested uvs and others use flat uvs. I wonder how feasible it'd be, in a follow-up issue, to have a clear separation of user inputs from internal representation so that we can e.g. always use flat uvs internally without having to handle multiple types everywhere. I'm not sure if there's a clear point where we could convert them though, since you can make a new p5.Geometry and set uvs manually without us knowing when it's done being set up... but if anyone has ideas, feel free to raise an issue with a proposal!

[Srinidhi444]

hey @davepagurek i have added the visual tests please check

[bhabishnu]

I wonder how feasible it'd be, in a follow-up issue, to have a clear separation of user inputs from internal representation so that we can e.g. always use flat uvs internally without having to handle multiple types everywhere.

@davepagurek I dug into this a bit more. Nested UVs seem to come from only a few p5-owned paths (arc()/ellipse(), quad(), and the OBJ/STL loaders), while most other geometry uses flat UVs. buildGeometry() can even mix both.

The renderer mostly hides this because it flattens UVs before upload, but not every consumer does. saveObj() is one case that currently gets the wrong result with nested UVs.

I also couldn't find a reliable point where p5 can say a Geometry is "done" being built, so a shared internal read path that gives consumers flat UVs seems safer than changing how they're stored. I tried that locally and the existing WebGL/loadModel tests still passed.

Changing the producers themselves to always store flat UVs looks trickier since there is existing user code that relies on nested UV access like model.uvs[i][1].

@Srinidhi444 happy to add the repros/details to the follow-up issue if you end up opening one.

[Srinidhi444]

@Srinidhi444 happy to add the repros/details to the follow-up issue if you end up opening one.

Hey, really appreciate the help, man! It would be great if you could add the repros and details. I'll open a follow-up issue once this PR is closed and tag you there. Thanks again!

[bhabishnu]

Sounds good, happy to add the repros/details there once it's open!