feat(edit): delete + dissolve (Phase 4 topology op) - #323
Merged
Conversation
HalfEdgeMesh primitives: - deleteFaces / deleteEdges / deleteVertices: retire faces (and orphan vertices for the vertex variant), then run the standard rebuild trio (rebuildEdgesAndTwins / compactBoundaryHalfEdges / buildBoundaryHalfEdges / fixVertexHalfEdges). - dissolveEdges: for each interior edge whose two adjacent triangles are both in the same submesh, retires the pair and re-triangulates the resulting quad along the *other* diagonal so the mesh stays watertight. - dissolveVertices: for each non-boundary valence>=3 vertex, retires the surrounding triangle fan and fan-triangulates the resulting n-gon. EditModeController dispatchers: - deleteSelection() / dissolveSelection() route by current edit-mode (vertex / edge / face) and push a single EditMeshTopologyCommand through the same snapshot/normals/refresh plumbing the merge ops use. UI: - New Delete/Dissolve dropdown on the topology toolbar (✕ glyph), shown only in edit mode and enabled when the matching selection is non-empty. - X deletes the current selection in edit mode, Cmd/Ctrl+X dissolves it. X outside edit mode keeps its existing "toggle World/Local space" meaning. Shortcut Reference grew an "Edit Mode" category covering Tab / 1-2-3 / M / X / Cmd+X. Tests: - 14 new HalfEdgeMesh standalone tests covering each delete/dissolve variant, boundary/low-valence guards, invalid-input filtering, and the watertight quad-diagonal swap. - ShortcutDataHasAllSixCategories renamed/updated for the new "Edit Mode" category (now seven). Refs #259 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds delete and dissolve topology operations to edit mode: five new HalfEdgeMesh methods (deleteFaces/Edges/Vertices, dissolveEdges/Vertices), EditModeController dispatch methods, UI/shortcut hooks, undo integration, and unit tests validating deletion and dissolution behaviors. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant MW as MainWindow
participant EMC as EditModeController
participant HEM as HalfEdgeMesh
participant Undo as UndoSystem
participant Ogre as OgreRenderer
User->>MW: Press X or Ctrl/Cmd+X
MW->>EMC: deleteSelection() / dissolveSelection()
EMC->>HEM: Build HE from EditableMesh
HEM-->>EMC: HE instance
EMC->>EMC: Snapshot submeshes & selection (undo)
EMC->>HEM: deleteFaces/Edges/Vertices or dissolveEdges/Vertices
HEM->>HEM: Retire elements / Re-triangulate / Rebuild
HEM-->>EMC: affected count
EMC->>EMC: Convert HE -> EditableMesh, recompute normals
EMC->>Ogre: Resize/rewrite buffers, refresh entity
Ogre-->>EMC: GPU buffers updated
EMC->>Undo: Push EditMeshTopologyCommand (label)
EMC->>EMC: Clear selection, emit signals (editSelectionChanged, meshDataChanged)
EMC-->>MW: return affected count
Ogre->>User: Updated render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: c46246b895
ℹ️ 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".
Comment on lines
+4371
to
+4374
| for (int e : edgeIndices) { | ||
| if (!processedHandles.insert(e).second) continue; | ||
| if (e < 0 || e >= static_cast<int>(m_edges.size())) continue; | ||
| if (m_edges[e].halfEdge < 0) continue; |
There was a problem hiding this comment.
When dissolving more than one selected edge in a single call, this loop keeps using the original numeric edge indices (e) even after each successful dissolve rebuilds m_edges (rebuildEdgesAndTwins/compactBoundaryHalfEdges). Those rebuilds can reorder edge slots, so later iterations can dissolve the wrong edge or skip intended ones depending on topology changes. This shows up with multi-edge selections (e.g., two disjoint interior edges) where only the first dissolve is reliable.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/PropertiesPanelController.cpp (1)
283-288: Optional: list "X — Delete" before the modified variant for consistency.The other category groups list the unmodified key first and the modifier variants after (e.g.,
Ctrl+ZUndo and thenCtrl+Shift+ZRedo are sibling File entries). HereCmd/Ctrl+X — DissolveprecedesX — Delete, which is the inverse pattern and slightly harder to scan in the help dialog. Swapping the two lines would mirror the rest of the table without changing functionality.♻️ Suggested reorder
data << entry("Edit Mode", "M", "Merge vertices at center"); + data << entry("Edit Mode", "X", "Delete selection"); `#ifdef` Q_OS_MACOS data << entry("Edit Mode", "Cmd + X", "Dissolve selection"); `#else` data << entry("Edit Mode", "Ctrl + X", "Dissolve selection"); `#endif` - data << entry("Edit Mode", "X", "Delete selection");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/PropertiesPanelController.cpp` around lines 283 - 288, Swap the order of the "Delete selection" and "Dissolve selection" entries so the unmodified key appears first like other groups: move the data << entry("Edit Mode", "X", "Delete selection"); line to precede the conditional block that emits data << entry("Edit Mode", "Cmd + X"/"Ctrl + X", "Dissolve selection"); — locate these calls in PropertiesPanelController.cpp (the two data << entry(...) lines shown) and reorder them accordingly.src/HalfEdgeMesh.cpp (1)
4285-4301: O(V·H) orphan-vertex scan can be one pass.For each touched vertex this loops every half-edge, giving O(V·H) where V can grow with the deletion size and H scales with mesh size. A single sweep building the alive-vertex set is O(H+V) and also drops the redundant
prev->vertex == vbranch — every face vertex is already the.vertexof exactly one HE in that face's loop, so checking the "to" side suffices.♻️ Single-pass alive-set
- for (int v : touchedVerts) { - // Vertex's halfEdge may now point at a retired HE; refresh first. - // facesAroundVertex returns empty either way once everything is gone, - // so checking it is enough. - bool anyAlive = false; - for (int he = 0; he < static_cast<int>(m_halfEdges.size()); ++he) { - if (m_halfEdges[he].face < 0) continue; - if (m_halfEdges[he].vertex == v) { anyAlive = true; break; } - // Also check the "from" side: prev->vertex == v - int prev = m_halfEdges[he].prev; - if (prev >= 0 && m_halfEdges[prev].vertex == v) { - anyAlive = true; - break; - } - } - if (!anyAlive) m_vertices[v].halfEdge = -1; - } + // Single O(H) pass marks every vertex still referenced by an alive face. + // Every face vertex is the `.vertex` of exactly one HE in the face's + // loop, so checking the "to" side covers both endpoints. + std::unordered_set<int> aliveVerts; + aliveVerts.reserve(m_halfEdges.size()); + for (const auto& he : m_halfEdges) { + if (he.face >= 0) aliveVerts.insert(he.vertex); + } + for (int v : touchedVerts) { + if (!aliveVerts.count(v)) m_vertices[v].halfEdge = -1; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/HalfEdgeMesh.cpp` around lines 4285 - 4301, Replace the O(V·H) nested scan in the touchedVerts loop with a single-pass alive-set: allocate a temporary alive flag array sized to m_vertices, iterate once over m_halfEdges and set alive[m_halfEdges[he].vertex] = true for every half-edge with face >= 0, then loop touchedVerts and set m_vertices[v].halfEdge = -1 only when alive[v] is false; this removes the inner prev->vertex check and uses symbols touchedVerts, m_halfEdges, and m_vertices to locate and update the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/EditModeController.cpp`:
- Around line 2730-2738: The code currently looks up selection indices
(edgeIdxs) on a separate probe mesh but then calls
mutate/applyTopologyMutationNoSurvivor on a newly built HalfEdgeMesh (hm),
causing index mismatch; fix by resolving selected vertices/edges/faces from the
same HalfEdgeMesh instance you mutate (use hm to map selEdges/selVerts/selFaces
into edge/vertex/face indices) before calling mutate or
applyTopologyMutationNoSurvivor so indices apply to hm, not the probe; update
the same pattern in dissolveSelection and the other affected blocks (the regions
around applyTopologyMutationNoSurvivor, dissolveSelection and the uses of
HalfEdgeMesh hm at lines noted) so all selection-to-index lookups occur on the
hm instance that is mutated.
- Around line 2765-2766: The breadcrumb category passed to
SentryReporter::addBreadcrumb should be "ui.action" rather than "edit_mode";
update the call to SentryReporter::addBreadcrumb in EditModeController.cpp (the
invocation that currently uses QString("%1
(count=%2)").arg(opLabel).arg(affected)) to use "ui.action" as the first
argument so user-facing edit actions (delete/dissolve) are logged under the
ui.action taxonomy while keeping the existing message construction intact.
In `@src/EditModeController.h`:
- Around line 405-417: The toolbar wrongly enables Dissolve for FaceMode while
EditMode::dissolveSelection() is a no-op; fix by adding a canDissolve(mode)
predicate and wiring it into MainWindow::refreshTopoButtons so the
Delete/Dissolve action and Ctrl/Cmd+X shortcut are disabled/hidden when
canDissolve(currentMode) is false (i.e., FaceMode), or alternatively update the
UI text by setting the Dissolve action's tooltip/shortcut text in
refreshTopoButtons to document that dissolve is not supported in FaceMode;
reference dissolveSelection(), MainWindow::refreshTopoButtons, and the new
canDissolve(mode) predicate when making the change.
In `@src/HalfEdgeMesh.cpp`:
- Around line 4371-4456: The loop uses original edgeIndices but calls
rebuildEdgesAndTwins() which reassigns m_edges, so subsequent e values no longer
refer to the intended edges; fix by snapshotting endpoint vertex pairs up front
(use edgeVertices(e) for each e in edgeIndices to build a vector of
std::pair<int,int> of (eu,ev)), then inside the loop for each saved pair
re-resolve the current edge handle via the mesh's lookup (e.g. a function that
finds an edge by its two endpoint vertices or replicate that lookup used in
cutPath) before doing bounds/halfEdge checks, and skip if the lookup fails; keep
using
rebuildEdgesAndTwins()/compactBoundaryHalfEdges()/buildBoundaryHalfEdges()/fixVertexHalfEdges()
after each successful dissolve so topology is consistent.
In `@src/HalfEdgeMesh.h`:
- Around line 562-575: The documentation for dissolveVertices claims the N-gon
is re-triangulated "via a fan from the lowest-index boundary vertex" but the
implementation fans from loop[0] (built from *remaining.begin()/the incident
face), which is not guaranteed to be the lowest-index vertex; fix by
rotating/sorting the loop vector so its first element is the minimum vertex
index before creating the fan (e.g. find auto it =
std::min_element(loop.begin(), loop.end()) and rotate loop so loop[0] == *it),
or alternatively update the docstring to describe that the fan apex is loop[0]
(the first non-v vertex in winding order) if you prefer changing documentation
instead of code; target symbols: dissolveVertices, loop, remaining.begin().
In `@src/mainwindow.cpp`:
- Around line 1245-1262: Here the edit-mode Key_X handler always calls
editCtrl->deleteSelection() and accepts the event even when there is no
selection, which silently swallows the keystroke; change the Key_X branch so you
first test whether there is a non-empty selection (use the same predicate the
toolbar uses, e.g. the selection-check used in refreshTopoButtons or an
editCtrl->hasSelection()/selectionCount() equivalent) and only call
editCtrl->deleteSelection(), SentryReporter::addBreadcrumb(...) and
event->accept()/return when that test is true; keep the Ctrl/Cmd+X dissolve path
(editCtrl->dissolveSelection()) as-is, and if no selection is present allow the
plain X to fall through (so Alt+X / outer "toggle transform space" handler can
run).
---
Nitpick comments:
In `@src/HalfEdgeMesh.cpp`:
- Around line 4285-4301: Replace the O(V·H) nested scan in the touchedVerts loop
with a single-pass alive-set: allocate a temporary alive flag array sized to
m_vertices, iterate once over m_halfEdges and set alive[m_halfEdges[he].vertex]
= true for every half-edge with face >= 0, then loop touchedVerts and set
m_vertices[v].halfEdge = -1 only when alive[v] is false; this removes the inner
prev->vertex check and uses symbols touchedVerts, m_halfEdges, and m_vertices to
locate and update the logic.
In `@src/PropertiesPanelController.cpp`:
- Around line 283-288: Swap the order of the "Delete selection" and "Dissolve
selection" entries so the unmodified key appears first like other groups: move
the data << entry("Edit Mode", "X", "Delete selection"); line to precede the
conditional block that emits data << entry("Edit Mode", "Cmd + X"/"Ctrl + X",
"Dissolve selection"); — locate these calls in PropertiesPanelController.cpp
(the two data << entry(...) lines shown) and reorder them accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c30a187-e822-4ac2-9053-49d074d13cc7
📒 Files selected for processing (8)
src/EditModeController.cppsrc/EditModeController.hsrc/HalfEdgeMesh.cppsrc/HalfEdgeMesh.hsrc/HalfEdgeMesh_test.cppsrc/PropertiesPanelController.cppsrc/PropertiesPanelController_test.cppsrc/mainwindow.cpp
Comment on lines
+2765
to
+2766
| SentryReporter::addBreadcrumb("edit_mode", | ||
| QString("%1 (count=%2)").arg(opLabel).arg(affected)); |
There was a problem hiding this comment.
Use ui.action for this breadcrumb category.
Delete/dissolve are user-facing edit actions, so logging them under edit_mode will make Sentry filtering inconsistent with the repo’s breadcrumb taxonomy.
As per coding guidelines, "Track all user-facing actions and significant operations with SentryReporter::addBreadcrumb(category, message). Use ui.action for toolbar/menu clicks, ai.tool_call for MCP tool invocations, file.import/file.export for I/O operations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/EditModeController.cpp` around lines 2765 - 2766, The breadcrumb category
passed to SentryReporter::addBreadcrumb should be "ui.action" rather than
"edit_mode"; update the call to SentryReporter::addBreadcrumb in
EditModeController.cpp (the invocation that currently uses QString("%1
(count=%2)").arg(opLabel).arg(affected)) to use "ui.action" as the first
argument so user-facing edit actions (delete/dissolve) are logged under the
ui.action taxonomy while keeping the existing message construction intact.
Comment on lines
+405
to
+417
| /** | ||
| * @brief Dissolve the current edit-mode selection. | ||
| * | ||
| * Dispatches by selection mode: | ||
| * - VertexMode → HEMesh::dissolveVertices | ||
| * - EdgeMode → HEMesh::dissolveEdges | ||
| * - FaceMode → no-op (face dissolve is not part of MVP) | ||
| * | ||
| * Pushes one undo command labeled "Dissolve <Mode>". Returns the | ||
| * number of elements actually dissolved, or 0 on no-op. | ||
| */ | ||
| Q_INVOKABLE int dissolveSelection(); | ||
| /// @} |
There was a problem hiding this comment.
Minor UX gap: dissolveSelection() is a no-op in FaceMode, but the toolbar enables it there.
The doc comment correctly notes that face dissolve is out of scope for MVP, but MainWindow::refreshTopoButtons enables the Delete/Dissolve toolbar button (and Ctrl/Cmd+X is unconditional in edit mode) whenever there's a non-empty face selection. So a user in face mode who picks "Dissolve" from the dropdown — or hits Ctrl+X — gets only a Sentry breadcrumb and no visible change.
Consider either:
- Hiding/disabling the "Dissolve" menu item in face mode (i.e., have
refreshTopoButtonsquery acanDissolve(mode)predicate), or - Documenting the no-op in the user-facing tooltip / shortcut reference so the silence doesn't look like a bug.
The header itself is fine — flagging this here because it's the source-of-truth for the operation's contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/EditModeController.h` around lines 405 - 417, The toolbar wrongly enables
Dissolve for FaceMode while EditMode::dissolveSelection() is a no-op; fix by
adding a canDissolve(mode) predicate and wiring it into
MainWindow::refreshTopoButtons so the Delete/Dissolve action and Ctrl/Cmd+X
shortcut are disabled/hidden when canDissolve(currentMode) is false (i.e.,
FaceMode), or alternatively update the UI text by setting the Dissolve action's
tooltip/shortcut text in refreshTopoButtons to document that dissolve is not
supported in FaceMode; reference dissolveSelection(),
MainWindow::refreshTopoButtons, and the new canDissolve(mode) predicate when
making the change.
Face dissolve previously returned 0 in MVP; on a pure triangle mesh it's identical to face delete (no coplanar neighbors to merge into an n-gon), so the toolbar/menu entry now stays active and predictable. dissolveSelection in FaceMode delegates to HEMesh::deleteFaces with the "Dissolve Faces" undo label. Controller-level E2E tests cover: - DeleteSelectionFaceModeRemovesTwoTriangles — buffer count drops by 2. - DissolveSelectionFaceModeMatchesDelete — face dissolve == face delete. - DeleteSelectionEmptyIsNoOp — empty selection returns 0. - DeleteSelectionPushesUndoCommand — undo restores triangle count. Tests skip locally on macOS (no Ogre plugin) but run on Linux CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Correctness: - dissolveEdges (HE): rebuildEdgesAndTwins reorders m_edges after each iteration, so subsequent indices in the input pointed at unrelated edges. Snapshot endpoint vertex pairs up front and re-resolve the edge index each iteration. Mirrors cutPath's pattern. Added a regression test (DissolveEdgesMultipleDisjointEdgesAllProcessed) on a 2x1 quad strip with two interior diagonals — both must dissolve regardless of slot reordering. - EditModeController.deleteSelection / dissolveSelection: edge indices used to be looked up against a probe HEMesh and applied to the live one built inside applyTopologyMutationNoSurvivor. Move the lookup inside the mutate lambda so both lookups share a single instance. UX: - Bare X in edit mode now falls through when nothing is selected, so the Object-mode "toggle World/Local space" still works in edit mode with no selection. Mirrors the toolbar's gating predicate. Also excludes Shift modifier from the delete path (was harmless but unintended). Docs: - dissolveVertices header doc now describes the actual fan apex (loop[0] / first non-v vertex of the lowest-indexed incident face), not "lowest-index boundary vertex". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/EditModeController_test.cpp (1)
1551-1628: Add controller-level coverage forEdgeModeandVertexMode.These new E2Es only exercise
FaceMode, so the controller-specificVertexModepath and theEdgeModepair→HE-edge translation never run here. One end-to-end case per mode would cover the dispatcher logic this PR adds, not just the underlyingHalfEdgeMeshprimitives.Based on learnings, "Applies to src/**/*_test.cpp : Add Google Test unit tests for new functionality. Test files live alongside source in src/ with _test.cpp suffix (e.g., Manager_test.cpp)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/EditModeController_test.cpp` around lines 1551 - 1628, The tests only exercise FaceMode; add equivalent E2E tests that set the controller selection mode to EditModeController::EdgeMode and EditModeController::VertexMode via EditModeController::setSelectionMode and then exercise the controller-level dispatcher by selecting an edge (e.g., EditModeController::selectEdge or the controller's edge-selection API) and a vertex (EditModeController::selectVertex), performing the same operations you already test for faces (deleteSelection, dissolveSelection where applicable) and asserting triangle/vertex/edge buffer counts and undo behavior; ensure the new tests reference EditModeController::EdgeMode and EditModeController::VertexMode so the EdgeMode pair→HE-edge translation and VertexMode paths are covered at the controller level.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/EditModeController.cpp`:
- Around line 2717-2768: applyTopologyMutationNoSurvivor fails to refresh mesh
validation state after rewriting editableMesh, leaving
degenerateTriangleCount/hasValidationWarnings stale; call the controller's
validation routine (use the provided self pointer) after
editableMesh->subMeshes() is replaced and normals/entity buffers are updated but
before clearing selections/pushing the EditMeshTopologyCommand so the mesh
validation state is current (i.e., invoke self->validateMesh() or the correct
validateMesh method on EditModeController after
rewriteEntityAfterTopologyChange(editEntity)).
- Around line 2772-2818: Before mutating topology in
EditModeController::deleteSelection (and the other method at 2828-2877), ensure
any interactive preview sessions are ended: if m_bevelSession.active is true,
call either commitBevel() or cancelBevel() (choose consistent behavior used
elsewhere) and if m_knifeSession.active is true, call commitKnife() or
cancel/reject the knife session so the live session state cannot later replay
against stale topology; perform these session commits/cancels immediately before
constructing the mutate lambda and calling applyTopologyMutationNoSurvivor so
the mutation always operates on current preview-free topology.
---
Nitpick comments:
In `@src/EditModeController_test.cpp`:
- Around line 1551-1628: The tests only exercise FaceMode; add equivalent E2E
tests that set the controller selection mode to EditModeController::EdgeMode and
EditModeController::VertexMode via EditModeController::setSelectionMode and then
exercise the controller-level dispatcher by selecting an edge (e.g.,
EditModeController::selectEdge or the controller's edge-selection API) and a
vertex (EditModeController::selectVertex), performing the same operations you
already test for faces (deleteSelection, dissolveSelection where applicable) and
asserting triangle/vertex/edge buffer counts and undo behavior; ensure the new
tests reference EditModeController::EdgeMode and EditModeController::VertexMode
so the EdgeMode pair→HE-edge translation and VertexMode paths are covered at the
controller level.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f497d11-f7ec-435c-8e18-126d8d15393e
📒 Files selected for processing (3)
src/EditModeController.cppsrc/EditModeController.hsrc/EditModeController_test.cpp
- applyTopologyMutationNoSurvivor now calls validateMesh() after the mesh rewrite so degenerateTriangleCount / hasValidationWarnings don't carry stale state past a delete or dissolve. Other topology paths already do this. (Minor) - deleteSelection / dissolveSelection now cancel any active bevel or knife session before mutating topology. Without the guard, a live bevel snapshot or knife point list would replay against the post-delete mesh on commit/cancel and either overwrite the delete result or crash on stale indices. (Major) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Delete and Dissolve operations to Edit Mode (issue #259, Phase 4 topology checklist).
The HE-side primitives reuse the same rebuild trio (`rebuildEdgesAndTwins` → `compactBoundaryHalfEdges` → `buildBoundaryHalfEdges` → `fixVertexHalfEdges`) that `mergeVertices` introduced. The controller dispatchers route by selection mode and push a single `EditMeshTopologyCommand` through the same snapshot / normals / refresh plumbing used by merge.
UI:
Test plan
Refs #259
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests