RFR: 8236753: Animations do not play backwards after being stopped
kcr at openjdk.java.net
Fri Jan 10 00:40:04 UTC 2020
On Fri, 10 Jan 2020 00:37:58 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> The private field `lastPlayFinished` is responsible for 2 cases where an animation in `STOPPED` status does not play after `play()` is called if the rate is negative:
>> 1. When the animation is created, it is `STOPPED` and `lastPlayFinished` is `false`. Setting a negative rate and calling `play()` will not jump to the end of the animation (in order to play it backwards) because the `if (lastPlayedFinished)` check is `false`. Creating the animation with `lastPlayFinished = true` fixes this. However, `SequentialTransitionPlayTest#testCycleReverse`'s initial state test implies that the original behavior is correct. *That test currently fails with this change.* Either the fix is reverted or the test is corrected.
>> 2. When the animation is stopped (if it was not `STOPPED` already), `jumpTo(Duration.ZERO)` sets `lastPlayFinished` to `false`, which causes the same issue above with `play()`. Setting `lastPlayFinished = true` at the end `stop()` fixes this issue.
>> A test was added for case 2 to check that the playing head indeed jumps to the end of the animation. Without this fix, it stays at the start.
>> I'm still somewhat confused as to what constitutes a "last play finished". Any `jumpTo` resets `lastPlayFinished` to `false`, even if the jump is to the start/end of the animation. In this case, stopping an animation, jumping to its start/end, setting the rate to negative/positive, and playing, will do nothing as the end condition is reached immediately. This is what the behavior that was fixed for cases 1 and 2, but maybe this is also incorrect behavior for jumping to start/end.
>> A test app is included in the "parent" [bug](https://bugs.openjdk.java.net/browse/JDK-8210238), which also mentions a bug relating to **pausing** and playing backwards, so be mindful of it when testing.
> I'll review this next week. This seems a fine candidate for openjfx14, so it (along with a couple other pending reviews that can be for 14) will be a good test of targeting a PR to the stabilization branch.
> I also request @arapte to review.
I should add that it will depend on whether there are any regressions. One thing we do need to be careful of is introducing regressions during rampdown.
More information about the openjfx-dev