quality presets: one preset/key-ownership transaction for every settings save

The boot launcher, soft-return launcher and the in-game menu's live
settings now save settings.toml through psxq::settle_save:
- a one-run PSX_QUALITY override never persists: governed keys and the
  quality_* keys come from disk, so the previous preset or Custom values
  survive even when the boot seed was built empty;
- a governed live edit (psxq::live_edit) or launcher Custom becomes
  Custom from the current base, with the edited values, and survives
  restart instead of being masked away;
- a launcher pick records the preset; unrelated edits keep it.
Tests reproduce both review cases through a settings.toml round trip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

recompiler/tests/quality_presets_test.cpp

@@ -12,6 +12,7 @@
  */
 #include "config_loader.h"
 #include "quality_presets.h"
+#include "host_overlay.h"
 
 #include <cstdio>
 #include <cstring>
@@ -372,6 +373,134 @@ static void test_settings_round_trip() {
     fs::remove(p);
 }
 
+/* The preset/key-ownership transaction (psxq::settle_save), on the two cases
+ * from the #591 review, through a real settings.toml round trip. */
+static PSXRecompV4::UserSettings save_load(const PSXRecompV4::UserSettings& s) {
+    fs::path p = fs::temp_directory_path() / "psxrecomp_quality_txn.toml";
+    check(PSXRecompV4::save_user_settings(p, s), "txn: save");
+    PSXRecompV4::UserSettings r = PSXRecompV4::load_user_settings(p);
+    fs::remove(p);
+    return r;
+}
+
+/* What the next launch does with the file: decide, then mask while a preset
+ * (not Custom) is in force (main.cpp quality_preset_in_force). */
+static PSXRecompV4::UserSettings next_launch(const PSXRecompV4::UserSettings& disk,
+                                             const PsxHostInfo& h,
+                                             const std::vector<std::string>& keys,
+                                             psxq::Decision* out) {
+    psxq::Decision d = psxq::decide(0xFu, disk, h, nullptr, false, nullptr);
+    PSXRecompV4::UserSettings us = disk;
+    if (d.preset >= 0 && d.preset < PSX_QUALITY_COUNT) psxq::mask_user_settings(us, keys);
+    if (out) *out = d;
+    return us;
+}
+
+static void test_ownership_txn() {
+    const std::vector<std::string> keys = {"texture_filtering", "frame_generation",
+                                           "dynamic_resolution_min"};
+    PsxHostInfo m4 = host("Apple M4", "Apple M4", 10, 16);
+    char fp[17];
+    psx_quality_fingerprint(&m4, fp);
+
+    /* Saved Ultra on this machine. */
+    PSXRecompV4::UserSettings ultra;
+    ultra.has_quality_preset = true;   ultra.quality_preset = "ultra";
+    ultra.has_quality_base = true;     ultra.quality_base = "ultra";
+    ultra.has_quality_hardware = true; ultra.quality_hardware = fp;
+    ultra.has_quality_detected = true; ultra.quality_detected = "ultra";
+    ultra.has_scanlines = true;        ultra.scanlines = false;
+
+    /* Review case 1: PSX_QUALITY=low for one run, Settings opened and quit
+     * with nothing changed. The boot launcher's seed is built empty and
+     * filled with this run's (Low) values. */
+    {
+        psxq::Decision run = psxq::decide(0xFu, ultra, m4, "low", false, nullptr);
+        check(run.env_override && run.preset == PSX_QUALITY_LOW, "txn1: override in force");
+        PSXRecompV4::UserSettings seed;                    /* empty boot seed */
+        seed.has_texture_filter = true;  seed.texture_filter = 1;     /* Low: bilinear */
+        seed.has_frame_generation = true; seed.frame_generation = false;
+        seed.has_scanlines = true;       seed.scanlines = false;
+        psxq::settle_save(seed, ultra, run, psxq::Edit::NONE, PSX_QUALITY_NONE, keys);
+        PSXRecompV4::UserSettings disk = save_load(seed);
+        check(disk.quality_preset == "ultra" && disk.quality_base == "ultra",
+              "txn1: a one-run override never persists the preset");
+        check(!disk.has_texture_filter && !disk.has_frame_generation,
+              "txn1: nor the override's governed values (empty seed)");
+        psxq::Decision nd;
+        next_launch(disk, m4, keys, &nd);
+        check(nd.preset == PSX_QUALITY_ULTRA, "txn1: next launch is Ultra again");
+
+        /* Even a governed edit under the override applies this run only. */
+        PSXRecompV4::UserSettings edited = seed;
+        edited.texture_filter = 0;
+        psxq::Decision run2 = psxq::decide(0xFu, ultra, m4, "low", false, nullptr);
+        psxq::settle_save(edited, ultra, run2, psxq::Edit::GOVERNED_EDIT, PSX_QUALITY_NONE, keys);
+        check(edited.quality_preset == "ultra" && !edited.has_texture_filter,
+              "txn1: a governed edit under the override is not saved");
+    }
+    /* The override keeps a previous Custom profile and its values. */
+    {
+        PSXRecompV4::UserSettings custom = ultra;
+        custom.quality_preset = "custom";
+        custom.has_texture_filter = true; custom.texture_filter = 1;
+        psxq::Decision run = psxq::decide(0xFu, custom, m4, "low", false, nullptr);
+        PSXRecompV4::UserSettings seed;
+        seed.has_texture_filter = true; seed.texture_filter = 0;
+        psxq::settle_save(seed, custom, run, psxq::Edit::NONE, PSX_QUALITY_NONE, keys);
+        PSXRecompV4::UserSettings disk = save_load(seed);
+        check(disk.quality_preset == "custom" && disk.quality_base == "ultra" &&
+              disk.has_texture_filter && disk.texture_filter == 1,
+              "txn1: Custom and its values survive a one-run override");
+    }
+
+    /* Review case 2: a live in-game change to a governed value (texture
+     * filter) while Ultra is in force becomes Custom and survives restart. */
+    {
+        psxq::Decision run = psxq::decide(0xFu, ultra, m4, nullptr, false, nullptr);
+        PsxHostVideoSettings cur{}, next{};
+        cur.texture_filter = 0; next = cur; next.texture_filter = 1;
+        const psxq::Edit e = psxq::live_edit(cur, next, keys);
+        check(e == psxq::Edit::GOVERNED_EDIT, "txn2: texture filter is a governed live edit");
+        PSXRecompV4::UserSettings us = ultra;              /* the live path loads disk */
+        us.has_texture_filter = true; us.texture_filter = 1;
+        check(psxq::settle_save(us, ultra, run, e, PSX_QUALITY_NONE, keys),
+              "txn2: the state changes");
+        PSXRecompV4::UserSettings disk = save_load(us);
+        check(disk.quality_preset == "custom" && disk.quality_base == "ultra",
+              "txn2: a governed live edit becomes Custom from Ultra");
+        psxq::Decision nd;
+        PSXRecompV4::UserSettings eff = next_launch(disk, m4, keys, &nd);
+        check(nd.preset == PSX_QUALITY_CUSTOM && eff.has_texture_filter &&
+              eff.texture_filter == 1,
+              "txn2: the next launch keeps the manual texture filter");
+    }
+    /* An unrelated live edit (scanlines) keeps the preset. */
+    {
+        psxq::Decision run = psxq::decide(0xFu, ultra, m4, nullptr, false, nullptr);
+        PsxHostVideoSettings cur{}, next{};
+        next = cur; next.scanlines = 1;
+        const psxq::Edit e = psxq::live_edit(cur, next, keys);
+        check(e == psxq::Edit::NONE, "txn3: scanlines are not governed");
+        PSXRecompV4::UserSettings us = ultra;
+        us.scanlines = true;
+        us.has_texture_filter = true; us.texture_filter = 0;   /* the live path writes every row */
+        psxq::settle_save(us, ultra, run, e, PSX_QUALITY_NONE, keys);
+        check(us.quality_preset == "ultra" && us.scanlines && !us.has_texture_filter,
+              "txn3: unrelated edits keep the preset; its keys stay the preset's");
+    }
+    /* A launcher pick is recorded; the picked preset owns its keys. */
+    {
+        psxq::Decision run = psxq::decide(0xFu, ultra, m4, nullptr, false, nullptr);
+        PSXRecompV4::UserSettings seed;
+        seed.has_texture_filter = true; seed.texture_filter = 1;
+        psxq::settle_save(seed, ultra, run, psxq::Edit::PICK, PSX_QUALITY_MEDIUM, keys);
+        check(seed.quality_preset == "medium" && seed.quality_base == "medium" &&
+              seed.quality_hardware == fp && !seed.has_texture_filter,
+              "txn4: a picked preset is saved with the hardware it was picked on");
+    }
+}
+
 int main() {
     test_parse();
     test_classify();
@@ -379,6 +508,7 @@ int main() {
     test_decide();
     test_mask();
     test_settings_round_trip();
+    test_ownership_txn();
     if (failures) {
         std::fprintf(stderr, "quality_presets_test: %d failure(s)\n", failures);
         return 1;

runtime/include/quality_presets.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 namespace PSXRecompV4 { struct UserSettings; struct GameConfig; }
+struct PsxHostVideoSettings;
 
 namespace psxq {
 
@@ -47,4 +48,39 @@ void mask_user_settings(PSXRecompV4::UserSettings& us,
 /* Store a decision's quality_* keys in us (for save_user_settings). */
 void record(PSXRecompV4::UserSettings& us, const Decision& d);
 
+/* What the player did in a settings editor (boot launcher, soft-return
+ * launcher, or the in-game menu's live settings), as the frontend reports it.
+ * Both launchers report it through Settings.quality_preset: a tier picked =
+ * PICK, 5 (Custom) after a governed row changed = GOVERNED_EDIT. The live
+ * path compares the governed values it applies with the ones in force. */
+enum class Edit { NONE, GOVERNED_EDIT, PICK };
+
+/* The one preset/key-ownership transaction every settings.toml save that may
+ * carry governed keys goes through. `save` is what the editor wants to write
+ * (it may have been built from an empty seed); `disk` is settings.toml as it
+ * is now. On return `save` holds the governed keys and quality_* metadata
+ * that may be written, and `run` this run's state:
+ *  - a one-run PSX_QUALITY override never persists: the governed keys and
+ *    quality_* keys are restored from disk (so the previous preset or Custom
+ *    values survive), even after a governed edit, which applies this run only;
+ *  - PICK (not under an override): the picked preset is recorded; it owns its
+ *    keys, so their saved values are kept as they are on disk;
+ *  - GOVERNED_EDIT, or Custom already: Custom from the current base, with the
+ *    edited governed values, so the next launch keeps them;
+ *  - NONE with a preset in force: governed keys and quality_* keys as on disk.
+ * Unrelated keys in `save` are never touched. Returns true if the state in
+ * `run` changed (for the caller's log line). */
+bool settle_save(PSXRecompV4::UserSettings& save, const PSXRecompV4::UserSettings& disk,
+                 Decision& run, Edit edit, int picked,
+                 const std::vector<std::string>& keys);
+
+/* The live path's classification: GOVERNED_EDIT when a value the in-game
+ * menu changes (cur -> next) belongs to a governed key, else NONE. */
+Edit live_edit(const PsxHostVideoSettings& cur, const PsxHostVideoSettings& next,
+               const std::vector<std::string>& keys);
+
+/* Copy one governed key's settings.toml field(s) (value and presence). */
+void copy_key(PSXRecompV4::UserSettings& dst, const PSXRecompV4::UserSettings& src,
+              const std::string& key);
+
 }  // namespace psxq

runtime/src/main.cpp

@@ -1617,6 +1617,36 @@ static void quality_startup(PSXRecompV4::GameConfig& gc,
     }
 }
 
+/* The preset/key-ownership transaction (psxq::settle_save) in front of every
+ * save that may carry governed keys: boot launcher, soft-return launcher and
+ * the in-game menu's live settings. The pending edit is what the frontend
+ * reported since the last save. */
+static psxq::Edit g_quality_edit = psxq::Edit::NONE;
+static int g_quality_picked = PSX_QUALITY_NONE;
+static bool quality_save_user_settings(const std::filesystem::path& path,
+                                       PSXRecompV4::UserSettings& us) {
+    if (g_quality_offered) {
+        const PSXRecompV4::UserSettings disk = PSXRecompV4::load_user_settings(path);
+        if (!disk.parse_error) {
+            const bool was_override = g_quality.env_override;
+            if (psxq::settle_save(us, disk, g_quality, g_quality_edit, g_quality_picked,
+                                  g_quality_keys))
+                std::fprintf(stdout, "psxrecomp: graphics preset %s%s%s%s\n",
+                             psx_quality_name(g_quality.preset),
+                             g_quality.preset == PSX_QUALITY_CUSTOM ? " (from " : "",
+                             g_quality.preset == PSX_QUALITY_CUSTOM
+                                 ? psx_quality_name(g_quality.base) : "",
+                             g_quality.preset == PSX_QUALITY_CUSTOM ? ")" : "");
+            else if (was_override && g_quality_edit != psxq::Edit::NONE)
+                std::fprintf(stdout, "psxrecomp: graphics settings changed under "
+                             "PSX_QUALITY: this run only, not saved\n");
+        }
+    }
+    g_quality_edit = psxq::Edit::NONE;
+    g_quality_picked = PSX_QUALITY_NONE;
+    return PSXRecompV4::save_user_settings(path, us);
+}
+
 static void quality_apply_video_globals(const PSXRecompV4::RuntimeConfig& r) {
     g_video_scale        = r.video_supersampling;
     g_video_internal_res = r.video_internal_resolution;
@@ -9840,6 +9870,10 @@ extern "C" unsigned psx_video_apply_live(const PsxHostVideoSettings *next) {
     PsxHostVideoSettings cur;
     psx_video_current(&cur);
     unsigned m = psx_host_video_live_changes(&cur, next);
+    /* A governed value changed here makes the preset Custom when it is saved
+     * below (psxq::settle_save), so the next launch keeps it. */
+    if (g_quality_offered && g_quality_edit == psxq::Edit::NONE)
+        g_quality_edit = psxq::live_edit(cur, *next, g_quality_keys);
     unsigned deferred = 0;
     /* Netplay: dynamic resolution, frame generation and the filters change
      * how a peer renders mid-match; only the presentation of the frame we
@@ -9890,7 +9924,7 @@ extern "C" unsigned psx_video_apply_live(const PsxHostVideoSettings *next) {
             us.render_thread = next->render_thread != 0;       us.has_render_thread = true;
             us.present_thread = next->present_thread != 0;     us.has_present_thread = true;
             us.internal_resolution = next->internal_resolution; us.has_internal_resolution = true;
-            (void)PSXRecompV4::save_user_settings(g_overlay_settings_path, us);
+            (void)quality_save_user_settings(g_overlay_settings_path, us);
         } catch (...) {
         }
     }
@@ -16048,30 +16082,30 @@ namespace {
         ls->quality_base = g_quality.base >= 0 ? g_quality.base + 1 : 0;
     }
 
-    /* After the launcher: apply a newly picked preset's keys the launcher
-     * has no row for, and store the state for settings.toml. */
+    /* After the launcher: classify what the player did (Settings.quality_preset,
+     * the same contract in recomp-ui and recomp-launcher), and apply a newly
+     * picked preset's keys the launcher has no row for. The state itself is
+     * settled when settings.toml is saved (quality_save_user_settings). */
     void quality_take_launcher(const RecompLauncherCSettings* ls,
-                               PSXRecompV4::UserSettings* us, bool apply_now) {
+                               PSXRecompV4::UserSettings* /*us*/, bool apply_now) {
         if (!g_quality_offered || ls->quality_preset < 1 || ls->quality_preset > 5) return;
         const int preset = ls->quality_preset - 1;
-        const int base = (ls->quality_base >= 1 && ls->quality_base <= 4)
-            ? ls->quality_base - 1 : g_quality.base;
-        if (apply_now && base != g_quality.base) {
-            if (const auto* p = quality_preset_for(base)) {
+        if (preset == PSX_QUALITY_CUSTOM) {
+            if (g_quality.preset != PSX_QUALITY_CUSTOM) g_quality_edit = psxq::Edit::GOVERNED_EDIT;
+            return;
+        }
+        if (preset == g_quality.preset) return;
+        g_quality_edit = psxq::Edit::PICK;
+        g_quality_picked = preset;
+        if (apply_now && !g_quality.env_override) {
+            if (const auto* p = quality_preset_for(preset)) {
                 /* The launcher wrote this preset's values into its rows,
                  * which the launch path applies next; this covers the rest
                  * (supersample, PGXP extras, ...). */
                 quality_apply_video_globals(p->runtime);
-                std::fprintf(stdout, "psxrecomp: graphics preset %s\n", psx_quality_name(base));
+                std::fprintf(stdout, "psxrecomp: graphics preset %s\n", psx_quality_name(preset));
             }
         }
-        g_quality.preset = preset;
-        g_quality.base = base;
-        if (us) {
-            psxq::Decision d = g_quality;
-            d.detected_now = g_quality.detected_now;
-            psxq::record(*us, d);
-        }
     }
 #endif
 
@@ -19055,8 +19089,11 @@ int main(int argc, char** argv) {
 
             if (lr == 1) {
                 /* Keep settings changed and then dismissed with Quit. */
-                PSXRecompV4::save_user_settings(
-                    exe_dir_from_argv(argv[0]) / "settings.toml", seed);
+                {
+                    PSXRecompV4::UserSettings save = seed;
+                    (void)quality_save_user_settings(
+                        exe_dir_from_argv(argv[0]) / "settings.toml", save);
+                }
                 std::fprintf(stdout, "psxrecomp: launcher closed; exiting.\n");
                 if (overlay_init_thread.joinable())
                     overlay_init_thread.join();
@@ -19244,8 +19281,11 @@ int main(int argc, char** argv) {
                 }
                 g_video_win_w = seed.window_width;
                 /* Persist the user's choices next to the exe. */
-                PSXRecompV4::save_user_settings(
-                    exe_dir_from_argv(argv[0]) / "settings.toml", seed);
+                {
+                    PSXRecompV4::UserSettings save = seed;
+                    (void)quality_save_user_settings(
+                        exe_dir_from_argv(argv[0]) / "settings.toml", save);
+                }
             }
         }
     }
@@ -21503,7 +21543,7 @@ soft_return_lobby:
                     us.has_bios_path = false;
                     write_cached_path(argv[0], "bios.cfg", {});
                 }
-                (void)PSXRecompV4::save_user_settings(settings_path, us);
+                (void)quality_save_user_settings(settings_path, us);
             }
             g_video_renderer = ls.renderer;
             g_video_internal_res = ir.preset;

runtime/src/quality_presets.cpp

@@ -9,6 +9,8 @@
  * this); Custom is never overridden again, only re-detected on request. */
 #include "quality_presets.h"
 
+#include "host_overlay.h"
+
 #include "config_loader.h"
 
 #include <algorithm>
@@ -165,4 +167,85 @@ void record(PSXRecompV4::UserSettings& us, const Decision& d) {
     }
 }
 
+
+void copy_key(PSXRecompV4::UserSettings& d, const PSXRecompV4::UserSettings& s,
+              const std::string& k) {
+#define QCOPY(f) do { d.has_##f = s.has_##f; d.f = s.f; } while (0)
+    if (k == "internal_resolution" || k == "supersampling") {
+        QCOPY(internal_resolution); QCOPY(supersampling);
+    } else if (k == "dynamic_resolution") { QCOPY(dynamic_resolution);
+    } else if (k == "dynamic_resolution_min") { QCOPY(dynamic_resolution_min);
+    } else if (k == "frame_generation") { QCOPY(frame_generation);
+    } else if (k == "render_thread") { QCOPY(render_thread);
+    } else if (k == "present_thread") { QCOPY(present_thread);
+    } else if (k == "texture_filtering") { QCOPY(texture_filter);
+    } else if (k == "fmv_filter") { QCOPY(fmv_filter);
+    } else if (k == "antialiasing") { QCOPY(antialiasing);
+    } else if (k == "geometry_correction") { QCOPY(geometry_correction);
+    } else if (k == "perspective_texturing") { QCOPY(perspective_texturing);
+    } else if (k == "frame_interpolation") { QCOPY(frame_interpolation);
+    } else if (k == "frame_interpolation_fps") { QCOPY(frame_interpolation_fps);
+    }
+#undef QCOPY
+}
+
+Edit live_edit(const PsxHostVideoSettings& c, const PsxHostVideoSettings& n,
+               const std::vector<std::string>& keys) {
+    for (const std::string& k : keys) {
+        const bool diff =
+            (k == "texture_filtering" && c.texture_filter != n.texture_filter) ||
+            (k == "fmv_filter" && c.fmv_filter != n.fmv_filter) ||
+            (k == "antialiasing" && c.present_linear != n.present_linear) ||
+            (k == "dynamic_resolution" && c.dynamic_resolution != n.dynamic_resolution) ||
+            (k == "frame_generation" && c.frame_generation != n.frame_generation) ||
+            (k == "render_thread" && c.render_thread != n.render_thread) ||
+            (k == "present_thread" && c.present_thread != n.present_thread) ||
+            ((k == "internal_resolution" || k == "supersampling") &&
+             (c.internal_resolution != n.internal_resolution ||
+              c.supersampling != n.supersampling));
+        if (diff) return Edit::GOVERNED_EDIT;
+    }
+    return Edit::NONE;
+}
+
+static void copy_meta(PSXRecompV4::UserSettings& d, const PSXRecompV4::UserSettings& s) {
+    d.has_quality_preset = s.has_quality_preset;     d.quality_preset = s.quality_preset;
+    d.has_quality_base = s.has_quality_base;         d.quality_base = s.quality_base;
+    d.has_quality_hardware = s.has_quality_hardware; d.quality_hardware = s.quality_hardware;
+    d.has_quality_detected = s.has_quality_detected; d.quality_detected = s.quality_detected;
+}
+
+bool settle_save(PSXRecompV4::UserSettings& save, const PSXRecompV4::UserSettings& disk,
+                 Decision& run, Edit edit, int picked,
+                 const std::vector<std::string>& keys) {
+    if (run.preset == PSX_QUALITY_NONE) return false;   /* no presets offered */
+    auto keep_disk = [&] {
+        for (const std::string& k : keys) copy_key(save, disk, k);
+        copy_meta(save, disk);
+    };
+    if (run.env_override) {           /* one run: nothing quality-related persists */
+        keep_disk();
+        return false;
+    }
+    if (edit == Edit::PICK && picked >= 0 && picked < PSX_QUALITY_COUNT) {
+        const bool changed = run.preset != picked;
+        run.preset = run.base = picked;
+        for (const std::string& k : keys) copy_key(save, disk, k);
+        copy_meta(save, disk);
+        record(save, run);
+        return changed;
+    }
+    if (edit == Edit::GOVERNED_EDIT || run.preset == PSX_QUALITY_CUSTOM) {
+        const bool changed = run.preset != PSX_QUALITY_CUSTOM;
+        run.preset = PSX_QUALITY_CUSTOM;   /* base stays: Custom "from" it */
+        copy_meta(save, disk);
+        record(save, run);
+        return changed;
+    }
+    keep_disk();                          /* preset in force, nothing governed changed */
+    if (run.persist) record(save, run);
+    return false;
+}
+
 }  // namespace psxq
+