feat(animation): curve editor per-channel data + interp-mode picker (Phase 5 slice D3b) - #381
Conversation
…slice D3b) Closes part of #380 / closes #378's editing surface. Controller (C++) - Q_INVOKABLE channelValuesAt(bone, channel) — returns the scalar value of one TRS channel at every keyframe of the bone's track, in time order. Curve editor reads this to draw real curves instead of D3a's placeholder samples. - Q_INVOKABLE setKeyframeValue(bone, channel, time, value) — writes one channel back into the keyframe's TransformKeyFrame, leaving the other 9 channels untouched. Pushes a SetKeyframeValueCommand for undo. - Both methods reject unknown channel ids and bail cleanly when no animation/bone is selected. Commands - New commands/SetKeyframeValueCommand. Captures the previous channel value on first redo() so undo restores it exactly. Stores channel as a string so the command survives any track rebuilds between push and undo. 3 unit tests cover redo/undo round-trip, unknown-channel no-op, and missing-time no-op. QML (AnimationCurveEditor.qml) - Per-channel value cache (channelValues map) refreshed via refreshChannelValues() whenever the controller emits boneRows / selection / boneList changes. Curve drawing now uses real values via CurveEditModel.evaluate. - Right-click on a keyframe square opens an interp-mode picker (Bezier / Linear / Stepped / Auto). Hit-tests against the rendered keyframe screen positions, so the user clicks where they see the square. - pickKeyframeAt(px, py) helper for the right-click hit test. - panArea now also accepts RightButton; routes to the mode picker on hits, keeps middle-drag pan untouched. Tests - 3 pure-data + 2 Ogre-fixture cases for the new controller APIs: empty selection / unknown channel / actual values readback / set one channel without disturbing the others. - 3 SetKeyframeValueCommand cases: redo→undo round-trip, unknown- channel no-op, missing-time no-op. Out of scope (deferred) - Free-form tangent handle drag — the canvas-only paint plus the cost of correctly placing handle MouseAreas under arbitrary curves is a substantial QML refactor; opening as a follow-up issue. - Resample-into-TransformKeyFrame on tangent change — only matters once handle drag lands. - Sidecar JSON persistence. The curve editor is now visually live and per-keyframe interpolation mode is editable. The remaining handle-drag + resample are tracked separately so this PR stays reviewable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ 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. Review rate limit: 0/1 reviews remaining, refill in 19 minutes and 50 seconds.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: c2a78abb47
ℹ️ 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".
| } | ||
| onPositionChanged: function(mouse) { | ||
| if (!pressed) return | ||
| if (!pressed || mouse.button !== Qt.MiddleButton) return |
There was a problem hiding this comment.
onPositionChanged gates panning on mouse.button === Qt.MiddleButton, but for move events button is the event trigger, not the currently held button, so this condition is false during normal dragging and middle-button panning stops working. In this handler you need to check the pressed-button bitmask (mouse.buttons) instead to keep drag-to-pan functional.
Useful? React with 👍 / 👎.
|
|
||
| // Cache per-channel values keyed by channel id. Refreshed whenever | ||
| // the controller emits boneRowsChanged. | ||
| property var channelValues: ({}) |
There was a problem hiding this comment.
The per-channel cache starts as {} and is only filled from signal handlers, so when this view is opened after a selection is already active (no fresh controller signal yet), valueAtTimeForChannel() reads empty arrays and CurveEditModel.evaluate() returns 0.0, drawing incorrect flat curves/keyframe Y positions until another signal happens. The cache needs an initial refresh on component startup (or before first paint).
Useful? React with 👍 / 👎.
| UndoManager::getSingleton()->push(cmd); | ||
| refreshSliderTicks(); | ||
| emit boneRowsChanged(); | ||
| return true; |
There was a problem hiding this comment.
setKeyframeValue() always returns true after pushing the command, even when the command does nothing (e.g., missing bone/track or no keyframe at time), because no pre-validation/result check is done here. That reports success for failed edits and also pollutes undo history with no-op commands; this should mirror moveKeyframe()'s upfront validation and return false when no keyframe is actually updated.
Useful? React with 👍 / 👎.
Codex P1: pan drag never fired after press
- onPositionChanged checked mouse.button === MiddleButton, but
mouse.button on a move event is the event trigger (typically
NoButton), not the held buttons. Use mouse.buttons & MiddleButton
bitmask. Without this, middle-drag in the curve editor only
moves on the very first move event before the cursor exits the
press position.
Codex P2: empty curves until first signal
- Per-channel cache started as {} and was only filled from the
controller's signal handlers. Opening the curve editor after a
selection was already active rendered empty curves until the
user touched the selection again. Populate via Component.onCompleted
on the canvas so the first paint already has data.
Codex P2: setKeyframeValue returns true on no-op
- The controller pushed the SetKeyframeValueCommand even when the
target time had no matching keyframe (or the bone/track was
missing), bloating the undo stack. Pre-validate the keyframe
exists before pushing. Mirrors the dope sheet's moveKeyframe
pattern.
- New SetKeyframeValueRejectsWithoutSelection (pure-data) and
SetKeyframeValueRejectsMissingTime (Ogre fixture) tests guard
against regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
1 participant
Summary
Builds on D3a (#379). Curve editor is now visually live (real per-channel values) and per-keyframe interpolation mode is user-editable via a right-click picker.
What's in this PR
channelValuesAt(bone, channel)— controller reads the scalar value of one TRS channel at every keyframe of the bone's track. Curve editor uses it to plot real curves.setKeyframeValue(bone, channel, time, value)— writes one channel back without disturbing the other 9. Undoable via the newSetKeyframeValueCommand.Tests
SetKeyframeValueCommandTestcases (redo→undo round-trip, unknown-channel no-op, missing-time no-op).Out of scope (deferred to follow-up)
I'll open a follow-up issue tracking these so the curve editor work is fully captured.
🤖 Generated with Claude Code