Anim: Discuss edge cases and add regression tests for NLA strip evaluation #109212

Merged
Nate Rupsis merged 1 commits from pamadini/blender:nla-adjacent-strips-evaluate-later into main 2023-11-20 17:52:37 +01:00
Contributor

This pull request has a wider discussion of some NLA edge cases and adds regression tests. The core change that was agreed upon has now been rebased on the latest main branch and moved to #113487.


This changes the behavior of adjacent NLA strips for Blender 4.0, as discussed in the 2023-06-08 Animation & Rigging module meeting.

I'd like to write some regression tests before requesting review.

Optionally, the first part of the nlastrips_ctime_get_strip function could be streamlined with a larger refactoring.

This pull request has a wider discussion of some NLA edge cases and adds regression tests. The core change that was agreed upon has now been rebased on the latest main branch and moved to #113487. --- This changes the behavior of adjacent NLA strips for Blender 4.0, as discussed in the [2023-06-08 Animation & Rigging module meeting](https://devtalk.blender.org/t/2023-06-08-animation-rigging-module-meeting/29793#nla-behavior-last-frame-of-clip-8). I'd like to write some regression tests before requesting review. Optionally, the first part of the `nlastrips_ctime_get_strip` function could be streamlined with a larger refactoring.
Iliya Katueshenock added the
Interest
Animation & Rigging
label 2023-06-21 20:14:30 +02:00
Sybren A. Stüvel added
Module
Animation & Rigging
and removed
Interest
Animation & Rigging
labels 2023-06-22 19:18:58 +02:00
Member

First thanks for exploring the NLA :)

Second I don't think having inconsistent behavior if the clips are on the same track or split should happen, if it changes it so that the next clips start frame is more important to show than the first clips last frame, that should be how it is no matter where the clip is..touching or above/below...the next clip is the next clip.

Third, I don't know how this changes how the HOLD and HOLD forward behavior works with the clips on the same track.

If you can fix it so that it is consistent when the clips are on different tracks and can you please test how this impacts/changes how with two clips with Blending together would work.

Looking testing this in other tools when not touching or blending the last frame of the clip is shown, when they touch or blend, the NEXT clips start frame becomes the winner and is shown.... and I wonder if this would help blending between clips work better in Blender.

First thanks for exploring the NLA :) Second I don't think having inconsistent behavior if the clips are on the same track or split should happen, if it changes it so that the next clips start frame is more important to show than the first clips last frame, that should be how it is no matter where the clip is..touching or above/below...the next clip is the next clip. Third, I don't know how this changes how the HOLD and HOLD forward behavior works with the clips on the same track. If you can fix it so that it is consistent when the clips are on different tracks and can you please test how this impacts/changes how with two clips with Blending together would work. Looking testing this in other tools when not touching or blending the last frame of the clip is shown, when they touch or blend, the NEXT clips start frame becomes the winner and is shown.... and I wonder if this would help blending between clips work better in Blender.
Author
Contributor

Thank you @BClark! I had a more complete write-up made before I saw your comment, I originally wanted to update the pull request description but I'll just include the write-up in its original form below, since you didn't have a chance to see it before, and I didn't see you comment either.

Let me know if this answers some or all of your questions. The behavior with clips on a different track is not changed in the pull request based on the assumptions I made. My thinking is still that no change is necessary there, but we can discuss further and I don't feel strongly either way, as it covers a non-default use case that takes a few steps to trigger.

Summary

The goal of this task is to identify and implement the desired behavior for non-linear animation (NLA) strips when evaluated at the boundary of their frame range in Blender 4.0 and beyond.

This investigation is motivated by an agreed inconsistency in Blender 3.x and earlier, when two strips are adjacent on the same track. The last frame of the first strip is evaluated, instead of the first frame of the second strip. This is not so much the result of deliberate design, as much as the result of how the current code iterates through strips.

While it is unambiguous that the first frame of a strip should be evaluated when encountered, there are alternatives about what to do when the time is exactly on the last frame of a strip, and no other strip is present at the same time.

