"Repeat" property of NLA action strips not updated when splitting #91299

New Issue

Toby Yang · 2021-09-10T00:16:54+02:00

Toby Yang commented

2021-09-10 00:16:54 +02:00

System Information
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Graphics card: GeForce GTX 1650/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 457.49

Blender Version
Broken: version: 2.93.2, branch: master, commit date: 2021-08-03 05:58, hash: 1eb06de260
Worked: (exists at least since 2.6)

Short description of error
The "Repeat" property under the Action Clip panel is not updated when splitting strips. So changing the repeat value after a split will cause the strip to "jump back" to the original length.

Exact steps for others to reproduce the error

Open blender, switch to the animation workspace, pull up the NLA editor.
Add two keyframes to the cube.
In the NLA editor, press the "push down action" button to make the animation an action strip.
Select the action strip, change the "repeat" property to 3 (or any number).
Move the time cursor to the middle of the strip and press Y to split the strip.
Select one of the two strips, now the "repeat" property is still 3, which should be something close to 1.5
Drag the "repeat" property to change the value. Now the strip jumps back to the original length.

**System Information** Operating system: Windows-10-10.0.19041-SP0 64 Bits Graphics card: GeForce GTX 1650/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 457.49 **Blender Version** Broken: version: 2.93.2, branch: master, commit date: 2021-08-03 05:58, hash: `1eb06de260` Worked: (exists at least since 2.6) **Short description of error** The "Repeat" property under the Action Clip panel is not updated when splitting strips. So changing the repeat value after a split will cause the strip to "jump back" to the original length. **Exact steps for others to reproduce the error** - Open blender, switch to the animation workspace, pull up the NLA editor. - Add two keyframes to the cube. - In the NLA editor, press the "push down action" button to make the animation an action strip. - Select the action strip, change the "repeat" property to 3 (or any number). - Move the time cursor to the middle of the strip and press Y to split the strip. - Select one of the two strips, now the "repeat" property is still 3, which should be something close to 1.5 - Drag the "repeat" property to change the value. Now the strip jumps back to the original length.

Toby Yang commented

2021-09-10 00:16:54 +02:00

Added subscriber: @tianer2820-2

Philipp Oeser commented

2021-09-13 17:12:16 +02:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2021-09-13 17:12:16 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Philipp Oeser commented

2021-09-13 17:12:16 +02:00

Can confirm, cant think of a situation where this wouldnt make sense.
Not sure if this would really be considered a bug (I would lean towards calling it one).

Looks like a #good_first_issue .

Can confirm, cant think of a situation where this wouldnt make sense. Not sure if this would really be considered a bug (I would lean towards calling it one). Looks like a #good_first_issue .

Thibault de Villèle commented

2021-09-23 13:11:15 +02:00

Added subscriber: @thibaulltt

Thibault de Villèle self-assigned this 2021-09-23 13:11:15 +02:00

Thibault de Villèle commented

2021-09-23 13:11:15 +02:00

Hi, I worked on this as my very first issue, and I think I have a working, albeit slightly behaviour-changing solution.

Here is the new behaviour of the split operation on an action strip in regards to its repeat property :

Do all the steps laid out in the original bug report until the splitting operation
Position the cursor over the action clip, and press Y to split it
The split is done according to this new rule :
- Determine the frame from the source action clip that the cursor is positioned at (named splitaframe & contained within [strip->actstart, strip->actend])
- Iterate over the whole repeats of the strip (whole repeats = repeats - modf(repeats, 1.0)) :
  - Duplicate the original action clip into two strips with a repeat of 1.0 each, and with combined length equal to the source action clip's length
  - Set their actstart and actend properties to 'split' them at splitaframe
- For the remainder of the repeat (if the repeat property is not a whole number) :
  - If this last repeat of the clip 'contains' the frame splitaframe, split this repeat in two according to the same rules laid out above
  - If not, simply duplicate the original strip and set the start, actstart, end, and actend properties to reflect the end of the strip
- Remove the original action strip

Since the NLA strips are users of an action clip (according to the code & the documentation), this means the newly-split NLA strips will each consume only a part of the source action clip. This enables the user to specify a repeat property of 2.0 on a sub-split, and have only this part of the action clip repeated.

Here are the things I made sure to do in the code, and tested :

