update normal gen tests + fixes

- fix normal gen for varying face types
- fix normalization of face_normals

Author: ffreyer

src/geometry_primitives.jl

@@ -160,15 +160,15 @@ function normals(vertices::AbstractVector{Point{3,T}}, faces::AbstractVector{F};
     return normals(vertices, faces, normaltype)
 end
 
-function normals(vertices::AbstractVector{<:Point{3}}, faces::AbstractVector{F},
-                 ::Type{NormalType}) where {F<:NgonFace,NormalType}
+function normals(vertices::AbstractVector{<:Point{3}}, faces::AbstractVector{<: NgonFace},
+                 ::Type{NormalType}) where {NormalType}
 
     normals_result = zeros(NormalType, length(vertices))
     for face in faces
         v = vertices[face]
         # we can get away with two edges since faces are planar.
         n = orthogonal_vector(v[1], v[2], v[3])
-        for i in 1:length(F)
+        for i in 1:length(face)
             fi = face[i]
             normals_result[fi] = normals_result[fi] .+ n
         end
@@ -198,14 +198,14 @@ end
     FT = ifelse(isconcretetype(F), :($F), :(typeof(f)))
 
     quote
-        normals = sizehint!(Vec3f[], length(fs))
-        faces   = sizehint!(F[], length(fs))
+        normals = resize!(NormalType[], length(fs))
+        faces   = resize!(F[], length(fs))
 
-        for f in fs
+        for (i, f) in enumerate(fs)
             ps = positions[f]
             n = GeometryBasics.orthogonal_vector(ps[1], ps[2], ps[3])
-            push!(normals, n)
-            push!(faces, $(FT)(length(normals)))
+            normals[i] = normalize(n)
+            faces[i] = $(FT)(i)
         end
 
         return FaceView(normals, faces)

test/geometrytypes.jl

@@ -138,16 +138,31 @@ NFace = NgonFace
 end
 
 @testset "Normals" begin
-    # Needs to be reworked
-    @test_broken false
-    
-    # r = centered(Rect3{Float64})
-    # n64 = [n for n in normals(r) for _ in 1:4]
-    # n32 = map(Vec{3,Float32}, n64)
-    # m32 = normal_mesh(r)
-    # m64 = normal_mesh(r, pointtype = Point3d)
-    # @test normals(coordinates(m32), GeometryBasics.faces(m32)) == n32
-    # @test normals(coordinates(m64), GeometryBasics.faces(m64)) == n64
+    # per face normals
+    r = Rect3f(Point3f(0), Vec3f(1))
+    ns = face_normals(coordinates(r), faces(r))
+    ux = unit(Vec3f, 1); uy = unit(Vec3f, 2); uz = unit(Vec3f, 3)
+    @test ns == normals(r)
+    @test values(ns) == [-ux, ux, -uy, uy, -uz, uz]
+
+    # typing
+    ux = unit(Vec3d, 1); uy = unit(Vec3d, 2); uz = unit(Vec3d, 3)
+    ns = face_normals(decompose(Point3d, r), faces(r))
+    @test ns isa FaceView{Vec3d}
+    @test values(ns) == [-ux, ux, -uy, uy, -uz, uz]
+
+    # Mixed
+    c = Cylinder(Point3f(0), Point3f(0,0,1), 0.5f0)
+    ns = normals(c)
+    # caps without mantle
+    f_ns = face_normals(coordinates(c), filter!(f -> f isa TriangleFace, faces(c)))
+    @test all(n -> n == values(ns)[end-1], values(f_ns)[1:15])
+    @test all(n -> n == values(ns)[end], values(f_ns)[16:end])
+    # Mantle without caps
+    v_ns = normals(coordinates(c), filter!(f -> f isa QuadFace, faces(c)))[1:end-2]
+    @test values(ns)[1:15] ≈ v_ns[1:15]
+    @test values(ns)[1:15] ≈ v_ns[16:30] # repeated via FaceView in ns
+
 end
 
 @testset "HyperSphere" begin

test/meshes.jl

@@ -35,7 +35,6 @@ end
 
 @testset "Vertex Index Remapping" begin
     # Sanity Check
-    # TODO: extend
     m = Mesh(
         position = GeometryBasics.FaceView(Point2f[(0, 0), (1, 0), (1, 1), (0, 1)], [QuadFace(1,2,3,4)]),
         normal   = GeometryBasics.FaceView([Vec3f(0,0,1)], [QuadFace(1)])

test/runtests.jl

@@ -44,7 +44,6 @@ end
             @test mesh.normal === normals
             @test mesh.position === points
             @test GeometryBasics.faces(mesh) === tfaces
-            # TODO: Is the order variable with Dict?
             @test propertynames(mesh) == (:vertex_attributes, :faces, :views, :position, :normal, :stress)
         end
     end
@@ -295,12 +294,6 @@ end
     @test <(x, x1)
 end
 
-@testset "Face normals" begin
-    r = Rect3f(Point3f(0), Vec3f(1))
-    ns = normals(r)
-    @test GeometryBasics.face_normals(coordinates(r), faces(r)) == ns
-end
-
 @testset "Tests from GeometryTypes" begin
     include("geometrytypes.jl")
 end