Improve HalfEdgeTopology(::Vector{<:Connectivity}) correctness

src/topologies/halfedge.jl

@@ -139,89 +139,138 @@ function HalfEdgeTopology(halves::AbstractVector{Tuple{HalfEdge,HalfEdge}}; nele
   HalfEdgeTopology(halfedges, half4elem, half4vert, edge4pair)
 end
 
+function any_edges_exist(inds, half4pair)
+  n = length(inds)
+  for i in eachindex(inds)
+    uv = (inds[i], inds[mod1(i + 1, n)])
+    if haskey(half4pair, uv)
+      return true
+    end
+  end
+  return false
+end
+
+const NULLEDGE = HalfEdge(0, nothing)
+function any_claimed_edges_exist(inds, half4pair)
+  n = length(inds)
+  for i in eachindex(inds)
+    uv = (inds[i], inds[mod1(i + 1, n)])
+    if !isnothing(get(half4pair, uv, NULLEDGE).elem)
+      return true
+    end
+  end
+  return false
+end
+
 function HalfEdgeTopology(elems::AbstractVector{<:Connectivity}; sort=true)
   assertion(all(e -> paramdim(e) == 2, elems), "invalid element for half-edge topology")
 
   # sort elements to make sure that they
   # are traversed in adjacent-first order
-  adjelems = sort ? adjsort(elems) : elems
-  eleminds = sort ? indexin(adjelems, elems) : 1:length(elems)
+  elemsort = sort ? adjsort(elems) : elems
+  eleminds = sort ? indexin(elemsort, elems) : 1:length(elems)
+  adjelems::Vector{Vector{Int}} = map(collect ∘ indices, elemsort)
 
   # start assuming that all elements are
-  # oriented consistently as CCW
-  CCW = trues(length(adjelems))
+  # oriented consistently
+  isreversed = falses(length(adjelems))
 
   # initialize with first element
   half4pair = Dict{Tuple{Int,Int},HalfEdge}()
-  elem = first(adjelems)
-  inds = collect(indices(elem))
-  v = CircularVector(inds)
-  n = length(v)
-  for i in 1:n
-    half4pair[(v[i], v[i + 1])] = HalfEdge(v[i], eleminds[1])
+  inds = first(adjelems)
+  for i in eachindex(inds)
+    u = inds[i]
+    u1 = inds[mod1(i + 1, length(inds))]
+    ei = eleminds[1]
+    he = get!(() -> HalfEdge(u, ei), half4pair, (u, u1))
+    # reserve half-edge to enable recognizing orientation mismatches
+    half = get!(() -> HalfEdge(u1, nothing), half4pair, (u1, u))
+    he.half = half
+    half.half = he
   end
 
   # insert all other elements
-  for e in 2:length(adjelems)
-    elem = adjelems[e]
-    inds = collect(indices(elem))
-    v = CircularVector(inds)
-    n = length(v)
-    for i in 1:n
-      # if pair of vertices is already in the
-      # dictionary this means that the current
-      # polygon has inconsistent orientation
-      if haskey(half4pair, (v[i], v[i + 1]))
-        # delete inserted pairs so far
-        CCW[e] = false
-        for j in 1:(i - 1)
-          delete!(half4pair, (v[j], v[j + 1]))
-        end
-        break
+  remaining = collect(2:length(adjelems))
+  added = false
+  disconnected = false
+  while !isempty(remaining)
+    iter = 1
+    while iter ≤ length(remaining)
+      e = remaining[iter]
+      inds = adjelems[e]
+      n = length(inds)
+      if any_edges_exist(inds, half4pair) || disconnected
+        # at least one edge has been reserved, so we can assess the orientation w.r.t.
+        # previously added elements/edges
+        deleteat!(remaining, iter)
+        added = true
+        disconnected = false
       else
-        # insert pair in consistent orientation
-        half4pair[(v[i], v[i + 1])] = HalfEdge(v[i], eleminds[e])
+        iter += 1
+        continue
       end
-    end
 
-    if !CCW[e]
-      # reinsert pairs in CCW orientation
-      for i in 1:n
-        half4pair[(v[i + 1], v[i])] = HalfEdge(v[i + 1], eleminds[e])
+      if any_claimed_edges_exist(inds, half4pair)
+        isreversed[e] = true
       end
-    end
-  end
 
-  # add missing pointers
-  for (e, elem) in Iterators.enumerate(adjelems)
-    inds = CCW[e] ? indices(elem) : reverse(indices(elem))
-    v = CircularVector(collect(inds))
-    n = length(v)
-    for i in 1:n
-      # update pointers prev and next
-      he = half4pair[(v[i], v[i + 1])]
-      he.prev = half4pair[(v[i - 1], v[i])]
-      he.next = half4pair[(v[i + 1], v[i + 2])]
-
-      # if not a border element, update half
-      if haskey(half4pair, (v[i + 1], v[i]))
-        he.half = half4pair[(v[i + 1], v[i])]
-      else # create half-edge for border
-        be = HalfEdge(v[i + 1], nothing)
-        be.half = he
-        he.half = be
+      ei = eleminds[e]
+      if isreversed[e]
+        # insert pairs in consistent orientation
+        for i in eachindex(inds)
+          u = inds[i]
+          u1 = inds[mod1(i + 1, n)]
+          he = get!(() -> HalfEdge(u1, ei), half4pair, (u1, u))
+          if !isnothing(he.elem)
+            @assert he.elem === ei lazy"inconsistent duplicate edge $he for $(ei) and $(he.elem)"
+          else
+            he.elem = ei
+          end
+          half = get!(() -> HalfEdge(u, nothing), half4pair, (u, u1))
+          he.half = half
+          half.half = he
+        end
+      else
+        for i in eachindex(inds)
+          u = inds[i]
+          u1 = inds[mod1(i + 1, n)]
+          he = get!(() -> HalfEdge(u, ei), half4pair, (u, u1))
+          he.elem = ei # this may be a pre-existing/reserved edge with a nothing `elem` field
+          half = get!(() -> HalfEdge(u1, nothing), half4pair, (u1, u))
+          he.half = half
+          half.half = he
+        end
       end
     end
+
+    if added
+      added = false
+    elseif !isempty(remaining)
+      disconnected = true
+      added = false
+    end
   end
 
-  # save halfedges in a vector of pairs
+  # add missing pointers and save halfedges in a vector of pairs
   halves = Vector{Tuple{HalfEdge,HalfEdge}}()
   visited = Set{Tuple{Int,Int}}()
-  for ((u, v), he) in half4pair
-    uv = minmax(u, v)
-    if uv ∉ visited
-      push!(halves, (he, he.half))
-      push!(visited, uv)
+  for (e, inds) in enumerate(adjelems)
+    inds = isreversed[e] ? circshift!(reverse!(inds), 1) : inds
+    n = length(inds)
+    for i in eachindex(inds)
+      vi = inds[i]
+      vi1 = inds[mod1(i + 1, n)]
+      vi2 = inds[mod1(i + 2, n)]
+      # update pointers prev and next
+      he = half4pair[(vi, vi1)]
+      he.next = half4pair[(vi1, vi2)]
+      he.next.prev = he
+
+      uv = minmax(vi, vi1)
+      if uv ∉ visited
+        push!(halves, (he, he.half))
+        push!(visited, uv)
+      end
     end
   end
 

test/topologies.jl

@@ -372,8 +372,15 @@ end
     for e in 1:nelements(topology)
       he = half4elem(topology, e)
       inds = indices(elems[e])
-      @test he.elem == e
-      @test he.head ∈ inds
+      for _ in inds
+        @test he.elem == e
+        @test he.head ∈ inds
+        @test he.next.elem == e
+        @test he.prev.elem == e
+        @test he.next.prev == he
+        @test he.prev.next == he
+        he = he.next
+      end
     end
   end
 
@@ -483,6 +490,7 @@ end
   # correct construction from inconsistent orientation
   e = connect.([(1, 2, 3), (3, 4, 2), (4, 3, 5), (6, 3, 1)])
   t = HalfEdgeTopology(e)
+  test_halfedge(e, t)
   n = collect(elements(t))
   @test n[1] == e[1]
   @test n[2] != e[2]
@@ -492,14 +500,19 @@ end
   # more challenging case with inconsistent orientation
   e = connect.([(4, 1, 5), (2, 6, 4), (3, 5, 6), (4, 5, 6)])
   t = HalfEdgeTopology(e)
+  test_halfedge(e, t)
   n = collect(elements(t))
-  @test n == connect.([(5, 4, 1), (6, 2, 4), (6, 5, 3), (4, 5, 6)])
+  @test n == connect.([(4, 1, 5), (4, 6, 2), (6, 5, 3), (4, 5, 6)])
+
+  e = connect.([(1, 2, 3), (1, 3, 4), (2, 5, 3), (5, 4, 6), (3, 5, 4)])
+  t = HalfEdgeTopology(e)
+  test_halfedge(e, t)
 
   # indexable api
   g = GridTopology(10, 10)
   t = convert(HalfEdgeTopology, g)
-  @test t[begin] == connect((13, 12, 1, 2), Quadrangle)
-  @test t[end] == connect((110, 121, 120, 109), Quadrangle)
+  @test t[begin] == g[begin]
+  @test t[end] == connect((120, 109, 110, 121), Quadrangle)
   @test t[10] == connect((22, 21, 10, 11), Quadrangle)
   @test length(t) == 100
   @test eltype(t) == Connectivity{Quadrangle,4}

test/toporelations.jl

@@ -445,30 +445,30 @@ end
   elems = connect.([(1, 2, 3), (4, 3, 2)])
   t = HalfEdgeTopology(elems)
   ∂ = Boundary{2,0}(t)
-  @test ∂(1) == (2, 3, 1)
+  @test ∂(1) == (1, 2, 3)
   @test ∂(2) == (3, 2, 4)
   ∂ = Boundary{2,1}(t)
-  @test ∂(1) == (1, 3, 2)
-  @test ∂(2) == (1, 4, 5)
+  @test ∂(1) == (1, 2, 3)
+  @test ∂(2) == (2, 5, 4)
   ∂ = Boundary{1,0}(t)
-  @test ∂(1) == (3, 2)
-  @test ∂(2) == (1, 2)
+  @test ∂(1) == (1, 2)
+  @test ∂(2) == (2, 3)
   @test ∂(3) == (3, 1)
-  @test ∂(4) == (2, 4)
-  @test ∂(5) == (4, 3)
+  @test ∂(4) == (4, 3)
+  @test ∂(5) == (2, 4)
   𝒞 = Coboundary{0,1}(t)
-  @test 𝒞(1) == (2, 3)
-  @test 𝒞(2) == (4, 1, 2)
-  @test 𝒞(3) == (3, 1, 5)
-  @test 𝒞(4) == (5, 4)
+  @test 𝒞(1) == (1, 3)
+  @test 𝒞(2) == (5, 2, 1)
+  @test 𝒞(3) == (3, 2, 4)
+  @test 𝒞(4) == (4, 5)
   𝒞 = Coboundary{0,2}(t)
   @test 𝒞(1) == (1,)
   @test 𝒞(2) == (2, 1)
   @test 𝒞(3) == (1, 2)
   @test 𝒞(4) == (2,)
   𝒞 = Coboundary{1,2}(t)
-  @test 𝒞(1) == (2, 1)
-  @test 𝒞(2) == (1,)
+  @test 𝒞(1) == (1,)
+  @test 𝒞(2) == (1, 2)
   @test 𝒞(3) == (1,)
   @test 𝒞(4) == (2,)
   @test 𝒞(5) == (2,)
@@ -484,30 +484,30 @@ end
   ∂ = Boundary{2,0}(t)
   @test ∂(1) == (1, 2, 6, 5)
   @test ∂(2) == (6, 2, 4)
-  @test ∂(3) == (6, 4, 3, 5)
-  @test ∂(4) == (3, 1, 5)
+  @test ∂(3) == (5, 6, 4, 3)
+  @test ∂(4) == (1, 5, 3)
   ∂ = Boundary{2,1}(t)
-  @test ∂(1) == (1, 3, 5, 6)
-  @test ∂(2) == (3, 9, 4)
-  @test ∂(3) == (4, 7, 8, 5)
-  @test ∂(4) == (2, 6, 8)
+  @test ∂(1) == (1, 2, 3, 4)
+  @test ∂(2) == (2, 7, 8)
+  @test ∂(3) == (3, 8, 9, 5)
+  @test ∂(4) == (4, 5, 6)
   ∂ = Boundary{1,0}(t)
   @test ∂(1) == (1, 2)
-  @test ∂(2) == (3, 1)
-  @test ∂(3) == (6, 2)
-  @test ∂(4) == (4, 6)
-  @test ∂(5) == (5, 6)
-  @test ∂(6) == (1, 5)
-  @test ∂(7) == (4, 3)
-  @test ∂(8) == (3, 5)
-  @test ∂(9) == (2, 4)
+  @test ∂(2) == (2, 6)
+  @test ∂(3) == (6, 5)
+  @test ∂(4) == (5, 1)
+  @test ∂(5) == (5, 3)
+  @test ∂(6) == (3, 1)
+  @test ∂(7) == (2, 4)
+  @test ∂(8) == (4, 6)
+  @test ∂(9) == (4, 3)
   𝒞 = Coboundary{0,1}(t)
-  @test 𝒞(1) == (1, 6, 2)
-  @test 𝒞(2) == (9, 3, 1)
-  @test 𝒞(3) == (2, 8, 7)
-  @test 𝒞(4) == (7, 4, 9)
-  @test 𝒞(5) == (5, 8, 6)
-  @test 𝒞(6) == (3, 4, 5)
+  @test 𝒞(1) == (1, 4, 6)
+  @test 𝒞(2) == (7, 2, 1)
+  @test 𝒞(3) == (6, 5, 9)
+  @test 𝒞(4) == (9, 8, 7)
+  @test 𝒞(5) == (3, 5, 4)
+  @test 𝒞(6) == (2, 8, 3)
   𝒞 = Coboundary{0,2}(t)
   @test 𝒞(1) == (1, 4)
   @test 𝒞(2) == (2, 1)
@@ -517,14 +517,14 @@ end
   @test 𝒞(6) == (2, 3, 1)
   𝒞 = Coboundary{1,2}(t)
   @test 𝒞(1) == (1,)
-  @test 𝒞(2) == (4,)
-  @test 𝒞(3) == (2, 1)
-  @test 𝒞(4) == (2, 3)
-  @test 𝒞(5) == (3, 1)
-  @test 𝒞(6) == (4, 1)
-  @test 𝒞(7) == (3,)
-  @test 𝒞(8) == (3, 4)
-  @test 𝒞(9) == (2,)
+  @test 𝒞(2) == (1, 2)
+  @test 𝒞(3) == (1, 3)
+  @test 𝒞(4) == (1, 4)
+  @test 𝒞(5) == (4, 3)
+  @test 𝒞(6) == (4,)
+  @test 𝒞(7) == (2,)
+  @test 𝒞(8) == (2, 3)
+  @test 𝒞(9) == (3,)
   𝒜 = Adjacency{0}(t)
   @test 𝒜(1) == (2, 5, 3)
   @test 𝒜(2) == (4, 6, 1)
@@ -539,17 +539,17 @@ end
   𝒜 = Adjacency{2}(t)
   @test 𝒜(1) == (2, 3, 4)
   @test 𝒜(2) == (1, 3)
-  @test 𝒜(3) == (2, 4, 1)
+  @test 𝒜(3) == (1, 2, 4)
   @test 𝒜(4) == (1, 3)
 
   # 4 quadrangles in a grid
   elems = connect.([(1, 2, 5, 4), (2, 3, 6, 5), (4, 5, 8, 7), (5, 6, 9, 8)])
   t = HalfEdgeTopology(elems)
   𝒜 = Adjacency{2}(t)
-  @test 𝒜(1) == (3, 2)
+  @test 𝒜(1) == (2, 3)
   @test 𝒜(2) == (1, 4)
   @test 𝒜(3) == (1, 4)
-  @test 𝒜(4) == (3, 2)
+  @test 𝒜(4) == (2, 3)
 
   # invalid relations
   elems = connect.([(1, 2, 3), (4, 3, 2)])