Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Animation.FillMode when cue isn't 0% or 100% #13775

Merged
merged 5 commits into from
Dec 1, 2023

Conversation

MrJul
Copy link
Member

@MrJul MrJul commented Nov 29, 2023

What does the pull request do?

This PR fixes Animation.FillMode for any KeyFrame.Cue.
Additionally, it makes sure that the key frames insertion order doesn't matter.

What is the current behavior?

FillMode.Forward doesn't work if there's no frame with a 100% cue.
FillMode.Backward doesn't work if there's no frame with a 0% cue.
An animation can fail if the key frames order doesn't match the cues order.

What is the updated/expected behavior with this PR?

FillMode works with any cue.

How was the solution implemented (if it's not obvious)?

Currently, the Animator has a concept of neutral key frames, which are added at cues 0% and 100% if they don't exist. They're used to fall back to the neutral (non-animated) value outside of the animation's bounds. This isn't compatible with FillMode: the animation value should be used in this case instead of the neutral value.

This PR removes the neutral key frames. Instead, two properties FillBefore and FillAfter are added to the internal AnimatorKeyFrame, set on the appropriate frames when the animator is created. The animator can use these new properties to know if it should use the animated value or the neutral value.

There were already tests for FillMode, with problems:

  • Check_FillMode_Start_Value worked partly because an Animation.Delay was set, and because the animated value was 0, matching the neutral's frame defaults. After removing it, several test cases were failing. Test cases have been added with and without delay, and with a non-zero value.
  • Check_FillMode_End_Value didn't use the test case parameters at all. Using them resulted in failures.

Fixed issues

Copy link
Member

@maxkatz6 maxkatz6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@maxkatz6 maxkatz6 added this pull request to the merge queue Dec 1, 2023
@maxkatz6 maxkatz6 added the backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch label Dec 1, 2023
Merged via the queue into AvaloniaUI:master with commit fa7b470 Dec 1, 2023
6 checks passed
maxkatz6 pushed a commit that referenced this pull request Dec 5, 2023
* Added failing Animation.FillMode tests

* Fix Animation.FillMode when cue isn't 0% or 100%

* Ensure keyframes are ordered in animator

* Fix Animation.Clock nullability

* Fix some inverted Assert.Equal parameters
@maxkatz6 maxkatz6 added backported-11.0.x and removed backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch labels Dec 5, 2023
@MrJul MrJul deleted the fix/keyframe-fillmode branch February 29, 2024 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KeyFrame animation whose last animated value has Cue < 1.0 doesn't hold value
2 participants