feat(#407): native auto-rigging (Pinocchio is LGPL → native template embedding) - #754
Conversation
Pinocchio (Baran & Popović 2007) is LGPL-2.1, which conflicts with the project's statically-linked permissive-distribution stance — so, like #401 (Instant Meshes) and #402 (libigl/TetGen), this is a native from-scratch implementation of the published *algorithm* (skeleton-template embedding), zero new deps. - AutoRig (src/AutoRig.h/.cpp): Ogre-free pure-data core — built-in templates (humanoid 19-bone / biped / quadruped / generic), fitTemplate() maps a template's normalised joint graph into the mesh AABB then recentres flagged joints toward per-height-slab centroids (spine→medial line, limb roots inside the silhouette). rigEntity() builds an Ogre::Skeleton (parent-relative bone positions, setBindingPose), binds via mesh->_notifySkeleton + entity ->_initialise(true) — the _initialise is REQUIRED or the exporters (both gate on entity->hasSkeleton()) silently drop the new rig. - AutoRigController (QML singleton, mirrors SkinWeightsController) for the GUI. - CLI: `qtmesh rig <file> [--skeleton T] [--skin] [--up-axis x|y|z] -o out` (cmdRig) — import, rig, optionally chain SkinWeights::computeAndApply, export. Registered in run() dispatch + AppLaunchHandler subcommand list. - AutoRig_test.cpp: pure-data unit tests (template well-formedness, AABB containment, vertical ordering, degenerate-input robustness, string/JSON). - Sentry breadcrumb ai.assist.auto_rig. Verified end-to-end: static OBJ -> 19-bone humanoid + skin -> glTF export with 1 skin / 17 joints; FBX export carries the skeleton too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- MCP: `auto_rig` { template, skin?, up_axis?, output_path? }
(MCPServer::toolAutoRig) — rigs the selected static mesh, optional skin chain
+ optional re-export. Registered + advertised. Breadcrumb ai.assist.auto_rig.
- GUI: AutoRigDialog.qml (template + up-axis pickers, "also skin" checkbox)
driven by AutoRigController; new "Rigging" CollapsibleSection in Animation
Mode → Mode Tools, gated on AutoRigController.hasRiggableSelection (a static
mesh — already-rigged meshes show "Skinning" instead). Lazy-loaded Loader +
openAutoRigDialog(), registered in qml_resources.qrc.
- Tests: CLIPipeline_cmdrig_coverage_test.cpp (arg-validation + file-missing
branches need no GL; success path skips gracefully without Xvfb).
- CLAUDE.md: CLI examples (skin + rig), recognized-subcommand list, and a full
AutoRig architecture entry (incl. the LGPL→native rationale, the
_initialise(true) export gotcha, and documented quality limits).
App + UnitTests build clean on macOS arm64.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 16 minutes and 46 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
📝 WalkthroughWalkthroughAdds a native automatic rigging feature that fits predefined skeleton templates (humanoid, biped, quadruped, generic) onto unrigged Ogre mesh entities using AABB-based joint placement with optional vertex-centroid recentering. The pipeline is exposed via a new ChangesAuto-Rig Pipeline
macOS CI Cache Improvements
Sequence DiagramsequenceDiagram
participant User
participant AutoRigDialog
participant AutoRigController
participant AutoRig
participant OgreSkeleton
User->>AutoRigDialog: click Auto-Rig (template=humanoid, alsoSkin=true)
AutoRigDialog->>AutoRigController: autoRigSelected("humanoid", "y", true)
AutoRigController->>AutoRigController: emit busyChanged(true)
AutoRigController->>AutoRig: rigEntity(entity, opts)
AutoRig->>AutoRig: fitTemplate (AABB fit + recenter joints)
AutoRig->>OgreSkeleton: create bones, parent/child hierarchy
OgreSkeleton->>OgreSkeleton: setBindingPose()
AutoRig->>OgreSkeleton: _notifySkeleton, _initialise(true)
AutoRig-->>AutoRigController: Report{applied=true, boneCount=19}
AutoRigController->>AutoRigController: emit busyChanged(false)
AutoRigController-->>AutoRigDialog: QVariantMap result
AutoRigDialog-->>User: display success status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db6937ebe1
ℹ️ 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".
| sectionVisible: root.currentTab === root.modeToolsTab | ||
| && root.modeToolMatches(EditorModeController.AnimationMode) | ||
| && AutoRigController.hasRiggableSelection |
There was a problem hiding this comment.
Register AutoRigController before binding it in QML
This binding references AutoRigController, but the app uses the QRC/manual-registration path for PropertiesPanel singletons: the neighboring controllers such as QuadRetopoController and SkinWeightsController are explicitly registered in mainwindow.cpp, and no AutoRigController registration was added. With qt_add_qml_module disabled, QML_ELEMENT alone will not make this singleton available, so the Rigging section/dialog hits ReferenceError: AutoRigController is not defined and the GUI auto-rig entry point cannot work.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bede436 — registered AutoRigController via qmlRegisterSingletonType under the 'PropertiesPanel' URI in mainwindow.cpp (next to SkinWeights/QuadRetopo) + added its kill(). Verified at runtime: no ReferenceError when the Rigging section/dialog loads.
| const r = AutoRigController.autoRigSelected( | ||
| dialog.templates[dialog.templateIndex], | ||
| dialog.alsoSkin) |
There was a problem hiding this comment.
Pass the selected up axis to auto-rig
When a user selects x or z in the dialog's Up axis picker, runRig() still calls autoRigSelected() with only the template and skin flag. The controller constructs AutoRig::Options with the default +Y axis, so the GUI always rigs as Y-up even though the UI advertises X/Z support, which misplaces skeletons for meshes using those axes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in bede436 — autoRigSelected now takes upAxis; the dialog passes dialog.upAxes[dialog.upAxisIndex] and the controller maps x/y/z → Options::upAxis.
unit-tests-linux failed to link: AutoRig::* symbols undefined in libqtmesh_test_common.a (MCPServer::toolAutoRig and CLIPipeline::cmdRig reference them). The test target has its own TEST_SRC_FILES list separate from the app's src/CMakeLists.txt — add AutoRig.cpp + AutoRigController.cpp there, next to SkinWeights (same omission class as #738's PbrMapSynth gap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
src/AutoRigController.cpp (1)
88-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider the
selectionChangedemit at line 114.After rigging completes,
hasRiggableSelection()will returnfalsebecause the entity now has a skeleton. The manualemit selectionChanged()refreshes QML property bindings so the dialog button disables correctly.This works but couples entity-state changes to the selection-change signal. If this pattern causes confusion later, consider adding a dedicated
riggabilityChanged()signal.🤖 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/AutoRigController.cpp` around lines 88 - 114, The current implementation uses emit selectionChanged() at the end of the AutoRig::rigEntity operation to refresh QML bindings after the entity rigging state changes, but this semantically misuses the selection-change signal for a rigging-state change. Add a dedicated riggabilityChanged() signal to the class header, then replace the emit selectionChanged() call at the end of the try block with emit riggabilityChanged() to make the intent clearer and decouple entity-state changes from the selection-change signal.src/MCPServer.cpp (1)
6412-6421: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPublish enum constraints for finite string options.
templateandup_axisare documented as fixed value sets, but the schema exposes free-form strings. Addenumso MCP clients can validate calls before runtime.Proposed fix
props["template"] = QJsonObject{{"type", "string"}, + {"enum", QJsonArray{"humanoid", "biped", "quadruped", "generic"}}, {"description", "Skeleton template: 'humanoid' (19-bone, default), 'biped', " "'quadruped', or 'generic' (3-joint spine fallback)."}}; @@ props["up_axis"] = QJsonObject{{"type", "string"}, + {"enum", QJsonArray{"x", "y", "z"}}, {"description", "Mesh up axis: 'x', 'y' (default), or 'z'."}};🤖 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/MCPServer.cpp` around lines 6412 - 6421, The `template` and `up_axis` properties in the schema definitions have documented finite value sets but lack `enum` constraints, preventing MCP clients from validating input before runtime. Add an `enum` field to each QJsonObject to specify the allowed values: for the `template` property include the enum values ['humanoid', 'biped', 'quadruped', 'generic'], and for the `up_axis` property include the enum values ['x', 'y', 'z']. This allows clients to validate calls against these constraints before execution.
🤖 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 `@qml/AutoRigDialog.qml`:
- Around line 41-58: In the runRig() function, the call to
AutoRigController.autoRigSelected() is missing the upAxis parameter that the
user selected via the up axis picker. Add dialog.upAxes[dialog.upAxisIndex] as a
parameter to the autoRigSelected() method call between the template parameter
and the alsoSkin parameter to pass the user's selected up axis value to the
controller.
In `@qml/PropertiesPanel.qml`:
- Around line 4200-4205: The openAutoRigDialog() function does not handle the
case where autoRigLoader enters an error state, which leaves the loader in a
permanent dead state where active is true but item is null, preventing any
subsequent retries. Add a third condition to check if autoRigLoader is in an
error state (when active is true but item is null, indicating a failed load),
and in that case, reset active to false to allow the loader to be retried on the
next click. Reference the error-retry pattern already implemented in the
openIsometricSpritesDialog() function for consistency.
In `@src/AutoRig.cpp`:
- Around line 325-335: When entity initialization fails with an Ogre exception,
the mesh still references the skeleton even though the skeleton resource is
being removed. In the catch block that handles the Ogre::Exception, after
calling skelMgr.remove(skelName), you must also detach the skeleton from the
mesh to clear its reference. Add a call to mesh->_notifySkeleton with a
null/empty skeleton value (similar to how it was attached earlier) to reset the
mesh's skeleton reference and ensure hasSkeleton() returns false on subsequent
operations.
- Around line 221-237: The vbuf->lock() call can return nullptr if the buffer is
write-only or the lock fails, but the code does not check for this before
dereferencing base in the loop that calls baseVertexPointerToElement. Add a null
check after the lock() call for the base pointer, and if it is null, shrink the
out vector back to base0 (using out.resize(base0)) to avoid leaving
uninitialized slots, then return false to indicate the operation failed.
In `@src/AutoRigController.h`:
- Around line 37-38: The autoRigSelected method in AutoRigController.h is
missing the upAxis parameter that the AutoRigDialog.qml UI is collecting from
the user's segmented control selection. Add a const QString& upAxis parameter to
the autoRigSelected method signature in AutoRigController.h, and then update the
corresponding method call in AutoRigDialog.qml to pass
dialog.upAxes[dialog.upAxisIndex] as the third argument so the user's selected
axis choice is actually transmitted to the backend instead of being ignored.
In `@src/MCPServer.cpp`:
- Around line 1709-1721: The try/catch block surrounding AutoRig::rigEntity and
SkinWeights::computeAndApply only catches Ogre::Exception, but export operations
occur outside this block and std::runtime_error exceptions are not caught,
allowing them to bypass the MCP error response path. Either extend the catch
block to also catch std::runtime_error in addition to Ogre::Exception to match
the error handling pattern used by the runOgreOp helper, or expand the try block
to wrap the complete mutating and export section to ensure all potential errors
(including export failures and std::runtime_error) are properly caught and
converted to MCP error responses.
- Around line 1711-1716: When the SkinWeights computeAndApply() method fails in
the code block where report.applied is true and alsoSkin is requested, the
current implementation only sets report.error but does not prevent the handler
from returning success. To fix this, in addition to setting the error message
when sw.applied is false, also set report.applied to false so that the handler
properly returns an error instead of continuing with an unskinned export. This
ensures that a failed skin operation (when requested with skin=true) causes the
overall operation to fail rather than silently exporting without proper
skinning.
- Around line 1734-1735: The SentryReporter::addBreadcrumb call for the auto_rig
export operation includes the full outputPath in the message, which may contain
sensitive information such as usernames or private project directories. Modify
the message parameter passed to addBreadcrumb to redact or remove the path
details while keeping the breadcrumb itself intact for I/O operation tracking.
Either omit the path from the message entirely or replace the arg(outputPath)
with a generic indicator that does not expose the actual file system location.
- Around line 1726-1728: The code in MCPServer.cpp does not validate the type of
the `output_path` argument before processing it. When
`args["output_path"].toString()` is called on a non-string value, it silently
converts to an empty string, allowing malformed MCP calls to report success
without exporting anything. Add a type validation check for the `output_path`
argument similar to how `skin` and `template` parameters are validated, to
ensure the argument is a string before proceeding with the export logic, and
handle malformed inputs appropriately by returning an error instead of silently
succeeding.
---
Nitpick comments:
In `@src/AutoRigController.cpp`:
- Around line 88-114: The current implementation uses emit selectionChanged() at
the end of the AutoRig::rigEntity operation to refresh QML bindings after the
entity rigging state changes, but this semantically misuses the selection-change
signal for a rigging-state change. Add a dedicated riggabilityChanged() signal
to the class header, then replace the emit selectionChanged() call at the end of
the try block with emit riggabilityChanged() to make the intent clearer and
decouple entity-state changes from the selection-change signal.
In `@src/MCPServer.cpp`:
- Around line 6412-6421: The `template` and `up_axis` properties in the schema
definitions have documented finite value sets but lack `enum` constraints,
preventing MCP clients from validating input before runtime. Add an `enum` field
to each QJsonObject to specify the allowed values: for the `template` property
include the enum values ['humanoid', 'biped', 'quadruped', 'generic'], and for
the `up_axis` property include the enum values ['x', 'y', 'z']. This allows
clients to validate calls against these constraints before execution.
🪄 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: 24f2ce69-7806-4adc-9d43-fed3cb63eae8
📒 Files selected for processing (16)
CLAUDE.mdqml/AutoRigDialog.qmlqml/PropertiesPanel.qmlsrc/AppLaunchHandler.cppsrc/AutoRig.cppsrc/AutoRig.hsrc/AutoRigController.cppsrc/AutoRigController.hsrc/AutoRig_test.cppsrc/CLIPipeline.cppsrc/CLIPipeline.hsrc/CLIPipeline_cmdrig_coverage_test.cppsrc/CMakeLists.txtsrc/MCPServer.cppsrc/MCPServer.hsrc/qml_resources.qrc
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Status (automated): The CI hasn't re-run yet: GitHub Actions appears to have stalled repo-wide (no workflow run created for any branch since ~15:50 UTC, despite Actions being enabled — pushes, an empty re-trigger commit |
# Conflicts: # CLAUDE.md
* fix(gui): blank QML panels + white models in installed builds Two packaging/runtime bugs that only manifested in installed builds (Homebrew .app, .deb) — dev SDK runs masked both: 1. Blank white QML panels (Inspector / Context / Material docks). The Qt Quick *software* scene-graph backend was forced in the MainWindow ctor — AFTER QApplication, by which point Qt has already locked the default RHI. Moved QSG_RHI_BACKEND / setGraphicsApi(Software) to the top of main(), before QApplication (the only point where it takes effect). Replaced the now-dead call in mainwindow.cpp with a note. 2. White / untextured models in the macOS .app. Relative resource locations from resources.cfg were resolved against macBundlePath() (the .app bundle ROOT), but the media tree lives under Contents/MacOS/media (== applicationDirPath()). So <App>.app/media/... didn't exist, Ogre loaded no RTSS GLSL programs or textures, and every mesh rendered flat white. Resolve relative paths against applicationDirPath() first (matches Linux), with the bundle root kept as a fallback; only add a location if it exists. Verified: a macdeployqt'd bundle now loads media from Contents/MacOS/media and renders the mage.glb fully textured. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: bump version to 3.9.1 (installed-build QML + macOS white-model fix) Bugfix release: QML software-backend set before QApplication, and macOS .app resource paths resolved against applicationDirPath() instead of the bundle root. Synced README + qtmesh action ref (verify-doc-versions gate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code review (Codex + CodeRabbit) on the auto-rig PR: - CRITICAL: register AutoRigController as a QML singleton in mainwindow.cpp (PropertiesPanel URI) like the sibling controllers + add its kill(). With qt_add_qml_module disabled, QML_SINGLETON alone doesn't expose it, so the Rigging section/dialog would ReferenceError. Verified no error at runtime now. - CRITICAL: the dialog's Up-axis picker was ignored — autoRigSelected() didn't take upAxis. Added a `const QString& upAxis` param (controller maps x/y/z → Options::upAxis) and pass dialog.upAxes[dialog.upAxisIndex] from QML. - AutoRig::appendPositions: guard a null vbuf->lock() (shrink `out` back, return false) instead of dereferencing. - AutoRig::rigEntity: on _initialise failure, detach the half-built skeleton (mesh->_notifySkeleton(null)) before removing it, so hasSkeleton() resets and a retry / exporter doesn't pick up a partial rig. - MCP toolAutoRig: validate output_path type; a requested skin that fails is now a hard error (no unskinned export reported as success); export wrapped in the try/catch (also catches std::exception); Sentry breadcrumb no longer logs the full output path. - PropertiesPanel openAutoRigDialog(): handle Loader.Error to allow retry. App + UnitTests build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…atch) build-macos kept failing with "No rule to make target '.../Xcode_26.5.../libz.tbd'" even after the Pin-Xcode step: the producer (build-n-cache-ogre-macos) resolved "newest" to Xcode 26.3 on its runner image and cached OGRE with 26.3's absolute libz.tbd path, while the consumer (build-macos) resolved 26.5 and linked against the missing path. "newest" (sort -V | tail -1) is NOT deterministic across the per-job runner images. Fold the resolved Xcode app name into XCODE_TAG (exported by the Pin step) and append it to all macOS assimp/ogre cache keys + restore-keys. A consumer on a different Xcode now cache-misses and rebuilds OGRE/Assimp against its own SDK instead of linking a stale path. (Belongs on master too — same deploy.yml.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
build-macos (incl. the 3.9.1 release deploy) failed:
No rule to make target '.../Xcode_26.5/.../libz.tbd', needed by QtMeshEditor
Diagnosis: the Pin-Xcode step reliably selects Xcode 26.3 on ALL macOS jobs
(verified across runs), so compilation is consistent — but the restored OGRE
cache was built earlier under Xcode 26.5 and its CMake export hardcodes 26.5's
libz.tbd path. Restoring that into a 26.3 build breaks the link.
Fix: bump MACOS_CACHE_VERSION xcode26b → xcode263 so OGRE/Assimp are rebuilt
under the currently-pinned Xcode (26.3) and the stale 26.5 cache is discarded.
Also reverted the earlier XCODE_TAG-in-cache-key experiment: build-macos only
RESTORES the OGRE cache (no rebuild step), so a per-job Xcode-keyed miss would
leave it with no OGRE at all ("Could not find OGRE"). With Xcode pinned
consistently, a plain version bump is the correct, sufficient fix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The real cause of the build-macos libz.tbd failure: the producer (build-n-cache-ogre-macos) and consumer (build-macos) run on DIFFERENT runner images whose "newest Xcode" differs — producer resolved Xcode 26.5 and cached OGRE with 26.5's absolute libz.tbd path baked into its CMake export; consumer resolved 26.3 and linked against the missing 26.5 path. Just pinning "newest" or bumping the cache version doesn't help because the two images disagree. Fix (self-healing): - Fold the resolved Xcode app name into XCODE_TAG and append it to all macOS assimp/ogre cache keys + restore-keys, so a job only restores a cache built under its OWN Xcode. - Give build-macos (consumer) the same "check out + build OGRE on cache miss" steps the producer has. When the consumer's Xcode differs from the producer's (cache miss), it rebuilds OGRE under its own SDK instead of failing on a stale libz.tbd path. This makes the macOS build robust regardless of which Xcode each runner image ships. (Bigger than the earlier one-line bump, but that couldn't fix a cross-image Xcode disagreement.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Previous commit Xcode-keyed BOTH the assimp and ogre macOS caches. That broke build-macos on a runner whose Xcode differed from the producer's: assimp cache-missed (no assimp-build-on-miss exists) so find_package(assimp) failed with "Could not find a package configuration file provided by assimp". Assimp is a plain static lib that doesn't bake absolute SDK paths, so one assimp cache is valid across Xcode versions — revert XCODE_TAG on the 3 assimp keys, keeping it ONLY on the 2 ogre keys (ogre's CMake export DOES bake an absolute libz.tbd path, which is why ogre needs per-Xcode keying + the consumer's rebuild-on-miss). The shared assimp cache is then always present for the ogre rebuild to link against. Verified on the failing run: Qt + OGRE now resolve and link (no libz.tbd error); this removes the remaining assimp-not-found failure. 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 @.github/workflows/deploy.yml:
- Around line 1806-1812: In the "Check out ogre repo (cache miss)" step, replace
the unpinned `actions/checkout@master` reference with a pinned commit SHA (e.g.,
`actions/checkout@<full-commit-sha>`), and add `persist-credentials: false` to
the `with` block to prevent unnecessary token exposure. This applies the same
security practices to the OGRE cache-miss checkout that should be used across
all checkout actions in the workflow.
🪄 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: d3c65e2e-ed51-47fd-91f1-97b380bfe674
📒 Files selected for processing (11)
.github/workflows/deploy.ymlCLAUDE.mdqml/AutoRigDialog.qmlqml/PropertiesPanel.qmlsrc/AutoRig.cppsrc/AutoRigController.cppsrc/AutoRigController.hsrc/CLIPipeline.cppsrc/MCPServer.cppsrc/MCPServer.hsrc/mainwindow.cpp
✅ Files skipped from review due to trivial changes (2)
- src/mainwindow.cpp
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (6)
- src/MCPServer.h
- qml/PropertiesPanel.qml
- qml/AutoRigDialog.qml
- src/AutoRig.cpp
- src/MCPServer.cpp
- src/CLIPipeline.cpp
… first) This test failed intermittently on CI (Xvfb) with: Value of: window->m_modeBarShell->isHidden() Actual: true Expected: false The fixture constructs MainWindow but never show()s it. QToolBar::isHidden() reflects effective visibility, which is only realized once the parent window is mapped — so under Xvfb the shell reports hidden and the assertion is racy. It hit BOTH this branch and the unrelated CI-only PR #756 (which has no source changes), confirming it's a pre-existing flake, not a regression. Fix: show() the window and processEvents() before the visibility assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
build-macos still failed with the Xcode_26.5 libz.tbd path even after pinning DEVELOPER_DIR=Xcode_26.3 and rebuilding OGRE: CMake's find_package(ZLIB) resolved to the SDK that `xcrun` defaults to (26.5 on these images) rather than the xcode-select'd one, so the OGRE SDK's CMake export baked a 26.5 libz.tbd path that the cache then carried forward. Fix: export SDKROOT (from `xcrun --sdk macosx --show-sdk-path` under the pinned Xcode) in the Pin step, so clang AND CMake resolve system libs under the SAME pinned SDK on every macOS job. Bump MACOS_CACHE_VERSION → sdkpin1 to discard the OGRE caches that still carry the 26.5 path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oy (#756) * ci: bust stale macOS OGRE cache (xcode263) — unblock the macOS deploy The 3.9.1 release deploy failed on build-macos: No rule to make target '.../Xcode_26.5/.../libz.tbd', needed by QtMeshEditor The Pin-Xcode step selects Xcode 26.3 consistently on all macOS jobs, but the OGRE cache under key 'xcode26b' was built earlier under Xcode 26.5 and its CMake export hardcodes 26.5's libz.tbd path. Restoring it into a 26.3 build breaks the link. Bump MACOS_CACHE_VERSION xcode26b → xcode263 so OGRE/Assimp rebuild under the pinned 26.3 and the stale cache is discarded. (Windows + Linux .deb artifacts already published for 3.9.1; this lets the macOS artifact + Homebrew cask update complete on a deploy re-run.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: self-heal macOS OGRE cache across differing per-job Xcode images The real cause of the build-macos libz.tbd failure: the producer (build-n-cache-ogre-macos) and consumer (build-macos) run on DIFFERENT runner images whose "newest Xcode" differs — producer resolved Xcode 26.5 and cached OGRE with 26.5's absolute libz.tbd path baked into its CMake export; consumer resolved 26.3 and linked against the missing 26.5 path. Just pinning "newest" or bumping the cache version doesn't help because the two images disagree. Fix (self-healing): - Fold the resolved Xcode app name into XCODE_TAG and append it to all macOS assimp/ogre cache keys + restore-keys, so a job only restores a cache built under its OWN Xcode. - Give build-macos (consumer) the same "check out + build OGRE on cache miss" steps the producer has. When the consumer's Xcode differs from the producer's (cache miss), it rebuilds OGRE under its own SDK instead of failing on a stale libz.tbd path. This makes the macOS build robust regardless of which Xcode each runner image ships. (Bigger than the earlier one-line bump, but that couldn't fix a cross-image Xcode disagreement.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: keep assimp macOS cache Xcode-agnostic (only ogre is Xcode-keyed) Previous commit Xcode-keyed BOTH the assimp and ogre macOS caches. That broke build-macos on a runner whose Xcode differed from the producer's: assimp cache-missed (no assimp-build-on-miss exists) so find_package(assimp) failed with "Could not find a package configuration file provided by assimp". Assimp is a plain static lib that doesn't bake absolute SDK paths, so one assimp cache is valid across Xcode versions — revert XCODE_TAG on the 3 assimp keys, keeping it ONLY on the 2 ogre keys (ogre's CMake export DOES bake an absolute libz.tbd path, which is why ogre needs per-Xcode keying + the consumer's rebuild-on-miss). The shared assimp cache is then always present for the ogre rebuild to link against. Verified on the failing run: Qt + OGRE now resolve and link (no libz.tbd error); this removes the remaining assimp-not-found failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: fix flaky MainWindowTest.ModeBarLoadsAndModeChange (show window first) This test failed intermittently on CI (Xvfb) with: Value of: window->m_modeBarShell->isHidden() Actual: true Expected: false The fixture constructs MainWindow but never show()s it. QToolBar::isHidden() reflects effective visibility, which is only realized once the parent window is mapped — so under Xvfb the shell reports hidden and the assertion is racy. It hit BOTH this branch and the unrelated CI-only PR #756 (which has no source changes), confirming it's a pre-existing flake, not a regression. Fix: show() the window and processEvents() before the visibility assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: pin SDKROOT so CMake ZLIB resolves under the selected Xcode (macOS) build-macos still failed with the Xcode_26.5 libz.tbd path even after pinning DEVELOPER_DIR=Xcode_26.3 and rebuilding OGRE: CMake's find_package(ZLIB) resolved to the SDK that `xcrun` defaults to (26.5 on these images) rather than the xcode-select'd one, so the OGRE SDK's CMake export baked a 26.5 libz.tbd path that the cache then carried forward. Fix: export SDKROOT (from `xcrun --sdk macosx --show-sdk-path` under the pinned Xcode) in the Pin step, so clang AND CMake resolve system libs under the SAME pinned SDK on every macOS job. Bump MACOS_CACHE_VERSION → sdkpin1 to discard the OGRE caches that still carry the 26.5 path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ft) (#758) Even with SDKROOT pinned, build-macos failed because the macOS jobs ran on runner images with different *newest* Xcodes: the ogre/assimp producers sometimes resolved Xcode 26.5 (newest on their image) and cached an SDK whose Codec_Assimp/ogre CMake export hardcodes 26.5's libz.tbd path, while the consumer (26.3) couldn't link it. "sort -V | tail -1" is non-deterministic across images. Pin a SPECIFIC Xcode (26.3) that's present on all current macos-latest images, falling back to newest only if absent — so producers and consumer always agree on the SDK. Bump MACOS_CACHE_VERSION → xc263pin to discard the assimp+ogre caches that still carry a 26.5 path. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#757) * fix(import): restore textured viewport and paint preview on File→Open Rebind RTSS materials for newly imported entities (same path cloud downloads already used) and load texture-paint buffers from embedded/disk sources before GPU readback, which fails for many imported FBX textures on Linux. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(import): scope texture rebind per entity source file Use each mesh's qtme.source_path binding for sidecar texture lookup so multi-file File→Open imports cannot cross-bind common texture basenames. Co-authored-by: Cursor <cursoragent@cursor.com> * chore: bump version to 3.9.2 Sync README and website pinned refs via sync-doc-versions-from-cmake.sh. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Pinocchio (Baran & Popović 2007) is LGPL-2.1, which conflicts with the project's statically-linked permissive-distribution stance — so, like #401 (Instant Meshes) and #402 (libigl/TetGen), this is a native from-scratch implementation of the published *algorithm* (skeleton-template embedding), zero new deps. - AutoRig (src/AutoRig.h/.cpp): Ogre-free pure-data core — built-in templates (humanoid 19-bone / biped / quadruped / generic), fitTemplate() maps a template's normalised joint graph into the mesh AABB then recentres flagged joints toward per-height-slab centroids (spine→medial line, limb roots inside the silhouette). rigEntity() builds an Ogre::Skeleton (parent-relative bone positions, setBindingPose), binds via mesh->_notifySkeleton + entity ->_initialise(true) — the _initialise is REQUIRED or the exporters (both gate on entity->hasSkeleton()) silently drop the new rig. - AutoRigController (QML singleton, mirrors SkinWeightsController) for the GUI. - CLI: `qtmesh rig <file> [--skeleton T] [--skin] [--up-axis x|y|z] -o out` (cmdRig) — import, rig, optionally chain SkinWeights::computeAndApply, export. Registered in run() dispatch + AppLaunchHandler subcommand list. - AutoRig_test.cpp: pure-data unit tests (template well-formedness, AABB containment, vertical ordering, degenerate-input robustness, string/JSON). - Sentry breadcrumb ai.assist.auto_rig. Verified end-to-end: static OBJ -> 19-bone humanoid + skin -> glTF export with 1 skin / 17 joints; FBX export carries the skeleton too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- MCP: `auto_rig` { template, skin?, up_axis?, output_path? }
(MCPServer::toolAutoRig) — rigs the selected static mesh, optional skin chain
+ optional re-export. Registered + advertised. Breadcrumb ai.assist.auto_rig.
- GUI: AutoRigDialog.qml (template + up-axis pickers, "also skin" checkbox)
driven by AutoRigController; new "Rigging" CollapsibleSection in Animation
Mode → Mode Tools, gated on AutoRigController.hasRiggableSelection (a static
mesh — already-rigged meshes show "Skinning" instead). Lazy-loaded Loader +
openAutoRigDialog(), registered in qml_resources.qrc.
- Tests: CLIPipeline_cmdrig_coverage_test.cpp (arg-validation + file-missing
branches need no GL; success path skips gracefully without Xvfb).
- CLAUDE.md: CLI examples (skin + rig), recognized-subcommand list, and a full
AutoRig architecture entry (incl. the LGPL→native rationale, the
_initialise(true) export gotcha, and documented quality limits).
App + UnitTests build clean on macOS arm64.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
unit-tests-linux failed to link: AutoRig::* symbols undefined in libqtmesh_test_common.a (MCPServer::toolAutoRig and CLIPipeline::cmdRig reference them). The test target has its own TEST_SRC_FILES list separate from the app's src/CMakeLists.txt — add AutoRig.cpp + AutoRigController.cpp there, next to SkinWeights (same omission class as #738's PbrMapSynth gap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code review (Codex + CodeRabbit) on the auto-rig PR: - CRITICAL: register AutoRigController as a QML singleton in mainwindow.cpp (PropertiesPanel URI) like the sibling controllers + add its kill(). With qt_add_qml_module disabled, QML_SINGLETON alone doesn't expose it, so the Rigging section/dialog would ReferenceError. Verified no error at runtime now. - CRITICAL: the dialog's Up-axis picker was ignored — autoRigSelected() didn't take upAxis. Added a `const QString& upAxis` param (controller maps x/y/z → Options::upAxis) and pass dialog.upAxes[dialog.upAxisIndex] from QML. - AutoRig::appendPositions: guard a null vbuf->lock() (shrink `out` back, return false) instead of dereferencing. - AutoRig::rigEntity: on _initialise failure, detach the half-built skeleton (mesh->_notifySkeleton(null)) before removing it, so hasSkeleton() resets and a retry / exporter doesn't pick up a partial rig. - MCP toolAutoRig: validate output_path type; a requested skin that fails is now a hard error (no unskinned export reported as success); export wrapped in the try/catch (also catches std::exception); Sentry breadcrumb no longer logs the full output path. - PropertiesPanel openAutoRigDialog(): handle Loader.Error to allow retry. App + UnitTests build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Marker-guided auto-rig refinement: the user clicks 10 humanoid markers on the mesh in the viewport (chin, L/R shoulder, L/R wrist, L/R hip, L/R knee, pelvis) and each anchors its joint while the limb/spine chains interpolate between the anchors — so the rig follows real (incl. cartoon) proportions instead of the fixed proportional template. - AutoRig::fitTemplateWithMarkers + layChain (generic anchor→tip chain): arms lay shoulder→arm→forearm→hand toward the wrist; legs lay hip socket→knee→foot; spine distributes Spine/Chest/Neck evenly between the marked pelvis and chin; hips carry unmarked thigh roots, explicit hip markers override. Every marker optional (empty set ≡ fitTemplate). - AutoRigController marker session (begin/skip/undo/cancel/commit) with ray-picked PT_SPHERE overlays; clicks routed via TransformOperator before the knife/select paths. AutoRigDialog made non-modal so viewport clicks reach the scene; onClosing cancels any active session. - Undo/redo: AutoRig::unrigEntity + AutoRigCommand wrap rig (+ optional skin) in one undoable unit; both GUI paths push through UndoManager. Single Ctrl+Z reverts rig and skin together. - Skeleton section extracted in the Inspector (skeleton/weights toggles no longer gated on animations). - Tests: marker order/labels, empty≡fitTemplate, arm/leg chain layout, shoulder/hip anchoring, spine interpolation, command error/undo branches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Replace the modal AutoRigDialog with an inline Rigging section in the Inspector (riggingToolsComponent). Skeleton-type picker is a primary control; up-axis stays under "Advanced options". Smart show/hide: idle shows entry points + options, marker mode swaps to the guidance label + Skip/Undo/Cancel/Rig-from-markers controls. Section cancels any active marker session when it disappears (replaces the dialog's onClosing). - Markers only offered for the humanoid template (they're humanoid-specific). - Fix Skip: marker progress is now a CURSOR into the order list. Skip advances the cursor past a slot without storing a marker (joint keeps the template fit); the old code pushed an unset placeholder that didn't count as resolved, so the cursor stuck and Skip did nothing. Place advances + stores; Undo steps back, dropping the marker if that slot was placed. New markerPlacedCount drives "Rig from markers"; markerCount = resolved slots (placed + skipped) for the N/total readout. - Remove qml/AutoRigDialog.qml + its qrc entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rigging with skin runs Ogre's _compileBoneAssignments, which adds BLEND_INDICES/BLEND_WEIGHTS vertex elements. On undo, clearing the bone-assignment list is NOT enough — Ogre only removes those elements inside compileBoneAssignments, which it skips when the list is empty, so the declaration kept advertising blend elements while the entity had no skeleton → null SkeletonInstance deref on the next _initialise/render. AutoRig::unrigEntity now explicitly strips the blend elements (unbind the buffer + removeElement) on shared + per-submesh vertex data, leaving a genuinely static mesh. AutoRigCommand also calls AutoRigController::notifyRiggingChanged on redo/undo — on undo BEFORE detaching, so any active skeleton-debug overlay tears down while the skeleton still exists (else it dangles), and the Inspector re-evaluates the Rigging/Skeleton sections. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace per-marker patching with a resolve-then-lay model: fitTemplate gives a proportional baseline + template segment vectors, then every key joint (Head, L/R Shoulder, L/R Hand, Hips, L/R UpLeg, L/R Knee) is resolved marked → inferred-from-marked-neighbours → template, and the dependent chains (spine/arms/legs) are laid from those anchors. A partial marker set now yields an anatomically-sane skeleton instead of stranding unmarked joints at the template (no more shoulder-above-head). Inference: Hips ← up-leg midpoint + template rise; Head ← template offset above Hips; UpLeg ← mirror the other / pelvis + socket offset; Shoulder ← along the live Hips→Head line at the template height fraction + lateral offset (chin+hips imply sane shoulders), else mirror; Hand ← shoulder + template arm vector (marked shoulder + skipped wrist still lays a full arm); mirroring reflects across the auto-detected sagittal plane. Mesh-bounds clamp: inferred legs no longer punch through the model. Knee skipped → foot drops to the mesh floor (AABB mn[up]) below the up-leg, knee halfway between; marked knee → foot extrapolated but floor-clamped; knee always kept between up-leg and foot in the up axis. Empty marker set still early-returns fitTemplate unchanged. Tests added for each inference rule + the floor clamp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…i-autorig-407 # Conflicts: # .github/workflows/deploy.yml # CLAUDE.md # qml/PropertiesPanel.qml # src/AutoRig.cpp # src/AutoRig.h # src/AutoRigController.cpp # src/AutoRigController.h # src/AutoRig_test.cpp
…cache - AutoRigMarkers.OrderAndLabelsAreStable still asserted 6 markers (and order[5]==Hips) from the first marker commit; the set grew to 10 (added L/R shoulder + L/R hip). unit-tests-linux caught it. Assert 10 and use front()/back() so the count is the single source of truth. - Bump MACOS_CACHE_VERSION sdkpin1→sdkpin2: the assimp macOS cache was built under Xcode 26.5 and baked .../MacOSX26.5.sdk/.../libz.tbd into Codec_Assimp, so OGRE's cache-miss rebuild under the pinned 26.3 failed with "No rule to make target .../libz.tbd". Busting the cache rebuilds assimp under the pinned SDK. (Pre-existing CI infra issue, not from this feature.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Minor bump for the #754 native auto-rigging feature merged after 3.9.2: template-embedding skeletons, Mixamo-style marker placement with coherent inference + mesh-bounds clamping, inline Inspector UI, and undoable rig+skin. Doc pins synced via scripts/sync-doc-versions-from-cmake.sh. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>



Closes #407.
Summary
Predicts a skeleton for an unrigged mesh. The issue proposed wrapping Pinocchio (Baran & Popović, SIGGRAPH 2007), but Pinocchio's core library is LGPL-2.1-or-later (only its demo CLI is MIT). Statically vendoring LGPL imposes relink/object-file obligations that conflict with this project's statically-linked, permissively-redistributed binaries (Homebrew/Snap/WinGet/Docker) and its permissive-license stance — the same call #401 (Instant Meshes) and #402 (libigl/TetGen) made. Pinocchio's algorithm is published and unencumbered; only its code is LGPL, so this is a from-scratch native implementation with zero new dependencies.
What's new
AutoRig(src/AutoRig.h/.cpp) — Ogre-free, unit-tested core:fitTemplate(): map the template into the mesh AABB, then recentre flagged joints (spine, limb roots) toward the centroid of vertices in a thin slab at the joint's up-height → spine onto the medial line, limb roots inside the silhouette.rigEntity(): build anOgre::Skeleton(parent-relative bone positions,setBindingPose), bind viamesh->_notifySkeleton+entity->_initialise(true)— the re-initialise is required or both exporters (which gate onentity->hasSkeleton()) silently drop the rig.qtmesh rig <file> [--skeleton humanoid|biped|quadruped|generic] [--skin] [--up-axis x|y|z] -o out—--skinchainsSkinWeights::computeAndApplyfor one-click rig+skin.auto_rig{ template, skin?, up_axis?, output_path? }.AutoRigDialog.qml+AutoRigController), gated on a static-mesh selection.ai.assist.auto_rig.AutoRig_test.cpp) + CLI coverage (CLIPipeline_cmdrig_coverage_test.cpp).Acceptance criteria
Ogre::Skeleton, exported to FBX/glTF/.skeleton).ai.assist.auto_rig.Notes
App + UnitTests build clean on macOS arm64. The full rig→skin→export path is verified locally on a static mesh; the GL-dependent unit tests run on CI Linux + Xvfb. Decision rationale (LGPL → native) recorded in
CLAUDE.md.🤖 Generated with Claude Code
Update — marker-guided fit, inline UI, undo/redo (follow-up commits)
Since the initial push, the feature was substantially refined:
Mixamo-style marker placement
AutoRig::humanoidMarkerOrder(). Clicks are ray-picked (getCameraToViewportRay→ Möller-Trumbore) and shown as yellowPT_SPHEREoverlays; routing lives inTransformOperator::mousePressEvent, session state inAutoRigController(begin/skip/undo/cancel/commit, cursor-based progress).AutoRig::fitTemplateWithMarkers): rather than per-marker patching, it resolves an anchor for every key joint as marked → inferred-from-marked-neighbours → template, then lays the spine/arm/leg chains. So a partial marker set yields an anatomically-sane skeleton (no shoulder-above-head): hips ← up-leg midpoint; shoulder ← along the Hips→Head line at the template height fraction; hand ← shoulder + template arm vector; unmarked L/R limbs mirror the marked side across the auto-detected sagittal plane.mn[up]) with the knee halfway; a marked knee's foot is floor-clamped.Inline Inspector UI (no modal dialog)
riggingToolsComponent); the old modalAutoRigDialog.qmlwas removed. Skeleton-type picker is a primary control; smart show/hide swaps to the per-marker guidance + Skip/Undo/Cancel/"Rig from markers" controls while placing. The section cancels any active marker session when it disappears.Undo/redo
AutoRig::unrigEntity+AutoRigCommand(src/commands/) wrap rig (+ optional skin) in one undoable unit — a single Ctrl+Z reverts both. Undo strips theBLEND_INDICES/BLEND_WEIGHTSvertex elements the skin step added (Ogre skips that removal for an empty assignment list, which otherwise left a null-skeleton deref → crash) and drops any skeleton-debug overlay before detaching.Tests
AutoRigCommanderror/undo branches.Summary by CodeRabbit
Release Notes
New Features
Documentation
rigsubcommand and added AutoRig usage notes.Tests
rigcommand.