Add half-edge surface mesh class with bidirectional fv_surface_mesh conversion by Copilot · Pull Request #49 · hdclark/Ygor

Copilot · 2026-03-31T05:20:51Z

Adds he_surface_mesh<T,I> in src/YgorMeshesHalfEdge.{h,cc} — a half-edge (DCEL) representation complementing fv_surface_mesh with O(1) adjacency queries.

Data structure

half_edge_t<I> record: vertex, face, twin, next, prev
Per-vertex outgoing half-edge, per-face boundary half-edge
Sentinel (std::numeric_limits<I>::max()) for boundary/missing references

API surface (mirrors `fv_surface_mesh` where appropriate)

Constructors, operator==/!=, swap
surface_area(), compute_vertex_normals(), pack/unpack_RGBA32_colour
MetadataKeyPresent(), GetMetadataValueAs<U>()

Half-edge specific

vertex_faces(), vertex_neighbours(), face_vertices(), face_neighbours()
is_boundary_vertex(), is_boundary_halfedge()
num_vertices(), num_faces(), num_halfedges(), num_edges()

Bidirectional conversion

fv_surface_mesh<double, uint64_t> fvsm = /* ... */;
auto he = convert_fv_to_he_surface_mesh(fvsm);       // fv → he
auto fvsm_rt = he.convert_to_fv_surface_mesh();       // he → fv

Input validation

convert_fv_to_he_surface_mesh() requires exactly 3 vertices per face (triangular meshes only), rejects invalid vertex indices, duplicate vertex indices within a face (self-loop edges), non-manifold edges (repeated directed edges), and non-manifold vertices (bow-tie / disconnected fans) — all throw std::runtime_error with descriptive messages.
Mesh size is validated to fit within the index type I before conversion proceeds, preventing index overflow.
compute_vertex_normals() validates that all faces are triangular and throws for non-triangular faces.
Internal conversion counters use size_t with bounds-checked casts to I at assignment points.
Boundary meshes are supported via boundary half-edge selection during conversion.

Tests

99886 assertions in tests2/YgorMeshesHalfEdge.cc covering round-trip fidelity, adjacency correctness, Euler invariant, half-edge structural consistency, boundary handling, and error cases (degenerate faces, non-triangular faces, duplicate vertex indices, invalid vertex indices, non-manifold edges, bow-tie vertices). Full suite (290 cases / 99886 assertions) passes.

Original prompt

Ygor contains an implementation of fv_surface_mesh in src/YgorMath.{h,cc}. It includes an involved faces index which can speed up access to adjacency search for nearby faces and vertices, but use and maintenance of the index is cumbersome.

Your task is to add a second polyhedral surface mesh class that uses the half-edge representation. Use similarly named class methods, where appropriate, as the fv_surface_mesh class to ease swapping classes for end-users. The new half-edge class should be defined in new files src/YgorMeshesHalfEdge.{h,cc}.

Be sure to add bidirectional conversion routines so users can conveniently convert between representations where one or the other might be more suited to the specific problem domain.

Ensure the code is valid by adding unit tests as needed.

Adhere to the local styles and conventions. All new C++ should strictly adhere to the C++17 standard.

… conversion and tests Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/709d4ce9-9149-4cb9-8375-2bfcb59fe693 Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/709d4ce9-9149-4cb9-8375-2bfcb59fe693 Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Copilot

Pull request overview

Adds a half-edge (DCEL) surface mesh implementation (he_surface_mesh<T,I>) to complement the existing fv_surface_mesh and provide faster adjacency queries, plus bidirectional conversion utilities and unit tests.

Changes:

Introduces he_surface_mesh<T,I> with adjacency/boundary queries, normals, surface area, and metadata helpers.
Adds convert_fv_to_he_surface_mesh() and he_surface_mesh::convert_to_fv_surface_mesh() for round-trip conversion.
Adds a dedicated doctest suite and wires it into tests2/compile_and_run.sh.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
tests2/YgorMeshesHalfEdge.cc	New doctest coverage for conversion, adjacency, invariants, and error cases.
tests2/compile_and_run.sh	Adds the new test translation unit to the explicit compile list.
src/YgorMeshesHalfEdge.h	Declares the new half-edge mesh type and conversion API.
src/YgorMeshesHalfEdge.cc	Implements the half-edge mesh operations and fv↔he conversion logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T05:51:30Z

