From 0e7fcb3daf039e5e7213bf06e06dd6eeb23280d3 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Thu, 13 Aug 2026 09:36:21 +0200 Subject: [PATCH] Recipe v5: length-frame every feature, so the format stops orphaning projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every version bump so far has permanently orphaned every project saved before it. deserialize_recipe refused anything that was not exactly the current version, and with no migration path v2 and v3 projects are unopenable today — the 3MF still carries the mesh, so the user gets a frozen solid and no feature history, which is the whole point of the subsystem silently absent. The cause was the shape of the data, not the gate. save/load is one flat symmetric list of ~90 fields with no framing, so a reader has no way to know where a feature ends unless it agrees on every field. Each feature is now written as its own cereal stream behind a length prefix, and the same few lines handle both directions of mismatch. Older file, newer build: the sub-stream ends early, the read throws, and the fields already assigned are kept while the rest default — cereal assigns sequentially, so a mid-list throw leaves the earlier fields set, and that is what makes this work. Newer file, older build: the sub-stream holds more bytes than the reader knows; it reads what it knows and stops, and the outer stream is untouched because the length prefix was consumed in full. A field a project predates is not a corrupt project, so neither case is an error. v4 keeps its own pre-framing flat path and opens exactly as before — cad_recipe_v4.bin is untouched and now serves as the witness for that. v2 and v3 stay refused, by name: their field lists no longer exist in this code. This fixes the future, not the past, and the comment says so rather than implying otherwise. From here a new field only needs appending to save/load — no bump, no orphaned projects. That removes the cost that had blocked snaporca-44m and snaporca-dgv. The helix round-trip test was reading the blob back flat, reaching into the format instead of through it; framing necessarily breaks that, so it now goes through deserialize_recipe, which is a stronger assertion than it made before. Every field check it carried is unchanged. Tests: four new [CadDocument][recipe] cases, including the one the change exists for — a deliberately truncated feature blob must LOAD, keeping what it could read. Suite 177 -> 181 cases, 2366 -> 2411 assertions. snaporca-2txy. --- src/libslic3r/CadDocument.cpp | 85 ++++++++++-- src/libslic3r/CadDocument.hpp | 12 +- tests/data/cad_recipe_v5.bin | Bin 0 -> 35064 bytes tests/libslic3r/test_caddocument.cpp | 186 +++++++++++++++++++++++++-- 4 files changed, 255 insertions(+), 28 deletions(-) create mode 100644 tests/data/cad_recipe_v5.bin diff --git a/src/libslic3r/CadDocument.cpp b/src/libslic3r/CadDocument.cpp index 422616d922..c9a001fb78 100644 --- a/src/libslic3r/CadDocument.cpp +++ b/src/libslic3r/CadDocument.cpp @@ -3595,8 +3595,28 @@ std::string CadDocument::serialize_recipe() const cereal::BinaryOutputArchive ar(oss); uint32_t v = SNAPORCA_CAD_RECIPE_VERSION; ar(v); - ar(features); - ar(variables); + uint32_t n = static_cast(features.size()); + ar(n); + for (const CadFeature& f : features) { + std::ostringstream fos; + { + cereal::BinaryOutputArchive fa(fos); + fa(f); + } + std::string fb = fos.str(); + uint32_t len = static_cast(fb.size()); + ar(len); + ar(cereal::binary_data(fb.data(), fb.size())); + } + std::ostringstream vos; + { + cereal::BinaryOutputArchive va(vos); + va(variables); + } + std::string vb = vos.str(); + uint32_t vlen = static_cast(vb.size()); + ar(vlen); + ar(cereal::binary_data(vb.data(), vb.size())); } return oss.str(); } @@ -3615,14 +3635,61 @@ bool CadDocument::deserialize_recipe(const std::string& blob) + std::to_string(SNAPORCA_CAD_RECIPE_VERSION) + ")"; return false; } - if (v < SNAPORCA_CAD_RECIPE_VERSION) { - error = "saved with an older version of SnapOrca CAD (format v" - + std::to_string(v) + "); this project cannot be opened by this build"; - return false; + 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 + // (cereal assigns sequentially, so a mid-list throw leaves the earlier fields set) + // while the rest default. Newer file, older build: the sub-stream has MORE bytes + // than this build knows how to read; it reads what it knows and simply stops, and + // the outer stream is unaffected because the length prefix was consumed in full. + features.clear(); + uint32_t count; + ar(count); + for (uint32_t i = 0; i < count; ++i) { + uint32_t len; + ar(len); + std::string buf(len, '\0'); + if (len > 0) + ar(cereal::binary_data(&buf[0], len)); // consume EXACTLY len bytes from the outer stream + CadFeature f; + try { + std::istringstream fs(buf); + cereal::BinaryInputArchive fa(fs); + fa(f); + } catch (...) { + // An OLDER file: the blob ran out before this build's field list did. Everything read + // so far is kept and the remaining fields stay at their defaults. Deliberately NOT an + // error — a field a project predates is not a corrupt project. + } + features.push_back(f); + } + variables.clear(); + uint32_t vlen; + ar(vlen); + std::string vbuf(vlen, '\0'); + if (vlen > 0) + ar(cereal::binary_data(&vbuf[0], vlen)); + try { + std::istringstream vs(vbuf); + cereal::BinaryInputArchive va(vs); + va(variables); + } catch (...) { + // Variables blob predates this build: keep what was read, default the rest. + } + return recompute(); } - ar(features); - ar(variables); - return recompute(); + if (v == 4) { + // Pre-framing flat path, unchanged: v4 projects keep opening exactly as before. + ar(features); + ar(variables); + return recompute(); + } + // v < 4: the field lists for v2/v3 no longer exist in this code, so those files + // cannot be recovered here. This fix is for the future, not the past. + error = "saved with an older version of SnapOrca CAD (format v" + + std::to_string(v) + "); this project cannot be opened by this build"; + return false; } catch (const Standard_Failure& e) { const char* what = e.GetMessageString(); error = std::string("CAD data could not be read") diff --git a/src/libslic3r/CadDocument.hpp b/src/libslic3r/CadDocument.hpp index 14448e0428..049ba43b3f 100644 --- a/src/libslic3r/CadDocument.hpp +++ b/src/libslic3r/CadDocument.hpp @@ -616,7 +616,6 @@ public: bool recompute(); // replay features -> body + display_mesh; false on error // CadRecipe serialization contract: - // - bump this whenever CadFeature::save/load gains or loses a field // - v1 blobs are deliberately not loadable; there is no migration path by design // - append fields ONLY at the end of save/load, never reorder (golden fixture enforces this) // Bumped every time the bodies are rebuilt, i.e. every time the face and edge MAPS change. @@ -629,12 +628,11 @@ public: // ids themselves cannot keep. uint64_t topo_generation{1}; - // v4: coordsys_face_kind + coordsys_face_edges appended (connector face-drift fingerprint). - // The bump is not optional. deserialize_recipe() gates on v == VERSION and then reads a FLAT - // symmetric field list, so a v3 blob under a v3 build that has grown two fields passes the - // gate and then reads two ints past the end of every connector — straight into the next - // feature's bytes. That is silent corruption of a saved project, not a load error. - static constexpr uint32_t SNAPORCA_CAD_RECIPE_VERSION = 4; + // 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 SNAPORCA_CAD_RECIPE_VERSION = 5; std::string serialize_recipe() const; bool deserialize_recipe(const std::string& blob); diff --git a/tests/data/cad_recipe_v5.bin b/tests/data/cad_recipe_v5.bin new file mode 100644 index 0000000000000000000000000000000000000000..718f2d2d844044d946fbf743b9f60b0ecb0e8229 GIT binary patch literal 35064 zcmeHPOK%)S5Z*un0m36-A`S#X6hU|>A_*vPVcU;DkOCNN@yKPgipR3a#v5%miGhm` z{0aCGToJcik>JE103jjq1_>b!NKiG^-}Gyych~Wfap;je)m>fvn5z1^Yr4m~tJP{9 zq`#kUrW;*%2x%>x%LivyvYF1se1~R@L4IqCFlb#>zx)LylFw**L{s5z;*Ji8{-ZBx z>L;s10&VJy}zPf?vp6*hqVH(aH5e|I$+aUn3rd z0EMB>A|=M&Fj#Q#k2)n$EwOr67e@t zhd}&`tKDus$WC_#$DiHAbZr5x>M2Sdpr>2!Xij`*aQ%Yph z8ovprF^d%pU=-_{K=PrS?@*`*bHhMzFqDr>>pHL+utwc*#B)#x#LupD&M)WvY_WIY z*d~kzy?|Q!aFTvG3;_y5IhDrApaY_C52+q&*_8~tb4*IbCJ%>7h~I?MB*cfVMsH;k zit2wNxbLJ6Z`Ylk>UHz%wdLg#X@&RAVDRn7^baYKeN@xdJvy;lQ%nfyEnXC5-xJ<# zZ6G}>zBN9iJvITJt&#vwN-$9os{|KRgM3L*z~THZ;v$@1THQNUVw?p?t^!cKU%)C1Mn2C<5*WLn^%4Csu* z`MZgUaDMg@1@qn23kmJeR6xf@JHpAPzr54?h--E@NgWl{ysALm+JWM#7-pHWJE@Wc zFO;i`wiDWYo$(R;ZN$X({CsCH$op&A>D9HgT?b`_v{{Jf@Jy~J6;kP=n?e%_@o)&t zlD`t-IhMau7tD7%Yk4-;8C*JlEI}UH0%fg9x>=AXBkIfAl5)PYdtO0;sd)~aaTxA0 z%|)?G+D(hRt5#L9%i!r<6jy(r)DHdYd>ls?Ug7nJwQuB?UBipikUL^4&^mFy9DWLl znVITi6#S8kT!$4>qvi4X?AF?X9G-$GAR(<;t!lFK8QCX;js=6BcNA#GN*R%(5~50Tt}tF zgm}_}rPc-`eo==JcyrKU#KM*Nes9^ez1Vih-9lPa5^@lNOM+U!#3)Q8fl8y&-D*_H zG3D?SdR{Rmw}u}y3G4&~nqgYGZu3eyVPsptP_{v;jRtd1DJh2+S4&5@aHxog8BLQv zDX5=UI)fD;I`Wh#@)hFeA?H)b!4jab6v>Y~ij8UIH$UM#Mo$z@C_P%k4~F1M2F2Eb zl;#yzS1Dd0Z`mI6%8wM?@+l^tZ}7EnCC@KpXKtwfG(s%4?O#uH7;%@FO5fA#Nek1v zqZi6xc28jh=I5vN>j_*)zn+}#Ee~dT*Y{&3Ju+>>2d_$^q^c$#@J9|qq(R)_Ln%Zk zG=lg})MKq>CX9#Xyuc5xR!5~;3K2*TmReOAm|9F*pwxD4Vp1m~kvW6@K`{b1_jJvS z+k%>5R-_<@BKRceA?3~)2d|1`NmWfg;71-pny9{GfN9Ps^aHzyvTQ#tox9YdtMR(I z0(_~K<_yw*X!7L^ggn{r!P=tfgMq-85 zKwQs^wd_iU%^QO@VH*LP!uQZX_tM{vbo|i@91BK-*{76n;AkixnZmIm6^xrSd7F~| z^NHUra3VO=G;G!5JfJ+r zR%J1IqJV=mZ4E!@%asg@Ew9<$lZxg20tJTwtIG33Cv{Y$(xgIqu(a6Vw+D_Q*u;n; z@SBGV@11uoFP0s0w{RAfgdBu0n}Eh(i8@h=4m9AWO5x{K@mS{B9#>@7SJ(~R5z^9P>#)(tNS62J|UO#*LjDdvb_F)$2LW!RT-=~lR%t5yr?#2?a zmR%S2$7<&j++AH6xZ7G9F`fRQvPw~tI#KwZ;Le*%{An~_)9b4jv&H!_z%KR7&I~{d zzEAyffE+XiNx!YiJUSrix@D|oS2Apu%Y~c3?&`{b-EtbJ8b-yh)IXs_iO*>|U4&b_ zOTll2e1>HHIg~OL7dp2MQXxhxjA2SiImS{g9pS>EA|hrqO_Rp_ zc&pd%rlSDgw#J|d?|_`>s1onK`s(t-7WW60)I6Z+!Qz*st%-$-zixm05ntc8j~CZh zZK;#+E_yJVG#ty{M@+=>r}FOVdNzCdl&erwtEB$q=jSUc5fgHlbwOdJx_U$TXqc$$ zn6Z{!$*_+EcWB5^(b7cWUTp{rK}un7U-mI2oV}VJlbsv-hE_AAc=)}p1FNQSk0vkg zzZ2YfCyRd$iLaM>-KBhOvENy{xZLaCNc(lh9@heT(GuU&8t-DRv7m>!Yh(I7O00UX zru#G%u346l|4RoCJGGpyzcHhB7$<8o==t|i?x&~0m-QO-E30SEX&1E2Zg2ldL!r20a4dEV=cRqvAz=IU0rQo$j7W~m%*WIUTymCc7cCaN6sF8ZPl`1 zQhRoMCdX?$&g2C03m3YpH|&=^rB3~XZy>+iQL$0=cji;Y6csDtLEiTc8_~*(ECbo$j7dTMQ~m!mSmX772hI+9 z{bmI9JW;yW;j5A_sdDp@yT<$6RcYX$5;~_T^1_YXH=(|&<`7M8>FY_Fx-elGD#}mM z3hBYPN0WCX_!rsvl(GCp_J!<~&LE%O1n?}7RbjB?K@JRTLV(7$3{WRZ(Lq9cp(f~6 zbY{7w_^88h&2DgAost$OiuOm9baGfz`6dCwk+a3m^cmyl?JvLJ>!)pHM18uz$Pu{;Bop?shm)P7YYH +#include #include #include #include @@ -2118,6 +2119,173 @@ TEST_CASE("deserialize_recipe error is non-empty on every failure path", "[CadDo } } +TEST_CASE("a v4 project still opens", "[CadDocument][recipe]") +{ + // The regression guard for everyone who already has projects: a pre-framing v4 + // recipe must keep opening on this build, through the unchanged flat path — not be + // refused at the version gate after the bump to v5. + std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v4.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_FALSE(blob.empty()); + + // Trusted reference read of the flat v4 layout (what the golden test's Layer 1 does). + std::vector flat; + { + std::istringstream iss(blob); + cereal::BinaryInputArchive ar(iss); + uint32_t v; + ar(v); + REQUIRE(v == 4); + ar(flat); + } + REQUIRE_FALSE(flat.empty()); + + CadDocument doc; + doc.deserialize_recipe(blob); + // Not refused at the gate: the only failure allowed is the fixture's own geometry + // (the golden doc is a serialization-coverage tree whose fillet radius is too large), + // which is orthogonal to the v5 framing change and unchanged by it. + REQUIRE(doc.error.find("older version") == std::string::npos); + REQUIRE(doc.error.find("newer version") == std::string::npos); + // The v4 flat path read the same feature tree as the trusted reference. + REQUIRE(doc.features.size() == flat.size()); +} + +TEST_CASE("a v5 round trip is exact", "[CadDocument][recipe]") +{ + using Catch::Matchers::WithinRel; + CadDocument doc; + + // Body 1: rectangle sketch + extrude + fillet + int sk1 = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), + 20, 20, 10, "Rect1"); + REQUIRE(sk1 >= 0); + doc.add_extrude(sk1, 10.0, false, BooleanMode::New, "Extrude1"); + doc.add_fillet(2.0, FaceGroup::All, "Fillet1"); + REQUIRE(doc.recompute()); + REQUIRE(doc.error.empty()); + + // Body 2: circle sketch + extrude (separate New body) + SketchEntity circ; + circ.type = SketchEntity::Type::Circle; + circ.center = Vec2d(0, 0); + circ.radius = 8.0; + int sk2 = doc.add_sketch_entities({circ}, SketchPlane::XZ(), "Circle2"); + doc.add_extrude(sk2, 6.0, false, BooleanMode::New, "Extrude2"); + REQUIRE(doc.recompute()); + REQUIRE(doc.error.empty()); + + REQUIRE(doc.bodies.size() >= 2); + std::vector orig_vols; + for (const auto& b : doc.bodies) + orig_vols.push_back(double(SketchEngine::tessellate(b.shape).volume())); + + auto blob = doc.serialize_recipe(); + REQUIRE_FALSE(blob.empty()); + + CadDocument doc2; + REQUIRE(doc2.deserialize_recipe(blob)); + REQUIRE(doc2.error.empty()); + REQUIRE(doc2.features.size() == doc.features.size()); + for (size_t i = 0; i < doc.features.size(); ++i) + REQUIRE(doc2.features[i].type == doc.features[i].type); + REQUIRE(doc2.bodies.size() == doc.bodies.size()); + for (size_t i = 0; i < doc.bodies.size(); ++i) { + double v2 = double(SketchEngine::tessellate(doc2.bodies[i].shape).volume()); + REQUIRE_THAT(v2, WithinRel(orig_vols[i], 1e-6)); + } +} + +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 + // 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. + CadDocument doc; + int sk = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 20, 10, 0, "Sk1"); + REQUIRE(sk >= 0); + doc.add_extrude(sk, 5.0, false, BooleanMode::New, "Ex1"); + int sk2 = doc.add_sketch(SketchShape::Rectangle, SketchPlane::XY(), 12, 12, 0, "Sk2"); + doc.add_extrude(sk2, 3.0, false, BooleanMode::New, "Ex2"); + REQUIRE(doc.recompute()); + REQUIRE(doc.error.empty()); + + // Two trailing connector-fingerprint fields, made distinctive so the cut is observable: + // coordsys_face_kind sits right before the LAST serialized field (coordsys_face_edges), + // which is the one we drop. They are ignored by an Extrude's recompute, so the truncated + // document still replays cleanly. + doc.features[1].coordsys_face_kind = 777; + doc.features[1].coordsys_face_edges = 888; + + auto blob = doc.serialize_recipe(); + REQUIRE(blob.size() > 8); + + auto rd32 = [](const std::string& s, size_t off) -> uint32_t { + uint32_t u; + std::memcpy(&u, s.data() + off, sizeof(u)); + return u; + }; + auto wr32 = [](std::string& s, size_t off, uint32_t u) { + std::memcpy(&s[off], &u, sizeof(u)); + }; + + REQUIRE(rd32(blob, 0) == 5); + uint32_t count = rd32(blob, 4); + REQUIRE(count == doc.features.size()); + + // Walk the outer framing: version + count, then [len][bytes] per feature. + std::vector f_off, f_len; + size_t off = 8; + for (uint32_t i = 0; i < count; ++i) { + f_off.push_back(off); + f_len.push_back(rd32(blob, off)); + off += 4 + f_len.back(); + } + + // Shorten feature 1 by dropping its final field (4 bytes): rewrite its length prefix + // and erase the tail bytes. The reader then runs out inside fa(f), throws, and keeps + // everything it had already assigned — that is the whole point of the try/catch. + const size_t drop = sizeof(uint32_t); + REQUIRE(f_len[1] > drop); + std::string shortened = blob; + shortened.erase(f_off[1] + 4 + f_len[1] - drop, drop); + wr32(shortened, f_off[1], static_cast(f_len[1] - drop)); + + CadDocument loaded; + REQUIRE(loaded.deserialize_recipe(shortened)); + REQUIRE(loaded.error.empty()); + REQUIRE(loaded.features.size() == count); + + // Fields before the cut survived; the cut field defaulted; the neighbours on both sides + // are intact, proving the outer framing held. + REQUIRE(loaded.features[1].name == doc.features[1].name); + REQUIRE(loaded.features[1].coordsys_face_kind == 777); // right before the cut + REQUIRE(loaded.features[1].coordsys_face_edges == -1); // defaulted by the cut + REQUIRE(loaded.features[0].name == doc.features[0].name); + REQUIRE(loaded.features[2].name == doc.features[2].name); +} + +TEST_CASE("a v3 recipe is refused with a message naming the version", "[CadDocument][recipe]") +{ + // Honesty guard: the framing fixes the future, not the past. A v3 field list no longer + // exists in this code, so a v3 blob must be refused cleanly and must say which version. + std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v3.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_FALSE(blob.empty()); + + CadDocument doc; + REQUIRE_FALSE(doc.deserialize_recipe(blob)); + REQUIRE_FALSE(doc.error.empty()); + REQUIRE(doc.error.find("v3") != std::string::npos); +} + TEST_CASE("re-edit: editing a mid-timeline feature rebuilds downstream", "[CadDocument]") { using Catch::Matchers::WithinRel; @@ -2895,19 +3063,13 @@ TEST_CASE("helix serialization round-trip with distinctive values", "[CadDocumen auto blob = doc.serialize_recipe(); REQUIRE_FALSE(blob.empty()); - // Deserialize into a feature list directly — recompute fails because a lone + // Deserialize through the real entry point — recompute fails because a lone // helix doesn't produce a solid, but the serialized field values must survive. - std::vector features2; - { - std::istringstream iss(blob); - cereal::BinaryInputArchive ar(iss); - uint32_t v; - ar(v); - ar(features2); - } - REQUIRE(features2.size() == 1); + CadDocument doc2; + doc2.deserialize_recipe(blob); + REQUIRE(doc2.features.size() == 1); - const auto& f = features2[0]; + const auto& f = doc2.features[0]; REQUIRE(f.type == CadFeatureType::Helix); REQUIRE(f.name == "Helix_RT"); REQUIRE_THAT(f.helix_radius, WithinAbs(7.5, 1e-9)); @@ -3767,7 +3929,7 @@ 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_v4.bin"; + std::string path = std::string(TEST_DATA_DIR) + "/cad_recipe_v5.bin"; std::ofstream ofs(path, std::ios::binary); REQUIRE(ofs.is_open()); ofs.write(blob.data(), static_cast(blob.size()));