Commit Graph

219 Commits

Author SHA1 Message Date
Nathan Vegdahl
a36599b323 Anim: implement 3D viewport keyframing functionality for layered actions
This modifies the 3D viewport keyframing operators to work with layered
actions.  The functionality is relative basic, and still leaves some things out.
Of particular note:

- Keyframing with keying sets does not yet work.
- User preferences such as the `XYZ to RGB` flag and the keyframe interpolation
  type are not yet used/respected.
- Something not caused by this PR but revealed by it: when the last keyframe of
  an fcurve is deleted in a layered action, the fcurve still sticks around. This
  is different from legacy actions, which delete fcurves when their last key is
  deleted. Since the "Only Insert Available" feature is based on the existence
  of fcurves, that makes the feature appear broken in some circumstances right
  now with layered actions.

Pull Request: https://projects.blender.org/blender/blender/pulls/121661
2024-06-07 13:26:18 +02:00
Sybren A. Stüvel
e49963c6a7 Refactor: Anim: type conversions for Strip and KeyframeStrip
Two quality-of-life additions to juggle the `Strip` and `KeyframeStrip`
types:

- New function `Layer.strip_add<KeyframeStrip>()` that returns a
  `KeyframeStrip` instead of a `Strip`.
- Implicit conversion operator `operator Strip &()` on `KeyframeStrip`
  so that `KeyframeStrip` can be passed to a parameter of type
  `Strip &`.

No functional changes.

Pull Request: https://projects.blender.org/blender/blender/pulls/122659
2024-06-06 15:29:05 +02:00
Campbell Barton
7f7648c6ed Cleanup: spelling in code comments & minor edits
- Use uppercase NOTE: tags.
- Correct bNote -> bNode.
- Use colon after parameters.
- Use doxy-style doc-strings.
2024-06-06 09:55:13 +10:00
Nathan Vegdahl
b9c7e5e766 Anim: add keyframing unit tests
Add unit tests for the two main keyframing API functions: `insert_key_rna()` and `insert_keyframe()`.

The tests are not exhaustive of every possible permutation of parameters, but the tests do try to hit each of the behaviors in at least one permutation.  In the future tests should also be added for the lower-level keying functions and behaviors as well, but I'm leaving that as future work since we aren't changing/refactoring those functions right now.

Pull Request: https://projects.blender.org/blender/blender/pulls/122553
2024-06-04 16:03:52 +02:00
Sybren A. Stüvel
0be9540775 Refactor: Anim: move animation.cc into action.cc
Move the content of `animation.cc` into `action.cc`. This is part of the
removal of the `Animation` datablock, and the injection of its
functionality into the `Action` datablock.

The test file `animation_test.cc` is renamed to `action_test.cc`.

No functional changes.

Pull Request: https://projects.blender.org/blender/blender/pulls/122480
2024-05-30 12:23:58 +02:00
Nathan Vegdahl
b821e56165 Refactor: Anim: put default channel group names in one place
Previously they were split between two places, and were not fully
consistent with each other.  The inconsistency didn't result in
user-facing inconsistency due to what was called when at a higher
level, but this will ensure continued user-facing consistency as we make
more changes to the keyframing code.

Pull Request: https://projects.blender.org/blender/blender/pulls/122317
2024-05-28 09:59:35 +02:00
Nathan Vegdahl
ec0a3e966c Refactor: Anim: make insert_key_rna() take RNAPath's instead of strings
This allows each path to optionally specify a single array index,
enabling `insert_key_rna()` to optionally insert keys for just a single
element of an array property. This will allow us to use it in more
places, and eventually reduce the total number of keying functions
needed in the code base.

PR #121879

Pull Request: https://projects.blender.org/blender/blender/pulls/121879
2024-05-24 15:06:21 +02:00
Nathan Vegdahl
98b0bfa9e7 Refactor: make yet more fcurve evaluation functions take const fcurves
This follows on after #121788.

