-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Description
Description
There is a critical bug in DropFramesFrameScheduler.getTargetRenderTimeMs() method where the loop uses the wrong
variable (frameNumber instead of i), causing incorrect animation timing calculations when using jumpToFrame().
Affected Version
- Fresco version: all versions
- Module: animated-drawable
- File: com/facebook/fresco/animation/frame/DropFramesFrameScheduler.java
Problem Code Location
File: DropFramesFrameScheduler.java
Current buggy code (line ~75-80):
public long getTargetRenderTimeMs(int frameNumber) {
long targetRenderTimeMs = 0;
for (int i = 0; i < frameNumber; i++) {
targetRenderTimeMs += mAnimationInformation.getFrameDurationMs(frameNumber); // ❌ BUG: Should be 'i', not
'frameNumber'
}
return targetRenderTimeMs;
}
Root Cause
The method incorrectly uses frameNumber (the parameter) instead of the loop variable i when calling
getFrameDurationMs(). This causes it to repeatedly add the duration of the target frame instead of summing up
durations of all frames leading up to it.
Impact
This bug severely affects GIF animations with non-uniform frame durations, especially when using
AnimatedDrawable2.jumpToFrame():
- Incorrect time calculation: For a GIF with frames of different durations, jumping to a specific frame calculates
wrong animation time - Animation stops prematurely: The wrong timing causes loopCount to be calculated incorrectly, leading to early
animation termination - Loop count failure: Animations may stop even when configured for multiple or infinite loops
Steps to Reproduce
- Load a GIF with non-uniform frame durations (e.g., 18 frames at 30ms, 2 frames at 270ms)
- Call animatedDrawable2.jumpToFrame(19) to jump to the last frame
- Expected behavior: Animation should continue looping correctly
- Actual behavior: Animation stops after 4-5 loops instead of continuing
Example Calculation Error
GIF Structure:
- Frames 0-17: 30ms each (total: 540ms)
- Frame 18: 270ms
- Frame 19: 270ms
- Total loop duration: 1080ms
When calling jumpToFrame(19):
❌ Current buggy calculation:
targetRenderTimeMs = 0;
for (int i = 0; i < 19; i++) {
targetRenderTimeMs += getFrameDurationMs(19); // Always adds 270ms
}
Result: 270ms × 19 = 5130ms // WRONG!
✅ Correct calculation:
targetRenderTimeMs = 0;
for (int i = 0; i < 19; i++) {
targetRenderTimeMs += getFrameDurationMs(i); // Adds each frame's actual duration
}
Result: (30ms × 18) + 270ms = 810ms // CORRECT!
The wrong timing (5130ms) causes the animation system to believe it's already in loop 4 (5130 / 1080 ≈ 4), leading to
premature animation termination.
Proposed Fix
Change line ~78 from:
targetRenderTimeMs += mAnimationInformation.getFrameDurationMs(frameNumber);
To:
targetRenderTimeMs += mAnimationInformation.getFrameDurationMs(i);
Fixed Code
public long getTargetRenderTimeMs(int frameNumber) {
long targetRenderTimeMs = 0;
for (int i = 0; i < frameNumber; i++) {
targetRenderTimeMs += mAnimationInformation.getFrameDurationMs(i); // ✅ FIXED: Use 'i' instead of
'frameNumber'
}
return targetRenderTimeMs;
}