Assumptions

Unless otherwise specified, throughout the discussion it is assumed that the first and last frame of an NLA strip are not extrapolated beyond the boundary, despite the default in Blender being to hold the values of the first and last frames. This means that, when the NLA track is evaluated at a time with no strip, the track has no effect on the result.

The problem space of defining frame ranges in non-linear animation tracks is slightly more nuanced than non-linear video editing software, because frame numbers can be fractional. I’d note that in every case I found in practice, the boundaries are integers, but this can’t be relied upon.

When expressing frame ranges in natural language, it is my experience that the first and last frames are assumed to be included. For example:

  • A walk cycle exercise would include instructions to set up the scene from frame 1001 to 1033, and set keys at regular intervals on the first, middle, and last frame at 1001, 1017, and 1033. The first and last frames would each be played separately to check that they are equal.
  • In film production, when a shot runs from frame 1001 to 1010, it would result in 10 frames being rendered. The next shot will start at frame 1011, consistently with how ranges are expressed in non-linear video editing.
  • A four-frame strip of an eye closing would consist of frames 1, 2, 3, and 4, with frame 1 fully open and 4 fully closed. This means 3 new poses in addition to the first one.

However, when game developers deploy non-linear animation, it may be convenient to reason "mathematically" and never evaluate the last frame. It seems to be the case both in Unity’s Timeline and Unreal’s Sequencer that a clip extending from frames 1 to 4 would evaluate at frames 1, 2, 3, and up to 3.999…, but go back to no evaluation at frame 4.

Inside a strip, animation curves are not necessarily continuous. For example, when constant (stepped) tangents are used on keys at frames 3 and 4, the value at frame 3.999… may be different from the value at frame 4.

Current Behavior and Alternatives

At present, Blender evaluates strips on their first and last frames, both included. My opinion is that this makes sense, thinking of the wider target audience that it has compared to just game development. For example, consider this workflow:

  1. Create a new default scene.
  2. Set a key on the cube on frames 1 and 4.
  3. “Push down” the action in the NLA editor.
    The strip extends from frame 1 to 4, and the key on frame 4 is evaluated.
  4. Set “Extrapolation” to “Nothing”.
    The strip is still evaluated at frame 4, and back to default at frame 5.

It might be confusing if the last frame was always excluded, as a key would be there and visible in most editors, but it would have no effect. This could also lead to workarounds such as extending the last frame of the strip to just a little over the last frame, such as frame 4.00...01. The exact value to choose would be ambiguous.

Using the last key only when "Extrapolation" is not "Nothing" would mean that the strip would effectively “lose” its last frame when the extrapolation is changed. It would require moving the boundary to keep the last pose. On the other hand, it would be consistent with how game engines work, and address differently the edge case on different tracks that is discussed below.

A new extrapolation mode "Nothing, but include the last frame" would require too much micro-management, and more variables would have to be considered when debugging an NLA sequence.

Adjacent Clips on The Same Track (Changed)

When a strip is manually repeated, a new copy or instance is placed adjacent to the previous one. For example, a cycle from 1001 to 1033 would be followed by the same strip running from 1033 to 1065. In this case, regardless of the extrapolation, frame 1033 should be the first frame of the second strip, which is the only change made by this pull request.

This new behavior would be the same as when the strip is extended and set to repeat (loop) internally. The last frame of the underlying action is only evaluated if the last iteration of the loop falls exactly at the end boundary of the strip. This is the same frame that is held afterwards.

As a side note, Unity has a flag to control looping, and when enabled, the last frame would always be excluded from evaluation. Setting "Post-extrapolate" to "Hold" would hold the first frame of the underlying animation at the end, instead of the last frame.

Clips on Different Tracks (Kept as Now)

If the two strips in the example above have "Extrapolation" set to "Nothing" and one of them is moved to a different track, the value at frame 1033 would be combined based on the strip blending modes. With the default of "Replace", the result would be different based on which clip is above the other.

