From 2063f0b2a707ccd374193b44a63ef95d3a68dc67 Mon Sep 17 00:00:00 2001 From: Hanif Koh Date: Tue, 13 Oct 2026 18:21:21 +0800 Subject: [PATCH] Back Up Before the User Presets Lock and Merge Migrated Printers Into the Live List The lock guard is meant for a few file operations, and zipping a large user folder is not one, so the backup is taken before it and the guard covers the writes only; each write re-reads its file, so a save that slipped in before is still respected. The in-memory update adds the lost printers to the list the preset holds at that moment rather than replacing it with the one found at detection, and leaves a pending cloud sync state alone: a download waiting to be saved kept its newer list and its "save" flag, which "update" would have cancelled. The lock message no longer blames another instance alone, since the lock is also refused for a while after a miss and when the lock file cannot be opened. --- src/libslic3r/Preset.cpp | 17 +++++++--- src/libslic3r/PresetBundle.cpp | 17 +++++----- src/libslic3r/PresetBundle.hpp | 6 ++-- src/slic3r/GUI/GUI_App.cpp | 11 ++++--- .../libslic3r/test_preset_bundle_loading.cpp | 33 +++++++++++++++---- 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/src/libslic3r/Preset.cpp b/src/libslic3r/Preset.cpp index f4e6de9b76..3f4eb787eb 100644 --- a/src/libslic3r/Preset.cpp +++ b/src/libslic3r/Preset.cpp @@ -2846,16 +2846,23 @@ void PresetCollection::apply_legacy_user_filament_presets(const std::vector &saved = it->config.option("compatible_printers", true)->values; std::vector &edited = m_edited_preset.config.option("compatible_printers", true)->values; + // Added to the list the preset holds now rather than replacing it: the cloud sync may have given the + // preset a newer list since it was found, and that list is still to be written out. + std::vector printers = saved; + for (const std::string &printer : legacy.printers) + if (std::find(printers.begin(), printers.end(), printer) == printers.end()) + printers.push_back(printer); // A change to the list pending in the settings tab stays pending. Without one, the edited copy and the // snapshot unsaved changes are measured against take the written list, so nothing reads as modified. if (m_idx_selected == static_cast(it - m_presets.begin()) && edited == saved) { - edited = legacy.compatible_printers; + edited = printers; if (m_saved_preset.name == it->name) - m_saved_preset.config.option("compatible_printers", true)->values = legacy.compatible_printers; + m_saved_preset.config.option("compatible_printers", true)->values = printers; } - saved = legacy.compatible_printers; - // Push the change to the cloud copy, as an edit in the settings tab would. - if (!it->setting_id.empty()) { + saved = printers; + // Push the change to the cloud copy, as an edit in the settings tab would. A sync already pending for + // the preset (a download waiting to be saved, a deletion) keeps its state. + if (!it->setting_id.empty() && it->sync_info.empty()) { it->sync_info = "update"; it->save_info(); } diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index 0b00eb35d4..b339449c9d 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -1105,20 +1105,21 @@ PresetBundle::LegacyUserFilamentPresetMigration PresetBundle::migrate_legacy_use { LegacyUserFilamentPresetMigration result; std::vector written; + const std::string zip_path = data_dir() + "/filament_preset_migration_" + + Utils::time2str(Utils::TimeZone::utc, Utils::TimeFormat::iso8601Z) + "_backup.zip"; + // Before the lock: zipping a large folder takes longer than a guard may be held. + if (!backup_user_folder_to_zip(zip_path)) + return result; + result.backup = zip_path; { - // One hold for the backup and every write: no other instance can save a preset in between, and a lock - // the other instance keeps is one outcome rather than a failure per preset (the lock backs off for a - // while after a miss). Held without the collection mutex, the order InstanceLock requires. + // One hold for every write, so a lock another instance keeps is one outcome rather than a failure per + // preset (the lock backs off for a while after a miss). Held without the collection mutex, the order + // InstanceLock requires; each write re-reads its file, so a save that slipped in before is respected. const std::string lock_path = user_presets_lock_path(); InstanceLock instance_lock(lock_path); if (!lock_path.empty() && !instance_lock.locked()) return result; result.locked = true; - const std::string zip_path = data_dir() + "/filament_preset_migration_" + - Utils::time2str(Utils::TimeZone::utc, Utils::TimeFormat::iso8601Z) + "_backup.zip"; - if (!backup_user_folder_to_zip(zip_path)) - return result; - result.backup = zip_path; for (const LegacyUserFilamentPreset &legacy : legacy_presets) if (PresetCollection::write_legacy_user_filament_preset(legacy)) written.push_back(legacy); diff --git a/src/libslic3r/PresetBundle.hpp b/src/libslic3r/PresetBundle.hpp index 4a3dd057c2..8da0e0e196 100644 --- a/src/libslic3r/PresetBundle.hpp +++ b/src/libslic3r/PresetBundle.hpp @@ -382,9 +382,9 @@ public: // User filament presets made against an older release that no longer show on printers their parent // listed then (resources/info/legacy_compatible_printers.json). std::vector legacy_user_filament_presets(); - // Under one hold of the user presets lock, backs up the user preset folder into a zip in data_dir() and writes - // each preset's printers into its file. Nothing is written when the lock is held elsewhere (locked false) or - // the backup failed (backup empty); otherwise failed names the presets whose files could not be written. + // Backs up the user preset folder into a zip in data_dir(), then writes each preset's printers into its file + // under one hold of the user presets lock. Nothing is written when the backup failed (backup empty) or the + // lock could not be taken (locked false); otherwise failed names the presets whose files could not be written. struct LegacyUserFilamentPresetMigration { bool locked{false}; std::string backup; diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index a1f9abfe0a..045bcf2f3c 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -6850,17 +6850,18 @@ void GUI_App::check_legacy_user_filament_presets(bool by_user) return; const PresetBundle::LegacyUserFilamentPresetMigration migration = preset_bundle->migrate_legacy_user_filament_presets(legacy_presets); - if (!migration.locked) { - show_error(mainframe, _L("Another OrcaSlicer instance is saving presets right now, so nothing was migrated. " - "Try again later from Help > Migrate Filament Presets.")); - return; - } const std::string &backup = migration.backup; const std::vector &failed = migration.failed; if (backup.empty()) { show_error(mainframe, _L("Your presets could not be backed up, so they were not migrated.")); return; } + if (!migration.locked) { + // The lock is also refused for a while after a miss, and when the lock file itself cannot be opened. + show_error(mainframe, _L("The user presets folder could not be locked, so nothing was migrated. Another OrcaSlicer " + "instance may be saving presets; try again later from Help > Migrate Filament Presets.")); + return; + } if (failed.size() == legacy_presets.size()) { show_error(mainframe, format_wxstr(_L("Your filament presets could not be migrated. A backup of your presets was saved to:\n%1%"), backup)); return; diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index edba12c2b1..a611ea0a84 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -7425,18 +7425,39 @@ TEST_CASE("Migrating user filaments backs up and writes nothing while it can not REQUIRE(legacy_presets.size() == 1); // A folder in the lock file's place can not be opened as the lock; a fresh data dir, since loading has - // already opened the lock file of the first one. One hold covers the backup and every write, so a lock - // held elsewhere is one outcome, not a failure per preset. + // already opened the lock file of the first one. The backup is taken before the lock, then one hold covers + // every write, so a lock held elsewhere is one outcome, not a failure per preset. const fs::path other_data = temp_dir.path() / "data"; - ScopedDataDir data_for_migration(other_data); + fs::create_directories(other_data / PRESET_USER_DIR); + ScopedDataDir data_for_migration(other_data); fs::create_directories(fs::path(user_presets_lock_path())); const PresetBundle::LegacyUserFilamentPresetMigration migration = bundle.migrate_legacy_user_filament_presets(legacy_presets); + CHECK_FALSE(migration.backup.empty()); CHECK_FALSE(migration.locked); - CHECK(migration.backup.empty()); CHECK(migration.failed.empty()); CHECK(read_file(file) == original); - for (const fs::directory_entry &entry : fs::directory_iterator(other_data)) - CHECK(entry.path().extension() != ".zip"); +} + +TEST_CASE("Applying a migration adds the printers to the list the preset holds now and keeps a pending cloud sync", "[Preset][Bundle]") +{ + ScopedTemporaryDir temp_dir; + RenameTestFilaments filaments; + add_system_filament(filaments, k1_parent, { "Creality K1C 0.6 nozzle" }); + write_user_filament(temp_dir.path(), "My PETG", k1_parent); + load_user_filaments(filaments, temp_dir.path()); + const std::vector legacy_presets = + filaments.legacy_user_filament_presets({ { k1_parent, { "Creality K1C 0.4 nozzle" } } }); + REQUIRE(legacy_presets.size() == 1); + + // Between detection and the write, the cloud sync gave the preset a newer list that is still to be saved. + Preset *preset = filaments.find_preset("My PETG", false, true); + preset->config.option("compatible_printers", true)->values = { "Creality K1C 0.8 nozzle" }; + preset->setting_id = "GFSA01"; + preset->sync_info = "save"; + filaments.apply_legacy_user_filament_presets(legacy_presets); + CHECK(preset->config.option("compatible_printers")->values == + std::vector{ "Creality K1C 0.8 nozzle", "Creality K1C 0.4 nozzle" }); + CHECK(preset->sync_info == "save"); } TEST_CASE("Migrating the selected user filament keeps a printer list edit pending in the settings tab", "[Preset][Bundle]")