feat(morph): A5b — read-only morph rows in the dope sheet#580
Conversation
Final sub-slice of #518. The C++ data API (`allMorphRows`) shipped in #576 as a Q_INVOKABLE on AnimationControlController; this PR wires it into the dope-sheet view. ## What ships - New `morphRows` property on the dope-sheet root, bound to `AnimationControlController.allMorphRows()`. Refreshed via the existing `onSelectionChanged` handler so a clip change rebuilds both bone and morph rows in lockstep. - `rowsView` (bone ListView) bottom anchor switches to `morphBand.top` when the morph band is visible. Bone-only entities → band collapses to `height=0` → bone list draws full height as before. - New `morphBand` Rectangle at the bottom: header row showing "Morph Targets (N)", then one row per pose with the target name on the left and `#88ccff` diamond markers at each keyframe time on the right. Diamonds share the bone-row timeline math (`pxPerSec`, `viewStart`) so they line up vertically. ## Why this is small and safe The original A5 (PR #576's first commit) deterministically crashed unrelated MainWindow / MCPServer visibility-toggle suites in CI. Root cause was never fully isolated but the suspicion was an `import PropertiesPanel 1.0` triggering a different QML singleton chain during MainWindow construction. This PR keeps the discipline that worked for A3b: - **No new QML imports.** `AnimationControl 1.0` was already imported. `allMorphRows()` was deliberately put on AnimationControlController (not MorphAnimationManager) for exactly this reason. - **No new `Connections` blocks.** The existing one already fires on the signals we care about. - **No MouseAreas, no selection, no drag** on morph diamonds — this is strictly read-only display. Interactive morph keyframes (selection, multi-select, drag, copy/paste) need their own controller surface and land in a separate PR. ## #518 status After this slice the issue is feature-complete and ready to close: | Sub-slice | Status | |-|-| | A1 — Importer + manager + CLI list | shipped (#569) | | A2 — Inspector subgroup | shipped (#570) | | A3 — Authoring data layer | shipped (#578) | | A3b — Inspector authoring UI | shipped (#579) | | A4a — glTF export | shipped (#573) | | A4b — FBX export | shipped (#575) | | A5 — Controller API | shipped (#576) | | A5b — Dope-sheet UI | **this PR** | | A6 — MCP tools | shipped (#571) | 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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: c4913498b5
ℹ️ 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".
| height: visible | ||
| ? (morphHeader.height + morphRowsRep.count * (root.rowHeight + 1) + 4) | ||
| : 0 |
There was a problem hiding this comment.
The morph band height is computed as morphRowsRep.count * (rowHeight + 1) with no upper bound or internal scrolling, so entities with many morph targets can make this rectangle taller than the whole dope sheet. In that case rowsView is anchored to morphBand.top, which can move above the list’s top anchor and collapse/hide bone tracks, making the dope sheet unusable for common high-count blendshape rigs.
Useful? React with 👍 / 👎.
| Item { | ||
| anchors.left: parent.left; anchors.leftMargin: root.leftStripWidth | ||
| anchors.right: parent.right | ||
| height: root.rowHeight |
There was a problem hiding this comment.
This morph timeline container does not set clip: true, but each diamond’s x is driven by (keyTime - viewStart) * pxPerSec. When the user pans/zooms so keys are off-screen, negative/overflow x values will still paint outside the lane (into the name strip or neighboring rows), unlike the bone track strips that are explicitly clipped.
Useful? React with 👍 / 👎.
|
Addresses Codex feedback on PR #580 (merged): ## P1 — bounded morph band height The band's height was `count * (rowHeight + 1)` with no upper bound. On Mixamo-style rigs (50+ blendshapes) the band could push past the top anchor of `rowsView`, hiding the bone tracks entirely and making the dope sheet unusable. Fix: cap the band at 40% of the dope-sheet root height with a floor of one full row. When natural content exceeds the cap, the inner list becomes scrollable via Flickable (plain Flickable + Column instead of ListView so the existing Repeater rows continue to work unchanged). ## P2 — clip morph key lane The per-row timeline `Item` had no `clip: true`, so off-screen diamonds from pan/zoom could paint outside the lane (into the name strip or neighbouring rows). The bone-track strips clip explicitly; the morph band now matches. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Final sub-slice of #518. The C++ data API (
AnimationControlController::allMorphRows) shipped in #576 as aQ_INVOKABLE; this PR wires it into the dope-sheet view.What ships
morphRowsproperty on the dope-sheet root, bound toAnimationControlController.allMorphRows(). Refreshed via the existingonSelectionChangedhandler so a clip change rebuilds both bone and morph rows in lockstep.rowsView(boneListView) bottom anchor switches tomorphBand.topwhen the morph band is visible. Bone-only entities → band collapses toheight=0→ bone list draws full height as before.morphBandRectangle at the bottom: header row showing "Morph Targets (N)", then one row per pose with the target name on the left and#88ccffdiamond markers at each keyframe time on the right. Diamonds share the bone-row timeline math (pxPerSec,viewStart) so they line up vertically with skeletal keyframes.Why this is small and safe
The original A5 (PR #576's first commit) deterministically crashed unrelated
MainWindow/MCPServervisibility-toggle suites in CI. Root cause was never fully isolated but the suspicion was animport PropertiesPanel 1.0triggering a different QML singleton chain during MainWindow construction. This PR keeps the discipline that worked for A3b (#579):AnimationControl 1.0was already imported.allMorphRows()was deliberately put onAnimationControlController(notMorphAnimationManager) for exactly this reason.Connectionsblocks. The existing one already fires on the signals we care about.MouseArea, no selection, no drag on morph diamonds — strictly read-only display. Interactive morph keyframes (selection, multi-select, drag, copy/paste) need their own controller surface and land in a separate PR.#518 status after this PR
The morph epic is feature-complete:
Test plan
🤖 Generated with Claude Code