[pull] master from libretro:master#974
Merged
pull[bot] merged 17 commits intoAlexandre1er:masterfrom Apr 28, 2026
Merged
Conversation
… + CI
Three independent issues found during a focused audit of gfx/drivers/
vulkan.c and gfx/common/vulkan_common.c, all in the same memory-safety
class (writes past compile-time-sized arrays or undersized
allocations). Submitting together because they share the same audit
context and none stands alone particularly well. Adds AddressSanitizer
regression tests for each under samples/gfx/, wired into a new
.github/workflows/Linux-samples-gfx.yml CI job.
* vulkan_find_device_extensions: heap overflow
(gfx/common/vulkan_common.c)
The required-extension list is currently written twice -- once via
memcpy at the top of the body, then again in a per-element loop
that appears to be left over from an earlier refactor. The
instance-side counterpart vulkan_find_instance_extensions only
logs in its loop, confirming the divergence. The redundant write
consumes more slots in the caller's `enabled` buffer than the slot
accounting expects.
Inside vulkan_context_create_device_wrapper the buffer is sized
for (info.enabledExtensionCount + ARRAY_SIZE(required) +
ARRAY_SIZE(optional)) entries, but the function actually writes
(enabledExtensionCount + 2*required + optional) entries
worst-case. With current values (1 required, 3 optional) that is
a one-element heap overflow at the end of the malloc'd block
whenever a libretro core uses the Vulkan HW context-negotiation
interface and the GPU supports at least one optional extension.
The other call site in vulkan_context_init_device uses a stack
buffer of size 8 and stays inside it today, but only by a margin
of three.
The duplicate writes also produce duplicate entries in
ppEnabledExtensionNames passed to vkCreateDevice, which the spec
forbids (VUID-VkDeviceCreateInfo-ppEnabledExtensionNames-01840);
strict validation layers will have been complaining.
Drop the per-element loop -- the early vulkan_find_extensions()
check at the top of the function already validates that all
required extensions are present, so the per-element re-check has
never done any work. Keep the debug log.
* vulkan_create_swapchain: OOB writes from unclamped image count
(gfx/common/vulkan_common.c)
vulkan_create_swapchain calls vkGetSwapchainImagesKHR twice --
first to query the count, then to fill swapchain_images[] -- and
never clamps the count between the two calls. swapchain_images,
swapchain_fences, the four swapchain_*_semaphores arrays,
readback.staging[] and vk->swapchain[] are all sized at compile
time to VULKAN_MAX_SWAPCHAIN_IMAGES (8). If a driver returns
more images than that the second vkGetSwapchainImagesKHR writes
past swapchain_images[], and every loop bounded by
num_swapchain_images (~12 sites across init/deinit/textures/
buffers/descriptor pools/command buffers/readback and direct
vk->swapchain[i] accesses) walks past its array.
Two clamps. First, cap desired_swapchain_images to
VULKAN_MAX_SWAPCHAIN_IMAGES before vkCreateSwapchainKHR so we
never ask a well-behaved driver for more images than we can
hold. Second, clamp num_swapchain_images between the two
vkGetSwapchainImagesKHR calls to handle drivers that return
more images than were requested -- the spec permits this.
* vulkan_create_texture: 32-bit overflow in staging-buffer sizing
(gfx/drivers/vulkan.c)
buffer_width = width * bpp and buffer_info.size = buffer_width *
height are both unsigned * unsigned in 32-bit before being
widened to VkDeviceSize on assignment. With dimensions large
enough to wrap (e.g. width=65536, height=16385, bpp=4 ->
0x1_0004_0000 wraps to 0x40000), the staging buffer is allocated
at the wrapped (small) size while the per-row upload memcpy loop
later in the same function walks the unmodified width x height.
The mapped region is smaller than what the loop writes, so the
memcpy walks past the mapping into adjacent heap memory.
Reachable from libretro cores supplying oversized
retro_framebuffer dimensions and from vulkan_load_texture /
vulkan_set_texture_frame, which take dimensions originating in
image decoders.
Compute the staging-buffer size in 64-bit, and widen the upload
loop's stride and per-row copy size to size_t. VkDeviceSize is
already 64-bit so the assignment to buffer_info.size is now
correct end-to-end. The loop's pointer math was already
implicitly widened on 64-bit hosts but is now unconditionally
correct on 32-bit hosts (3DS, Vita, PSP, Wii, Wii U, older
Android, 32-bit Windows) as well.
* samples/gfx/, .github/workflows/Linux-samples-gfx.yml
Three regression tests, one per fix above, following the pattern
established by the security regression tests under samples/tasks/
(archive_name_safety_test, http_method_match_test,
video_shader_wildcard_test, input_remap_bounds_test,
bsv_replay_bounds_test, bps_patch_bounds_test). Each test keeps
a verbatim copy of the relevant post-fix predicate, demarcated by
`=== verbatim copy ===` markers; an `IMPORTANT: ... must follow`
comment documents the convention. Plain C, asserts manually,
exits nonzero on failure -- matches the harness the existing
Linux-samples-{tasks,libretro-{common,db}}-samples.yml workflows
understand.
vulkan_extension_count_test (5 cases): create_device_wrapper-
shaped caller, init_device-shaped caller, GPU supporting no
optional extensions, GPU missing a required extension, and the
prepushed-via-negotiation-interface case. Verified pre/post-
patch discrimination: under the pre-fix double-write shape the
create_device_wrapper case ASan-aborts with heap-buffer-overflow.
vulkan_swapchain_clamp_test (5 cases): well-behaved 3-image
driver, at-capacity boundary, 9-image (MAX+1) driver,
pathological 64-image driver, and the request-side cap across
six input values. Verified pre/post-patch discrimination: under
the pre-fix no-clamp shape the 9-image case ASan-aborts on the
second vkGetSwapchainImagesKHR fill.
vulkan_texture_size_test (6 cases): typical 320x240, 4K RGBA,
the smoking-gun 65536x16385x4 wrap case, an assorted-shapes
table, the per-row upload-loop strides under an ASan-instrumented
buffer, and a 64-bit-result sanity check. Verified pre/post-
patch discrimination: under the pre-fix 32-bit arithmetic the
overflow cases produce arithmetic mismatches against the 64-bit
reference.
Linux-samples-gfx.yml is a near-verbatim copy of
Linux-samples-tasks.yml, runs each test under
-fsanitize=address with a per-step explanation of what the test
is regression-testing and the `verbatim copy must follow`
reminder. ~3 seconds per test on Ubuntu 24 with system gcc.
Found during a continued audit of gfx/drivers_shader/shader_vulkan.cpp and slang_process.cpp after the v3 vulkan-driver fixes (da5e650). Adds an AddressSanitizer regression test under samples/gfx/ slang_texture_index_bounds/ wired into the existing Linux-samples-gfx CI workflow as a fourth step. slang_process.cpp's add_active_buffer_ranges() and the direct sampler- binding loop in slang_reflect() both consume an unsigned array_index parsed from the suffix of arrayed semantic names like `OriginalHistory42` / `PassFeedback9` / `User7`. The parsing happens in slang_name_to_texture_semantic_array() via strtoul with no upper bound. That index then flows into resize_minimum() at slang_process.c line 1116 and the direct-binding write at line 1701, growing reflection->semantic_textures[sem] to (index + 1) entries. PASS_OUTPUT was already bounded against reflection->pass_number, with a dedicated "Non causal filter chain detected" diagnostic; ORIGINAL_HISTORY, PASS_FEEDBACK and USER had no upper bound. A malicious slang shader declaring layout(set = 0, binding = 1) uniform sampler2D OriginalHistory4294967294; makes std::vector::resize() request ~128 GiB on a 64-bit host, which throws std::bad_alloc. The exception propagates out of slang_reflect_spirv() up to vulkan_filter_chain_create_from_preset() with no try/catch in between, and the unhandled C++ exception terminates RetroArch. Reachable from any slang preset -- preset packs are downloaded via the Online Updater and shipped third-party. Severity is DoS rather than memory corruption, but the threat class mirrors the recently-fixed .bsv replay file, .bps soft-patch, and image-decoder bugs in 7335b37: a downloadable file that an end user opens, leading to a guaranteed process termination. * gfx/drivers_shader/slang_process.cpp Lift the existing PASS_OUTPUT bound check into a small helper validate_texture_semantic_index() and extend it to cover all arrayed semantics with their natural caps: PASS_OUTPUT -> reflection->pass_number (unchanged) PASS_FEEDBACK -> GFX_MAX_SHADERS (64) ORIGINAL_HISTORY -> GFX_MAX_FRAME_HISTORY (128) USER -> GFX_MAX_TEXTURES (64) Non-arrayed semantics (Original, Source) carry index 0 by construction in slang_name_to_texture_semantic_array() and accept unconditionally. PASS_OUTPUT keeps its dedicated "Non causal filter chain detected" log verbiage so existing shader-author muscle memory and log-grepping scripts continue to work; the other semantics use a uniform "Texture semantic <name> index #N exceeds bound (<cap_label> = <cap>)" message that names both the offending index and the cap that rejected it. The helper is called from both call sites that lead into a resize_minimum(): - add_active_buffer_ranges() (UBO uniform path), replacing the inline PASS_OUTPUT-only check. - slang_reflect()'s direct sampler-binding loop (the SLANG_ INVALID_TEXTURE_SEMANTIC short-circuit is preserved). * samples/gfx/slang_texture_index_bounds/, .github/workflows/Linux-samples-gfx.yml Regression test following the same convention as the v3 vulkan/ tests in this directory and the security regression tests under samples/tasks/. Keeps a verbatim copy of validate_texture_semantic_index() demarcated by `=== verbatim copy ===` markers, with an `IMPORTANT: ... must follow` comment. Plain C, asserts manually, exits nonzero on failure. 21 cases across six probe groups: legitimate-in-bounds across all four arrayed semantics, at-cap rejections, the smoking-gun UINT32_MAX-1 indices on each path including the previously- unbounded ORIGINAL_HISTORY/PASS_FEEDBACK/USER ones, log-dispatch consistency, non-arrayed (ORIGINAL/SOURCE) accept-everything, and the PASS_OUTPUT-in-pass-0 reject-everything case. Verified pre/post-patch discrimination: under the pre-fix PASS_OUTPUT-only shape the 7 cases that exercise the new bounds fail (3 boundary + 3 smoking-gun + 1 ORIGINAL_HISTORY at cap); PASS_OUTPUT cases continue to pass. Wires into the existing Linux-samples-gfx.yml workflow as a fourth step, modeled on the three vulkan/ steps from da5e650 with the same per-step explanation of what the test is regression-testing and the "verbatim copy must follow" reminder.
Found during the v2 audit of gfx/common/vulkan_common.c, deferred from the v3 patch (da5e650) which addressed the three high-severity heap-corruption findings. Adds an AddressSanitizer + LSan regression test under samples/gfx/vulkan_mailbox_init_leak/ wired into the existing Linux-samples-gfx CI workflow as a fifth step. vulkan_emulated_mailbox_init has three sequential allocations (scond_new, slock_new, sthread_create) and on any of the latter two failures returns `false` directly, leaking the already- allocated cond and/or lock. Both production call sites in vulkan_create_swapchain ignore the return value -- so an init failure also leaves vk->mailbox.lock == NULL and vk->mailbox.cond == NULL while VK_DATA_FLAG_EMULATING_MAILBOX is still set, setting up a NULL-deref the next time vulkan_acquire_next_image() routes into the emulated path (slock_lock(NULL) inside vulkan_emulated_mailbox_acquire_next_image()). The leak itself is small (the libretro-common slock_t / scond_t each hold a pthread mutex / cond worth of data); the trigger requires scond_new or slock_new to fail, which means OOM or pthread resource exhaustion. Realistic mainly on memory- constrained devices (older Android, Vita, 3DS, Wii U) where filter-chain teardown / re-init under thrashing memory pressure can put the system in this state. * gfx/common/vulkan_common.c Route every early-failure path in vulkan_emulated_mailbox_init through `goto error` to a single cleanup that calls vulkan_emulated_mailbox_deinit(). The deinit function is null-safe (each free is guarded by an `if (mailbox->X)` check) and ends with a memset, so the deinit-on-failure shape matches the deinit-on-shutdown shape exactly -- the two production callers that ignore the return value get a self-healing failure mode. The post-deinit memset also clears mailbox->swapchain, which trips the existing guard at vulkan_acquire_next_image: if (vk->mailbox.swapchain == VK_NULL_HANDLE) err = VK_ERROR_OUT_OF_DATE_KHR; closing the NULL-deref crash that the bare `return false` left open. No caller-side changes needed. * samples/gfx/vulkan_mailbox_init_leak/, .github/workflows/Linux-samples-gfx.yml Regression test following the convention of the v3 vulkan/ tests, the v4 slang test, and the security regression tests under samples/tasks/. Keeps verbatim copies of both vulkan_emulated_mailbox_init() and vulkan_emulated_mailbox_ deinit() demarcated by `=== verbatim copy ===` markers, with an `IMPORTANT: ... must follow` comment. Plain C, asserts manually, exits nonzero on failure. Five probes: scond fails first (was already correct, baseline case); slock fails second (pre-fix: scond leak); sthread fails third (pre-fix: scond + slock leak); full success (sanity check on alloc/free pairing post-deinit); and an explicit __lsan_do_recoverable_leak_check() call after running all three failure stages. Mocks scond_new / slock_new / sthread_create with controllable failure injection so each stage can be exercised deterministically. Verified pre/post-patch discrimination: under the pre-fix `return false` shape, ASan's LeakSanitizer reports 384 bytes leaked across 6 allocations (1 scond from slock-fails + 2 alloc/leak pairs from sthread-fails + 3 from the lsan-check probe), each with a full backtrace pointing into vulkan_emulated_mailbox_init. Under the post-fix shape all five probes pass clean. The CI step runs with ASAN_OPTIONS=detect_leaks=1 so leaks fail the run with a non-zero exit.
Found during the audit of the Vulkan context shims after f5c7df5. Adds an AddressSanitizer regression test under samples/gfx/ vulkan_ctx_double_free/ wired into the Linux-samples-gfx CI workflow as a sixth step. Three of the five Vulkan context drivers' set_video_mode hooks called their own destroy() (or destroy_resources() + free()) on ctx_data before returning false: wayland_vk_ctx.c:218 gfx_ctx_wl_destroy(data) w_vk_ctx.c:229 gfx_ctx_w_vk_destroy(data) x_vk_ctx.c:489-499 destroy_resources + free The caller in gfx/drivers/vulkan.c::vulkan_init treats a false return from set_video_mode (line 4938) as a failed in-flight vk_t construction and `goto error`s into vulkan_free() (line 5120), which at line 4665 calls vk->ctx_driver->destroy(vk->ctx_data); -- on the very pointer the inner set_video_mode just freed. The ctx_driver->destroy hook then walks the freed struct (gfx_ctx_wl_destroy_resources / gfx_ctx_x_vk_destroy_resources dereference fields off the freed pointer) and free()s the same pointer a second time. On glibc with default malloc the second free is detected (`double free or corruption`) and the process aborts. Under jemalloc / mimalloc / older glibc the second free silently corrupts the heap, with a write-what-where primitive controlled by the allocator's freelist layout. Reachability: vulkan_surface_create() failure (missing extension or driver-level Vulkan error), Wayland's set_video_mode_common_* helpers failing, X11's XGetVisualInfo returning NULL or x11_input_ctx_new failing, win32_set_video_mode failing. None are common but all are reachable on misconfigured systems, headless tests, or CI environments without a working display. * gfx/drivers_context/wayland_vk_ctx.c * gfx/drivers_context/w_vk_ctx.c * gfx/drivers_context/x_vk_ctx.c Drop the destroy/free call from each set_video_mode error path. Cocoa (cocoa_vk_ctx.m) and Android (android_vk_ctx.c) already implement this correctly: just `return false` and let the caller's vulkan_free chain do the single cleanup. This makes the other three match. X11's local XFree(vi) and `g_x11_screen = 0` reset are preserved -- those are local-resource and global-state cleanups respectively, not duplicates of the upper-layer destroy. Wayland and Win32 had no such locals to preserve. Each error-path edit carries an inline comment naming the upper-layer cleanup site (gfx/drivers/vulkan.c::vulkan_free line 4665) so a future maintainer who wonders why the inner destroy is absent doesn't reintroduce it. * samples/gfx/vulkan_ctx_double_free/, .github/workflows/Linux-samples-gfx.yml Regression test following the convention of the v3 vulkan/ tests, the v4 slang test, the v5 mailbox-leak test, and the security regression tests under samples/tasks/. Models the lifecycle contract abstractly rather than building any one driver -- the bug is the shape of the cleanup flow, not a driver-specific quirk. Five probes: post-fix success path (1 alloc / 1 destroy); post-fix failure path (the smoking gun: 1 alloc / 1 destroy, no double-free, no UAF); 16 back-to-back failures (no accumulation, no leaks); interleaved success/failure; and a documentation probe describing how a maintainer can swap the function pointer to re-trigger the pre-fix UAF. Verified pre/post-patch discrimination: under the pre-fix shape ASan reports `double-free` at the upper-layer destroy with the freed allocation traced back to set_video_mode's inner destroy. Under the post-fix shape all five probes pass clean. Wires into Linux-samples-gfx.yml as a sixth step, modeled on the existing five with the same per-step explanation of what the test is regression-testing and the "verbatim copy must follow" reminder.
Stands up a complementary CI job to the per-sample regression-test workflows shipped in v3-v6 (Linux-samples-gfx.yml). Builds full RetroArch with -fsanitize=address,undefined via the existing top- level Makefile SANITIZER= plumbing, then runs `./retroarch --help` and `./retroarch --features` under AddressSanitizer's abort-on-error mode and UBSan in soft-fail mode. The per-sample harnesses regression-test specific predicates that had bugs and by design only cover code that can be exercised by a small standalone test program with mocked dependencies. This job is complementary -- it sanitizer-instruments the entire RetroArch binary as it actually links and runs, so any future heap-corruption / use-after-free / signed-overflow / null-deref bug in code reachable from main() is caught for free as a side effect of any change. Same template as c82db9f's libretro-db samples build under sanitizer in CI, scaled up to the full retroarch binary. * .github/workflows/Linux-asan-ubsan.yml - Build matches Linux-Headless.yml's apt set and --disable-menu choice. Adds --disable-discord --disable-cheevos --disable-networking on the first iteration to shrink the third-party surface so that any sanitizer hit is a RetroArch- internal bug rather than noise from a vendored subsystem. Each can be re-enabled in follow-ups as the baseline stays green. - Build with `make SANITIZER=address,undefined`, which the top- level Makefile (line 153) propagates into CFLAGS / CXXFLAGS / LDFLAGS for every TU and the final link. No new apt packages: libasan / libubsan ship with the default gcc on ubuntu-latest. - Smoke runs --help and --features. Both exit(0) directly after printing. Coverage scope is honest in the inline comment: libc init, main(), frontend driver bootstrap, argv duplication, the getopt walk over the full option table, the print functions themselves, and the static feature-table walks --features performs. Doesn't reach core loading, video init, or cleanup- on-shutdown -- a follow-up step running with --max-frames=N against a noop core can extend coverage to the full lifecycle, but adding the noop-core dependency to CI is a separate step better handled in its own patch. - Sanitizer settings: ASAN_OPTIONS=abort_on_error=1:detect_leaks=0: print_stacktrace=1:strict_string_checks=1 UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=0 LSAN_OPTIONS=exitcode=0 ASan in abort-on-error: any heap corruption fails the job immediately. Leak detection off for now -- libGL / Mesa / Wayland symbol-resolution drives a non-trivial baseline of "leaks" at process shutdown that aren't RetroArch bugs; can revisit with a suppression file once the rest is clean. UBSan in report-only: the first iteration is a discovery run. Pre-existing signed-overflow / alignment / shift-too-large diagnostics in audio resampling, color math, and vendored deps are likely to be present. The "Summarize UBSan baseline" step prints the deduplicated list so follow-up patches have a target to drive to zero. Once that count reaches zero, flip halt_on_error=1 in the smoke steps to enforce. Each smoke step independently greps stderr for "AddressSanitizer:" and exits 1 on any match -- belt-and- suspenders to abort_on_error in case a future ASan release changes its default exit semantics. - Kept as its own workflow rather than folded into Linux-Headless so the existing green-path build smoke stays fast (no ~3-5x ASan slowdown), and so this one can own its longer 25-minute timeout independently. - Trigger matches the per-sample workflows: push to master, pull_request to master, workflow_dispatch.
…ry is disabled When 'Settings -> Playlists -> History' (history_list_enable) is OFF, CMD_EVENT_HISTORY_INIT early-returns and leaves g_defaults.content_history, image_history, music_history, and video_history all NULL. The Playlists tab generator was still appending entries for these with paths obtained via playlist_get_conf_path(NULL), which returns NULL and renders as a blank title. The icons and localized sublabels remained, producing four "empty" rows in Main Menu -> Playlists. Gate the four append calls additionally on the corresponding g_defaults.*_history pointer being non-NULL. This also covers the secondary early-return path in CMD_EVENT_HISTORY_INIT when content_history_size is 0. Mirror the same guard for the History entry in the rgui/glui (no nav bar) branch. Favorites is unaffected: g_defaults.content_favorites is initialized independently of history_list_enable.
Follow-up to b9777c8 (the ASan+UBSan workflow). Run #1 of the new job came back clean: zero distinct UBSan diagnostics across the --help and --features smoke invocations. https://github.com/libretro/RetroArch/actions/runs/25056073385 * .github/workflows/Linux-asan-ubsan.yml - Flip UBSAN_OPTIONS=halt_on_error=0 to halt_on_error=1 in both smoke steps. A future change that introduces signed-overflow / alignment / shift-too-large UB along the option-parsing or print paths will now fail the run instead of being silently logged in a discovery summary. - Add a parallel grep -q "runtime error:" check after each timeout invocation so the failure is visible at exit code level even if a future sanitizer release changes the default abort semantics, mirroring the existing "AddressSanitizer:" check. - Drop the "Summarize UBSan baseline" step. Its job was to surface diagnostics during the discovery phase; with strict enforcement above it any non-zero count would have already aborted the run, so the summary is dead weight that suggested the workflow was still in soft-fail mode. - Step name updates: "ASan strict, UBSan soft-fail" -> "ASan + UBSan strict" on both smoke runs. Honest scoping note unchanged from b9777c8: --help and --features both exit(0) directly without going through the normal shutdown path, so this enforces UBSan-cleanliness only for libc init / main / frontend bootstrap / option parsing / the print functions. Audio resampling, color math, parsers, and full-lifecycle cleanup still aren't covered. The follow-up step running with --max-frames=N against a noop core (deferred from b9777c8) will extend coverage, and that step's first run will start in soft-fail mode the same way this one did.
… is disabled The ozone sidebar exposes History, Images, Music, and Video tabs whose content is populated exclusively from the playlists managed by CMD_EVENT_HISTORY_INIT. When history_list_enable is OFF those playlists are never initialized, so the tabs become permanent dead ends - selecting History, for example, just shows "No History Available" forever. Gate the four tabs in ozone_refresh_system_tabs_list on history_list_enable in addition to the existing menu_content_show_* flags. Favorites is unaffected (independent setting). Also wire MENU_ENUM_LABEL_HISTORY_LIST_ENABLE into the MENU_ENVIRON_RESET_HORIZONTAL_LIST dispatch in general_write_handler so toggling the setting refreshes the tab list immediately rather than requiring a restart, matching the behaviour of the neighbouring menu_content_show_* toggles.
…s disabled, refresh on toggle Apply the same fix as ozone (bdcf692) to the XMB driver: gate the History, Images, Music, and Video system tabs in xmb_refresh_system_tabs_list on history_list_enable in addition to the existing menu_content_show_* flags. Without history tracking these tabs are permanent dead ends with no possibility of content. Favorites is unaffected (independent setting). Additionally, fix the live-toggle behaviour. The previous patch wired MENU_ENUM_LABEL_HISTORY_LIST_ENABLE into the MENU_ENVIRON_RESET_HORIZONTAL_LIST dispatch but stale state in g_defaults.{content,image,music,video}_history meant that the gating logic in menu_displaylist.c (which checks both the menu_content_show_* flag and the corresponding playlist pointer) read the wrong values until the next launch. As a result, the materialui Playlists screen did not visibly update when toggling the setting. Split MENU_ENUM_LABEL_HISTORY_LIST_ENABLE into its own case in general_write_handler that fires CMD_EVENT_HISTORY_INIT (which internally deinits first, then early-returns when the new setting value is OFF), keeping g_defaults pointers in sync with the setting. Also set MENU_ST_FLAG_ENTRIES_NEED_REFRESH so the currently-displayed displaylist is rebuilt - this is what makes materialui's Playlists screen update on toggle, since materialui has no horizontal tab bar of its own to refresh.
The Win32 taskbar progress overlay (the green bar drawn on the RetroArch taskbar icon during downloads) never appeared. Two independent bugs were keeping it dark: 1) wndproc: the 'TaskbarButtonCreated' message check was nested inside unrelated 'case' arms (WM_MOUSEMOVE..WM_NCLBUTTONDBLCLK, WM_DROPFILES..WM_COMMAND, and the WM_PAINT branch of the GDI wrappers). Since 'TaskbarButtonCreated' is a registered window message with its own dynamically-allocated value, those arms could never match it, so WIN32_CMN_FLAG_TASKBAR_CREATED was never set and dispserv_win32's set_window_progress() always early-outed without calling ITaskbarList3::SetProgressValue. Move the comparison to the top of each dispatcher (wnd_proc_common, wnd_proc_common_internal, wnd_proc_winraw_common_internal, wnd_proc_common_dinput_internal) so it sees every incoming message. Short-circuit on the flag bit so the comparison only runs until the message arrives once. The HAVE_TASKBAR gate is unchanged; pre-Win7 builds either don't define it or fall back through the existing taskbar_list NULL guard in dispserv_win32. 2) task_core_updater: even with the flag set, the five outer aggregating tasks (get_list, single download, update_installed_cores, play_feature_delivery_install, play_feature_delivery_switch) never set a progress_cb. They spawn inner http transfers with mute=true to suppress per-core on-screen toasts, which also suppresses the inner tasks' progress_cb invocation in task_queue_push_progress(). Net result: video_display_server_set_window_progress() was never being called during a Core Updater run. Promote the existing http progress forwarder (http_transfer_progress_cb in task_http.c) to a public helper named task_window_progress_cb, declare it in tasks_internal.h, and wire it as the progress_cb of all five outer tasks. The aggregating tasks already drive task->progress in 0..100 across their state machines, so no other plumbing is needed. Single-zip update flows (Update Assets, Update Cheats, Update Databases, etc.) were already working because their underlying http transfer is unmuted and titled, and the existing progress_cb fired for those.
Build on the task_window_progress_cb helper introduced in d1228e6 and propagate the Win32 taskbar progress overlay (and any other platform window progress hook) to more long-running user-visible tasks. Helper relocation: - Move the implementation from task_http.c (HAVE_NETWORKING-gated) to task_file_transfer.c (always built), so non-network tasks can use it without pulling in HAVE_NETWORKING. - Move the prototype in tasks_internal.h out of the HAVE_NETWORKING block for the same reason. - Drop the duplicated copy in task_database.c (task_database_progress_cb) in favour of the shared helper, and the now-unused video_display_server.h include alongside it. New coverage: - task_pl_thumbnail_download.c: the system-wide bulk thumbnail download task. Long, user-initiated, already drives task->progress monotonically across two outer phases. - task_cloudsync.c: cloud sync, which can be very long and reports progress as files are reconciled. - task_core_backup.c: both the backup and restore outer tasks, so user-initiated backup/restore of cores reflects on the taskbar. The single-entry pl_thumbnail outer task and short-running tasks (decompress, disc scan, playlist manager, menu explore) are left unchanged for now -- they finish too quickly for the taskbar indicator to be useful, and adding them is a trivial follow-up if that turns out to be wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )