JuliaGeometry / GeometryTypes.jl

Geometry types for Julia
Other
67 stars 41 forks source link

Type instability in GLNormalMesh(vertices, faces) #178

Open gwater opened 5 years ago

gwater commented 5 years ago

GLNormalMesh(vertices, faces) is not type stable.

How to reproduce:

verts = [Point{3}(rand(Float32, 3)...) for i in 1:3]
faces = [Triangle(1, 2, 3)]
@code_warntype GLNormalMesh(verts, faces)

(tested in GeometryTypes v0.7.5)

sjkelly commented 5 years ago

It seems like the round-tripping from SimpleMesh to HomogenousMesh is adding ambiguity. The following fixes the type instability, but breaks a call to HomogenousMesh:

function (::Type{M})(
               vertices::AbstractVector{Point{3, VT}}, faces::AbstractVector{FT}
           ) where {M <: HMesh, VT, FT <: Face}
     NV = vertextype(M)
    NF = facetype(M)
    M(convert(Vector{NV}, vertices), convert(Vector{NF}, faces))
end

Compared to: https://github.com/JuliaGeometry/GeometryTypes.jl/blob/9294dcba10e7fbb4b6f504b6c56d3fcbc3bc5ca7/src/meshes.jl#L79-L87

I'll polish this up and make a PR.