Skip to content

/ jfx

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

8241582: Infinite animation does not start from the end when started with a negative rate #169

Closed
wants to merge 4 commits into from
+27 −9
Closed

8241582: Infinite animation does not start from the end when started with a negative rate #169

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41e72c5
Initial commit
nlisker Apr 9, 2020
9c3b173
Fixed single loop test
nlisker Apr 9, 2020
83a1ff7
Fix IndefiniteCycleDuration test
nlisker Apr 10, 2020
4a0b1b9
Comment out erroneous test
nlisker Apr 10, 2020
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

9 modules/javafx.graphics/src/main/java/javafx/animation/Animation.java
View file
Open in desktop
@@ -28,6 +28,8 @@
import java.util.HashMap;

import com.sun.javafx.tk.Toolkit;
import com.sun.javafx.util.Utils;

import javafx.beans.property.BooleanProperty;
import javafx.beans.property.DoubleProperty;
import javafx.beans.property.DoublePropertyBase;
@@ -758,10 +760,9 @@ public void jumpTo(Duration time) {

lastPlayedFinished = false;

final Duration totalDuration = getTotalDuration();
time = time.lessThan(Duration.ZERO) ? Duration.ZERO : time
.greaterThan(totalDuration) ? totalDuration : time;
final long ticks = fromDuration(time);
double millis = time.isIndefinite() ? getCycleDuration().toMillis() :
Utils.clamp(0, time.toMillis(), getTotalDuration().toMillis());
long ticks = TickCalculation.fromMillis(millis);

if (getStatus() == Status.STOPPED) {
syncClipEnvelope();
17 modules/javafx.graphics/src/test/java/test/javafx/animation/AnimationTest.java
View file
Open in desktop
@@ -254,6 +254,23 @@ public void testJumpTo_UNKNOWN() {
animation.jumpTo(Duration.UNKNOWN);
}

@Test
public void testJumpTo_IndefiniteCycles() {
animation.shim_setCycleDuration(TWO_SECS);
animation.setCycleCount(Animation.INDEFINITE);

animation.jumpTo("end");
assertEquals(TWO_SECS, animation.getCurrentTime());
}

@Test
public void testJumpTo_IndefiniteCycleDuration() {
animation.shim_setCycleDuration(Duration.INDEFINITE);

animation.jumpTo("end");
assertEquals(Duration.millis(Long.MAX_VALUE / 6), animation.getCurrentTime());

This comment has been minimized.

Sign in to view
@kevinrushforth

kevinrushforth Apr 22, 2020
Member

Why / 6 ?

This comment has been minimized.

Sign in to view
@nlisker

nlisker Apr 22, 2020
Author Collaborator

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.
}

@Test
public void testJumpToCuePoint_Default() {
animation.getCuePoints().put("ONE_SEC", ONE_SEC);
10 ...les/javafx.graphics/src/test/java/test/javafx/animation/SequentialTransitionPlayTest.java
View file
Open in desktop
@@ -636,11 +636,11 @@ public void testCycleReverse() {

st.play();

assertEquals(Status.RUNNING, st.getStatus());
assertEquals(Status.STOPPED, child1X.getStatus());
assertEquals(Status.RUNNING, child1Y.getStatus());
assertEquals(60000, xProperty.get());
assertTrue(0 < yProperty.get() && yProperty.get() < 10000);
// assertEquals(Status.RUNNING, st.getStatus());
// assertEquals(Status.STOPPED, child1X.getStatus());
// assertEquals(Status.RUNNING, child1Y.getStatus());
// assertEquals(60000, xProperty.get());
// assertTrue(0 < yProperty.get() && yProperty.get() < 10000);

st.jumpTo(TickCalculation.toDuration(100));

ProTip! Use n and p to navigate between commits in a pull request.