In game engines, always excluding the last frame has the effect that the result at the boundary does not depend on the order of the tracks, or whether the same track is used for both clips. However, the context in which game developers operate may be different, and what may seem mathematically “cleaner” to reason about for a developer may be more confusing for a target audience that is accustomed to considering the key at the last frame to be significant.

Moreover, with "Hold" being the default extrapolation, the result for all frames, not just 1033, is already different when the clips are moved to a different track. Thus, there may be simply not be enough motivation to change the current behavior for a non-default edge case.

Test File

The following test file has an action running from frames 1 to 4, where the X Location of the cube is the same as the frame number, with constant tangents. This action is referenced by different strips in the NLA editor.

With the change in the pull request, the value at frame 4 will be "1", like it is at frame 22, instead of "4", like it is at frame 13.

Thank you @BClark! I had a more complete write-up made before I saw your comment, I originally wanted to update the pull request description but I'll just include the write-up in its original form below, since you didn't have a chance to see it before, and I didn't see you comment either. Let me know if this answers some or all of your questions. The behavior with clips on a different track is not changed in the pull request based on the assumptions I made. My thinking is still that no change is necessary there, but we can discuss further and I don't feel strongly either way, as it covers a non-default use case that takes a few steps to trigger. ### Summary The goal of this task is to identify and implement the desired behavior for non-linear animation (NLA) strips when evaluated at the boundary of their frame range in Blender 4.0 and beyond. This investigation is motivated by an [agreed inconsistency](https://devtalk.blender.org/t/2023-06-08-animation-rigging-module-meeting/29793#nla-behavior-last-frame-of-clip-8) in Blender 3.x and earlier, when two strips are adjacent on the same track. The last frame of the first strip is evaluated, instead of the first frame of the second strip. This is not so much the result of deliberate design, as much as the result of how the [current code](https://projects.blender.org/blender/blender/src/commit/91ff6457bed71289a88fe34d35ae4ccb4451b73c/source/blender/blenkernel/intern/anim_sys.c#L989-L993) iterates through strips. While it is unambiguous that the first frame of a strip should be evaluated when encountered, there are alternatives about what to do when the time is exactly on the last frame of a strip, and no other strip is present at the same time. ### Assumptions Unless otherwise specified, throughout the discussion it is assumed that the first and last frame of an NLA strip are **not extrapolated** beyond the boundary, despite the default in Blender being to hold the values of the first and last frames. This means that, when the NLA track is evaluated at a time with no strip, the track has no effect on the result. The problem space of defining frame ranges in non-linear animation tracks is slightly more nuanced than non-linear video editing software, because **frame numbers can be fractional**. I’d note that in every case I found in practice, the boundaries are integers, but this can’t be relied upon. When expressing frame ranges **in natural language**, it is my experience that the **first and last frames are assumed to be included**. For example: - A walk cycle exercise would include instructions to set up the scene from frame 1001 to 1033, and set keys at regular intervals on the first, middle, and last frame at 1001, 1017, and 1033. The first and last frames would each be played separately to check that they are equal. - In film production, when a shot runs from frame 1001 to 1010, it would result in 10 frames being rendered. The next shot will start at frame 1011, consistently with how ranges are expressed in non-linear video editing. - A four-frame strip of an eye closing would consist of frames 1, 2, 3, and 4, with frame 1 fully open and 4 fully closed. This means 3 new poses in addition to the first one. However, when **game developers** deploy non-linear animation, it may be convenient to reason "mathematically" and never evaluate the last frame. It seems to be the case both in Unity’s Timeline and Unreal’s Sequencer that a clip extending from frames 1 to 4 would evaluate at frames 1, 2, 3, and up to 3.999…, but go back to no evaluation at frame 4. Inside a strip, animation curves are **not necessarily continuous**. For example, when constant (stepped) tangents are used on keys at frames 3 and 4, the value at frame 3.999… may be different from the value at frame 4. ### Current Behavior and Alternatives At present, Blender evaluates strips on their first and last frames, both included. My opinion is that this makes sense, thinking of the wider target audience that it has compared to just game development. For example, consider this workflow: 1. Create a new default scene. 2. Set a key on the cube on frames 1 and 4. 3. “Push down” the action in the NLA editor. _The strip extends from frame 1 to 4, and the key on frame 4 is evaluated._ 4. Set “Extrapolation” to “Nothing”. _The strip is still evaluated at frame 4, and back to default at frame 5._ It might be confusing if the last frame was always excluded, as a key would be there and visible in most editors, but it would have no effect. This could also lead to workarounds such as extending the last frame of the strip to just a little over the last frame, such as frame 4.00...01. The exact value to choose would be ambiguous. Using the last key only when "Extrapolation" is not "Nothing" would mean that the strip would effectively “lose” its last frame when the extrapolation is changed. It would require moving the boundary to keep the last pose. On the other hand, it would be consistent with how game engines work, and address differently the edge case on different tracks that is discussed below. A new extrapolation mode "Nothing, but include the last frame" would require too much micro-management, and more variables would have to be considered when debugging an NLA sequence. ### Adjacent Clips on The Same Track (Changed) When a strip is manually repeated, a new copy or instance is placed adjacent to the previous one. For example, a cycle from 1001 to 1033 would be followed by the same strip running from 1033 to 1065. In this case, regardless of the extrapolation, frame 1033 should be the first frame of the second strip, which is the only change made by this pull request. This new behavior would be the same as when the strip is extended and set to repeat (loop) internally. The last frame of the underlying action is only evaluated if the last iteration of the loop falls exactly at the end boundary of the strip. This is the same frame that is held afterwards. As a side note, Unity has a flag to control looping, and when enabled, the last frame would always be excluded from evaluation. Setting "Post-extrapolate" to "Hold" would hold the first frame of the underlying animation at the end, instead of the last frame. ### Clips on Different Tracks (Kept as Now) If the two strips in the example above have "Extrapolation" set to "Nothing" and one of them is moved to a different track, the value at frame 1033 would be combined based on the strip blending modes. With the default of "Replace", the result would be different based on which clip is above the other. In game engines, always excluding the last frame has the effect that the result at the boundary does not depend on the order of the tracks, or whether the same track is used for both clips. However, the context in which game developers operate may be different, and what may seem mathematically “cleaner” to reason about for a developer may be more confusing for a target audience that is accustomed to considering the key at the last frame to be significant. Moreover, with "Hold" being the default extrapolation, the result for all frames, not just 1033, is already different when the clips are moved to a different track. Thus, there may be simply not be enough motivation to change the current behavior for a non-default edge case. ### Test File The following test file has an action running from frames 1 to 4, where the X Location of the cube is the same as the frame number, with constant tangents. This action is referenced by different strips in the NLA editor. With the change in the pull request, the value at frame 4 will be "1", like it is at frame 22, instead of "4", like it is at frame 13.
Paolo Amadini force-pushed nla-adjacent-strips-evaluate-later from 075cdbc1cb to c60c9acfc0 2023-06-29 17:51:18 +02:00 Compare
Member

I still don't have a good sense for how this is going to feel/change but I hope I can look at it when I have some more rest next week.
Thank you for your interest and looking at the NLA code :)

I have 15 or so issues in my document that are still open and some are very tiny and some are whole new features involving the NLA.

I still don't have a good sense for how this is going to feel/change but I hope I can look at it when I have some more rest next week. Thank you for your interest and looking at the NLA code :) I have 15 or so issues in my document that are still open and some are very tiny and some are whole new features involving the NLA.

I really like that you've added new tests for the NLA. How would you feel about moving these into a separate PR, so that they cover the current behaviour of Blender? That way this PR can remain focused on the change in behaviour, keeping the two goals nicely separated. Approving & landing the new tests will be considerably easier than discussing the change in behaviour, so let's keep the latter as small as possible.

I really like that you've added new tests for the NLA. How would you feel about moving these into a separate PR, so that they cover the current behaviour of Blender? That way this PR can remain focused on the change in behaviour, keeping the two goals nicely separated. Approving & landing the new tests will be considerably easier than discussing the change in behaviour, so let's keep the latter as small as possible.
Author
Contributor

Thank you, Sybren, for taking a look!

My availability will be limited for the time being, and between the two goals I'd rather focus on making the behavior consistent. However, the tests are already nicely separated in their own commit, and even if they have not landed yet, the core team is very welcome to take the commit and use the code as a template to cover the current behavior, or use it for testing other parts of the nonlinear animation editor.

