On Fri, 10 Jan 2020 00:37:58 GMT, Kevin Rushforth <k...@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. ------------- PR: https://git.openjdk.java.net/jfx/pull/82
