From c74fb602116b5276b212d693231a0e108a55c2d6 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Tue, 11 Aug 2026 19:35:22 -0700 Subject: [PATCH] Test that the 3MF we write is actually compressed writeThreeMf() has called writeDeflateZip() all along, but nothing checked it. The round-trip tests cannot: readStoredZipEntry accepts both STORED and DEFLATE, so swapping the writer back to writeStoredZip leaves every existing test green and shows up only as files several times larger than they need to be. Confirmed by doing exactly that -- ThreeMfRoundTripPreservesVolume still passed. ThreeMfIsDeflateCompressed reads the compression method straight out of the central directory of a real exported .3mf and requires method 8, plus a compressed size below the raw one. It fails on that same swap. Four smaller tests do the same for the zip layer itself: writeDeflateZip emits method 8 for compressible input, actually shrinks it, falls back to STORED for input too small to benefit, and writeStoredZip still emits method 0 as the control. export.hpp said the 3MF was "written as a stored (uncompressed) ZIP", which has been wrong since the deflate path landed and is corrected here. That comment was read as fact and repeated as one, which is what prompted this. 830 tests pass. Co-Authored-By: Claude Opus 5 (1M context) --- include/openscad_cpp_evaluator/export.hpp | 5 +- pyproject.toml | 2 +- tests/test_import_export.cpp | 34 +++++++++ tests/test_zip_stored.cpp | 84 +++++++++++++++++++++++ 4 files changed, 123 insertions(+), 2 deletions(-) diff --git a/include/openscad_cpp_evaluator/export.hpp b/include/openscad_cpp_evaluator/export.hpp index dff88a6..d8c3b14 100644 --- a/include/openscad_cpp_evaluator/export.hpp +++ b/include/openscad_cpp_evaluator/export.hpp @@ -39,7 +39,10 @@ void writeObj(const std::string& path, const std::vector& bodies); void writeOff(const std::string& path, const std::vector& bodies); // 3MF: one mesh object + a base-color colorgroup per body (skipping empty -// bodies), written as a stored (uncompressed) ZIP -- see zip_stored.hpp. +// bodies), written as a DEFLATE-compressed ZIP (writeDeflateZip, see +// zip_stored.hpp) -- the XML is highly compressible, so storing it would +// make the file several times larger for nothing. An entry that does not +// actually shrink is stored raw, as any ZIP writer would. // Mirrors export.py's write_3mf's XML shape exactly (core + material // namespaces, %.6g vertex formatting); throws std::runtime_error if there's // no geometry to export. diff --git a/pyproject.toml b/pyproject.toml index 9ba7cd1..3e06118 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.29.1" +version = "0.29.2" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/tests/test_import_export.cpp b/tests/test_import_export.cpp index 25b06fa..ead7977 100644 --- a/tests/test_import_export.cpp +++ b/tests/test_import_export.cpp @@ -77,6 +77,40 @@ TEST(ImportModuleContext, ThreeMfRoundTripPreservesVolume) { std::filesystem::remove(path); } +// The .3mf writeThreeMf() produces is DEFLATE-compressed, not stored. The +// round-trip test above passes either way -- the reader accepts both +// methods -- so without this a regression to STORED would show up only as +// files several times bigger than they need to be. +TEST(ImportModuleContext, ThreeMfIsDeflateCompressed) { + const auto path = tempPath("cube_compressed.3mf"); + writeCubeAs(path, &writeThreeMf); + + std::ifstream in(path, std::ios::binary); + ASSERT_TRUE(in); + std::vector buf((std::istreambuf_iterator(in)), std::istreambuf_iterator()); + in.close(); + + bool sawModel = false; + for (size_t i = 0; i + 46 <= buf.size(); ++i) { + if (!(buf[i] == 0x50 && buf[i + 1] == 0x4B && buf[i + 2] == 0x01 && buf[i + 3] == 0x02)) continue; + const uint16_t method = static_cast(buf[i + 10] | (buf[i + 11] << 8)); + const uint32_t compressedSize = + static_cast(buf[i + 20] | (buf[i + 21] << 8) | (buf[i + 22] << 16) | (buf[i + 23] << 24)); + const uint32_t rawSize = + static_cast(buf[i + 24] | (buf[i + 25] << 8) | (buf[i + 26] << 16) | (buf[i + 27] << 24)); + const uint16_t nameLen = static_cast(buf[i + 28] | (buf[i + 29] << 8)); + if (i + 46 + nameLen > buf.size()) continue; + const std::string name(reinterpret_cast(&buf[i + 46]), nameLen); + if (name.find("3dmodel.model") == std::string::npos) continue; + sawModel = true; + EXPECT_EQ(method, 8) << "3dmodel.model stored uncompressed"; + EXPECT_LT(compressedSize, rawSize) << "compressed " << compressedSize << " vs raw " << rawSize; + } + EXPECT_TRUE(sawModel) << "no 3dmodel.model entry in the archive"; + + std::filesystem::remove(path); +} + TEST(ImportModuleContext, UnsupportedExtensionErrors) { Evaluator ev; auto ast = parseSrc("import(\"nope.xyz\");"); diff --git a/tests/test_zip_stored.cpp b/tests/test_zip_stored.cpp index 8fe699c..8194094 100644 --- a/tests/test_zip_stored.cpp +++ b/tests/test_zip_stored.cpp @@ -1,6 +1,12 @@ // Direct tests for zip_stored.hpp -- writeStoredZip()'s own STORED-entry // round trip, plus reading a DEFLATE-compressed (method 8) entry, which is // what most third-party ZIP/3MF writers actually emit. +// +// Also what writeDeflateZip() and writeThreeMf() put in the archive. That +// went untested: the round-trip tests pass either way, since the reader +// handles both methods, so a silent regression to STORED would only show +// up as files several times larger. The method byte is read out of the +// central directory here so it cannot regress unnoticed. #include "openscad_cpp_evaluator/zip_stored.hpp" @@ -169,3 +175,81 @@ TEST(ZipStored, UnsupportedCompressionMethodThrows) { EXPECT_THROW(readStoredZipEntry(path.string(), "hello.txt"), std::runtime_error); std::filesystem::remove(path); } + + +// --- what we WRITE, not just what we can read ------------------------- + +namespace { + +// The compression method of the first central-directory entry whose name +// ends with `suffix`. Read from the central directory rather than the local +// header, since that is the authoritative copy. +uint16_t centralDirMethodBySuffix(const std::string& path, const std::string& suffix) { + std::ifstream in(path, std::ios::binary); + EXPECT_TRUE(in) << "cannot open " << path; + std::vector buf((std::istreambuf_iterator(in)), std::istreambuf_iterator()); + // Walk central-directory headers by signature; these archives are small + // and have no ZIP comment, so a forward scan is enough. + for (size_t i = 0; i + 46 <= buf.size(); ++i) { + if (!(buf[i] == 0x50 && buf[i + 1] == 0x4B && buf[i + 2] == 0x01 && buf[i + 3] == 0x02)) continue; + const uint16_t method = static_cast(buf[i + 10] | (buf[i + 11] << 8)); + const uint16_t nameLen = static_cast(buf[i + 28] | (buf[i + 29] << 8)); + if (i + 46 + nameLen > buf.size()) continue; + const std::string name(reinterpret_cast(&buf[i + 46]), nameLen); + if (name.size() >= suffix.size() && name.compare(name.size() - suffix.size(), suffix.size(), suffix) == 0) + return method; + } + ADD_FAILURE() << "no central-directory entry ending in " << suffix; + return 0xFFFF; +} + +// Compressible: long runs, which is what 3MF's indented XML looks like. +std::vector compressibleBytes(size_t n) { + std::vector out; + out.reserve(n); + const std::string unit = "\t\t\t\t\t\n"; + while (out.size() < n) out.insert(out.end(), unit.begin(), unit.end()); + out.resize(n); + return out; +} + +} // namespace + +TEST(ZipStored, DeflateZipReallyWritesMethod8) { + const auto path = tempPath("deflate_method.zip"); + const auto data = compressibleBytes(64 * 1024); + writeDeflateZip(path.string(), {ZipEntry{"3D/3dmodel.model", data}}); + EXPECT_EQ(centralDirMethodBySuffix(path.string(), "3dmodel.model"), 8); + // And it still reads back byte for byte. + EXPECT_EQ(readStoredZipEntry(path.string(), "3D/3dmodel.model"), data); + std::filesystem::remove(path); +} + +TEST(ZipStored, DeflateZipActuallyShrinksCompressibleData) { + const auto path = tempPath("deflate_size.zip"); + const auto data = compressibleBytes(256 * 1024); + writeDeflateZip(path.string(), {ZipEntry{"3D/3dmodel.model", data}}); + const auto onDisk = std::filesystem::file_size(path); + // Highly repetitive input; anything near its original size means the + // compressor did not run. + EXPECT_LT(onDisk, data.size() / 4) << "archive " << onDisk << " for " << data.size() << " bytes of input"; + std::filesystem::remove(path); +} + +TEST(ZipStored, DeflateZipStoresDataThatWouldNotShrink) { + const auto path = tempPath("deflate_tiny.zip"); + const std::vector tiny{'x'}; + writeDeflateZip(path.string(), {ZipEntry{"a.txt", tiny}}); + EXPECT_EQ(centralDirMethodBySuffix(path.string(), "a.txt"), 0); + EXPECT_EQ(readStoredZipEntry(path.string(), "a.txt"), tiny); + std::filesystem::remove(path); +} + +TEST(ZipStored, StoredZipWritesMethod0) { + // The control for the tests above: same call shape, opposite method. + const auto path = tempPath("stored_method.zip"); + const auto data = compressibleBytes(64 * 1024); + writeStoredZip(path.string(), {ZipEntry{"3D/3dmodel.model", data}}); + EXPECT_EQ(centralDirMethodBySuffix(path.string(), "3dmodel.model"), 0); + std::filesystem::remove(path); +}