Skip to content
This repository has been archived by the owner on Nov 22, 2023. It is now read-only.

Type instability in GLNormalMesh(vertices, faces) #178

Open
gwater opened this issue Aug 14, 2019 · 1 comment
Open

Type instability in GLNormalMesh(vertices, faces) #178

gwater opened this issue Aug 14, 2019 · 1 comment

Comments

@gwater
Copy link

gwater commented Aug 14, 2019

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
Copy link
Member

sjkelly commented Aug 14, 2019

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:

#Should be:
#function call{M <: HMesh, VT <: Point, FT <: Face}(::Type{M}, vertices::Vector{VT}, faces::Vector{FT})
# Haven't gotten around to implement the types correctly with abstract types in FixedSizeArrays
function (::Type{M})(
vertices::AbstractVector{Point{3, VT}}, faces::AbstractVector{FT}
) where {M <: HMesh, VT, FT <: Face}
msh = PlainMesh{VT, FT}(vertices = vertices, faces = faces)
convert(M, msh)
end

I'll polish this up and make a PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants