diff --git a/docs/design_tab_upstream_portability.md b/docs/design_tab_upstream_portability.md index a39bc46d47..a7e9cd84ed 100644 --- a/docs/design_tab_upstream_portability.md +++ b/docs/design_tab_upstream_portability.md @@ -98,7 +98,7 @@ std::string cad_recipe; // empty for non-CAD projects ``` ``` -Metadata/SnapOrca_cad.bin // written only when cad_recipe is non-empty +Metadata/orca_cad.bin // written only when cad_recipe is non-empty ``` Readers that do not know the entry ignore it; writers skip it entirely when empty. So @@ -107,8 +107,7 @@ existing projects are bit-identical and old readers are unaffected. Good. **But the moment upstream ships this, it owns forward-compatibility forever.** Three things should be settled *before* the first release, because none can be changed after: -1. **Name.** `SnapOrca_cad.bin` is fork-branded in an upstream project file. Rename to a - neutral path (e.g. `Metadata/cad_recipe.bin`). +1. **Name.** Renamed to `Metadata/orca_cad.bin`. 2. **Encoding.** The recipe is an opaque **cereal `PortableBinaryArchive`** blob whose layout is the field order of `CadFeature::serialize`. Portable across endianness and word size — *not* across a field reorder. Append-only is currently a convention held by diff --git a/src/libslic3r/CAD/CadDocument.cpp b/src/libslic3r/CAD/CadDocument.cpp index 52d64bb4c7..46f219c17d 100644 --- a/src/libslic3r/CAD/CadDocument.cpp +++ b/src/libslic3r/CAD/CadDocument.cpp @@ -3808,7 +3808,13 @@ bool CadDocument::deserialize_recipe(const std::string& blob) + std::to_string(ORCA_CAD_RECIPE_VERSION) + ")"; return false; } - if (v == 5) { + // The framed layout has been byte-identical since v5 (v6 advanced the stamp without + // touching the bytes), so every version from 5 up is read below. A future bump that DOES + // change the framing must exclude itself there — this is what forces that decision + // instead of letting a v7 blob be silently misread by the v5 reader. + static_assert(ORCA_CAD_RECIPE_VERSION <= 6, + "recipe version bumped: confirm the new version still uses the v5 framing"); + if (v >= 5) { // Framed path: every feature is a length-prefixed self-contained cereal stream, so // the same four lines handle BOTH directions of mismatch. Older file, newer build: // the sub-stream ends early, fa(f) throws, and the fields already assigned are kept diff --git a/src/libslic3r/CAD/CadDocument.hpp b/src/libslic3r/CAD/CadDocument.hpp index 3ee5dc566c..450e65d776 100644 --- a/src/libslic3r/CAD/CadDocument.hpp +++ b/src/libslic3r/CAD/CadDocument.hpp @@ -639,8 +639,10 @@ public: // v5: every feature is length-framed, so a reader can stop early on an older file and skip // the tail of a newer one. This is the LAST version that has to break anything — from here a // new field only needs appending to save/load, with no bump and no orphaned projects. - // v4 is still read, by the pre-framing flat path, so existing projects keep opening. - static constexpr uint32_t ORCA_CAD_RECIPE_VERSION = 5; + // v6: no wire-format change — the bytes are v5's, and both are read by the same framed path. + // The stamp advances only to put a project-container change on the record; the 3MF backends + // own that story. v4 still opens through the pre-framing flat path. + static constexpr uint32_t ORCA_CAD_RECIPE_VERSION = 6; std::string serialize_recipe() const; bool deserialize_recipe(const std::string& blob); diff --git a/src/libslic3r/Format/3mf.cpp b/src/libslic3r/Format/3mf.cpp index 6fbd3e3e11..d5a2d928e4 100644 --- a/src/libslic3r/Format/3mf.cpp +++ b/src/libslic3r/Format/3mf.cpp @@ -74,6 +74,9 @@ const std::string PRINT_CONFIG_FILE = "Metadata/Slic3r_PE.config"; const std::string MODEL_CONFIG_FILE = "Metadata/Slic3r_PE_model.config"; const std::string LAYER_HEIGHTS_PROFILE_FILE = "Metadata/Slic3r_PE_layer_heights_profile.txt"; const std::string CAD_RECIPE_FILE = "Metadata/orca_cad.bin"; +// Read-only: the recipe entry's pre-rename name. A reader that knows only the new one drops the +// feature tree of every project written before the move, without a word. Never written. +const std::string LEGACY_CAD_RECIPE_FILE = "Metadata/SnapOrca_cad.bin"; const std::string LAYER_CONFIG_RANGES_FILE = "Metadata/Prusa_Slicer_layer_config_ranges.xml"; const std::string SLA_SUPPORT_POINTS_FILE = "Metadata/Slic3r_PE_sla_support_points.txt"; const std::string SLA_DRAIN_HOLES_FILE = "Metadata/Slic3r_PE_sla_drain_holes.txt"; @@ -806,7 +809,8 @@ ModelVolumeType type_from_string(const std::string &s) return false; } } - if (boost::algorithm::iequals(name, CAD_RECIPE_FILE)) { + if (boost::algorithm::iequals(name, CAD_RECIPE_FILE) + || boost::algorithm::iequals(name, LEGACY_CAD_RECIPE_FILE)) { if (stat.m_uncomp_size > 0) { std::string buffer((size_t)stat.m_uncomp_size, 0); if (mz_zip_reader_extract_file_to_mem(&archive, stat.m_filename, diff --git a/src/libslic3r/Format/bbs_3mf.cpp b/src/libslic3r/Format/bbs_3mf.cpp index 2a084ecbb5..fe497c9223 100644 --- a/src/libslic3r/Format/bbs_3mf.cpp +++ b/src/libslic3r/Format/bbs_3mf.cpp @@ -215,6 +215,9 @@ const std::string SLICE_INFO_CONFIG_FILE = "Metadata/slice_info.config"; const std::string FILAMENT_SEQUENCE_FILE = "Metadata/filament_sequence.json"; const std::string BBS_LAYER_HEIGHTS_PROFILE_FILE = "Metadata/layer_heights_profile.txt"; const std::string ORCA_CAD_RECIPE_FILE = "Metadata/orca_cad.bin"; +// Read-only: the recipe entry's pre-rename name. A reader that knows only the new one drops the +// feature tree of every project written before the move, without a word. Never written. +const std::string LEGACY_CAD_RECIPE_FILE = "Metadata/SnapOrca_cad.bin"; const std::string LAYER_CONFIG_RANGES_FILE = "Metadata/layer_config_ranges.xml"; const std::string BRIM_EAR_POINTS_FILE = "Metadata/brim_ear_points.txt"; /*const std::string SLA_SUPPORT_POINTS_FILE = "Metadata/Slic3r_PE_sla_support_points.txt"; @@ -1971,7 +1974,8 @@ void PlateData::parse_filament_info(GCodeProcessorResult *result) // extract slic3r print config file _extract_project_config_from_archive(archive, stat, config, config_substitutions, model); } - else if (boost::algorithm::iequals(name, ORCA_CAD_RECIPE_FILE)) { + else if (boost::algorithm::iequals(name, ORCA_CAD_RECIPE_FILE) + || boost::algorithm::iequals(name, LEGACY_CAD_RECIPE_FILE)) { // Restore the editable CAD recipe (optional; absent in non-CAD projects). if (stat.m_uncomp_size > 0) { std::string buf((size_t)stat.m_uncomp_size, '\0'); diff --git a/tests/libslic3r/test_3mf.cpp b/tests/libslic3r/test_3mf.cpp index 6a4a28f7c4..93de4c8dbd 100644 --- a/tests/libslic3r/test_3mf.cpp +++ b/tests/libslic3r/test_3mf.cpp @@ -5,6 +5,7 @@ #include "libslic3r/Format/bbs_3mf.hpp" #include "libslic3r/Format/STL.hpp" #include "libslic3r/miniz_extension.hpp" +#include "libslic3r/Zipper.hpp" #include "libslic3r/PrintConfig.hpp" #include "libslic3r/Semver.hpp" #include "libslic3r/Preset.hpp" @@ -154,8 +155,12 @@ static std::string make_cad_recipe() return std::string(blob, sizeof(blob) - 1); } -// Pulls Metadata/orca_cad.bin out of a 3mf archive; false when the entry is absent. -static bool read_cad_recipe_entry(const std::string& path, std::string& out) +static const std::string CAD_RECIPE_ENTRY = "Metadata/orca_cad.bin"; +static const std::string LEGACY_CAD_RECIPE_ENTRY = "Metadata/SnapOrca_cad.bin"; + +// Pulls one named entry out of a 3mf archive; false when it is absent. +static bool read_cad_recipe_entry(const std::string& path, std::string& out, + const std::string& entry = CAD_RECIPE_ENTRY) { mz_zip_archive zip; mz_zip_zero_struct(&zip); @@ -167,7 +172,7 @@ static bool read_cad_recipe_entry(const std::string& path, std::string& out) if (!mz_zip_reader_file_stat(&zip, i, &st)) continue; std::string name(st.m_filename); std::replace(name.begin(), name.end(), '\\', '/'); - if (boost::algorithm::iequals(name, std::string("Metadata/orca_cad.bin"))) { + if (boost::algorithm::iequals(name, entry)) { out.resize(st.m_uncomp_size); found = mz_zip_reader_extract_to_mem(&zip, i, out.data(), out.size(), 0) != 0; break; @@ -177,6 +182,48 @@ static bool read_cad_recipe_entry(const std::string& path, std::string& out) return found; } +// Rewrites the archive at `path` with the recipe entry back under the name it had before the +// rename, which is what every project saved by an earlier build looks like on disk. Generated +// rather than checked in because a whole project archive is not frozen evidence the way a bare +// recipe blob is -- it has to be whatever today's exporter writes, with only the name aged. +// miniz cannot rename in place and open_zip_writer truncates, so the entries are held across +// the switch. +static void rename_cad_recipe_entry_to_legacy(const std::string& path) +{ + std::vector> entries; + bool renamed = false; + { + mz_zip_archive zip; + mz_zip_zero_struct(&zip); + REQUIRE(open_zip_reader(&zip, path)); + mz_uint n = mz_zip_reader_get_num_files(&zip); + for (mz_uint i = 0; i < n; ++i) { + mz_zip_archive_file_stat st; + REQUIRE(mz_zip_reader_file_stat(&zip, i, &st)); + if (st.m_is_directory) continue; + std::string name(st.m_filename); + std::replace(name.begin(), name.end(), '\\', '/'); + std::string data((size_t) st.m_uncomp_size, '\0'); + if (st.m_uncomp_size > 0) + REQUIRE(mz_zip_reader_extract_to_mem(&zip, i, data.data(), data.size(), 0)); + if (boost::algorithm::iequals(name, CAD_RECIPE_ENTRY)) { + name = LEGACY_CAD_RECIPE_ENTRY; + renamed = true; + } + entries.emplace_back(std::move(name), std::move(data)); + } + close_zip_reader(&zip); + } + // Without this the scenario would degrade silently into re-testing the new name if the + // exporter's constant ever moved again: every load below would still pass. + REQUIRE(renamed); + + Zipper out(path); + for (const auto& e : entries) + out.add_entry(e.first, e.second.data(), e.second.size()); + out.finalize(); +} + SCENARIO("CAD recipe blob survives a 3mf save/load cycle", "[3mf][CAD]") { GIVEN("a model carrying a binary cad_recipe") { Model src_model; @@ -301,6 +348,89 @@ SCENARIO("CAD recipe is embedded in the BBS 3mf archive", "[3mf][CAD]") { } } +// The recipe entry was renamed from Metadata/SnapOrca_cad.bin to Metadata/orca_cad.bin. Nothing +// in the blob marks that move, so a reader that knows only the new name loads a project written +// before it with an empty cad_recipe and no error at all — a feature tree gone with no symptom +// but an empty Design tab. Both backends must still accept the old name; neither may write it. +SCENARIO("a project saved under the pre-rename recipe name still loads", "[3mf][CAD]") { + GIVEN("a project whose recipe entry carries the old name") { + Model model; + std::string src = std::string(TEST_DATA_DIR) + "/test_3mf/Prusa.stl"; + REQUIRE(load_stl(src.c_str(), &model)); + model.add_default_instances(); + const std::string recipe = make_cad_recipe(); + model.cad_recipe = recipe; + + WHEN("it was written by the PrusaSlicer-format backend") { + ScopedTemporaryFile temp(".3mf"); + const std::string test_file = temp.string(); + REQUIRE(store_3mf(test_file.c_str(), &model, nullptr, false)); + rename_cad_recipe_entry_to_legacy(test_file); + + THEN("the recipe still comes back byte-for-byte") { + Model dst_model; + DynamicPrintConfig dst_config; + ConfigSubstitutionContext ctxt{ ForwardCompatibilitySubstitutionRule::Disable }; + REQUIRE(load_3mf(test_file.c_str(), dst_config, ctxt, &dst_model, false)); + REQUIRE(dst_model.cad_recipe == recipe); + } + } + + WHEN("it was written by the BBS backend (the format the GUI uses)") { + ScopedTemporaryDir backup_dir("orca_cad_legacy"); + model.set_backup_path(backup_dir.string()); + + ScopedTemporaryFile temp(".3mf"); + const std::string test_file = temp.string(); + DynamicPrintConfig cfg; + StoreParams sp; + sp.path = test_file.c_str(); + sp.model = &model; + sp.config = &cfg; + sp.strategy = SaveStrategy::Zip64 | SaveStrategy::Silence; + REQUIRE(store_bbs_3mf(sp)); + rename_cad_recipe_entry_to_legacy(test_file); + + // Catch2 replays the enclosing sections per THEN, so one load here serves both. + Model dst_model; + ScopedTemporaryDir dst_backup_dir("orca_cad_legacy_dst"); + dst_model.set_backup_path(dst_backup_dir.string()); + + DynamicPrintConfig dst_config; + ConfigSubstitutionContext ctxt{ ForwardCompatibilitySubstitutionRule::Enable }; + PlateDataPtrs dst_plates; + std::vector project_presets; + bool is_bbl_3mf = false, is_orca_3mf = false; + Semver file_version; + REQUIRE(load_bbs_3mf(test_file.c_str(), &dst_config, &ctxt, &dst_model, &dst_plates, + &project_presets, &is_bbl_3mf, &is_orca_3mf, &file_version, nullptr, + LoadStrategy::LoadModel | LoadStrategy::LoadConfig)); + release_PlateData_list(dst_plates); + + THEN("the recipe still comes back byte-for-byte") { + REQUIRE(dst_model.cad_recipe == recipe); + } + + THEN("re-saving migrates it to the new name and leaves the old one behind") { + ScopedTemporaryFile again(".3mf"); + const std::string resaved = again.string(); + DynamicPrintConfig cfg2; + StoreParams sp2; + sp2.path = resaved.c_str(); + sp2.model = &dst_model; + sp2.config = &cfg2; + sp2.strategy = SaveStrategy::Zip64 | SaveStrategy::Silence; + REQUIRE(store_bbs_3mf(sp2)); + + std::string got; + REQUIRE(read_cad_recipe_entry(resaved, got)); + REQUIRE(got == recipe); + REQUIRE_FALSE(read_cad_recipe_entry(resaved, got, LEGACY_CAD_RECIPE_ENTRY)); + } + } + } +} + // .3mf multi-nozzle round-trip. // Locks the load/save handling for the H2C multi-nozzle plate metadata: // * filament_volume_maps -> plate config "filament_volume_map" (with the >1 -> 0 clamp) diff --git a/tests/libslic3r/test_caddocument.cpp b/tests/libslic3r/test_caddocument.cpp index 2746ce655e..356886c557 100644 --- a/tests/libslic3r/test_caddocument.cpp +++ b/tests/libslic3r/test_caddocument.cpp @@ -2159,6 +2159,33 @@ TEST_CASE("a v4 project still opens", "[CadDocument][recipe]") REQUIRE(doc.features.size() == flat.size()); } +// Do NOT regenerate cad_recipe_v5.bin either. It was written by the build that predates the +// bump to v6, which is the whole reason it can prove the gate lets a v5 project through. +TEST_CASE("a v5 project still opens", "[CadDocument][recipe]") +{ + // The bump to v6 renamed nothing inside the blob — it records the recipe's move to + // Metadata/orca_cad.bin — so a v5 project must still come through the framed path rather + // than be refused at the version gate for being old. + std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v5.bin"; + std::ifstream ifs(path, std::ios::binary); + REQUIRE(ifs.is_open()); + std::string blob((std::istreambuf_iterator(ifs)), std::istreambuf_iterator()); + ifs.close(); + REQUIRE(blob.size() >= sizeof(uint32_t)); + + uint32_t stamped = 0; + std::memcpy(&stamped, blob.data(), sizeof(stamped)); + REQUIRE(stamped == 5); // the fixture really is pre-bump, not a regenerated one + + CadDocument doc; + doc.deserialize_recipe(blob); + // As with the v4 fixture: the only failure allowed is the golden tree's own geometry, which + // is orthogonal to the version gate. Being turned away at the gate is not. + REQUIRE(doc.error.find("older version") == std::string::npos); + REQUIRE(doc.error.find("newer version") == std::string::npos); + REQUIRE_FALSE(doc.features.empty()); +} + TEST_CASE("a v5 round trip is exact", "[CadDocument][recipe]") { using Catch::Matchers::WithinRel; @@ -2206,7 +2233,7 @@ TEST_CASE("a v5 round trip is exact", "[CadDocument][recipe]") TEST_CASE("a truncated feature keeps what it could read", "[CadDocument][recipe]") { - // The forward-compat proof: a v5 reader that meets a feature blob shorter than its own + // The forward-compat proof: a framed reader that meets a feature blob shorter than its own // field list must keep what it read and default the rest, not error. Simulate an older // file by hand-shortening ONE feature's frame: rewrite its length prefix and drop the // tail bytes, then confirm the load still succeeds and the fields before the cut survive. @@ -2238,7 +2265,9 @@ TEST_CASE("a truncated feature keeps what it could read", "[CadDocument][recipe] std::memcpy(&s[off], &u, sizeof(u)); }; - REQUIRE(rd32(blob, 0) == 5); + // Against the constant, not a literal: what this asserts is that the outer frame opens with + // the version stamp, which stays true across every bump. + REQUIRE(rd32(blob, 0) == CadDocument::ORCA_CAD_RECIPE_VERSION); uint32_t count = rd32(blob, 4); REQUIRE(count == doc.features.size()); @@ -3934,7 +3963,10 @@ TEST_CASE("regenerate golden recipe fixture", "[.regen]") auto blob = doc.serialize_recipe(); REQUIRE_FALSE(blob.empty()); - std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v5.bin"; + // Version-free on purpose: this writes whatever today's format is, so a version bump does + // not have to remember to edit it. The version-stamped fixtures beside it are frozen + // evidence from older builds and are never regenerated. + std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_current.bin"; std::ofstream ofs(path, std::ios::binary); REQUIRE(ofs.is_open()); ofs.write(blob.data(), static_cast(blob.size()));