[Srinidhi444]

Resolves #9205

Changes:

• Fixed computeNormals(SMOOTH) so it no longer permanently deduplicates geometry vertices based only on position.
• Smooth normals are calculated using temporary position-based vertex groups and then mapped back to the original vertices.
• This preserves per-vertex attributes such as UVs while still allowing vertices at the same position to share smooth normals.
• Added a unit test covering vertices with the same position but different UV coordinates.

Screenshots of the change:

PR Checklist

• npm run lint passes
• Inline reference is included / updated
• Unit tests are included / updated

[p5-bot]

Continuous Release

CDN link

• https://raw.esm.sh/pr/p5@071aeca/lib/p5.min.js

Published Packages

• https://pkg.pr.new/p5@071aeca

Commit hash: 071aeca

Previous deployments

b337eeb

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

4bae8cb

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

---

This is an automated message.

[davepagurek]

Logic looks good! To confirm that the normals themselves haven't changed since before, would you be up for adding a visual test with maybe normalMaterial() on a shape with computeNormals(SMOOTH) applied so that we can more easily visually inspect it? If you need some example geometry, there's an example sketch in the "smooth shading" section of https://p5js.org/tutorials/custom-geometry/ that we could borrow.

[davepagurek]

Nice!

[Srinidhi444]

Logic looks good! To confirm that the normals themselves haven't changed since before, would you be up for adding a visual test with maybe normalMaterial() on a shape with computeNormals(SMOOTH) applied so that we can more easily visually inspect it? If you need some example geometry, there's an example sketch in the "smooth shading" section of https://p5js.org/tutorials/custom-geometry/ that we could borrow.

Logic looks good! To confirm that the normals themselves haven't changed since before, would you be up for adding a visual test with maybe normalMaterial() on a shape with computeNormals(SMOOTH) applied so that we can more easily visually inspect it? If you need some example geometry, there's an example sketch in the "smooth shading" section of https://p5js.org/tutorials/custom-geometry/ that we could borrow.

sure

[Srinidhi444]

Logic looks good! To confirm that the normals themselves haven't changed since before, would you be up for adding a visual test with maybe normalMaterial() on a shape with computeNormals(SMOOTH) applied so that we can more easily visually inspect it? If you need some example geometry, there's an example sketch in the "smooth shading" section of https://p5js.org/tutorials/custom-geometry/ that we could borrow.

hey @davepagurek added the requested visual test for computeNormals(SMOOTH) using normalMaterial() on the warped tube geometry from the smooth shading example.
The visual test passes and the expected screenshot has been generated pls check it and let me know if any issues

[davepagurek]

Hi @Srinidhi444! When you add visual tests, you have to run them locally and commit the image files that get added to the project. The idea is that those images remain checked into source control so that when future changes happen, the tests are rerun and compared to those images to make sure that nothing breaks.

[Srinidhi444]

Hi @Srinidhi444! When you add visual tests, you have to run them locally and commit the image files that get added to the project. The idea is that those images remain checked into source control so that when future changes happen, the tests are rerun and compared to those images to make sure that nothing breaks.

hey @davepagurek my bad didn't knew that i have committed the necessary changes let me know if they are correct.

[davepagurek]

Hmm when I run the tests on CI, it says it expected this:

but received this:

Any idea where the difference comes from? Do you get that when running it locally?

[Srinidhi444]

Hmm when I run the tests on CI, it says it expected this:

but received this:

Any idea where the difference comes from? Do you get that when running it locally?
Hey @davepagurek, I found the issue. It was caused by p5.noise() generating a different noise pattern between test runs, which resulted in different radii and therefore different screenshots. I fixed it by setting a fixed noise seed with p5.noiseSeed(0).

[davepagurek]

Good catch, thanks @Srinidhi444! I think this is good to go.

[davepagurek]

@all-contributors please add @Srinidhi444 for code

[allcontributors]

@davepagurek

I've put up a pull request to add @Srinidhi444! 🎉

[davepagurek]

@Srinidhi444 are you interested in making follow-up issues for the next steps mentioned in #9205 ? Feel free to do so + also work on them if you're up for it!

