feat(quads): loop cut on quad meshes - #341
Conversation
Loop cut walks the perpendicular ring of quads adjacent to a
selected edge, bisecting each one with a new midpoint chain. Quad-
only operation — that's the geometric definition: triangles have
no opposite-edge correspondence, so a "loop" through tri-tri pairs
is ambiguous and not implemented in this MVP. Matches Blender's
behaviour (loop cut on a triangulated mesh is also a no-op there).
HalfEdgeMesh::loopCut(startEdgeIdx)
Walk plan:
- Start: the two faces adjacent to startEdgeIdx.
- Each step: in the current quad, find the opposite edge (the
side two positions away in the loop) and record the rail
pair (entry edge + opposite). Cross opposite into the next
face.
- Stop on: closed loop (back to start edge), boundary edge
(no second face), non-quad face (no opposite-edge
correspondence).
Walk both directions from the start edge so an interior edge of
an open mesh produces cuts on both sides. visitedFaces guards
against re-entry once a closed ring closes.
Materialisation: collect rails up-front (vertex-pair-keyed since
edge indices shift across splitEdge). Per step, ensure midpoint
on each rail (shared between consecutive steps via map cache),
then splitFace bisects the now-hexagonal face along the new
diagonal. Returns the new midpoint vertex indices in walk order.
EditModeController::loopCutSelection()
Edge-mode only. Uses the FIRST selected edge as start (multi-
edge loop cuts aren't in MVP scope — each cut is independent).
Pushes one undo command labeled "Loop Cut".
UI
Toolbar button (‖ double vertical line) next to Fill, with
Ctrl+R shortcut. The Ctrl modifier disambiguates from R = Scale
mode (Unity convention). Falls through to Scale when the loop
cut returns 0 (e.g. nothing selected, non-quad neighborhood),
so Ctrl+R remains harmless outside the loop-cut context.
Tests
- LoopCutOnQuadStripCutsEachQuadOnce: 3-quad strip → 4 rail
midpoints, 6 quads after cut.
- LoopCutClosedRingOnQuadCubeReturnsToStart: quad cube → 4
rail midpoints (closed ring), 10 quads (6 + 4 cuts).
- LoopCutFailsOnNonQuadAdjacency: triangle pair → empty.
- LoopCutFailsOnInvalidEdgeIndex: -1 / out-of-range → empty.
- 244 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 |
![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: 5292e164ba
ℹ️ 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".
| SentryReporter::addBreadcrumb("ui.shortcut", "R — Scale mode"); | ||
| setTransformState(TransformOperator::TS_SCALE); |
There was a problem hiding this comment.
When Ctrl+R is pressed in edge edit mode and loopCutSelection() returns 0 (for example on triangle/non-quad adjacency), execution falls through to the default R handler and changes the tool to Scale. This makes a failed loop-cut attempt perform a different action instead of a no-op, which is surprising and can interrupt the current edit workflow; the Ctrl-modified path should be consumed once attempted.
Useful? React with 👍 / 👎.
| const auto [f1, f2] = edgeFaces(startEdgeIdx); | ||
| walkDirection(f1, startA, startB); | ||
| walkDirection(f2, startA, startB); | ||
|
|
||
| if (walk.empty()) return newVertices; |
There was a problem hiding this comment.
The function documentation says the start edge must have two quad-adjacent faces or return empty, but the implementation walks both sides independently and only aborts when walk is fully empty. That means an edge with one quad side and one non-quad side still produces a partial cut (mutating topology) instead of failing, which violates the stated contract and causes unexpected edits near mixed tri/quad regions.
Useful? React with 👍 / 👎.
|
1 participant
Summary
Implements loop cut — the original target that started the quad-migration epic. Walks the perpendicular ring of quads adjacent to a selected edge and bisects each one with a midpoint chain.
HalfEdgeMesh::loopCut(startEdgeIdx)startEdgeIdx.splitFacebisects each step's face.EditModeController::loopCutSelection()Edge-mode only; uses first selected edge. Pushes "Loop Cut" undo command.
UI
Quad-only behaviour
Loop cut is geometrically a quad operation — triangles have no opposite-edge correspondence, so there's no well-defined "loop". MVP returns empty on triangle adjacency, matching Blender's behaviour. A tri-pair fallback (treat coplanar triangle pairs as quads for the walk) could be added later if needed.
Test plan
LoopCutOnQuadStripCutsEachQuadOnce— 3-quad strip → 4 rail midpoints, 6 quads after cut, no fan diagonals.LoopCutClosedRingOnQuadCubeReturnsToStart— quad cube → 4 rail midpoints (closed ring), 10 quads (6 + 4 cuts).LoopCutFailsOnNonQuadAdjacency— triangle pair → empty.LoopCutFailsOnInvalidEdgeIndex— -1 / out-of-range → empty.Known follow-ups
tparameter to position the cut along the loop instead of always at midpoint.