Skip to content

[pull] master from libretro:master#985

Merged
pull[bot] merged 14 commits intoAlexandre1er:masterfrom
libretro:master
May 1, 2026
Merged

[pull] master from libretro:master#985
pull[bot] merged 14 commits intoAlexandre1er:masterfrom
libretro:master

Conversation

@pull
Copy link
Copy Markdown

@pull pull Bot commented May 1, 2026

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 : )

ui_qt.cpp wraps RetroArch headers in extern "C" { ... } when CXX_BUILD
is not defined. In non-unity builds the chain

  ui_qt.cpp (extern "C") -> menu_driver.h -> gfx_thumbnail.h
    -> retro_atomic.h -> <atomic>

ends up parsing libstdc++ <atomic> under C linkage, producing dozens of
"template with C linkage" errors on g++. The same hazard exists for any
C++ TU that wraps RetroArch headers in extern "C", and is currently
masked elsewhere only because no other such TU happens to pull in a
header that reaches retro_atomic.h.

Fix at the header level so every caller is protected, not just ui_qt.cpp:

 - Add RETRO_BEGIN_DECLS_CXX / RETRO_END_DECLS_CXX to retro_common_api.h.
   They expand to extern "C++" { ... } in C++ mode (with CXX_BUILD off)
   and to nothing otherwise, mirroring the existing RETRO_BEGIN_DECLS /
   RETRO_END_DECLS pair.

 - Use them around the <atomic> / <cstddef> include in retro_atomic.h's
   C++11 backend. The standard headers parse at C++ linkage regardless
   of any surrounding extern "C" the caller imposes; the rest of the
   file (typedefs and macros, no function declarations) is unaffected.

The previous comment claiming "no need for RETRO_BEGIN_DECLS / extern
\"C\" wrapping ... the wrapper would actively break that path" addressed
only half the problem -- the file's own wrapping -- and missed that the
caller's wrapper leaks into the <atomic> include. Updated to reflect
the real constraint.
…tom shader

When HDR is enabled and the active shader chain doesn't emit HDR
natively, vulkan_frame() routes overlay-style content (menu, on-screen
messages, widgets, software overlays) through an offscreen SDR
buffer, then composites it back over the HDR swapchain image. The
gating condition for that detour listed every overlay source except
the statistics overlay.

The result: with HDR on, a non-HDR-emitting shader chain loaded, and
the statistics overlay enabled (with no menu/message/widget visible),
the main render pass was ended after the HDR composite but never
re-begun. The subsequent font_driver_render_msg() call for the stat
text recorded draw commands against a command buffer with no active
render pass.

Most desktop Vulkan drivers tolerate this with a validation error;
MoltenVK on macOS dereferences a null subpass in
MVKCommandEncoder::getSubpass() during vkCmdDrawIndexed encoding and
crashes with SIGSEGV. The HDR-native shaders under slang_shaders/hdr
were unaffected because they avoid the offscreen-buffer detour
entirely (use_offscreen_buffer stays false when the chain emits HDR10
or HDR16).

Add statistics_show to both gating conditions so the begin-redirect
and the matching post-blit composite stay balanced.
Three -Wformat= warnings surface on i386 Linux where size_t is unsigned
int and uint64_t is unsigned long long (both differ from x86_64, where
the existing specifiers happen to match):

  display_linux.cpp:166         %ld    on  vector::size_type (size_t)
  custom_video_xrandr.cpp:679   %lx    on  uint64_t platform_data
  custom_video_xrandr.cpp:685   %lx    on  uint64_t platform_data

The size_t case is signedness-only and benign in practice; switch to
%zu, the C99 specifier intended for size_t. The uint64_t cases are an
actual 32-bit truncation bug -- %lx reads four bytes off the stack on
i386 cdecl, dropping the high 32 bits of platform_data and misaligning
any following format arguments. Use %llx with an explicit
(unsigned long long) cast, matching the raw-printf idiom Switchres
uses everywhere else (no <inttypes.h> / PRIx64 elsewhere in the tree).

deps/switchres/ is a verbatim mirror of antonioginer/switchres; this
fix should also be sent upstream so the next sync doesn't reintroduce
the warnings.
Refactor pass on ui/drivers/ui_qt.{cpp,h} and ui_qt_widgets.{cpp,h}
to reduce Qt-specific surface, lift pure-C work out of C++ syntax,
and use libretro-common idioms in place of locally-rolled equivalents.

  ui/drivers/ui_qt.cpp           5620 -> 5462    (-158)
  ui/drivers/ui_qt_widgets.cpp   8888 -> 8783    (-105)
  ui/drivers/ui_qt.h              761 ->  747    ( -14)
  ui/drivers/ui_qt_widgets.h     1442 -> 1381    ( -61)
                                                 -----
  TOTAL                         16711 -> 16373   (-338)

