Fix the Rust/C++ Timer API to be more convenient to use

Allow calling restart() on a repeated timer if it has been previously stopped.

CHANGELOG.md

@@ -9,6 +9,8 @@ All notable changes to this project will be documented in this file.
    has changed. This was undocumented, but if one was handling this in the
    `FocusScope` event, these keys will now be ignored. Use the `Keys.LeftArrow`
    and other code exposed in the `Keys` namespace instead
+ - For `sixtyfps::Timer` (C++ and Rust), it's now possible to call `restart()` after
+   a timer has been stopped previously by calling `stop()`.
 
 ### Added
 

api/sixtyfps-cpp/include/sixtyfps.h

@@ -374,7 +374,7 @@ struct Timer
     }
     Timer(const Timer &) = delete;
     Timer &operator=(const Timer &) = delete;
-    ~Timer() { cbindgen_private::sixtyfps_timer_stop(id); }
+    ~Timer() { cbindgen_private::sixtyfps_timer_destroy(id); }
 
     /// Starts the timer with the given \a mode and \a interval, in order for the \a callback to
     /// called when the timer fires. If the timer has been started previously and not fired yet,
@@ -388,12 +388,12 @@ struct Timer
     }
     /// Stops the previously started timer. Does nothing if the timer has never been started. A
     /// stopped timer cannot be restarted with restart() -- instead you need to call start().
-    void stop()
-    {
-        cbindgen_private::sixtyfps_timer_stop(id);
-        id = -1;
-    }
-    /// Restarts the timer, if it was previously started.
+    void stop() { cbindgen_private::sixtyfps_timer_stop(id); }
+    /// Restarts the timer. If the timer was previously started by calling [`Self::start()`]
+    /// with a duration and callback, then the time when the callback will be next invoked
+    /// is re-calculated to be in the specified duration relative to when this function is called.
+    ///
+    /// Does nothing if the timer was never started.
     void restart() { cbindgen_private::sixtyfps_timer_restart(id); }
     /// Returns true if the timer is running; false otherwise.
     bool running() const { return cbindgen_private::sixtyfps_timer_running(id); }

api/sixtyfps-cpp/tests/eventloop.cpp

@@ -77,6 +77,57 @@ TEST_CASE("C++ Restart Singleshot Timer")
     REQUIRE(timer_was_running);
 }
 
+TEST_CASE("C++ Restart Repeated Timer")
+{
+    int timer_triggered = 0;
+    sixtyfps::Timer timer;
+
+    timer.start(sixtyfps::TimerMode::Repeated, std::chrono::milliseconds(30),
+                [&]() { timer_triggered++; });
+
+    REQUIRE(timer_triggered == 0);
+
+    bool timer_was_running = false;
+
+    sixtyfps::Timer::single_shot(std::chrono::milliseconds(500), [&]() {
+        timer_was_running = timer.running();
+        sixtyfps::quit_event_loop();
+    });
+
+    sixtyfps::run_event_loop();
+
+    REQUIRE(timer_triggered > 1);
+    REQUIRE(timer_was_running);
+
+    timer_was_running = false;
+    timer_triggered = 0;
+    timer.stop();
+    sixtyfps::Timer::single_shot(std::chrono::milliseconds(500), [&]() {
+        timer_was_running = timer.running();
+        sixtyfps::quit_event_loop();
+    });
+
+    sixtyfps::run_event_loop();
+
+    REQUIRE(timer_triggered == 0);
+    REQUIRE(!timer_was_running);
+
+    timer_was_running = false;
+    timer_triggered = 0;
+
+    timer.restart();
+
+    sixtyfps::Timer::single_shot(std::chrono::milliseconds(500), [&]() {
+        timer_was_running = timer.running();
+        sixtyfps::quit_event_loop();
+    });
+
+    sixtyfps::run_event_loop();
+
+    REQUIRE(timer_triggered > 1);
+    REQUIRE(timer_was_running);
+}
+
 TEST_CASE("Quit from event")
 {
     int called = 0;

sixtyfps_runtime/corelib/timers.rs

@@ -110,17 +110,20 @@ impl Timer {
         })
     }
 
-    /// Stops the previously started timer. Does nothing if the timer has never been started. A stopped
-    /// timer cannot be restarted with restart() -- instead you need to call start().
+    /// Stops the previously started timer. Does nothing if the timer has never been started.
     pub fn stop(&self) {
-        if let Some(id) = self.id.take() {
+        if let Some(id) = self.id.get() {
             CURRENT_TIMERS.with(|timers| {
-                timers.borrow_mut().remove_timer(id);
+                timers.borrow_mut().deactivate_timer(id);
             });
         }
     }
 
-    /// Restarts the timer, if it was previously started.
+    /// Restarts the timer. If the timer was previously started by calling [`Self::start()`]
+    /// with a duration and callback, then the time when the callback will be next invoked
+    /// is re-calculated to be in the specified duration relative to when this function is called.
+    ///
+    /// Does nothing if the timer was never started.
     pub fn restart(&self) {
         if let Some(id) = self.id.get() {
             CURRENT_TIMERS.with(|timers| {
@@ -359,7 +362,7 @@ pub(crate) mod ffi {
     /// Start a timer with the given mode, duration in millisecond and callback. A timer id may be provided (first argument).
     /// A value of -1 for the timer id means a new timer is to be allocated.
     /// The (new) timer id is returned.
-    /// The timer MUST be destroyed with sixtyfps_timer_stop.
+    /// The timer MUST be destroyed with sixtyfps_timer_destroy.
     #[no_mangle]
     pub extern "C" fn sixtyfps_timer_start(
         id: i64,
@@ -396,12 +399,23 @@ pub(crate) mod ffi {
 
     /// Stop a timer and free its raw data
     #[no_mangle]
+    pub extern "C" fn sixtyfps_timer_destroy(id: i64) {
+        if id == -1 {
+            return;
+        }
+        let timer = Timer { id: Cell::new(Some(id as _)) };
+        drop(timer);
+    }
+
+    /// Stop a timer
+    #[no_mangle]
     pub extern "C" fn sixtyfps_timer_stop(id: i64) {
         if id == -1 {
             return;
         }
         let timer = Timer { id: Cell::new(Some(id as _)) };
-        timer.stop()
+        timer.stop();
+        timer.id.take(); // Make sure that dropping the Timer doesn't unregister it. C++ will call destroy() in the destructor.
     }
 
     /// Restart a repeated timer
@@ -412,7 +426,7 @@ pub(crate) mod ffi {
         }
         let timer = Timer { id: Cell::new(Some(id as _)) };
         timer.restart();
-        timer.id.take(); // Make sure that dropping the Timer doesn't unregister it. C++ will call stop() in the destructor.
+        timer.id.take(); // Make sure that dropping the Timer doesn't unregister it. C++ will call destroy() in the destructor.
     }
 
     /// Returns true if the timer is running; false otherwise.
@@ -423,7 +437,7 @@ pub(crate) mod ffi {
         }
         let timer = Timer { id: Cell::new(Some(id as _)) };
         let running = timer.running();
-        timer.id.take(); // Make sure that dropping the Timer doesn't unregister it. C++ will call stop() in the destructor.
+        timer.id.take(); // Make sure that dropping the Timer doesn't unregister it. C++ will call destroy() in the destructor.
         running
     }
 }