Skip to content

[pull] master from libretro:master#979

Merged
pull[bot] merged 14 commits intoAlexandre1er:masterfrom
libretro:master
Apr 30, 2026
Merged

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

Conversation

@pull
Copy link
Copy Markdown

@pull pull Bot commented Apr 30, 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 : )

LibretroAdmin and others added 14 commits April 30, 2026 00:24
Substitute the per-handle cached size for video_driver_get_size
across the three main menu drivers.  All three handles now expose
last_width / last_height fields populated every render frame, so
input-time and layout-time code paths can read the size without
acquiring video_st's context_lock + display_lock.

ozone (menu/drivers/ozone.c)
  ozone_handle_t already carries last_width / last_height,
  written every frame in ozone_render whenever the size differs
  from the cached value.  Six get_size sites converted:

    ozone_list_cache              (after select / list change)
    ozone_sidebar_goto            (sidebar nav)
    ozone_update_scroll           (scroll position update)
    ozone_compute_entries_position (entries layout recompute)
    ozone_draw_entries            (per-frame, render path)
    ozone_pointer_up              (pointer release gesture)

  ozone_draw_entries also receives video_width / video_height as
  parameters from its render-path caller; the local
  video_info_width / video_info_height that this commit
  substitutes remain a separate set of variables (pre-existing).
  Both end up holding the same value at runtime.

materialui (menu/drivers/materialui.c)
  materialui_handle_t already carries last_width / last_height,
  written every frame in materialui_render.  Three sites
  converted:

    materialui_get_scroll      (called from input-handling paths)
    materialui_pointer_down    (touch-down: scrollbar drag check)
    materialui_pointer_up      (touch-release: gesture dispatch)

  The substitution in materialui_get_scroll moves the read past
  the existing null check on mui.  In materialui_pointer_up the
  read sits at the same line as the old get_size call so the
  early-return paths still bail before any size lookup.

xmb (menu/drivers/xmb.c)
  Unlike the other two, xmb_handle_t did not previously carry
  last_width / last_height.  Add them, and write them at two
  points:

    xmb_init  -- one-shot, populated from the get_size already
                 present in init
    xmb_frame -- every render call, sourced from
                 video_info->{width,height}

  Six get_size sites then converted:

    xmb_selection_pointer_changed   (cursor movement)
    xmb_list_open_new               (list expansion)
    xmb_list_switch_new             (tab / category switch)
    xmb_layout                      (layout recompute)
    xmb_list_cache                  (after a list change)
    xmb_pointer_up                  (touch-release gesture)

  The xmb_layout call site is reached through xmb_context_reset_
  internal, which itself runs after init has primed the cache,
  so the field is always valid when read.  The xmb_frame write
  is two unsigned stores after a null-check on xmb that already
  guards the rest of the function.

