quads: n-gon-aware bevel (edges + vertices) - #339
Conversation
New `HalfEdgeMesh::bevelEdgesNgon` handles arbitrary face arity.
`applyBevelTopology` dispatches to it when the editable mesh actually
has n-gon canonical faces; triangle-only meshes still go through the
existing `bevelEdges` so we don't lose its quality features (crease
detection, segments, profile curves).
Algorithm
Per beveled edge (v1, v2) with adjacent faces f1, f2:
1. Compute four "inner" vertices, each a perpendicular-in-face
offset from one endpoint at distance w. Width is clamped per
edge to 0.4 × shortest perimeter edge (walking the actual
face perimeter; the triangle bevel guessed a "third vertex"
which is the diagonal on a quad and gave wrong clamps).
2. Replace f1's loop: substitute v1 → innerV1F1, v2 → innerV2F1.
A triangle stays a triangle, a quad stays a quad — no fan
diagonals introduced. Same for f2.
3. Emit chamfer strip:
- One central quad bridging f1's inner pair to f2's
inner pair.
- Two corner triangles, one at each endpoint, joining
the inner vertices via the original endpoint.
4. Neighbor (non-beveled) faces aren't touched — v1/v2 stay
valid for them; the corner triangle bridges the gap.
This drops the previous "build from triangle copy + restore
untouched submeshes" workaround: the n-gon path produces real
quads + chamfer faces directly, no global triangulation of the
touched submesh.
MVP scope
- Isolated bevels only — input edges sharing an endpoint are
rejected (chained bevels need ring-aware logic; matches the
triangle bevel's same restriction).
- Single-segment flat chamfer; `segments > 1` and `profilePoints`
are reserved for a future extension.
- Vertex bevel (`bevelVertices`) keeps its triangle-mode
workaround for now — will be the next follow-up.
Tests
- BevelEdgesNgonOnQuadEdgeKeepsQuads: two adjacent quads, bevel
the shared edge → 4 inner vertices, 5 faces (2 modified quads +
1 chamfer + 2 corner caps), no fan diagonals introduced.
- BevelEdgesNgonRejectsBoundaryEdge: boundary edge skipped.
- BevelEdgesNgonRejectsChainedSelection: 4-quad cross arrangement,
two interior edges sharing the center vertex; both rejected.
- 238 standalone tests pass.
Extends the n-gon bevel work to (a) the vertex-bevel API and (b)
multi-segment chamfers on the edge bevel.
bevelVerticesNgon
Single-segment flat-cap implementation. Per beveled vertex v of
valence ≥ 3:
- For each incident face f, create one inner vertex at distance
`width` from v along the direction toward f's centroid. Width
clamps to half the shortest incident edge.
- Replace each incident face's loop: substitute v → inner_f.
Original arity preserved (triangle stays triangle, quad stays
quad — only the v-corner moves inward).
- Cap the corner with a single n-gon face walking the inner
vertices in ring order.
applyBevelVertexTopology dispatches to the n-gon variant when the
editable mesh has n-gon canonical faces, falls back to the
existing triangle-only bevelVertices otherwise.
bevelEdgesNgon segments support
Per beveled edge, build a chain of N+1 vertices at each endpoint
spanning innerVF1 → innerVF2 with N-1 intermediates. Each
intermediate is a linear blend along the chord plus a profile-
controlled bulge along the "outward" axis (toward the original
endpoint, projected perpendicular to the chord). Chamfer becomes
N segment quads instead of one; corner caps become N-triangle
fans per endpoint. profilePoints / profile drive the bulge curve
exactly as for the triangle bevel — sin-envelope synthesis when
no per-point vector is supplied.
Tests
- BevelEdgesNgonSegments3ProducesRoundedChamfer: 4 inner + 4
intermediate verts, 11 active faces (2 quads + 3 chamfer
segments + 6 corner-cap triangles).
- BevelVerticesNgonOnQuadCornerKeepsQuads: 4 quads in a + cross
around a valence-4 vertex; bevel produces 4 modified quads + 1
cap quad with no fan diagonals.
- 240 standalone tests pass.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tions Per-iteration the function does retireFace + appendFace + rebuildEdgesAndTwins, which renumbers edge slots and may invalidate the captured face indices. The cached EdgeInfo's `edgeIdx`, `f1`, `f2` then no longer match the live mesh, so the second edge in a multi-edge selection used to operate on stale references and corrupt the topology. Fix: re-resolve the edge by (v1, v2) vertex pair against the live mesh at the top of each iteration. Vertex indices are append-only and stable across rebuilds. Refresh f1, f2, subMeshIndex, and the face loops from the resolved live edge before computing the bevel. Mirrors the same pattern cutPath / dissolveEdges already use for this class of staleness bug.
Higher-valence endpoints (vertex on more than two faces) left a non-
manifold gap with the previous implementation: the corner cap
triangle bridged innerVF1 → v → innerVF2, but the OTHER incident
faces (e.g. the cube's left/right faces when beveling its top edge)
still terminated at v with the original v-edges. The new chamfer's
side edges had no twin in those neighbor faces, so the result was
non-manifold around v.
Fix: for each neighbor face g of v1 / v2 (not f1 / f2), splice the
appropriate inner vertex into g's loop right next to v. Walk g's
loop; when an outgoing edge is the (v, sideNeighbor) edge that's
shared with f1 or f2, insert the corresponding inner vertex
between v and sideNeighbor. The neighbor's arity grows by one per
splice, but the manifold stays closed: each new edge has exactly
two adjacent faces.
The standalone corner-cap triangle is now emitted ONLY when v is
"isolated" (incident only to f1 and f2, no neighbor faces) — that's
the 2-face local topology the previous version handled correctly.
For higher valence the splicing replaces it.
Tests
- New BevelEdgesNgonOnQuadCubeProducesManifoldOutput: quad cube
with valence-3 endpoints. Without the splicing fix the result
was non-manifold; now it's clean.
- 241 standalone tests pass.
Document at the top of bevelEdges and bevelVertices that they are the triangle-only path, that controllers should dispatch to the n-gon variant when the editable mesh has n-gon canonical faces, and that splitting these big functions into phase-sized helpers is now lower priority since the common case (quad-imported assets) takes the simpler n-gon path. Pure docs change — no behaviour difference.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 587cc2e148
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| bool v1Isolated = true; | ||
| bool v2Isolated = true; | ||
| for (const auto& nr : neighborRebuilds) { | ||
| // The original neighbor face's loop contained either v1 | ||
| // or v2 (or both). Re-fetch from the original loops we |
There was a problem hiding this comment.
The n-gon edge bevel suppresses corner fans whenever an endpoint has any neighboring face, but the chamfer strip still creates side-chain edges that are not inserted into those neighbor loops. On interior edges of closed meshes (for example cube edges where endpoint valence is 3+), this leaves open boundary seams at the bevel endpoints because innerVF1 and innerVF2 are never connected by any non-chamfer face. The endpoint fan should still be emitted (or the full chain must be spliced into neighbors) to keep the result closed.
Useful? React with 👍 / 👎.
| const int idx = static_cast<int>(m_vertices.size()); | ||
| m_vertices.push_back(std::move(nv)); | ||
| newVertices.push_back(idx); | ||
| innerVerts.push_back(idx); | ||
| } |
There was a problem hiding this comment.
This loop appends new inner vertices to m_vertices and newVertices before all incident faces are validated, then exits early on continue if a later face is invalid or produces a zero-length direction. In that case the function can report created vertices even though no face replacement/cap was applied, so callers may treat the bevel as successful while topology is unchanged. Buffering candidate vertices until validation succeeds (or explicitly removing them on failure) avoids this inconsistent state.
Useful? React with 👍 / 👎.
P1 — n-gon edge bevel left boundary edges at endpoints with
neighbors. The previous "skip caps when v has neighbors"
optimization meant the chamfer's v-side edge (innerVF1, innerVF2)
had no twin face — neighbor-face splicing inserts those inner
verts AROUND v in the neighbor loop, but never the
innerVF1↔innerVF2 edge itself. Closed meshes (cube interior
edges, valence ≥ 3) ended up with boundary seams at the chamfer
endpoints.
Fix: emit corner caps unconditionally. The cap closes the chamfer
side-edge AND its other two edges twin against the neighbor
splice's (v, innerVF) edges. Cap winding flipped to
(v, bV1, aV1) (resp. (v, aV1, bV1)) so it walks edges in the
opposite direction from the neighbor splice — twins match,
no inverted-tri non-manifold.
P2 — n-gon vertex bevel partially mutated state on failure. The
inner-vertex append happened inline with the per-face validation
loop, so a mid-loop `continue` could leave orphan vertices in
m_vertices and report them in the return value, making callers
think the bevel succeeded.
Fix: two-phase. Validate every incident face FIRST and stash each
inner position in a temporary vector. Only if all faces validate
do we actually push the new HEVertex slots and grow newVertices.
Tests
- BevelEdgesNgonOnQuadCubeProducesManifoldOutput: previously
passed by isManifold's permissive boundary check; with the
cap-restore fix the result is now properly closed (no
boundary edges at all on a closed input).
- 241 standalone tests still pass.
|
1 participant
Summary
N-gon-aware bevel for quad-imported meshes. Replaces the previous "triangle-mode HE copy + restore untouched submeshes" workaround that triangulated the entire touched submesh.
HalfEdgeMesh::bevelEdgesNgonPer beveled edge (v1, v2) with adjacent faces f1, f2:
segments-1intermediates. Each intermediate gets a linear blend along the chord plus a profile-controlled bulge.HalfEdgeMesh::bevelVerticesNgonPer beveled vertex v of valence ≥ 3:
Dispatcher
applyBevelTopologyandapplyBevelVertexTopologypick the n-gon variant when any submesh has n-gon canonical faces. Triangle-only meshes (procedural primitives, post-edit re-entries) keep using the existing well-testedbevelEdges/bevelVerticesso we don't lose their crease detection / coplanar-sibling logic.MVP scope
Test plan
BevelEdgesNgonOnQuadEdgeKeepsQuads: 2 quads sharing an edge → 4 inner verts, 5 faces (2 modified quads + 1 chamfer + 2 corner caps), no fan diagonals.BevelEdgesNgonSegments3ProducesRoundedChamfer: segments=3 → 8 new verts, 11 active faces (2 quads + 3 chamfer segments + 6 corner caps).BevelEdgesNgonRejectsBoundaryEdge/BevelEdgesNgonRejectsChainedSelection.BevelVerticesNgonOnQuadCornerKeepsQuads: 4-quad cross around valence-4 vertex → 4 modified quads + 1 cap quad.Known follow-ups