gui: simplify screen updates

commit c0a8303a9c98c87c0a68bb288f0d1664dca9cc31
Author: Christian Soffke <[email protected]>
Date:   Sun May 24 20:04:28 2026 +0200

    gui: simplify screen updates
    
    Eliminate the somewhat circuitous skin_defer_rendering
    and skin_render_deferred functions introduced in commit
    c145d19e85.
    
    skin_render (and thus skin_update and sb_skin_update),
    along with skinlist_draw, list_draw (and thus
    gui_synclist_draw), and quickscreen_draw, will never
    update the screen anymore. Instead, the skin is only
    marked as dirty now.
    
    GUI_EVENT_ACTIONUPDATE redraws the skin as necessary,
    and updates the screen afterwards, if the skin is dirty.
    This event is sent from get_action, just like before.
    
    A new event, GUI_EVENT_ACTIONREDRAW allows you to redraw
    the skin without causing an immediate update at the same
    time.
    
    Change-Id: Ib20644853ead901e32f639000f044d6935135bae

diff --git a/apps/appevents.h b/apps/appevents.h
index 1efd8035e3..2af2c2cb26 100644
--- a/apps/appevents.h
+++ b/apps/appevents.h
@@ -73,11 +73,13 @@ enum {
 
 /** Generic GUI class events **/
 enum {
-    GUI_EVENT_STATUSBAR_TOGGLE = (EVENT_CLASS_GUI|1),
+    /* Redraw skin as needed (or by force) */
+    GUI_EVENT_ACTIONREDRAW = (EVENT_CLASS_GUI|1),
+    /* Redraw skin, and update the screen if it's dirty */
     GUI_EVENT_ACTIONUPDATE,
     GUI_EVENT_THEME_CHANGED,
     /* Called when the UI viewport is cleared in the skin engine to
-     * notify the current screen that it needs to do an update */
+     * notify the current screen that it needs to redraw itself */
     GUI_EVENT_NEED_UI_UPDATE,
 };
 
diff --git a/apps/gui/bitmap/list-skinned.c b/apps/gui/bitmap/list-skinned.c
index b9bd18c2d1..505525f3b2 100644
--- a/apps/gui/bitmap/list-skinned.c
+++ b/apps/gui/bitmap/list-skinned.c
@@ -281,7 +281,6 @@ bool skinlist_draw(struct screen *display, struct gui_synclist *list)
     }
     current_column = -1;
     current_row = -1;
-    skin_render_deferred(display, parent);
     current_drawing_line = list->selected_item;
     return true;
 }
diff --git a/apps/gui/bitmap/list.c b/apps/gui/bitmap/list.c
index 34ae79e316..db18f125e7 100644
--- a/apps/gui/bitmap/list.c
+++ b/apps/gui/bitmap/list.c
@@ -446,7 +446,6 @@ void list_draw(struct screen *display, struct gui_synclist *list)
 
         callback_draw_item(&list_info);
     }
-    skin_render_deferred(display, parent);
     display->set_viewport(last_vp);
 }
 
diff --git a/apps/gui/list.c b/apps/gui/list.c
index f5f02f2cb8..28d4bacc7e 100644
--- a/apps/gui/list.c
+++ b/apps/gui/list.c
@@ -238,9 +238,9 @@ void gui_synclist_draw(struct gui_synclist *gui_list)
     {
         sb_title_is_dirty = theme_enabled = false;
 
-        /* tell skin engine to refresh, then call us back */
+        /* Redraw skin, and make skin engine call us back */
         add_event_ex(GUI_EVENT_NEED_UI_UPDATE, true, sb_title_cb, gui_list);
-        send_event(GUI_EVENT_ACTIONUPDATE, (void*)1);
+        send_event(GUI_EVENT_ACTIONREDRAW, (void*)1);
         remove_event_ex(GUI_EVENT_NEED_UI_UPDATE, sb_title_cb, gui_list);
 
         /* sb_title_cb was only called if theme is enabled */
@@ -258,6 +258,7 @@ void gui_synclist_draw(struct gui_synclist *gui_list)
     {
         if (!skinlist_draw(&screens[i], gui_list))
             list_draw(&screens[i], gui_list);
+        skin_mark_dirty(i);
     }
 }
 