No functional change. Build verified for ui_qt.o, ui_qt_widgets.o,
moc_ui_qt.o and moc_ui_qt_widgets.o; the only warnings emitted are
the same three pre-existing -Wdeprecated-declarations warnings the
original tree emits (QDir::operator=(const QString&),
QByteArray::append(const QString&), QListWidget::isItemHidden).

Iterative-build wall clock on the four targets drops ~9%
(28.1s -> 25.6s, hyperfine n=5), MOC-generated .cpp drops ~9%
(8666 -> 7874 lines), and total object size drops ~4%
(2.05 MiB -> 1.97 MiB).

Changes:

* Drop string_split_to_qt and use QString::split. The function was
  declared inside RETRO_BEGIN_DECLS - i.e. with C linkage - but
  returned QStringList, which is not a C-compatible type. The four
  call sites become QString::split('|') / .split(' ').

* Replace MainWindow::getScrubbedString with a static scrub_qstring()
  helper using string_replace_all_chars from libretro-common. Moves
  the function out of MOC (one fewer slot to generate).

* Collapse 11 trivial OptionsPage subclasses (MenuSoundsPage,
  UpdaterPage, QuickMenuPage, DriversPage, DirectoryPage,
  ConfigurationPage, CorePage, LoggingPage, AIServicePage,
  FrameThrottlePage, RewindPage) into one runtime-parameterised
  SimplePage class. Each was a do-nothing constructor plus a
  one-line widget() returning create_widget(DISPLAYLIST_X). The 12
  instantiation sites in *Category::pages() become
  new SimplePage(DISPLAYLIST_X, MENU_ENUM_LABEL_VALUE_X, this).

* Extract MainWindow::getCoreInfo() into a C helper qt_core_info_collect
  that emits two parallel struct string_list outputs (keys, values)
  with a status code (enum qt_core_info_row_status) packed into
  values->elems[i].attr.i. The walking logic is now pure C using
  libretro-common idioms (string_list_*, core_info_*,
  fill_pathname_basedir, string_count_occurrences_single_character);
  the C++ wrapper is a ~50-line loop applying status-driven HTML/CSS
  styling for firmware rows. Both consumers
  (onLaunchWithComboBoxIndexChanged, CoreInfoDialog::showCoreInfo)
  work unchanged.

* Replace four parallel thumbnail paths with arrays. The
  m_thumbnailPixmap{,2,3,4} pointers, onResizeThumbnail{One,Two,
  Three,Four} slots, and thumbnailChanged{,2,3,4} signals are
  consolidated into m_thumbnailPixmaps[4] and three static tables.
  The four thumbnailChanged* signals were never emitted and never
  connected anywhere - deleted. The four onResizeThumbnail* slots
  were one-line wrappers around setThumbnail("thumbnailN", ...) -
  deleted, callers use setThumbnail directly. Destructor,
  onCurrentItemChanged and onThumbnailDropped collapse to short
  loops; an explicit ThumbnailType -> widget-index mapping handles
  the asymmetric ordering between the enum (BOXART, SCREENSHOT,
  TITLE_SCREEN, LOGO) and the 1..4 widget naming (boxart, title,
  screenshot, logo).

* Convert MainWindow::getSelectedCore (which returned
  QHash<QString,QString> but only ever set a single field) to
  getSelectedCorePath returning QString. Sole caller updated.

* Replace QFileInfo/QDir/QFile usage with file_path.h /
  file_stream.h equivalents in three high-leverage spots:

  - renamePlaylistItem: now uses path_basename, path_basedir,
    path_get_extension, fill_pathname_slash,
    string_is_equal_case_insensitive, and filestream_rename.
  - onExtractArchive's per-entry delete-or-rename loop: now uses
    filestream_exists, filestream_delete, filestream_rename. Also
    fixes a pre-existing leak: the original code returned -1 on
    three error paths without freeing file_list.
  - loadContent's extension-filter block: collapses lastIndexOf('.')
    + .mid() onto path_get_extension, which the same block was
    already using three lines below.
  - Three QFileInfo(pathData).suffix() callers in
    add-files-to-playlist code: replaced with
    QString::fromUtf8(path_get_extension(pathData)).