Across all three drivers the init-time get_size is preserved by
design: at the moment it runs, last_width / last_height are
either zero (xmb, ozone freshly calloc'd) or being primed for
the first time (materialui).  That get is the only remaining
get_size call in each of these files.

Per-call savings: video_driver_get_size acquires context_lock
plus, when threaded video is active, display_lock.  These reads
now go directly to handle fields with no synchronization.  Most
of the converted paths fire on user input rather than per-frame,
but ozone_draw_entries and xmb_layout sit on the render / layout
hot paths and do benefit from frame-rate scale savings.
Each of the three D3D fixed-pipeline drivers carries an identical
gfx_display_prim_to_d3d{8,9_cg,9_hlsl}_enum helper plus a caller-
side switch on draw->prim_type.  Both are dead.

Every gfx_display_ctx_draw_t in the codebase sets prim_type to
GFX_DISPLAY_PRIM_TRIANGLESTRIP before invoking dispctx->draw.  All
19 "prim_type =" assignments under gfx/, menu/, and gfx/widgets/
assign TRIANGLESTRIP -- never TRIANGLES, never NONE.  Every local
declaration of the draw struct assigns prim_type before the draw
call (swept each one).  At runtime the helper switch always hits
the TRIANGLESTRIP arm and the caller's per-call branch always
takes the tristrip path.

The dead non-tristrip branch was also buggy: it mapped TRIANGLES
to D3DPT_TRIANGLESTRIP (wrong primitive -- gl1/2/3 correctly map
it to GL_TRIANGLES) and used count = vertices (wrong formula for
DrawPrimitive, which takes PrimitiveCount; the right formula is
vertices/3).  And the helpers' TODO/FIXME default returned zero,
which is not a legal D3DPRIMITIVETYPE value (the enum starts at
D3DPT_POINTLIST = 1) -- a debug runtime would reject the call.

Inline D3DPT_TRIANGLESTRIP at the DrawPrimitive site, drop the
helpers and the dead caller branches, drop the now-unused
D3DPRIMITIVETYPE locals.  Document at each call site that this
assumes TRIANGLESTRIP and would need to grow back into a switch
if a future non-tristrip caller appears.

While here, harden d3d9_cg and d3d9_hlsl against vertices < 3.
Both used count = vertices - 2 unconditionally, which on an
unsigned subtraction with vertices < 2 wraps to a huge primitive
count and walks off the vertex buffer in the GPU.  d3d8 already
had this guard; the d3d9 pair did not.  No caller passes < 3
vertices today, but the same "no caller does this" reasoning is
what kept the dead switch alive, so add the guard symmetrically.

Net -38 lines.  No behavioural change for any existing caller.
Companion to 1ee88d0 (the d3d helpers).  Each of gl1, gl2,
gl3 carries an identical gfx_display_prim_to_gl{1,,3}_enum helper
that maps GFX_DISPLAY_PRIM_TRIANGLESTRIP -> GL_TRIANGLE_STRIP and
GFX_DISPLAY_PRIM_TRIANGLES -> GL_TRIANGLES.  Same dead-code
argument as 1ee88d0: every gfx_display_ctx_draw_t in the
codebase sets prim_type to GFX_DISPLAY_PRIM_TRIANGLESTRIP before
invoking dispctx->draw, all 15 "prim_type =" assignments under
gfx/, menu/, and gfx/widgets/ assign TRIANGLESTRIP, and every
local declaration assigns prim_type before the draw call.  The
TRIANGLES, NONE, and default arms are unreachable.

Unlike the d3d helpers (which mapped TRIANGLES to the wrong
primitive type and returned an illegal D3DPRIMITIVETYPE on the
default branch), the gl helpers were correct in their dead
branches -- TRIANGLES -> GL_TRIANGLES is the right mapping, and
the "return 0" default falls back to GL_POINTS, a valid (if
nonsensical for menu draws) GL enum.  So this is purely
dead-code removal, not a latent-bug fix.

Inline GL_TRIANGLE_STRIP at the glDrawArrays call site, drop the
helpers.  gl3 has two glDrawArrays sites (the chain.active branch
and the HAVE_SLANG else branch); both get the same treatment, and
the explicit switch on prim_type at the second site collapses to
a single glDrawArrays call as well.  Document at the call sites
that the constant assumes TRIANGLESTRIP.

Net -48 lines.  No behavioural change for any existing caller.
Companion to 1ee88d0 and 2711c42727 (the d3d and gl helpers).
MenuDisplay's private _toPrimitiveType: maps
GFX_DISPLAY_PRIM_TRIANGLESTRIP -> MTLPrimitiveTypeTriangleStrip
and falls through to MTLPrimitiveTypeTriangle for any other
input.  Same dead-code argument: every gfx_display_ctx_draw_t in
the codebase sets prim_type to GFX_DISPLAY_PRIM_TRIANGLESTRIP
before invoking the draw path, so the fallthrough to
MTLPrimitiveTypeTriangle is unreachable.

Inline MTLPrimitiveTypeTriangleStrip at the single -drawPrimitives:
call site, drop the helper.  Document at the call site that the
constant assumes TRIANGLESTRIP.

While here: note that the dead fallthrough returned
MTLPrimitiveTypeTriangle (a triangle list) rather than something
nonsensical, so unlike the d3d helpers this was not also a latent
bug -- just dead.

No behavioural change for any existing caller.
Continuation of the prim_type dead-code sweep (1ee88d0,
696efa0, 6cd9ea6).  Vulkan didn't have a prim_to_vk helper
to delete, but the same dead pattern is present, expressed
through array slot indexing instead of a switch statement.

Old layout
----------

vk->display.pipelines was an 18-slot array (9 logical pipelines x
2 topologies) indexed by `(prim_type == TRIANGLESTRIP) << 1 | blend`
for the display alpha-blend pair, and by to_menu_pipeline()
returning `6 + 2*shader_idx + (prim_type == TRIANGLESTRIP)` for
the menu shaders.  Since every draw passes TRIANGLESTRIP (same
audit as the d3d/gl/metal cleanups: 15 of 15 prim_type assignments
in the codebase assign TRIANGLESTRIP, never TRIANGLES, never NONE),
the LIST entries get built by init and never read.

Slots [0], [1], [6], [8], [10], [12], [14], [16] -- the LIST
variants -- were dead at runtime in every configuration.

Slots [4] and [5] -- the HDR-only display pipelines populated by
the for(i=4;i<6;i++) loop introduced in 12d188e -- were also
dead.  Both iterations of that loop produce identical pipeline
objects (no varying state between them) and no consumer ever
indexes those slots.  The dedicated vk->pipelines.hdr field
built right above the dead loop is the actual HDR composition
pipeline used by vulkan_run_hdr_pipeline at swapchain present.
The dead [4,5] population looks like copy-paste residue from
the SDR alpha-blend loop above it.

New layout
----------

Drop the LIST variants and the dead HDR slots.  Collapse to 8
slots, all TRIANGLE_STRIP:

    [0] alpha_blend, no blend
    [1] alpha_blend, blend
    [2] ribbon
    [3] ribbon_simple
    [4] snow_simple
    [5] snow
    [6] bokeh
    [7] snowflake

Same layout for vk->display.pipelines_sdr (the parallel HDR-on
SDR-offscreen variants).

Consumer updates
----------------

  - gfx_display_vk_draw at the disp_pipeline site:
        ((prim_type == TRIANGLESTRIP) << 1) | blend  ->  blend
    (always picks slot 0 or 1 in the new layout)

  - to_menu_pipeline drops its prim_type parameter and returns
    a constant 2..7 keyed off pipeline_id.

  - The overlay path's hardcoded vk->display.pipelines[3]
    ("Strip with blend") becomes vk->display.pipelines[1] (same
    pipeline, new slot number).

Init loop changes
-----------------

  - The display alpha-blend build (was for(i=0;i<4;i++) varying
    topology and blend on i bits) becomes for(i=0;i<2;i++) with
    topology set once before the loop.
  - The menu shader build (was for(i=0;i<SIZE-6;i++) using
    `i >> 1` for shader index and `i & 1` for topology) becomes
    for(i=0;i<6;i++), one iteration per shader.
  - Mirror changes for the SDR sibling loops.
  - The dead HDR-display loop at lines 3537-3542 (for(i=4;i<6;i++))
    is removed.  The vk->pipelines.hdr build right above it stays.

Why STRIP for everything is fine
--------------------------------

Same argument as the prior sweep commits: all menu draws are
axis-aligned quads (4 vertices = 2 triangles).  A tristrip
issues N-2 triangles for N vertices; a list needs 3*N vertices
for N triangles.  For a quad: 4 vs 6 vertex shader invocations.
STRIP is strictly cheaper per draw than LIST for this geometry,
so there is no performance reason to keep the LIST pipelines.

Init-time saving is small but real: 8 fewer vkCreateGraphicsPipelines
calls per init in SDR builds, 10 fewer when HDR is on.  Pipeline
creation is one of Vulkan's more expensive operations on first
compile (shader bytecode + state object).  Most of the saving
is in driver pipeline cache footprint and shader-module count;
hot-path draws hit the same pipelines they did before, just at
different slot numbers.

Verification
------------

  - clang -fsyntax-only -Wall on three configuration permutations:
    HDR + shaderpipeline, no HDR + shaderpipeline, no HDR + no
    shaderpipeline.  All clean.
  - Brace and paren balance preserved against master.
  - Diff is +78/-75 (net +3 lines) because most additions are
    layout-documenting comments; the executable code shrinks
    substantially.
The prim_type sweep across d3d/gl/metal/vulkan (1ee88d0, 696efa0,
6cd9ea6, 9915c52) removed the dead per-driver dispatch branches
that handled non-TRIANGLESTRIP cases.  After those commits the field
was set to GFX_DISPLAY_PRIM_TRIANGLESTRIP at every call site and read
nowhere -- a vestigial concept the codebase still paid for in struct
size, init code, and reader cognitive load.

Survey before this commit:

  - 16 assignments of draw->prim_type, all to
    GFX_DISPLAY_PRIM_TRIANGLESTRIP (11 caller-side, 5 driver-side
    overrides for the menu shader path)
  - 0 reads that branched on the value
  - GFX_DISPLAY_PRIM_NONE and GFX_DISPLAY_PRIM_TRIANGLES: defined,
    never assigned anywhere in tree

Removed:

  - enum gfx_display_prim_type (gfx/gfx_display.h)
  - struct gfx_display_ctx_draw::prim_type field
  - All 16 prim_type assignments
  - The "every caller sets prim_type to TRIANGLESTRIP, so the per-call
    switch was dead" comments left in d3d8.c, d3d9cg.c, d3d9hlsl.c,
    gl1.c, gl2.c, gl3.c, metal.m, and vulkan.c by the prior cleanup
    commits, replaced with shorter comments noting only the strip
    layout assumption (and the vertex underflow guard rationale, which
    is still relevant for d3d).

Effects on consumers:

  - struct gfx_display_ctx_draw shrinks by sizeof(enum) + alignment
    padding (typically 4 bytes).  The struct is stack-allocated at all
    call sites; no heap layout changes.
  - No ABI exposure to libretro cores -- gfx_display_ctx_draw_t is
    internal to RetroArch.
  - Out-of-tree menu driver forks that wrote draw.prim_type would see
    a compile error.  In-tree consumers all updated.

Verification:

  - clang -fsyntax-only -Wall on vulkan.c across three configurations
    (HDR + shaderpipeline, no HDR + shaderpipeline, no HDR + no
    shaderpipeline): clean.
  - clang -fsyntax-only on gfx/gfx_display.c, gfx/gfx_thumbnail.c,
    gfx/gfx_widgets.c, materialui.c, xmb.c: clean.
  - Brace and paren balance preserved against master in all 15
    modified files.
vk_draw_triangles' indexed_quads field was a code path for indexed
quad drawing via the shared quad_ibo (vk->quad_ibo).  It was never
exercised on master:

  - Both call sites in gfx_display_vk_draw set call.indexed_quads
    to false (L1995, L2027 before this commit).
  - The if (call->indexed_quads && ...) branch in
    vulkan_draw_triangles was therefore unreachable.
  - The font path used to take this branch via vulkan_font_flush(),
    but that helper was inlined into vulkan_font_render_msg, where
    the indexed draw is now issued directly.  The "always true for
    fonts" comment in vulkan_font_render_msg was documenting the
    pre-inlining state and survived as a stale reference.

Remove:

  - bool indexed_quads from struct vk_draw_triangles
  - the two call.indexed_quads = false; assignments in
    gfx_display_vk_draw
  - the dead if/else in vulkan_draw_triangles, leaving the single
    vkCmdDraw that the live (else) branch always took
  - the obsolete "the runtime branch on indexed_quads" bullet in
    the vulkan_font_render_msg flush comment

The shared quad_ibo (vk->quad_ibo) itself stays -- it is still
consumed by vulkan_font_render_msg's inlined draw path and by
vulkan_draw_quad (the menu compositing path) at L5965 and L5982.
This commit only removes its dead-on-arrival use through the
generic vulkan_draw_triangles dispatch.

Effects:

  - struct vk_draw_triangles loses 1 byte + alignment padding
    (typically 4 bytes after the trailing unsigned).  The struct
    is stack-allocated; no heap layout concerns.
  - vulkan_draw_triangles loses an unreachable branch and an IBO
    capacity check that always saw num_quads = 0.

Verification:

  - clang -fsyntax-only -Wall on three configurations (HDR + SP,
    no HDR + SP, no HDR + no SP): all clean.
  - Brace and paren balance preserved against master.
  - Diff is +1/-30 lines.
@pull pull Bot locked and limited conversation to collaborators Apr 30, 2026
@pull pull Bot added the ⤵️ pull label Apr 30, 2026
@pull pull Bot merged commit 33f3aba into Alexandre1er:master Apr 30, 2026
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.

2 participants