[Srinidhi444]

@Srinidhi444 are you interested in making follow-up issues for the next steps mentioned in #9205 ? Feel free to do so + also work on them if you're up for it!

@davepagurek yes sure i will understand them first properly and then will come with a approach and then will let you know

[Srinidhi444]

Hey @davepagurek!

I wanted to confirm my understanding of the remaining improvements mentioned in #9205 before creating the follow-up issues.

From what I understand, there are two main changes left:

1. Rename FLAT to KEEP_CONNECTIONS

The first change would be to introduce KEEP_CONNECTIONS for the existing FLAT behavior, while keeping FLAT as a deprecated alias for backward compatibility.

Just wanted to clarify one thing here: is the plan to keep FLAT as a deprecated alias indefinitely, or would we eventually remove it in a future version? Also, should we mark it as deprecated in the documentation for now?

2. Introduce SEGMENTED for true flat shading

The second change would be to implement actual flat shading, which, from my understanding, is somewhat the opposite of what we did for SMOOTH.

My current understanding of the approach is:

• Calculate the normal of each face separately.
• Create independent vertices for each triangle so that shared vertices can have different normals.
• Duplicate the corresponding UVs and other per-vertex attributes to preserve their alignment.
• Assign each triangle's face normal to its own vertices.
• Update the face indices to reference the new vertices.

Something like this, if I'm understanding it correctly:

3. Selective smoothing / sharp edges

I also saw your suggestion about allowing some edges to remain sharp while others are smoothed, possibly through an additional options object.

I haven't explored this part deeply yet, as I think it would require some more thought and investigation. For now, I was planning to focus on the first two improvements and revisit this later.

Regarding the follow-up issues and PRs:

I was thinking of creating two separate issues and two separate PRs:

• One for introducing KEEP_CONNECTIONS and deprecating FLAT.
• Another for implementing SEGMENTED with true flat shading.
  i think that would be clean ,let me know if i have misunderstood something or there is a better approach before creating the issues.

Thanks!

[davepagurek]

Just wanted to clarify one thing here: is the plan to keep FLAT as a deprecated alias indefinitely, or would we eventually remove it in a future version? Also, should we mark it as deprecated in the documentation for now?

Right, we could then remove it in the net major version of p5 (3.0) whenever that happens (sooner than 2.0 came after 1.0 though!) Generally we try to not make breaking changes in minor or patch versions, so while we can nudge people off of old things, people can update minor versions without worrying about having to update old sketches.

Your understanding of SEGMENTED matches mine. Thanks for the diagram too!

Possibly we could create another issue as well for the idea of keeping some edges hard while smoothing others, but we can also wait until we have a more concrete idea. (Could also be worth posting in Discord about it casually in case anyone else has one or is interested in thinking about it.)

Anyway everything you mentioned sounds good, go ahead!

[Srinidhi444]

Just wanted to clarify one thing here: is the plan to keep FLAT as a deprecated alias indefinitely, or would we eventually remove it in a future version? Also, should we mark it as deprecated in the documentation for now?

Right, we could then remove it in the net major version of p5 (3.0) whenever that happens (sooner than 2.0 came after 1.0 though!) Generally we try to not make breaking changes in minor or patch versions, so while we can nudge people off of old things, people can update minor versions without worrying about having to update old sketches.

Your understanding of SEGMENTED matches mine. Thanks for the diagram too!

Possibly we could create another issue as well for the idea of keeping some edges hard while smoothing others, but we can also wait until we have a more concrete idea. (Could also be worth posting in Discord about it casually in case anyone else has one or is interested in thinking about it.)

Anyway everything you mentioned sounds good, go ahead!

Got it, thanks for the clarification @davepagurek! I'll start working on the first two issues.

Regarding the selective smoothing idea (keeping some edges sharp while smoothing others), I was thinking of bringing it up on Discord after the first two changes are done. That way, we can avoid potentially overlapping implementations or unnecessary merge conflicts if someone starts working on it simultaneously.

I'd also be happy to take up that issue later maybe.

Let me know if that sounds good!