Skip to content

Commit

Permalink
Fix floating point errors calculating keyframe end progress (#2588)
Browse files Browse the repository at this point in the history
This fixed a really tricky bug that led to the wrong keyframe being used due to a floating point rounding error.
Given specific composition start frames and the existing floating point rounding, you could wind up with the following situation:

Keyframe 1 has an endFrame of 48 and an endProgress of 0.095051385
Keyframe 2 has a startFrame of 48 and a startProgress of 0.09505139
The Keyframe.containsProgress check intentionally leaves the upper end of the range open to make it unambiguous that the progress on the boundary of two keyframes should use the latter one.

However, due to this floating point error, there was a gap and if the progress == the endProgress of the first keyframe, it wouldn't match either.

I was able to reconstruct this specific scenario with a unit test and confirmed that this fixed it. However, it is not impossible that there are other scenarios in which this could happen. However, I would rather avoid allocating doubles for everything which is more expensive unless we find a specific repro again
  • Loading branch information
gpeal authored Dec 4, 2024
1 parent 255352b commit 5273ec6
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lottie/src/main/java/com/airbnb/lottie/value/Keyframe.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ public float getEndProgress() {
} else {
float startProgress = getStartProgress();
float durationFrames = endFrame - startFrame;
float durationProgress = durationFrames / composition.getDurationFrames();
endProgress = startProgress + durationProgress;
double durationProgress = durationFrames / (double) composition.getDurationFrames();
endProgress = (float) (startProgress + durationProgress);
}
}
return endProgress;
Expand Down
40 changes: 40 additions & 0 deletions lottie/src/test/java/com/airbnb/lottie/value/KeyframeTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.airbnb.lottie.value;

import static org.junit.Assert.*;

import android.graphics.Rect;
import android.view.animation.LinearInterpolator;
import androidx.collection.LongSparseArray;
import androidx.collection.SparseArrayCompat;
import com.airbnb.lottie.LottieComposition;
import org.junit.Test;

import java.util.ArrayList;
import java.util.HashMap;

public class KeyframeTest {

@Test
public void testStartFrame() {
LottieComposition composition = new LottieComposition();
composition.init(
new Rect(),
0f,
504.99f,
60f,
new ArrayList<>(),
new LongSparseArray<>(),
new HashMap<>(),
new HashMap<>(),
1f,
new SparseArrayCompat<>(),
new HashMap<>(),
new ArrayList<>(),
0,
0
);
Keyframe<Float> keyframe1 = new Keyframe<>(composition, 200f, 321f, new LinearInterpolator(), 28f, 48f);
Keyframe<Float> keyframe2 = new Keyframe<>(composition, 321f, 300f, new LinearInterpolator(), 48f, 56f);
assertEquals(keyframe2.getStartProgress(), keyframe1.getEndProgress(), 0f);
}
}

0 comments on commit 5273ec6

Please sign in to comment.