quads chunk 5a: Catmull-Clark subdivide - #333
Conversation
Adds the classic Catmull-Clark subdivision-surface operation on the
half-edge mesh. Always produces an all-quad output regardless of
input topology — a triangle becomes 3 quads, a quad becomes 4, an
N-gon becomes N. Output geometry approaches a C¹-continuous limit on
closed manifolds via the standard rule:
- Face point Fp = average of corner positions.
- Edge point Ep = (a + b + Fp1 + Fp2) / 4 on interior edges,
(a + b) / 2 on boundary or cross-submesh edges.
- Smoothed vertex V' = (F + 2R + (n-3) V) / n on interior vertices,
chord rule on boundary vertices. F = avg of adjacent face points,
R = avg of adjacent edge midpoints (NOT edge points — spec).
Cross-submesh edges are treated as boundaries so material seams stay
sharp through the subdivision. Bone weights on smoothed vertices keep
the original assignment (the smoothed position is still mostly "near
V"); other attributes (UV, normal, color, tangent) blend with the
same weights as positions.
UI:
- The Subdivide toolbar button becomes a dropdown:
1. Standard — the existing 1-to-4 triangle split on selected
faces / edges. Unchanged behaviour.
2. Catmull-Clark — whole-mesh subdivide-surface step. Selection
is cleared after the op (partial-CC with selection
preservation needs a more sophisticated boundary blend that
isn't in this MVP).
- Button is enabled whenever in edit mode; menu items self-gate.
Tests (+7 standalone):
- 1 quad → 4 sub-quads.
- 1 triangle → 3 sub-quads.
- Closed cube stays closed (12 → 36 quads, no boundary edges).
- 2x2 quad grid → 16 sub-quads.
- Planar quad face point lands at arithmetic centre.
- Empty mesh is a no-op.
- Round-trip through EditableMesh preserves the all-quad output.
204 standalone tests pass (was 197; +7).
Towards #326.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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: 7a61fcd85b
ℹ️ 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".
| for (int e = 0; e < origEdgeCount; ++e) { | ||
| if (m_edges[e].halfEdge < 0) continue; | ||
| const auto [a, b] = edgeVertices(e); | ||
| if ((a == va && b == vb) || (a == vb && b == va)) return e; |
There was a problem hiding this comment.
findEdgeIdxByVerts does a full linear scan over origEdgeCount every time a face side is processed, and it is called for every side of every face during subdivision. On typical quad meshes this turns the rebuild phase into roughly O(F*E) (effectively O(F²)), which will make whole-mesh Catmull-Clark stall or hang on larger assets. Pre-indexing original edges once (e.g., undirected (min(v0,v1), max(v0,v1)) -> edgeIdx) would keep this step linear in mesh size.
Useful? React with 👍 / 👎.
`findEdgeIdxByVerts` was a linear scan over `m_edges` (size = origEdgeCount), called for every side of every face during the rebuild step of Catmull-Clark subdivision. On typical quad meshes this turned the rebuild phase into roughly O(F·E) (effectively O(F²)) and would stall on larger assets. Fix: pre-build an undirected (min(va,vb), max(va,vb)) → edgeIdx hash inside the existing edge-walk loop, then look up by vertex pair in O(1). The hash is populated only for live edges (`halfEdge >= 0`) with valid vertex indices, so behaviour matches the previous lambda exactly. All 7 Catmull-Clark standalone tests still pass.
|
1 participant
Summary
Chunk 5a of the quad migration (#326). Adds the classic Catmull-Clark subdivision-surface op on the half-edge mesh and wires it through the toolbar as a second Subdivide menu item.
Stacked on chunk 4 (#332). When chunk 4 lands on
feat/quads, this PR's base auto-updates.What it does
HalfEdgeMesh::subdivideCatmullClark()— one full CC step on the whole mesh. Always emits quads regardless of input topology (triangle → 3 quads, quad → 4 quads, N-gon → N quads). Geometry is smoothed via the standard rule (face points / edge points / smoothed vertex positions). Cross-submesh edges are treated as boundaries so material seams stay sharp.EditModeController::subdivideCatmullClarkAll()— wraps it with the standard pre-cancel-bevel/knife + undo-command + re-pack pipeline. Selection clears after the op.Tests (+7 standalone)
204 standalone tests pass (was 197; +7).
Test plan
Deferred to follow-up chunks
🤖 Generated with Claude Code