Pull Request: https://projects.blender.org/blender/blender/pulls/121882
2024-05-17 15:56:57 +02:00
Nathan Vegdahl
6709d598cf Anim: don't split keyframing error messages into singular and plural
Having different strings for singular/plural is meaningless or
insufficient in some languages, and thus complicates localization
unnecessarily.
2024-05-14 11:55:14 +02:00
Sybren A. Stüvel
0caf6397df Anim, remove Action::binding_for_id() method
Remove the `Action::binding_for_id()` method, as it is a bit dangerous,
only useful in a very specific situation, and can be removed altogether
with just a little bit of refactoring. This in turn made some other
functions a bit simpler too.

Almost no functional changes. The only change is that when through some
magic (aka data corruption or other bug) the Binding is no longer valid
for the animated ID's type, the IDs properties may still show as
animated in the interface.

Pull Request: https://projects.blender.org/blender/blender/pulls/121748
2024-05-13 17:30:37 +02:00
Sybren A. Stüvel
d94a56bdad Anim: merge Animation data-block into bAction
The new/experimental, layered `Animation` data-block is merged with the
existing `bAction` data-block.

The `Animation` data-block is considerably newer than `bAction`, so the
supporting code that was written for it is also more modern. When moving
that code into `bAction`, I chose to keep the modernity where possible,
and thus some of the old code has been updated as well. Things like
preferring references over pointers.

The `Animation` data-block is now gone from DNA, the main database, etc.
As this was still an experimental feature, there is no versioning code
to convert any of that to Actions.

The DNA struct `bAction` now has a C++ wrapper `animrig::Action`, that
can be obtained via `some_action->wrap()`.

`animrig::Action` has functions `is_empty()`, `is_action_legacy()`, and
`is_action_layered()`. They **all** return `true` when the Action is
empty, as in that case none of the data that makes an action either
'legacy' or 'layered' is there.

The 'animation filtering' code (for showing things in the dope sheet,
graph editor, etc) that I wrote for `Animation` is intentionally kept
around. These types now target 'layered actions' and the
already-existing ones 'legacy actions'. A future PR may merge these two
together, but given how much work it was to add something new there, I'd
rather wait until the dust has settled on this commit.

There are plenty of variables (and some comments) named `anim` or
`animation` that now are of type `animrig::Action`. I haven't renamed
them all, to keep the noise level low in this commit (it's already big
enough). This can be done in a followup, non-functional PR.

Related task: #121355

Pull Request: https://projects.blender.org/blender/blender/pulls/121357
2024-05-13 15:58:04 +02:00
Nathan Vegdahl
58cf1a7c44 Refactor: centralize responsibility for "Only Insert Needed"
This moves the responsibility for handling the "Only Insert Needed"
flag from `insert_keyframe_value()` to the lower-level function
`insert_vert_fcurve()`.

We're doing this because `insert_vert_fcurve()` is the common entry
point between the new animation system's and old animation system's
keyframing code. This therefore ensures that both systems will
behave the same way with respect to the "Only Insert Needed" flag.

Pull Request: https://projects.blender.org/blender/blender/pulls/121525
2024-05-10 17:11:19 +02:00
Nathan Vegdahl
fa201712e1 Anim: simplify object/pose autokeying code
Both `autokeyframe_object()` and `autokeyframe_pose_channel()` had an
odd structure where they would call completely different keying
functions depending on whether "Only Insert Available" was toggled on
or not. This not only overcomplicated the code, but also introduced
the possibility for divergent keyframing behavior depending on whether
that flag was set or not, beyond the intended effect of the flag.

This commit changes both functions to call the same keyframing function
(which already handles that flag correctly on its own) regardless of
whether that flag is set or not, unifying the keyframing behavior and
simplifying the code.

As a happy side effect, auto-keyframing failures are now reported even
when "Only Insert Available" is disabled, which wasn't the case before.
(An example of the aforementioned unintentional divergent behavior.)

Pull Request: https://projects.blender.org/blender/blender/pulls/121476
2024-05-10 17:00:03 +02:00
Nathan Vegdahl
44d020771d Anim: include "Only Key Available" in derived auto-keying flags
The flag for "Only Key Available" was weirdly omitted from the flags
returned by `get_autokey_flags()`, always being set to false. This
happens to be okay at the places the function is currently used, but
it is unexpected and a likely footgun for future changes to the code
that uses it.
2024-05-10 16:59:57 +02:00
Nathan Vegdahl
05bdd6bf34 Cleanup: use "EXPECT" instead of "ASSERT", and rewrap a failure message
I missed some review comments in #120428 that were relevant to the
split-off PR #121517 that recently landed, so am addressing those
comments now as a separate commit:

