Skip to content

Commit 2a8d0a7

Browse files
committed
MENU/XMB: Fix three icon regressions and a wallpaper ordering bug in
xmb_context_reset_internal This fixes three related icon-display symptoms in the XMB driver — all tracing back to the same underlying confusion between animation-suppression state and data-resolution state, compounded by the recent async icon-loading migration — and a fourth cosmetic ordering bug in xmb_context_reset_internal that was discovered while walking the reset flow for the icon fixes. Symptom 1 — Main Menu > Load Core has no category icon. The small rocket icon + `<` arrow to the left of the selected entry never renders on cold navigation into the submenu. Symptom 2 — Icon thumbnails (Boxart etc.) show the default disc icon until navigating in/out of submenus or fullscreen-toggling. Per-entry boxart thumbnails stay at their default stock fallback even though the setting is enabled and artwork exists. Symptom 3 — After the earlier two were fixed, a fullscreen toggle while inside a QuickMenu wiped out the category icon on the left, and only re-entering the submenu restored it. Subsequent toggles did not help. Symptom 4 — In xmb_context_reset_internal the dynamic wallpaper was loaded against a stale xmb->title_name. xmb_path_dynamic_wallpaper() reads title_name to build the wallpaper filename; the call was sequenced BEFORE xmb_set_title() which writes title_name. On cold boot title_name is empty (calloc), and after navigation it is whatever the previous stack position left behind. In practice this usually self-corrected on the next xmb_populate_entries -> xmb_update_dynamic_wallpaper chain (which uses the correct set_title -> update_wallpaper order), but one frame of wrong wallpaper could render. xmb_set_title does not read anything xmb_update_dynamic_wallpaper writes, so the swap is safe. Root cause of symptoms 1-3 — two problems, same family: (a) allow_horizontal_animation was used as a gate on three paths that have nothing to do with animation. It is an animation-suppression flag: calloc-initialized to false, set true only by the restore halves of the multi-tab keyboard jump (line 6150), the gesture/mouse jump (line 10000), and the reinit_textures==false branch of xmb_context_reset_internal (the deferred scale-factor path). None of those run on first navigation into a submenu, so any non-animation logic guarded by the flag never executes until the user does a tab jump. The three misplaced gates were: - xmb_set_title (from 0be544b "XMB: Current menu icon refactor"): an early return in front of the entire current-menu-icon resolution block, pure label/enum/texture computation with no animation in it. With the gate, xmb->current_menu_icon stayed 0 and the "Current menu icon + arrow" draw at line 8341 rendered nothing. This is symptom 1. - xmb_selection_pointer_changed and xmb_populate_dynamic_icons (both from 24a1a4c "XMB: Icon thumbnail cleanups"): these are the only two sites that set pending_icons = XMB_PENDING_THUMBNAIL_ICONS, which is what xmb_render reads to kick off per-entry icon-thumbnail loads. With the gate, entries in a fresh playlist stayed at GFX_THUMBNAIL_STATUS_UNKNOWN and the draw at line 5569 (status == AVAILABLE check) fell back to the stock disc icon. This is symptom 2. Fix: drop the three misplaced gates. The removed work is cheap path-setting and flag-flipping. Intermediate tab-jump calls self-cancel via gfx_thumbnail_cancel_pending_requests() in the loop body, so there is no load storm during fast jumps. The two legitimate animation-suppression uses (line 2600 gating gfx_animation_push in xmb_list_switch_horizontal_list, line 9555 gating entry alpha animation) are untouched. (b) After (a), xmb_set_title does run during context reset. But xmb_context_reset_horizontal_list queues sidebar and content icons via the async gfx_display_load_icon path (introduced by the 4b3b8ed / 9b7eff2 / 5c1e31a async migration). Inside xmb_context_reset_internal the order is: xmb_context_reset_textures() // zeros current_menu_icon, sync xmb_context_reset_horizontal_list() // queues async icon tasks xmb_set_title() // runs NOW, tasks haven't landed In the DEFERRED_RPL_ENTRY_ACTIONS case of xmb_set_title, the code unconditionally assigned db_node->content_icon (or ->icon) to the resolved texture. With the tasks still in flight those handles are 0, so xmb->current_menu_icon got set to 0 and the draw guard refused to render. Each subsequent fullscreen toggle re-ran destroy -> reset -> queue -> set_title with the same race window, so subsequent toggles didn't fix it either. Out-and-back navigation worked only because the tasks had wall-clock time to land before the next xmb_populate_entries -> xmb_set_title. This is symptom 3. Fix: two parts. 1. Guard the db_icon and sidebar_node->icon assignments in xmb_set_title: only overwrite `texture` when the handle is actually loaded. Otherwise leave `texture` at its prior value (XMB_TEXTURE_QUICKMENU default, or a previously-set sidebar icon). This alone prevents the blank-icon symptom — the QuickMenu default shows instead. 2. Self-healing retry via a new uint8_t current_menu_icon_retry countdown on xmb_handle_t. xmb_set_title arms the counter (60 frames on fresh trigger, prev_retry - 1 on ongoing retry) when it had to substitute a fallback for an unresolved async icon. xmb_render polls the counter each frame and re-invokes xmb_set_title while it is nonzero. On successful resolution the fallback branches don't fire and the counter stays 0. On permanent failure (missing asset) the countdown terminates after ~1s of cheap retries. The counter is a uint8_t packed next to the existing system_tab_end field — no practical size increase. Bisect trail: 0be544b "XMB: Current menu icon refactor (#18508)" — introduced the misplaced gate in xmb_set_title that caused symptom 1. The feature shipped with the broken gate from day one. 24a1a4c "XMB: Icon thumbnail cleanups" — added the misplaced gates at the two icon-thumbnail population sites that caused symptom 2. 4b3b8ed "Use non-blocking image loading instead...", 9b7eff2 "Move xmb_context_reset_horizontal_list to async...", 5c1e31a "libretro-common/file/nbio/nbio_intf.c ..." — the async icon-loading migration. This made horizontal-list icons load asynchronously and exposed the latent assumption in xmb_set_title that the sidebar/db-node icon handles would be populated synchronously by context reset. Symptom 3 follows from this + the feature from 0be544b. Symptom 4 (the wallpaper ordering) has been latent since xmb_update_dynamic_wallpaper and xmb_set_title coexisted — not tied to a specific recent commit.
1 parent f2e3f9e commit 2a8d0a7

