RFR: 8241582: Infinite animation does not start from the end when started with a negative rate
nlisker at openjdk.java.net
Wed Apr 22 12:58:53 UTC 2020
On Wed, 22 Apr 2020 00:08:05 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> ### Cause
>> `Animation#jumpTo(Duration)` does not handle `Duration.INDEFINITE` properly. This causes
>> `InfiniteClipEnvelope#jumpTo(long)` to receive an erroneous value and start the animation not from the end, depending
>> on the modulo calculation. ### Fix
>> For infinite cycles, use the `cycleDuration` as the destination since that is the effective end point.
>> ### Tests
>> * Added an `testJumpTo_IndefiniteCycles` test that fails without the patch and passes after it.
>> * Added a `testJumpTo_IndefiniteCycleDuration` that passes both before and after, just to make sure that this type of
>> infinite duration is correct.
>> * Removed a test in `SequentialTransition` that fails with this patch, but it passed before it only because the modulo
>> value happened to land in the right place. Changing the duration of one of the child animation can cause it to fail, so
>> the test is faulty anyway at this stage.
>> ### Future work
>> Playing backwards will still not work correctly, but at least now it start from the correct value. This is the first of
>> a series of fixes under the umbrella issue [JDK-8210238](https://bugs.openjdk.java.net/browse/JDK-8210238).
> modules/javafx.graphics/src/test/java/test/javafx/animation/AnimationTest.java line 271:
>> 270: animation.jumpTo("end");
>> 271: assertEquals(Duration.millis(Long.MAX_VALUE / 6), animation.getCurrentTime());
>> 272: }
> Why `/ 6` ?
This is a good question and I think I should have explained it in a comment because it also points to a mini-flaw. The
short answer is that the 6 comes from the `TicksCalculation` class, which defines `TICKS_PER_SECOND = 6000` and
`TICKS_PER_MILI = TICKS_PER_SECOND / 1000.0` which is 6.
The test works as follows:
This is the "ticks from duration" calculation for jumping to the end (`Duration.INDEFINITE`), demonstrated by
mathematical substitutions: double millis = Duration.INDEFINITE.toMillis() = Double.POSITIVE_INFINITY
long ticks = TickCalculation.fromMillis(millis) = Math.round(TICKS_PER_MILI * millis)
= Math.round(6 * Double.POSITIVE_INFINITY) = Math.round(Double.POSITIVE_INFINITY)
= Long.MAX_VALUE (as per the spec. of Math.round)
Notice that we lose precision when multiplying `POSITIVE_INFINITY`.
Then getting the current time calculates "duration from ticks":
animation.getCurrentTime() = TickCalculation.toDuration(currentTicks)
= ticks / TICKS_PER_MILI = Long.MAX_VALUE / 6
So the division carries out normally (there's no `Long.POSITIVE_INFINITY`), but the multiplication does not.
Maybe we shouldn't convert between the `double`-based `Duration` and the `long` ticks so much, but I think that this is
My next patch is refactoring in preparation of the next, heavier, changes, so I will add this explanation on the test.
More information about the openjfx-dev