- Some of the test cases were unnecessarily using ASSERT, and are
  better expressed with EXPECT.
- One of the test-case failure messages needed to be re-wrapped.

Pull Request: https://projects.blender.org/blender/blender/pulls/121528
2024-05-10 11:19:03 +02:00
Sybren A. Stüvel
b56daddda6 Anim: restore logging of key insertion errors on stdout
An earlier commit accidentally removed too much code. This is now
restored, so that when keyframe insertion fails, at least on stdout it's
logged which F-Curve caused this failure.

8dbd167e80 did improve the error
reporting, but even that does not explicitly mention which F-Curves
caused those errors (for good reason, as cluttering the UI would be very
simple then).

Pull Request: https://projects.blender.org/blender/blender/pulls/121527
2024-05-07 17:57:26 +02:00
Nathan Vegdahl
8dbd167e80 Anim: add failure propagation to more lower-level keying functions
This changes the following keyframing functions to return
`SingleKeyingResult`, which is in turn used for better failure
reporting in the higher-level functions that call them:

- `KeyframeStrip::keyframe_insert()`
- `insert_vert_fcurve()`

As a side effect, this also means that
`rna_KeyframeAnimationStrip_key_insert()` can no longer return an
`FCurve *`, and now instead returns a bool indicating success.

This is part of an ongoing progressive refactor to improve error
messages and failure handling in the keyingframing code.

Pull Request: https://projects.blender.org/blender/blender/pulls/121517
2024-05-07 15:06:57 +02:00
Campbell Barton
bb172dae61 Cleanup: use const variables & arguments 2024-05-06 09:20:03 +10:00
Campbell Barton
9918488bb1 Cleanup: use uppercase tags, following own style guide 2024-05-03 11:33:21 +10:00
Christoph Lendenfeld
5b7a0ed1b5 Refactor: move keyingset enums to animrig
No functional changes expected.

This PR moves the enums
`eModifyKey_Modes` and `eModifyKey_Returns` to animrig
as enum classes.

Functions that take or return that value were also modified.

Pull Request: https://projects.blender.org/blender/blender/pulls/121132
2024-05-02 12:00:37 +02:00
Campbell Barton
f6b7464b4c Cleanup: spelling in comments 2024-05-02 16:44:10 +10:00
Sybren A. Stüvel
15cc446773 Cleanup: anim, rename 'out' to 'binding' in unit test
A unit test file was still using `out`, short for 'output', which was the
original name for what later became 'binding'. This is now updated, and
the variables are now named appropriately.

Pull Request: https://projects.blender.org/blender/blender/pulls/121273
2024-04-30 18:01:49 +02:00
Sybren A. Stüvel
10c00b6390 Anim: Add RNA enum for animation bindings + operator to un-assign
Add an RNA enum property `AnimData.animation_binding` that lists all the
bindings available in `AnimData.animation`.

The list of bindings is filtered to only contain the bindings suitable
for the animated ID. This prevents assigning a 'camera' binding to a
mesh.

Un-assigning is done via an operator, represented as an 'X' button in
the interface.

The enum property contains up to two special items:
- "New" to create a new binding for the ID.
- "(none/legacy)" to indicate that this ID doesn't have a binding
  assigned. This one is conditional, and only appears when it is
  necessary.

These two special items are experimental, and mostly exist because we're
still evaluating things and building a better UI. It is intended that
the binding selector will become as close to the ID selector as
possible.

-----------

Note that this PR also contains #121268 as it builds up from that one, and I didn't want to wait with testing on the buildbot until that one lands.

The new Baklava panel:

![image](/attachments/ac357f32-d50a-481b-8b3c-9c0be07424b6)

Pull Request: https://projects.blender.org/blender/blender/pulls/121269
2024-04-30 17:37:16 +02:00
Sybren A. Stüvel
0da53b5e62 Anim: Change how names of Bindings work, and how Bindings are created/assigned
This cleans up some of the Animation/Binding API, and adds a distinction
between a binding's "name" and its "display name".

