Skip to content

Commit

Permalink
Reworked copying of PresetBundles:
Browse files Browse the repository at this point in the history
1) Simplified by using the default copy constructors and copy operators.
2) Made safer by not allowing PresetCollection and PhysicalPrinterPresetCollection
   to be copied or instantiated outside of PresetBundle.
3) Fixed Preset::vendor pointers after copying PresetBundle.
  • Loading branch information
bubnikv committed Feb 3, 2021
1 parent a8f0b7a commit e781cea
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 82 deletions.
63 changes: 12 additions & 51 deletions src/libslic3r/Preset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -615,38 +615,6 @@ PresetCollection::PresetCollection(Preset::Type type, const std::vector<std::str
m_edited_preset.config.apply(m_presets.front().config);
}

PresetCollection::PresetCollection(const PresetCollection& other)
{
m_type = other.m_type ;
m_presets = other.m_presets ;
m_edited_preset = other.m_edited_preset ;
m_idx_selected = other.m_idx_selected ;
m_default_suppressed = other.m_default_suppressed ;
m_num_default_presets = other.m_num_default_presets ;
m_dir_path = other.m_dir_path ;
m_map_alias_to_profile_name = other.m_map_alias_to_profile_name ;
m_map_system_profile_renamed = other.m_map_system_profile_renamed;
}

PresetCollection& PresetCollection::operator=(const PresetCollection& other)
{
m_type = other.m_type ;
m_presets = other.m_presets ;
m_edited_preset = other.m_edited_preset ;
m_idx_selected = other.m_idx_selected ;
m_default_suppressed = other.m_default_suppressed ;
m_num_default_presets = other.m_num_default_presets ;
m_dir_path = other.m_dir_path ;
m_map_alias_to_profile_name = other.m_map_alias_to_profile_name ;
m_map_system_profile_renamed = other.m_map_system_profile_renamed;

return *this;
}

PresetCollection::~PresetCollection()
{
}

void PresetCollection::reset(bool delete_files)
{
if (m_presets.size() > m_num_default_presets) {
Expand Down Expand Up @@ -1305,6 +1273,18 @@ std::vector<std::string> PresetCollection::merge_presets(PresetCollection &&othe
return duplicates;
}

void PresetCollection::update_vendor_ptrs_after_copy(const VendorMap &new_vendors)
{
for (Preset &preset : m_presets)
if (preset.vendor != nullptr) {
assert(! preset.is_default && ! preset.is_external);
// Re-assign a pointer to the vendor structure in the new PresetBundle.
auto it = new_vendors.find(preset.vendor->id);
assert(it != new_vendors.end());
preset.vendor = &it->second;
}
}

void PresetCollection::update_map_alias_to_profile_name()
{
m_map_alias_to_profile_name.clear();
Expand Down Expand Up @@ -1742,25 +1722,6 @@ std::string PhysicalPrinterCollection::path_from_name(const std::string& new_nam
return (boost::filesystem::path(m_dir_path) / file_name).make_preferred().string();
}

PhysicalPrinterCollection& PhysicalPrinterCollection::operator=(const PhysicalPrinterCollection& other)
{
m_printers = other.m_printers;
m_default_config = other.m_default_config;
m_idx_selected = other.m_idx_selected;
m_selected_preset = other.m_selected_preset;
m_dir_path = other.m_dir_path;
return *this;
}

PhysicalPrinterCollection::PhysicalPrinterCollection(const PhysicalPrinterCollection& other)
{
m_printers = other.m_printers;
m_default_config = other.m_default_config;
m_idx_selected = other.m_idx_selected;
m_selected_preset = other.m_selected_preset;
m_dir_path = other.m_dir_path;
}

void PhysicalPrinterCollection::save_printer(PhysicalPrinter& edited_printer, const std::string& renamed_from/* = ""*/)
{
// controll and update preset_names in edited_printer config
Expand Down
38 changes: 22 additions & 16 deletions src/libslic3r/Preset.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class Preset
};

Preset(Type type, const std::string &name, bool is_default = false) : type(type), is_default(is_default), name(name) {}
Preset(){}
Preset() = default;

Type type = TYPE_INVALID;

Expand Down Expand Up @@ -257,10 +257,6 @@ class PresetCollection
public:
// Initialize the PresetCollection with the "- default -" preset.
PresetCollection(Preset::Type type, const std::vector<std::string> &keys, const Slic3r::StaticPrintConfig &defaults, const std::string &default_name = "- default -");
PresetCollection() {}
PresetCollection(const PresetCollection& other);
PresetCollection& operator=(const PresetCollection& other);
~PresetCollection();

typedef std::deque<Preset>::iterator Iterator;
typedef std::deque<Preset>::const_iterator ConstIterator;
Expand Down Expand Up @@ -464,6 +460,15 @@ class PresetCollection
size_t num_default_presets() { return m_num_default_presets; }

protected:
PresetCollection() = default;
// Copy constructor and copy operators are not to be used from outside PresetBundle,
// as the Profile::vendor points to an instance of VendorProfile stored at parent PresetBundle!
PresetCollection(const PresetCollection &other) = default;
PresetCollection& operator=(const PresetCollection &other) = default;
// After copying a collection with the default operators above, call this function
// to adjust Profile::vendor pointers.
void update_vendor_ptrs_after_copy(const VendorMap &vendors);

// Select a preset, if it exists. If it does not exist, select an invalid (-1) index.
// This is a temporary state, which shall be fixed immediately by the following step.
bool select_preset_by_name_strict(const std::string &name);
Expand All @@ -478,10 +483,6 @@ class PresetCollection
void update_map_system_profile_renamed();

private:
//PresetCollection();
//PresetCollection(const PresetCollection &other);
//PresetCollection& operator=(const PresetCollection &other);

// Find a preset position in the sorted list of presets.
// The "-- default -- " preset is always the first, so it needs
// to be handled differently.
Expand Down Expand Up @@ -535,7 +536,7 @@ class PresetCollection
// Path to the directory to store the config files into.
std::string m_dir_path;

// to access select_preset_by_name_strict()
// to access select_preset_by_name_strict() and the default & copy constructors.
friend class PresetBundle;
};

Expand All @@ -546,10 +547,17 @@ class PrinterPresetCollection : public PresetCollection
public:
PrinterPresetCollection(Preset::Type type, const std::vector<std::string> &keys, const Slic3r::StaticPrintConfig &defaults, const std::string &default_name = "- default -") :
PresetCollection(type, keys, defaults, default_name) {}
PrinterPresetCollection() : PresetCollection() {}

const Preset& default_preset_for(const DynamicPrintConfig &config) const override;

const Preset* find_by_model_id(const std::string &model_id) const;

private:
PrinterPresetCollection() = default;
PrinterPresetCollection(const PrinterPresetCollection &other) = default;
PrinterPresetCollection& operator=(const PrinterPresetCollection &other) = default;

friend class PresetBundle;
};

namespace PresetUtils {
Expand Down Expand Up @@ -639,10 +647,6 @@ class PhysicalPrinterCollection
{
public:
PhysicalPrinterCollection(const std::vector<std::string>& keys);
PhysicalPrinterCollection() {}
PhysicalPrinterCollection(const PhysicalPrinterCollection& other);
PhysicalPrinterCollection& operator=(const PhysicalPrinterCollection& other);
~PhysicalPrinterCollection() {}

typedef std::deque<PhysicalPrinter>::iterator Iterator;
typedef std::deque<PhysicalPrinter>::const_iterator ConstIterator;
Expand Down Expand Up @@ -733,7 +737,9 @@ class PhysicalPrinterCollection
const DynamicPrintConfig& default_config() const { return m_default_config; }

private:
// PhysicalPrinterCollection& operator=(const PhysicalPrinterCollection& other);
friend class PresetBundle;
PhysicalPrinterCollection() = default;
PhysicalPrinterCollection& operator=(const PhysicalPrinterCollection& other) = default;

// Find a physical printer position in the sorted list of printers.
// The name of a printer should be unique and case insensitive
Expand Down
36 changes: 23 additions & 13 deletions src/libslic3r/PresetBundle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,33 @@ PresetBundle::PresetBundle() :
this->project_config.apply_only(FullPrintConfig::defaults(), s_project_options);
}

PresetBundle::PresetBundle(const PresetBundle& src)
PresetBundle::PresetBundle(const PresetBundle &rhs)
{
prints = src.prints;
sla_prints = src.sla_prints;
filaments = src.filaments;
sla_materials = src.sla_materials;
printers = src.printers;
physical_printers = src.physical_printers;

filament_presets = src.filament_presets;
project_config = src.project_config;
vendors = src.vendors;
obsolete_presets = src.obsolete_presets;
*this = rhs;
}

PresetBundle::~PresetBundle()
PresetBundle& PresetBundle::operator=(const PresetBundle &rhs)
{
prints = rhs.prints;
sla_prints = rhs.sla_prints;
filaments = rhs.filaments;
sla_materials = rhs.sla_materials;
printers = rhs.printers;
physical_printers = rhs.physical_printers;

filament_presets = rhs.filament_presets;
project_config = rhs.project_config;
vendors = rhs.vendors;
obsolete_presets = rhs.obsolete_presets;

// Adjust Preset::vendor pointers to point to the copied vendors map.
prints .update_vendor_ptrs_after_copy(this->vendors);
sla_prints .update_vendor_ptrs_after_copy(this->vendors);
filaments .update_vendor_ptrs_after_copy(this->vendors);
sla_materials.update_vendor_ptrs_after_copy(this->vendors);
printers .update_vendor_ptrs_after_copy(this->vendors);

return *this;
}

void PresetBundle::reset(bool delete_files)
Expand Down
4 changes: 2 additions & 2 deletions src/libslic3r/PresetBundle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ class PresetBundle
{
public:
PresetBundle();
PresetBundle(const PresetBundle& src);
~PresetBundle();
PresetBundle(const PresetBundle &rhs);
PresetBundle& operator=(const PresetBundle &rhs);

// Remove all the presets but the "-- default --".
// Optionally remove all the files referenced by the presets from the user profile directory.
Expand Down

0 comments on commit e781cea

Please sign in to comment.