On Wed, 22 Apr 2020 00:08:05 GMT, Kevin Rushforth <k...@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 somewhat negligible. My next patch is refactoring in preparation of the next, heavier, changes, so I will add this explanation on the test. ------------- PR: https://git.openjdk.java.net/jfx/pull/169
