put_line/scrolling: Make the scroll engine inform custom scrollers about start/stop of scrolling.

With the new lcd_putsxy_scroll_func() code can register custom scroll functions
(put_line() makes use of that). In order for the custom scroller to be able
to properly manage its userdata pointer (set via struct scrollinfo::userdata)
the scroll engine must inform the scroller about start and stop of scrolling.

To inform about start the lcd_scroll_* functions now return true when
the line will scroll. To inform about stop the scroll engine calls into the
scroller one last time, with the text set to NULL.

put_line() can use this to release the userdata registered per scrolling line
so that it can be recycled.

This fixes that some scrolling lines became glitchy after some time because
the userdata was recycled too early.

Change-Id: Iff0a6ce2a4f9ae2bada1b8e62f4f5950224942a9

8 files changed, 95 insertions, 54 deletions

diff --git a/apps/gui/line.c b/apps/gui/line.c
index d561f08c76..e2eb6f277b 100644
--- a/apps/gui/line.c
+++ b/apps/gui/line.c
@@ -50,18 +50,27 @@ static void style_line(struct screen *display, int x, int y, struct line_desc *l
 static void put_text(struct screen *display, int x, int y, struct line_desc *line,
                       const char *text, bool prevent_scroll, int text_skip_pixels);
 
+struct line_desc_scroll {
+    struct line_desc desc; /* must be first! */
+    bool used;
+};
+
+#define NOINLINE __attribute__ ((noinline))
 
-static struct line_desc *get_line_desc(void)
+struct line_desc_scroll *get_line_desc(void) NOINLINE;
+struct line_desc_scroll *get_line_desc(void)
 {
-    static struct line_desc lines[MAX_LINES];
+    static struct line_desc_scroll lines[MAX_LINES];
     static unsigned line_index;
-    struct line_desc *ret;
+    struct line_desc_scroll *this;
 
-    ret = &lines[line_index++];
-    if (line_index >= ARRAYLEN(lines))
-        line_index = 0;
+    do {
+        this = &lines[line_index++];
+        if (line_index >= ARRAYLEN(lines))
+            line_index = 0;
+    } while (this->used);
 
-    return ret;
+    return this;
 }
 
 static void scroller(struct scrollinfo *s, struct screen *display)
@@ -72,9 +81,17 @@ static void scroller(struct scrollinfo *s, struct screen *display)
      * line padding. this needs to be corrected for calling style_line().
      * The alternative would be to really redraw only the text area,
      * but that would complicate the code a lot */ 
-    struct line_desc *line = s->userdata;
-    style_line(display, s->x, s->y - (line->height/2 - display->getcharheight()/2), line);
-    put_text(display, s->x, s->y, line, s->line, true, s->offset);
+    struct line_desc_scroll *line = s->userdata;
+    if (!s->line)
+    {
+        line->used = false;
+    }
+    else
+    if (s->line)
+    {
+        style_line(display, s->x, s->y - (line->desc.height/2 - display->getcharheight()/2), &line->desc);
+        put_text(display, s->x, s->y, &line->desc, s->line, true, s->offset);
+    }
 }
 
 static void scroller_main(struct scrollinfo *s)
@@ -126,14 +143,16 @@ static void put_text(struct screen *display,
 
     if (line->scroll && !prevent_scroll)
     {
-        struct line_desc *line_data = get_line_desc();
-        *line_data = *line;
+        bool scrolls;
+        struct line_desc_scroll *line_data = get_line_desc();
+        line_data->desc = *line;
         /* precalculate to avoid doing it in the scroller, it's save to
          * do this on the copy of the original line_desc*/
-        if (line_data->height == -1)
-            line_data->height = display->getcharheight();
-        display->putsxy_scroll_func(x, y, text,
+        if (line_data->desc.height == -1)
+            line_data->desc.height = display->getcharheight();
+        scrolls = display->putsxy_scroll_func(x, y, text,
             scrollers[display->screen_type], line_data, text_skip_pixels);
+        line_data->used = scrolls;
     }
     else
         display->putsxy_scroll_func(x, y, text, NULL, NULL, text_skip_pixels);
diff --git a/apps/plugin.h b/apps/plugin.h
index 2cf40d5758..5d6527d7a4 100644
--- a/apps/plugin.h
+++ b/apps/plugin.h
@@ -197,7 +197,7 @@ struct plugin_api {
     void (*lcd_putsxyf)(int x, int y, const unsigned char *fmt, ...);
     void (*lcd_puts)(int x, int y, const unsigned char *string);
     void (*lcd_putsf)(int x, int y, const unsigned char *fmt, ...);
-    void (*lcd_puts_scroll)(int x, int y, const unsigned char* string);
+    bool (*lcd_puts_scroll)(int x, int y, const unsigned char* string);
     void (*lcd_scroll_stop)(void);
 #ifdef HAVE_LCD_CHARCELLS
     void (*lcd_define_pattern)(unsigned long ucs, const char *pattern);
diff --git a/apps/screen_access.h b/apps/screen_access.h
index 7efc38b174..c4a87849b8 100644
--- a/apps/screen_access.h
+++ b/apps/screen_access.h
@@ -126,8 +126,8 @@ struct screen
     void (*putsxy)(int x, int y, const unsigned char *str);
     void (*puts)(int x, int y, const unsigned char *str);
     void (*putsf)(int x, int y, const unsigned char *str, ...);
-    void (*puts_scroll)(int x, int y, const unsigned char *string);
-    void (*putsxy_scroll_func)(int x, int y, const unsigned char *string,
+    bool (*puts_scroll)(int x, int y, const unsigned char *string);
+    bool (*putsxy_scroll_func)(int x, int y, const unsigned char *string,
                                void (*scroll_func)(struct scrollinfo *),
                                void *data, int x_offset);
     void (*scroll_speed)(int speed);
diff --git a/firmware/drivers/lcd-bitmap-common.c b/firmware/drivers/lcd-bitmap-common.c
index a71f5b2862..22430d4e50 100644
--- a/firmware/drivers/lcd-bitmap-common.c
+++ b/firmware/drivers/lcd-bitmap-common.c
@@ -316,15 +316,19 @@ static struct scrollinfo* find_scrolling_line(int x, int y)
 
 void LCDFN(scroll_fn)(struct scrollinfo* s)
 {
+    /* with line == NULL when scrolling stops. This scroller
+     * maintains no userdata so there is nothing left to do */
+    if (!s->line)
+        return;
     /* Fill with background/backdrop to clear area.
-     * cannot use clear_viewport_rect() since stops scrolling as well */
+     * cannot use clear_viewport_rect() since would stop scrolling as well */
     LCDFN(set_drawmode)(DRMODE_SOLID|DRMODE_INVERSEVID);
     LCDFN(fillrect)(s->x, s->y, s->width, s->height);
     LCDFN(set_drawmode)(DRMODE_SOLID);
     LCDFN(putsxyofs)(s->x, s->y, s->offset, s->line);
 }
 
-static void LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string,
+static bool LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string,
                                      int x_offset,
                                      bool linebased,
                                      void (*scroll_func)(struct scrollinfo *),
@@ -336,7 +340,7 @@ static void LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string,
     bool restart;
 
     if (!string)
-        return;
+        return false;
 
     /* prepare rectangle for scrolling. x and y must be calculated early
      * for find_scrolling_line() to work */
@@ -347,32 +351,25 @@ static void LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string,
     width = current_vp->width - x;
 
     if (y >= current_vp->height)
-        return;
+        return false;
 
     s = find_scrolling_line(x, y);
     restart = !s;
 
-    if (restart) {
-        /* remove any previously scrolling line at the same location */
-        LCDFN(scroll_stop_viewport_rect)(current_vp, x, y, width, height);
-        LCDFN(putsxyofs)(x, y, x_offset, string);
-
-        if (LCDFN(scroll_info).lines >= LCDM(SCROLLABLE_LINES))
-            return;
-    }
-
     /* get width (pixeks) of the string */
     LCDFN(getstringsize)(string, &w, &h);
 
-    /* check if scrolling is actually necessary (consider the actual start
-     * of the line) */
-    if (width >= w)
-        return;
-
-    if (restart) {
-        /* prepare scroll line */
+    /* Remove any previously scrolling line at the same location. If
+     * the string width is too small to scroll the scrolling line is
+     * cleared as well */
+    if (w < width || restart) {
+        LCDFN(scroll_stop_viewport_rect)(current_vp, x, y, width, height);
+        LCDFN(putsxyofs)(x, y, x_offset, string);
+        /* nothing to scroll, or out of scrolling lines. Either way, get out */
+        if (w < width || LCDFN(scroll_info).lines >= LCDM(SCROLLABLE_LINES))
+            return false;
+        /* else restarting: prepare scroll line */
         s = &LCDFN(scroll_info).scroll[LCDFN(scroll_info).lines];
-        s->start_tick = current_tick + LCDFN(scroll_info).delay;
     }
 
     /* copy contents to the line buffer */
@@ -385,9 +382,6 @@ static void LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string,
     else
         s->bidir = false;
 
-    s->scroll_func = scroll_func;
-    s->userdata = data;
-
     if (restart) {
         s->offset = x_offset;
         s->backward = false;
@@ -397,26 +391,41 @@ static void LCDFN(puts_scroll_worker)(int x, int y, const unsigned char *string,
         s->width = width;
         s->height = height;
         s->vp = current_vp;
+        s->start_tick = current_tick + LCDFN(scroll_info).delay;
         LCDFN(scroll_info).lines++;
     } else {
-        /* if only the text was updated render immediately */
-        LCDFN(scroll_now(s));
+        /* not restarting, however we are about to assign new userdata;
+         * therefore tell the scroller that it can release the previous userdata */
+        s->line = NULL;
+        s->scroll_func(s);
     }
+
+    s->scroll_func = scroll_func;
+    s->userdata = data;
+
+    /* if only the text was updated render immediately */
+    if (!restart)
+        LCDFN(scroll_now(s));
+
+    return true;
 }
 
-void LCDFN(putsxy_scroll_func)(int x, int y, const unsigned char *string,
+bool LCDFN(putsxy_scroll_func)(int x, int y, const unsigned char *string,
                                      void (*scroll_func)(struct scrollinfo *),
                                      void *data, int x_offset)
 {
+    bool retval = false;
     if (!scroll_func)
         LCDFN(putsxyofs)(x, y, x_offset, string);
     else
-        LCDFN(puts_scroll_worker)(x, y, string, x_offset, false, scroll_func, data);
+        retval = LCDFN(puts_scroll_worker)(x, y, string, x_offset, false, scroll_func, data);
+
+    return retval;
 }
 
-void LCDFN(puts_scroll)(int x, int y, const unsigned char *string)
+bool LCDFN(puts_scroll)(int x, int y, const unsigned char *string)
 {
-    LCDFN(puts_scroll_worker)(x, y, string, 0, true, LCDFN(scroll_fn), NULL);
+    return LCDFN(puts_scroll_worker)(x, y, string, 0, true, LCDFN(scroll_fn), NULL);
 }
 
 #if !defined(HAVE_LCD_COLOR) || !defined(MAIN_LCD)
diff --git a/firmware/drivers/lcd-scroll.c b/firmware/drivers/lcd-scroll.c
index a1bde9fe12..5162f9a100 100644
--- a/firmware/drivers/lcd-scroll.c
+++ b/firmware/drivers/lcd-scroll.c
@@ -30,7 +30,7 @@
 #define MAIN_LCD
 #endif
 
-static struct scrollinfo LCDFN(scroll)[LCD_SCROLLABLE_LINES];
+static struct scrollinfo LCDFN(scroll)[LCDM(SCROLLABLE_LINES)];
 
 struct scroll_screen_info LCDFN(scroll_info) =
 {
@@ -51,6 +51,13 @@ struct scroll_screen_info LCDFN(scroll_info) =
 
 void LCDFN(scroll_stop)(void)
 {
+    for (int i = 0; i < LCDFN(scroll_info).lines; i++)
+    {
+        /* inform scroller about end of scrolling */
+        struct scrollinfo *s = &LCDFN(scroll_info).scroll[i];
+        s->line = NULL;
+        s->scroll_func(s);
+    }
     LCDFN(scroll_info).lines = 0;
 }
 
@@ -66,6 +73,9 @@ void LCDFN(scroll_stop_viewport_rect)(const struct viewport *vp, int x, int y, i
             && (x < (s->x+s->width) && (x+width) >= s->x)
             && (y < (s->y+s->height) && (y+height) >= s->y))
         {
+            /* inform scroller about end of scrolling */
+            s->line = NULL;
+            s->scroll_func(s);
             /* If i is not the last active line in the array, then move
                the last item to position i. This compacts
                the scroll array at the same time of removing the line */
diff --git a/firmware/export/lcd-remote.h b/firmware/export/lcd-remote.h
index bc26a23cb7..1819a4de72 100644
--- a/firmware/export/lcd-remote.h
+++ b/firmware/export/lcd-remote.h
@@ -176,8 +176,8 @@ extern void lcd_remote_clear_viewport(void);
 extern void lcd_remote_puts(int x, int y, const unsigned char *str);
 extern void lcd_remote_putsf(int x, int y, const unsigned char *fmt, ...);
 extern void lcd_remote_putc(int x, int y, unsigned short ch);
-extern void lcd_remote_puts_scroll(int x, int y, const unsigned char *str);
-extern void lcd_remote_putsxy_scroll_func(int x, int y, const unsigned char *string,
+extern bool lcd_remote_puts_scroll(int x, int y, const unsigned char *str);
+extern bool lcd_remote_putsxy_scroll_func(int x, int y, const unsigned char *string,
                                           void (*scroll_func)(struct scrollinfo *),
                                           void *data, int x_offset);
 
diff --git a/firmware/export/lcd.h b/firmware/export/lcd.h
index 87476d9dda..386ac5a8bf 100644
--- a/firmware/export/lcd.h
+++ b/firmware/export/lcd.h
@@ -179,8 +179,8 @@ extern void lcd_putsxy_style_offset(int x, int y, const unsigned char *str,
 extern void lcd_puts(int x, int y, const unsigned char *string);
 extern void lcd_putsf(int x, int y, const unsigned char *fmt, ...);
 extern void lcd_putc(int x, int y, unsigned long ucs);
-extern void lcd_puts_scroll(int x, int y, const unsigned char* string);
-extern void lcd_putsxy_scroll_func(int x, int y, const unsigned char *string,
+extern bool lcd_puts_scroll(int x, int y, const unsigned char* string);
+extern bool lcd_putsxy_scroll_func(int x, int y, const unsigned char *string,
                                    void (*scroll_func)(struct scrollinfo *),
                                    void *data, int x_offset);
 
diff --git a/firmware/export/scroll_engine.h b/firmware/export/scroll_engine.h
index 64e1d6d6ae..ce230a218c 100644
--- a/firmware/export/scroll_engine.h
+++ b/firmware/export/scroll_engine.h
@@ -79,7 +79,10 @@ struct scrollinfo
     bool bidir;
     long start_tick;
 
-    /* support for custom scrolling functions */
+    /* support for custom scrolling functions,
+     * must be called with ::line == NULL to indicate that the line
+     * stops scrolling or when the userdata pointer is going to be changed
+     * (the custom scroller can release the userdata then) */
     void (*scroll_func)(struct scrollinfo *s);
     void *userdata;
 };