Thank you, Sybren, for taking a look! My availability will be limited for the time being, and between the two goals I'd rather focus on making the behavior consistent. However, the tests are already nicely separated in [their own commit](https://projects.blender.org/blender/blender/commit/075cdbc1cbe47c9429b85b1f5d99bc0af5bffb63), and even if they have not landed yet, the core team is very welcome to take the commit and use the code as a template to cover the current behavior, or use it for testing other parts of the nonlinear animation editor.

Having separate commits is good. The thing is mostly that the test now sits after the change in functionality and thus will need adjustment if they're to be committed to main first.

Having separate commits is good. The thing is mostly that the test now sits *after* the change in functionality and thus will need adjustment if they're to be committed to `main` first.
Paolo Amadini changed title from WIP: Animation: At the boundary frame between non-muted NLA strips, evaluate the first frame of the later strip instead of the last frame of the earlier. to WIP: Discuss edge cases and add regression tests for NLA strip evaluation 2023-10-10 13:15:24 +02:00
Author
Contributor

Having separate commits is good. The thing is mostly that the test now sits after the change in functionality and thus will need adjustment if they're to be committed to main first.

Exactly, I was suggesting the team could use the code "as a template" if they wanted to make the adjustments you mention, or if they wanted to add tests to other pull requests that they were working on. This didn't happen in the meantime, since other pull requests touching the same file landed without tests, which is fine as the current convention for that file is that tests are not required.

In fact, I believe you are fundamentally correct, in that adding regression tests and multi-track behavior to the discussion in the same pull request has caused some scope bleed. Thus, I moved the fundamental change to #113487 to keep it separate and not overload reviewers with too much information. I may pick the tests up again and ask for review sometime later, but for now I'd like to save reviewer bandwidth for the change that actually matters the most to the users.

> Having separate commits is good. The thing is mostly that the test now sits *after* the change in functionality and thus will need adjustment if they're to be committed to `main` first. Exactly, I was suggesting the team could use the code "as a template" if they wanted to make the adjustments you mention, or if they wanted to add tests to other pull requests that they were working on. This didn't happen in the meantime, since other pull requests touching the same file landed without tests, which is fine as the current convention for that file is that tests are not required. In fact, I believe you are fundamentally correct, in that adding regression tests and multi-track behavior to the discussion in the same pull request has caused some scope bleed. Thus, I moved the fundamental change to #113487 to keep it separate and not overload reviewers with too much information. I may pick the tests up again and ask for review sometime later, but for now I'd like to save reviewer bandwidth for the change that actually matters the most to the users.

This is actually the opposite of what I intended. One of the big issues with the NLA, which makes working on it so cumbersome and risky, is the lack of unit tests. I wasn't trying to point out scope creep, but rather trying to motivate you to take the tests that you have in this PR, and adjust them so that they work on current main. Then a separate PR can alter Blender's behaviour, which would impact both the main code and the tests (which is a good thing).

This is actually the opposite of what I intended. One of the big issues with the NLA, which makes working on it so cumbersome and risky, is the lack of unit tests. I wasn't trying to point out scope creep, but rather trying to motivate you to take the tests that you have in this PR, and adjust them so that they work on current `main`. Then a separate PR can alter Blender's behaviour, which would impact both the main code and the tests (which is a good thing).
Paolo Amadini force-pushed nla-adjacent-strips-evaluate-later from c60c9acfc0 to 5eda2f8766 2023-10-17 14:39:03 +02:00 Compare
Paolo Amadini changed title from WIP: Discuss edge cases and add regression tests for NLA strip evaluation to Discuss edge cases and add regression tests for NLA strip evaluation 2023-10-17 14:40:13 +02:00
Author
Contributor

