shaders: the menu copies the driver's loaded preset, not the disk

Actions the TODO above the synchronous fallback asked for: applying
a preset paid three walks of the preset-reference chain - the
driver's own parse-and-compile, then, to mirror the result into the
menu, video_shader_load_preset_into_shader's root-chain crawl plus
its re-read of the original file for reference validation. The
deferred load's completion paid the same two menu-side walks. On
slow storage with chained presets, every apply read every file in
the chain three times.

The struct audit says a copy can replace the re-parse:
struct video_shader is self-contained inline arrays with exactly one
owned-pointer exception, pass[i].source.string.{vertex,fragment},
which driver compilation (glsl, d3d) fills and owns and menu code
never reads. video_shader_copy_for_menu() is a struct copy that
clears those two pointers per pass and runs the same file-watch
registration the parse path runs. menu_shader_manager_set_preset_
from_live() wraps it in set_preset's success-path bookkeeping, and
both call sites - the synchronous fallback and the deferred
completion - use it when the driver exposes get_current_shader,
parsing from disk as before when it does not. Reading the live
struct there is main-thread reader against video-thread reader:
after set_shader's blocking window the struct's only mutating field
has one sanctioned writer, and it is this same main thread.

The copy also retires a latent index hazard the parse path merely
got away with: the slang backends rebuild the parameter list by
reflection, and the parameter menu indexes the driver's live struct
with positions taken from the menu's own copy. A copy of the
driver's struct agrees with it by construction; a fresh parse only
happens to.

samples/gfx/shader_copy links the shipping objects and drives what
the copy replaces: a child preset referencing a parent whose pass
source carries a #pragma parameter, parsed through the real
reference-chain walk with the referenced value applied; heap source
strings planted the way compilation does; then the copy must be
byte-identical modulo the two cleared pointers, leave the driver's
strings unstolen (the ASan-clean single free is the no-aliasing
proof), and be independent under mutation.

Verified: the parity harness green under ASan, playlist_nav green,
full build clean, C89 gates on the three touched translation units,
thread-read audit 0 findings, hot-function budgets, and the 74-lane
compile matrix.

gfx/video_driver.c

@@ -815,8 +815,18 @@ void video_driver_shader_deferred_tick(void)
                struct video_shader *shader = menu_shader_get();
                if (shader)
                {
-                  video_shader_load_preset_into_shader(
-                        d->preset_path, shader);
+                  /* The driver just finished loading this preset;
+                   * copy its struct instead of re-walking the
+                   * preset chain from disk. Parse fallback for
+                   * drivers without get_current_shader. */
+                  video_shader_ctx_t live = {0};
+                  video_shader_driver_get_current_shader(&live);
+                  if (live.data)
+                     menu_shader_manager_set_preset_from_live(
+                           shader, live.data);
+                  else
+                     video_shader_load_preset_into_shader(
+                           d->preset_path, shader);
                   shader->flags &= ~SHDR_FLAG_MODIFIED;
                }
 #endif

gfx/video_shader_parse.c