* Split ui_companion_qt_init from a 516-line monolith into six
  focused static helpers (qt_companion_build_menubar,
  qt_companion_build_browser_dock, qt_companion_build_thumbnail_docks,
  qt_companion_build_core_selection_dock, qt_companion_restore_settings,
  qt_companion_select_initial_playlist) plus a ~75-line orchestrator.
  Behaviour preserved verbatim. The thumbnail-dock setup that was
  65 lines of 4-way copy-paste collapses to a 30-line loop using
  qt_thumbnail_widget_names from above.
Selecting an image file in the file browser ran the PNG/JPEG decode
synchronously on the UI thread via image_texture_load. For ordinary
scraper-sized boxart this is fine, but for the multi-GiB ARGB32
bitmaps that come out of decoding a high-resolution upscayl-style
PNG the UI froze for hundreds of milliseconds to several seconds.
The same stall fired on every selection change, including the
implicit ones triggered by collapsing a tree node when the current
selection became invisible.

Add a ThumbnailLoader owned by MainWindow (parallel to the one
PlaylistModel owns for the grid view) and route the preview-pane
decode through it. The panes are cleared immediately on selection
change; the loaded pixmap is installed when the worker's
imageLoaded signal fires. A pending-path token guards against rapid
selection changes flickering stale images in.

Playlist-thumbnail (scraper boxart) decoding stays synchronous
because those images are bounded in size; the only change there is
that m_pendingPreviewPath is cleared on entry so a previously-issued
preview request can't land in the panes after the playlist code
already painted boxart there.
The proxy model's filterAcceptsRow is called once per (row, parent)
pair the source QFileSystemModel surfaces, which can run into the
tens of thousands during a directory load, sort, or filter pass on
a populous tree. The original implementation resolved the source
model's root path to a QModelIndex via sm->index(sm->rootPath()) on
every single call - cheap individually, but it adds up.

The root path itself only changes when the user navigates, so cache
both the path and its resolved index on the proxy. On each call,
compare the cached path against the current one (a QString equality
check, much cheaper than re-walking the model); refresh on mismatch.

No behaviour change.
* Drop three unused Qt includes:

  - ui_qt.h: QDialog, QStyledItemDelegate. Both are still needed in
    ui_qt_widgets.h (PlaylistEntryDialog inherits QDialog,
    ThumbnailDelegate inherits QStyledItemDelegate), and ui_qt.h
    pulls in ui_qt_widgets.h, so consumers continue to get them
    transitively for free.
  - ui_qt_widgets.h: QTabWidget. Not used in this header at all.

* Split MainWindow::MainWindow body into helpers. The constructor
  was 446 body lines of mixed widget creation, layout assembly,
  model setup, file-system configuration, dock setup, and signal
  wiring, with locals declared at the top and used hundreds of
  lines later. Five private methods now own the chunks that don't
  cross-depend:

  - setupPlaylistFooter() builds the zoom slider, view-type and
    thumbnail-type push buttons, items count label, and grid
    progress widget, attaching them under the playlist views.
  - setupModels() configures the playlist + filesystem proxy
    models and the table / file-table / grid views.
  - setupFileSystemBrowser() configures the QFileSystemModels
    (filters, root paths) and the directory tree view.
  - setupDockWidgets() owns the search / core-info / log dock
    widgets, including the searchResetButton that previously lived
    as a constructor local.
  - setupSignalConnections() owns the bulk of the QObject::connect
    block plus the m_dirTree initial-selection and m_thumbnailTimer
    setup that was interleaved with it.

  Constructor body: 446 -> 101 lines.

* Three small file-static helpers collapse repeated boilerplate:

  - qt_button_set_action_label() replaces the
    setDefaultAction(new QAction(msg_hash_to_str(label), btn)) +
    setFixedSize(sizeHint()) pair (5 sites).
  - qt_dock_add_to() replaces the
    addDockWidget(static_cast<Qt::DockWidgetArea>(
       dock->property("default_area").toInt()), dock)
    idiom (7 sites).
  - qt_dock_configure() replaces a setObjectName / setProperty(
    "default_area") / setProperty("menu_text") / setWidget
    four-line block (6 sites).

