Paths

Table of Contentst

Differential D10401

Fix T85564: FCurve modifier zero influence on restrict range borders
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Feb 12 2021, 11:28 AM.

Details

Reviewers

Sybren A. Stüvel (sybren)

Group Reviewers

Animation & Rigging

Maniphest Tasks

T85564: graph editor generator generates 0 at the conjuction point

Commits

rB9dee0a10c81d: Fix T85564: FCurve modifier zero influence on restrict range borders

Summary

When using FModifier Restrict Frame Range, the resulting influence was zero being exactly on Start / End range borders (so borders were exclusive).
This made it impossible to chain FModifers together (forcing the user to specify values slightly below the desired border in following FModifiers).
This is now corrected to be inclusive on Start / End range borders.

Before

After

Testfile

releaselogs.blend786 KBDownload

In the case of touching open borders (so [frame A frame B] followed by [frame B frame C]) both modifiers are evaluated (in stack order).
If the later modifier has full influence (and is not additive) this simply means the result is the same as the later modifier's value.
If influences below 1 are used (or modifiers are additive) both modifier's values are interpolated/added accordingly.

technical notes:

this was caused by the introduction of FModifier Influence/BlendIn-Out in rB185663b52b61.
for comparison, see other occurrences of FMODIFIER_FLAG_RANGERESTRICT.
the following calculations in eval_fmodifier_influence for blend in/out can remain unchanged since being exactly at the border should return full influence.

Diff Detail

Repository

rB Blender

Branch

T85564 (branched from master)

Build Status

Buildable 16955
Build 16955: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Feb 12 2021, 11:28 AM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B12832: Diff 33896.Feb 12 2021, 11:28 AM

Philipp Oeser (lichtwerk) added a reviewer: Animation & Rigging.Feb 12 2021, 11:28 AM

Philipp Oeser (lichtwerk) added a project: Animation & Rigging.

Sybren A. Stüvel (sybren) accepted this revision.Mar 8 2021, 3:38 PM

This revision is now accepted and ready to land.Mar 8 2021, 3:38 PM

I accepted to soon.

This clearly is a mistake in the code, as it's indeed inconsistent with the other uses of the frame range. I'm just wondering how many files there are now that have set their modifier frame range slightly differently than they'd actually want, basically to counteract this bug. If we now fix it, that means potentially breaking the animation in those files.

This revision now requires changes to proceed.Mar 8 2021, 3:41 PM

In D10401#269348, @Sybren A. Stüvel (sybren) wrote:

I accepted to soon.

This clearly is a mistake in the code, as it's indeed inconsistent with the other uses of the frame range. I'm just wondering how many files there are now that have set their modifier frame range slightly differently than they'd actually want, basically to counteract this bug. If we now fix it, that means potentially breaking the animation in those files.

I propose to warn about this in the release logs.
Saying this because I would not see an alternative action here, do_version could not predict how the user wanted the modifier to behave.
If we want to protect old files and keep the bug, we should close T85564 as a Known Issue instead.
But I would not know what changes are requested here?

2.93 is going to be another LTS release, and introducing such backward-incompatible changes could make it unusable for studios. I think Blender 3.0 may be a better release to put this fix into.

Philipp Oeser (lichtwerk) mentioned this in T85564: graph editor generator generates 0 at the conjuction point.Mar 9 2021, 8:48 AM

Agree to disagree :)

But moving happily forward, we dont really have a status that reflects what we are in now, do we?
Needs Revision is clearly wrong, because once we hit 3.0 (or whatever version this ends up in) it will be in this form, right?
Would propose to already create a BF Blender (3.0) tag, tag this Revision with that, but change status to Accepted?

@Sybren A. Stüvel (sybren): tagged with BF Blender (3.0), would you agree to accept then?

Just to share my 2 cents:

Leave major backward compatibility breakages to major releases +1
Take LTS into consideration -1

If such a change could potentially make things unstable for studios, that's a good thing it is in a LTS. Not the other way around. Meaning there is more time to address and port back eventual issues.

That said is the animation module call and I think is reasonable for this to be postponed. But mostly because of 3.0, not so much because of the LTS.

Now that master is at 3.0, we can merge this. @Philipp Oeser (lichtwerk) can you update the description so that it has a text that can go into the release notes, and some images that clearly illustrate the change in behavour?

In D10401#281058, @Sybren A. Stüvel (sybren) wrote:

Now that master is at 3.0, we can merge this. @Philipp Oeser (lichtwerk) can you update the description so that it has a text that can go into the release notes, and some images that clearly illustrate the change in behavour?

Updated the description now, is this sufficient?

Philipp Oeser (lichtwerk) requested review of this revision.Jul 26 2021, 10:19 PM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Aug 12 2021, 9:03 AM

Currently the end frame is not computed correctly when influence blending is used: