Blender Git Loki

Blender Git "arcpatch-D8867_Nla_Merge_Strips" branch commits.

October 21, 2020, 04:19 (GMT)
Feature: NLA Merge Strips

Todo: add media and example files

This patch is relative to {D8296} (which is relative to {D9247}. Apply {D9247} first then {D8296} then this one.

For user-facing design discussion: {T80235}
____

**Problem/Solution:**
Before, the only way to combine multiple Nla strips into a single strip is to execute a Bake operator. However, this will merge **all** strips into a replace strip with full influence. This patch creates a new operator "Resample Strips To New" which effectively allows merging a set of selected NlaStrips into a single strip of any blendmode and any nonzero influence. The old selected strips will be muted while preserving the whole Nla result. (I used the word "resample" because it's more general. But most animators will recognize it as a "merge"). This patch offers a more flexible solution to {T69105}, where the solution implemented was to force baking to a single replace strip with full influence.

**Feature:** Nla add support for resampling selected strips into a new track. The more specific use is to support merging selected Nla strips into a single strip. The core resampling function is (anim_sys.c) //BKE_animsys_resample_selected_strips()//

(direct copy+paste of function comment)
```
/** Mute selected NLA strips and resample into a new track. The final Nla stack result will be
* preserved when possible. New resampled strip will be selected. Previously selected strips will
* be muted and deselected afterward.
*
* param resample_blendmode: Resulting resampled strip's blend mode.
* param resample_influence: Resulting resampled strip's influence. above.
* param resample_insertion_nlt_index: NlaTrack to insert the resample track above or below.
* param insert_track_lower: Side of resample_insertion_nlt_index to place resample track.
* returns: The new resample track. Returns NULL and does nothing if in tweak mode, resample
* influence zero, or no fcurves are involved in the resample.
*/
```

**Intended Uses:**

Merge Strips: User selects a block of NlaStrips and Resamples. Effectively, all selected strips will become muted and a new "merged" track will be created. Since the overall NLA stack result is preserved, the result is equivalent to a merge.

Convert Strip: User selects a single NlaStrip and Resamples with a different blendmode and/or influence.

**Potential improvements/changes:**

For frames where user had a keyframe, make them non-selected. Select non-user keys. This allows a follow-up op to do an Fcurve simplify or decimate of only the baked keyframes. Effectively it allows a follow-up Smart Bake that preserves user keys. Perhaps this can be done by the caller.

Allow user to somehow select channels to be resampled. Currently all channels found in all selected strips are resampled. Though a simple work around is to delete the undesired channels after the resample.

**Limitations and potential problems:**

Design: When resample strip value not valid, what should we do? Currently we write a default value. Nothing we write will preserve the animation. This leaves the problem as a "Known Issue".

This operator will not properly resample outside of the resample bounds. Generally, it's not possible since multiple strips with non-None extend modes can not be represented by a single strip of any extend mode.. Maybe it's possible by properly setting the pre and post extrapolation for individual fcurves?

_____

**Some existing functions in (anim_sys.c) that had to be modified slightly for //BKE_animsys_resample_selected_strips()//:**

- changes (anim_sys.c) //nlastrip_evaluate()// and its nested calls to pass //allow_alloc_channels //to control whether NlaEvalChannels are allocated or to only blend existing channels. Flushing and keyframing require allocation. The new merging operator only needs to blend existing channels and requires no allocation.

There are potential areas for refactoring but haven't been implemented to keep the review simpler and more linear. It's an attempt to prevent introducing bugs that occur as a result of refactoring instead of the patch itself.

Differential Revision: https://developer.blender.org/D8867
October 21, 2020, 04:18 (GMT)
Feature: NLA: Evaluate Whole NLA Stack in Tweak Mode

Feature: NLA: show and evaluate whole NLA stack while in tweak mode:

**Note for reviewers**
This patch is relative to {D9247}. Apply that patch first, then apply this one afterward.

For reviewers, the two core functions changed are in anim_sys.c (BKE_animsys_nla_remap_keyframe_values and animsys_calculate_nla). I've separated the old animsys_evaluate_nla() into for_flush and for_keyframing variations to make the intent clear.

I should add that the nlastrip_evaluate() and recursive strip evaluate calls have been duplicated for inverting. They can be refactored but I've decided against it. I didn't want to refactor and add a new feature in a single patch. Keeping those calls nearly the same should make them easier to understand relative to the old implementation. I suppose I could've refactored first then made the patch, but without the duplication it could've been difficult to see the motivation for refactoring choices.

**Question**: I also noticed a UI-based confusion introduced by patch. If the animator has a nonpushed action then it has influence even while in tweak mode which is expected. However, the bounds of that influence is currently not drawn in any way while in tweak mode. It can be a surprise and source of confusion when the non-pushed action begins to influence the animation result. I should talk to the UI team about how it should be drawn. Or maybe it should be a separate patch so the UI can be developed separately given this patch is relatively big already? Should it just be part of {D7600} since that patch is close to the problem area and this one depends on it?

**Problem/Solution**
This feature solves the problem of not being able to see and consider the final animation result while tweaking a strip. Before, as a user keyframes a strip, the upper strips would be disabled. Now, the user can optionally view the upper strip affects while keyframing. The strips above it are accounted for such that the final NLA result matches what the user visually keyframed. The feature pros and cons itself sounds self explanatory but let me know if I should explain more in depth.

**Implementation**
The core implementation is that each upper strip needs to be inverted separately, solving for the blend output of the lower stack. That goes on until we have solved for the blend output of the tweaked strip and lower stack. Then we use the old existing invert math to get the fcurve value of the tweak strip. The lower stack can be inverted as a group instead of separately because we're solving for the tweak strip's value, not the lower blended value, and it doesn't include the tweaked strip.

**Changes to old behavior (Evaluating without upper stack)**
Evaluating upper stack is optional and, for now, set as default (Tab). The RClick context menu and header menu exposes the option to evaluate without the upper stack (Ctrl+Tab).

Tweaked strip no longer uses animdata->act_influence since that's used by the non-pushed action.

**Bugfixes Included Relative to Public Version **
This includes bugfixes such as evaluating meta strips correctly when next to a transition {D8287}.

**Limitations Introduced by Patch**
Keyframing through a quaternion transition of (Combine<->Replace/Add/Sub/Mul) is not handled properly and will fail to insert a keyframe. I'm still working out the math, at least for the case of (Combine<->Replace). Other quaternion transition cases should be handled properly (Combine<->Combine) and (Add/Sub/Mul/Replace <-> Add/Sub/Mul/Replace). All non-quaternion transition cases should be handled properly. This limitation only applies when such transitions appear above the tweaked strip and the upper NLA stack is enabled. Transitions that appear below the tweaked strip does not break keyframing.

**Example Files**
Location Focused:
{F8802904}
Quaternion Focused:
{F8802907}
Scale Focused:
{F8802906}

Each file has multiple objects in it that have simple NLA setups. It's just meant to save a little bit of time. Autokeying is on and keying sets match the file focus. Per check, just unhide one object at a time, enter tweak mode on the lowest strip, then try to keyframe. In most cases, your overall NLA results should be preserved when refreshing the keyframe(changing to different frame then back to the keyframed one). General failure cases include not being able to keyframe through full (influence = 1) Replace strips nor Multiply Zero strips. These are expected, mathematically there is no solution. For quaternion, an additional failure case can occur even when keyframing through a partial Replace strip. Quaternions only interpolate within 180 degrees so if the solution requires that it rotates further than 180, then the keyframe will be wrong. **(Note to self: Todo)** Current patch implementation does not properly fail on this case.

For location, it's easy to verify by using auto keying and snapping Suzanne to cursor. Refresh the frame and her origin should match the cursor. Generally, I just eyeball things and use the grid.

No specific Euler file since NLA evaluation is the same as location values. The Scale file only includes Combine strip examples since other blends modes evaluate the same as any other property, like location.

**Existing Limitations of NLA Not Solved By Patch**
If the tweaked strip's action evaluates multiple times in the same frame, then it's not solved for correctly. To support this is nontrivial. We have to allow keyframing at multiple frames and account for the different flags of each strip that uses the same action. The lower stack would have to be inverted separately just like the upper stack. Each strip may also sample the action using a different action range too.. Transitions between the linked action.. It's complicated. I expect it to be rare for the user to linked duplicate an action, overlay them stack-wise and want to keyframe.. That feature is out of scope. The attached file is a quick setup to verify the limitation. Autokeying is already on. Move the cube on the X-Axis then refresh the frame (change back to the same frame). The cube will be in a different location than keyed.
{F8883083}

Differential Revision: https://developer.blender.org/D8296
October 21, 2020, 04:18 (GMT)
Refactor: NLA Prep for D8296

No intended functional changes.

This is the refactor of NLA evaluation to prepare for {D8296}. I was using TortoiseGitMerge to look at the diff to try to make the diff more readable.. but it looks like arcanist(?)/this-site uses a different method.

Refactor notes

- generally, functions not shuffled around so diffs are clearer
- in //(anim_sys.c) nlaeval_blend_value()//, the bitmap writes to// NlaEvalChannel->valid// are unused and redundant so they're removed.
- renames //NlaEvalChannel->valid// to //NlaEvalChannel->domain// for term consistency with usage
- for blending functions, more descriptive names used and float equality checks changed to use //IS_EQF()// instead of ==.
- //(anim_sys.c) animsys_evaluate_nla()// separated into //for_flush //and //for_keyframing //variants to reduce complexity by making the use clear.
- the dummy strip creation has been refactored to two separate functions for the tweak strip and nonpushed action strip. Both are evaluated differently from other strips and eachother. There's no need to interweave them. A future patch {D8296} generally requires both strips.
- XXX //(anim_sys.c) nlatrack_find_tweaked()// is a work around and temporary. If anyone has any insight into this problem, help is appreciated.
- //(anim_sys.c) nonstrip_action_fill_strip_data()// doesn't call// nlastrips_ctime_get_strip()// due to a future patch {D8867} needs to call the latter N times but the former only needs to be called once.
- //(anim_sys.c) BKE_is_nlatrack_evaluatable() //is an API function so a future patch {D8867} can access it.
- //(anim_sys.c) BKE_animsys_nla_remap_keyframe_values()// removed full replace strip early out. Future patch {D8296} can't use it.
- add const, limited to refactored areas
- remove redundant switch-case fall throughs to default
- //nlaeval_blend_flush()// small refactor, replace branching with assertion
- return value of //nla_combine_quaternion_invert_get_fcurve_values()// implemented to handle case where influence == 0. This failure case added to other blend inversions too. The Nla remap function // (anim_sys.c) BKE_animsys_nla_remap_keyframe_values()// does this check but future patch {D8867} will not use the remap function directly.

Differential Revision: https://developer.blender.org/D9247
Tehnyt: Miika HämäläinenViimeksi päivitetty: 07.11.2014 14:18MiikaH:n Sivut a.k.a. MiikaHweb | 2003-2021