nicklockwood / Euclid

A Swift library for creating and manipulating 3D geometry
MIT License
639 stars 53 forks source link

hasVertexNormals returning incorrect results #125

Closed zilmarinen closed 2 months ago

zilmarinen commented 2 months ago

When creating an instance of SCNGeometry from a given Mesh, the value returned from mesh.hasVertexNormals is false even when normals have been supplied.

It appear this Polygon method is returning false when the normal is equal to the plane which I believe should be a valid scenario?

/// A Boolean value that indicates whether the polygon includes vertex normals that differ from the face normal.
var hasVertexNormals: Bool {
    vertices.contains(where: { !$0.normal.isEqual(to: plane.normal) && $0.normal != .zero })
}

I have created a reproducible example mesh.json to further illustrate the issue.

Here are the console logs when debugging a simple example where the polygon plane normal matches the vertex normals.

po mesh.hasVertexNormals
false

po mesh.polygons.first?.hasVertexNormals
▿ Optional<Bool>
  - some : false

po mesh.polygons.first?.vertices.first?.normal
▿ Optional<Vector>
  ▿ some : Vector(0.0, 1.0)
    - x : 0.0
    - y : 1.0
    - z : 0.0

po mesh.polygons.first?.plane.normal
▿ Optional<Vector>
  ▿ some : Vector(0.0, 1.0)
    - x : 0.0
    - y : 1.0
    - z : -0.0
nicklockwood commented 2 months ago

This isn't a bug - it was a deliberate optimization that was applied because SCNGeometry uses flat shading by default if the normals aren't supplied.

Is this causing a problem for you? If so can you elaborate?

zilmarinen commented 2 months ago

I can appreciate the optimisation for SceneKits default rendering pipeline.

I am using a shader program which expects the normals to be supplied when rendering the geometry. SceneKit kicks out a handful of errors stating that there are no normals so the shader fails to render anything at all.

An example of this is for the mesh provided which is run through a sobel filter as part of a SCNTechnique for edge detection.

Often the geometry I am rendering is comprised of polygons made up of only 3 or 4 vertices, each with a normal that is equal to the plane. I had expected that these would be passed along to the SCNGeometrySource as provided.

Would it be beneficial for me to write a custom initialiser for SCNGeometry to skip these optimisations?

nicklockwood commented 2 months ago

I think it's probably reasonable to just remove this optimization. It's not a huge performance win and I only included it because I thought it would have no impact on real-world use-cases

zilmarinen commented 2 months ago

Closing this issue as this is now resolved by PR 127.