update merge for MultiFace and views

Author: ffreyer

src/basic_types.jl

@@ -67,11 +67,25 @@ MultiFace{Names}(args...) where {Names} = MultiFace(NamedTuple{Names}(args))
 MultiFace{Names}(args::Tuple{Vararg{<: AbstractFace}}) where {Names} = MultiFace(NamedTuple{Names}(args))
 MultiFace{Names, FT}(args) where {Names, FT <: AbstractFace} = MultiFace(NamedTuple{Names}(FT.(args)))
 
+function MultiFace{Names}(f::MultiFace) where {Names}
+    return MultiFace{Names}(getproperty.((f,), Names))
+end
+
 Base.getindex(f::MultiFace, i::Integer) = Base.getindex(getfield(f, :faces), i)
 @inline Base.hasproperty(f::MultiFace, field::Symbol) = hasproperty(getfield(f, :faces), field)
 @inline Base.getproperty(f::MultiFace, field::Symbol) = getproperty(getfield(f, :faces), field)
 @inline Base.propertynames(f::MultiFace) = propertynames(getfield(f, :faces))
 @inline Base.propertynames(::Type{<: MultiFace{N, T, FT, Names}}) where {N, T, FT, Names} = Names
+Base.eltype(::MultiFace{N, T, FT}) where {N, T, FT} = FT
+Base.eltype(::Type{<: MultiFace{N, T, FT}}) where {N, T, FT} = FT
+
+function simplify_faces(::Type{MF1}, fs::AbstractVector{MF2}) where {MF1 <: MultiFace, MF2 <: MultiFace}
+    return simplify_faces(propertynames(MF1), fs)
+end
+
+function simplify_faces(names::NTuple{N, Symbol}, fs::AbstractVector{MF2}) where {N, MF2 <: MultiFace}
+    return map(f -> MultiFace{names}(f), fs)
+end
 
 # TODO: Some shorthands
 const NormalFace = MultiFace{(:position, :normal)}
