Skip to content

Commit

Permalink
fix: 🐛 unexpected is_complete result for Bounce modes on load (#130)
Browse files Browse the repository at this point in the history 
* fix: 🐛 unexpected is_complete result for Bounce modes on load

* chore: 🤖 add another assertion for the reverse bounce mode test
  • Loading branch information
@theashraf
theashraf authored Apr 9, 2024
1 parent a2d65ca commit 217f27a
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 77 deletions.
4 changes: 3 additions & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,9 @@ test-all:
.PHONY: bench
bench:
$(info $(YELLOW)Running benchmarks for workspace$(NC))
cargo bench
cargo bench --manifest-path $(CORE)/Cargo.toml
cargo bench --manifest-path $(FMS)/Cargo.toml
cargo bench --manifest-path $(RUNTIME_FFI)/Cargo.toml

.PHONY: help
help:
Expand Down
48 changes: 26 additions & 22 deletions dotlottie-rs/src/dotlottie_player.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,11 @@ impl DotLottieRuntime {
Direction::Reverse => end_frame - raw_next_frame,
};

let next_frame =
if self.config.use_frame_interpolation {
next_frame
} else {
next_frame.round()
};
let next_frame = if self.config.use_frame_interpolation {
next_frame
} else {
next_frame.round()
};

// to ensure the next_frame won't go beyond the start & end frames
let next_frame = next_frame.clamp(start_frame, end_frame);
Expand Down Expand Up @@ -638,22 +637,21 @@ impl DotLottieRuntime {
let first_animation: Result<String, DotLottieError> =
self.dotlottie_manager.get_active_animation();

let ok =
match first_animation {
Ok(animation_data) => {
self.markers = extract_markers(animation_data.as_str());
let ok = match first_animation {
Ok(animation_data) => {
self.markers = extract_markers(animation_data.as_str());

// For the moment we're ignoring manifest values
// For the moment we're ignoring manifest values

// self.load_playback_settings();
self.load_animation_common(
|renderer, w, h| renderer.load_data(&animation_data, w, h, false),
width,
height,
)
}
Err(_error) => false,
};
// self.load_playback_settings();
self.load_animation_common(
|renderer, w, h| renderer.load_data(&animation_data, w, h, false),
width,
height,
)
}
Err(_error) => false,
};

if ok {
self.active_animation_id = self.dotlottie_manager.active_animation_id();
Expand Down Expand Up @@ -736,8 +734,14 @@ impl DotLottieRuntime {
return false;
}
match self.config.mode {
Mode::Forward | Mode::ReverseBounce => self.current_frame() >= self.end_frame(),
Mode::Reverse | Mode::Bounce => self.current_frame() <= self.start_frame(),
Mode::Forward => self.current_frame() >= self.end_frame(),
Mode::Reverse => self.current_frame() <= self.start_frame(),
Mode::Bounce => {
self.current_frame() <= self.start_frame() && self.direction == Direction::Reverse
}
Mode::ReverseBounce => {
self.current_frame() >= self.end_frame() && self.direction == Direction::Forward
}
}
}

Expand Down
125 changes: 71 additions & 54 deletions dotlottie-rs/tests/play_mode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,12 @@ mod play_mode_tests {

#[test]
fn test_forward_play_mode() {
let player =
DotLottiePlayer::new(Config {
mode: Mode::Forward,
autoplay: true,
use_frame_interpolation: false,
..Config::default()
});
let player = DotLottiePlayer::new(Config {
mode: Mode::Forward,
autoplay: true,
use_frame_interpolation: false,
..Config::default()
});

assert!(
player.load_animation_path("tests/assets/test.json", WIDTH, HEIGHT),
Expand Down Expand Up @@ -131,13 +130,12 @@ mod play_mode_tests {

#[test]
fn test_reverse_play_mode() {
let player =
DotLottiePlayer::new(Config {
mode: Mode::Reverse,
autoplay: true,
use_frame_interpolation: false,
..Config::default()
});
let player = DotLottiePlayer::new(Config {
mode: Mode::Reverse,
autoplay: true,
use_frame_interpolation: false,
..Config::default()
});

assert!(
player.load_animation_path("tests/assets/test.json", WIDTH, HEIGHT),
Expand Down Expand Up @@ -178,15 +176,13 @@ mod play_mode_tests {
}

#[test]
#[ignore = "fail cause of a bug in is_complete()"]
fn test_bounce_play_mode() {
let player =
DotLottiePlayer::new(Config {
mode: Mode::Bounce,
autoplay: true,
use_frame_interpolation: false,
..Config::default()
});
let player = DotLottiePlayer::new(Config {
mode: Mode::Bounce,
autoplay: true,
use_frame_interpolation: false,
..Config::default()
});

assert!(
player.load_animation_path("tests/assets/test.json", WIDTH, HEIGHT),
Expand All @@ -213,45 +209,55 @@ mod play_mode_tests {
"Expected rendered frames to be greater than or equal to total frames"
);

// check if the rendered frames are increasing and decreasing
let mut prev_frame = 0.0;
for frame in &rendered_frames {
let mut frame_idx = 0;

while frame_idx < rendered_frames.len()
&& rendered_frames[frame_idx] < player.total_frames()
{
assert!(
frame >= &prev_frame,
"Expected frame to be greater than or equal to previous frame"
rendered_frames[frame_idx] <= rendered_frames[frame_idx + 1],
"Expected frame to be less than or equal to next frame"
);
prev_frame = *frame;
frame_idx += 1;
}

for frame in rendered_frames.iter().rev() {
assert!(
rendered_frames[frame_idx] == player.total_frames(),
"Expected frame to be total frames at index {}",
frame_idx
);

while frame_idx < rendered_frames.len() && rendered_frames[frame_idx] > 0.0 {
assert!(
frame <= &prev_frame,
"Expected frame to be less than or equal to previous frame"
rendered_frames[frame_idx] >= rendered_frames[frame_idx + 1],
"Expected frame to be greater than or equal to next frame"
);
prev_frame = *frame;
frame_idx += 1;
}

// check if the last frame is 0
assert_eq!(0.0, prev_frame, "Expected last frame to be 0");
assert!(
rendered_frames[frame_idx] == 0.0,
"Expected frame to be 0 at index {}",
frame_idx
);
}

#[test]
#[ignore = "fail cause of a bug in is_complete()"]
fn test_reverse_bounce_play_mode() {
let player =
DotLottiePlayer::new(Config {
mode: Mode::ReverseBounce,
autoplay: true,
use_frame_interpolation: false,
..Config::default()
});
let player = DotLottiePlayer::new(Config {
mode: Mode::ReverseBounce,
autoplay: true,
use_frame_interpolation: false,
..Config::default()
});

assert!(
player.load_animation_path("tests/assets/test.json", WIDTH, HEIGHT),
"Animation should load"
);

assert!(player.is_playing(), "Animation should be playing");
assert!(!player.is_complete(), "Animation should not be complete");

let mut rendered_frames: Vec<f32> = vec![];

Expand All @@ -270,25 +276,36 @@ mod play_mode_tests {
"Expected rendered frames to be greater than or equal to total frames"
);

// check if the rendered frames are decreasing and increasing
let mut prev_frame = player.total_frames();
for frame in &rendered_frames {
let mut frame_idx = 0;

while frame_idx < rendered_frames.len() && rendered_frames[frame_idx] > 0.0 {
assert!(
frame <= &prev_frame,
"Expected frame to be less than or equal to previous frame"
rendered_frames[frame_idx] >= rendered_frames[frame_idx + 1],
"Expected frame to be greater than or equal to next frame"
);
prev_frame = *frame;
frame_idx += 1;
}

for frame in rendered_frames.iter().rev() {
assert!(
rendered_frames[frame_idx] == 0.0,
"Expected frame to be 0 at index {}",
frame_idx
);

while frame_idx < rendered_frames.len()
&& rendered_frames[frame_idx] < player.total_frames()
{
assert!(
frame >= &prev_frame,
"Expected frame to be greater than or equal to previous frame"
rendered_frames[frame_idx] <= rendered_frames[frame_idx + 1],
"Expected frame to be less than or equal to next frame"
);
prev_frame = *frame;
frame_idx += 1;
}

// check if the last frame is 0
assert_eq!(0.0, prev_frame, "Expected last frame to be 0");
assert!(
rendered_frames[frame_idx] == player.total_frames(),
"Expected frame to be total frames at index {}",
frame_idx
);
}
}

0 comments on commit 217f27a

Please sign in to comment.