1 file changed

Lines changed: 57 additions & 7 deletions

File tree

menu/drivers/xmb.c

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,11 @@ typedef struct xmb_handle
443443

444444
uint8_t system_tab_end;
445445
uint8_t tabs[XMB_SYSTEM_TAB_MAX_LENGTH];
446+
/* Frames remaining to retry xmb_set_title() after a context reset
447+
* left an async-loaded sidebar / db-node icon unresolved. Capped
448+
* so that a permanently-missing icon asset doesn't cause a
449+
* perpetual retry loop. */
450+
uint8_t current_menu_icon_retry;
446451

447452
char title_name[NAME_MAX_LENGTH];
448453
char title_name_alt[NAME_MAX_LENGTH];
@@ -1855,7 +1860,6 @@ static void xmb_selection_pointer_changed(
18551860
real_iy = iy + xmb->margins_screen_top;
18561861

18571862
if ( xmb->is_playlist
1858-
&& xmb->allow_horizontal_animation
18591863
&& gfx_thumbnail_is_enabled(menu_st->thumbnail_path_data, GFX_THUMBNAIL_ICON)
18601864
&& !string_is_equal(xmb->title_name, msg_hash_to_str(MENU_ENUM_LABEL_VALUE_IMAGES_TAB))
18611865
&& !string_is_equal(xmb->title_name, msg_hash_to_str(MENU_ENUM_LABEL_VALUE_MUSIC_TAB))
@@ -2322,9 +2326,6 @@ static void xmb_set_title(xmb_handle_t *xmb)
23222326
xmb->title_name[sub] = '\0';
23232327
}
23242328

2325-
if (!xmb->allow_horizontal_animation)
2326-
return;
2327-
23282329
if (config_get_ptr()->uints.menu_xmb_current_menu_icon)
23292330
{
23302331
char label_temp[NAME_MAX_LENGTH];
@@ -2337,6 +2338,15 @@ static void xmb_set_title(xmb_handle_t *xmb)
23372338
uintptr_t texture = xmb->textures.list[XMB_TEXTURE_QUICKMENU];
23382339
bool search = true;
23392340

2341+
/* Preserve and decrement any in-flight retry countdown from
2342+
* xmb_render(). Fresh fallback (prev_retry == 0) (re)arms to
2343+
* ~1s @ 60fps; ongoing retry counts down one frame at a time
2344+
* so a permanently-missing asset can't spin forever. Cleared
2345+
* unconditionally first — any fallback branch below will set
2346+
* the next value. */
2347+
uint8_t prev_retry = xmb->current_menu_icon_retry;
2348+
xmb->current_menu_icon_retry = 0;
2349+
23402350
menu_entries_get_last_stack(&path, &label, &type, &enum_idx, &entry_idx);
23412351
label_original = label;
23422352

@@ -2489,6 +2499,13 @@ static void xmb_set_title(xmb_handle_t *xmb)
24892499
sidebar_node = (xmb_node_t*)file_list_get_userdata_at_offset(&xmb->horizontal_list, i);
24902500
if (sidebar_node && sidebar_node->icon)
24912501
texture = sidebar_node->icon;
2502+
else if (sidebar_node)
2503+
/* Async load still in flight (e.g. right after a
2504+
* fullscreen toggle triggered xmb_context_reset) —
2505+
* ask xmb_render() to retry. Fresh trigger arms to
2506+
* 60 frames (~1s); ongoing retry decrements one
2507+
* step so a missing asset terminates the loop. */
2508+
xmb->current_menu_icon_retry = prev_retry ? prev_retry - 1 : 60;
24922509
}
24932510

24942511
/* Playlists entries */
@@ -2498,7 +2515,23 @@ static void xmb_set_title(xmb_handle_t *xmb)
24982515
if ( pl_entry
24992516
&& (pl_entry->db_name && *pl_entry->db_name)
25002517
&& (db_node = RHMAP_GET_STR(xmb->playlist_db_node_map, pl_entry->db_name)))
2501-
texture = (enum_idx == MENU_ENUM_LABEL_HORIZONTAL_MENU) ? db_node->icon : db_node->content_icon;
2518+
{
2519+
/* Sidebar / content icons load asynchronously via
2520+
* xmb_context_reset_horizontal_list(). A context reset
2521+
* (e.g. fullscreen toggle) will zero these handles and
2522+
* re-queue the loads; xmb_set_title() runs before the
2523+
* tasks complete. Fall back to the existing `texture`
2524+
* (XMB_TEXTURE_QUICKMENU by default, or sidebar_node->icon
2525+
* if that happened to be set above) rather than assigning
2526+
* a 0 handle — which would blank the icon until the next
2527+
* navigation refresh. Flag pending so xmb_render() retries. */
2528+
uintptr_t db_icon = (enum_idx == MENU_ENUM_LABEL_HORIZONTAL_MENU)
2529+
? db_node->icon : db_node->content_icon;
2530+
if (db_icon)
2531+
texture = db_icon;
2532+
else
2533+
xmb->current_menu_icon_retry = prev_retry ? prev_retry - 1 : 60;
2534+
}
25022535
else
25032536
{
25042537
const playlist_config_t *pl_config = playlist_get_config(playlist_get_cached());
@@ -2677,7 +2710,6 @@ static void xmb_populate_dynamic_icons(xmb_handle_t *xmb)
26772710
video_driver_get_size(NULL, &height);
26782711
xmb_calculate_visible_range(xmb, height, end, (unsigned)selection, &entry_start, &entry_end);
26792712
if ( xmb->is_playlist
2680-
&& xmb->allow_horizontal_animation
26812713
&& !string_is_equal(xmb->title_name, msg_hash_to_str(MENU_ENUM_LABEL_VALUE_IMAGES_TAB))
26822714
&& !string_is_equal(xmb->title_name, msg_hash_to_str(MENU_ENUM_LABEL_VALUE_MUSIC_TAB))
26832715
&& !string_is_equal(xmb->title_name, msg_hash_to_str(MENU_ENUM_LABEL_VALUE_VIDEO_TAB))
@@ -6835,9 +6867,18 @@ static void xmb_context_reset_internal(xmb_handle_t *xmb,
68356867
xmb->allow_dynamic_wallpaper = true;
68366868
}
68376869

6838-
xmb_update_dynamic_wallpaper(xmb, true);
68396870
xmb_context_reset_horizontal_list(xmb);
68406871
xmb_set_title(xmb);
6872+
/* Must run after xmb_set_title(): xmb_path_dynamic_wallpaper()
6873+
* reads xmb->title_name to build the wallpaper filename. The
6874+
* previous order invoked this with a stale title_name from the
6875+
* prior stack position (or empty-string on cold boot, since
6876+
* xmb_handle_t is calloc-initialized), so the wallpaper loaded
6877+
* here could briefly be the wrong one until the next
6878+
* xmb_populate_entries -> xmb_update_dynamic_wallpaper chain
6879+
* corrected it. xmb_populate_entries already uses the correct
6880+
* set_title -> update_dynamic_wallpaper order. */
6881+
xmb_update_dynamic_wallpaper(xmb, true);
68416882

68426883
menu_screensaver_context_destroy(xmb->screensaver);
68436884

@@ -6918,6 +6959,15 @@ static void xmb_render(void *data,
69186959
settings->uints.menu_xmb_theme);
69196960
}
69206961

6962+
/* Retry current-menu-icon resolution if a previous xmb_set_title()
6963+
* (typically the one invoked from xmb_context_reset_internal() after
6964+
* a fullscreen toggle) could not resolve the icon because an
6965+
* async-loaded sidebar or db-node icon handle was still 0.
6966+
* xmb_set_title() decrements the counter on each retry and clears
6967+
* it on success, so the loop terminates either way. */
6968+
if (xmb->current_menu_icon_retry > 0)
6969+
xmb_set_title(xmb);
6970+
69216971
xmb->use_ps3_layout = xmb_use_ps3_layout(settings->uints.menu_xmb_layout, width, height);
69226972
scale_factor = xmb_get_scale_factor(settings->floats.menu_scale_factor,
69236973
xmb->use_ps3_layout, width);

0 commit comments

Comments
 (0)