diff --git a/apps/gui/quickscreen.c b/apps/gui/quickscreen.c
index b0d74d43a5..3841268023 100644
--- a/apps/gui/quickscreen.c
+++ b/apps/gui/quickscreen.c
@@ -40,6 +40,7 @@
 #include "debug.h"
 #include "shortcuts.h"
 #include "appevents.h"
+#include "statusbar-skinned.h"
 
  /* 1 top, 1 bottom, 2 on either side, 1 for the icons
   * if enough space, top and bottom have 2 lines */
@@ -233,7 +234,7 @@ static void quickscreen_draw(struct quickscreen *qs, enum screen_type screen)
             (vp_icons->width/2) - 4, vp_icons->height - 8, 7, 8);
     }
 
-    skin_render_deferred(display, parent);
+    skin_mark_dirty(display->screen_type);
     display->set_viewport(last_vp);
 }
 
@@ -418,13 +419,11 @@ static void quickscreen_run(struct quickscreen * qs)
             can_quit = true;
         else if (button == ACTION_QS_VOLUP) {
             adjust_volume(1);
-            FOR_NB_SCREENS(i)
-                skin_update(CUSTOM_STATUSBAR, i, SKIN_REFRESH_NON_STATIC);
+            sb_skin_force_next_update();
         }
         else if (button == ACTION_QS_VOLDOWN) {
             adjust_volume(-1);
-            FOR_NB_SCREENS(i)
-                skin_update(CUSTOM_STATUSBAR, i, SKIN_REFRESH_NON_STATIC);
+            sb_skin_force_next_update();
         }
         else if (button == ACTION_STD_CONTEXT)
         {
diff --git a/apps/gui/skin_engine/skin_display.c b/apps/gui/skin_engine/skin_display.c
index 2a6f8072ec..4fbe9ac805 100644
--- a/apps/gui/skin_engine/skin_display.c
+++ b/apps/gui/skin_engine/skin_display.c
@@ -71,8 +71,20 @@
 #include "statusbar-skinned.h"
 #include "skin_display.h"
 
+static bool dirty[NB_SCREENS];
+
 void skin_render(struct gui_wps *gwps, unsigned refresh_mode);
 
+bool skin_is_dirty(enum screen_type screen)
+{
+    return dirty[screen] && !(dirty[screen] = false);
+}
+
+void skin_mark_dirty(enum screen_type screen)
+{
+    dirty[screen] = true;
+}
+
 /* update a skinned screen, update_type is WPS_REFRESH_* values.
  * Usually it should only be WPS_REFRESH_NON_STATIC
  * A full update will be done if required (skin_do_full_update() == true)
@@ -90,6 +102,7 @@ void skin_update(enum skinnable_screens skin, enum screen_type screen,
 
     skin_render(gwps, skin_do_full_update(skin, screen) ?
                         SKIN_REFRESH_ALL : update_type);
+    skin_mark_dirty(screen);
 }
 
 #ifdef AB_REPEAT_ENABLE
diff --git a/apps/gui/skin_engine/skin_engine.h b/apps/gui/skin_engine/skin_engine.h
index f753dc8087..b819d56773 100644
--- a/apps/gui/skin_engine/skin_engine.h
+++ b/apps/gui/skin_engine/skin_engine.h
@@ -50,12 +50,8 @@ void skin_disarm_touchregions(struct gui_wps *gwps);
 /* Do a update_type update of the skinned screen */
 void skin_update(enum skinnable_screens skin, enum screen_type screen,
                  unsigned int update_type);
-
-/* Defer updates in skin_render */
-void skin_defer_rendering(bool deferred);
-/* Render viewport together with deferred updates */
-void skin_render_deferred(struct screen *display, struct viewport *vp);
-
+void skin_mark_dirty(enum screen_type screen);
+bool skin_is_dirty(enum screen_type screen);
 bool skin_has_sbs(struct gui_wps *gwps);
 
 
diff --git a/apps/gui/skin_engine/skin_render.c b/apps/gui/skin_engine/skin_render.c
index eefbc80ee7..486b7caa0b 100644
--- a/apps/gui/skin_engine/skin_render.c
+++ b/apps/gui/skin_engine/skin_render.c
@@ -84,8 +84,6 @@ static void skin_render_playlistviewer(struct playlistviewer* viewer,
                                        unsigned long refresh_type);
 
 static char* skin_buffer;
-static bool defer_rendering;
-static bool dirty[NB_SCREENS];
 
 static inline struct skin_element*
 get_child(OFFSETTYPE(struct skin_element**) children, int child)
@@ -843,29 +841,6 @@ void skin_render_viewport(struct skin_element* viewport, struct gui_wps *gwps,
     wps_display_images(gwps, &skin_viewport->vp);
 }
 
-void skin_defer_rendering(bool deferred)
-{
-    defer_rendering = deferred;
-}
-
-void skin_render_deferred(struct screen *display, struct viewport *vp)
-{
-    if (defer_rendering)
-        return;
-
-    if (dirty[display->screen_type])
-    {
-        dirty[display->screen_type] = false;
-        display->set_viewport(NULL);
-        display->update();
-    }
-    else
-    {
-        display->set_viewport(vp);
-        display->update_viewport();
-    }
-}
-
 void skin_render(struct gui_wps *gwps, unsigned refresh_mode)
 {
     const int vp_is_appearing = (VP_DRAW_WASHIDDEN|VP_DRAW_HIDEABLE);
@@ -957,19 +932,14 @@ void skin_render(struct gui_wps *gwps, unsigned refresh_mode)
     skin_backdrop_show(data->backdrop_id);
 #endif
 
-    dirty[display->screen_type] = defer_rendering;
     if (((refresh_mode&SKIN_REFRESH_ALL) == SKIN_REFRESH_ALL))
     {
-        defer_rendering = true;
         /* If this is the UI viewport then let the UI know
          * to redraw itself */
         send_event(GUI_EVENT_NEED_UI_UPDATE, NULL);
-        defer_rendering = dirty[display->screen_type];
     }
     /* Restore the default viewport */
     display->set_viewport_ex(NULL, VP_FLAG_VP_SET_CLEAN);
-    if (!defer_rendering)
-        display->update();
 }
 
 static __attribute__((noinline))
diff --git a/apps/gui/viewport.c b/apps/gui/viewport.c
index 6bc51401f5..0c52b7afe3 100644
--- a/apps/gui/viewport.c
+++ b/apps/gui/viewport.c
@@ -83,7 +83,7 @@ static void evt_toggle(bool enable, unsigned short id,
 
 static void toggle_events(bool enable)
 {
-    evt_toggle(enable, GUI_EVENT_ACTIONUPDATE, viewportmanager_redraw);
+    evt_toggle(enable, GUI_EVENT_ACTIONREDRAW, viewportmanager_redraw);
     evt_toggle(enable, PLAYBACK_EVENT_TRACK_CHANGE, do_sbs_update_callback);
     evt_toggle(enable, PLAYBACK_EVENT_NEXTTRACKID3_AVAILABLE, do_sbs_update_callback);
 #if defined(HAVE_LCD_ENABLE) || defined(HAVE_LCD_SLEEP)
@@ -119,8 +119,7 @@ static void toggle_theme(enum screen_type screen, bool force)
     {
         last_vp = screens[screen].set_viewport(NULL);
         bool first_boot = theme_stack_top[screen] == 0;
-        /* remove the left overs from the previous screen.
-         * could cause a tiny flicker. Redo your screen code if that happens */
+        /* Remove the left overs from the previous screen */
 #ifdef HAVE_BACKDROP_IMAGE
         skin_backdrop_show(sb_get_backdrop(screen));
 #endif
@@ -154,9 +153,7 @@ static void toggle_theme(enum screen_type screen, bool force)
         }
         intptr_t force = first_boot?0:1;
 
-        skin_defer_rendering(true);
-        send_event(GUI_EVENT_ACTIONUPDATE, (void*)force);
-        skin_defer_rendering(false);
+        send_event(GUI_EVENT_ACTIONREDRAW, (void*)force);
         if (!first_boot)
             sb_skin_force_next_update();
     }
@@ -167,6 +164,7 @@ static void toggle_theme(enum screen_type screen, bool force)
 #endif
         screens[screen].scroll_stop();
         skinlist_set_cfg(screen, NULL);
+        skin_is_dirty(screen); /* remove dirty flag */
     }
     /* let list initialize viewport in case viewport dimensions is changed. */
     send_event(GUI_EVENT_THEME_CHANGED, NULL);
@@ -219,10 +217,16 @@ static void viewportmanager_redraw(unsigned short id, void* data)
 {
     (void)id;
     FOR_NB_SCREENS(i)
-    {
         if (is_theme_enabled(i))
             sb_skin_update(i, NULL != data);
-    }
+}
+
+static void viewportmanager_update(unsigned short id, void* data)
+{
+    viewportmanager_redraw(id, data);
+    FOR_NB_SCREENS(i)
+        if (skin_is_dirty(i))
+            screens[i].update();
 }
 
 void viewportmanager_init(void)
@@ -233,6 +237,7 @@ void viewportmanager_init(void)
         /* We always want the theme enabled by default... */
         viewportmanager_theme_enable(i, true, NULL);
     }