`name`: internal name that is unique within the `Animation`. As such, it
        is also the key into the `anim.bindings` collection.
  - To ensure the uniqueness, `name` is always prefxed with the ID
    identifier, like `OBCube` and `CACamera`.
  - A binding that was not created to animate a specific ID will be
    called `XXBinding`.
`name_display`: display name that strips the first two characters, so in
        the above examples would be `Cube`, `Camera`, and `Binding`.

### RNA setter behaviour

`name`: always sets the name, emitting a warning when the name's prefix
doesn't match the ID type of the Binding. This implicitly changes the
display name (as they are two views into the same string).

`name_display`: sets `name = prefix_for_ID_type + name_display`. So even
when the old name was `QQSomethingWeird`, setting `binding.name_display
= "NewName"` would effectively set `binding.name = "OBNewName"`
(assuming it was already bound to some object earlier).

Bindings now also **always have a name**. Previously it was possible to
create bindings named `""`, but that's no longer possible.

Bindings used to be **renamed automatically** when they were first
assigned, for example from `XXBinding` to `OBCube`. This behaviour has
been removed, as it could potentially cause confusion.

Pull Request: https://projects.blender.org/blender/blender/pulls/120941
2024-04-30 15:51:47 +02:00
Christoph Lendenfeld
1cf193336a Refactor: Rename enum entries of BakeCurveRemove
No functional changes.

The enum entries of `BakeCurveRemove` used to
start with `REMOVE` which just doubled that word.

Removing that simplifies the enum entries.

This has been brought up by @dr.sybren in #120984

Pull Request: https://projects.blender.org/blender/blender/pulls/121258
2024-04-30 14:40:26 +02:00
Christoph Lendenfeld
33c6e9f92c Refactor: move FCurve baking code to animrig
No functional changes.

This PR moves the following public functions to animrig:
* sample_fcurve_segment
* bake_fcurve
* bake_fcurve_segments

The `bake_fcurve_segments` also gets a docstring and its comments have been
updated to follow the style guide.

Pull Request: https://projects.blender.org/blender/blender/pulls/120984
2024-04-30 12:22:47 +02:00
Christoph Lendenfeld
5ba5fc8962 Anim: make Keyframing error messages translateable
This fixes an issue with the error messages that keying
can produce where they couldn't be translated.

To make translation possible, two things changed:
* use of `RPT_`
* simplified string formatting

Pull Request: https://projects.blender.org/blender/blender/pulls/121112
2024-04-30 09:59:51 +02:00
Christoph Lendenfeld
0dd0583a75 Refactor: move get_keyframing_flags function to animrig
No functional changes.

This PR moves the `ANIM_get_keyframing_flags` function to
animrig as `get_keyframing_flags`.
The docstring has been fixed since it was out of date.

Pull Request: https://projects.blender.org/blender/blender/pulls/121120
2024-04-26 13:21:55 +02:00
Campbell Barton
1865776767 Cleanup: doxygen syntax, use colons after "param" arguments, not "note" 2024-04-24 10:55:44 +10:00
Campbell Barton
07e365fcbf Cleanup: remove unused variable 2024-04-23 18:18:47 +10:00
Christoph Lendenfeld
c30647a6fc Refactor: remove ReportList argument from insert_key
The insert key function in `animrig/keyframing.cc` took a `ReportList`
argument which it used to print messages in case of failures.

Instead this now returns a `CombinedKeyingResult` and the caller is
responsible for creating reports out of that.

To make that simpler the `ID` argument has been changed from a pointer to a reference.
The calling functions now make sure that it's not a `nullptr`.

This has the effect that there will be less messages printed in the Info panel when e.g. inserting keys with a keyingset.
This still doesn't make an error message pop up though.

Related to #119776

Pull Request: https://projects.blender.org/blender/blender/pulls/120784
2024-04-23 10:10:23 +02:00
Sybren A. Stüvel
933f74ca02 Anim: Baklava, add Animation data-block to anim filtering code
Add minimal support for the `Animation` data-block to the anim filtering
code. This means:

- F-Curves in the `Animation` data-block are shown in the Dope Sheet and
  Graph Editor.
- The `Animation` is shown underneath each animated data-block, just
  like an `Action` would be.