I've taken the tests and adjusted them so that they work on current main. Who should I ask for review?

I've taken the tests and adjusted them so that they work on current `main`. Who should I ask for review?

You can add @nrupsis and me as reviewers.

You can add @nrupsis and me as reviewers.
Paolo Amadini requested review from Sybren A. Stüvel 2023-10-24 17:18:22 +02:00
Paolo Amadini requested review from Nate Rupsis 2023-10-24 17:18:22 +02:00
Nate Rupsis reviewed 2023-10-25 20:30:40 +02:00
@ -0,0 +1,120 @@
# SPDX-FileCopyrightText: 2023 Blender Authors
#
# SPDX-License-Identifier: GPL-2.0-or-later
Member

Just double checking we're using GPL-2.0

I'm seeing a lot of test files with SPDX-License-Identifier: Apache-2.0

Just double checking we're using GPL-2.0 I'm seeing a lot of test files with `SPDX-License-Identifier: Apache-2.0`
Author
Contributor

Both seem to be in use, happy to switch if it's needed once you've determined which license works best for new files here.

Both seem to be in use, happy to switch if it's needed once you've determined which license works best for new files here.

Blender is "GPLv2.0 or later". Other code could have been submitted with a different (but compatible) license. When in doubt, use GPLv2+.

Blender is "GPLv2.0 or later". Other code could have been submitted with a different (but compatible) license. When in doubt, use GPLv2+.
dr.sybren marked this conversation as resolved
@ -0,0 +27,4 @@
def setUpClass(cls):
bpy.ops.wm.read_factory_settings()
cls.test_object = bpy.data.objects["Cube"]
Member

Might be worth creating / tearing down an object as well, that way we're not reliant on the default Blender scene which could (probably won't) change.

Might be worth creating / tearing down an object as well, that way we're not reliant on the default Blender scene which could (probably won't) change.
pamadini marked this conversation as resolved
@ -0,0 +47,4 @@
strip.extrapolation = "NOTHING"
return strip
def assertFrameValue(self, frame: float, value: float):
Member

Would change the parameter name to "expected_value". Gives a clearer indication what the method does.

Would change the parameter name to "expected_value". Gives a clearer indication what the method does.
pamadini marked this conversation as resolved
@ -0,0 +117,4 @@
if __name__ == "__main__":
# Drop all arguments before "--", or everything if the delimiter is absent. Keep the executable path.
unittest.main(argv=sys.argv[:1] + (sys.argv[sys.argv.index("--") + 1:] if "--" in sys.argv else []))
Member

(Unrelated to this PR. ) This seems like a pretty consistent pattern across the python tests.

@dr.sybren would it make sense for this to be abstracted into a Util of some sort?

(Unrelated to this PR. ) This seems like a pretty consistent pattern across the python tests. @dr.sybren would it make sense for this to be abstracted into a Util of some sort?

Maybe at some point, but not in this PR ;-)

Maybe at some point, but not in this PR ;-)
dr.sybren marked this conversation as resolved
Paolo Amadini force-pushed nla-adjacent-strips-evaluate-later from 5eda2f8766 to 8db7f4f041 2023-10-29 17:34:43 +01:00 Compare
Nate Rupsis approved these changes 2023-11-02 16:56:04 +01:00
Nate Rupsis left a comment
Member

Awesome to see some python tests for the NLA!

Awesome to see some python tests for the NLA!

@pamadini for some reason anonymous access to https://projects.blender.org/pamadini/blender is blocked. Was this something you configured? To integrate well with other systems, please make sure the project is publicly accessible.

@pamadini for some reason anonymous access to https://projects.blender.org/pamadini/blender is blocked. Was this something you configured? To integrate well with other systems, please make sure the project is publicly accessible.
Member

@pamadini for some reason anonymous access to https://projects.blender.org/pamadini/blender is blocked. Was this something you configured? To integrate well with other systems, please make sure the project is publicly accessible.

Huh, his fork is loading fine for me...