+    I he_idx = static_cast<I>(0);
+    for(size_t f = 0; f < N_faces; ++f){
+        const auto &fv = fvsm.faces[f];
+        const auto n = fv.size();
+        if(n < 2) continue; // Degenerate face, skip.
+
+        const I first_he = he_idx;
+        out.face_halfedges[f] = first_he;


convert_fv_to_he_surface_mesh() skips faces with n < 2, but halfedges was pre-sized using the original face sizes and face_halfedges was resized for all faces. This leaves extra (default-initialized) halfedges at the end and leaves face_halfedges[f] unset for skipped faces, which can corrupt num_halfedges()/adjacency queries and can make face_vertices()/convert_to_fv_surface_mesh() index invalid data. Consider either (a) rejecting/throwing on non-triangular/degenerate faces up front, or (b) compacting the halfedge buffer (push_back) and setting face_halfedges[f]=sentinel for skipped faces, then resizing halfedges to he_idx at the end.

reject/throw on non-triangular/degenerate faces up front

Done — convert_fv_to_he_surface_mesh() now throws std::runtime_error for faces with < 3 vertices. Test added in b76d689.

Copilot · 2026-03-31T05:51:31Z

+// The mesh may represent manifold surfaces, manifold surfaces with boundary,
+// or be empty.  Non-manifold meshes cannot be faithfully represented and
+// construction from such a mesh will throw.


The header comment states that construction from “non-manifold meshes … will throw”, but convert_fv_to_he_surface_mesh() currently only detects repeated directed edges. Non-manifold vertices (e.g., a bow-tie / multiple disconnected fans around the same vertex) can still be created without triggering this check and will break vertex fan traversals. Either broaden validation to detect non-manifold vertices or narrow/clarify this statement to match the actual guarantees.

broaden validation to detect non-manifold vertices

Done — after twin linking, the converter now walks each vertex's one-ring fan and compares the count against total outgoing half-edges. A mismatch (disconnected fans / bow-tie) throws std::runtime_error. Test added in b76d689.

Copilot · 2026-03-31T05:51:32Z

+    const auto N_faces = this->face_halfedges.size();
+    for(size_t f = 0; f < N_faces; ++f){
+        const auto verts = this->face_vertices(static_cast<I>(f));
+        if(verts.size() < 3) continue;
+
+        const auto P_A = this->vertices.at( verts[0] );
+        const auto P_B = this->vertices.at( verts[1] );
+        const auto P_C = this->vertices.at( verts[2] );
+


compute_vertex_normals() implicitly assumes triangular faces (uses verts[0..2] to compute C) but then applies that normal to all vertices in verts. If a face has >3 vertices, this silently produces incorrect results (and differs from surface_area(), which explicitly throws on non-triangular faces). Consider either validating verts.size()==3 here (and throwing) or implementing a polygon-normal/triangulation strategy consistent with the rest of the API.

Validate verts.size()==3 here (and throw if not valid)

Done — compute_vertex_normals() now throws std::runtime_error if any face has verts.size() != 3. Applied in b76d689.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

hdclark · 2026-03-31T05:56:13Z

@copilot apply changes based on the comments in this thread

Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/e406e7fa-47e0-4b99-b168-5679860bd5c6 Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

hdclark · 2026-03-31T06:10:10Z

@copilot apply changes based on the comments in this thread

…y vertex_neighbours walk, add error path tests Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/33312766-f381-4a3f-bf5f-853bba54fa2c Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Copilot · 2026-03-31T06:25:22Z

@copilot apply changes based on the comments in this thread

Applied all three changes from the review in commit b76d689:

compute_vertex_normals() validation: Now throws std::runtime_error if any face has verts.size() != 3 instead of silently skipping.
Degenerate face rejection: Already applied in a prior commit — convert_fv_to_he_surface_mesh() throws for faces with < 3 vertices.
Non-manifold vertex (bow-tie) detection: Already applied in a prior commit — validates vertex fan connectivity after twin linking.

Also fixed a bug in vertex_neighbours() where the boundary reverse walk used next→twin instead of twin→next, causing incorrect neighbours for boundary vertices.

Added test cases for all three new error paths (degenerate face, invalid vertex index, bow-tie vertex). Full suite passes: 290 cases / 99884 assertions.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T13:51:08Z

+// The half-edge data structure provides efficient adjacency queries for
+// manifold (or manifold-with-boundary) triangle meshes.  Each directed edge
+// is stored as a "half-edge" that knows its origin vertex, incident face,


The file-level comment says this half-edge structure is for manifold triangle meshes, but convert_fv_to_he_surface_mesh() only rejects faces with <3 vertices and will happily build non-triangular (n>3) face loops. Either tighten the converter to require exactly 3 vertices per face (so downstream APIs like surface_area()/compute_vertex_normals() don’t fail later), or update the header documentation to explicitly allow polygonal faces and document which operations require triangulation.

tighten the converter to require exactly 3 vertices per face

Copilot · 2026-03-31T13:51:09Z

+    const auto N_verts = fvsm.vertices.size();
+    const auto N_faces = fvsm.faces.size();
+
+    out.vertex_halfedges.assign(N_verts, sentinel);
+    out.face_halfedges.resize(N_faces);
+
+    // First pass: count the total number of half-edges needed.
+    size_t total_he = 0;
+    for(const auto &fv : fvsm.faces){
+        total_he += fv.size();
+    }
+    out.halfedges.resize(total_he);


convert_fv_to_he_surface_mesh() uses size_t for N_verts/N_faces/total_he but then drives indexing with I (he_idx, first_he, he.next/prev). If the input exceeds numeric_limits::max(), he_idx/indices can wrap and lead to out-of-bounds access/undefined behavior. Consider validating that vertices/faces/total half-edges all fit in I (and throwing) and/or using size_t for internal counters while only casting to I at assignment points after bounds checks.

use size_t for internal counters while only casting to I at assignment points after bounds checks

Copilot · 2026-03-31T13:51:09Z

+            // Insert into edge map for twin lookup.
+            const I v_from = v_idx;
+            const I v_to   = fv[(j + 1) % n];
+            auto key = std::make_pair(v_from, v_to);
+
+            auto [it, inserted] = edge_to_he.emplace(key, he_idx);
+            if(!inserted){
+                throw std::runtime_error("Non-manifold edge detected: directed edge ("
+                    + std::to_string(static_cast<uint64_t>(v_from)) + " -> "
+                    + std::to_string(static_cast<uint64_t>(v_to))
+                    + ") appears in multiple faces. Cannot build half-edge mesh.");
+            }
+
+            ++he_idx;
+        }
+    }
+
+    // Third pass: link twins.
+    for(auto &[key, idx] : edge_to_he){
+        auto twin_key = std::make_pair(key.second, key.first);
+        auto it = edge_to_he.find(twin_key);
+        if(it != edge_to_he.end()){
+            out.halfedges[idx].twin       = it->second;
+            out.halfedges[it->second].twin = idx;
+        }


The converter does not reject degenerate edges like v_from==v_to or repeated vertex indices within a face. Those cases can create self-loop directed edges where twin_key==key, causing a half-edge to become its own twin (or twins within the same face), which breaks half-edge invariants and can corrupt adjacency queries. Add validation per face to reject self-loop edges / duplicate vertices (and/or guard the twin-linking pass against self-twinning).

implement this suggestion

hdclark · 2026-03-31T14:34:16Z

@copilot apply changes based on the comments in this thread

hdclark · 2026-03-31T14:36:59Z

@copilot apply changes based on the comments in this thread

hdclark · 2026-03-31T15:44:06Z

@copilot apply changes based on the comments in this thread

hdclark · 2026-04-01T02:02:15Z

@copilot apply changes based on the comments in this thread

…nds checks, reject duplicate vertices Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/8426bb94-691c-45b7-ada7-acaba582d5cf Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/8426bb94-691c-45b7-ada7-acaba582d5cf Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Copilot · 2026-04-01T02:16:08Z

@copilot apply changes based on the comments in this thread

Applied all three changes from the review in commits d4c6156 and b2b8d36:

Triangular faces only: convert_fv_to_he_surface_mesh() now rejects faces with n != 3 (was n < 3), consistent with surface_area() and compute_vertex_normals().
size_t internal counters: he_idx and first_he are now size_t, cast to I only at assignment. Added upfront validation that vertex/face/half-edge counts fit in I.
Duplicate vertex rejection: Added per-face check that rejects faces with repeated vertex indices (self-loop edges).

Tests added for non-triangular faces and duplicate vertices. Full suite passes: 290 cases / 99886 assertions.

Initial plan

e855327

Copilot AI assigned Copilot and hdclark Mar 31, 2026

Copilot started work on behalf of hdclark March 31, 2026 05:20 View session

Copilot AI and others added 2 commits March 31, 2026 05:35

Add half-edge surface mesh class (he_surface_mesh) with bidirectional…

d927a7c

… conversion and tests Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/709d4ce9-9149-4cb9-8375-2bfcb59fe693 Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Fix review comment: simplify test assertion for metadata value

bd8b96e

Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/709d4ce9-9149-4cb9-8375-2bfcb59fe693 Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Add half-edge surface mesh class implementation~~ Add half-edge surface mesh class with bidirectional fv_surface_mesh conversion Mar 31, 2026

Copilot AI requested a review from hdclark March 31, 2026 05:41

Copilot finished work on behalf of hdclark March 31, 2026 05:41

hdclark marked this pull request as ready for review March 31, 2026 05:46

Copilot AI review requested due to automatic review settings March 31, 2026 05:46

Copilot started reviewing on behalf of hdclark March 31, 2026 05:46 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

hdclark and others added 4 commits March 30, 2026 22:53

Update src/YgorMeshesHalfEdge.cc

a3974a0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/YgorMeshesHalfEdge.cc

7d95689

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update tests2/YgorMeshesHalfEdge.cc

770671f

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/YgorMeshesHalfEdge.h

b43dbe4

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of hdclark March 31, 2026 05:56 View session

Changes before error encountered

cdb883e

Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/e406e7fa-47e0-4b99-b168-5679860bd5c6 Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Copilot started work on behalf of hdclark March 31, 2026 06:10 View session

Address review: validate faces in compute_vertex_normals, fix boundar…

b76d689

…y vertex_neighbours walk, add error path tests Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/33312766-f381-4a3f-bf5f-853bba54fa2c Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Copilot finished work on behalf of hdclark March 31, 2026 06:26

hdclark requested a review from Copilot March 31, 2026 13:44

Copilot started reviewing on behalf of hdclark March 31, 2026 13:45 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

Copilot started work on behalf of hdclark April 1, 2026 02:02 View session

Copilot AI and others added 2 commits April 1, 2026 02:11

Tighten converter: require triangular faces, size_t counters with bou…

d4c6156

…nds checks, reject duplicate vertices Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/8426bb94-691c-45b7-ada7-acaba582d5cf Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Simplify duplicate vertex check for triangular faces

b2b8d36

Agent-Logs-Url: https://github.com/hdclark/Ygor/sessions/8426bb94-691c-45b7-ada7-acaba582d5cf Co-authored-by: hdclark <934858+hdclark@users.noreply.github.com>

Copilot finished work on behalf of hdclark April 1, 2026 02:17

Conversation

Copilot AI commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Data structure

API surface (mirrors fv_surface_mesh where appropriate)

Half-edge specific

Bidirectional conversion

Input validation

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

hdclark Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

hdclark Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

hdclark Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

hdclark commented Mar 31, 2026

Uh oh!

hdclark commented Mar 31, 2026

Uh oh!

Copilot AI commented Mar 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

hdclark Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

hdclark Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

hdclark Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

hdclark commented Mar 31, 2026

Uh oh!

hdclark commented Mar 31, 2026

Uh oh!

hdclark commented Mar 31, 2026

Uh oh!

Copilot AI commented Mar 31, 2026 •

edited

Loading

API surface (mirrors `fv_surface_mesh` where appropriate)