diff --git a/.github/scripts/test_art_priority_queue.py b/.github/scripts/test_art_priority_queue.py deleted file mode 100644 index 8e254d657..000000000 --- a/.github/scripts/test_art_priority_queue.py +++ /dev/null @@ -1,236 +0,0 @@ -"""Verify unified art priority queue tiering and active trio ordering. - -Tests: -1. texcache.c queue mechanics (extracted directly from production src/texcache.c): - - artPush appends non-priority items at the tail (FIFO). - - artPushPriority inserts items at the end of the high-priority section (FIFO within priority) - ahead of any speculative lookahead items. - - artPromote splices an existing speculative lookahead into the end of the priority block. - - artPromote on an item already at the head or already priority operates safely without corrupting links. - - artPop drains priority items first in insertion order, then speculative items. - - artPop preserves SIO2 defer when navigation is active (Issue #340 protection). - -2. themes.c source structure: - - Pre-requests highlighted COV and ICO with isPriority = 1 during ELEM_TYPE_BACKGROUND draw. - - Passes icoElem through thmGetElemForItem for row-specific redirection. - - Requests BG with isPriority = 1. - - Removes artificial coverflowCoverSettled stall. -""" -from pathlib import Path -import re -import subprocess -import sys -import tempfile - -root = Path(__file__).resolve().parents[2] -failures = [] - - -def read(rel): - return (root / rel).read_text(encoding='utf-8').replace('\r\n', '\n') - - -def function_text(source, where, signature): - match = re.search(r'^' + re.escape(signature) + r'[^;{]*\)\s*\{', source, re.M) - if match is None: - failures.append('%s: %s...) not found' % (where, signature)) - return '' - end = source.index('\n}', match.start()) - return source[match.start():end] + '\n}\n' - - -texcache = read('src/texcache.c') -themes = read('src/themes.c') - -# --- Source structural guards --- - -if 'artPushPriority' not in texcache: - failures.append('texcache.c: artPushPriority function missing') - -if 'unsigned char priority;' not in texcache: - failures.append('texcache.c: load_image_request_t priority field missing') - -if 'abortRequested = 1;' not in texcache: - failures.append('texcache.c: abortRequested not found in texcache.c') - -# In themes.c, verify active trio pre-requests COV and ICO with isPriority = 1 -if 'getGameImageTextureEx(selImg->cache, menu->item->userdata, &item->item, 1)' not in themes and \ - 'getGameImageTextureEx(cfImg->cache, menu->item->userdata, &item->item, 1)' not in themes: - failures.append('themes.c: COV pre-request with isPriority = 1 missing in drawGameImage') - -if 'getGameImageTextureEx(icoImg->cache, menu->item->userdata, &item->item, 1)' not in themes: - failures.append('themes.c: ICO pre-request with isPriority = 1 missing in drawGameImage') - -# Verify ICO redirect via thmGetElemForItem -if 'icoElem = thmGetElemForItem(menu, item, icoElem);' not in themes: - failures.append('themes.c: icoElem missing thmGetElemForItem redirection') - -if 'coverflowCoverSettled' in themes: - failures.append('themes.c: coverflowCoverSettled should be removed to allow fluid BG streaming') - -queue_functions = ''.join(function_text(texcache, 'src/texcache.c', sig) for sig in ( - 'static void artPush(', - 'static void artPushPriority(', - 'static void artPromote(', - 'static load_image_request_t *artPop(', -)) - -QUEUE_HARNESS = r''' -#include -#include -#include - -static void DIntr(void) {} -static void EIntr(void) {} - -typedef struct load_image_request { - struct load_image_request *next; - struct load_image_request *prev; - unsigned int queueEpoch; - volatile int abortRequested; - unsigned char priority; - unsigned char sio2; - char *value; -} load_image_request_t; - -static load_image_request_t *gArtReqList = NULL; -static load_image_request_t *gArtReqEnd = NULL; -static load_image_request_t *volatile gArtCurrentReq = NULL; -static volatile int gArtQueuedCount = 0; -static volatile int gArtActiveCount = 0; -static unsigned int gArtQueueEpoch = 1; -static int gArtNavActive = 0; - -@QUEUE_FUNCTIONS@ - -int main(void) -{ - int failed = 0; - - // Test 1: Sequence with speculative lookaheads then priority trio - // Suppose neighbor covers lookahead +1 and +2 are pushed as non-priority - load_image_request_t lookahead1 = { .value = "LOOKAHEAD_+1" }; - load_image_request_t lookahead2 = { .value = "LOOKAHEAD_+2" }; - artPush(&lookahead1); - artPush(&lookahead2); - - // Active selection arrives: COV, ICO, BG pushed with artPushPriority - load_image_request_t cov = { .value = "COV_SEL" }; - load_image_request_t ico = { .value = "ICO_SEL" }; - load_image_request_t bg = { .value = "BG_SEL" }; - artPushPriority(&cov); - artPushPriority(&ico); - artPushPriority(&bg); - - // Later neighbor lookahead -1 is pushed non-priority - load_image_request_t lookahead_prev = { .value = "LOOKAHEAD_-1" }; - artPush(&lookahead_prev); - - // Verify order in queue must be: COV_SEL -> ICO_SEL -> BG_SEL -> LOOKAHEAD_+1 -> LOOKAHEAD_+2 -> LOOKAHEAD_-1 - const char *expected_order[] = { - "COV_SEL", "ICO_SEL", "BG_SEL", "LOOKAHEAD_+1", "LOOKAHEAD_+2", "LOOKAHEAD_-1" - }; - int idx = 0; - for (load_image_request_t *cur = gArtReqList; cur; cur = cur->next) { - if (strcmp(cur->value, expected_order[idx]) != 0) { - printf("FAIL: at pos %d, expected %s, got %s\n", idx, expected_order[idx], cur->value); - failed = 1; - } - idx++; - } - if (idx != 6) { - printf("FAIL: expected 6 items in queue, found %d\n", idx); - failed = 1; - } - - // Verify bi-directional links (prev/next consistency) - load_image_request_t *prev = NULL; - for (load_image_request_t *cur = gArtReqList; cur; cur = cur->next) { - if (cur->prev != prev) { - printf("FAIL: prev pointer mismatch for %s\n", cur->value); - failed = 1; - } - prev = cur; - } - if (gArtReqEnd != prev) { - printf("FAIL: gArtReqEnd does not match tail\n"); - failed = 1; - } - - // Test 2: artPromote: promote LOOKAHEAD_+2 to priority - artPromote(&lookahead2); - // After promotion, lookahead2 should be appended to the priority block: - // COV_SEL -> ICO_SEL -> BG_SEL -> LOOKAHEAD_+2 -> LOOKAHEAD_+1 -> LOOKAHEAD_-1 - const char *expected_promoted[] = { - "COV_SEL", "ICO_SEL", "BG_SEL", "LOOKAHEAD_+2", "LOOKAHEAD_+1", "LOOKAHEAD_-1" - }; - idx = 0; - for (load_image_request_t *cur = gArtReqList; cur; cur = cur->next) { - if (strcmp(cur->value, expected_promoted[idx]) != 0) { - printf("FAIL: after promote at pos %d, expected %s, got %s\n", idx, expected_promoted[idx], cur->value); - failed = 1; - } - idx++; - } - - // Test 3: artPop drains in exact expected order - for (int i = 0; i < 6; i++) { - load_image_request_t *p = artPop(); - if (!p || strcmp(p->value, expected_promoted[i]) != 0) { - printf("FAIL pop %d: expected %s, got %s\n", i, expected_promoted[i], p ? p->value : "NULL"); - failed = 1; - } - } - if (gArtReqList != NULL || gArtReqEnd != NULL || gArtQueuedCount != 0) { - printf("FAIL: queue not empty after popping all elements\n"); - failed = 1; - } - - // Test 4: SIO2 defer behavior (Issue #340 protection) - load_image_request_t sio2_req = { .value = "SIO2_COVER", .sio2 = 1 }; - artPushPriority(&sio2_req); - gArtNavActive = 1; // user is holding D-pad - load_image_request_t *popped_sio2 = artPop(); - if (popped_sio2 != NULL) { - printf("FAIL: artPop must return NULL when SIO2 head is deferred during navigation\n"); - failed = 1; - } - gArtNavActive = 0; // user released D-pad - popped_sio2 = artPop(); - if (popped_sio2 != &sio2_req) { - printf("FAIL: artPop must return deferred SIO2 item once navigation idle\n"); - failed = 1; - } - - if (!failed) - printf("art priority queue: production functions FIFO priority tiering and SIO2 defer verified\n"); - - return failed; -} -''' - - -def compile_and_run(name, program): - with tempfile.TemporaryDirectory() as tmp: - src = Path(tmp) / (name + '.c') - exe = Path(tmp) / (name + ('.exe' if sys.platform == 'win32' else '')) - src.write_text(program, encoding='utf-8') - build = subprocess.run(['gcc', '-std=gnu99', '-Wall', '-Wno-unused-function', '-Wno-unused-variable', '-o', str(exe), str(src)], - capture_output=True, text=True, check=False) - if build.returncode != 0: - failures.append('%s harness did not compile:\n%s' % (name, build.stderr)) - return - run = subprocess.run([str(exe)], capture_output=True, text=True, check=False) - sys.stdout.write(run.stdout) - if run.returncode != 0: - failures.append('%s harness reported failures' % name) - - -if not failures: - compile_and_run('art_priority_queue', QUEUE_HARNESS.replace('@QUEUE_FUNCTIONS@', queue_functions)) - -if failures: - print('Art priority checks FAILED:') - for failure in failures: - print(' - ' + failure) - sys.exit(1) diff --git a/.github/scripts/test_art_queue_order.py b/.github/scripts/test_art_queue_order.py new file mode 100644 index 000000000..21fecbe03 --- /dev/null +++ b/.github/scripts/test_art_queue_order.py @@ -0,0 +1,284 @@ +"""Pin the art queue order that issue #772 depends on. + +#770 replaced "the newest priority request goes to the FRONT" with a FIFO priority tier and stopped +promoting a request that was already priority. While scrolling Coverflow, every cover passed stays +on screen as a neighbour and keeps its priority request alive, so the cover actually SELECTED ended +up behind all of them -- zack's "covers pop in late while scrolling" (#772). #773 restored the old +order; this test keeps it restored. + +Also pins the BG half of the same report. The "latest selection wins" abort for per-game +backgrounds used to be guarded by `cache->count == 1`, but initMutableImage floors EVERY per-game +art cache at 2 slots, so that guard made the abort dead code: an abandoned background's decode kept +running in front of the next cover. The abort now lives in cacheAbortOtherBackgrounds, which scans +every slot, and is tested here with a real 2-slot cache. + +Everything below is compiled from the production source text of src/texcache.c, not a copy. +""" +from pathlib import Path +import re +import subprocess +import sys +import tempfile + +root = Path(__file__).resolve().parents[2] +failures = [] + + +def read(rel): + return (root / rel).read_text(encoding='utf-8').replace('\r\n', '\n') + + +def function_text(source, where, signature): + match = re.search(r'^' + re.escape(signature) + r'[^;{]*\)\s*\{', source, re.M) + if match is None: + failures.append('%s: %s...) not found' % (where, signature)) + return '' + end = source.index('\n}', match.start()) + return source[match.start():end] + '\n}\n' + + +texcache = read('src/texcache.c') +themes = read('src/themes.c') + +# --- The cache floor that makes a `count == 1` guard dead code --------------------------------- +if not re.search(r'if \(cachePattern != NULL && cacheCount < 2\)\s*\n\s*cacheCount = 2;', themes): + failures.append('themes.c: per-game art caches are no longer floored at 2 slots -- re-check ' + 'whether cacheAbortOtherBackgrounds still needs to scan every slot') + +# --- The BG abort must be the tested helper, called for BG caches whatever their size ---------- +if re.search(r'cache->count\s*==\s*1\s*&&\s*cache->suffix', texcache): + failures.append('texcache.c: the BG abort is guarded by cache->count == 1 again -- BG caches ' + 'have at least 2 slots, so it would never fire') +if not re.search(r'strcmp\(cache->suffix, "BG"\) == 0\)\s*\n\s*cacheAbortOtherBackgrounds\(cache, value\);', + texcache): + failures.append('texcache.c: cacheGetTextureEx no longer calls cacheAbortOtherBackgrounds for ' + 'BG caches') + +# --- Coverflow still holds a new background until its cover has had its turn ----------------- +if 'coverflowCoverSettled' not in themes or \ + 'texture = getGameImageCached(gameImage->cache, &item->item);' not in themes: + failures.append('themes.c: the Coverflow background gate (coverflowCoverSettled) is gone -- a ' + 'full-screen BG read would start ahead of the carousel covers again (#772)') +if re.search(r'texture = getGameImageTextureEx\(gameImage->cache, menu->item->userdata, &item->item, 1\);\s*\n\s*\} else \{', + themes): + failures.append('themes.c: the per-game background is requested as a PRIORITY image again (#772)') + +functions = ''.join(function_text(texcache, 'src/texcache.c', sig) for sig in ( + 'static void artPush(', + 'static void artPushFront(', + 'static void artPromote(', + 'static load_image_request_t *artPop(', + 'static void cacheAbortOtherBackgrounds(', +)) + +HARNESS = r''' +#include +#include + +static void DIntr(void) {} +static void EIntr(void) {} + +typedef struct load_image_request { + struct load_image_request *next; + struct load_image_request *prev; + unsigned int queueEpoch; + volatile int abortRequested; + unsigned char sio2; + char *value; +} load_image_request_t; + +typedef struct { + void *qr; + char key[64]; +} cache_entry_t; + +typedef struct { + int count; + cache_entry_t *content; +} image_cache_t; + +static load_image_request_t *gArtReqList = NULL; +static load_image_request_t *gArtReqEnd = NULL; +static load_image_request_t *volatile gArtCurrentReq = NULL; +static volatile int gArtQueuedCount = 0; +static volatile int gArtActiveCount = 0; +static unsigned int gArtQueueEpoch = 1; +static volatile int gArtNavActive = 0; + +@FUNCTIONS@ + +static int failed = 0; + +static void expectOrder(const char *what, const char **want, int n) +{ + int i = 0; + load_image_request_t *prev = NULL, *cur; + + for (cur = gArtReqList; cur; cur = cur->next, i++) { + if (i >= n || strcmp(cur->value, want[i]) != 0) { + printf("FAIL %s: position %d is %s, expected %s\n", what, i, cur->value, i < n ? want[i] : "(end)"); + failed = 1; + return; + } + if (cur->prev != prev) { + printf("FAIL %s: prev link broken at %s\n", what, cur->value); + failed = 1; + } + prev = cur; + } + if (i != n) { + printf("FAIL %s: %d queued, expected %d\n", what, i, n); + failed = 1; + } + if (gArtReqEnd != prev) { + printf("FAIL %s: tail pointer does not match the last request\n", what); + failed = 1; + } +} + +static void drain(void) +{ + while (artPop() != NULL) + gArtCurrentReq = NULL; + gArtActiveCount = 0; +} + +int main(void) +{ + // 1. Scrolling Coverflow A -> B -> C -> D. Each newly selected cover is a priority request. The + // ones already passed stay queued (they are still on screen as neighbours). The cover the user + // is LOOKING AT must be read first -- under #770's FIFO tier it was read last. + load_image_request_t a = {.value = "COV_A"}, b = {.value = "COV_B"}; + load_image_request_t c = {.value = "COV_C"}, d = {.value = "COV_D"}; + artPushFront(&a); + artPushFront(&b); + artPushFront(&c); + artPushFront(&d); + { + const char *want[] = {"COV_D", "COV_C", "COV_B", "COV_A"}; + expectOrder("scroll A->D", want, 4); + } + load_image_request_t *first = artPop(); + if (first != &d) { + printf("FAIL scroll A->D: the selected cover (COV_D) must be read first, got %s\n", first ? first->value : "NULL"); + failed = 1; + } + gArtCurrentReq = NULL; + drain(); + + // 2. A neighbour already warming at the back of the queue becomes the selection: promote moves + // it to the front in one splice, and the tail pointer follows when it was the tail. + load_image_request_t n1 = {.value = "NEIGHBOUR_1"}, n2 = {.value = "NEIGHBOUR_2"}; + load_image_request_t n3 = {.value = "NEIGHBOUR_3"}; + artPush(&n1); + artPush(&n2); + artPush(&n3); + artPromote(&n3); + { + const char *want[] = {"NEIGHBOUR_3", "NEIGHBOUR_1", "NEIGHBOUR_2"}; + expectOrder("promote the tail", want, 3); + } + artPromote(&n1); // from the middle + { + const char *want[] = {"NEIGHBOUR_1", "NEIGHBOUR_3", "NEIGHBOUR_2"}; + expectOrder("promote the middle", want, 3); + } + artPromote(&n1); // already at the head: no-op + { + const char *want[] = {"NEIGHBOUR_1", "NEIGHBOUR_3", "NEIGHBOUR_2"}; + expectOrder("promote the head", want, 3); + } + drain(); + + // 3. The request the worker is executing is never spliced back into the queue. + load_image_request_t run = {.value = "RUNNING"}, q1 = {.value = "QUEUED"}; + artPush(&run); + artPush(&q1); + if (artPop() != &run) { + printf("FAIL running: expected RUNNING to pop first\n"); + failed = 1; + } + artPromote(&run); + { + const char *want[] = {"QUEUED"}; + expectOrder("promote the running request", want, 1); + } + gArtCurrentReq = NULL; + drain(); + + // 4. SIO2 defer (#340): an SIO2 head is not started while a direction is held. + load_image_request_t sio2 = {.value = "SIO2_COVER", .sio2 = 1}; + artPushFront(&sio2); + gArtNavActive = 1; + if (artPop() != NULL) { + printf("FAIL sio2: an SIO2 cover must not start while a direction is held\n"); + failed = 1; + } + gArtNavActive = 0; + if (artPop() != &sio2) { + printf("FAIL sio2: the deferred SIO2 cover must start once navigation stops\n"); + failed = 1; + } + gArtCurrentReq = NULL; + drain(); + + // 5. Latest background wins, in a real 2-slot cache (the floor initMutableImage applies). A + // background still pending for another game is aborted; the one for the game now selected, an + // idle slot, and a slot with no key are left alone. + load_image_request_t oldBg = {.value = "OLD_GAME"}, newBg = {.value = "NEW_GAME"}; + cache_entry_t slots[2]; + image_cache_t bg = {2, slots}; + memset(slots, 0, sizeof(slots)); + slots[0].qr = &oldBg; + strcpy(slots[0].key, "OLD_GAME"); + slots[1].qr = &newBg; + strcpy(slots[1].key, "NEW_GAME"); + cacheAbortOtherBackgrounds(&bg, "NEW_GAME"); + if (!oldBg.abortRequested) { + printf("FAIL bg: the abandoned background (slot 0 of 2) was not aborted\n"); + failed = 1; + } + if (newBg.abortRequested) { + printf("FAIL bg: the selected game's own background was aborted\n"); + failed = 1; + } + oldBg.abortRequested = 0; + slots[1].qr = NULL; // idle slot + slots[0].key[0] = '\0'; // pending request with no reunion key: nothing to compare, leave it + cacheAbortOtherBackgrounds(&bg, "NEW_GAME"); + if (oldBg.abortRequested) { + printf("FAIL bg: a request without a key must not be aborted\n"); + failed = 1; + } + + if (!failed) + printf("art queue order: selected cover first, promote-to-front, SIO2 defer and latest-background-wins verified\n"); + return failed; +} +''' + + +def compile_and_run(name, program): + with tempfile.TemporaryDirectory() as tmp: + src = Path(tmp) / (name + '.c') + exe = Path(tmp) / (name + ('.exe' if sys.platform == 'win32' else '')) + src.write_text(program, encoding='utf-8') + build = subprocess.run(['gcc', '-std=gnu99', '-Wall', '-Wno-unused-function', '-o', str(exe), str(src)], + capture_output=True, text=True, check=False) + if build.returncode != 0: + failures.append('%s harness did not compile:\n%s' % (name, build.stderr)) + return + run = subprocess.run([str(exe)], capture_output=True, text=True, check=False) + sys.stdout.write(run.stdout) + if run.returncode != 0: + failures.append('%s harness reported failures' % name) + + +if not failures: + compile_and_run('art_queue_order', HARNESS.replace('@FUNCTIONS@', functions)) + +if failures: + print('Art queue order checks FAILED:') + for failure in failures: + print(' - ' + failure) + sys.exit(1) diff --git a/.github/workflows/flavours.yml b/.github/workflows/flavours.yml index 59d80ede0..de11e111f 100644 --- a/.github/workflows/flavours.yml +++ b/.github/workflows/flavours.yml @@ -103,8 +103,8 @@ jobs: run: python3 .github/scripts/test_bdm_scan_retry.py - name: Check the IO worker skips duplicate module passes and APPS rebuilds on L3 run: python3 .github/scripts/test_io_queue_trims.py - - name: Check unified art priority queue and active trio ordering - run: python3 .github/scripts/test_art_priority_queue.py + - name: Check the selected cover is read first and an abandoned background is aborted (#772) + run: python3 .github/scripts/test_art_queue_order.py udpfs-host-tests: runs-on: ubuntu-24.04 diff --git a/src/texcache.c b/src/texcache.c index 690592436..45f891f1f 100644 --- a/src/texcache.c +++ b/src/texcache.c @@ -52,8 +52,6 @@ typedef struct load_image_request // streaming PNG reader. For the device #340 is actually about, NOT ISSUING the read is the only // lever -- which is what the SIO2 defer in artPop is for. volatile int abortRequested; - // Priority level (1 = active selection tier, 0 = speculative lookahead) - unsigned char priority; // Resolved on the GUI THREAD at enqueue: does this cover ride SIO2, the controller's own bus? // Not resolvable on the worker -- answering it reads the support's private device data, which a // background rescan rewrites underneath. One byte, copied in, is immune. @@ -477,7 +475,6 @@ static void artPush(load_image_request_t *req) req->next = NULL; req->prev = gArtReqEnd; req->queueEpoch = gArtQueueEpoch; - req->priority = 0; if (gArtReqEnd) gArtReqEnd->next = req; else @@ -487,37 +484,17 @@ static void artPush(load_image_request_t *req) EIntr(); } -static void artPushPriority(load_image_request_t *req) +static void artPushFront(load_image_request_t *req) { DIntr(); + req->prev = NULL; + req->next = gArtReqList; req->queueEpoch = gArtQueueEpoch; - req->priority = 1; - - if (gArtReqList == NULL) { - req->prev = NULL; - req->next = NULL; - gArtReqList = req; - gArtReqEnd = req; - } else if (!gArtReqList->priority) { - // Queue head is non-priority; insert req at the head - req->prev = NULL; - req->next = gArtReqList; + if (gArtReqList) gArtReqList->prev = req; - gArtReqList = req; - } else { - // Walk priority items to append at the end of the priority section (FIFO within priority) - load_image_request_t *curr = gArtReqList; - while (curr->next != NULL && curr->next->priority) { - curr = curr->next; - } - req->prev = curr; - req->next = curr->next; - if (curr->next != NULL) - curr->next->prev = req; - else - gArtReqEnd = req; - curr->next = req; - } + else + gArtReqEnd = req; + gArtReqList = req; gArtQueuedCount++; EIntr(); } @@ -528,39 +505,25 @@ static void artPromote(load_image_request_t *req) return; DIntr(); - // Guard against promoting the request the worker is already executing, or one already marked priority. - if (gArtCurrentReq != req && req->queueEpoch == gArtQueueEpoch && !req->priority) { - req->priority = 1; - - if (req->prev != NULL) { - load_image_request_t *prev = req->prev; - load_image_request_t *next = req->next; + // The selected cover can be a warmed neighbour buried in a deep queue. This used to scan the + // singly-linked FIFO under DIntr(), making one selection's priority handoff O(queue depth) and + // extending the interrupts-off window as browsing filled cache slots. The intrusive back link + // keeps this splice O(1); no allocation, free, or device work occurs in the bracket. + // Guard against promoting the request the worker is already executing, or one already at head. + if (gArtCurrentReq != req && req->queueEpoch == gArtQueueEpoch && req->prev != NULL) { + load_image_request_t *prev = req->prev; + load_image_request_t *next = req->next; + + prev->next = next; + if (next) + next->prev = prev; + else + gArtReqEnd = prev; - prev->next = next; - if (next) - next->prev = prev; - else - gArtReqEnd = prev; - - if (!gArtReqList->priority) { - req->prev = NULL; - req->next = gArtReqList; - gArtReqList->prev = req; - gArtReqList = req; - } else { - load_image_request_t *curr = gArtReqList; - while (curr->next != NULL && curr->next->priority) { - curr = curr->next; - } - req->prev = curr; - req->next = curr->next; - if (curr->next != NULL) - curr->next->prev = req; - else - gArtReqEnd = req; - curr->next = req; - } - } + req->prev = NULL; + req->next = gArtReqList; + gArtReqList->prev = req; + gArtReqList = req; } EIntr(); } @@ -1357,6 +1320,22 @@ static int cacheResolveArtArchive(item_list_t *list, const char *value, char *pa return 0; } +// Abort every background still queued or loading for a game other than `value`. Scans EVERY slot: +// initMutableImage floors each per-game art cache at 2, so the `cache->count == 1` guard this used to +// sit behind made it dead code, and an abandoned background kept its decode running in front of the +// next cover (#772). Marking it aborted rather than dropping it keeps one owner for the request: the +// worker releases it on its own terms, whether it is still queued or already inside the read. +static void cacheAbortOtherBackgrounds(image_cache_t *cache, const char *value) +{ + int i; + + for (i = 0; i < cache->count; i++) { + cache_entry_t *entry = &cache->content[i]; + if (entry->qr != NULL && entry->key[0] != '\0' && strcmp(entry->key, value) != 0) + ((load_image_request_t *)entry->qr)->abortRequested = 1; + } +} + GSTEXTURE *cacheGetTextureEx(image_cache_t *cache, item_list_t *list, int *cacheId, int *UID, char *value, int isPriority) { int i, rtime; @@ -1506,22 +1485,13 @@ GSTEXTURE *cacheGetTextureEx(image_cache_t *cache, item_list_t *list, int *cache return NULL; } - // LATEST SELECTION WINS on a one-slot background cache. With one slot there is no eviction to - // fall back on: a background queued or loading for a selection the user has already left owns - // the only slot until it finishes, so the cover for where they ARE cannot even be requested - // until the abandoned one has been read off the device in full -- seconds, on the evidence - // above. The stale-stamp cancellation in cacheLoadImage cannot help, because the row it belongs - // to may still be on screen and still stamping. - // - // LATEST SELECTION WINS on background cache. With a focused game selection, any background - // queued or loading for an abandoned selection is aborted so the new selection is not delayed. - if (cache->suffix != NULL && strcmp(cache->suffix, "BG") == 0) { - for (int b = 0; b < cache->count; b++) { - cache_entry_t *bentry = &cache->content[b]; - if (bentry->qr != NULL && bentry->key[0] != '\0' && strcmp(bentry->key, value) != 0) - ((load_image_request_t *)bentry->qr)->abortRequested = 1; - } - } + // LATEST SELECTION WINS on the background cache. A background queued or loading for a selection + // the user has already left holds its slot until it finishes, so the cover for where they ARE + // waits behind somebody else's scenery -- seconds, on the evidence above. The stale-stamp + // cancellation in cacheLoadImage cannot help, because the row it belongs to may still be on + // screen and still stamping. + if (cache->suffix != NULL && strcmp(cache->suffix, "BG") == 0) + cacheAbortOtherBackgrounds(cache, value); cache_entry_t *currEntry, *oldestEntry = NULL; @@ -1659,10 +1629,11 @@ GSTEXTURE *cacheGetTextureEx(image_cache_t *cache, item_list_t *list, int *cache // switch would otherwise do. cache->activeRequests++; - // Priority requests push into the high-priority selection tier (FIFO within priority) unless + // artPush takes its own DIntr bracket and does the gArtQueuedCount++ inside it. It cannot + // fail: no allocation, no queue cap. Priority requests push to the head of the FIFO unless // an SIO2 request is being made while a direction is held (which falls back to tail to protect #340). if (isPriority && !(req->sio2 && gArtNavActive)) - artPushPriority(req); + artPushFront(req); else artPush(req); cacheWakeArtWorker(); diff --git a/src/themes.c b/src/themes.c index 72a016e37..eb50b09c7 100644 --- a/src/themes.c +++ b/src/themes.c @@ -228,8 +228,6 @@ static int thmElemSkipsDevice(const theme_element_t *elem, int iconId) return 0; } -static theme_element_t *thmFindElemBySuffix(theme_elems_t *elems, const char *suffix, theme_element_t *like); - // Nav-side twin of drawItemsList's gate: pick the FILTERED ItemsList that covers this page, else // the family's slot element (fallback). menusys assigns gTheme->itemsList through this so paging // math (displayedItems) always reads the exact element whose rows are on screen. @@ -1120,17 +1118,27 @@ static void drawGameImage(struct menu_list *menu, struct submenu_list *item, con if (!gEnableDiscArt && isDiscArtCache(gameImage->cache)) return; - // Active selection art trio (COV, ICO, BG): all three share Tier 1 priority. - // Background is drawn first in painter's order, so it pre-requests COV and ICO before - // requesting BG. In the FIFO priority queue, this stacks them in the optimal decode order: - // COV (fast box art) -> ICO (fast disc) -> BG (wallpaper). All three complete together - // for the selected game before any off-screen neighbor lookaheads run. + // A per-game BACKGROUND must never be REQUESTED before the cover. This function draws both, + // and the theme's Background element is the FIRST in the list (painter's order), so on the + // settle frame its request reached the empty queue first and became the executing head -- + // and the head cannot be jumped by any amount of priority, because the worker is already + // inside it. A full-screen PNG is ~5x the pixels of a cover, so the cover the user is + // actually waiting for sat behind the better part of a second of somebody else's scenery. + // That is the whole gap to official OPL, whose built-in theme has no per-game background on + // the main page at all (it uses a static image), and it explains why a device with no _BG + // art in its set feels fine while one with a full art set does not. + // + // So hold the background back: request it only once the cover has had its turn -- an extra + // idle margin beyond the art delay AND an idle art path. Until then draw whatever is already + // cached, which keeps a background that IS loaded on screen instead of flickering to the + // default. Nothing is lost but the order. int isBackground = (drawElem->type == ELEM_TYPE_BACKGROUND); GSTEXTURE *texture; + int coverflowCoverSettled = 1; if (isBackground) { - // Prioritize the highlighted game's cover (COV) and disc (ICO) before the background (BG) - // so the small cover and disc are read and displayed first before starting the larger background read. + // Prioritize the highlighted game's cover before the background so the small cover + // is read and displayed first before starting the larger background read. if (gTheme != NULL && !item->item.isFolder) { if (gTheme->itemsList != NULL && gTheme->itemsList->extended != NULL) { items_list_t *itemsList = (items_list_t *)gTheme->itemsList->extended; @@ -1149,32 +1157,33 @@ static void drawGameImage(struct menu_list *menu, struct submenu_list *item, con if (cfElem != NULL && cfElem->extended != NULL) { mutable_image_t *cfImg = (mutable_image_t *)cfElem->extended; if (cfImg != NULL && cfImg->cache != NULL) { - getGameImageTextureEx(cfImg->cache, menu->item->userdata, &item->item, 1); - } - } - } - - // Pre-request the highlighted game's disc/icon (ICO) if present in this family - theme_elems_t *fam = drawElem->family ? drawElem->family : (gTheme ? &gTheme->mainElems : NULL); - if (gEnableDiscArt && fam != NULL) { - struct theme_element *icoElem = thmFindElemBySuffix(fam, "ICO", NULL); - if (icoElem != NULL) { - icoElem = thmGetElemForItem(menu, item, icoElem); - if (icoElem != NULL && icoElem->extended != NULL) { - mutable_image_t *icoImg = (mutable_image_t *)icoElem->extended; - if (icoImg != NULL && icoImg->cache != NULL) - getGameImageTextureEx(icoImg->cache, menu->item->userdata, &item->item, 1); + GSTEXTURE *coverTexture = getGameImageTextureEx(cfImg->cache, menu->item->userdata, &item->item, 1); + + // Coverflow draws after the Background element. If its selected cover is still + // pending, starting a full-screen BG read here would occupy the art worker before + // the carousel gets to enqueue its visible neighbours. Hold only this background + // request for that short window. A loaded cover, disabled art, or a confirmed + // absent cover (-2) releases the gate immediately. + if (gEnableArt && coverTexture == NULL && cfImg->cache->userId >= 0 && + cfImg->cache->userId < gTheme->gameCacheCount && item->item.cache_id != NULL && + item->item.cache_id[cfImg->cache->userId] != -2) + coverflowCoverSettled = 0; } } } } - // High priority (isPriority = 1) for the active selection's background as well. - // In the FIFO priority queue, COV and ICO run first (fast decodes), followed immediately - // by BG, all ahead of off-screen neighbor lookaheads. - texture = getGameImageTextureEx(gameImage->cache, menu->item->userdata, &item->item, 1); + // List mode keeps master's immediate background request. Coverflow is different: its + // Background element is painted before the carousel, so on a cold selection the large BG read + // could start before the side covers even reach the queue. Draw an already-cached background + // while the selected cover settles; the same frame's Coverflow draw then gets first claim on + // visible-neighbour reads. This is admission ordering, not an artificial frame delay. + if (coverflowCoverSettled) + texture = getGameImageTexture(gameImage->cache, menu->item->userdata, &item->item); + else + texture = getGameImageCached(gameImage->cache, &item->item); } else { - // PRIORITY: the highlighted game's own cover or icon. + // PRIORITY: the highlighted game's own cover, the one image the user is looking for. texture = getGameImageTextureEx(gameImage->cache, menu->item->userdata, &item->item, 1); }