RFR: 8236753: Animations do not play backwards after being stopped
nlisker at openjdk.java.net
Sat Jan 18 02:14:45 UTC 2020
On Fri, 17 Jan 2020 22:48:11 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>>> So this PR may need a document change for `Animation.play()`
>> Yes, and the docs need clarification in other places anyway. The [parent issue](https://bugs.openjdk.java.net/browse/JDK-8210238) from which this bug was isolated talks about these. Not sure at what point I will go over it since `Animation` is going to get some more changes in the (hopefully) near future.
> Based on my testing, and thinking through all of the implications of the fix, I think the proposed fix is correct. I modified the Timeline.java test program attached to [JDK-8210238](https://bugs.openjdk.java.net/browse/JDK-8210238) to add controls for auto-reverse and cycle count (1, 2, or INDEFINITE), and attached it as TimelineTest2.java. I tried several combinations, and the change in behavior looks correct in all cases to me.
> As you point out in the description, the failing unit test, [SequentialTransitionPlayTest.testCycleReverse](https://github.com/openjdk/jfx/blob/16cea4111b25a00f812b2f6ba8a54adbcffc1c86/modules/javafx.graphics/src/test/java/test/javafx/animation/SequentialTransitionPlayTest.java#L632) is coded to test for the existing behavior when you first call play with `rate = -1`. I think it likely that this is merely because that's how it worked rather than because there was deliberate thought given to whether it was the right behavior. So I think the right thing to do is fix the test.
> You also mentioned that jumping, even to the start or end, sets `lastPlayFinished` to `false`. I believe this is correct behavior and should not be changed.
> As part of my testing, I ran into what may be a bug, but since the behavior is the same with or without your patch, I think it would be best to file a new bug and deal with it separately (if, in fact, it is a bug).
> Here are the steps I used:
> 1. Run TimelineTest2
> 2. Select cycle=2; Select AutoReverse
> 3. Play animation: it correctly goes forward from start to end and then backwards from end to start
> 3. Set rate=-1
> 5. Play animation: BUG? It still goes forward from start to end and then backwards from end to start
> Subsequent calls to play do what I would expect, in that it goes backwards from end to start and then forward from start to end.
> The same thing happens when switching back to rate=1 : the first time it continues the direction it was prior to changing the rate.
> So I think the right thing to do is fix the test.
Since the animation plays from the end for an arbitrary duration (at least I think it's arbitrary, compared to a pulse), I changed the assert to check that the property value is in the playing range of the animation and not equals to some specific value.
More information about the openjfx-dev