preserve the NLATrack linked list consistency (all newly created splits are linked to each other, and the extremities are linked to the original strip's prev and next pointers)
make sure the code could not split strips with repeats of less than 0.0 (shouldn't even be allowed, or is there a specific use case ?)
make sure the code would behave similarly whether the track was placed in the beginning of the timeline or not (position-agnostic)
make sure to not split if we're in the middle of repeats (caused a crash on my end, prevented for now)

Here are some things I'm not too sure of, and I'd like a bit of guidance about :

At the end of the splitting process, I delete the original strip. Is it my job to do ? Or is it another function's or module's job ?
For now, I disallowed splitting clips when the cursor is placed at the edge of a strip repeat. Should we allow to split into even repeats when the cursor is placed there ?
And most of all : is this the behaviour we want ? I also thought of a scenario where we only split in two the repeat where the cursor is placed, and separate the other whole repeats as separate strips. Not sure of the best use case of the feature.

I have done all the code and have a patch (nearly) ready, I have yet to run the clang-format utility on it :)
I'll assign the task to myself and eagerly wait for feedback !

Hi, I worked on this as my very first issue, and I think I have a working, albeit slightly behaviour-changing solution. Here is the new behaviour of the split operation on an action strip in regards to its repeat property : - Do all the steps laid out in the original bug report until the splitting operation - Position the cursor over the action clip, and press Y to split it - The split is done according to this new rule : - Determine the frame *from the source action clip* that the cursor is positioned at (named `splitaframe` & contained within `[strip->actstart, strip->actend]`) - Iterate over the whole repeats of the strip (whole repeats = `repeats - modf(repeats, 1.0)`) : - Duplicate the original action clip into two strips with a repeat of 1.0 each, and with combined length equal to the source action clip's length - Set their `actstart` and `actend` properties to 'split' them at `splitaframe` - For the remainder of the repeat (if the repeat property is not a whole number) : - If this last repeat of the clip 'contains' the frame `splitaframe`, split this repeat in two according to the same rules laid out above - If not, simply duplicate the original strip and set the `start`, `actstart`, `end`, and `actend` properties to reflect the end of the strip - Remove the original action strip Since the NLA strips are users of an action clip (according to the code & the documentation), this means the newly-split NLA strips will each consume only a part of the source action clip. This enables the user to specify a repeat property of 2.0 on a sub-split, and have only this part of the action clip repeated. Here are the things I made sure to do in the code, and tested : - preserve the `NLATrack` linked list consistency (all newly created splits are linked to each other, and the extremities are linked to the original strip's `prev` and `next` pointers) - make sure the code could not split strips with repeats of less than 0.0 (shouldn't even be allowed, or is there a specific use case ?) - make sure the code would behave similarly whether the track was placed in the beginning of the timeline or not (position-agnostic) - make sure to not split if we're in the middle of repeats (caused a crash on my end, prevented for now) Here are some things I'm not too sure of, and I'd like a bit of guidance about : - At the end of the splitting process, I delete the original strip. Is it my job to do ? Or is it another function's or module's job ? - For now, I disallowed splitting clips when the cursor is placed at the edge of a strip repeat. Should we allow to split into even repeats when the cursor is placed there ? - And most of all : ***is this the behaviour we want ?*** I also thought of a scenario where we only split in two the repeat where the cursor is placed, and separate the other whole repeats as separate strips. Not sure of the best use case of the feature. I have done all the code and have a patch (nearly) ready, I have yet to run the `clang-format` utility on it :) I'll assign the task to myself and eagerly wait for feedback !

Thibault de Villèle commented

2021-09-23 13:12:47 +02:00

Oh and one last thing : my work is based on the blender-v2.93-release branch, should I base it on master instead ?

Oh and one last thing : my work is based on the `blender-v2.93-release` branch, should I base it on `master` instead ?

Toby Yang commented

2021-09-23 19:20:28 +02:00

Hi, I see what you mean.
This is the behavior I expected originally:

(the drawing is bad but should show what I mean)

However, Blender currently does not allow a strip to start from the middle of an action while repeating the entire action (which is the case of strip2 in the drawing) due to how it saves the strip data.

So the most "clear" way to solve this I can think of would be to change how Blender saves NLA strips, but that might be too much work and is unnecessary.

I will prefer the second way you proposed, that "we only split in two the repeat where the cursor is placed, and separate the other whole repeats as separate strips". And maybe group the new strips with meta-strips to make the resulting strips clearer.

Hi, I see what you mean. This is the behavior I expected originally: ![image.png](https://archive.blender.org/developer/F10560913/image.png) (the drawing is bad but should show what I mean) However, Blender currently does not allow a strip to start from the middle of an action while repeating the entire action (which is the case of strip2 in the drawing) due to how it saves the strip data. So the most "clear" way to solve this I can think of would be to change how Blender saves NLA strips, but that might be too much work and is unnecessary. I will prefer the second way you proposed, that *"we only split in two the repeat where the cursor is placed, and separate the other whole repeats as separate strips"*. And maybe group the new strips with meta-strips to make the resulting strips clearer.

Thibault de Villèle commented

2021-09-24 14:08:17 +02:00

@tianer2820-2 Alright. I simply misunderstood your original intention ! I'll work on updating the fix to more closely match the expected behaviour. I'll keep you updated.

Brad Clark commented

2022-02-10 18:27:17 +01:00

Added subscriber: @BClark

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

"Repeat" property of NLA action strips not updated when splitting #91299