From 4148d7332e66f3b66c330505ec1625c6a1072a9b Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Tue, 26 Apr 2022 14:43:07 +0200 Subject: [PATCH] Prohibit multiple formats with the same extension (zip) Archive format can be specified as a hint when a reader is created. --- src/libslic3r/Format/SLAArchiveReader.cpp | 29 ++++++++++++++----- src/libslic3r/Format/SLAArchiveReader.hpp | 9 ++++-- src/slic3r/GUI/Jobs/SLAImportDialog.hpp | 10 +++++++ src/slic3r/GUI/Jobs/SLAImportJob.cpp | 13 +++++++-- src/slic3r/GUI/Jobs/SLAImportJob.hpp | 1 + .../sla_print/sla_archive_readwrite_tests.cpp | 15 ++++++---- 6 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/libslic3r/Format/SLAArchiveReader.cpp b/src/libslic3r/Format/SLAArchiveReader.cpp index e62b7e061..09c157059 100644 --- a/src/libslic3r/Format/SLAArchiveReader.cpp +++ b/src/libslic3r/Format/SLAArchiveReader.cpp @@ -38,7 +38,7 @@ static const std::map REGISTERED_ARCHIVES { }, { "SL2", - { L("SL2 archive files"), {"sl2", "sl1_svg", "zip"}, + { L("SL2 archive files"), {"sl2", "sl1_svg"/*, "zip"*/}, // also a zip but unnecessary hassle to implement single extension for multiple archives [] (const std::string &fname, SLAImportQuality quality, const ProgrFn &progr) { return std::make_unique(fname, quality, progr); }} }, // TODO: pwmx and future others. @@ -48,26 +48,39 @@ static const std::map REGISTERED_ARCHIVES { std::unique_ptr SLAArchiveReader::create( const std::string &fname, + const std::string &format_id, SLAImportQuality quality, const ProgrFn & progr) { // Create an instance of SLAArchiveReader using the registered archive - // reader implementations. Only checking the file extension and comparing - // with the registered readers advertised extensions. - // The first match will be used. + // reader implementations. + // If format_id is specified and valid, that archive format will be + // preferred. When format_id is emtpy, the file extension is compared + // with the advertised extensions of registered readers and the first + // match will be used. std::string ext = boost::filesystem::path(fname).extension().string(); boost::algorithm::to_lower(ext); std::unique_ptr ret; + auto arch_from = REGISTERED_ARCHIVES.begin(); + auto arch_to = REGISTERED_ARCHIVES.end(); + + auto arch_it = REGISTERED_ARCHIVES.find(format_id); + if (arch_it != REGISTERED_ARCHIVES.end()) { + arch_from = arch_it; + arch_to = arch_it; + } + if (!ext.empty()) { if (ext.front() == '.') ext.erase(ext.begin()); auto extcmp = [&ext](const auto &e) { return e == ext; }; - for (const auto &[format_id, entry] : REGISTERED_ARCHIVES) { + for (auto it = arch_from; it != arch_to; ++it) { + const auto &[format_id, entry] = *it; if (std::any_of(entry.extensions.begin(), entry.extensions.end(), extcmp)) ret = entry.factoryfn(fname, quality, progr); } @@ -124,6 +137,7 @@ static SliceParams get_slice_params(const DynamicPrintConfig &cfg) } ConfigSubstitutions import_sla_archive(const std::string &zipfname, + const std::string &format_id, indexed_triangle_set &out, DynamicPrintConfig &profile, SLAImportQuality quality, @@ -131,7 +145,7 @@ ConfigSubstitutions import_sla_archive(const std::string &zipfname, { ConfigSubstitutions ret; - if (auto reader = SLAArchiveReader::create(zipfname, quality, progr)) { + if (auto reader = SLAArchiveReader::create(zipfname, format_id, quality, progr)) { std::vector slices; ret = reader->read(slices, profile); @@ -148,11 +162,12 @@ ConfigSubstitutions import_sla_archive(const std::string &zipfname, } ConfigSubstitutions import_sla_archive(const std::string &zipfname, + const std::string &format_id, DynamicPrintConfig &out) { ConfigSubstitutions ret; - if (auto reader = SLAArchiveReader::create(zipfname)) { + if (auto reader = SLAArchiveReader::create(zipfname, format_id)) { ret = reader->read(out); } else { throw ReaderUnimplementedError("Reader unimplemented"); diff --git a/src/libslic3r/Format/SLAArchiveReader.hpp b/src/libslic3r/Format/SLAArchiveReader.hpp index d3b718513..e7a99b043 100644 --- a/src/libslic3r/Format/SLAArchiveReader.hpp +++ b/src/libslic3r/Format/SLAArchiveReader.hpp @@ -38,9 +38,13 @@ public: virtual ConfigSubstitutions read(DynamicPrintConfig &profile) = 0; // Creates a reader instance based on the provided file path. - // Currently only considers the file extension. + // format_id can be one of the archive type identifiers returned by + // registered_archives(). If left empty, only the file extension will + // be considered. If more archive types have the same extension (like *.zip) + // The first match is used. static std::unique_ptr create( const std::string &fname, + const std::string &format_id, SLAImportQuality quality = SLAImportQuality::Balanced, const ProgrFn &progr = [](int) { return false; }); @@ -65,6 +69,7 @@ class ReaderUnimplementedError : public RuntimeError // Can throw ReaderUnimplementedError or MissingProfileError ConfigSubstitutions import_sla_archive( const std::string &zipfname, + const std::string &format_id, indexed_triangle_set &out, DynamicPrintConfig &profile, SLAImportQuality quality = SLAImportQuality::Balanced, @@ -72,9 +77,9 @@ ConfigSubstitutions import_sla_archive( // Only reads the profile, doesn't reconstruct the model. ConfigSubstitutions import_sla_archive(const std::string &zipfname, + const std::string &format_id, DynamicPrintConfig &out); - } // namespace Slic3r #endif // SLAARCHIVEREADER_HPP diff --git a/src/slic3r/GUI/Jobs/SLAImportDialog.hpp b/src/slic3r/GUI/Jobs/SLAImportDialog.hpp index e5a807011..5477e51e7 100644 --- a/src/slic3r/GUI/Jobs/SLAImportDialog.hpp +++ b/src/slic3r/GUI/Jobs/SLAImportDialog.hpp @@ -18,6 +18,8 @@ #include "slic3r/GUI/GUI_App.hpp" #include "slic3r/GUI/Plater.hpp" +#include + //#include "libslic3r/Model.hpp" //#include "libslic3r/PresetBundle.hpp" @@ -160,6 +162,14 @@ public: { return m_filepicker->GetPath().ToUTF8().data(); } + + std::string get_archive_format() const override + { + // TODO: the choosen format is inside the file dialog which is not + // accessible from the file picker object. The file picker could be + // changed to a custom file dialog. + return {}; + } }; }} // namespace Slic3r::GUI diff --git a/src/slic3r/GUI/Jobs/SLAImportJob.cpp b/src/slic3r/GUI/Jobs/SLAImportJob.cpp index fac4983c2..c258c9c95 100644 --- a/src/slic3r/GUI/Jobs/SLAImportJob.cpp +++ b/src/slic3r/GUI/Jobs/SLAImportJob.cpp @@ -56,17 +56,21 @@ void SLAImportJob::process(Ctl &ctl) if (p->path.empty()) return; std::string path = p->path.ToUTF8().data(); + std::string format_id = p->import_dlg->get_archive_format(); try { switch (p->sel) { case Sel::modelAndProfile: case Sel::modelOnly: - p->config_substitutions = import_sla_archive(path, p->mesh, + p->config_substitutions = import_sla_archive(path, + format_id, + p->mesh, p->profile, p->quality, progr); break; case Sel::profileOnly: - p->config_substitutions = import_sla_archive(path, p->profile); + p->config_substitutions = import_sla_archive(path, format_id, + p->profile); break; } } catch (MissingProfileError &) { @@ -162,6 +166,11 @@ void SLAImportJob::finalize(bool canceled, std::exception_ptr &eptr) bool is_centered = false; p->plater->sidebar().obj_list()->load_mesh_object(TriangleMesh{std::move(p->mesh)}, name, is_centered); + } else if (p->sel == Sel::modelOnly || p->sel == Sel::modelAndProfile) { + p->plater->get_notification_manager()->push_notification( + NotificationType::CustomNotification, + NotificationManager::NotificationLevel::WarningNotificationLevel, + _u8L("No object could be retrieved from the archive. The slices might be corrupted or missing.")); } if (! p->config_substitutions.empty()) diff --git a/src/slic3r/GUI/Jobs/SLAImportJob.hpp b/src/slic3r/GUI/Jobs/SLAImportJob.hpp index 0a691b6e8..3a578855c 100644 --- a/src/slic3r/GUI/Jobs/SLAImportJob.hpp +++ b/src/slic3r/GUI/Jobs/SLAImportJob.hpp @@ -16,6 +16,7 @@ public: virtual Sel get_selection() const = 0; virtual SLAImportQuality get_quality() const = 0; virtual std::string get_path() const = 0; + virtual std::string get_archive_format() const { return ""; } }; class Plater; diff --git a/tests/sla_print/sla_archive_readwrite_tests.cpp b/tests/sla_print/sla_archive_readwrite_tests.cpp index 251d7e8f3..a7ed7f0a4 100644 --- a/tests/sla_print/sla_archive_readwrite_tests.cpp +++ b/tests/sla_print/sla_archive_readwrite_tests.cpp @@ -11,14 +11,13 @@ using namespace Slic3r; TEST_CASE("Archive export test", "[sla_archives]") { - constexpr const char *PNAME = "extruder_idler"; - + for (const char * pname : {"20mm_cube", "extruder_idler"}) for (auto &archname : SLAArchiveWriter::registered_archives()) { INFO(std::string("Testing archive type: ") + archname + " -- writing..."); SLAPrint print; SLAFullPrintConfig fullcfg; - auto m = Model::read_from_file(TEST_DATA_DIR PATH_SEPARATOR + std::string(PNAME) + ".obj", nullptr); + auto m = Model::read_from_file(TEST_DATA_DIR PATH_SEPARATOR + std::string(pname) + ".obj", nullptr); fullcfg.printer_technology.setInt(ptSLA); // FIXME this should be ensured fullcfg.set("sla_archive_format", archname); @@ -33,9 +32,9 @@ TEST_CASE("Archive export test", "[sla_archives]") { print.process(); ThumbnailsList thumbnails; - auto outputfname = std::string("output.") + SLAArchiveWriter::get_extension(archname); + auto outputfname = std::string("output_") + pname + "." + SLAArchiveWriter::get_extension(archname); - print.export_print(outputfname, thumbnails, PNAME); + print.export_print(outputfname, thumbnails, pname); // Not much can be checked about the archives... REQUIRE(boost::filesystem::exists(outputfname)); @@ -52,11 +51,15 @@ TEST_CASE("Archive export test", "[sla_archives]") { DynamicPrintConfig cfg; try { - import_sla_archive(outputfname, its, cfg); + // Leave format_id deliberetaly empty, guessing should always + // work here. + import_sla_archive(outputfname, "", its, cfg); } catch (...) { REQUIRE(false); } + // its_write_obj(its, (outputfname + ".obj").c_str()); + REQUIRE(!cfg.empty()); REQUIRE(!its.empty());