Follow-up a6dea252432eeb34153d5ab767b1a9d315926ee3 and ae8684a4a46bdaa8fb7dda03b38561ffe170cb47 - Code refactoring

src/libslic3r/GCode/Thumbnails.cpp

@@ -9,6 +9,9 @@
 #include <jpeglib.h>
 #include <jerror.h>
 
+#include <boost/algorithm/string.hpp>
+#include <string>
+
 namespace Slic3r::GCodeThumbnails {
 
 using namespace std::literals;
@@ -120,18 +123,19 @@ std::unique_ptr<CompressedImageBuffer> compress_thumbnail(const ThumbnailData &d
     }
 }
 
-std::vector<std::pair<GCodeThumbnailsFormat, Vec2d>> make_thumbnail_list(const DynamicPrintConfig &config)
+std::vector<std::pair<GCodeThumbnailsFormat, Vec2d>> make_and_check_thumbnail_list(const std::string& thumbnails_string, ThumbnailErrors& errors, std::string def_ext /*= "PNG"*/)
 {
-    // ??? Unit tests or command line slicing may not define "thumbnails" or "thumbnails_format".
+    if (thumbnails_string.empty())
-    // ??? If "thumbnails_format" is not defined, export to PNG.
+        return {};
+
+    const auto& extentions = ConfigOptionEnum<GCodeThumbnailsFormat>::get_enum_names();
+
+    std::istringstream is(thumbnails_string);
+    std::string point_str;
 
     // generate thumbnails data to process it
 
     std::vector<std::pair<GCodeThumbnailsFormat, Vec2d>> thumbnails_list;
-    if (const auto thumbnails_value = config.option<ConfigOptionString>("thumbnails")) {
-        std::string str = thumbnails_value->value;
-        std::istringstream is(str);
-        std::string point_str;
     while (std::getline(is, point_str, ',')) {
         Vec2d point(Vec2d::Zero());
         GCodeThumbnailsFormat format;
@@ -141,14 +145,46 @@ std::vector<std::pair<GCodeThumbnailsFormat, Vec2d>> make_thumbnail_list(const D
             std::istringstream(coord_str) >> point(0);
             if (std::getline(iss, coord_str, '/')) {
                 std::istringstream(coord_str) >> point(1);
+
+                if (0 < point(0) && point(0) < 1000 && 0 < point(1) && point(1) < 1000) {
                     std::string ext_str;
-                    if (std::getline(iss, ext_str, '/'))
+                    std::getline(iss, ext_str, '/');
+
+                    if (ext_str.empty())
+                        ext_str = def_ext;
+                    else {
+                        // check validity of extention
+                        boost::to_upper(ext_str);
+                        if (std::find(extentions.begin(), extentions.end(), ext_str) == extentions.end())
+                            errors = enum_bitmask(errors | ThumbnailError::InvalidExt);
+                    }
                     format = ext_str == "JPG" ? GCodeThumbnailsFormat::JPG :
                              ext_str == "QOI" ? GCodeThumbnailsFormat::QOI : GCodeThumbnailsFormat::PNG;
-                }
-            }
                     thumbnails_list.emplace_back(std::make_pair(format, point));
                 }
+                else
+                    errors = enum_bitmask(errors | ThumbnailError::OutOfRange);
+                continue;
+            }
+        }
+        errors = enum_bitmask(errors | ThumbnailError::InvalidVal);
+    }
+
+    return thumbnails_list;
+}
+
+std::vector<std::pair<GCodeThumbnailsFormat, Vec2d>> make_thumbnail_list(const DynamicPrintConfig& config)
+{
+    // ??? Unit tests or command line slicing may not define "thumbnails" or "thumbnails_format".
+    // ??? If "thumbnails_format" is not defined, export to PNG.
+
+    // generate thumbnails data to process it
+
+    std::vector<std::pair<GCodeThumbnailsFormat, Vec2d>> thumbnails_list;
+    if (const auto thumbnails_value = config.option<ConfigOptionString>("thumbnails")) {
+        ThumbnailErrors errors;
+        thumbnails_list = make_and_check_thumbnail_list(thumbnails_value->value, errors);
+        assert(errors == enum_bitmask<ThumbnailError>());
     }
 
     return thumbnails_list;

src/libslic3r/GCode/Thumbnails.hpp

@@ -17,6 +17,14 @@
 
 #include <boost/beast/core/detail/base64.hpp>
 
+#include "../libslic3r/enum_bitmask.hpp"
+
+namespace Slic3r {
+    enum class ThumbnailError : int { InvalidVal, OutOfRange, InvalidExt };
+    using ThumbnailErrors = enum_bitmask<ThumbnailError>;
+    ENABLE_ENUM_BITMASK_OPERATORS(ThumbnailError);
+}
+
 namespace Slic3r::GCodeThumbnails {
 
 struct CompressedImageBuffer
@@ -29,6 +37,7 @@ struct CompressedImageBuffer
 
 std::unique_ptr<CompressedImageBuffer> compress_thumbnail(const ThumbnailData &data, GCodeThumbnailsFormat format);
 
+std::vector<std::pair<GCodeThumbnailsFormat, Vec2d>> make_and_check_thumbnail_list(const std::string& thumbnails_string, ThumbnailErrors& errors, std::string def_ext = "PNG");
 std::vector<std::pair<GCodeThumbnailsFormat, Vec2d>> make_thumbnail_list(const DynamicPrintConfig &config);
 
 template<typename WriteToOutput, typename ThrowIfCanceledCallback>

src/libslic3r/PrintConfig.cpp

@@ -24,6 +24,7 @@
 #include "I18N.hpp"
 
 #include "SLA/SupportTree.hpp"
+#include "GCode/Thumbnails.hpp"
 
 #include <set>
 #include <boost/algorithm/string/replace.hpp>
@@ -4371,6 +4372,31 @@ void PrintConfigDef::handle_legacy(t_config_option_key &opt_key, std::string &va
 // Don't convert single options here, implement such conversion in PrintConfigDef::handle_legacy() instead.
 void PrintConfigDef::handle_legacy_composite(DynamicPrintConfig &config)
 {
+    if (config.has("thumbnails")) {
+        std::string extention = "PNG";
+        if (config.has("thumbnails_format")) {
+            if (const ConfigOptionDef* opt = config.def()->get("thumbnails_format")) {
+                auto label = opt->enum_def->enum_to_label(config.option("thumbnails_format")->getInt());
+                if (label.has_value())
+                    extention = *label;
+            }
+        }
+        std::string thumbnails_str = config.opt_string("thumbnails");
+
+        ThumbnailErrors errors;
+        auto thumbnails_list = GCodeThumbnails::make_and_check_thumbnail_list(thumbnails_str, errors, extention);
+        assert(errors == enum_bitmask<ThumbnailError>());
+
+        if (!thumbnails_list.empty()) {
+            const auto& extentions = ConfigOptionEnum<GCodeThumbnailsFormat>::get_enum_names();
+            thumbnails_str.clear();
+            for (const auto& [ext, size] : thumbnails_list)
+                thumbnails_str += format("%1%x%2%/%3%, ", size.x(), size.y(), extentions[int(ext)]);
+            thumbnails_str.resize(thumbnails_str.length() - 2);
+
+            config.set_key_value("thumbnails", new ConfigOptionString(thumbnails_str));
+        }
+    }
 }
 
 const PrintConfigDef print_config_def;

src/slic3r/GUI/Field.cpp

@@ -15,6 +15,7 @@
 
 #include "libslic3r/PrintConfig.hpp"
 #include "libslic3r/enum_bitmask.hpp"
+#include "libslic3r/GCode/Thumbnails.hpp"
 
 #include <regex>
 #include <wx/numformatter.h>
@@ -33,13 +34,7 @@
 #define wxOSX false
 #endif
 
-namespace Slic3r {
+namespace Slic3r :: GUI {
-
-enum class ThumbnailError : int { InvalidVal, OutOfRange, InvalidExt };
-using ThumbnailErrors = enum_bitmask<ThumbnailError>;
-ENABLE_ENUM_BITMASK_OPERATORS(ThumbnailError);
-
-namespace GUI {
 
 wxString double_to_string(double const value, const int max_precision /*= 4*/)
 {
@@ -71,82 +66,24 @@ wxString double_to_string(double const value, const int max_precision /*= 4*/)
     return s;
 }
 
-bool is_valid_thumbnails_extention(wxString& ext)
-{
-    ext.UpperCase();
-    static const std::vector<wxString> extentions = { "PNG", "JPG", "QOI" };
-
-    return std::find(extentions.begin(), extentions.end(), ext) != extentions.end();
-}
-
 ThumbnailErrors validate_thumbnails_string(wxString& str, const wxString& def_ext = "PNG")
 {
-    bool invalid_val, out_of_range_val, invalid_ext;
+    std::string input_string = into_u8(str);
-    invalid_val = out_of_range_val = invalid_ext = false;
-    str.Replace(" ", wxEmptyString, true);
-
-    if (!str.IsEmpty()) {
-
-        std::vector<std::pair<Vec2d, std::string>> out_thumbnails;
-
-        wxStringTokenizer thumbnails(str, ",");
-        while (thumbnails.HasMoreTokens()) {
-            wxString token = thumbnails.GetNextToken();
-            double x, y;
-            wxStringTokenizer thumbnail(token, "x");
-            if (thumbnail.HasMoreTokens()) {
-                wxString x_str = thumbnail.GetNextToken();
-                if (x_str.ToDouble(&x) && thumbnail.HasMoreTokens()) {
-                    wxStringTokenizer y_and_ext(thumbnail.GetNextToken(), "/");
-
-                    wxString y_str = y_and_ext.GetNextToken();
-                    if (y_str.ToDouble(&y)) {
-                        // thumbnail has no extension 
-                        if (0 < x && x < 1000 && 0 < y && y < 1000) {
-                            wxString ext = y_and_ext.HasMoreTokens() ? y_and_ext.GetNextToken() : def_ext;
-                            bool is_valid_ext = is_valid_thumbnails_extention(ext);
-                            invalid_ext |= !is_valid_ext;
-                            out_thumbnails.push_back({ Vec2d(x, y), into_u8(is_valid_ext ? ext : def_ext) });
-                            continue;
-                        }
-                        out_of_range_val |= true;
-                        continue;
-                    }
-                }
-            }
-            invalid_val |= true;
-        }
 
     str.Clear();
-        for (const auto& [size, ext] : out_thumbnails)
+    ThumbnailErrors errors;
-            str += format_wxstr("%1%x%2%/%3%, ", size.x(), size.y(), ext);
+
+    auto thumbnails_list = Slic3r::GCodeThumbnails::make_and_check_thumbnail_list(input_string, errors);
+    if (!thumbnails_list.empty()) {
+        const auto& extentions = ConfigOptionEnum<GCodeThumbnailsFormat>::get_enum_names();
+        for (const auto& [format, size] : thumbnails_list)
+            str += format_wxstr("%1%x%2%/%3%, ", size.x(), size.y(), extentions[int(format)]);
         str.resize(str.Len() - 2);
     }
 
-    ThumbnailErrors errors = only_if(invalid_val,      ThumbnailError::InvalidVal) |
-                             only_if(invalid_ext,      ThumbnailError::InvalidExt) |
-                             only_if(out_of_range_val, ThumbnailError::OutOfRange);
-
     return errors;
 }
 
-wxString get_valid_thumbnails_string(const DynamicPrintConfig& config)
-{
-    // >>> ysFIXME - temporary code, till "thumbnails_format" options exists in config
-    wxString format = "PNG";
-    if (const ConfigOptionDef* opt = config.def()->get("thumbnails_format"))
-        if (auto label = opt->enum_def->enum_to_label(config.option("thumbnails_format")->getInt());
-            label.has_value())
-            format = from_u8(*label);
-    // <<<
-
-    wxString str = from_u8(config.opt_string("thumbnails"));
-    validate_thumbnails_string(str, format);
-
-    return str;
-}
-
-
 Field::~Field()
 {
 	if (m_on_kill_focus)
@@ -1765,5 +1702,5 @@ boost::any& SliderCtrl::get_value()
 }
 
 
-} // GUI
+} // Slic3r :: GUI
-} // Slic3r
+

src/slic3r/GUI/Field.hpp

@@ -41,7 +41,6 @@ using t_change = std::function<void(const t_config_option_key&, const boost::any
 using t_back_to_init = std::function<void(const std::string&)>;
 
 wxString double_to_string(double const value, const int max_precision = 4);
-wxString get_valid_thumbnails_string(const DynamicPrintConfig& config);
 
 class UndoValueUIManager
 {

src/slic3r/GUI/OptionsGroup.cpp

@@ -925,9 +925,6 @@ boost::any ConfigOptionsGroup::get_config_value(const DynamicPrintConfig& config
 		}
 		break;
 	case coString: {
-        if (opt_key == "thumbnails")
-            ret = get_valid_thumbnails_string(config);
-        else
         ret = from_u8(config.opt_string(opt_key));
         break;
     }

src/slic3r/GUI/Plater.hpp

@@ -43,8 +43,6 @@ namespace Slic3r {
 class BuildVolume;
 class Model;
 class ModelObject;
-enum class ModelObjectCutAttribute : int;
-using ModelObjectCutAttributes = enum_bitmask<ModelObjectCutAttribute>;
 class ModelInstance;
 class Print;
 class SLAPrint;

src/slic3r/GUI/Tab.cpp

@@ -662,21 +662,6 @@ void Tab::update_changed_ui()
             if (tab->m_sys_extruders_count != tab->m_extruders_count)
                 nonsys_options.emplace_back("extruders_count");
         }
-
-        // "thumbnails" can not containe a extentions in old config but are valid and use PNG extention by default
-        // So, check if "thumbnails" is really changed
-        // We will compare full strings for thumnails instead of exactly config values
-        {
-            auto check_thumbnails_option = [](std::vector<std::string>& keys, const DynamicPrintConfig& config, const DynamicPrintConfig& config_new) {
-                if (auto it = std::find(keys.begin(), keys.end(), "thumbnails"); it != keys.end())
-                    if (get_valid_thumbnails_string(config) == get_valid_thumbnails_string(config_new))
-                        // if those strings are actually the same, erase them from the list of dirty oprions
-                        keys.erase(it);
-            };
-            check_thumbnails_option(dirty_options, m_presets->get_edited_preset().config, m_presets->get_selected_preset().config);
-            if (const Preset* parent_preset = m_presets->get_selected_preset_parent())
-                check_thumbnails_option(nonsys_options, m_presets->get_edited_preset().config, parent_preset->config);
-        }
     }
 
     for (auto& it : m_options_list)

src/slic3r/GUI/UnsavedChangesDialog.cpp

@@ -1160,11 +1160,8 @@ static wxString get_string_value(std::string opt_key, const DynamicPrintConfig&
         }
         return _L("Undef");
     }
-    case coString: {
+    case coString:
-        if (opt_key == "thumbnails")
-            return get_valid_thumbnails_string(config);
         return from_u8(config.opt_string(opt_key));
-    }
     case coStrings: {
         const ConfigOptionStrings* strings = config.opt<ConfigOptionStrings>(opt_key);
         if (strings) {