- Contrary to Actions, the expanded/collapsed state is stored on
  `id->adt` and not on `Animation`. Because an `Animation` is intended
  to be used by multiple data-blocks, they each should have their own
  flag for this.
- In the filtering code, this PR adds the 'fillanim' channel type. This
  is simply mimicking the name from the Action's 'fillact' type.

Limitations:

- Deleting of F-Curves is explicitly blocked, as that'll be introduced
  in a later PR. The block prevents Blender from crashing.
- The Dope Sheet doesn't have an Animation mode yet, that'll be for a
  later PR too.

Pull Request: https://projects.blender.org/blender/blender/pulls/120654
2024-04-20 12:19:16 +02:00
Sybren A. Stüvel
f9890fd98d Anim: fix animation paths when renaming bone collections
Bone collections are drawn in the new tree view by accessing properties
from `armature.collections_all[...]`, which means that drivers and
fcurves also target those. This is now taken into account when updating
the bone collection name.

For context: in Blender 4.0 the UIList used `.collections[...]` instead,
for which renaming was already handled properly.

Pull Request: https://projects.blender.org/blender/blender/pulls/120517
2024-04-11 15:40:59 +02:00
Campbell Barton
3a8cceee7d Cleanup: remove redundant checks & assignments 2024-04-11 20:47:07 +10:00
Campbell Barton
09ee8d97e6 Cleanup: use C-style comments for descriptive text 2024-04-11 17:44:27 +10:00
Campbell Barton
74a65d77cc Cleanup: spelling in comments 2024-04-10 12:28:33 +10:00
Nathan Vegdahl
05226881f7 Anim: make CombinedKeyingResult plain old data
The data in CombinedKeyingResult was using BLI's Array type, which
heap allocates.  However, we know the array size at compile time,
and therefore can use std::array instead, as suggested in Array's
documentation.  This makes CombinedKeyingResult a
plain-old-data class.

Pull Request: https://projects.blender.org/blender/blender/pulls/120425
2024-04-09 17:27:20 +02:00
Christoph Lendenfeld
956d8379a4 Anim: Detailed report if no keyframes have been inserted
With this PR, when pressing `I` in the viewport and the code
is unable to insert **ANY** keyframes, the user will be presented
with a single message detailing exactly why it has failed.

This PR promotes the functionality introduced in
#117449 into the header file so it can be used elsewhere.

The `CombinedKeyingResult` class is returned
from `insert_key_action` and `insert_key_rna`, and
used to produce a single report from the operator if it
failed to insert any keyframes.

In order to easily create a report from a `CombinedKeyingResult`
the function `generate_keyframe_reports_from_result`
has been moved into the class as `generate_reports`.

In addition to that the `UNABLE_TO_INSERT_TO_NLA_STACK` result
has been added. This notifies the user if keyframe insertion is not
possible due to NLA stack settings.

Pull Request: https://projects.blender.org/blender/blender/pulls/119201
2024-04-09 09:39:13 +02:00
Sybren A. Stüvel
694e5b50f4 Anim: show animated properties with the appropriate color in the GUI
Use the `Animation` data-block to find F-Curves, for drawing the
background color of properties in the GUI (green for 'animated', yellow
for 'keyed on this frame', etc.).

This assumes (correctly) that the `Animation` is limited to a single
layer with a single strip. As such, there is only one set of F-Curves
for every animated ID, which means that the correct F-Curve will be
found. This will need adjustment when the same property can have
multiple F-Curves (due to layers) or when an F-Curve may not be
applicable for each point in time (due to the use of finite strips).

Pull Request: https://projects.blender.org/blender/blender/pulls/118677
2024-04-08 12:53:32 +02:00
Sybren A. Stüvel
631f72265d Anim: add evaluation of Animation data-blocks
Include Animation data-block handling in Blender's animation evaluation
stack. If an `Animation` is assigned to an `ID`, it will take precedence
over the NLA and/or any `Action` that might be assigned as well.

For more info, see #113594.

Pull Request: https://projects.blender.org/blender/blender/pulls/118677
2024-04-08 12:53:32 +02:00
Sybren A. Stüvel
8879654dd0 Anim: allow inserting keys in Animation data-block
Allow inserting keys into Keyframe strips (which is the only type of
strip that is currently implemented).