@@ -327,12 +341,12 @@ struct Mesh{
 
     vertex_attributes::NamedTuple{Names, VertexAttribTypes}
     connectivity::FaceVecType
-    views::Vector{UnitRange}
+    views::Vector{UnitRange{Int}}
 
     function Mesh(
             va::NamedTuple{Names, VAT}, 
             f::FVT, 
-            views::Vector{UnitRange} = UnitRange[]
+            views::Vector{UnitRange{Int}} = UnitRange{Int}[]
         ) where {
             D, T, FT, Names,
             VAT <: Tuple{AbstractVector{Point{D, T}}, Vararg{AbstractVector}},
@@ -346,7 +360,18 @@ struct Mesh{
 
         if FT <: MultiFace
             f_names = propertynames(FT)
-            if Names != f_names
+            # if Names != f_names
+            #     error(
+            #         "Cannot construct a mesh with vertex attribute names $Names and MultiFace " * 
+            #         "attribute names $f_names. These must include the same names in the same order."
+            #     )
+            # end
+            if Names == f_names
+                # all good
+            elseif issubset(Names, f_names)
+                # remove the extras/fix order
+                f = simplify_faces(Names, f)
+            else
                 error(
                     "Cannot construct a mesh with vertex attribute names $Names and MultiFace " * 
                     "attribute names $f_names. These must include the same names in the same order."
@@ -362,7 +387,7 @@ struct Mesh{
             error("Face vectors that may include `MultiFace`s with different names are not allowed. (Type $FT too abstract.)")
         end
 
-        return new{D, T, FT, Names, VAT, FVT}(va, f, views)
+        return new{D, T, eltype(typeof(f)), Names, VAT, typeof(f)}(va, f, views)
     end
 end
 

src/meshes.jl

@@ -27,12 +27,11 @@ function mesh(primitive::AbstractGeometry; pointtype=Point, facetype=GLTriangleF
         if _f isa AbstractVector{<: MultiFace}
             if facetype isa MultiFace
                 # drop faces that facetype doesn't include
-                names = propertynames(facetype)
-                _f = map(f -> MultiFace{names}(getproperty.((f,), names)), _f)
+                _f = simplify_faces(facetype, _f)
             else
                 # drop faces for vertex attributes that aren't given
                 names = (:position, keys(vertex_attributes)...)
-                _f2 = map(f -> MultiFace{names}(getproperty.((f,), names)), _f)
+                _f2 = simplify_faces(names, _f)
 
                 # and remap to a simple face type so that decompose can handle the rest
                 _f, mappings = merge_vertex_indices(_f2)
@@ -47,7 +46,6 @@ function mesh(primitive::AbstractGeometry; pointtype=Point, facetype=GLTriangleF
     return Mesh(positions, f; vertex_attributes...)
 end
 
-
 const SimpleMesh{N, T, FT} = Mesh{N, T, FT, (:position,), Tuple{Vector{Point{N, T}}}, Vector{FT}}
 const SimpleTriangleMesh{N} = SimpleMesh{N, Float32, GLTriangleFace}
 
@@ -123,41 +121,133 @@ function volume(mesh::Mesh)
     return sum(volume, mesh)
 end
 
+# TODO: Is this ok as "public" function?
+# MultiFace(f1, f2, f3) + (o1, o2, o3) = MultiFace(f1 + o1, f2 + o2, f3 + o3)
+function Base.:+(f::MultiFace{N, T, FT, Names, M}, o::NTuple{M, T}) where {N, T, FT, Names, M}
+    return MultiFace{Names}(ntuple(m -> f[m] + o[m], M))
+end
+
 function Base.merge(meshes::AbstractVector{<:Mesh})
     return if isempty(meshes)
         return Mesh(Point3f[], GLTriangleFace[])
     elseif length(meshes) == 1
         return meshes[1]
     else
-        ps = reduce(vcat, coordinates.(meshes))
-        fs = reduce(vcat, faces.(meshes))
-        idx = length(faces(meshes[1]))
-        offset = length(coordinates(meshes[1]))
-        for mesh in Iterators.drop(meshes, 1)
-            N = length(faces(mesh))
-            for i = idx .+ (1:N)
-                fs[i] = fs[i] .+ offset
-            end
-            idx += N
-            offset += length(coordinates(mesh))
+
+        m1 = meshes[1]
+
+        # Check that all meshes use the same VertexAttributes
+        # Could also do this via typing the function, but maybe error is nice?
+        names = propertynames(m1.vertex_attributes)
+        idx = findfirst(m -> propertynames(m.vertex_attributes) != names, meshes)
+        if idx !== nothing
+            error(
+                "Cannot merge meshes with different vertex attributes. " * 
+                "First missmatch between meshes[1] with $names and " * 
+                "meshes[$idx] with $(propertynames(meshes[idx]))."
+            )
         end
-        return Mesh(ps, fs)
-    end
-end
 
-function Base.merge(meshes::AbstractVector{T}) where T <: MetaMesh
-    isempty(meshes) && return T(Point3f[], GLTriangleFace[])
-    big_mesh = merge(map(Mesh, meshes))
-    big_meta = deepcopy(meta(meshes[1]))
-    for mesh in Iterators.drop(meshes, 1)
-        mm = meta(mesh)
-        for (k, v) in pairs(mm)
-            append!(big_meta[k], v)
+        # We can't merge MultiFace with standard faces because MutliFace allows
+        # desynchronizes vertex indices that normal faces assume synchronized.
+        is_multi = facetype(m1) <: MultiFace
+
+        if all(m -> is_multi == (facetype(m) <: MultiFace), meshes)
+
+            # All the same kind of face, can just merge
+
+            new_attribs = NamedTuple{names}(map(names) do name
+                return mapreduce(m -> getproperty(m, name), vcat, meshes)
+            end)
+            fs = reduce(vcat, faces.(meshes))
+
+            # TODO: is the type difference in offset bad?
+            idx = length(faces(m1))
+            offset = is_multi ? length.(values(vertex_attributes(m1))) : length(coordinates(m1))
+            views = isempty(m1.views) ? [1:idx] : copy(m1.views)
+            
+            for mesh in Iterators.drop(meshes, 1)
+                # update face indices
+                N = length(faces(mesh))
+                for i = idx .+ (1:N)
+                    fs[i] = fs[i] + offset
+                end
+
+                # add views
+                if isempty(mesh.views)
+                    push!(views, idx+1 : idx+N)
+                else
+                    append!(views, (view + idx for view in mesh.views))
+                end
+    
+                idx += N
+                if is_multi
+                    offset = offset .+ length.(values(vertex_attributes(mesh)))
+                else
+                    offset += length(coordinates(mesh))
+                end
+            end
+
+            return Mesh(new_attribs, fs, views)
+
+        else
+
+            # TODO: We can probably simplify this to `merge(merge_vertex_indices.(meshes))`
+            #       but need to check performance
+
+
+            # Varying ace types, need to convert MultiFace
+            new_attribs = NamedTuple{names}(similar.(values(vertex_attributes(m1)), 0))
+            FT = facetype(m1) <: MultiFace ? eltype(facetype(m1)) : facetype(m1)
+            remapped_faces = []
+            new_views = UnitRange{Int}[]
+            vertex_index_counter = eltype(FT)(1)
+
+            for mesh in meshes
+                # convert MultiFace mesh to normal faces, synchronizing vertex indices
+                attribs, fs, views = merge_vertex_indices(
+                    vertex_attributes(mesh), faces(mesh), mesh.views, vertex_index_counter)
+                
+                # increment first vertex index used by faces of the next iteration
+                vertex_index_counter += length(attribs[1])
+                
+                # update merged data
+                for name in names
+                    append!(new_attribs[name], attribs[name])
+                end
+
+                push!(remapped_faces, fs)
+
+                if isempty(views)
+                    push!(new_views, 1:length(fs))
+                else 
+                    append!(new_views, views)
+                end
+            end
+
+            # We did MultiFace -> normal face, now equalize normal face types
+            new_faces = reduce(vcat, remapped_faces)
+
+            return Mesh(new_attribs, new_faces, new_views)
         end
+
     end
-    return MetaMesh(big_mesh, big_meta)
 end
 
+# TODO: Probably not our problem
+# function Base.merge(meshes::AbstractVector{T}) where T <: MetaMesh
+#     isempty(meshes) && return T(Point3f[], GLTriangleFace[])
+#     big_mesh = merge(map(Mesh, meshes))
+#     big_meta = deepcopy(meta(meshes[1]))
+#     for mesh in Iterators.drop(meshes, 1)
+#         mm = meta(mesh)
+#         for (k, v) in pairs(mm)
+#             append!(big_meta[k], v)
+#         end
+#     end
+#     return MetaMesh(big_mesh, big_meta)
+# end
+
 # TODO: naming
 # synchronize_vertex_attributes
 # merge_vertex_(attribute)_indices
@@ -170,10 +260,28 @@ function merge_vertex_indices(mesh)
     return Mesh(attribs, fs, views)
 end
 
+function merge_vertex_indices(
+        attribs::NamedTuple{Names}, 
+        faces::AbstractVector{<: FT},
+        views::Vector{UnitRange{Int}},
+        vertex_index_counter = nothing
+    ) where {Names, FT <: AbstractFace}
+
+    if FT <: MultiFace
+        error(
+            "Failed to call correct method. This likely happened because vertex " *
+            "attributes names $Names do not match face name $(propertynames(first(faces)))."
+        )
+    end
+
+    return attribs, faces, views
+end
+
 function merge_vertex_indices(
         attribs::NamedTuple{Names}, 
         faces::AbstractVector{<: MultiFace{N, T, FT, Names}},
-        views::Vector{UnitRange}
+        views::Vector{UnitRange{Int}},
+        vertex_index_counter = T(1) # TODO: test 0 vs 1 base
     ) where {Names, N, T, FT}
 
     # Note: typing checks for matching Names
@@ -186,17 +294,16 @@ function merge_vertex_indices(
 
     new_attribs = NamedTuple((Pair(k, similar(v, 0)) for (k, v) in pairs(attribs)))
     new_faces = similar(faces, FT, 0)
-    new_views = UnitRange[]
+    new_views = UnitRange{Int}[]
 
+    # TODO: this depends on T in Face (1 based -> 1, 0 based -> 0)
     for idxs in views
-        # TODO: this depends on T in Face (1 based -> 1, 0 based -> 0)
-        vertex_index_counter = T(length(new_attribs[1]) + 1)
-
         # Generate new face from current view, with the first vertex_index 
         # corresponding to the first vertex attribute added in this iteration
         face_view = view(faces, idxs)
         new_faces_in_view, vertex_map = merge_vertex_indices(face_view, vertex_index_counter)
-        
+        vertex_index_counter += length(vertex_map)
+
         # update vertex attributes
         for (name, indices) in pairs(vertex_map)
             append!(new_attribs[name], view(attribs[name], indices))
@@ -211,6 +318,17 @@ function merge_vertex_indices(
     return new_attribs, new_faces, new_views
 end
 
+function merge_vertex_indices(
+        faces::AbstractVector{FT},
+        vertex_index_counter = T(1)
+    ) where {N, T, FT <: AbstractFace{N, T}}
+
+    @assert !(FT <: MultiFace) "Dispatch failed?"
+    
+    N_vert = mapreduce(f -> max(f), max, faces)
+    return faces, (1:N_vert) .+ vertex_index_counter
+end
+
 function merge_vertex_indices(
         faces::AbstractVector{<: MultiFace{N, T, FT, Names, N_Attrib}},
         vertex_index_counter = T(1)