feat(#859): PartOps — segmentation preview + split into per-part submeshes (Slices A+B) - #923
Conversation
…tion preview Epic #859 "PartOps" turns AI mesh segmentation into real authoring ops. This lands the tested pure-data core and Slice A (segmentation preview). SubMeshOps (Ogre-buffer-free, unit-tested) — the shared brain for every PartOps surface, operating on the attribute-complete EditableSubMesh representation so split/join/explode/peg math is headless-testable: - groupFacesByLabel: MeshSegmenter face labels -> stable FaceGroup list. - splitByFaceGroups (#861): one submesh per accepted group; duplicates boundary vertices so parts are independent; preserves normals/uv/colour/ tangent/bone-assignments + source material; rebuilds n-gon faces; optional connected-component sub-split + per-part material naming; excluded groups dropped. - joinParts (#862): merge parts, baking each part's world transform into positions (+ rotating normals/tangents); same-material submeshes coalesce. - explodeOffsets (#862): outward per-part offsets from the assembly centroid. - estimateBoundaryPlane + buildAlignmentPegs (#863): covariance best-fit seam plane (rejects tiny/non-planar boundaries) + cylindrical male-peg / female-socket connector geometry. Slice A (#860) — EditModeController now caches the last segmentation's per-triangle labels so the preview reuses them without rerunning the model: partGroups() model for QML, selectPartGroup / setPartGroupHidden / setPartGroupExcluded / setPartGroupDisplayName, cleared on edit-mode exit and on any topology change (via meshDataChanged). Existing Select by Part is unchanged; the rig-prior / ONNX / geometric dispatch is untouched. 12 SubMeshOps unit tests (grouping, split incl. attribute/bone preservation + boundary dup + exclude + mismatch reject, join transform-bake + material separation, explode direction/magnitude, boundary-plane accept/reject, peg gen/skip) — all headless-safe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… submeshes
`qtmesh segment <mesh> --split-parts -o out.fbx` turns a fused character into
one named submesh per detected body part (head/torso/left_arm/right_arm/
left_leg/right_leg), the epic's headline authoring op.
PartOpsMesh (Ogre adapter over the tested SubMeshOps core):
- readSubMeshes: entity -> attribute-complete EditableSubMeshes via
EditableMesh::loadFromEntity (same submesh-then-local triangle order as
AutoRig::gatherGeometry, so MeshSegmenter faceLabels map 1:1).
- buildMesh: EditableSubMeshes -> fresh Ogre::Mesh (createNewMesh), and —
crucially for skinned inputs — rebinds the source SKELETON and recompiles
per-submesh bone assignments so split characters stay riggable (mirrors
resizeEntityBuffers, which createNewMesh alone skips).
- splitEntity: read -> SubMeshOps::splitByFaceGroups -> buildMesh, without
touching the live entity (CLI exports it; a future GUI path swaps it via
an undo command).
CLI (#864): segment gains --split-parts (needs -o) and --write-labels
<json> (dumps per-vertex+per-face labels, schema qtmesh-partops-labels-v1).
Both run after the existing predict, so the rig-prior / ONNX / geometric
dispatch is unchanged; --no-model keeps everything offline. Breadcrumbs
mesh.parts.split_segments / mesh.parts.segment_preview.
Verified end-to-end on Rumba Dancing.fbx (--no-model): 1 fused body -> 6 part
submeshes, 10220 tris preserved, +183 boundary-duplicated verts, skeleton
kept (70 bones). FBX preserves the 6 submeshes; glTF coalesces same-material
parts (documented).
Tests: CLIPipeline_cmdsplitparts_coverage_test.cpp — rigged round-trip
(multi-submesh + tri count + skeleton preserved), --split-parts-requires-o
usage guard, and --write-labels schema/array validation. All headless-safe.
TestHelpers gains testAssetPath() for optional repo fixtures.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThe PR adds deterministic PartOps mesh operations, preserves mesh attributes and skeleton bindings during splitting, exposes label and split outputs through CLI, GUI, and MCP paths, preserves named submeshes across import/export, and adds segmentation preview caching with regression coverage. ChangesPartOps segmentation workflow
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant User
participant PropertiesPanel
participant PartOpsController
participant SplitMeshCommand
participant MeshSegmenter
participant PartOpsMesh
participant OgreScene
User->>PropertiesPanel: Start Split into Parts
PropertiesPanel->>PartOpsController: splitSelectedIntoParts(...)
PartOpsController->>SplitMeshCommand: Push undoable command
SplitMeshCommand->>MeshSegmenter: Predict part labels
SplitMeshCommand->>PartOpsMesh: Build split mesh
PartOpsMesh->>OgreScene: Replace entity mesh
OgreScene-->>PropertiesPanel: Emit split completion status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: e62d320c81
ℹ️ 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".
| // borrow it by seeding an EditableMesh's submesh vector directly. | ||
| EditableMesh em; | ||
| em.subMeshes() = subMeshes; | ||
| Ogre::MeshPtr mesh = em.createNewMesh(baseName); |
There was a problem hiding this comment.
When --split-parts builds the output mesh, this call goes through EditableMesh::createNewMesh, which unconditionally recalculates normals before uploading the buffers. For meshes with authored/custom normals or hard edges, the split output therefore loses the normals that SubMeshOps copied from the source, changing shading even though the split is intended to preserve vertex attributes. Use a build path that uploads the copied normals directly, or add an option to skip normal recomputation for this adapter.
Useful? React with 👍 / 👎.
| opts.assignPartMaterials | ||
| ? (opts.namePrefix.isEmpty() | ||
| ? b.name.toStdString() | ||
| : (opts.namePrefix + QStringLiteral(".") + b.name).toStdString()) | ||
| : src.materialName; |
There was a problem hiding this comment.
With the default assignPartMaterials=false, a part label that covers triangles from multiple source submeshes/materials is still accumulated into a single builder, and the output material is chosen from whichever triangle contributes first. In common multi-material assets, later triangles from the same label but different source materials are exported under the wrong material, so --split-parts changes the model's appearance; the split needs to key builders by label plus source material, or otherwise emit separate material sections per part.
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: 4
🧹 Nitpick comments (1)
src/SubMeshOps.cpp (1)
350-355: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale comment:
joinPartsdoes not rebuild n-gon faces.The comment claims trivial n-gon faces are rebuilt for the editor's canonical-faces invariant, but the merged submeshes are moved as-is with
facesleft empty and sourcefacesdropped. Either drop the comment or actually rebuild faces if downstream consumers expect them non-empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/SubMeshOps.cpp` around lines 350 - 355, Remove the stale comment above the result assignment in joinParts; the function currently moves merged submeshes as-is without rebuilding n-gon faces or preserving source faces. Keep the existing result.subMeshes, result.ok, and return behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/EditModeController.cpp`:
- Around line 195-196: Invalidate in-flight segmentation results on topology
changes by incrementing a mesh/segmentation generation or cancelling the active
job in the meshDataChanged connection at src/EditModeController.cpp:195-196. In
the completion and cache-update logic around
src/EditModeController.cpp:1092-1100, capture the generation when starting the
job and cache results only if it still matches the current generation; preserve
existing cache-clearing behavior.
- Around line 1221-1258: In src/EditModeController.cpp lines 1221-1258, update
EditModeController::selectPartGroup to add a SentryReporter::addBreadcrumb with
category ui.action after a successful part-group selection, using a fixed action
description that excludes the user-provided label. In src/EditModeController.cpp
lines 1260-1284, add equivalent ui.action breadcrumbs after each successful
hidden, excluded, and display-name state change, likewise omitting the
user-provided display name.
In `@src/EditModeController.h`:
- Around line 712-715: Update the documentation for
EditModeController::selectPartGroup to state that its return value is the number
of selected polygons, preserving the existing behavior and the rest of the
selection semantics.
In `@src/SubMeshOps.cpp`:
- Around line 505-512: Replace M_PI with Ogre::Math::PI in the shown ring
construction loop and the matching peg loop below, preserving the existing angle
calculation and behavior.
---
Nitpick comments:
In `@src/SubMeshOps.cpp`:
- Around line 350-355: Remove the stale comment above the result assignment in
joinParts; the function currently moves merged submeshes as-is without
rebuilding n-gon faces or preserving source faces. Keep the existing
result.subMeshes, result.ok, and return behavior unchanged.
🪄 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: f62e6fc3-29b7-477b-8c42-94a4d5166a8f
📒 Files selected for processing (12)
CLAUDE.mdsrc/CLIPipeline.cppsrc/CLIPipeline_cmdsplitparts_coverage_test.cppsrc/CMakeLists.txtsrc/EditModeController.cppsrc/EditModeController.hsrc/PartOpsMesh.cppsrc/PartOpsMesh.hsrc/SubMeshOps.cppsrc/SubMeshOps.hsrc/SubMeshOps_test.cppsrc/TestHelpers.h
| connect(this, &EditModeController::meshDataChanged, | ||
| this, &EditModeController::clearSegmentationCache); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invalidate in-flight segmentation jobs, not only the existing cache.
A topology edit clears the cache, but an already-running worker can later repopulate it. If the edited mesh retains the same triangle count, the check at completion passes and old labels are mapped to the new triangle stream, affecting selection and downstream split/explode operations.
src/EditModeController.cpp#L195-L196: increment a mesh/segmentation generation or cancel the active segment job when topology changes.src/EditModeController.cpp#L1092-L1100: cache results only when their captured generation still matches the current mesh.
📍 Affects 1 file
src/EditModeController.cpp#L195-L196(this comment)src/EditModeController.cpp#L1092-L1100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/EditModeController.cpp` around lines 195 - 196, Invalidate in-flight
segmentation results on topology changes by incrementing a mesh/segmentation
generation or cancelling the active job in the meshDataChanged connection at
src/EditModeController.cpp:195-196. In the completion and cache-update logic
around src/EditModeController.cpp:1092-1100, capture the generation when
starting the job and cache results only if it still matches the current
generation; preserve existing cache-clearing behavior.
…ancel stale segment Codex P2 (normals): --split-parts routed through EditableMesh::createNewMesh, which unconditionally recomputes normals — flattening authored / hard-edge normals that SubMeshOps had copied from the source. createNewMesh gains a recomputeNormals flag (default true, all existing callers unchanged); PartOpsMesh::buildMesh passes false so the copied normals upload verbatim. Codex P2 (multi-material): a part label spanning triangles from >1 source material collapsed onto whichever triangle came first, repainting the model. splitByFaceGroups now keys builders by (label, source material) so each material yields its own submesh; names get a per-label suffix (torso, torso.1, …). assignPartMaterials still collapses to one generated material. Two new unit tests cover both branches. CodeRabbit Major (stale in-flight segmentation): a topology edit cleared the cache, but a still-running worker could repopulate it with labels indexed to the OLD triangle stream (the completion size-check passes when the tri count is unchanged). clearSegmentationCache now fires the existing m_segmentCancel atomic; the worker completion already no-ops on a set cancel flag, so a superseded job reports "cancelled" instead of writing stale labels. All 17 PartOps tests green (14 core + 3 CLI incl. the rigged multi-material round-trip). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed all three findings in f6385ab: @codex (P2 — normals recomputed): Fixed. @codex (P2 — labels spanning materials): Fixed. @coderabbitai (Major — stale in-flight segmentation): Fixed. All 17 PartOps tests green locally (14 core + 3 CLI). |
|
To use Codex here, create an environment for this repo. |
|
Tip For best results, initiate chat on the files or code changes.
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
CLIPipeline.cpp (compiled into qtmesh_test_common) now calls SubMeshOps and PartOpsMesh, but tests/CMakeLists.txt's explicit TEST_SRC_FILES list didn't include the two new .cpp files — only src/CMakeLists.txt (the UnitTests target) did. So the per-file test executables (MaterialEditorQML_test, …) that link the common lib failed with undefined references to groupFacesByLabel / splitEntity. Add both files to the test-common source list. Verified: libqtmesh_test_common.a now exports both symbols as defined (T). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h FBX The Scene tree showed split parts as "Submesh 0..11" because Ogre submeshes had no names. Now each part's name (head/torso/left_arm/…) is registered and survives export→reimport, so the tree shows the part instead of an index. - PartOpsMesh::buildMesh names each output submesh via Mesh::nameSubMesh from SubMeshOps' partNames (threaded through splitEntity). - FBXExporter: Geometry + Model node names now come from the registered submesh name (getSubMeshNameMap) instead of the positional "<entity>_submeshN", so the part names are embedded in the FBX. - MeshProcessor (Assimp import): captures aiMesh::mName into SubMeshData and calls Mesh::nameSubMesh, so imported FBX/glTF meshes recover named submeshes (previously only the OgreXML path did). - SceneTreeModel: labels each SubEntity with its registered submesh name when present, else the positional index (unchanged for unnamed meshes). Round-trip verified: the rigged-split coverage test now asserts the reimported mesh has a non-empty submesh-name map and every name is a known body part. Live: Rumba split → FBX embeds head/torso/left_arm/… → reload shows named submeshes. All 17 PartOps tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Assimp/MeshProcessor.cpp`:
- Around line 184-192: The submesh registration in MeshProcessor must avoid
collisions when multiple aiMesh entries share the same name. Before calling
ogreMesh->nameSubMesh() in the subMeshData name-registration block, detect
duplicates and assign deterministic unique names (or reject the duplicate
according to existing import conventions), then add a regression test covering
duplicate aiMesh::mName values and verifying distinct lookup results.
🪄 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 Plus
Run ID: b20738c7-f44b-496a-9e6b-7a1135fbd6ff
📒 Files selected for processing (7)
src/Assimp/MeshProcessor.cppsrc/Assimp/MeshProcessor.hsrc/CLIPipeline_cmdsplitparts_coverage_test.cppsrc/FBX/FBXExporter.cppsrc/PartOpsMesh.cppsrc/PartOpsMesh.hsrc/SceneTreeModel.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
- src/CLIPipeline_cmdsplitparts_coverage_test.cpp
- src/PartOpsMesh.h
- src/PartOpsMesh.cpp
…ard) Adds a normal-integrity check to the rigged split round-trip: reimport the split FBX and assert >90% of normals are unit-length with near-zero degenerate ones. Guards the recomputeNormals=false build path against regressing into black/unlit geometry, and documents that a "model is dark on import" report is an environment/RTSS-cache issue, not the split (the split's material sidecar carries identical lighting terms to a plain re-export, and reimport uses the same MaterialProcessor RTSS wiring as any FBX). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…CP tool Completes Slice B's in-editor path (the CLI shipped earlier). Object-mode Inspector "Split into Parts (AI)" section splits the selected fused mesh into named per-part submeshes, undoable via Ctrl+Z. - SplitMeshCommand (undoable): segments the entity (rig-prior/ONNX/geometric, same pipeline as the CLI) → PartOpsMesh::splitEntity → swaps the whole mesh on the scene node. A submesh-count change can't go through the in-place EditMeshTopologyCommand/resizeEntityBuffers path, so the entity is recreated; undo restores the original MeshPtr (kept resident). - Crash fix: the swap destroyed the old entity while SelectionSet (+ transform gizmos + Scene tree) still referenced it — those only auto-clean on sceneNodeDestroyed, but a mesh swap keeps the node, so the pointers dangled and the next gizmo/tree touch crashed. Now clears the entity + sub-entity selection refs BEFORE destroy, then reselects the node. - PartOpsController (QML_SINGLETON): splitSelectedIntoParts pushes the command, reports the part count via splitFinished. - QML: section uses the inspector's themed ThemedComboBox/ThemedCheckBox/ ThemedButton + PropertiesPanelController colors (was raw Quick Controls). - MCP split_mesh_by_segments: same command headless; returns created submesh count + part names (also how the crash fix was verified without the GUI). - SplitMeshCommand_test covers the no-Ogre error/undo-before-redo branches; the full swap round-trip is covered by the Ogre-gated CLI split test. Verified: load fused Rumba → split → 12 named parts, app stays alive, scene entity swaps to the split mesh; undo restores it. 21 PartOps tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Object-mode "Split into Parts" section used ThemedCheckBox + ThemedButton, but the /PropertiesPanel qresource prefix only listed ThemedComboBox (the one themed control existing panel code used). QML couldn't resolve the two missing local types, so the ENTIRE inspector panel failed to load — blank inspector. Add both .qml files to the /PropertiesPanel prefix. Also list PartOpsController.h in the moc header list for parity with the other manually-registered controllers. qmllint passes in isolation because the type resolution is a runtime resource lookup, not a parse error — the failure only shows when the panel loads. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rename AI toggle Crash: SplitMeshCommand's mesh swap freed the old entity with a targeted removeOne, but SelectionSet::getResolvedEntities() resolves through the SUB-entity list (sub->getParent()), and destroyEntity frees the sub-entities too. A GUI selection populates sub-entity refs, so any that survived the destroy made the next selection query dereference freed memory — the crash the user hit clicking Split (offline path, but not path-specific). Switch to SelectionSet::clearList() — the API documented for "items in the list have been destroyed" — which drops node/entity/sub-entity refs together without touching the objects; then reselect the node. UX: the "Offline (skip AI model)" checkbox is now "AI assisted", CHECKED by default (both paths run locally; the model is only what downloads — "offline" was misleading). noModel is the inverse of the checkbox. Verified: offline + AI split both succeed via MCP, app survives. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "AI assisted" checkbox + "Split into Parts" button used ThemedCheckBox / ThemedButton, which are the MATERIAL EDITOR's styling — visibly different from the rest of the inspector. Switch to the inspector's own InspectorCheckBox (root-level component) and inline the InspectorButton Rectangle+MouseArea idiom (that component is scoped to another section's tree, so it's replicated here). ThemedComboBox stays for the category picker — it's the one Themed* control the inspector already shares. Drops the section's dependency on ThemedCheckBox/ ThemedButton entirely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MCPServer.cpp`:
- Around line 4666-4721: Capture the selected entity name in a stable value
before calling UndoManager::getSingleton()->push(cmd) in
MCPServer::toolSplitMeshBySegments. Use that captured name for command
construction and the subsequent o["entity"] response field, avoiding any access
to entity after SplitMeshCommand::redo() may destroy it; leave entity lookup and
error handling unchanged.
In `@src/PartOpsController.cpp`:
- Around line 45-78: Add a SentryReporter::addBreadcrumb call with category
“ui.action” at the start of splitSelectedIntoParts() to record the GUI split
button action, before selection validation or command execution. Preserve the
existing mesh.parts.split_segments breadcrumb for successful split details.
🪄 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 Plus
Run ID: b48a2c2b-8bcc-4080-a352-7806323dbbfc
📒 Files selected for processing (14)
CLAUDE.mdqml/PropertiesPanel.qmlsrc/CLIPipeline_cmdsplitparts_coverage_test.cppsrc/CMakeLists.txtsrc/MCPServer.cppsrc/MCPServer.hsrc/PartOpsController.cppsrc/PartOpsController.hsrc/commands/SplitMeshCommand.cppsrc/commands/SplitMeshCommand.hsrc/commands/SplitMeshCommand_test.cppsrc/mainwindow.cppsrc/qml_resources.qrctests/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/CLIPipeline_cmdsplitparts_coverage_test.cpp
…bs, dup names) CI (unit-tests-linux build-wrapper): PartOpsController.cpp was only in src/CMakeLists.txt, so mainwindow.cpp (in the test-common lib) failed to link its qmlInstance in the per-file test executables. Add it to tests/CMakeLists.txt. (My local build masked this — tests/ links Codec_Assimp.dylib unconditionally and my SDK lacks it, so the link never reached the undefined symbol.) Code review: - CodeRabbit Critical (MCPServer split tool): use-after-free — read entity->getName() AFTER push() destroyed the entity. Capture the name before push. - CodeRabbit Major (SubMeshOps): M_PI needs _USE_MATH_DEFINES on MSVC → Ogre::Math::PI (portable, no Windows break). - CodeRabbit Major (MeshProcessor): nameSubMesh overwrites on collision, so a duplicate aiMesh::mName made two submeshes resolve to one. Disambiguate a repeated name with an "_N" suffix instead of clobbering. - CodeRabbit Major (EditModeController): add ui.action breadcrumbs for part-group select/hide/show/exclude/include/rename (part id only, never the user display name). - CodeRabbit Minor (PartOpsController): add a ui.action breadcrumb for the GUI split button. - CodeRabbit Minor (EditModeController.h): document selectPartGroup returns the polygon count, not faces. Test: the round-trip assertion now also checks every submesh name is unique and strips only NUMERIC .N/_N suffixes (part names contain '_', e.g. "right_leg"). 21 PartOps tests green. Prior-round findings (normals recompute, per-material split, in-flight segment cancel) were already fixed in earlier commits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the review + fixed the CI link failure in 984bf76: CI ( @coderabbitai Critical — use-after-free in @coderabbitai Major — @coderabbitai Major — duplicate submesh names on import: fixed. @coderabbitai Major — part-group @coderabbitai Minor — GUI split @coderabbitai Minor — Already fixed in earlier commits (stale findings): @codex P2 normals-recompute (→ 21 PartOps tests green locally. |
|
To use Codex here, create an environment for this repo. |
|
Tip For best results, initiate chat on the files or code changes.
Great to see the PartOps suite at 21 passing tests locally. |
|
1 participant
Epic #859 PartOps — turning AI mesh segmentation into real authoring operations. Delivers the tested pure-data core, segmentation preview (Slice A #860), and the split feature end-to-end (Slice B #861) across GUI + CLI + MCP.
User-facing value
Turn a single fused character mesh into named per-part submeshes —
head,torso,left_arm,right_arm,left_leg,right_leg— boundary vertices duplicated so parts are independent, all attributes preserved, and (for skinned inputs) the skeleton + bone weights kept so the split character stays riggable.qtmesh segment character.fbx --split-parts -o parts.fbx(+--write-labels labels.json,--no-model).split_mesh_by_segments.What's in this PR
SubMeshOps— Ogre-buffer-free, unit-tested core (split/join/explode/peg math onEditableSubMeshdata; foundation for slices C/D too).PartOpsMesh— Ogre adapter: reads an entity, runs the split, builds a fresh mesh, rebinds the skeleton + recompiles bone assignments, names each submesh (round-trips through FBX → reimport → Scene tree).SplitMeshCommand— undoable mesh-swap (a submesh-count change can't go through the in-place edit path; recreates the entity on the node, clears selection refs first to avoid dangling-pointer crashes, restores the original mesh on undo).PartOpsController— QML singleton driving the GUI button.EditModeController(partGroups(), select/hide/exclude/rename; cleared on edit-mode exit + topology change).Testing
SubMeshOps_test(14): grouping, split incl. attribute/bone preservation + boundary dup + per-material split + exclude + mismatch reject, join transform-bake + material separation, explode, boundary-plane accept/reject, peg gen/skip.SplitMeshCommand_test(4): ctor/accessor/error/undo-before-redo branches.CLIPipeline_cmdsplitparts_coverage_test(3): rigged FBX round-trip asserting multi-submesh + preserved triangle count + preserved skeleton + named submeshes + unit-length normals (dark-model guard).All headless-safe; verified live on Rumba Dancing.fbx (multi-material → 12 parts) and Hip Hop Dancing.fbx (single-material → 6 clean parts).
Limitations
Body-centric part labels; glTF coalesces same-material parts (FBX preserves them); for a multi-material part you get one submesh per source material (
head,head.1, …).Remaining epic slices (follow-up PRs, on the tested core)
C explode/join scene nodes · D print-peg dialog · E remaining MCP tools (explode/join/print) · F docs polish.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation