fix(label): fix updating scrolling label text (#7533)

Co-authored-by: Liam <30486941+liamHowatt@users.noreply.github.com>

src/misc/lv_anim.c

@@ -19,8 +19,16 @@
 /*********************
  *      DEFINES
  *********************/
+
+/**Perform linear animations in max 1024 steps. Used in `path_cb`s*/
 #define LV_ANIM_RESOLUTION 1024
+
+/**log2(LV_ANIM_RESOLUTION)*/
 #define LV_ANIM_RES_SHIFT 10
+
+/**In an anim. time this bit indicates that the value is speed, and not time*/
+#define LV_ANIM_SPEED_MASK 0x80000000
+
 #define state LV_GLOBAL_DEFAULT()->anim_state
 #define anim_ll_p &(state.anim_ll)
 
@@ -36,7 +44,6 @@ static void anim_mark_list_change(void);
 static void anim_completed_handler(lv_anim_t * a);
 static int32_t lv_anim_path_cubic_bezier(const lv_anim_t * a, int32_t x1,
                                          int32_t y1, int32_t x2, int32_t y2);
-static uint32_t convert_speed_to_time(uint32_t speed, int32_t start, int32_t end);
 static void resolve_time(lv_anim_t * a);
 static bool remove_concurrent_anims(lv_anim_t * a_current);
 static void remove_anim(void * a);
@@ -218,7 +225,7 @@ uint32_t lv_anim_speed_clamped(uint32_t speed, uint32_t min_time, uint32_t max_t
     min_time = (min_time + 5) / 10;
     max_time = (max_time + 5) / 10;
 
-    return 0x80000000 + (max_time << 20) + (min_time << 10) + speed;
+    return LV_ANIM_SPEED_MASK + (max_time << 20) + (min_time << 10) + speed;
 
 }
 
@@ -478,9 +485,25 @@ lv_anim_t * lv_anim_custom_get(lv_anim_t * a, lv_anim_custom_exec_cb_t exec_cb)
     return lv_anim_get(a ? a->var : NULL, (lv_anim_exec_xcb_t)exec_cb);
 }
 
+uint32_t lv_anim_resolve_speed(uint32_t speed_or_time, int32_t start, int32_t end)
+{
+    /*It was a simple time*/
+    if((speed_or_time & LV_ANIM_SPEED_MASK) == 0) return speed_or_time;
+
+    uint32_t d    = LV_ABS(start - end);
+    uint32_t speed = speed_or_time & 0x3FF;
+    uint32_t time = (d * 100) / speed; /*Speed is in 10 units per sec*/
+    uint32_t max_time = (speed_or_time >> 20) & 0x3FF;
+    uint32_t min_time = (speed_or_time >> 10) & 0x3FF;
+
+    return LV_CLAMP(min_time * 10, time, max_time * 10);
+}
+
+
 /**********************
  *   STATIC FUNCTIONS
  **********************/
+
 /**
  * Periodically handle the animations.
  * @param param unused
@@ -641,26 +664,12 @@ static int32_t lv_anim_path_cubic_bezier(const lv_anim_t * a, int32_t x1, int32_
     return new_value;
 }
 
-static uint32_t convert_speed_to_time(uint32_t speed_or_time, int32_t start, int32_t end)
-{
-    /*It was a simple time*/
-    if((speed_or_time & 0x80000000) == 0) return speed_or_time;
-
-    uint32_t d    = LV_ABS(start - end);
-    uint32_t speed = speed_or_time & 0x3FF;
-    uint32_t time = (d * 100) / speed; /*Speed is in 10 units per sec*/
-    uint32_t max_time = (speed_or_time >> 20) & 0x3FF;
-    uint32_t min_time = (speed_or_time >> 10) & 0x3FF;
-
-    return LV_CLAMP(min_time * 10, time, max_time * 10);
-}
-
 static void resolve_time(lv_anim_t * a)
 {
-    a->duration = convert_speed_to_time(a->duration, a->start_value, a->end_value);
-    a->reverse_duration = convert_speed_to_time(a->reverse_duration, a->start_value, a->end_value);
-    a->reverse_delay = convert_speed_to_time(a->reverse_delay, a->start_value, a->end_value);
-    a->repeat_delay = convert_speed_to_time(a->repeat_delay, a->start_value, a->end_value);
+    a->duration = lv_anim_resolve_speed(a->duration, a->start_value, a->end_value);
+    a->reverse_duration = lv_anim_resolve_speed(a->reverse_duration, a->start_value, a->end_value);
+    a->reverse_delay = lv_anim_resolve_speed(a->reverse_delay, a->start_value, a->end_value);
+    a->repeat_delay = lv_anim_resolve_speed(a->repeat_delay, a->start_value, a->end_value);
 }
 
 /**

src/misc/lv_anim.h

@@ -412,15 +412,21 @@ uint16_t lv_anim_count_running(void);
 
 /**
  * Store the speed as a special value which can be used as time in animations.
- * It will be converted to time internally based on the start and end values
+ * It will be converted to time internally based on the start and end values.
+ * The return value can be used as a constant with multiple animations
+ * and let LVGL convert the speed to time based on the actual values.
+ * LIMITATION: the max time stored this way can be 10,000 ms.
  * @param speed         the speed of the animation in with unit / sec resolution in 0..10k range
  * @return              a special value which can be used as an animation time
+ * @note                internally speed is stored as 10 unit/sec
  */
 uint32_t lv_anim_speed(uint32_t speed);
 
 /**
  * Store the speed as a special value which can be used as time in animations.
- * It will be converted to time internally based on the start and end values
+ * It will be converted to time internally based on the start and end values.
+ * The return value can be used as a constant with multiple animations
+ * and let LVGL convert the speed to time based on the actual values.
  * @param speed         the speed of the animation in as unit / sec resolution in 0..10k range
  * @param min_time      the minimum time in 0..10k range
  * @param max_time      the maximum time in 0..10k range
@@ -431,8 +437,21 @@ uint32_t lv_anim_speed(uint32_t speed);
  */
 uint32_t lv_anim_speed_clamped(uint32_t speed, uint32_t min_time, uint32_t max_time);
 
+/**
+ * Resolve the speed (created with `lv_anim_speed` or `lv_anim_speed_clamped`) to time
+ * based on start and end values.
+ * @param speed     return values of `lv_anim_speed` or `lv_anim_speed_clamped`
+ * @param start     the start value of the animation
+ * @param end       the end value of the animation
+ * @return          the time required to get from `start` to `end` with the given `speed` setting
+ */
+uint32_t lv_anim_resolve_speed(uint32_t speed, int32_t start, int32_t end);
+
 /**
  * Calculate the time of an animation based on its speed, start and end values.
+ * It simpler than `lv_anim_speed` or `lv_anim_speed_clamped` as it converts
+ * speed, start, and end to a time immediately.
+ * As it's simpler there is no limit on the maximum time.
  * @param speed         the speed of the animation
  * @param start         the start value
  * @param end           the end value
@@ -440,6 +459,7 @@ uint32_t lv_anim_speed_clamped(uint32_t speed, uint32_t min_time, uint32_t max_t
  */
 uint32_t lv_anim_speed_to_time(uint32_t speed, int32_t start, int32_t end);
 
+
 /**
  * Manually refresh the state of the animations.
  * Useful to make the animations running in a blocking process where

src/widgets/label/lv_label.c

@@ -902,7 +902,7 @@ static void overwrite_anim_property(lv_anim_t * dest, const lv_anim_t * src, lv_
 {
     switch(mode) {
         case LV_LABEL_LONG_MODE_SCROLL:
-            /** If the dest animation is already running, overwrite is not allowed */
+            /* If the dest animation is already running, overwrite is not allowed */
             if(dest->act_time <= 0)
                 dest->act_time = src->act_time;
             dest->repeat_cnt = src->repeat_cnt;
@@ -911,7 +911,7 @@ static void overwrite_anim_property(lv_anim_t * dest, const lv_anim_t * src, lv_
             dest->reverse_delay = src->reverse_delay;
             break;
         case LV_LABEL_LONG_MODE_SCROLL_CIRCULAR:
-            /** If the dest animation is already running, overwrite is not allowed */
+            /* If the dest animation is already running, overwrite is not allowed */
             if(dest->act_time <= 0)
                 dest->act_time = src->act_time;
             dest->repeat_cnt = src->repeat_cnt;
@@ -997,9 +997,11 @@ static void lv_label_refr_text(lv_obj_t * obj)
                 act_time = anim_cur->act_time;
                 reverse_play_in_progress = anim_cur->reverse_play_in_progress;
             }
-            if(act_time < a.duration) {
-                a.act_time = act_time;      /*To keep the old position*/
-                a.early_apply = 0;
+
+            int32_t duration_resolved = lv_anim_resolve_speed(anim_time, start, end);
+            /*To keep the old position*/
+            if(act_time < duration_resolved) {
+                a.act_time = act_time;
                 if(reverse_play_in_progress) {
                     a.reverse_play_in_progress = 1;
                     /*Swap the start and end values*/
@@ -1011,12 +1013,16 @@ static void lv_label_refr_text(lv_obj_t * obj)
             }
 
             lv_anim_set_duration(&a, anim_time);
-            lv_anim_set_reverse_duration(&a, a.duration);
+            lv_anim_set_reverse_duration(&a, anim_time);
 
             /*If a template animation exists, overwrite some property*/
             if(anim_template)
                 overwrite_anim_property(&a, anim_template, label->long_mode);
             lv_anim_start(&a);
+
+            /*If a delay is happening, apply the start value manually*/
+            if(act_time < 0) label->offset.x = start;
+
             hor_anim = true;
         }
         else {
@@ -1038,7 +1044,6 @@ static void lv_label_refr_text(lv_obj_t * obj)
             }
             if(act_time < a.duration) {
                 a.act_time = act_time;      /*To keep the old position*/
-                a.early_apply = 0;
                 if(reverse_play_in_progress) {
                     a.reverse_play_in_progress = 1;
                     /*Swap the start and end values*/
@@ -1050,11 +1055,12 @@ static void lv_label_refr_text(lv_obj_t * obj)
             }
 
             lv_anim_set_duration(&a, anim_time);
-            lv_anim_set_reverse_duration(&a, a.duration);
+            lv_anim_set_reverse_duration(&a, anim_time);
 
             /*If a template animation exists, overwrite some property*/
-            if(anim_template)
+            if(anim_template) {
                 overwrite_anim_property(&a, anim_template, label->long_mode);
+            }
             lv_anim_start(&a);
         }
         else {
@@ -1101,14 +1107,16 @@ static void lv_label_refr_text(lv_obj_t * obj)
             lv_anim_t * anim_cur = lv_anim_get(obj, set_ofs_x_anim);
             int32_t act_time = anim_cur ? anim_cur->act_time : 0;
 
+            /*To keep the old position when the label text is updated mid-scrolling*/
+            int32_t duration_resolved = lv_anim_resolve_speed(anim_time, a.start_value, a.end_value);
+            if(act_time < duration_resolved) {
+                a.act_time = act_time;
+            }
+
             /*If a template animation exists, overwrite some property*/
             if(anim_template) {
                 overwrite_anim_property(&a, anim_template, label->long_mode);
             }
-            else if(act_time < a.duration) {
-                a.act_time = act_time;      /*To keep the old position when the label text is updated mid-scrolling*/
-                a.early_apply = 0;
-            }
 
             lv_anim_start(&a);
             hor_anim = true;
@@ -1131,9 +1139,9 @@ static void lv_label_refr_text(lv_obj_t * obj)
             if(anim_template) {
                 overwrite_anim_property(&a, anim_template, label->long_mode);
             }
+            /*To keep the old position when the label text is updated mid-scrolling*/
             else if(act_time < a.duration) {
-                a.act_time = act_time;      /*To keep the old position when the label text is updated mid-scrolling*/
-                a.early_apply = 0;
+                a.act_time = act_time;
             }
 
             lv_anim_start(&a);

tests/src/test_cases/widgets/test_label.c

@@ -626,4 +626,41 @@ void test_label_with_recolor_cmd(void)
     TEST_ASSERT_EQUAL_SCREENSHOT("widgets/label_recolor.png");
 }
 
+static void scroll_next_step(lv_obj_t * label1, lv_obj_t * label2, const char * text1, const char * text2, uint32_t idx)
+{
+    lv_label_set_text(label1, (idx % 2) == 0 ? text1 : text2);
+    lv_label_set_text(label2, (idx % 2) == 0 ? text1 : text2);
+    lv_test_wait(783); /*Use an odd delay*/
+
+    char buf[128];
+    lv_snprintf(buf, sizeof(buf), "widgets/label_scroll_%d.png", idx);
+    TEST_ASSERT_EQUAL_SCREENSHOT(buf);
+}
+
+void test_label_scroll_mid_update(void)
+{
+    lv_obj_clean(lv_screen_active());
+
+    const char * text1 = "This is a long text that we will update while scrolling";
+    const char * text2 = "THIS IS A LONG TEXT THAT WE WILL UPDATE WHILE SCROLLING";
+
+    lv_obj_t * label1 = lv_label_create(lv_screen_active());
+    lv_label_set_long_mode(label1, LV_LABEL_LONG_MODE_SCROLL);
+    lv_label_set_text(label1, text1),
+                      lv_obj_set_width(label1, 150);
+    lv_obj_set_pos(label1, 10, 10);
+
+    lv_obj_t * label2 = lv_label_create(lv_screen_active());
+    lv_label_set_long_mode(label2, LV_LABEL_LONG_MODE_SCROLL_CIRCULAR);
+    lv_label_set_text(label2, text1),
+                      lv_obj_set_width(label2, 150);
+    lv_obj_set_pos(label2, 10, 80);
+
+    uint32_t i;
+    for(i = 0; i < 15; i++) {
+        scroll_next_step(label1, label2, text1, text2, i);
+    }
+
+}
+
 #endif