Note that the data model is currently limited to a single layer, with a
single infinite strip. Because of this, the strip will not be shown in
any UI, as there is no way to manipulate it anyway.

Note that the inserted keys are not yet evaluated, so the animation
isn't visible in the 3D viewport yet. That's for an upcoming commit.

For more info, see #113594.

Pull Request: https://projects.blender.org/blender/blender/pulls/118677
2024-04-08 12:53:32 +02:00
Sybren A. Stüvel
13f3a81842 Anim: allow assigning Animation data-blocks
Expand the `AnimData` struct with an `Animation *` + an
`binding_stable_index` field, and properly handle those relations.

This also adds functionality for actually pointing animated IDs to
`Animation` data-blocks, and automatically hooking up the relevant
`Binding`.

The Depsgraph code is extended to take these new relations into account,
but doesn't trigger any animation evaluation yet.

For more info, see #113594.

Pull Request: https://projects.blender.org/blender/blender/pulls/118677
2024-04-08 12:53:32 +02:00
Sybren A. Stüvel
38878b4ac2 Anim: add Animation data-block management functions
Add code (including RNA wrappers) for:

- Creating, removing, and accessing `Animation` data-blocks.
- Creating and removing layers, strips, and bindings on those `Animation`
  data-blocks.
- Accessing those via RNA.

Note that this does not include assignment to any animated data-block,
so it is of limited practical use.

For more info, see #113594.

Pull Request: https://projects.blender.org/blender/blender/pulls/118677
2024-04-08 12:53:32 +02:00
Sybren A. Stüvel
d6863d43da Cleanup: Anim, re-word the explanation of the term 'binding'
Re-word the explanation of 'binding', as the old explanation was still
based on the old name 'output'.

No functional changes.
2024-04-05 12:45:26 +02:00
Sybren A. Stüvel
85d77b79a6 Refactor: Anim, use eBezTriple_KeyframeType in more code
Instead of using `short key_type`, use `eBezTriple_KeyframeType key_type`,
so that it's clear which type it is, and so that a `switch()` can cause
compiler warnings when it's incomplete.

This also adds missing `case`s to `switch`es where necessary, in a way
that doesn't affect the outcome. There is one change that looks like it
is a functional change, but it should provide the same result:

```diff
- size -= 0.8f * key_type;
+ size *= 0.8f;
```

Since `size = 12` and in this case `key_type = 3`, the numerical values
are the same, but now the code is consistently multiplying and thus should
scale properly.

Furthermore some overly obvious comments are removed and some missing
`const` keywords have been added.

No functional changes.

Pull Request: https://projects.blender.org/blender/blender/pulls/120178
2024-04-05 11:53:57 +02:00
Campbell Barton
7e9f7320e4 Cleanup: spelling in comments & comment blocks 2024-04-04 11:26:28 +11:00
Campbell Barton
b03332a055 Cleanup: use BLI_assert_msg instead of checking string literals 2024-04-03 14:27:54 +11:00
Christoph Lendenfeld
fdf75dcf99 Refactor: Remove duplicate function get_keyframe_values
No functional changes.

The function `get_keyframe_values` existed twice.
Once in a legacy state, which also did NLA logic.
And also in a new implementation where it only returned keyframe values.

This PR removes the legacy function and moves the NLA logic to a separate function.

Pull Request: https://projects.blender.org/blender/blender/pulls/119798
2024-03-28 17:00:21 +01:00
Christoph Lendenfeld
6276dd2f64 Fix #119946: NLA stack decomposition doesn't work with bones
The issue was that the `PointerRNA` passed to `BKE_animsys_get_nla_keyframing_context`
needs to point to an `ID` which wasn't the case when keying bones.
That is because internally the `FCurve` path is used to resolve the property.
This can only work from the `ID` because the `FCurve` path is always stored relative to that.
While the function doesn't fail when the property can't resolve the path, it won't actually do
the remapping when passing it to `BKE_animsys_nla_remap_keyframe_values` later.

Pull Request: https://projects.blender.org/blender/blender/pulls/120008
2024-03-28 15:14:01 +01:00
Hans Goudey
efee753e8f Cleanup: Move BKE_idprop.h to C++ 2024-03-26 13:07:04 -04:00