griefith/test
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Cleanup: improve documentation of Action C++ APIs

Browse Source 
A pass at cleaning up / improving the code documentation for the layered action
C++ classes.

There is still more work to do here, but this should be good for an initial
first pass. I've focused on clarifying the behavior of and updating stale docs
of the methods in the action-related C++ classes.

Pull Request: https://projects.blender.org/blender/blender/pulls/131318
This commit is contained in:
Nathan Vegdahl
2025-01-10 15:13:38 +01:00
committed by Nathan Vegdahl
parent eb57878bec
commit 4fd011675b
2 changed files with 228 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files

278
source/blender/animrig/ANIM_action.hh
View File

@@ -43,13 +43,35 @@ class Slot;
/**
* Container of animation data for one or more animated IDs.
*
* Broadly an Action consists of Layers, each Layer has Strips, and it's the
* Strips that eventually contain the animation data.
* An Action broadly consists of four things:
*
* Temporary limitation: each Action can only contain one Layer.
* 1. Layers, which contain Strips.
* 2. Strips, which reference StripData.
* 3. StripData: Strip{TYPE}Data contains animation data of the given type. For
* example, StripKeyframeData (currently the only StripData type) contains
* keyframes.
* 4. Slots, which are used as identifiers for subsets of animation data within
* StripData items.
*
* Which sub-set of that data drives the animation of which ID is determined by
* which Slot is associated with that ID.
* StripData is not stored in the Strips themselves, but rather is stored
* separately at the top level of the Action, and each Strip *references* a
* StripData item. This allows Strip instancing by having more than one Strip
* reference the same StripData item.
*
* Each Action has a set of Slots defined at its top level. The animation data
* within a StripData item is organized into one or more subsets, each of which
* is marked as being for a different Slot.
*
* For an ID to be animated by an Action, the ID must specify both an Action and
* a Slot within that Action. The Slot that the ID uses determines which subset
* of the animation data throughout the Action it is animated by. If an Action
* but no Slot is specified, the ID is simply not animated.
*
* \note Temporary limitations: each Action can only contain one Layer, and each
* Layer can only contain one infinite Strip with no time offset. These
* limitations will be progressively lifted as we implement layered animation
* and non-linear animation functionality for Actions in the future. (See:
* `assert_baklava_phase_1_invariants()`.)
*
* \note This wrapper class for the `bAction` DNA struct only has functionality
* for the layered animation data. The legacy F-Curves (in `bAction::curves`)
@@ -66,6 +88,7 @@ class Slot;
*
* \see #AnimData::action
* \see #AnimData::slot_handle
* \see assert_baklava_phase_1_invariants()
* \see #animrig::versioning::action_is_layered()
*/
class Action : public ::bAction {
@@ -77,7 +100,12 @@ class Action : public ::bAction {
*/
Action(const Action &other) = delete;
/* Discriminators for 'legacy' and 'layered' Actions. */
/* Discriminators for 'legacy' and 'layered' Actions.
*
* Note: `is_action_legacy()` and `is_action_layered()` are transitional APIs,
* and should eventually be removed. See their documentation below for
* details.
*/
/**
* Return whether this Action has any data at all.
*
@@ -97,10 +125,10 @@ class Action : public ::bAction {
*
* \note This method will be removed when runtime support for legacy Actions
* is removed, so only use it in such runtime code. See
* `BKE_action_is_layered()` for uses that should stick around for the long
* term, such as blend file loading and versioning.
* `animrig::versioning::action_is_layered()` for uses that should stick
* around for the long term, such as blend file loading and versioning.
*
* \see #BKE_action_is_layered
* \see #animrig::versioning::action_is_layered()
*/
bool is_action_legacy() const;
/**
@@ -113,10 +141,10 @@ class Action : public ::bAction {
*
* \note This method will be removed when runtime support for legacy Actions
* is removed, so only use it in such runtime code. See
* `BKE_action_is_layered()` for uses that should stick around for the long
* term, such as blend file loading and versioning.
* `animrig::versioning::action_is_layered()` for uses that should stick
* around for the long term, such as blend file loading and versioning.
*
* \see #BKE_action_is_layered
* \see #animrig::versioning::action_is_layered()
*/
bool is_action_layered() const;
@@ -126,6 +154,23 @@ class Action : public ::bAction {
const Layer *layer(int64_t index) const;
Layer *layer(int64_t index);
/**
* Create a new layer in this Action.
*
* The new layer is added to the end of the layer array, and will be empty (no
* strips).
*
* \note At the time of writing this comment only a single layer per Action is
* supported in Blender, but this function does NOT enforce that. Be careful!
*
* \param name The name to give the new layer. If no name is given, a default
* name is used. The name may be altered (e.g. appending ".001") to enforce
* uniqueness within the Action.
*
* \return A reference to the newly created layer.
*
* \see assert_baklava_phase_1_invariants()
*/
Layer &layer_add(std::optional<StringRefNull> name);
/**
@@ -211,13 +256,31 @@ class Action : public ::bAction {
*/
void slot_identifier_propagate(Main &bmain, const Slot &slot);
/**
* Return the slot in this action with the given identifier, if any.
*
* \return A pointer to the matching slot, or nullptr if no matching slot is
* found.
*/
Slot *slot_find_by_identifier(StringRefNull slot_identifier);
/**
* Create a new, unused Slot.
* Create a new Slot.
*
* The returned slot will be suitable for any ID type. After slot to an
* ID, it be limited to that ID's type.
* This method should generally not be used outside of low-level code and
* legacy action versioning code, because it creates a Slot with an
* unspecified intended ID type, which should be avoided. Prefer
* `slot_add_for_id_type()` and `slot_add_for_id()` for adding new slots.
*
* TODO: we should probably rename this method to make it clear that it
* shouldn't be used as the standard way to add a slot.
*
* The slot is given a default name and will be suitable for any ID type.
* After assigning the slot to an ID, it will be changed to only be suitable
* for that ID's type.
*
* \see slot_add_for_id_type()
* \see slot_add_for_id()
*/
Slot &slot_add();
@@ -229,12 +292,14 @@ class Action : public ::bAction {
Slot &slot_add_for_id_type(ID_Type idtype);
/**
* Create a new slot suitable for the ID's type.
* Create a new, unused Slot suitable for the given ID.
*
* The slot will be named after `animated_id.adt.last_slot_identifier`, defaulting to the ID's
* name when that is not set. This is done so that toggling Actions works transparently, when
* toggling between `this` and the Action last assigned to the ID.
*
* The slot will only be suitable for the ID's type.
*
* Note that this assigns neither this Action nor the new Slot to the ID. This function
* merely initializes the Slot itself to suitable values to start animating this ID.
*/
@@ -290,6 +355,9 @@ class Action : public ::bAction {
/**
* Check if the slot with this handle has any keyframes.
*
* If called on a legacy action, `action_slot_handle` is ignored and the
* fcurves of the legacy action are checked for keyframes.
*
* \see is_slot_animated()
*/
bool has_keyframes(slot_handle_t action_slot_handle) const ATTR_WARN_UNUSED_RESULT;
@@ -403,6 +471,20 @@ class Action : public ::bAction {
void strip_keyframe_data_remove_if_unused(int index);
private:
/**
* Create a new slot for this Action, but *don't* add it to the Action's list
* of slots.
*
* This *does* give the slot a slot handle, and also correspondingly updates
* the Action's `last_slot_handle` field, hence why this is a method on
* Action.
*
* This is a low-level function. Prefer `slot_add()` and friends in most
* cases.
*
* \see slot_add()
* \see slot_add_for_id()
*/
Slot &slot_allocate();
/**
@@ -486,16 +568,33 @@ class Strip : public ::ActionStrip {
return Type(this->strip_type);
}
/**
* Return whether the strip's frame range extends from -infinity to +infinity.
*/
bool is_infinite() const;
/**
* Return whether the given frame is within the strip's frame range.
*
* \note Strip frame ranges are inclusive on both sides.
*/
bool contains_frame(float frame_time) const;
/**
* Return whether the end of the strip's frame range matches the given frame
* time.
*/
bool is_last_frame(float frame_time) const;
/**
* Set the start and end frame.
*
* Note that this does not do anything else. There is no check whether the
* frame numbers are valid (i.e. frame_start <= frame_end). Infinite values
* (negative for frame_start, positive for frame_end) are supported.
* This directly sets the start/end frames to the values given. It is up to
* the caller to ensure the invariants of the strip itself and of the layer it
* belongs to.
*
* `frame_start` must be less than or equal to `frame_end`. Infinite values
* (negative for `frame_start`, positive for `frame_end`) are supported.
*/
void resize(float frame_start, float frame_end);
@@ -504,6 +603,12 @@ class Strip : public ::ActionStrip {
*
* `T` *must* correspond to the strip's data type. In other words, this must
* hold true: `T::TYPE == strip.type()`.
*
* For example, to get a keyframe strip's data:
*
* ```
* StripKeyframeData &strip_data = strip.data<StripKeyframeData>(action);
* ```
*/
template<typename T> const T &data(const Action &owning_action) const;
template<typename T> T &data(Action &owning_action);
@@ -528,6 +633,10 @@ static_assert(sizeof(Strip) == sizeof(::ActionStrip),
*
* Temporary limitation: at most one strip may exist on a layer, and it extends
* from negative to positive infinity.
*
* Note: the invariants around multiple strips (such as strip overlap, ordering
* within the strip array, etc.) have not yet been decided. These will be
* decided and documented when support for multiple strips is added.
*/
class Layer : public ::ActionLayer {
public:
@@ -536,12 +645,12 @@ class Layer : public ::ActionLayer {
~Layer();
/**
* Duplicate the `Layer` and its `Strip`s, but only make shallow copies of the
* Duplicate the layer and its strips, but only make shallow copies of the
* strips.
*
* Specifically, this doesn't duplicate the strip data that's stored in e.g.
* `Action::strip_keyframe_data_array`, and it leaves the fields of the strips
* themselves exactly as-is.
* Specifically, this doesn't duplicate the strip data that's stored in the
* layer's owning action, leaving the fields of the strips themselves
* exactly as-is.
*
* WARNING: this method is primarily used in the code that makes full
* duplicates of actions, where the arrays of strip data are copied separately
@@ -579,7 +688,7 @@ class Layer : public ::ActionLayer {
return static_cast<MixMode>(this->layer_mix_mode);
}
/* Strip access. */
/* Strip array access. */
blender::Span<const Strip *> strips() const;
blender::Span<Strip *> strips();
const Strip *strip(int64_t index) const;
@@ -587,6 +696,10 @@ class Layer : public ::ActionLayer {
/**
* Add a new Strip of the given type.
*
* This creates a new infinite strip and appends it to the end of the layer's
* strip array. It does no validation of invariants, and it is up to the
* caller to ensure that invariants hold.
*/
Strip &strip_add(Action &owning_action, Strip::Type strip_type);
@@ -608,7 +721,10 @@ class Layer : public ::ActionLayer {
void slot_data_remove(Action &owning_action, slot_handle_t slot_handle);
protected:
/** Return the strip's index, or -1 if not found in this layer. */
/**
* Return the index of `strip` in this layer's strip array, or -1 if not found
* in this layer.
*/
int64_t find_strip_index(const Strip &strip) const;
};
static_assert(sizeof(Layer) == sizeof(::ActionLayer),
@@ -623,9 +739,6 @@ ENUM_OPERATORS(Layer::Flags, Layer::Flags::Enabled);
* to identify which F-Curves (and in the future other animation data) it will
* be animated by.
*
* This is called a 'slot' because it binds the animatable ID to the sub-set
* of animation data that should animate it.
*
* \see #AnimData::slot_handle
*/
class Slot : public ::ActionSlot {
@@ -658,23 +771,52 @@ class Slot : public ::ActionSlot {
static_assert(sizeof(NlaStrip::last_slot_identifier) == identifier_length_max);
/**
* Return the identifier prefix for the Slot's type.
* Return the prefix of this Slot's identifier.
*
* This is the ID name prefix, so "OB" for objects, "CA" for cameras, etc.
* This corresponds to the intended ID type (`idtype`) of the slot, e.g "OB"
* for object, "CA" for camera, etc.
*/
std::string identifier_prefix_for_idtype() const;
/**
* Return the identifier without the prefix, also known as the "display name".
* Return this Slot's identifier without the prefix, also known as the
* "display name".
*
* E.g. if the identifier is "OBCube", then "Cube" is returned.
*
* \see identifier_prefix_for_idtype
*/
StringRefNull identifier_without_prefix() const;
/** Return whether this Slot is usable by this ID type. */
/**
* Return whether this Slot is suitable to be used by the given ID.
*
* "Suitable" means that one of the following is true:
*
* - The Slot's intended ID type (`idtype`) matches the given ID's type.
* - The Slot's intended ID type is unspecified (see `has_idtype()`).
*
* If either of those hold true, the Slot is considered suitable for the ID.
* Otherwise it is considered unsuitable.
*
* Note that it is possible, but odd, for an ID to use a Slot that is not
* suitable for it. This is discouraged, and a best effort is made to prevent
* this in typical cases, but it is not possible to completely prevent due to
* library linking (e.g. an Action linked from another file may be replaced in
* that other file, causing its Slots to effectively change). Therefore this
* method returning `false` should NOT be taken as a guarantee that this Slot
* will never be used by the given ID or other IDs of the same type.
*
* \see identifier_prefix_for_idtype() \see has_idtype()
*/
bool is_suitable_for(const ID &animated_id) const;
/** Return whether this Slot has an `idtype` set. */
/**
* Return whether this Slot has a specified intended ID type (`idtype`) set.
*
* \see identifier_prefix_for_idtype()
* \see is_suitable_for()
*/
bool has_idtype() const;
/* Flags access. */
@@ -740,18 +882,25 @@ class Slot : public ::ActionSlot {
static void users_invalidate(Main &bmain);
/**
* Ensure the first two characters of the identifier match the ID type.
* Ensure the first two characters of this Slot's identifier match its
* intended ID type.
*
* This typically should not be called directly. Prefer assigning to an ID to
* get the idtype and identifier prefix properly set. Prefer calling
* `Action::slot_identifier_set()` if you want to set the slot identifier. Both of those
* approaches take care of ensuring uniqueness and other invariants.
* This typically does not need to be called outside of some low-level
* functions. Aside from versioning code that upgrades legacy actions, Slots
* should always be created with a specific intended ID type and corresponding
* identifier prefix that never changes after creation, making this method
* unnecessary.
*
* In the rare cases that a Slot does not have a specified intended ID type,
* this method *still* should typically not be called directly. In those cases
* prefer assigning to an ID (e.g. via `Action::assign_action_slot()`), which
* will set the Slot's intended ID type and identifier prefix to match the
* given ID's type, as well as ensure identifier uniqueness within the Action.
*
* \note This does NOT ensure identifier uniqueness within the Action. That is the
* responsibility of the caller.
*
* \see #assign_action_slot
* \see #Action::slot_identifier_set
*/
void identifier_ensure_prefix();
@@ -768,7 +917,13 @@ static_assert(sizeof(Slot) == sizeof(::ActionSlot),
ENUM_OPERATORS(Slot::Flags, Slot::Flags::Active);
/**
* Keyframe strips effectively contain a bag of F-Curves for each Slot.
* Keyframe animation data for a keyframe strip.
*
* This contains a set of Channelbags, up to one for each slot in the owning
* action. Each Channelbag contains the keyframe animation data for the slot it
* corresponds to.
*
* \see ChannelBag
*/
class StripKeyframeData : public ::ActionStripKeyframeData {
public:
@@ -786,9 +941,9 @@ class StripKeyframeData : public ::ActionStripKeyframeData {
Channelbag *channelbag(int64_t index);
/**
* Find the animation channels for this slot.
* Find the channelbag for the given slot.
*
* \return nullptr if there is none yet for this slot.
* \return nullptr if there is none yet for the given slot.
*/
const Channelbag *channelbag_for_slot(const Slot &slot) const;
Channelbag *channelbag_for_slot(const Slot &slot);
@@ -796,23 +951,23 @@ class StripKeyframeData : public ::ActionStripKeyframeData {
Channelbag *channelbag_for_slot(slot_handle_t slot_handle);
/**
* Add the animation channels for this slot.
* Add a channelbag for the given slot.
*
* Should only be called when there is no `Channelbag` for this slot yet.
*/
Channelbag &channelbag_for_slot_add(const Slot &slot);
/**
* Find the Channelbag for `slot`, or if none exists, create it.
* Find the channelbag for the given slot, or if none exists, create it.
*/
Channelbag &channelbag_for_slot_ensure(const Slot &slot);
/**
* Remove the Channelbag from this slot.
* Remove the given channelbag from this strip data.
*
* After this call the reference is no longer valid, as the memory will have been freed.
*
* \return true when the Channelbag was found & removed, false if it wasn't found.
* \return true when the channelbag was found & removed, false if it wasn't found.
*/
bool channelbag_remove(Channelbag &channelbag_to_remove);
@@ -823,7 +978,10 @@ class StripKeyframeData : public ::ActionStripKeyframeData {
*/
void slot_data_remove(slot_handle_t slot_handle);
/** Return the channelbag's index, or -1 if there is none for this slot handle. */
/**
* Return the index of `channelbag` in this strip data's channelbag array, or
* -1 if `channelbag` doesn't exist in this strip data.
*/
int64_t find_channelbag_index(const Channelbag &channelbag) const;
SingleKeyingResult keyframe_insert(Main *bmain,
@@ -839,6 +997,12 @@ static_assert(sizeof(StripKeyframeData) == sizeof(::ActionStripKeyframeData),
/**
* Collection of F-Curves, intended for a specific Slot handle.
*
* In addition to F-Curves, Channelbags can also contain ChannelGroups, which
* are used to organize F-Curves within the Channelbag (e.g. all F-Curves for a
* given bone can be put into a ChannelGroup with that bone's name).
*
* \see ChannelGroup
*/
class Channelbag : public ::ActionChannelbag {
public:
@@ -888,8 +1052,11 @@ class Channelbag : public ::ActionChannelbag {
/**
* Append an F-Curve to this Channelbag.
*
* Ownership of the F-Curve is also transferred to the Channelbag. The F-Curve
* will not belong to any channel group after appending.
* This transfers ownership of the F-Curve to this Channelbag, and it is up to
* the caller to ensure that this is valid (e.g. the F-Curve doesn't also
* belong to something else).
*
* The F-Curve will not be member of any group after appending.
*
* This is considered a low-level function. Things like depsgraph relations
* tagging is left to the caller.
@@ -899,7 +1066,7 @@ class Channelbag : public ::ActionChannelbag {
/**
* Remove an F-Curve from the Channelbag.
*
* Additionally, if the fcurve was the last fcurve in a channel group, that
* Additionally, if the F-Curve was the last F-Curve in a channel group, that
* channel group is also deleted.
*
* After this call, if the F-Curve was found, the reference will no longer be
@@ -959,6 +1126,9 @@ class Channelbag : public ::ActionChannelbag {
/**
* Remove all F-Curves from this Channelbag.
*
* Since all channel groups become empty, this also removes all channel
* groups.
*/
void fcurves_clear();
@@ -1145,16 +1315,16 @@ static_assert(sizeof(Channelbag) == sizeof(::ActionChannelbag),
* This does *not* own the fcurves--the Channelbag does. This just groups
* fcurves for organizational purposes, e.g. for use in the channel list in the
* animation editors.
*
* Usage of this wrapper typically indicates that the group is part of a layered
* action. However, the underlying `bActionGroup` struct is also used by legacy
* actions.
*/
class ChannelGroup : public ::bActionGroup {
public:
/**
* Determine whether this channel group is from a legacy action or a layered action.
*
* TODO: this should be removed, as it's currently only used by code that is
* no longer relevant and should also be removed due to legacy actions no
* longer being supported at runtime.
*
* \return True if it's from a legacy action, false if it's from a layered action.
*/
bool is_legacy() const;

14
source/blender/makesdna/DNA_action_types.h
View File

@@ -753,15 +753,9 @@ typedef enum eActionGroup_Flag {
/* Actions -------------------------------------- */
/**
* Action - reusable F-Curve 'bag' (act)
* Container of animation data.
*
* This contains F-Curves that may affect settings from more than one ID block-type and/or
* data-block (i.e. sub-data linked/used directly to the ID block that the animation data is linked
* to), but with the restriction that the other unrelated data (i.e. data that is not directly used
* or linked to by the source ID block).
*
* It serves as a 'unit' of reusable animation information (i.e. keyframes/motion data),
* that affects a group of related settings (as defined by the user).
* \see blender::animrig::Action for more detailed documentation.
*/
typedef struct bAction {
/** ID-serialization for relinking. */
@@ -1143,8 +1137,8 @@ typedef struct ActionLayer {
uint8_t _pad0[2];
/**
* There is always at least one strip.
* If there is only one, it can be infinite. This is the default for new layers.
* The layer's array of strips. See the documentation of
* #blender::animrig::Layer for the invariants of this array.
*/
struct ActionStrip **strip_array; /* Array of 'strip_array_num' strips. */
int strip_array_num;