feat(morph): sub-slice A2 — Inspector Morph Targets subgroup#570
Conversation
Second sub-slice of #518. A1 shipped the data path (importer + MorphAnimationManager + CLI list); A2 wires the QML Inspector surface that authors will actually use. ### What ships New "Morph Targets (N)" subgroup at the bottom of the Animations section in `qml/PropertiesPanel.qml`: - Bound to `MorphAnimationManager.morphTargetsForSelection()` — refreshes via the `morphTargetsChanged` signal when selection moves. - One row per target: name (elided), 0..1 slider (step 0.01), numeric readout. Slider `onMoved` writes back through `setWeightForSelection`, which drives the underlying Ogre `AnimationState` weight (the live preview path). - "Reset all" button — walks every target and sets weight to 0. - Filter / search text field, shown when target count > 6 (characters routinely ship 50+ shapes; flat list is hopeless). - Whole subgroup is `visible: targetCount > 0` so meshes without blend shapes don't show an empty header. Authoring (add new target from edit-mode delta, rename, delete) lands in A3. Export round-trip in A4. Dope-sheet integration in A5. MCP tools in A6. ### Manual smoke - Select an entity with blend shapes (any FBX from a DCC with facial rigs). The "Morph Targets (N)" subgroup appears at the bottom of the Animations section with one slider per shape. - Drag a slider — the mesh deforms in real time (the AnimationState weight drives the VAT_POSE track A1 created). - Click "Reset all" — every slider snaps back to 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. 📝 WalkthroughWalkthroughAdds a new "Morph Targets / Blend Shapes" subsection to the Animations panel in ChangesMorph Targets / Blend Shapes UI Subsection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 4d7146ec55
ℹ️ 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".
| Connections { | ||
| target: MorphAnimationManager | ||
| function onMorphTargetsChanged() { | ||
| morphCol.targets = MorphAnimationManager.morphTargetsForSelection() | ||
| } |
There was a problem hiding this comment.
The new morph-target panel only re-fetches targets on morphTargetsChanged, but each slider value is derived from weightForSelection(modelData) and there is no handler for morphWeightChanged. As a result, when weights are changed outside direct drag interaction (for example clicking Reset all or any other code path that calls setWeightForSelection), the backend weight updates but the displayed slider/readout can stay stale until the delegates are recreated. This makes the inspector show incorrect state immediately after valid edits.
Useful? React with 👍 / 👎.
Codex P1 on PR #570: The morph slider's `value` was bound to a one-shot function call (`MorphAnimationManager.weightForSelection(modelData)`), evaluated once at delegate creation. Any weight write that doesn't move the slider directly — Reset all, MCP, future dope-sheet scrubs — would update the backend AnimationState but the visible slider + numeric readout stayed stale until the delegate was recreated. From the user's perspective, "Reset all" appeared to do nothing. Fix: track a `weightTick` counter that bumps on `morphTargetsChanged` and `morphWeightChanged`, then bind the slider's `value` to a comma-expression that reads the tick before calling the weight getter — Qt's binding machinery picks up the tick dependency and re-evaluates the function when it changes. Same pattern QML typically uses to force re-evaluation of function-bound bindings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
qml/PropertiesPanel.qml (1)
4331-4331: ⚡ Quick winConsider adding null-safety for the targets property.
While
MorphAnimationManager.morphTargetsForSelection()likely returns a valid array, adding a defensive default prevents potential crashes if it ever returns null or undefined:🛡️ Proposed fix
- property var targets: MorphAnimationManager.morphTargetsForSelection() + property var targets: MorphAnimationManager.morphTargetsForSelection() || []function onMorphTargetsChanged() { - morphCol.targets = MorphAnimationManager.morphTargetsForSelection() + morphCol.targets = MorphAnimationManager.morphTargetsForSelection() || [] morphCol.weightTick = morphCol.weightTick + 1 }Also applies to: 4345-4345
🤖 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 `@qml/PropertiesPanel.qml` at line 4331, The property declaration for targets uses MorphAnimationManager.morphTargetsForSelection() without null-safety; update the property initialization (both occurrences that reference property 'targets') to defensively default to an empty array when MorphAnimationManager.morphTargetsForSelection() returns null or undefined (e.g. use a null-coalescing or default expression around the call) so any consumers of 'targets' always get an array.
🤖 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.
Nitpick comments:
In `@qml/PropertiesPanel.qml`:
- Line 4331: The property declaration for targets uses
MorphAnimationManager.morphTargetsForSelection() without null-safety; update the
property initialization (both occurrences that reference property 'targets') to
defensively default to an empty array when
MorphAnimationManager.morphTargetsForSelection() returns null or undefined (e.g.
use a null-coalescing or default expression around the call) so any consumers of
'targets' always get an array.
CodeRabbit nit on PR #570. The manager currently always returns a QStringList, but contracts drift; the `|| []` fallback keeps the delegate Repeater binding safe if the getter ever starts returning null/undefined. Applied at both call sites (property initialiser + onMorphTargetsChanged handler). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
1 participant
Second sub-slice of #518. A1 (#569) shipped the data path — importer +
MorphAnimationManager+qtmesh morph --list. A2 wires the QML Inspector surface authors will actually use.What ships
New Morph Targets (N) subgroup at the bottom of the Animations section in `qml/PropertiesPanel.qml`:
What's NOT in this PR (deferred to A3–A6)
list_morph_targets,set_morph_weight,add_morph_target,delete_morph_target.Manual smoke
Test plan
MorphAnimationManagerstill pass (this PR doesn't change the C++).🤖 Generated with Claude Code
Summary by CodeRabbit