> @pamadini for some reason anonymous access to https://projects.blender.org/pamadini/blender is blocked. Was this something you configured? To integrate well with other systems, please make sure the project is publicly accessible. > Huh, his fork is loading fine for me...

It loads fine when I'm logged in. When in a private window, though, I get a "404 not found" error.

It loads fine when I'm logged in. When in a private window, though, I get a "404 not found" error.
Member

It loads fine when I'm logged in. When in a private window, though, I get a "404 not found" error.

Oh okay, I'm experiencing that same behavior.

> It loads fine when I'm logged in. When in a private window, though, I get a "404 not found" error. Oh okay, I'm experiencing that same behavior.
Author
Contributor

It's fixed now! I noticed the same behavior, I thought it was expected but it turns out I've just flipped a profile setting at some point.

It's fixed now! I noticed the same behavior, I thought it was expected but it turns out I've just flipped a profile setting at some point.
Paolo Amadini force-pushed nla-adjacent-strips-evaluate-later from 8db7f4f041 to 6eea97b236 2023-11-07 20:54:14 +01:00 Compare
Paolo Amadini requested review from Nate Rupsis 2023-11-07 20:56:06 +01:00
Author
Contributor

I rebased this on the latest main and I've re-requested review to be able to land the updated version.

I rebased this on the latest `main` and I've re-requested review to be able to land the updated version.
Nate Rupsis approved these changes 2023-11-10 20:43:09 +01:00
Nate Rupsis left a comment
Member

Make sure to prefix the PR title with "Anim:"

https://wiki.blender.org/wiki/Style_Guide/Commit_Messages

Make sure to prefix the PR title with "Anim:" https://wiki.blender.org/wiki/Style_Guide/Commit_Messages
Paolo Amadini changed title from Discuss edge cases and add regression tests for NLA strip evaluation to Anim: Discuss edge cases and add regression tests for NLA strip evaluation 2023-11-12 14:37:00 +01:00
Author
Contributor

Make sure to prefix the PR title with "Anim:"

Done, and the commit message already started with "Animation".

The documentation seems to suggest that the final merge should be done by the reviewer? I still don't seem to have permission, even though I now have an up-to-date review and I've rebased the code.

> Make sure to prefix the PR title with "Anim:" Done, and the commit message already started with "Animation". The [documentation](https://wiki.blender.org/wiki/Tools/Pull_Requests#Merge_a_Pull_Request) seems to suggest that the final merge should be done by the reviewer? I still don't seem to have permission, even though I now have an up-to-date review and I've rebased the code.
Member

Make sure to prefix the PR title with "Anim:"

Done, and the commit message already started with "Animation".

The documentation seems to suggest that the final merge should be done by the reviewer? I still don't seem to have permission, even though I now have an up-to-date review and I've rebased the code.

About 2 months back we decided on the "Anim" prefix. probably should document it somewhere 😬

I can merge this in if you want :)

> > Make sure to prefix the PR title with "Anim:" > > Done, and the commit message already started with "Animation". > > The [documentation](https://wiki.blender.org/wiki/Tools/Pull_Requests#Merge_a_Pull_Request) seems to suggest that the final merge should be done by the reviewer? I still don't seem to have permission, even though I now have an up-to-date review and I've rebased the code. About 2 months back we decided on the "Anim" prefix. _probably_ should document it somewhere 😬 I can merge this in if you want :)
Paolo Amadini force-pushed nla-adjacent-strips-evaluate-later from 6eea97b236 to e4b2a428a0 2023-11-19 18:21:50 +01:00 Compare
Author
Contributor

About 2 months back we decided on the "Anim" prefix

Fixed!

I can merge this in if you want :)

Yes, thanks, I don't seem to have the required permissions to perform the merge after the review.

> About 2 months back we decided on the "Anim" prefix Fixed! > I can merge this in if you want :) Yes, thanks, I don't seem to have the required permissions to perform the merge after the review.
Nate Rupsis merged commit da29519c7a into main 2023-11-20 17:52:37 +01:00
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#109212
No description provided.