No behaviour change.
Reduces Qt-specific surface and silences three pre-existing
-Wdeprecated-declarations warnings. All changes preserve behaviour
on Qt 5.2 and up (RetroArch's minimum, per qb/config.libs.sh).

* MainWindow::MainWindow:

  Drop dead 'QDir playlistDir(path_dir_playlist)' and the
  path_dir_playlist local that only existed to construct it. Both
  unused since the constructor split. -Wunused-variable did not
  flag the QDir because of its non-trivial constructor.

* MainWindow::setCustomThemeFile:

  Replace QFile open / readAll / close with filestream_read_file
  plus filestream_exists. Same five error paths preserved
  (blank path, missing file, open failure, empty file, success).

* MainWindow::onFileBrowserTreeContextMenuRequested:

  Replace the QDir / dir.exists() existence check with
  path_is_directory(). Removes a Qt5/Qt6 ifdef ladder ('dir =
  string' on Qt5 vs 'dir.setPath(string)' on Qt6) and silences
  the QDir::operator=(QString) deprecation warning. The
  QDir::toNativeSeparators(...) display path stays - that side is
  fine on both Qt 5 and Qt 6.

* LoadCoreWindow::onCoreEnterPressed:

  Replace 'QByteArray::append(QString)' (Qt 5) /
  'QByteArray::append(QString::toStdString())' (Qt 6) ladder with
  'path.toUtf8().constData()' inline. The temporary QByteArray's
  lifetime extends through the loadCore() call so this is safe.
  Silences the QByteArray::append(QString) deprecation warning.

* MainWindow::onPlaylistWidgetContextMenuRequested:

  Replace 'm_listWidget->isItemHidden(item)' (deprecated since
  Qt 4.8) with 'item->isHidden()' (available since Qt 4.x).
  Removes a Qt5/Qt6 ifdef ladder. Silences the
  QListWidget::isItemHidden() deprecation warning.

Tested under Qt 5.15 with -Wdeprecated-declarations enabled; all
four object files (ui_qt, ui_qt_widgets, moc_ui_qt,
moc_ui_qt_widgets) now build with zero warnings.

No behaviour change.
Pure dead-code removal in three files. No behaviour change.

* ui_qt.h: drop 8 redundant forward declarations.

  GridView, ShaderParamsDialog, CoreOptionsDialog, CoreInfoDialog,
  PlaylistEntryDialog and ViewOptionsDialog all have full
  definitions in ui_qt_widgets.h, which ui_qt.h includes at line
  46 - the forward decls below the include were noise.

  MainWindow and ThumbnailWidget were forward-declared at the top
  of ui_qt.h but never referenced before their own definitions
  later in the same file. Only the ThumbnailLabel forward decl is
  load-bearing - ThumbnailWidget references ThumbnailLabel* as a
  member before ThumbnailLabel is defined.

* MainWindow: drop 5 dead signals and 4 dead connect() calls.

  - gotReloadCoreOptions, gotThumbnailDownload, updateThumbnails
    and extractArchiveDeferred each had a connect() in the
    constructor but were never emitted anywhere - the signal
    connections were inert. The slots they pointed to
    (onDownloadThumbnail, onGotReloadCoreOptions, onExtractArchive,
    updateVisibleItems) all stay - they have other callers, either
    via direct calls or via QMetaObject::invokeMethod.

  - gridItemChanged was declared but neither emitted nor connected.

* MainWindow: drop 3 dead public getters.

  contentTableView(), contentGridView() and searchWidget() were
  declared and defined but had zero callers across ui/, libretro-
  common/, frontend/, menu/, and retroarch.c.

* MainWindow::MainWindow: drop dead 'QDir playlistDir' and the
  path_dir_playlist local that only existed to construct it.
  Both unused since the constructor split.
  -Wunused-variable did not flag the QDir because of its
  non-trivial constructor.

* MainWindow::changeThumbnail: replace QDir.exists() / QDir.mkpath
  pair with path_is_directory() / path_mkdir() from libretro-
  common, dropping one more QDir local. mkpath(".") and
  path_mkdir() both create parent directories recursively.

* MainWindow::setCoreActions: drop dead 'QFileInfo info' block
  ('info' was assigned via setFile() but never read) and a dead
  'coreName = "<n/a>"' assignment in a then-branch that did
  nothing else with coreName.

* qt_companion_select_initial_playlist: drop redundant
  'listWidget->count() &&' guard from two for-loop conditions
  ('i < count' is already false when count is 0).

* ui_qt_widgets.cpp: drop pre-existing dead 'uint32_t flags =
  runloop_st->flags' local in CoreOptionsDialog (-Wunused-variable
  was suppressed in the build and so this slipped in years ago).
The Qt UI's playlist rows used to round-trip through
QHash<QString,QString> with magic string keys ('path', 'label',
'core_path', etc.). This commit replaces that with a typed
PlaylistEntry struct. The keys actually used at the call sites
collapsed cleanly into 9 fields plus a row index. Other
QHash<QString,QString> uses in the file (core-info display rows,
core-selection-dialog rows) follow a different schema and are
intentionally left alone.

* ui_qt_widgets.h: define PlaylistEntry. The struct lives in this
  header rather than ui_qt.h because PlaylistEntryDialog has it
  in a default argument and ui_qt_widgets.h is included by
  ui_qt.h, not the other way round. Q_DECLARE_METATYPE and
  qRegisterMetaType<PlaylistEntry>() enable QVariant transport.

* PlaylistModel: m_contents is now QVector<PlaylistEntry>.
  getThumbnailPath(), data() (HASH role), and setData() all
  consume the struct directly. The custom user role enum is
  renamed HASH -> ENTRY to match.

* MainWindow:
  - getCurrentContentHash() -> getCurrentContentEntry()
  - getFileContentHash()    -> getFileContentEntry()
  - loadContent(QHash)      -> loadContent(PlaylistEntry)
  - onCurrentItemChanged(QHash) -> onCurrentItemChanged(PlaylistEntry)
  - updateCurrentPlaylistEntry(QHash) -> (PlaylistEntry)
  All call sites (setCoreActions, getSelectedCorePath, onRunClicked,
  onCurrentTableItemDataChanged, onCurrentFileChanged,
  onFileDoubleClicked, changeThumbnail, deleteCurrentPlaylistItem,
  the playlist-context-menu handler, and the addFilesToPlaylist
  flow) updated to read entry.field instead of hash["field"].

* PlaylistEntryDialog::showDialog and setEntryValues now take
  const PlaylistEntry &. The 'multiple entries' detection that
  used QHash::isEmpty() now uses entry.path.isEmpty(); previously
  the addFilesToPlaylist() flow had to write two empty-string keys
  just to make the QHash non-empty as a signal, which is gone.

* Drop the dead 'index' field round-trip.
  updateCurrentPlaylistEntry() and deleteCurrentPlaylistItem() used
  to read 'index' from the hash via QString::toUInt, which meant
  PlaylistModel::addPlaylistItems() had to format every row's index
  as a decimal string into the hash. PlaylistEntry::index is just
  an unsigned now.

* Drop dead m_currentGridHash member from MainWindow. It was only
  ever cleared, never written or read, so it always held an empty
  QHash. Removed along with the related conversions.

No behaviour change. All four object files (ui_qt, ui_qt_widgets,
moc_ui_qt, moc_ui_qt_widgets) build with zero warnings.
Four small dead-code removals turned up in another sweep over
the audit areas. No behaviour change.

* TreeView::columnCountChanged: pure passthrough to
  QTreeView::columnCountChanged. Without this override, Qt's
  default does exactly the same thing. Drop the slot decl + impl.

* ListWidget::isEditorOpen: declared and defined, never called.
  TableView has the same method and one caller; ListWidget had
  none.

* AppHandler::onLastWindowClosed + the QApplication::lastWindowClosed
  -> onLastWindowClosed connect: the slot body was empty and the
  connection was therefore a no-op. Drop the slot, the connect()
  call, and the now-empty 'private slots:' section in the header.

* ThumbnailLabel::resizeEvent: pure passthrough to
  QWidget::resizeEvent, same situation as columnCountChanged.

All four object files (ui_qt, ui_qt_widgets, moc_ui_qt,
moc_ui_qt_widgets) build with zero warnings.
Three MainWindow methods exist solely to map an enum value to a
fixed string literal: getThemeString, getCurrentViewTypeString,
and getCurrentThumbnailTypeString. They returned QString built
from a string literal at every call. Returning the literal
directly as const char* is one less heap allocation per call and
the caller (QSettings::setValue, which takes QVariant) accepts
it via the implicit QString conversion either way.

* getThemeString: 4 lines -> 1 line per branch.
* getCurrentViewTypeString: switch over a 2-value enum collapses
  to an if.
* getCurrentThumbnailTypeString: dropped a dead trailing
  'return QString("list")' that the compiler couldn't reach -
  the THUMBNAIL_TYPE_BOXART branch already returned via the
  default label.

No behaviour change. All four object files build with zero
warnings.
@pull pull Bot locked and limited conversation to collaborators May 1, 2026
@pull pull Bot added the ⤵️ pull label May 1, 2026
@pull pull Bot merged commit 8f6d51b into Alexandre1er:master May 1, 2026
42 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant