jfx icon indicating copy to clipboard operation
jfx copied to clipboard

[WIP] Animation redesign

Open nlisker opened this issue 5 years ago • 3 comments

This PR is not meant to be integrated, but serve as a discussion for a few behavior questions about animations.

The following issues are solved with these changes:

You may verify these.

Summary of changes

  • currentRate's behavior has been redefined for Animation and ClipEnvelope. The internal currentRate has been removed from the ClipEnvelope. Now Animation only sets the current rate when the ClipEnvelope isn't running (the receiver is paused), which are the cases when the animation is paused, stopped or rate=0. Otherwise, ClipEnvelope provides a way to calculate this value based on its internal data and it updates the animation when the current rate changes due to alternating cycles.
  • The calculation of ticks in ClipEnvelope has been unified and a lot of functionality has been pulled up the hierarchy.
  • The lastPlayedForward value that Animation held has been replaced by startedPositive in MultiLoopClipEnvelope since it's an internal detail (though see below).
  • Added an internal cycle number property to ClipEnvelope. Currently it is only used to determine if a cycle is even or odd for the purpose of calculating the current rate, In the future it will report the cycle number (JDK-8091172).
  • Internal documentation has been added.
  • Parent animations haven't been dealt with thoroughly.

Discussion

  1. The starting direction of the animation is crucial to its behavior. As explained in the field's doc, it resolves an ambiguity: given an animation with 2 cycles and autoReverse=true, if the play head is in the initial cycle (say ticks=1), should the current rate be with or against the rate? If the initial rate was >0 the current rate is the same as rate, if <0, then it's the opposite of rate. Another point is that a negative rate's play direction is not well defined without a starting direction: if the animation starts with a positive rate it will play forwards and then backwards, then setting a negative rate will play it again forwards and then backwards (with respect to the cycle direction); but if the animation starts with a negative rate it will play backwards and then forwards. So, the direction of a negative rate depends on the starting rate.
  2. Related to 1, should jumpTo jump according to the starting direction (JDK-8237973)? If no, what should jumping to ticks=1 with 2 cycles and autoReverse=true do? The play head position at ticks=1 with respect to the positive direction is in the middle of the animation if it was started with a negative rate. This can make the animation continue its play incorrectly (JDK-8241237) in addition to possibly being confusing.
  3. How to deal with starting an animation with rate=0. A possibility is to count it as a positive rate.
  4. From points 1 and 2, if the starting direction is deemed a fundamental property of an animation, should it be exposed since the user behavior is based on it?
  5. Thinking ahead, how should the cycle number be calculated for infinite animations? For non-infinite animations, the number can be calculated based on the position of the play head out of the total playing time, but for infinite animations there is no total playing time. One option would be to only count consecutive cycles and resetting them after a jump. Another would be to extrapolate based on the cycleTicks.

Progress

  • [ ] Change must not contain extraneous whitespace
  • [ ] Commit message must refer to an issue
  • [ ] Change must be properly reviewed

Download

$ git fetch https://git.openjdk.java.net/jfx pull/262/head:pull/262 $ git checkout pull/262

nlisker avatar Jul 09 '20 19:07 nlisker

:wave: Welcome back nlisker! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request.

bridgekeeper[bot] avatar Jul 09 '20 19:07 bridgekeeper[bot]

@kevinrushforth @arapte Please take a look at this. No need for a formal code review.

nlisker avatar Jul 10 '20 17:07 nlisker

Sure, we'll take a look at it in the not-too-distant future.

kevinrushforth avatar Jul 21 '20 20:07 kevinrushforth

@nlisker this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout Animation_redesign
git fetch https://git.openjdk.org/jfx master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

openjdk[bot] avatar Nov 03 '22 11:11 openjdk[bot]

@nlisker This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Mar 31 '23 23:03 bridgekeeper[bot]

Still alive, bridgekeeperbot

nlisker avatar Apr 27 '23 09:04 nlisker

@nlisker This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Jun 22 '23 13:06 bridgekeeper[bot]

Still alive, bridgekeeperbot

nlisker avatar Jun 22 '23 13:06 nlisker

@nlisker This pull request has been inactive for more than 8 weeks and will be automatically closed if another 8 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Aug 17 '23 16:08 bridgekeeper[bot]

@nlisker This pull request has been inactive for more than 16 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

bridgekeeper[bot] avatar Oct 12 '23 18:10 bridgekeeper[bot]