Fixed typing animation won't stop, Fix #259

[alanpoon]

Fixes #259

1. Added back stop_animation
2. Changed timer to Option
   https://github.com/joulei/makepad_workshop_gosim_beijing/blob/main/solution/src/abs_routine_screen.rs#L181

[kevinaboos]

Is changing to Option<Timer> the correct way to handle this? That seems a bit strange to me. Wouldn't we want to stop or reset the timer in stop_animation() instead of just dropping it?

I don't have much experience with makepad timers, so I'm not 100% sure if that would be sufficient. But if you can test that out and it works, then I think that design would make more sense.

[alanpoon]

Is changing to Option<Timer> the correct way to handle this? That seems a bit strange to me. Wouldn't we want to stop or reset the timer in stop_animation() instead of just dropping it?

I don't have much experience with makepad timers, so I'm not 100% sure if that would be sufficient. But if you can test that out and it works, then I think that design would make more sense.

The last time, I tried if timer.is_event(event).is_some() kept returning true. I will do more testing to confirm if the change is necessary.

[ZhangHanDong]

@alanpoon

Using Option<Timer> set to None is essentially the same as using Timer::default().

Suggestions for improvement:

Add an is_animating flag to control the animation state. Check is_animating in update_animation(), and if it’s false, do not continue with the animation.

In stop_animation():

• Set is_animating = false. Clear the timer.
• Use animator_cut to immediately reset all animations to their initial state.
• Set is_animating = true in animate() to start the animation.

This ensures that:

• The animation can be completely stopped.
• It can be reset to its initial state.
• It prevents multiple animation loops from running simultaneously.
• It provides clearer animation state management.

The following code is just for reference:

pub struct TypingAnimation {
    #[deref] view: View,
    #[animator] animator: Animator,
    #[live(0.65)] animation_duration: f64,
    #[rust] timer: Option<Timer>,
    #[rust] current_animated_dot: CurrentAnimatedDot,
    #[rust] is_animating: bool, //  new flag
}

impl TypingAnimation {
    pub fn update_animation(&mut self, cx: &mut Cx) {
        if !self.is_animating {
            return;
        }
        
        self.current_animated_dot = self.current_animated_dot.next();

        match self.current_animated_dot {
            CurrentAnimatedDot::Dot1 => {
                self.animator_play(cx, id!(circle1.up));
                self.animator_play(cx, id!(circle3.down));
            }
            CurrentAnimatedDot::Dot2 => {
                self.animator_play(cx, id!(circle1.down)); 
                self.animator_play(cx, id!(circle2.up));
            }
            CurrentAnimatedDot::Dot3 => {
                self.animator_play(cx, id!(circle2.down));
                self.animator_play(cx, id!(circle3.up)); 
            }
        }

        self.timer = Some(cx.start_timeout(self.animation_duration * 0.5));
    }
}

impl TypingAnimationRef {
    pub fn animate(&self, cx: &mut Cx) {
        if let Some(mut inner) = self.borrow_mut() {
            inner.is_animating = true;
            inner.update_animation(cx);
        }
    }

    pub fn stop_animation(&self) {
        if let Some(mut inner) = self.borrow_mut() {
            inner.is_animating = false;
            inner.timer = None;
            
            // Reset all animations to their initial state

            inner.animator_cut(cx, id!(circle1.down));
            inner.animator_cut(cx, id!(circle2.down)); 
            inner.animator_cut(cx, id!(circle3.down));

        }
    }
}

[kevinaboos]

See my update here: #260 (comment)

[kevinaboos]

Upon further discussion, it turns out that we shouldn't even be using Timers for this animation purpose. Instead, we should use next_frame() for properly timing animation sequences, as with all other animations in Makepad.

[kevinaboos]

Now that we're calling stop_timer() explicitly, do we still need to wrap the Timer in an Option? If not, then let's avoid unnecessary Options where possible.

[alanpoon]

Upon further discussion, it turns out that we shouldn't even be using Timers for this animation purpose. Instead, we should use next_frame() for properly timing animation sequences, as with all other animations in Makepad.

Changed to using shader animation. Removed timer
https://github.com/user-attachments/assets/10537d16-691e-4bc8-b461-5a4b4278d99e

[alanpoon]

Not sure if we want to put typing users and the room_id into app_data, as resizing to desktop view from mobile, the typing notice will disappear.

[ZhangHanDong]

Based on the principle of smooth and controllable animation effects.

Key Improvements

1. Added animation control fields:

   • freq: Control animation speed
   • phase_offset: Control bounce dots phase difference
   • last_update: Ensure animation smoothness based on time

2. Improved animation smoothness:

let now = Cx::time_now();        // Get current timestamp
let delta = now - self.last_update;  // Calculate actual elapsed time
self.time += delta as f32;       // Accumulate real time
self.last_update = now;          // Update last timestamp

Using actual elapsed time instead of frame-based modulo ensures consistent animation speed across different devices and frame rates, preventing stuttering.

Full code, for reference only

Rust Structure and Implementation

pub struct TypingAnimation {
    #[deref] view: View,
    #[live] time: f32,
    #[live] freq: f32,          // Animation frequency
    #[live] phase_offset: f32,  // Phase difference
    #[rust] next_frame: NextFrame,
    #[rust] is_play: bool,
    #[rust] last_update: f64,   // Last update timestamp
}

impl TypingAnimation {
    pub fn init(cx: &mut Cx) -> Self {
        Self {
            view: View::default(),
            time: 0.0,
            freq: 5.0,           // Default frequency
            phase_offset: 120.0,  // Default phase offset (adjustable at runtime)
            next_frame: cx.new_next_frame(),
            is_play: true,
            last_update: Cx::time_now(),
        }
    }
}

impl Widget for TypingAnimation {
    fn handle_event(&mut self, cx: &mut Cx, event: &Event, scope: &mut Scope) {
        if let Some(_ne) = self.next_frame.is_event(event) {
            if !self.is_play {
                return
            }
            
            let now = Cx::time_now();
            let delta = now - self.last_update;
            self.time += delta as f32;
            self.last_update = now;
            
            self.redraw(cx);
            self.next_frame = cx.new_next_frame();
        }
        self.view.handle_event(cx, event, scope);
    }
}

Shader Implementation

show_bg: true,
draw_bg: {
    uniform time: float,
    uniform freq: float,
    uniform phase_offset: float,  // Phase offset for wave effect
    
    fn pixel(self) -> vec4 {
        let sdf = Sdf2d::viewport(self.pos * self.rect_size);
        let color = vec4(0.0, 0.0, 0.0, 1.0);
        
        let amplitude = self.rect_size.y * 0.3;
        let center_y = self.rect_size.y * 0.4;
        
        // First dot - 0 degree phase
        sdf.circle(
            self.rect_size.x * 0.25,
            amplitude * sin(self.time * self.freq) + center_y,
            1.6
        );
        sdf.fill(color);
        
        // Second dot - with phase offset
        sdf.circle(
            self.rect_size.x * 0.5,
            amplitude * sin(self.time * self.freq + self.phase_offset) + center_y,
            1.6
        );
        sdf.fill(color);
        
        // Third dot - with double phase offset
        sdf.circle(
            self.rect_size.x * 0.75,
            amplitude * sin(self.time * self.freq + self.phase_offset * 2.0) + center_y,
            1.6
        );
        sdf.fill(color);
        
        return sdf.result;
    }
}

Animation Effect

Time1:  ●     ○     ○   (Dot1 at top, Dot2 and 3 at middle)
Time2:  ○     ●     ○   (Dot2 at top, Dot1 and 3 at other positions)
Time3:  ○     ○     ●   (Dot3 at top, Dot1 and 2 at other positions)

Usage Example

// Create animation instance
let mut animation = TypingAnimation::init(cx);

// Adjust animation parameters
animation.is_play = true;      // Start animation
animation.freq = 5.0;          // Adjust speed
animation.phase_offset = 90.0; // Adjust phase difference (changes dots spacing)

Phase Offset Effects

• phase_offset = 120.0: Even distribution (360 degrees/3)
• phase_offset = 90.0: Tighter wave effect
• phase_offset = 180.0: Larger spacing

[ZhangHanDong]

lgtm.

[kevinaboos]

Not sure if we want to put typing users and the room_id into app_data, as resizing to desktop view from mobile, the typing notice will disappear.

We wouldn't want to put this into the top-level app data struct, but perhaps you were referring to the TimelineUiState, which is the struct that's used to hold, save, and restore a room's state (including some UI-related state) when the timeline is being hidden.

I intentionally did not including the typing users state in that struct because the state of whether a user is currently typing is very transient and short-lived (I think the Matrix spec says it lasts no more than 4 seconds). So, by the time you restore the state, it's very likely that the old state will be invalid; thus, we don't save and restore this state for each timeline/room.

[kevinaboos]

Thanks, the new shader-based implementation looks great.

However, the actual animation appearance has changed a bit, and now looks worse than it did before (granted, that's just my personal opinion). I had tweaked the timing, spacing, and height of the bouncing dots animation in order for it to look subtle, smooth, and more professional.

Can you tweak the animation parameters in this PR to make it exactly match the previous animation?

For sake of easy comparison, here's the previous version:

Screen.Recording.2024-11-25.at.1.12.38.PM.mov

and here's this PR's current version:

Screen.Recording.2024-11-25.at.1.09.00.PM.mov

[kevinaboos]

Also, if this animation sequence is too hard to tweak/adjust/maintain, then we can just use a simpler animation sequence if you want. No need to make it so complex.

(Purely optional; i'm fine with anything as long as it's minimal and subtle.)

Some suggestions for an animation that might be easier to adjust and maintain in the future:

• https://medium.com/@dit.cu/how-to-create-a-loading-animation-with-three-dots-in-swiftui-44ec7ac16cd5
• or this very basic one

Screen.Recording.2024-11-25.at.1.17.53.PM.mov

[alanpoon]

Also, if this animation sequence is too hard to tweak/adjust/maintain, then we can just use a simpler animation sequence if you want. No need to make it so complex.

(Purely optional; i'm fine with anything as long as it's minimal and subtle.)

Some suggestions for an animation that might be easier to adjust and maintain in the future:

• https://medium.com/@dit.cu/how-to-create-a-loading-animation-with-three-dots-in-swiftui-44ec7ac16cd5
• or this very basic one

Screen.Recording.2024-11-25.at.1.17.53.PM.mov

I can't do all three dots disappear at the same time as I am heavily reliant on trigo function. Without it, there is quite a lot of Modulus to do. I ended with something quite similar to the swiftui one.

hole_towards_right.mp4

[ZhangHanDong]

lgtm

[alanpoon]

Slow down the animation
https://github.com/user-attachments/assets/c8f13a55-23e4-4377-a1af-9f942caff8bc

[kevinaboos]

hmm, the flashing of the dots makes it look like its stuttering, as if the animation isn't smooth.

I didn't mean to suggest that the prior animation of bouncing dots was bad, I was only suggesting alternatives if and only if they were simpler. It seems like they're not any simpler, so let's just use the original one that Alex implemented since it looks better from an animation standpoint.

[kevinaboos]

i've tweaked the animation to a smooth, subtle effect and will push that fix, then merge this PR. thanks for fixing the original animation issue!