diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 8c0280d99fa..418e401bed5 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -27591,6 +27591,7 @@ impl View for Workspace { .cover() .with_opacity(opacity_ratio) .with_corner_radius(window_corner_radius) + .enable_animation_with_start_time(std::time::Instant::now()) .finish(), ) .finish(), diff --git a/crates/warpui_core/src/elements/gui/image.rs b/crates/warpui_core/src/elements/gui/image.rs index 2d9a76d6f28..2f93dd1a48d 100644 --- a/crates/warpui_core/src/elements/gui/image.rs +++ b/crates/warpui_core/src/elements/gui/image.rs @@ -171,6 +171,14 @@ impl Image { self } + /// Computes elapsed time in milliseconds since animation started. + /// If `started_at` was not set, uses the provided `now` time as the reference point + /// (meaning elapsed time would be 0, showing only the first frame). + fn compute_elapsed_time_ms(&self, now: Instant) -> u128 { + let started_at = self.started_at.unwrap_or_else(|| now); + now.duration_since(started_at).as_millis() + } + pub fn before_load(mut self, element: Box) -> Self { self.before_load_element = Some(element); self @@ -329,8 +337,7 @@ impl Image { ) { // If self.started_at is not provided, we set it to current time // so only the first frame is shown. - let started_at = self.started_at.unwrap_or_else(Instant::now); - let elapsed_time = started_at.elapsed().as_millis(); + let elapsed_time = self.compute_elapsed_time_ms(Instant::now()); // After about ~50 days, casting `elapsed_time` to a u32 will // silently overflow. The gif may jump and start playing from a // different frame. diff --git a/crates/warpui_core/src/elements/gui/image_tests.rs b/crates/warpui_core/src/elements/gui/image_tests.rs index a4411bb0c57..cf84ec355be 100644 --- a/crates/warpui_core/src/elements/gui/image_tests.rs +++ b/crates/warpui_core/src/elements/gui/image_tests.rs @@ -120,3 +120,137 @@ fn loading_timeout_survives_image_rebuild_for_same_source() { assert_eq!(timed_out_kind, Some(BackupElementKind::LoadTimeout)); assert_eq!(timed_out_repaint_after, None); } + +#[test] +fn enable_animation_with_start_time_sets_started_at() { + let mut image = test_image(); + assert_eq!(image.started_at, None, "started_at should initially be None"); + + let now = Instant::now(); + image = image.enable_animation_with_start_time(now); + assert_eq!( + image.started_at, Some(now), + "started_at should be set after calling enable_animation_with_start_time" + ); +} + +#[test] +fn enable_animation_with_start_time_can_be_chained() { + let start1 = Instant::now(); + let image = test_image().enable_animation_with_start_time(start1); + assert_eq!(image.started_at, Some(start1)); + + // Can chain multiple calls (though typically would only call once) + let start2 = start1 + Duration::from_millis(10); + let image = image.enable_animation_with_start_time(start2); + assert_eq!( + image.started_at, Some(start2), + "Later call to enable_animation_with_start_time should override" + ); +} + +#[test] +fn animation_with_no_started_at_uses_instant_now() { + // This test verifies the bug fix: when started_at is None, + // paint_animated_image falls back to Instant::now(), which means + // elapsed_time is always very small and the animation gets stuck. + // The fix ensures that animation callers set started_at via enable_animation_with_start_time. + + let image = test_image(); + + // When started_at is None, the image widget is configured to NOT animate + // (see paint_animated_image: it checks if self.started_at.is_some() before requesting repaint) + assert_eq!( + image.started_at, None, + "started_at should be None for non-animated setup" + ); +} + +#[test] +fn animation_started_at_must_be_set_before_paint_for_animation() { + // This test verifies that animation only works when started_at is explicitly set. + // The regression test ensures the fix in view.rs + // (adding .enable_animation_with_start_time(Instant::now())) is necessary. + + let image_no_start = test_image(); + let image_with_start = test_image().enable_animation_with_start_time(Instant::now()); + + // Verify the difference in started_at state + assert_eq!(image_no_start.started_at, None); + assert_ne!(image_with_start.started_at, None); +} + +#[test] +fn compute_elapsed_time_ms_uses_started_at_when_set() { + // This is a regression test for the bug where paint_animated_image would + // always compute a fresh Instant::now() and ignore self.started_at, + // causing elapsed_time to be ~0 and animations to freeze on first frame. + // + // The fix adds compute_elapsed_time_ms which respects self.started_at. + // If someone removes the self.started_at check, this test will fail. + + let past_time = Instant::now() - Duration::from_millis(1000); + let image = test_image().enable_animation_with_start_time(past_time); + + // Simulate a paint call at a specific time in the future + let now = past_time + Duration::from_millis(500); + let elapsed = image.compute_elapsed_time_ms(now); + + // Should compute elapsed time as the difference between now and past_time + assert_eq!(elapsed, 500, "elapsed time should be 500ms from past_time to now"); + assert!( + elapsed > 0, + "elapsed time must be > 0 when started_at is set in the past" + ); +} + +#[test] +fn compute_elapsed_time_ms_uses_provided_time_when_started_at_is_none() { + // When started_at is None (animation not enabled), the provided 'now' time + // is used as the reference, giving elapsed time of 0 (showing only first frame). + + let image = test_image(); // No enable_animation_with_start_time call + assert_eq!( + image.started_at, None, + "started_at should be None for non-animated setup" + ); + + let now = Instant::now(); + let elapsed = image.compute_elapsed_time_ms(now); + + // When started_at is None, elapsed time should be 0 (first frame only) + assert_eq!(elapsed, 0, "elapsed time should be 0 when started_at is None"); +} + +#[test] +fn compute_elapsed_time_ms_respects_large_time_differences() { + // Verify that compute_elapsed_time_ms correctly handles larger time spans. + // This tests that the method actually uses started_at for computation, + // not recalculating time fresh each time (which would always be ~0). + + let base_time = Instant::now(); + let started_at = base_time - Duration::from_secs(5); + let image = test_image().enable_animation_with_start_time(started_at); + + // Compute elapsed time at a point 3 seconds after started_at + let now = started_at + Duration::from_secs(3); + let elapsed = image.compute_elapsed_time_ms(now); + + assert_eq!( + elapsed, 3000, + "elapsed time should be 3000ms (3 seconds) from started_at" + ); + + // Compute elapsed time at a point 5+ seconds after started_at (past the original start+5s) + let now_later = started_at + Duration::from_secs(7); + let elapsed_later = image.compute_elapsed_time_ms(now_later); + + assert_eq!( + elapsed_later, 7000, + "elapsed time should be 7000ms (7 seconds) from started_at" + ); + assert!( + elapsed_later > elapsed, + "elapsed time should increase monotonically" + ); +} diff --git a/crates/warpui_core/src/image_cache_tests.rs b/crates/warpui_core/src/image_cache_tests.rs index 05e6ecd0677..5f7456d1f44 100644 --- a/crates/warpui_core/src/image_cache_tests.rs +++ b/crates/warpui_core/src/image_cache_tests.rs @@ -712,3 +712,256 @@ fn test_respects_max_dimensions_for_cacheoption_bysize() { // Assert that, when we specify a max dimension of 512, the image is resized accordingly. assert_eq!(image.img.dimensions(), (512, 512)); } + +#[test] +fn animated_image_get_current_frame_advances_with_elapsed_time() { + let asset_cache = new_asset_cache(); + let image_cache = ImageCache::new(); + + // Load an animated GIF with FullAnimation behavior to get an AnimatedImage + let image = load_bundled_image( + &image_cache, + &asset_cache, + "numbers-1000ms.gif", + Vector2I::new(16, 16), + FitType::Contain, + AnimatedImageBehavior::FullAnimation, + ); + + let Image::Animated(animated) = image.as_ref() else { + panic!("Expected animated image but got static image!"); + }; + + // The numbers-1000ms.gif has multiple frames with various delays + assert!(animated.frames.len() >= 2, "Expected at least 2 frames for testing"); + assert!(animated.duration > 0, "Expected positive total duration"); + + // Get frames at different elapsed times + let (_frame_0, remaining_0) = animated + .get_current_frame(0) + .expect("Should get frame at elapsed 0ms"); + let (_frame_100, remaining_100) = animated + .get_current_frame(100) + .expect("Should get frame at elapsed 100ms"); + let (_frame_500, remaining_500) = animated + .get_current_frame(500) + .expect("Should get frame at elapsed 500ms"); + + // Frames at different times should be different (for a proper animated GIF) + // or at least the remaining delays should be different + assert_ne!( + remaining_0, remaining_100, + "Remaining delay should differ at different elapsed times" + ); + assert_ne!( + remaining_100, remaining_500, + "Remaining delay should differ at different elapsed times" + ); +} + +#[test] +fn animated_image_get_current_frame_wraps_at_duration() { + let asset_cache = new_asset_cache(); + let image_cache = ImageCache::new(); + + let image = load_bundled_image( + &image_cache, + &asset_cache, + "numbers-1000ms.gif", + Vector2I::new(16, 16), + FitType::Contain, + AnimatedImageBehavior::FullAnimation, + ); + + let Image::Animated(animated) = image.as_ref() else { + panic!("Expected animated image but got static image!"); + }; + + let duration = animated.duration; + + // Get frames at start, middle, and end of cycle + let (frame_start, _) = animated + .get_current_frame(0) + .expect("Should get frame at start"); + let (frame_end, _) = animated + .get_current_frame(duration - 1) + .expect("Should get frame near end"); + + // After wrapping (elapsed >= duration), should return to start of animation + let (frame_wrapped_start, _) = animated + .get_current_frame(duration) + .expect("Should get frame after one complete cycle"); + let (frame_wrapped_end, _) = animated + .get_current_frame(duration * 2 - 1) + .expect("Should get frame in second cycle"); + + // The frame at the wrapped start should be the same as the original start + // (both should be the first frame) + assert_eq!( + Arc::ptr_eq(&frame_start, &frame_wrapped_start), + true, + "Frame at start should equal frame at wrapped start" + ); + + // The frame near end of first cycle should equal frame near end of second cycle + assert_eq!( + Arc::ptr_eq(&frame_end, &frame_wrapped_end), + true, + "Frame at cycle end should equal frame at next cycle end" + ); +} + +#[test] +fn static_image_not_affected_by_animation_behavior_change() { + let asset_cache = new_asset_cache(); + let image_cache = ImageCache::new(); + + // Load a static PNG with FullAnimation behavior (should remain static) + let static_full_animation = load_bundled_image( + &image_cache, + &asset_cache, + "local.png", + Vector2I::new(512, 512), + FitType::Cover, + AnimatedImageBehavior::FullAnimation, + ); + + // Load the same static PNG with FirstFramePreview behavior (should remain static) + let static_preview = load_bundled_image( + &image_cache, + &asset_cache, + "local.png", + Vector2I::new(512, 512), + FitType::Cover, + AnimatedImageBehavior::FirstFramePreview, + ); + + // Both should be static images + assert!(matches!(static_full_animation.as_ref(), Image::Static(_))); + assert!(matches!(static_preview.as_ref(), Image::Static(_))); + + // Extract and verify they're the same image data + let Image::Static(full_static) = static_full_animation.as_ref() else { + unreachable!(); + }; + let Image::Static(preview_static) = static_preview.as_ref() else { + unreachable!(); + }; + + // Both should have the same dimensions + assert_eq!( + full_static.img.dimensions(), + preview_static.img.dimensions() + ); +} + +#[test] +fn animated_gif_shows_first_frame_preview_when_requested() { + let asset_cache = new_asset_cache(); + let image_cache = ImageCache::new(); + + // Request FirstFramePreview behavior for an animated GIF + let preview = load_bundled_image( + &image_cache, + &asset_cache, + "numbers-1000ms.gif", + Vector2I::new(16, 16), + FitType::Contain, + AnimatedImageBehavior::FirstFramePreview, + ); + + // Should get a static image (the first frame) + let Image::Static(_) = preview.as_ref() else { + panic!("Expected static image for FirstFramePreview"); + }; + + // But the underlying asset in asset_cache should still be the full AnimatedBitmap + let asset: AssetState = asset_cache.load_asset(AssetSource::Bundled { + path: "numbers-1000ms.gif", + }); + let AssetState::Loaded { data } = asset else { + panic!("Bundled asset should be available immediately!"); + }; + assert!(matches!(data.as_ref(), ImageType::AnimatedBitmap { .. })); +} + +#[test] +fn animated_image_remaining_delay_decreases_within_frame() { + let asset_cache = new_asset_cache(); + let image_cache = ImageCache::new(); + + let image = load_bundled_image( + &image_cache, + &asset_cache, + "numbers-1000ms.gif", + Vector2I::new(16, 16), + FitType::Contain, + AnimatedImageBehavior::FullAnimation, + ); + + let Image::Animated(animated) = image.as_ref() else { + panic!("Expected animated image"); + }; + + // Query the remaining delay at the start of a frame and later in the same frame + // (e.g., at 0ms and 50ms, if the first frame is 100ms) + let (_, remaining_at_start) = animated + .get_current_frame(0) + .expect("Should get frame at 0ms"); + let (_, remaining_at_50) = animated + .get_current_frame(50) + .expect("Should get frame at 50ms"); + + // Within the same frame, remaining delay should decrease as time advances + assert!( + remaining_at_50 < remaining_at_start, + "Remaining delay should decrease as we progress through a frame" + ); +} + +#[test] +fn animated_webp_also_advances_frames() { + let asset_cache = new_asset_cache(); + let image_cache = ImageCache::new(); + + let image = load_bundled_image( + &image_cache, + &asset_cache, + "animated.webp", + Vector2I::new(16, 16), + FitType::Contain, + AnimatedImageBehavior::FullAnimation, + ); + + let Image::Animated(animated) = image.as_ref() else { + panic!("Expected animated image"); + }; + + assert!(animated.frames.len() > 1, "WebP should have multiple frames"); + + // Verify we can retrieve frames at different times throughout the animation + // Get frame at start and at the end to ensure we've traversed the full timeline + let (frame_0, _) = animated + .get_current_frame(0) + .expect("Should get frame at 0ms"); + let (frame_near_end, _) = animated + .get_current_frame(animated.duration - 1) + .expect("Should get frame near end"); + + // At start and near end, we should get different frames + // (unless the WebP only has one frame, which shouldn't happen for animated images) + assert!( + !Arc::ptr_eq(&frame_0, &frame_near_end) || animated.frames.len() == 1, + "WebP animation should traverse different frames or only have one frame" + ); + + // Verify wrapping: after one full duration, we're back at the start + let (frame_wrapped, _) = animated + .get_current_frame(animated.duration) + .expect("Should get frame after one cycle"); + assert_eq!( + Arc::ptr_eq(&frame_0, &frame_wrapped), + true, + "Frame should wrap back to start after duration" + ); +}