+    add_event(GUI_EVENT_ACTIONUPDATE, viewportmanager_update);
 }
 
 void viewportmanager_theme_changed(const int which)
diff --git a/apps/gui/wps.c b/apps/gui/wps.c
index 99a1f6917e..a5e7ad3029 100644
--- a/apps/gui/wps.c
+++ b/apps/gui/wps.c
@@ -533,11 +533,7 @@ static void gwps_leave_wps(bool theme_enabled)
                viewports drawn by the WPS. May need further thought... */
             struct wps_data *sbs = skin_get_gwps(CUSTOM_STATUSBAR, i)->data;
             if (gwps->data->use_extra_framebuffer && sbs->use_extra_framebuffer)
-            {
-                skin_defer_rendering(true);
                 skin_update(CUSTOM_STATUSBAR, i, SKIN_REFRESH_ALL);
-                skin_defer_rendering(false);
-            }
 #endif
             viewportmanager_theme_undo(i, skin_has_sbs(gwps));
         }
@@ -599,11 +595,7 @@ static void gwps_enter_wps(bool theme_enabled)
         skin_backdrop_show(gwps->data->backdrop_id);
 #endif
         display->clear_display();
-        if (skin_has_sbs(gwps))
-            skin_defer_rendering(true);
         skin_update(WPS, i, SKIN_REFRESH_ALL);
-        skin_defer_rendering(false);
-
     }
 #ifdef HAVE_TOUCHSCREEN
     gwps = skin_get_gwps(WPS, SCREEN_MAIN);
@@ -611,7 +603,7 @@ static void gwps_enter_wps(bool theme_enabled)
     if (gwps->data->touchregions < 0)
         touchscreen_set_mode(TOUCHSCREEN_BUTTON);
 #endif
-    /* force statusbar/skin update since we just cleared the whole screen */
+     /* Screen was cleared, so redraw SBS if enabled, and update screen */
     send_event(GUI_EVENT_ACTIONUPDATE, (void*)1);
 }
 
diff --git a/apps/misc.c b/apps/misc.c
index 7a11afdce1..f54c84beee 100644
--- a/apps/misc.c
+++ b/apps/misc.c
@@ -1807,11 +1807,7 @@ static void push_current_activity_refresh(enum current_activity screen, bool ref
     {
         skinlist_set_cfg(i, NULL);
         if (refresh)
-        {
-            skin_defer_rendering(true);
             skin_update(CUSTOM_STATUSBAR, i, SKIN_REFRESH_ALL);
-            skin_defer_rendering(false);
-        }
     }
     if (refresh)
         sb_skin_force_next_update();
@@ -1824,11 +1820,7 @@ static void pop_current_activity_refresh(bool refresh)
     {
         skinlist_set_cfg(i, NULL);
         if (refresh)
-        {
-            skin_defer_rendering(true);
             skin_update(CUSTOM_STATUSBAR, i, SKIN_REFRESH_ALL);
-            skin_defer_rendering(false);
-        }
     }
     if (refresh)
         sb_skin_force_next_update();
diff --git a/apps/plugin.c b/apps/plugin.c
index 98aa34c924..151fd4be4a 100644
--- a/apps/plugin.c
+++ b/apps/plugin.c
@@ -1023,10 +1023,8 @@ int plugin_load(const char* plugin, const void* parameter)
     pop_current_activity_without_refresh();
     if (get_current_activity() != ACTIVITY_WPS)
     {
-        skin_defer_rendering(true);
         FOR_NB_SCREENS(i)
-                skin_update(CUSTOM_STATUSBAR, i, SKIN_REFRESH_ALL);
-        skin_defer_rendering(false);
+            skin_update(CUSTOM_STATUSBAR, i, SKIN_REFRESH_ALL);
         sb_skin_force_next_update();
     }
 
diff --git a/apps/plugins/disktidy.c b/apps/plugins/disktidy.c
index 258ba95217..d2080b3cf1 100644
--- a/apps/plugins/disktidy.c
+++ b/apps/plugins/disktidy.c
@@ -329,7 +329,7 @@ static enum plugin_status display_run_stats(void)
     if (sbs_has_title)
     {
 #endif
-        rb->send_event(GUI_EVENT_ACTIONUPDATE, (void*)1);
+        rb->send_event(GUI_EVENT_ACTIONREDRAW, (void*)1);
     }
 
     rb->viewport_set_defaults(&vp, SCREEN_MAIN);
@@ -603,7 +603,7 @@ static enum plugin_status tidy_do(void)
 
     sbs_has_title = rb->sb_set_title_text(CLEANING_STR, Icon_NOICON, SCREEN_MAIN);
     if (sbs_has_title)
-        rb->send_event(GUI_EVENT_ACTIONUPDATE, (void*)1);
+        rb->send_event(GUI_EVENT_ACTIONREDRAW, (void*)1);
 
     run_stats.files_removed = 0;
     run_stats.dirs_removed = 0;
diff --git a/apps/plugins/lib/simple_viewer.c b/apps/plugins/lib/simple_viewer.c
index 27a30ef0b5..3cfe4b020f 100644
--- a/apps/plugins/lib/simple_viewer.c
+++ b/apps/plugins/lib/simple_viewer.c
@@ -299,9 +299,9 @@ int view_text(const char *title, const char *text)
     /* handle themes that draw over the UI viewport */
     rb->add_event_ex(GUI_EVENT_NEED_UI_UPDATE, false, ui_update_cb, &info);
 
-    /* skin engine needs to render title and redraw screen */
+    /* skin engine needs to draw title */
     if (info.sbs_has_title)
-        rb->send_event(GUI_EVENT_ACTIONUPDATE, (void*)1);
+        rb->send_event(GUI_EVENT_ACTIONREDRAW, (void*)1);
     else
         draw_text(&info);
 
diff --git a/apps/plugins/text_viewer/tv_window.c b/apps/plugins/text_viewer/tv_window.c
index cbef2e0ef7..919f562f0b 100644
--- a/apps/plugins/text_viewer/tv_window.c
+++ b/apps/plugins/text_viewer/tv_window.c
@@ -57,7 +57,7 @@ static void tv_draw_bookmarks(const struct tv_screen_pos *top_pos)
 void tv_update_sbs_title(void)
 {
     if (tv_set_sbs_title())
-        rb->send_event(GUI_EVENT_ACTIONUPDATE, (void*)1);
+        rb->send_event(GUI_EVENT_ACTIONREDRAW, (void*)1);
 }
 
 void tv_draw_window(void)
-- 
rockbox-cvs mailing list
[email protected]
https://lists.haxx.se/mailman/listinfo/rockbox-cvs