fix: preserve vertex attributes with smooth normals

[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