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

Allow p5.Geometry::computeNormals() to work with buildGeometry() outputs #6494

Closed
1 of 17 tasks
davepagurek opened this issue Oct 24, 2023 · 0 comments · Fixed by #6553
Closed
1 of 17 tasks

Allow p5.Geometry::computeNormals() to work with buildGeometry() outputs #6494

davepagurek opened this issue Oct 24, 2023 · 0 comments · Fixed by #6553

Comments

@davepagurek
Copy link
Contributor

Increasing Access

Currently, p5.Geometry includes the pretty useful functionality of generating normals for shapes based on the shape's faces. How p5.Geometry works is still largely undocumented, since it is also used internally and its inner workings may still be subject to change as we try to optimize the performance of the library.

buildGeometry is a way to turn existing p5 shape drawing methods into a p5.Geometry. However, outputs from this method will not include shared vertices within faces, since immediate mode shape building APIs do not include a concept of reusing an existing vertex. If one were to call computeNormals(), it would have flat shading, as only shared vertices between faces will create a smooth normal between those faces.

Updating the implementation of computeNormals() could make smooth shaded custom geometry within grasp for users who can understand the existing shape drawing commands, where currently it needs one to either look into the p5 source code to understand how to use it, or manually set normals with normal(), which is also quite tricky.

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)

Feature enhancement details

A possible approach that was being discussed on Discord is to perform vertex deduplication before computing smooth normals. By collapsing vertices at the same position into one shared index, that vertex will have a smooth normal across all its connected faces:

let vertexIndices = {}
// Maybe add `.toFixed(4)` in here for rounding?
let key = ({ position, uv }) => `${position.x},${position.y},${position.z},${uv.x},${uv.y}`
let vertices = []
let uvs = []
for (let triangle of oldFaces) {
  for (let vertex of triangle) {
    let k = key(vertex)
    if (vertexIndices[k] === undefined) {
      vertexIndices[k] = vertices.length
      vertices.push(vertex.position)
      uvs.push([vertex.uv.x, vertex.uv.y]) // Add uv at the same time as adding vertex
    }
  }
}
let faces = oldFaces.map((triangle) => {
  return triangle.map((vertex) => vertexIndices[key(vertex)])
})
this.vertices = vertices
this.uvs = uvs

A potential API for this could be computeNormals(SMOOTH) or computeNormals(FLAT) (FLAT being the existing behaviour, and SMOOTH doing deduplication beforehand.)

@davepagurek davepagurek moved this to Features Don't Work in All Contexts in p5.js WebGL Projects Oct 27, 2023
@davepagurek davepagurek moved this from Features Don't Work in All Contexts to DONE! 🎉 in p5.js WebGL Projects Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: DONE! 🎉
Development

Successfully merging a pull request may close this issue.

1 participant