@@ -2531,6 +2531,28 @@ bool video_shader_write_preset(const char *path,
  *
  * @return true on success, otherwise false on failure.
  **/
+/* Copies a driver's fully loaded shader into a menu-owned struct
+ * instead of re-parsing the preset chain from disk. The struct is
+ * self-contained inline arrays with one exception:
+ * pass[i].source.string.{vertex,fragment} are driver-owned heap
+ * strings filled during driver compilation - the copy must not
+ * alias them, and the menu never uses them, so they are cleared.
+ * File-watch registration runs here exactly as the parse path runs
+ * it, so the copy is a drop-in for the parse. */
+void video_shader_copy_for_menu(struct video_shader *dst,
+      const struct video_shader *src)
+{
+   unsigned i;
+   *dst = *src;
+   for (i = 0; i < GFX_MAX_SHADERS; i++)
+   {
+      dst->pass[i].source.string.vertex   = NULL;
+      dst->pass[i].source.string.fragment = NULL;
+   }
+   if (config_get_ptr()->bools.video_shader_watch_files)
+      video_shader_watch_preset_files(dst, NULL, NULL);
+}
+
 bool video_shader_load_preset_into_shader(const char *path,
       struct video_shader *shader)
 {
@@ -3864,10 +3886,22 @@ bool video_shader_apply_shader(
                strlcpy(runloop_st->runtime_shader_preset_path, preset_path,
                      sizeof(runloop_st->runtime_shader_preset_path));
 #ifdef HAVE_MENU
-            /* reflect in shader manager */
-            if (menu_shader_manager_set_preset(
-                     shader, type, preset_path, false))
-               shader->flags &= ~SHDR_FLAG_MODIFIED;
+            /* Reflect in the shader manager from the driver's own
+             * loaded struct - it just parsed and compiled this very
+             * preset, so re-walking the reference chain from disk
+             * (a root-chain crawl plus a re-read of the original
+             * file) buys nothing. Fall back to the parse when the
+             * driver has no get_current_shader. */
+            {
+               video_shader_ctx_t live = {0};
+               video_shader_driver_get_current_shader(&live);
+               if (live.data
+                     ? menu_shader_manager_set_preset_from_live(
+                          shader, live.data)
+                     : menu_shader_manager_set_preset(
+                          shader, type, preset_path, false))
+                  shader->flags &= ~SHDR_FLAG_MODIFIED;
+            }
 #endif
          }
          else

gfx/video_shader_parse.h

@@ -247,6 +247,13 @@ bool video_shader_load_current_parameter_values(config_file_t *conf, struct vide
  *
  * Returns: true (1) if successful, otherwise false (0).
  **/
+/* Struct copy of a driver's loaded shader for menu use: everything
+ * but the driver-owned pass source strings, which are cleared in
+ * the copy. Replaces a full re-parse of the preset chain when the
+ * driver has already done it. */
+void video_shader_copy_for_menu(struct video_shader *dst,
+      const struct video_shader *src);
+
 bool video_shader_load_preset_into_shader(const char *path, struct video_shader *shader);
 
 /**

menu/menu_driver.c

@@ -7399,6 +7399,30 @@ end:
  *
  * Sets shader preset.
  **/
+/* Same bookkeeping as menu_shader_manager_set_preset's success path,
+ * but the menu shader is copied from the driver's already-loaded
+ * struct instead of re-parsing the preset chain from disk (two
+ * chain walks saved per apply). Also removes a latent index
+ * mismatch: the slang backends rebuild the parameter list by
+ * reflection, and menu actions index the live struct with positions
+ * from the menu's own copy - a copy of the driver's struct agrees
+ * with it by construction, where a fresh parse only happens to. */
+bool menu_shader_manager_set_preset_from_live(
+      struct video_shader *menu_shader,
+      const struct video_shader *live_shader)
+{
+   struct menu_state *menu_st = &menu_driver_state;
+
+   if (!menu_shader || !live_shader)
+      return false;
+
+   video_shader_copy_for_menu(menu_shader, live_shader);
+
+   menu_st->flags |= MENU_ST_FLAG_ENTRIES_NEED_REFRESH;
+   command_event(CMD_EVENT_SHADER_PRESET_LOADED, NULL);
+   return true;
+}
+
 bool menu_shader_manager_set_preset(struct video_shader *menu_shader,
       enum rarch_shader_type type, const char *preset_path, bool apply)
 {

menu/menu_shader.h

@@ -57,6 +57,13 @@ bool menu_shader_manager_init(void);
  *
  * Sets shader preset.
  **/
+/* Menu shader from the driver's loaded struct, no disk re-parse;
+ * see the definition for why this also removes an index-mapping
+ * hazard between reflection-built and parse-built parameter lists. */
+bool menu_shader_manager_set_preset_from_live(
+      struct video_shader *menu_shader,
+      const struct video_shader *live_shader);
+
 bool menu_shader_manager_set_preset(
       struct video_shader *menu_shader,
       enum rarch_shader_type type,

samples/gfx/shader_copy/Makefile

@@ -0,0 +1,10 @@
+TARGET := shader_copy_test
+
+all:
+	@sh ./build.sh
+
+clean:
+	rm -f $(TARGET) retroarch_nomain.o retroarch_nomain.d \
+	      harness_main.o harness_main.d
+
+.PHONY: all clean

samples/gfx/shader_copy/build.sh

@@ -0,0 +1,71 @@
+#!/bin/sh
+# Builds the shader copy-for-menu parity harness.
+#
+# Like playlist_nav, this links the SHIPPING objects of a built
+# RetroArch with only main() replaced - nothing is stubbed.  The seam
+# under test runs through rgui_frame's aspect-change detection, the staged config on the
+# rgui handle, and the application rgui_render performs on the main
+# thread before CMD_EVENT_VIDEO_SET_ASPECT_RATIO fires.
+# real frame and render code can see whether the write lands in the
+# configuration or dies with the frame's snapshot copy.
+#
+# Requires a completed NON-Qt build: with Qt enabled main() lives in
+# ui_qt.o, which drags the whole Qt UI in with it.  From the repo
+# root:
+#
+#   ./configure --disable-qt && make
+#   samples/gfx/shader_copy/build.sh
+#
+# retroarch.c is recompiled here with main renamed, so the harness can
+# supply its own without having to exclude the object that holds most
+# of the frontend.  Both the compile and the link reuse the project's
+# own command lines, taken from make -n, so the harness cannot drift
+# away from how the program is really built.
+set -eu
+
+here=$(cd "$(dirname "$0")" && pwd)
+root=$(cd "$here/../../.." && pwd)
+
+cd "$root"
+
+if [ ! -f obj-unix/release/retroarch.o ]; then
+   echo "build RetroArch first: ./configure --disable-qt && make" >&2
+   exit 1
+fi
+
+cc_line=$(mktemp)
+ld_line=$(mktemp)
+trap 'rm -f "$cc_line" "$ld_line"' EXIT
+
+# The project's own compile line for retroarch.o ...
+touch retroarch.c
+make -n 2>/dev/null | grep -E '\-o obj-unix/release/retroarch\.o' | head -1 > "$cc_line"
+if [ ! -s "$cc_line" ]; then
+   echo "could not determine the compile command for retroarch.o" >&2
+   exit 1
+fi
+
+# ... reused twice: once for retroarch.c with main renamed out of the
+# way, once for the harness itself, so both are built exactly the way
+# the frontend is.
+sed 's#-o obj-unix/release/retroarch\.o#-Dmain=rarch_harness_unused_main -o samples/gfx/shader_copy/retroarch_nomain.o#' \
+   "$cc_line" | sh
+
+sed -e 's#-o obj-unix/release/retroarch\.o#-o samples/gfx/shader_copy/harness_main.o#' \
+    -e 's# retroarch\.c# samples/gfx/shader_copy/shader_copy_test.c#' \
+   "$cc_line" | sh
+
+# The project's own link line, with retroarch.o swapped for the pair
+# above.  Removing the binary first is what makes make -n emit it.
+rm -f retroarch
+make -n 2>/dev/null | grep -E ' -o retroarch ' | tail -1 > "$ld_line"
+if [ ! -s "$ld_line" ]; then
+   echo "could not determine the link command" >&2
+   exit 1
+fi
+
+sed -e 's#obj-unix/release/retroarch\.o#samples/gfx/shader_copy/retroarch_nomain.o samples/gfx/shader_copy/harness_main.o#' \
+    -e 's#-o retroarch #-o samples/gfx/shader_copy/shader_copy_test #' \
+   "$ld_line" | sh
+
+echo "built samples/gfx/shader_copy/shader_copy_test"

samples/gfx/shader_copy/shader_copy_test.c

@@ -0,0 +1,149 @@
+/* video_shader_copy_for_menu() is a drop-in for the disk re-parse.
+ *
+ * Links the shipping objects with only main() replaced. A fixture
+ * preset chain exercises what the copy replaces: a child preset
+ * referencing a parent, whose pass source carries a
+ * #pragma parameter, so the parse walks the reference chain and
+ * resolves parameters exactly as the driver's own load did.
+ *
+ * Lanes:
+ *  - parse the chain into "driver" and plant heap source strings in
+ *    a pass, the way glsl/d3d compilation does;
+ *  - copy_for_menu into "menu": every byte equal except the two
+ *    driver-owned string pointers, which must be NULL in the copy
+ *    while the originals stay intact (freed once at the end - the
+ *    run being ASan-clean is the no-aliasing proof);
+ *  - the copy is independent: mutating a menu parameter leaves the
+ *    driver struct untouched.
+ *
+ *   ./configure --disable-qt && make
+ *   samples/gfx/shader_copy/build.sh
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <boolean.h>
+#include <time/rtime.h>
+#include <file/config_file.h>
+#include <streams/file_stream.h>
+
+#include "../../../configuration.h"
+#include "../../../retroarch.h"
+#include "../../../gfx/video_shader_parse.h"
+
+static unsigned failures = 0;
+
+#define CHECK(cond, ...) \
+   do { \
+      if (!(cond)) \
+      { \
+         fprintf(stderr, "FAIL %s:%d: ", __FILE__, __LINE__); \
+         fprintf(stderr, __VA_ARGS__); \
+         fprintf(stderr, "\n"); \
+         failures++; \
+      } \
+   } while (0)
+
+static struct video_shader drv;
+static struct video_shader menu;
+
+int main(int argc, char *argv[])
+{
+   char dir[512];
+   char child[640];
+   char parent[640];
+   char slang[640];
+   char cmd[700];
+   FILE *f;
+
+   (void)argc;
+   (void)argv;
+
+   snprintf(dir, sizeof(dir), "/tmp/shader_copy_%ld", (long)getpid());
+   snprintf(cmd, sizeof(cmd), "mkdir -p %s", dir);
+   if (system(cmd) != 0)
+      return 1;
+   snprintf(child,  sizeof(child),  "%s/child.slangp",  dir);
+   snprintf(parent, sizeof(parent), "%s/parent.slangp", dir);
+   snprintf(slang,  sizeof(slang),  "%s/pass.slang",    dir);
+
+   config_file_set_io_default(config_file_io_filestream());
+   rtime_init();
+   retroarch_config_init();
+
+   if ((f = fopen(slang, "wb")))
+   {
+      fputs("#version 450\n"
+            "#pragma parameter COPY_GAIN \"Gain\" 1.5 0.0 3.0 0.1\n"
+            "#pragma stage vertex\nvoid main(){}\n"
+            "#pragma stage fragment\nvoid main(){}\n", f);
+      fclose(f);
+   }
+   if ((f = fopen(parent, "wb")))
+   {
+      fputs("shaders = 1\nshader0 = \"pass.slang\"\n"
+            "COPY_GAIN = \"2.5\"\n", f);
+      fclose(f);
+   }
+   if ((f = fopen(child, "wb")))
+   {
+      fprintf(f, "#reference \"parent.slangp\"\n");
+      fclose(f);
+   }
+
+   /* The parse the driver would have done. */
+   CHECK(video_shader_load_preset_into_shader(child, &drv),
+         "fixture: reference-chain parse failed");
+   CHECK(drv.passes == 1, "fixture: passes %u", drv.passes);
+   CHECK(drv.num_parameters >= 1,
+         "fixture: no parameters resolved from the pass source");
+   if (drv.num_parameters >= 1)
+      CHECK(drv.parameters[0].current > 2.4f
+            && drv.parameters[0].current < 2.6f,
+            "fixture: referenced value not applied (%f)",
+            (double)drv.parameters[0].current);
+
+   /* Driver-side compilation state: heap source strings. */
+   drv.pass[0].source.string.vertex   = strdup("void main(){}");
+   drv.pass[0].source.string.fragment = strdup("void main(){}");
+
+   video_shader_copy_for_menu(&menu, &drv);
+
+   /* Byte parity, modulo the two cleared pointers. */
+   CHECK(menu.pass[0].source.string.vertex   == NULL
+      && menu.pass[0].source.string.fragment == NULL,
+         "copy aliases the driver-owned source strings");
+   CHECK(drv.pass[0].source.string.vertex != NULL,
+         "copy stole the driver's source strings");
+   {
+      struct video_shader drv_view = drv;
+      drv_view.pass[0].source.string.vertex   = NULL;
+      drv_view.pass[0].source.string.fragment = NULL;
+      CHECK(memcmp(&drv_view, &menu, sizeof(drv_view)) == 0,
+            "copy differs from the parse beyond the cleared strings");
+   }
+
+   /* Independence. */
+   if (menu.num_parameters >= 1)
+   {
+      menu.parameters[0].current = 0.25f;
+      CHECK(drv.parameters[0].current > 2.4f,
+            "menu edit reached the driver struct");
+   }
+
+   free(drv.pass[0].source.string.vertex);
+   free(drv.pass[0].source.string.fragment);
+
+   snprintf(cmd, sizeof(cmd), "rm -rf %s", dir);
+   if (system(cmd) != 0) { /* fixture dir left behind; harmless */ }
+
+   if (failures)
+   {
+      fprintf(stderr, "FAILURES (%u)\n", failures);
+      return 1;
+   }
+   printf("shader_copy: all lanes passed\n");
+   return 0;
+}