From 804e3aadaeb3e40bddfc20d184ae105bae2a70c4 Mon Sep 17 00:00:00 2001 From: Thomas Martitz Date: Tue, 20 Oct 2009 23:12:20 +0000 Subject: [PATCH] Fix a few potential redraw problems with the custom statusbar and wps fighting for the same full redraw variable. Instead, introduce a new skin struct holding data which is meant for all screens for a single skin (struct wps_state is currently used by both at the same time). Also clean up (classic) statusbar handling for skins using this new struct. Also, implement deactivating updating of the custom statusbar when the LCD is deactivated, saving a bit battery life on some targets. git-svn-id: svn://svn.rockbox.org/rockbox/trunk@23304 a1c6a512-1295-4272-9138-f99709370657 --- apps/gui/skin_engine/skin_display.c | 6 ++-- apps/gui/skin_engine/wps_internals.h | 20 +++++++----- apps/gui/statusbar-skinned.c | 47 ++++++++++++++++++++++++---- apps/gui/wps.c | 37 +++++++++------------- 4 files changed, 72 insertions(+), 38 deletions(-) diff --git a/apps/gui/skin_engine/skin_display.c b/apps/gui/skin_engine/skin_display.c index 66675ce95a..13751a9b1c 100644 --- a/apps/gui/skin_engine/skin_display.c +++ b/apps/gui/skin_engine/skin_display.c @@ -100,9 +100,9 @@ bool skin_update(struct gui_wps *gwps, unsigned int update_type) * called from. This is also safe for skined screen which dont use the id3 */ struct mp3entry *id3 = gwps->state->id3; bool cuesheet_update = (id3 != NULL ? cuesheet_subtrack_changed(id3) : false); - gwps->state->do_full_update = cuesheet_update || gwps->state->do_full_update; + gwps->sync_data->do_full_update |= cuesheet_update; - retval = skin_redraw(gwps, gwps->state->do_full_update ? + retval = skin_redraw(gwps, gwps->sync_data->do_full_update ? WPS_REFRESH_ALL : update_type); return retval; } @@ -1145,7 +1145,7 @@ static bool skin_redraw(struct gui_wps *gwps, unsigned refresh_mode) if (refresh_mode & WPS_REFRESH_STATUSBAR) { - viewportmanager_set_statusbar(*gwps->statusbars); + viewportmanager_set_statusbar(gwps->sync_data->statusbars); } /* Restore the default viewport */ display->set_viewport(NULL); diff --git a/apps/gui/skin_engine/wps_internals.h b/apps/gui/skin_engine/wps_internals.h index 3dbf4e0087..1fd68646ad 100644 --- a/apps/gui/skin_engine/wps_internals.h +++ b/apps/gui/skin_engine/wps_internals.h @@ -296,9 +296,19 @@ struct wps_state bool wps_time_countup; struct mp3entry* id3; struct mp3entry* nid3; - bool do_full_update; }; +/* Holds data for all screens in a skin. */ +struct wps_sync_data +{ + /* suitable for the viewportmanager, possibly only temporary here + * needs to be same for all screens! can't be split up for screens + * due to what viewportmanager_set_statusbar() accepts + * (FIXME?) */ + int statusbars; + /* indicates whether the skin needs a full update for all screens */ + bool do_full_update; +}; /* change the ff/rew-status if ff_rew = true then we are in skipping mode @@ -318,12 +328,8 @@ struct gui_wps struct screen *display; struct wps_data *data; struct wps_state *state; - - /* suitable for the viewportmanager, possibly only temporary here - * needs to be same for all screens! can't be split up for screens - * due to what viewportmanager_set_statusbar() accepts - * (FIXME?) */ - int *statusbars; + /* must point to the same struct for all screens */ + struct wps_sync_data *sync_data; }; /* gui_wps end */ diff --git a/apps/gui/statusbar-skinned.c b/apps/gui/statusbar-skinned.c index 1cb6aa3623..e28782d85a 100644 --- a/apps/gui/statusbar-skinned.c +++ b/apps/gui/statusbar-skinned.c @@ -38,6 +38,7 @@ extern struct wps_state wps_state; /* from wps.c */ static struct gui_wps sb_skin[NB_SCREENS] = {{ .data = NULL }}; static struct wps_data sb_skin_data[NB_SCREENS] = {{ .wps_loaded = 0 }}; +static struct wps_sync_data sb_skin_sync_data = { .do_full_update = false }; /* initial setup of wps_data */ static void sb_skin_update(void*); @@ -87,9 +88,22 @@ inline bool sb_skin_get_state(enum screen_type screen) return loaded_ok[screen] && (skinbars & VP_SB_ONSCREEN(screen)); } + +static void do_update_callback(void *param) +{ + (void)param; + /* the WPS handles changing the actual id3 data in the id3 pointers + * we imported, we just want a full update */ + sb_skin_sync_data.do_full_update = true; + /* force timeout in wps main loop, so that the update is instantly */ + queue_post(&button_queue, BUTTON_NONE, 0); +} + + void sb_skin_set_state(int state, enum screen_type screen) { - sb_skin[screen].state->do_full_update = true; + sb_skin[screen].sync_data->do_full_update = true; + skinbars = sb_skin[screen].sync_data->statusbars; if (state) { skinbars |= VP_SB_ONSCREEN(screen); @@ -100,28 +114,45 @@ void sb_skin_set_state(int state, enum screen_type screen) } if (skinbars) + { +#if defined(HAVE_LCD_ENABLE) || defined(HAVE_LCD_SLEEP) + add_event(LCD_EVENT_ACTIVATION, false, do_update_callback); +#endif add_event(GUI_EVENT_ACTIONUPDATE, false, sb_skin_update); + } else + { +#if defined(HAVE_LCD_ENABLE) || defined(HAVE_LCD_SLEEP) + add_event(LCD_EVENT_ACTIVATION, false, do_update_callback); +#endif remove_event(GUI_EVENT_ACTIONUPDATE, sb_skin_update); + } + + sb_skin[screen].sync_data->statusbars = skinbars; } static void sb_skin_update(void* param) { static long next_update = 0; int i; - int forced_draw = param || sb_skin[SCREEN_MAIN].state->do_full_update; + int forced_draw = param || sb_skin[SCREEN_MAIN].sync_data->do_full_update; if (TIME_AFTER(current_tick, next_update) || forced_draw) { FOR_NB_SCREENS(i) { if (sb_skin_get_state(i)) { - skin_update(&sb_skin[i], forced_draw? - WPS_REFRESH_ALL : WPS_REFRESH_NON_STATIC); +#if defined(HAVE_LCD_ENABLE) || defined(HAVE_LCD_SLEEP) + /* currently, all remotes are readable without backlight + * so still update those */ + if (lcd_active() || (i != SCREEN_MAIN)) +#endif + skin_update(&sb_skin[i], forced_draw? + WPS_REFRESH_ALL : WPS_REFRESH_NON_STATIC); } } next_update = current_tick + update_delay; /* don't update too often */ - sb_skin[SCREEN_MAIN].state->do_full_update = false; + sb_skin[SCREEN_MAIN].sync_data->do_full_update = false; } } @@ -130,6 +161,7 @@ void sb_skin_set_update_delay(int delay) update_delay = delay; } + void sb_skin_init(void) { int i; @@ -147,6 +179,9 @@ void sb_skin_init(void) /* Currently no seperate wps_state needed/possible so use the only available ( "global" ) one */ sb_skin[i].state = &wps_state; - sb_skin[i].statusbars = &skinbars; + sb_skin_sync_data.statusbars = VP_SB_HIDE_ALL; + sb_skin[i].sync_data = &sb_skin_sync_data; } + add_event(PLAYBACK_EVENT_TRACK_CHANGE, false, do_update_callback); + add_event(PLAYBACK_EVENT_NEXTTRACKID3_AVAILABLE, false, do_update_callback); } diff --git a/apps/gui/wps.c b/apps/gui/wps.c index 6d9d8443dd..8d1a0b9bf9 100644 --- a/apps/gui/wps.c +++ b/apps/gui/wps.c @@ -78,13 +78,11 @@ /* 3% of 30min file == 54s step size */ #define MIN_FF_REWIND_STEP 500 -/* this is for the viewportmanager */ -static int wpsbars = 0; - /* currently only one wps_state is needed, initialize to 0 */ - struct wps_state wps_state = { .id3 = NULL }; + struct wps_state wps_state = { .id3 = NULL }; static struct gui_wps gui_wps[NB_SCREENS] = {{ .data = NULL }}; static struct wps_data wps_datas[NB_SCREENS] = {{ .wps_loaded = 0 }}; +static struct wps_sync_data wps_sync_data = { .do_full_update = false }; /* initial setup of wps_data */ static void wps_state_init(void); @@ -565,7 +563,7 @@ static void play_hop(int direction) static void wps_lcd_activation_hook(void *param) { (void)param; - wps_state.do_full_update = true; + wps_sync_data.do_full_update = true; /* force timeout in wps main loop, so that the update is instantly */ queue_post(&button_queue, BUTTON_NONE, 0); } @@ -1127,7 +1125,7 @@ long gui_wps_show(void) /* this case is used by the softlock feature * it requests a full update here */ case ACTION_REDRAW: - wps_state.do_full_update = true; + wps_sync_data.do_full_update = true; break; case ACTION_NONE: /* Timeout, do a partial update */ update = true; @@ -1157,7 +1155,7 @@ long gui_wps_show(void) break; } - if (wps_state.do_full_update || update) + if (wps_sync_data.do_full_update || update) { #if defined(HAVE_BACKLIGHT) || defined(HAVE_REMOTE_LCD) gwps_caption_backlight(&wps_state); @@ -1173,13 +1171,7 @@ long gui_wps_show(void) skin_update(&gui_wps[i], WPS_REFRESH_NON_STATIC); } } - /* currently skinned statusbar and wps share the same wps_state, - * don't steal do_full_update away */ - if (wps_state.do_full_update) - { - send_event(GUI_EVENT_ACTIONUPDATE, (void*)true); - wps_state.do_full_update = false; - } + wps_sync_data.do_full_update = false; update = false; } @@ -1242,13 +1234,13 @@ static void track_changed_callback(void *param) cue_find_current_track(wps_state.id3->cuesheet, wps_state.id3->elapsed); cue_spoof_id3(wps_state.id3->cuesheet, wps_state.id3); } - wps_state.do_full_update = true; + wps_sync_data.do_full_update = true; } static void nextid3available_callback(void* param) { (void)param; wps_state.nid3 = audio_next_track(); - wps_state.do_full_update = true; + wps_sync_data.do_full_update = true; } @@ -1267,7 +1259,7 @@ static void wps_state_init(void) wps_state.nid3 = NULL; } /* We'll be updating due to restore initialized with true */ - wps_state.do_full_update = false; + wps_sync_data.do_full_update = false; /* add the WPS track event callbacks */ add_event(PLAYBACK_EVENT_TRACK_CHANGE, false, track_changed_callback); add_event(PLAYBACK_EVENT_NEXTTRACKID3_AVAILABLE, false, nextid3available_callback); @@ -1280,7 +1272,8 @@ static void statusbar_toggle_handler(void *data) (void)data; int i; - wpsbars = VP_SB_HIDE_ALL; + int *wpsbars = &wps_sync_data.statusbars; + *wpsbars = VP_SB_HIDE_ALL; FOR_NB_SCREENS(i) { /* fix viewports if needed */ skin_statusbar_changed(&gui_wps[i]); @@ -1293,7 +1286,7 @@ static void statusbar_toggle_handler(void *data) else if (statusbar_position(i) != STATUSBAR_OFF) draw = true; if (draw) - wpsbars |= + *wpsbars |= (VP_SB_ONSCREEN(i) | VP_SB_IGNORE_SETTING(i)); } } @@ -1318,10 +1311,10 @@ void gui_sync_wps_init(void) so use the only available ( "global" ) one */ gui_wps[i].state = &wps_state; gui_wps[i].display->backdrop_unload(BACKDROP_SKIN_WPS); - /* only one wpsbars needed/wanted */ - gui_wps[i].statusbars = &wpsbars; + /* must point to the same struct for both screens */ + gui_wps[i].sync_data = &wps_sync_data; + gui_wps[i].sync_data->statusbars = VP_SB_ALLSCREENS; } - *(gui_wps[SCREEN_MAIN].statusbars) =VP_SB_ALLSCREENS; #ifdef HAVE_LCD_BITMAP add_event(GUI_EVENT_STATUSBAR_TOGGLE, false, statusbar_toggle_handler); #endif