From ca32338ceae57164c4cf1f9ebb076c2779088e47 Mon Sep 17 00:00:00 2001 From: bubnikv Date: Fri, 2 Nov 2018 14:47:13 +0100 Subject: [PATCH 1/5] ModelBase ID refactoring, WIP --- src/libslic3r/Model.cpp | 155 +++++++++++++++++++++++++++------------- src/libslic3r/Model.hpp | 134 ++++++++++++++++++++++++++-------- 2 files changed, 207 insertions(+), 82 deletions(-) diff --git a/src/libslic3r/Model.cpp b/src/libslic3r/Model.cpp index d33f44c35..01886e870 100644 --- a/src/libslic3r/Model.cpp +++ b/src/libslic3r/Model.cpp @@ -21,26 +21,40 @@ namespace Slic3r { unsigned int Model::s_auto_extruder_id = 1; -ModelID ModelBase::s_last_id = 0; +size_t ModelBase::s_last_id = 0; -Model::Model(const Model &rhs) +Model& Model::assign_copy(const Model &rhs) { - *this = rhs; -} - -Model& Model::operator=(const Model &rhs) -{ - m_id = rhs.m_id; + this->copy_id(rhs); // copy materials - for (const auto &m : rhs.materials) + for (const std::pair &m : rhs.materials) this->add_material(m.first, *m.second); // copy objects this->objects.reserve(rhs.objects.size()); for (const ModelObject *o : rhs.objects) - this->add_object(*o, true); + this->add_object(*o); return *this; } +Model& Model::assign_copy(Model &&rhs) +{ + this->copy_id(rhs); + this->materials = std::move(rhs.materials); + rhs.materials.clear(); + this->objects = std::move(rhs.objects); + rhs.objects.clear(); + return *this; +} + +void Model::assign_new_unique_ids_recursive() +{ + this->set_new_unique_id(); + for (std::pair &m : this->materials) + m.second->assign_new_unique_ids_recursive(); + for (ModelObject *model_object : this->objects) + model_object->assign_new_unique_ids_recursive(); +} + Model Model::read_from_file(const std::string &input_file, DynamicPrintConfig *config, bool add_default_instances) { Model model; @@ -150,9 +164,9 @@ ModelObject* Model::add_object(const char *name, const char *path, TriangleMesh return new_object; } -ModelObject* Model::add_object(const ModelObject &other, bool copy_volumes) +ModelObject* Model::add_object(const ModelObject &other) { - ModelObject* new_object = new ModelObject(this, other, copy_volumes); + ModelObject* new_object = new ModelObject(this, other); this->objects.push_back(new_object); return new_object; } @@ -483,10 +497,10 @@ void Model::reset_auto_extruder_id() s_auto_extruder_id = 1; } -ModelObject::ModelObject(Model *model, const ModelObject &rhs, bool copy_volumes) : +ModelObject::ModelObject(Model *model, const ModelObject &rhs) : m_model(model) { - this->assign(&rhs, copy_volumes); + this->assign_copy(rhs); } ModelObject::~ModelObject() @@ -495,41 +509,74 @@ ModelObject::~ModelObject() this->clear_instances(); } -// Clone this ModelObject including its volumes and instances, keep the IDs of the copies equal to the original. -// Called by Print::apply() to clone the Model / ModelObject hierarchy to the back end for background processing. -ModelObject* ModelObject::clone(Model *parent) +// maintains the m_model pointer +ModelObject& ModelObject::assign_copy(const ModelObject &rhs) { - return new ModelObject(parent, *this, true); -} + this->copy_id(rhs); -ModelObject& ModelObject::assign(const ModelObject *rhs, bool copy_volumes) -{ - m_id = rhs->m_id; - name = rhs->name; - input_file = rhs->input_file; - config = rhs->config; - sla_support_points = rhs->sla_support_points; - layer_height_ranges = rhs->layer_height_ranges; - layer_height_profile = rhs->layer_height_profile; - layer_height_profile_valid = rhs->layer_height_profile_valid; - origin_translation = rhs->origin_translation; - m_bounding_box = rhs->m_bounding_box; - m_bounding_box_valid = rhs->m_bounding_box_valid; + this->name = rhs.name; + this->input_file = rhs.input_file; + this->config = rhs.config; + this->sla_support_points = rhs.sla_support_points; + this->layer_height_ranges = rhs.layer_height_ranges; + this->layer_height_profile = rhs.layer_height_profile; + this->layer_height_profile_valid = rhs.layer_height_profile_valid; + this->origin_translation = rhs.origin_translation; + m_bounding_box = rhs.m_bounding_box; + m_bounding_box_valid = rhs.m_bounding_box_valid; - volumes.clear(); - instances.clear(); - if (copy_volumes) { - this->volumes.reserve(rhs->volumes.size()); - for (ModelVolume *model_volume : rhs->volumes) - this->add_volume(*model_volume); - this->instances.reserve(rhs->instances.size()); - for (const ModelInstance *model_instance : rhs->instances) - this->add_instance(*model_instance); - } + this->volumes.clear(); + this->volumes.reserve(rhs.volumes.size()); + for (ModelVolume *model_volume : rhs.volumes) + this->add_volume(*model_volume); + this->instances.clear(); + this->instances.reserve(rhs.instances.size()); + for (const ModelInstance *model_instance : rhs.instances) + this->add_instance(*model_instance); return *this; } +// maintains the m_model pointer +ModelObject& ModelObject::assign_copy(ModelObject &&rhs) +{ + this->copy_id(rhs); + + this->name = std::move(rhs.name); + this->input_file = std::move(rhs.input_file); + this->config = std::move(rhs.config); + this->sla_support_points = std::move(rhs.sla_support_points); + this->layer_height_ranges = std::move(rhs.layer_height_ranges); + this->layer_height_profile = std::move(rhs.layer_height_profile); + this->layer_height_profile_valid = std::move(rhs.layer_height_profile_valid); + this->origin_translation = std::move(rhs.origin_translation); + m_bounding_box = std::move(rhs.m_bounding_box); + m_bounding_box_valid = std::move(rhs.m_bounding_box_valid); + + this->volumes = std::move(rhs.volumes); + rhs.volumes.clear(); + this->instances = std::move(rhs.instances); + rhs.instances.clear(); + + return *this; +} + +void ModelObject::assign_new_unique_ids_recursive() +{ + this->set_new_unique_id(); + for (ModelVolume *model_volume : this->volumes) + model_volume->assign_new_unique_ids_recursive(); + for (ModelInstance *model_instance : this->instances) + model_instance->assign_new_unique_ids_recursive(); +} + +// Clone this ModelObject including its volumes and instances, keep the IDs of the copies equal to the original. +// Called by Print::apply() to clone the Model / ModelObject hierarchy to the back end for background processing. +//ModelObject* ModelObject::clone(Model *parent) +//{ +// return new ModelObject(parent, *this, true); +//} + ModelVolume* ModelObject::add_volume(const TriangleMesh &mesh) { ModelVolume* v = new ModelVolume(this, mesh); @@ -554,6 +601,14 @@ ModelVolume* ModelObject::add_volume(const ModelVolume &other) return v; } +ModelVolume* ModelObject::add_volume(const ModelVolume &other, TriangleMesh &&mesh) +{ + ModelVolume* v = new ModelVolume(this, other, std::move(mesh)); + this->volumes.push_back(v); + this->invalidate_bounding_box(); + return v; +} + void ModelObject::delete_volume(size_t idx) { ModelVolumePtrs::iterator i = this->volumes.begin() + idx; @@ -876,15 +931,13 @@ void ModelObject::split(ModelObjectPtrs* new_objects) mesh->repair(); - ModelObject* new_object = m_model->add_object(*this, false); - new_object->sla_support_points.clear(); - new_object->input_file = ""; - ModelVolume* new_volume = new_object->add_volume(*mesh); - new_volume->name = volume->name; - new_volume->config = volume->config; - new_volume->set_type(volume->type()); - new_volume->set_material_id(volume->material_id()); - + ModelObject* new_object = m_model->add_object(); + new_object->name = this->name; + new_object->config = this->config; + new_object->instances.reserve(this->instances.size()); + for (const ModelInstance *model_instance : this->instances) + new_object->add_instance(*model_instance); + new_object->add_volume(*volume, std::move(*mesh)); new_objects->push_back(new_object); delete mesh; } diff --git a/src/libslic3r/Model.hpp b/src/libslic3r/Model.hpp index b72f20501..90ee456ec 100644 --- a/src/libslic3r/Model.hpp +++ b/src/libslic3r/Model.hpp @@ -27,16 +27,31 @@ class Print; typedef std::string t_model_material_id; typedef std::string t_model_material_attribute; -typedef std::map t_model_material_attributes; +typedef std::map t_model_material_attributes; -typedef std::map ModelMaterialMap; +typedef std::map ModelMaterialMap; typedef std::vector ModelObjectPtrs; typedef std::vector ModelVolumePtrs; typedef std::vector ModelInstancePtrs; // Unique identifier of a Model, ModelObject, ModelVolume, ModelInstance or ModelMaterial. // Used to synchronize the front end (UI) with the back end (BackgroundSlicingProcess / Print / PrintObject) -typedef size_t ModelID; +// Valid IDs are strictly positive (non zero). +// It is declared as an object, as some compilers (notably msvcc) consider a typedef size_t equivalent to size_t +// for parameter overload. +struct ModelID +{ + ModelID(size_t id) : id(id) {} + + bool operator==(const ModelID &rhs) const { return this->id == rhs.id; } + bool operator!=(const ModelID &rhs) const { return this->id != rhs.id; } + bool operator< (const ModelID &rhs) const { return this->id < rhs.id; } + bool operator> (const ModelID &rhs) const { return this->id > rhs.id; } + bool operator<=(const ModelID &rhs) const { return this->id <= rhs.id; } + bool operator>=(const ModelID &rhs) const { return this->id >= rhs.id; } + + size_t id; +}; // Base for Model, ModelObject, ModelVolume, ModelInstance or ModelMaterial to provide a unique ID // to synchronize the front end (UI) with the back end (BackgroundSlicingProcess / Print / PrintObject). @@ -45,18 +60,71 @@ typedef size_t ModelID; class ModelBase { public: - ModelID id() const { return m_id; } + ModelID id() const { return m_id; } + // Use with caution! + void set_new_unique_id() { m_id = generate_new_id(); } + void set_invalid_id() { m_id = 0; } + // Use with caution! + void copy_id(const ModelBase &rhs) { m_id = rhs.id(); } protected: - // Constructor to be only called by derived classes. - ModelBase() {} - ModelID m_id = generate_new_id(); + // Constructors to be only called by derived classes. + // Default constructor to assign a unique ID. + ModelBase() : m_id(generate_new_id()) {} + // Constructor with ignored int parameter to assign an invalid ID, to be replaced + // by an existing ID copied from elsewhere. + ModelBase(int) : m_id(ModelID(0)) {} + + // Override this method if a ModelBase derived class owns other ModelBase derived instances. + void assign_new_unique_ids_recursive() { this->set_new_unique_id(); } private: - static inline ModelID generate_new_id() { return s_last_id ++; } - static ModelID s_last_id; + ModelID m_id; + + static inline ModelID generate_new_id() { return ModelID(++ s_last_id); } + static size_t s_last_id; }; +#define MODELBASE_DERIVED_COPY_MOVE_CLONE(TYPE) \ + /* To be able to return an object from own copy / clone methods. Hopefully the compiler will do the "Copy elision" */ \ + /* (Omits copy and move(since C++11) constructors, resulting in zero - copy pass - by - value semantics). */ \ + TYPE(const TYPE &rhs) : ModelBase(-1) { this->assign_copy(rhs); } \ + explicit TYPE(TYPE &&rhs) : ModelBase(-1) { this->assign_clone(std::move(rhs)); } \ + TYPE& operator=(const TYPE &rhs) { this->assign_copy(rhs); return *this; } \ + TYPE& operator=(TYPE &&rhs) { this->assign_copy(std::move(rhs)); return *this; } \ + /* Copy a model, copy the IDs. The Print::apply() will call the TYPE::copy() method */ \ + /* to make a private copy for background processing. */ \ + static TYPE* new_copy(const TYPE &rhs) { return new TYPE(rhs); } \ + static TYPE* new_copy(TYPE &&rhs) { return new TYPE(std::move(rhs)); } \ + static TYPE make_copy(const TYPE &rhs) { return TYPE(rhs); } \ + static TYPE make_copy(TYPE &&rhs) { return TYPE(std::move(rhs)); } \ + TYPE& assign_copy(const TYPE &rhs); \ + TYPE& assign_copy(TYPE &&rhs); \ + /* Copy a TYPE, generate new IDs. The front end will use this call. */ \ + TYPE* new_clone(const TYPE &rhs) { \ + /* Default constructor assigning an invalid ID. */ \ + auto obj = new TYPE(-1); \ + obj->assign_clone(rhs); \ + return obj; \ + } \ + TYPE make_clone(const TYPE &rhs) { \ + /* Default constructor assigning an invalid ID. */ \ + TYPE obj(-1); \ + obj.assign_clone(rhs); \ + return obj; \ + } \ + TYPE& assign_clone(const TYPE &rhs) { \ + this->assign_copy(rhs); \ + this->assign_new_unique_ids_recursive(); \ + return *this; \ + } + +#define MODELBASE_DERIVED_PRIVATE_COPY_MOVE(TYPE) \ +private: \ + /* Private constructor with an unused int parameter will create a TYPE instance with an invalid ID. */ \ + explicit TYPE(int) : ModelBase(-1) {}; \ + void assign_new_unique_ids_recursive(); + // Material, which may be shared across multiple ModelObjects of a single Model. class ModelMaterial : public ModelBase { @@ -119,15 +187,12 @@ public: when user expects that. */ Vec3d origin_translation; - // Assign a ModelObject to this object while keeping the original pointer to the parent Model. - // Make a deep copy. - ModelObject& assign(const ModelObject *rhs, bool copy_volumes = true); - Model* get_model() const { return m_model; }; ModelVolume* add_volume(const TriangleMesh &mesh); ModelVolume* add_volume(TriangleMesh &&mesh); ModelVolume* add_volume(const ModelVolume &volume); + ModelVolume* add_volume(const ModelVolume &volume, TriangleMesh &&mesh); void delete_volume(size_t idx); void clear_volumes(); bool is_multiparts() const { return volumes.size() > 1; } @@ -184,17 +249,16 @@ public: protected: friend class Print; - // Clone this ModelObject including its volumes and instances, keep the IDs of the copies equal to the original. - // Called by Print::apply() to clone the Model / ModelObject hierarchy to the back end for background processing. - ModelObject* clone(Model *parent); - void set_model(Model *model) { m_model = model; } + // Called by Print::apply() to set the model pointer after making a copy. + void set_model(Model *model) { m_model = model; } private: ModelObject(Model *model) : layer_height_profile_valid(false), m_model(model), origin_translation(Vec3d::Zero()), m_bounding_box_valid(false) {} - ModelObject(Model *model, const ModelObject &rhs, bool copy_volumes = true); - explicit ModelObject(ModelObject &rhs) = delete; + ModelObject(Model *model, const ModelObject &rhs); ~ModelObject(); - ModelObject& operator=(ModelObject &rhs) = default; + + MODELBASE_DERIVED_COPY_MOVE_CLONE(ModelObject) + MODELBASE_DERIVED_PRIVATE_COPY_MOVE(ModelObject) // Parent object, owning this ModelObject. Model *m_model; @@ -314,7 +378,7 @@ private: { this->set_material_id(other.material_id()); } - ModelVolume(ModelObject *object, const ModelVolume &other, const TriangleMesh &&mesh) : + ModelVolume(ModelObject *object, const ModelVolume &other, TriangleMesh &&mesh) : ModelBase(other), // copy the ID name(other.name), mesh(std::move(mesh)), config(other.config), m_type(other.m_type), object(object) { @@ -460,30 +524,32 @@ class Model : public ModelBase public: // Materials are owned by a model and referenced by objects through t_model_material_id. // Single material may be shared by multiple models. - ModelMaterialMap materials; + ModelMaterialMap materials; // Objects are owned by a model. Each model may have multiple instances, each instance having its own transformation (shift, scale, rotation). - ModelObjectPtrs objects; + ModelObjectPtrs objects; + // Default constructor assigns a new ID to the model. Model() {} - Model(const Model &rhs); - Model& operator=(const Model &rhs); ~Model() { this->clear_objects(); this->clear_materials(); } - // XXX: use fs::path ? + MODELBASE_DERIVED_COPY_MOVE_CLONE(Model) + static Model read_from_file(const std::string &input_file, DynamicPrintConfig *config = nullptr, bool add_default_instances = true); static Model read_from_archive(const std::string &input_file, DynamicPrintConfig *config, bool add_default_instances = true); /// Repair the ModelObjects of the current Model. /// This function calls repair function on each TriangleMesh of each model object volume - void repair(); + void repair(); + // Add a new ModelObject to this Model, generate a new ID for this ModelObject. ModelObject* add_object(); ModelObject* add_object(const char *name, const char *path, const TriangleMesh &mesh); ModelObject* add_object(const char *name, const char *path, TriangleMesh &&mesh); - ModelObject* add_object(const ModelObject &other, bool copy_volumes = true); - void delete_object(size_t idx); - void delete_object(ModelObject* object); - void clear_objects(); + ModelObject* add_object(const ModelObject &other); + void delete_object(size_t idx); + void delete_object(ModelID id); + void delete_object(ModelObject* object); + void clear_objects(); ModelMaterial* add_material(t_model_material_id material_id); ModelMaterial* add_material(t_model_material_id material_id, const ModelMaterial &other); @@ -520,8 +586,14 @@ public: static unsigned int get_auto_extruder_id(unsigned int max_extruders); static std::string get_auto_extruder_id_as_string(unsigned int max_extruders); static void reset_auto_extruder_id(); + +private: + MODELBASE_DERIVED_PRIVATE_COPY_MOVE(Model) }; +#undef MODELBASE_DERIVED_COPY_MOVE_CLONE +#undef MODELBASE_DERIVED_PRIVATE_COPY_MOVE + } #endif From d26d90ac85e948a1d54d8026d356290655871c6b Mon Sep 17 00:00:00 2001 From: bubnikv Date: Fri, 2 Nov 2018 15:08:08 +0100 Subject: [PATCH 2/5] ModelBase ID refactoring, WIP --- src/libslic3r/Print.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libslic3r/Print.cpp b/src/libslic3r/Print.cpp index 16b3ff773..402edaeac 100644 --- a/src/libslic3r/Print.cpp +++ b/src/libslic3r/Print.cpp @@ -774,7 +774,7 @@ Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &co for (PrintRegion *region : m_regions) delete region; m_regions.clear(); - m_model = model; + m_model.assign_copy(model); for (const ModelObject *model_object : m_model.objects) model_object_status.emplace(model_object->id(), ModelObjectStatus::New); } else { @@ -789,7 +789,8 @@ Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &co model_object_status.emplace(model_object->id(), ModelObjectStatus::Old); for (size_t i = m_model.objects.size(); i < model.objects.size(); ++ i) { model_object_status.emplace(model.objects[i]->id(), ModelObjectStatus::New); - m_model.objects.emplace_back(model.objects[i]->clone(&m_model)); + m_model.objects.emplace_back(ModelObject::new_copy(*model.objects[i])); + m_model.objects.back()->set_model(&m_model); } } else { // Reorder the objects, add new objects. @@ -806,7 +807,8 @@ Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &co auto it = std::lower_bound(model_objects_old.begin(), model_objects_old.end(), mobj, by_id_lower); if (it == model_objects_old.end() || (*it)->id() != mobj->id()) { // New ModelObject added. - m_model.objects.emplace_back((*it)->clone(&m_model)); + m_model.objects.emplace_back(ModelObject::new_copy(**it)); + m_model.objects.back()->set_model(&m_model); model_object_status.emplace(mobj->id(), ModelObjectStatus::New); } else { // Existing ModelObject re-added (possibly moved in the list). @@ -899,8 +901,8 @@ Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &co update_apply_status(it->print_object->invalidate_all_steps()); const_cast(*it).status = PrintObjectStatus::Deleted; } - // Copy content of the ModelObject including its ID, reset the parent. - model_object.assign(&model_object_new); + // Copy content of the ModelObject including its ID, do not change the parent. + model_object.assign_copy(model_object_new); } else if (support_blockers_differ || support_enforcers_differ) { // First stop background processing before shuffling or deleting the ModelVolumes in the ModelObject's list. m_cancel_callback(); From cf5dcfa9ed091d9a1b982ddb489ef72bb5528463 Mon Sep 17 00:00:00 2001 From: bubnikv Date: Fri, 2 Nov 2018 19:49:40 +0100 Subject: [PATCH 3/5] ModelBase ID refactoring, starting to work. Now it remains to clean up some of the no more used Model interfaces. --- src/libslic3r/Model.cpp | 112 +++++++++++++++++++++++++------------- src/libslic3r/Model.hpp | 100 ++++++++++++++++++++++------------ src/libslic3r/Print.cpp | 66 +++++++++++++++++++++- src/slic3r/GUI/Plater.cpp | 2 + xs/xsp/Model.xsp | 2 +- 5 files changed, 205 insertions(+), 77 deletions(-) diff --git a/src/libslic3r/Model.cpp b/src/libslic3r/Model.cpp index 01886e870..79fd7292f 100644 --- a/src/libslic3r/Model.cpp +++ b/src/libslic3r/Model.cpp @@ -27,21 +27,39 @@ Model& Model::assign_copy(const Model &rhs) { this->copy_id(rhs); // copy materials - for (const std::pair &m : rhs.materials) - this->add_material(m.first, *m.second); + this->clear_materials(); + this->materials = rhs.materials; + for (std::pair &m : this->materials) { + // Copy including the ID and m_model. + m.second = new ModelMaterial(*m.second); + m.second->set_model(this); + } // copy objects + this->clear_objects(); this->objects.reserve(rhs.objects.size()); - for (const ModelObject *o : rhs.objects) - this->add_object(*o); + for (const ModelObject *model_object : rhs.objects) { + // Copy including the ID, leave ID set to invalid (zero). + auto mo = ModelObject::new_copy(*model_object); + mo->set_model(this); + this->objects.emplace_back(mo); + } return *this; } Model& Model::assign_copy(Model &&rhs) { this->copy_id(rhs); - this->materials = std::move(rhs.materials); + // Move materials, adjust the parent pointer. + this->clear_materials(); + this->materials = std::move(rhs.materials); + for (std::pair &m : this->materials) + m.second->set_model(this); rhs.materials.clear(); + // Move objects, adjust the parent pointer. + this->clear_objects(); this->objects = std::move(rhs.objects); + for (ModelObject *model_object : this->objects) + model_object->set_model(this); rhs.objects.clear(); return *this; } @@ -166,7 +184,8 @@ ModelObject* Model::add_object(const char *name, const char *path, TriangleMesh ModelObject* Model::add_object(const ModelObject &other) { - ModelObject* new_object = new ModelObject(this, other); + ModelObject* new_object = ModelObject::new_clone(other); + new_object->set_model(this); this->objects.push_back(new_object); return new_object; } @@ -178,21 +197,36 @@ void Model::delete_object(size_t idx) this->objects.erase(i); } -void Model::delete_object(ModelObject* object) +bool Model::delete_object(ModelObject* object) { - if (object == nullptr) - return; - - for (ModelObjectPtrs::iterator it = objects.begin(); it != objects.end(); ++it) - { - ModelObject* obj = *it; - if (obj == object) - { - delete obj; - objects.erase(it); - return; + if (object != nullptr) { + size_t idx = 0; + for (ModelObject *model_object : objects) { + if (model_object == object) { + delete model_object; + objects.erase(objects.begin() + idx); + return true; + } + ++ idx; } } + return false; +} + +bool Model::delete_object(ModelID id) +{ + if (id.id != 0) { + size_t idx = 0; + for (ModelObject *model_object : objects) { + if (model_object->id() == id) { + delete model_object; + objects.erase(objects.begin() + idx); + return true; + } + ++ idx; + } + } + return false; } void Model::clear_objects() @@ -232,7 +266,8 @@ ModelMaterial* Model::add_material(t_model_material_id material_id, const ModelM ModelMaterial* material = this->get_material(material_id); delete material; // set new material - material = new ModelMaterial(this, other); + material = new ModelMaterial(other); + material->set_model(this); this->materials[material_id] = material; return material; } @@ -435,6 +470,7 @@ void Model::convert_multipart_object(unsigned int max_extruders) ModelObject* object = new ModelObject(this); object->input_file = this->objects.front()->input_file; object->name = this->objects.front()->name; + //FIXME copy the config etc? reset_auto_extruder_id(); @@ -497,12 +533,6 @@ void Model::reset_auto_extruder_id() s_auto_extruder_id = 1; } -ModelObject::ModelObject(Model *model, const ModelObject &rhs) : - m_model(model) -{ - this->assign_copy(rhs); -} - ModelObject::~ModelObject() { this->clear_volumes(); @@ -525,14 +555,18 @@ ModelObject& ModelObject::assign_copy(const ModelObject &rhs) m_bounding_box = rhs.m_bounding_box; m_bounding_box_valid = rhs.m_bounding_box_valid; - this->volumes.clear(); + this->clear_volumes(); this->volumes.reserve(rhs.volumes.size()); - for (ModelVolume *model_volume : rhs.volumes) - this->add_volume(*model_volume); - this->instances.clear(); + for (ModelVolume *model_volume : rhs.volumes) { + this->volumes.emplace_back(new ModelVolume(*model_volume)); + this->volumes.back()->set_model_object(this); + } + this->clear_instances(); this->instances.reserve(rhs.instances.size()); - for (const ModelInstance *model_instance : rhs.instances) - this->add_instance(*model_instance); + for (const ModelInstance *model_instance : rhs.instances) { + this->instances.emplace_back(new ModelInstance(*model_instance)); + this->instances.back()->set_model_object(this); + } return *this; } @@ -553,10 +587,16 @@ ModelObject& ModelObject::assign_copy(ModelObject &&rhs) m_bounding_box = std::move(rhs.m_bounding_box); m_bounding_box_valid = std::move(rhs.m_bounding_box_valid); + this->clear_volumes(); this->volumes = std::move(rhs.volumes); rhs.volumes.clear(); + for (ModelVolume *model_volume : this->volumes) + model_volume->set_model_object(this); + this->clear_instances(); this->instances = std::move(rhs.instances); rhs.instances.clear(); + for (ModelInstance *model_instance : this->instances) + model_instance->set_model_object(this); return *this; } @@ -919,7 +959,7 @@ void ModelObject::split(ModelObjectPtrs* new_objects) if (this->volumes.size() > 1) { // We can't split meshes if there's more than one volume, because // we can't group the resulting meshes by object afterwards - new_objects->push_back(this); + new_objects->emplace_back(this); return; } @@ -938,7 +978,7 @@ void ModelObject::split(ModelObjectPtrs* new_objects) for (const ModelInstance *model_instance : this->instances) new_object->add_instance(*model_instance); new_object->add_volume(*volume, std::move(*mesh)); - new_objects->push_back(new_object); + new_objects->emplace_back(new_object); delete mesh; } @@ -1060,9 +1100,8 @@ void ModelObject::print_info() const void ModelVolume::set_material_id(t_model_material_id material_id) { m_material_id = material_id; - // ensure m_material_id references an existing material - (void)this->object->get_model()->add_material(material_id); + this->object->get_model()->add_material(material_id); } ModelMaterial* ModelVolume::material() const @@ -1073,13 +1112,12 @@ ModelMaterial* ModelVolume::material() const void ModelVolume::set_material(t_model_material_id material_id, const ModelMaterial &material) { m_material_id = material_id; - (void)this->object->get_model()->add_material(material_id, material); + this->object->get_model()->add_material(material_id, material); } ModelMaterial* ModelVolume::assign_unique_material() { Model* model = this->get_object()->get_model(); - // as material-id "0" is reserved by the AMF spec we start from 1 m_material_id = 1 + model->materials.size(); // watchout for implicit cast return model->add_material(m_material_id); diff --git a/src/libslic3r/Model.hpp b/src/libslic3r/Model.hpp index 90ee456ec..adc1f3406 100644 --- a/src/libslic3r/Model.hpp +++ b/src/libslic3r/Model.hpp @@ -61,11 +61,6 @@ class ModelBase { public: ModelID id() const { return m_id; } - // Use with caution! - void set_new_unique_id() { m_id = generate_new_id(); } - void set_invalid_id() { m_id = 0; } - // Use with caution! - void copy_id(const ModelBase &rhs) { m_id = rhs.id(); } protected: // Constructors to be only called by derived classes. @@ -75,6 +70,12 @@ protected: // by an existing ID copied from elsewhere. ModelBase(int) : m_id(ModelID(0)) {} + // Use with caution! + void set_new_unique_id() { m_id = generate_new_id(); } + void set_invalid_id() { m_id = 0; } + // Use with caution! + void copy_id(const ModelBase &rhs) { m_id = rhs.id(); } + // Override this method if a ModelBase derived class owns other ModelBase derived instances. void assign_new_unique_ids_recursive() { this->set_new_unique_id(); } @@ -86,12 +87,6 @@ private: }; #define MODELBASE_DERIVED_COPY_MOVE_CLONE(TYPE) \ - /* To be able to return an object from own copy / clone methods. Hopefully the compiler will do the "Copy elision" */ \ - /* (Omits copy and move(since C++11) constructors, resulting in zero - copy pass - by - value semantics). */ \ - TYPE(const TYPE &rhs) : ModelBase(-1) { this->assign_copy(rhs); } \ - explicit TYPE(TYPE &&rhs) : ModelBase(-1) { this->assign_clone(std::move(rhs)); } \ - TYPE& operator=(const TYPE &rhs) { this->assign_copy(rhs); return *this; } \ - TYPE& operator=(TYPE &&rhs) { this->assign_copy(std::move(rhs)); return *this; } \ /* Copy a model, copy the IDs. The Print::apply() will call the TYPE::copy() method */ \ /* to make a private copy for background processing. */ \ static TYPE* new_copy(const TYPE &rhs) { return new TYPE(rhs); } \ @@ -101,7 +96,7 @@ private: TYPE& assign_copy(const TYPE &rhs); \ TYPE& assign_copy(TYPE &&rhs); \ /* Copy a TYPE, generate new IDs. The front end will use this call. */ \ - TYPE* new_clone(const TYPE &rhs) { \ + static TYPE* new_clone(const TYPE &rhs) { \ /* Default constructor assigning an invalid ID. */ \ auto obj = new TYPE(-1); \ obj->assign_clone(rhs); \ @@ -128,7 +123,6 @@ private: \ // Material, which may be shared across multiple ModelObjects of a single Model. class ModelMaterial : public ModelBase { - friend class Model; public: // Attributes are defined by the AMF file format, but they don't seem to be used by Slic3r for any purpose. t_model_material_attributes attributes; @@ -139,14 +133,22 @@ public: void apply(const t_model_material_attributes &attributes) { this->attributes.insert(attributes.begin(), attributes.end()); } +protected: + friend class Model; + // Constructor, which assigns a new unique ID. + ModelMaterial(Model *model) : m_model(model) {} + // Copy constructor copies the ID and m_model! + ModelMaterial(const ModelMaterial &rhs) = default; + void set_model(Model *model) { m_model = model; } + private: // Parent, owning this material. Model *m_model; - ModelMaterial(Model *model) : m_model(model) {} - ModelMaterial(Model *model, const ModelMaterial &other) : attributes(other.attributes), config(other.config), m_model(model) {} - explicit ModelMaterial(ModelMaterial &rhs) = delete; - ModelMaterial& operator=(ModelMaterial &rhs) = delete; + ModelMaterial() = delete; + ModelMaterial(ModelMaterial &&rhs) = delete; + ModelMaterial& operator=(const ModelMaterial &rhs) = delete; + ModelMaterial& operator=(ModelMaterial &&rhs) = delete; }; // A printable object, possibly having multiple print volumes (each with its own set of parameters and materials), @@ -254,16 +256,23 @@ protected: private: ModelObject(Model *model) : layer_height_profile_valid(false), m_model(model), origin_translation(Vec3d::Zero()), m_bounding_box_valid(false) {} - ModelObject(Model *model, const ModelObject &rhs); + ModelObject(Model *model, const ModelObject &rhs) { this->assign_copy(rhs); m_model = model; } ~ModelObject(); - MODELBASE_DERIVED_COPY_MOVE_CLONE(ModelObject) + /* To be able to return an object from own copy / clone methods. Hopefully the compiler will do the "Copy elision" */ + /* (Omits copy and move(since C++11) constructors, resulting in zero - copy pass - by - value semantics). */ + ModelObject(const ModelObject &rhs) : ModelBase(-1), m_model(rhs.m_model) { this->assign_copy(rhs); } + explicit ModelObject(ModelObject &&rhs) : ModelBase(-1) { this->assign_copy(std::move(rhs)); } + ModelObject& operator=(const ModelObject &rhs) { this->assign_copy(rhs); m_model = rhs.m_model; return *this; } + ModelObject& operator=(ModelObject &&rhs) { this->assign_copy(std::move(rhs)); m_model = rhs.m_model; return *this; } + + MODELBASE_DERIVED_COPY_MOVE_CLONE(ModelObject) MODELBASE_DERIVED_PRIVATE_COPY_MOVE(ModelObject) - // Parent object, owning this ModelObject. - Model *m_model; - // Bounding box, cached. + // Parent object, owning this ModelObject. Set to nullptr here, so the macros above will have it initialized. + Model *m_model = nullptr; + // Bounding box, cached. mutable BoundingBoxf3 m_bounding_box; mutable bool m_bounding_box_valid; }; @@ -353,6 +362,10 @@ public: const Transform3d& get_matrix(bool dont_translate = false, bool dont_rotate = false, bool dont_scale = false, bool dont_mirror = false) const { return m_transformation.get_matrix(dont_translate, dont_rotate, dont_scale, dont_mirror); } #endif // ENABLE_MODELVOLUME_TRANSFORM +protected: + explicit ModelVolume(ModelVolume &rhs) = default; + void set_model_object(ModelObject *model_object) { object = model_object; } + private: // Parent object owning this ModelVolume. ModelObject* object; @@ -373,21 +386,20 @@ private: ModelVolume(ModelObject *object, TriangleMesh &&mesh, TriangleMesh &&convex_hull) : mesh(std::move(mesh)), m_convex_hull(std::move(convex_hull)), m_type(MODEL_PART), object(object) {} ModelVolume(ModelObject *object, const ModelVolume &other) : - ModelBase(other), // copy the ID name(other.name), mesh(other.mesh), m_convex_hull(other.m_convex_hull), config(other.config), m_type(other.m_type), object(object) { - this->set_material_id(other.material_id()); + if (! other.material_id().empty()) + this->set_material_id(other.material_id()); } ModelVolume(ModelObject *object, const ModelVolume &other, TriangleMesh &&mesh) : - ModelBase(other), // copy the ID name(other.name), mesh(std::move(mesh)), config(other.config), m_type(other.m_type), object(object) { - this->set_material_id(other.material_id()); + if (! other.material_id().empty()) + this->set_material_id(other.material_id()); if (mesh.stl.stats.number_of_facets > 1) calculate_convex_hull(); } - explicit ModelVolume(ModelVolume &rhs) = delete; ModelVolume& operator=(ModelVolume &rhs) = delete; }; @@ -404,8 +416,6 @@ public: Num_BedStates }; - friend class ModelObject; - private: #if ENABLE_MODELVOLUME_TRANSFORM Geometry::Transformation m_transformation; @@ -494,12 +504,21 @@ public: bool is_printable() const { return print_volume_state == PVS_Inside; } +protected: + friend class Print; + friend class ModelObject; + + explicit ModelInstance(const ModelInstance &rhs) = default; + void set_model_object(ModelObject *model_object) { object = model_object; } + private: // Parent object, owning this instance. ModelObject* object; #if ENABLE_MODELVOLUME_TRANSFORM + // Constructor, which assigns a new unique ID. ModelInstance(ModelObject *object) : object(object), print_volume_state(PVS_Inside) {} + // Constructor, which assigns a new unique ID. ModelInstance(ModelObject *object, const ModelInstance &other) : m_transformation(other.m_transformation), object(object), print_volume_state(PVS_Inside) {} #else @@ -508,8 +527,10 @@ private: m_offset(other.m_offset), m_rotation(other.m_rotation), m_scaling_factor(other.m_scaling_factor), m_mirror(other.m_mirror), object(object), print_volume_state(PVS_Inside) {} #endif // ENABLE_MODELVOLUME_TRANSFORM - explicit ModelInstance(ModelInstance &rhs) = delete; - ModelInstance& operator=(ModelInstance &rhs) = delete; + ModelInstance() = delete; + explicit ModelInstance(ModelInstance &&rhs) = delete; + ModelInstance& operator=(const ModelInstance &rhs) = delete; + ModelInstance& operator=(ModelInstance &&rhs) = delete; }; // The print bed content. @@ -532,6 +553,13 @@ public: Model() {} ~Model() { this->clear_objects(); this->clear_materials(); } + /* To be able to return an object from own copy / clone methods. Hopefully the compiler will do the "Copy elision" */ + /* (Omits copy and move(since C++11) constructors, resulting in zero - copy pass - by - value semantics). */ + Model(const Model &rhs) : ModelBase(-1) { this->assign_copy(rhs); } + explicit Model(Model &&rhs) : ModelBase(-1) { this->assign_copy(std::move(rhs)); } + Model& operator=(const Model &rhs) { this->assign_copy(rhs); return *this; } + Model& operator=(Model &&rhs) { this->assign_copy(std::move(rhs)); return *this; } + MODELBASE_DERIVED_COPY_MOVE_CLONE(Model) static Model read_from_file(const std::string &input_file, DynamicPrintConfig *config = nullptr, bool add_default_instances = true); @@ -547,8 +575,8 @@ public: ModelObject* add_object(const char *name, const char *path, TriangleMesh &&mesh); ModelObject* add_object(const ModelObject &other); void delete_object(size_t idx); - void delete_object(ModelID id); - void delete_object(ModelObject* object); + bool delete_object(ModelID id); + bool delete_object(ModelObject* object); void clear_objects(); ModelMaterial* add_material(t_model_material_id material_id); @@ -558,9 +586,9 @@ public: return (i == this->materials.end()) ? nullptr : i->second; } - void delete_material(t_model_material_id material_id); - void clear_materials(); - bool add_default_instances(); + void delete_material(t_model_material_id material_id); + void clear_materials(); + bool add_default_instances(); // Returns approximate axis aligned bounding box of this model BoundingBoxf3 bounding_box() const; // Set the print_volume_state of PrintObject::instances, diff --git a/src/libslic3r/Print.cpp b/src/libslic3r/Print.cpp index 402edaeac..4052c79ac 100644 --- a/src/libslic3r/Print.cpp +++ b/src/libslic3r/Print.cpp @@ -709,8 +709,60 @@ static std::vector print_objects_from_model_object(const ModelOb return std::vector(trafos.begin(), trafos.end()); } +#ifdef _DEBUG +// Verify whether the IDs of Model / ModelObject / ModelVolume / ModelInstance / ModelMaterial are valid and unique. +static inline void check_model_ids_validity(const Model &model) +{ + std::set ids; + auto check = [&ids](ModelID id) { + assert(id.id > 0); + assert(ids.find(id) == ids.end()); + ids.insert(id); + }; + for (const ModelObject *model_object : model.objects) { + check(model_object->id()); + for (const ModelVolume *model_volume : model_object->volumes) + check(model_volume->id()); + for (const ModelInstance *model_instance : model_object->instances) + check(model_instance->id()); + } + for (const auto mm : model.materials) + check(mm.second->id()); +} + +static inline void check_model_ids_equal(const Model &model1, const Model &model2) +{ + // Verify whether the IDs of model1 and model match. + assert(model1.objects.size() == model2.objects.size()); + for (size_t idx_model = 0; idx_model < model2.objects.size(); ++ idx_model) { + const ModelObject &model_object1 = *model1.objects[idx_model]; + const ModelObject &model_object2 = * model2.objects[idx_model]; + assert(model_object1.id() == model_object2.id()); + assert(model_object1.volumes.size() == model_object2.volumes.size()); + assert(model_object1.instances.size() == model_object2.instances.size()); + for (size_t i = 0; i < model_object1.volumes.size(); ++ i) + assert(model_object1.volumes[i]->id() == model_object2.volumes[i]->id()); + for (size_t i = 0; i < model_object1.instances.size(); ++ i) + assert(model_object1.instances[i]->id() == model_object2.instances[i]->id()); + } + assert(model1.materials.size() == model2.materials.size()); + { + auto it1 = model1.materials.begin(); + auto it2 = model2.materials.begin(); + for (; it1 != model1.materials.end(); ++ it1, ++ it2) { + assert(it1->first == it2->first); // compare keys + assert(it1->second->id() == it2->second->id()); + } + } +} +#endif /* _DEBUG */ + Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &config_in) { +#ifdef _DEBUG + check_model_ids_validity(model); +#endif /* _DEBUG */ + // Make a copy of the config, normalize it. DynamicPrintConfig config(config_in); config.normalize(); @@ -807,7 +859,7 @@ Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &co auto it = std::lower_bound(model_objects_old.begin(), model_objects_old.end(), mobj, by_id_lower); if (it == model_objects_old.end() || (*it)->id() != mobj->id()) { // New ModelObject added. - m_model.objects.emplace_back(ModelObject::new_copy(**it)); + m_model.objects.emplace_back(ModelObject::new_copy(*mobj)); m_model.objects.back()->set_model(&m_model); model_object_status.emplace(mobj->id(), ModelObjectStatus::New); } else { @@ -935,8 +987,11 @@ Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &co model_object.name = model_object_new.name; model_object.input_file = model_object_new.input_file; model_object.clear_instances(); - for (const ModelInstance *model_instance : model_object_new.instances) - model_object.add_instance(*model_instance); + model_object.instances.reserve(model_object_new.instances.size()); + for (const ModelInstance *model_instance : model_object_new.instances) { + model_object.instances.emplace_back(new ModelInstance(*model_instance)); + model_object.instances.back()->set_model_object(&model_object); + } } } @@ -1138,6 +1193,11 @@ Print::ApplyStatus Print::apply(const Model &model, const DynamicPrintConfig &co object->update_layer_height_profile(); this->update_object_placeholders(); + +#ifdef _DEBUG + check_model_ids_equal(m_model, model); +#endif /* _DEBUG */ + return static_cast(apply_status); } diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 4cb274a50..a78b656d2 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -1435,7 +1435,9 @@ void Plater::priv::mirror(Axis axis) void Plater::priv::arrange() { + this->background_process.stop(); main_frame->app_controller()->arrange_model(); + this->schedule_background_process(); // ignore arrange failures on purpose: user has visual feedback and we don't need to warn him // when parts don't fit in print bed diff --git a/xs/xsp/Model.xsp b/xs/xsp/Model.xsp index 180e052ff..a198b1b9f 100644 --- a/xs/xsp/Model.xsp +++ b/xs/xsp/Model.xsp @@ -32,7 +32,7 @@ %name{_add_object} Ref add_object(); Ref _add_object_clone(ModelObject* other, bool copy_volumes = true) - %code%{ RETVAL = THIS->add_object(*other, copy_volumes); %}; + %code%{ auto ptr = THIS->add_object(*other); if (! copy_volumes) ptr->clear_volumes(); RETVAL = ptr; %}; void delete_object(size_t idx); void clear_objects(); size_t objects_count() From 5da83742a37141467a9887067592e72f8657e379 Mon Sep 17 00:00:00 2001 From: bubnikv Date: Fri, 2 Nov 2018 20:41:49 +0100 Subject: [PATCH 4/5] Some refactoring. --- src/libslic3r/Layer.cpp | 14 ++++++-------- src/libslic3r/Model.hpp | 19 ++++++++----------- src/libslic3r/Print.cpp | 8 +++++--- src/libslic3r/Print.hpp | 9 +++------ src/libslic3r/PrintObject.cpp | 17 +++++++++-------- 5 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/libslic3r/Layer.cpp b/src/libslic3r/Layer.cpp index afdde1852..5003c67d7 100644 --- a/src/libslic3r/Layer.cpp +++ b/src/libslic3r/Layer.cpp @@ -32,9 +32,8 @@ void Layer::make_slices() slices = m_regions.front()->slices; } else { Polygons slices_p; - FOREACH_LAYERREGION(this, layerm) { - polygons_append(slices_p, to_polygons((*layerm)->slices)); - } + for (LayerRegion *layerm : m_regions) + polygons_append(slices_p, to_polygons(layerm->slices)); slices = union_ex(slices_p); } @@ -53,7 +52,7 @@ void Layer::make_slices() // populate slices vector for (size_t i : order) - this->slices.expolygons.push_back(STDMOVE(slices[i])); + this->slices.expolygons.push_back(std::move(slices[i])); } void Layer::merge_slices() @@ -63,10 +62,9 @@ void Layer::merge_slices() // but use the non-split islands of a layer. For a single region print, these shall be equal. m_regions.front()->slices.set(this->slices.expolygons, stInternal); } else { - FOREACH_LAYERREGION(this, layerm) { + for (LayerRegion *layerm : m_regions) // without safety offset, artifacts are generated (GH #2494) - (*layerm)->slices.set(union_ex(to_polygons(STDMOVE((*layerm)->slices.surfaces)), true), stInternal); - } + layerm->slices.set(union_ex(to_polygons(std::move(layerm->slices.surfaces)), true), stInternal); } } @@ -80,7 +78,7 @@ void Layer::make_perimeters() // keep track of regions whose perimeters we have already generated std::set done; - FOREACH_LAYERREGION(this, layerm) { + for (LayerRegionPtrs::iterator layerm = m_regions.begin(); layerm != m_regions.end(); ++ layerm) { size_t region_id = layerm - m_regions.begin(); if (done.find(region_id) != done.end()) continue; BOOST_LOG_TRIVIAL(trace) << "Generating perimeters for layer " << this->id() << ", region " << region_id; diff --git a/src/libslic3r/Model.hpp b/src/libslic3r/Model.hpp index adc1f3406..cfa6ed25b 100644 --- a/src/libslic3r/Model.hpp +++ b/src/libslic3r/Model.hpp @@ -256,7 +256,6 @@ protected: private: ModelObject(Model *model) : layer_height_profile_valid(false), m_model(model), origin_translation(Vec3d::Zero()), m_bounding_box_valid(false) {} - ModelObject(Model *model, const ModelObject &rhs) { this->assign_copy(rhs); m_model = model; } ~ModelObject(); /* To be able to return an object from own copy / clone methods. Hopefully the compiler will do the "Copy elision" */ @@ -281,8 +280,6 @@ private: // ModelVolume instances are owned by a ModelObject. class ModelVolume : public ModelBase { - friend class ModelObject; - public: std::string name; // The triangular model. @@ -299,9 +296,6 @@ public: SUPPORT_BLOCKER, }; - // Clone this ModelVolume, keep the ID identical, set the parent to the cloned volume. - ModelVolume* clone(ModelObject *parent) { return new ModelVolume(parent, *this); } - // A parent object owning this modifier volume. ModelObject* get_object() const { return this->object; }; Type type() const { return m_type; } @@ -363,7 +357,10 @@ public: #endif // ENABLE_MODELVOLUME_TRANSFORM protected: - explicit ModelVolume(ModelVolume &rhs) = default; + friend class Print; + friend class ModelObject; + + explicit ModelVolume(ModelVolume &rhs) = default; void set_model_object(ModelObject *model_object) { object = model_object; } private: @@ -517,13 +514,13 @@ private: #if ENABLE_MODELVOLUME_TRANSFORM // Constructor, which assigns a new unique ID. - ModelInstance(ModelObject *object) : object(object), print_volume_state(PVS_Inside) {} + explicit ModelInstance(ModelObject *object) : object(object), print_volume_state(PVS_Inside) {} // Constructor, which assigns a new unique ID. - ModelInstance(ModelObject *object, const ModelInstance &other) : + explicit ModelInstance(ModelObject *object, const ModelInstance &other) : m_transformation(other.m_transformation), object(object), print_volume_state(PVS_Inside) {} #else - ModelInstance(ModelObject *object) : m_offset(Vec3d::Zero()), m_rotation(Vec3d::Zero()), m_scaling_factor(Vec3d::Ones()), m_mirror(Vec3d::Ones()), object(object), print_volume_state(PVS_Inside) {} - ModelInstance(ModelObject *object, const ModelInstance &other) : + explicit ModelInstance(ModelObject *object) : m_offset(Vec3d::Zero()), m_rotation(Vec3d::Zero()), m_scaling_factor(Vec3d::Ones()), m_mirror(Vec3d::Ones()), object(object), print_volume_state(PVS_Inside) {} + explicit ModelInstance(ModelObject *object, const ModelInstance &other) : m_offset(other.m_offset), m_rotation(other.m_rotation), m_scaling_factor(other.m_scaling_factor), m_mirror(other.m_mirror), object(object), print_volume_state(PVS_Inside) {} #endif // ENABLE_MODELVOLUME_TRANSFORM diff --git a/src/libslic3r/Print.cpp b/src/libslic3r/Print.cpp index 4052c79ac..1eacb50b1 100644 --- a/src/libslic3r/Print.cpp +++ b/src/libslic3r/Print.cpp @@ -634,7 +634,7 @@ static inline bool model_volume_list_changed(const ModelObject &model_object_old return false; } -static inline void model_volume_list_update_supports(ModelObject &model_object_dst, const ModelObject &model_object_src) +void Print::model_volume_list_update_supports(ModelObject &model_object_dst, const ModelObject &model_object_src) { // 1) Delete the support volumes from model_object_dst. { @@ -650,8 +650,10 @@ static inline void model_volume_list_update_supports(ModelObject &model_object_d } // 2) Copy the support volumes from model_object_src to the end of model_object_dst. for (ModelVolume *vol : model_object_src.volumes) { - if (vol->is_support_modifier()) - model_object_dst.volumes.emplace_back(vol->clone(&model_object_dst)); + if (vol->is_support_modifier()) { + model_object_dst.volumes.emplace_back(new ModelVolume(*vol)); + model_object_dst.volumes.back()->set_model_object(&model_object_dst); + } } } diff --git a/src/libslic3r/Print.hpp b/src/libslic3r/Print.hpp index 3c2a87c74..705c41458 100644 --- a/src/libslic3r/Print.hpp +++ b/src/libslic3r/Print.hpp @@ -564,6 +564,9 @@ private: void _make_wipe_tower(); void _simplify_slices(double distance); + // Declared here to have access to Model / ModelObject / ModelInstance + static void model_volume_list_update_supports(ModelObject &model_object_dst, const ModelObject &model_object_src); + PrintState m_state; // Mutex used for synchronization of the worker thread with the UI thread: // The mutex will be used to guard the worker thread against entering a stage @@ -601,12 +604,6 @@ private: friend class PrintObject; }; - -#define FOREACH_BASE(type, container, iterator) for (type::const_iterator iterator = (container).begin(); iterator != (container).end(); ++iterator) -#define FOREACH_OBJECT(print, object) FOREACH_BASE(PrintObjectPtrs, (print)->m_objects, object) -#define FOREACH_LAYER(object, layer) FOREACH_BASE(LayerPtrs, (object)->m_layers, layer) -#define FOREACH_LAYERREGION(layer, layerm) FOREACH_BASE(LayerRegionPtrs, (layer)->m_regions, layerm) - } #endif diff --git a/src/libslic3r/PrintObject.cpp b/src/libslic3r/PrintObject.cpp index 8f9146766..8ca9c37ae 100644 --- a/src/libslic3r/PrintObject.cpp +++ b/src/libslic3r/PrintObject.cpp @@ -170,8 +170,8 @@ void PrintObject::make_perimeters() // merge slices if they were split into types if (this->typed_slices) { - FOREACH_LAYER(this, layer_it) { - (*layer_it)->merge_slices(); + for (Layer *layer : m_layers) { + layer->merge_slices(); m_print->throw_if_canceled(); } this->typed_slices = false; @@ -1243,9 +1243,10 @@ void PrintObject::bridge_over_infill() *this ); - FOREACH_LAYER(this, layer_it) { + for (LayerPtrs::iterator layer_it = m_layers.begin(); layer_it != m_layers.end(); ++ layer_it) { // skip first layer - if (layer_it == m_layers.begin()) continue; + if (layer_it == m_layers.begin()) + continue; Layer* layer = *layer_it; LayerRegion* layerm = layer->m_regions[region_id]; @@ -1271,8 +1272,8 @@ void PrintObject::bridge_over_infill() // iterate through regions and collect internal surfaces Polygons lower_internal; - FOREACH_LAYERREGION(lower_layer, lower_layerm_it) - (*lower_layerm_it)->fill_surfaces.filter_by_type(stInternal, &lower_internal); + for (LayerRegion *lower_layerm : lower_layer->m_regions) + lower_layerm->fill_surfaces.filter_by_type(stInternal, &lower_internal); // intersect such lower internal surfaces with the candidate solid surfaces to_bridge_pp = intersection(to_bridge_pp, lower_internal); @@ -1734,8 +1735,8 @@ void PrintObject::_make_perimeters() // merge slices if they were split into types if (this->typed_slices) { - FOREACH_LAYER(this, layer_it) - (*layer_it)->merge_slices(); + for (Layer *layer : m_layers) + layer->merge_slices(); this->typed_slices = false; this->invalidate_step(posPrepareInfill); } From 3b72748489b544d95e741f67f971d2fd7c2b8e4d Mon Sep 17 00:00:00 2001 From: bubnikv Date: Fri, 2 Nov 2018 20:45:23 +0100 Subject: [PATCH 5/5] Removed the STDMOVE macro. --- src/libslic3r/Fill/Fill.cpp | 2 +- src/libslic3r/Layer.cpp | 2 +- src/libslic3r/LayerRegion.cpp | 28 ++++++++++++++-------------- src/libslic3r/SupportMaterial.cpp | 10 +++++----- src/libslic3r/libslic3r.h | 13 ------------- src/slic3r/GUI/OptionsGroup.cpp | 20 ++++++++++---------- 6 files changed, 31 insertions(+), 44 deletions(-) diff --git a/src/libslic3r/Fill/Fill.cpp b/src/libslic3r/Fill/Fill.cpp index f1436c931..016d162d8 100644 --- a/src/libslic3r/Fill/Fill.cpp +++ b/src/libslic3r/Fill/Fill.cpp @@ -247,7 +247,7 @@ void make_fill(LayerRegion &layerm, ExtrusionEntityCollection &out) // Only concentric fills are not sorted. eec->no_sort = f->no_sort(); extrusion_entities_append_paths( - eec->entities, STDMOVE(polylines), + eec->entities, std::move(polylines), is_bridge ? erBridgeInfill : (surface.is_solid() ? diff --git a/src/libslic3r/Layer.cpp b/src/libslic3r/Layer.cpp index 5003c67d7..7878bffab 100644 --- a/src/libslic3r/Layer.cpp +++ b/src/libslic3r/Layer.cpp @@ -135,7 +135,7 @@ void Layer::make_perimeters() // Separate the fill surfaces. ExPolygons expp = intersection_ex(to_polygons(fill_surfaces), (*l)->slices); (*l)->fill_expolygons = expp; - (*l)->fill_surfaces.set(STDMOVE(expp), fill_surfaces.surfaces.front()); + (*l)->fill_surfaces.set(std::move(expp), fill_surfaces.surfaces.front()); } } } diff --git a/src/libslic3r/LayerRegion.cpp b/src/libslic3r/LayerRegion.cpp index e0f97703c..6f70fba65 100644 --- a/src/libslic3r/LayerRegion.cpp +++ b/src/libslic3r/LayerRegion.cpp @@ -34,7 +34,7 @@ void LayerRegion::slices_to_fill_surfaces_clipped() // in place. However we're now only using its boundaries (which are invariant) // so we're safe. This guarantees idempotence of prepare_infill() also in case // that combine_infill() turns some fill_surface into VOID surfaces. -// Polygons fill_boundaries = to_polygons(STDMOVE(this->fill_surfaces)); +// Polygons fill_boundaries = to_polygons(std::move(this->fill_surfaces)); Polygons fill_boundaries = to_polygons(this->fill_expolygons); // Collect polygons per surface type. std::vector polygons_by_surface; @@ -133,9 +133,9 @@ void LayerRegion::process_external_surfaces(const Layer* lower_layer) if (internal_surface) // Make a copy as the following line uses the move semantics. internal.push_back(surface); - polygons_append(fill_boundaries, STDMOVE(surface.expolygon)); + polygons_append(fill_boundaries, std::move(surface.expolygon)); } else if (internal_surface) - internal.push_back(STDMOVE(surface)); + internal.push_back(std::move(surface)); } } @@ -192,7 +192,7 @@ void LayerRegion::process_external_surfaces(const Layer* lower_layer) polys = intersection(polys, to_polygons(fill_boundaries_ex[idx_island])); } bridge_bboxes.push_back(get_extents(polys)); - bridges_grown.push_back(STDMOVE(polys)); + bridges_grown.push_back(std::move(polys)); } } @@ -243,7 +243,7 @@ void LayerRegion::process_external_surfaces(const Layer* lower_layer) for (size_t i = 0; i < bridges.size(); ++ i) { if (bridge_group[i] != group_id) continue; - initial.push_back(STDMOVE(bridges[i].expolygon)); + initial.push_back(std::move(bridges[i].expolygon)); polygons_append(grown, bridges_grown[i]); } // detect bridge direction before merging grown surfaces otherwise adjacent bridges @@ -269,7 +269,7 @@ void LayerRegion::process_external_surfaces(const Layer* lower_layer) surfaces_append(bottom, union_ex(grown, true), bridges[idx_last]); } - fill_boundaries = STDMOVE(to_polygons(fill_boundaries_ex)); + fill_boundaries = std::move(to_polygons(fill_boundaries_ex)); BOOST_LOG_TRIVIAL(trace) << "Processing external surface, detecting bridges - done"; } @@ -284,7 +284,7 @@ void LayerRegion::process_external_surfaces(const Layer* lower_layer) Surfaces new_surfaces; { // Merge top and bottom in a single collection. - surfaces_append(top, STDMOVE(bottom)); + surfaces_append(top, std::move(bottom)); // Intersect the grown surfaces with the actual fill boundaries. Polygons bottom_polygons = to_polygons(bottom); for (size_t i = 0; i < top.size(); ++ i) { @@ -292,11 +292,11 @@ void LayerRegion::process_external_surfaces(const Layer* lower_layer) if (s1.empty()) continue; Polygons polys; - polygons_append(polys, STDMOVE(s1)); + polygons_append(polys, std::move(s1)); for (size_t j = i + 1; j < top.size(); ++ j) { Surface &s2 = top[j]; if (! s2.empty() && surfaces_could_merge(s1, s2)) { - polygons_append(polys, STDMOVE(s2)); + polygons_append(polys, std::move(s2)); s2.clear(); } } @@ -306,7 +306,7 @@ void LayerRegion::process_external_surfaces(const Layer* lower_layer) surfaces_append( new_surfaces, // Don't use a safety offset as fill_boundaries were already united using the safety offset. - STDMOVE(intersection_ex(polys, fill_boundaries, false)), + std::move(intersection_ex(polys, fill_boundaries, false)), s1); } } @@ -318,20 +318,20 @@ void LayerRegion::process_external_surfaces(const Layer* lower_layer) if (s1.empty()) continue; Polygons polys; - polygons_append(polys, STDMOVE(s1)); + polygons_append(polys, std::move(s1)); for (size_t j = i + 1; j < internal.size(); ++ j) { Surface &s2 = internal[j]; if (! s2.empty() && surfaces_could_merge(s1, s2)) { - polygons_append(polys, STDMOVE(s2)); + polygons_append(polys, std::move(s2)); s2.clear(); } } ExPolygons new_expolys = diff_ex(polys, new_polygons); polygons_append(new_polygons, to_polygons(new_expolys)); - surfaces_append(new_surfaces, STDMOVE(new_expolys), s1); + surfaces_append(new_surfaces, std::move(new_expolys), s1); } - this->fill_surfaces.surfaces = STDMOVE(new_surfaces); + this->fill_surfaces.surfaces = std::move(new_surfaces); #ifdef SLIC3R_DEBUG_SLICE_PROCESSING export_region_fill_surfaces_to_svg_debug("3_process_external_surfaces-final"); diff --git a/src/libslic3r/SupportMaterial.cpp b/src/libslic3r/SupportMaterial.cpp index bd1a9f3fb..31305f332 100644 --- a/src/libslic3r/SupportMaterial.cpp +++ b/src/libslic3r/SupportMaterial.cpp @@ -2450,7 +2450,7 @@ void LoopInterfaceProcessor::generate(MyLayerExtruded &top_contact_layer, const // Transform loops into ExtrusionPath objects. extrusion_entities_append_paths( top_contact_layer.extrusions, - STDMOVE(loop_lines), + std::move(loop_lines), erSupportMaterialInterface, flow.mm3_per_mm(), flow.width, flow.height); } @@ -2827,7 +2827,7 @@ void PrintObjectSupportMaterial::generate_toolpaths( to_infill = offset_ex(to_infill, float(- 0.4 * flow.scaled_spacing())); extrusion_entities_append_paths( support_layer.support_fills.entities, - to_polylines(STDMOVE(to_infill_polygons)), + to_polylines(std::move(to_infill_polygons)), erSupportMaterial, flow.mm3_per_mm(), flow.width, flow.height); } if (! to_infill.empty()) { @@ -2841,7 +2841,7 @@ void PrintObjectSupportMaterial::generate_toolpaths( // Destination support_layer.support_fills.entities, // Regions to fill - STDMOVE(to_infill), + std::move(to_infill), // Filler and its parameters filler, float(support_density), // Extrusion parameters @@ -3037,14 +3037,14 @@ void PrintObjectSupportMaterial::generate_toolpaths( to_infill = offset_ex(to_infill, - 0.4 * float(flow.scaled_spacing())); extrusion_entities_append_paths( base_layer.extrusions, - to_polylines(STDMOVE(to_infill_polygons)), + to_polylines(std::move(to_infill_polygons)), erSupportMaterial, flow.mm3_per_mm(), flow.width, flow.height); } fill_expolygons_generate_paths( // Destination base_layer.extrusions, // Regions to fill - STDMOVE(to_infill), + std::move(to_infill), // Filler and its parameters filler, density, // Extrusion parameters diff --git a/src/libslic3r/libslic3r.h b/src/libslic3r/libslic3r.h index a2ee3fc9f..f8088faea 100644 --- a/src/libslic3r/libslic3r.h +++ b/src/libslic3r/libslic3r.h @@ -47,19 +47,6 @@ typedef double coordf_t; #define scale_(val) ((val) / SCALING_FACTOR) #define SCALED_EPSILON scale_(EPSILON) -// Which C++ version is supported? -// For example, could optimized functions with move semantics be used? -#if __cplusplus==201402L - #define SLIC3R_CPPVER 14 - #define STDMOVE(WHAT) std::move(WHAT) -#elif __cplusplus==201103L - #define SLIC3R_CPPVER 11 - #define STDMOVE(WHAT) std::move(WHAT) -#else - #define SLIC3R_CPPVER 0 - #define STDMOVE(WHAT) (WHAT) -#endif - #define SLIC3R_DEBUG_OUT_PATH_PREFIX "out/" inline std::string debug_out_path(const char *name, ...) diff --git a/src/slic3r/GUI/OptionsGroup.cpp b/src/slic3r/GUI/OptionsGroup.cpp index 8e810fcda..a61713aac 100644 --- a/src/slic3r/GUI/OptionsGroup.cpp +++ b/src/slic3r/GUI/OptionsGroup.cpp @@ -22,18 +22,18 @@ const t_field& OptionsGroup::build_field(const t_config_option_key& id, const Co // is the normal type. if (opt.gui_type.compare("select") == 0) { } else if (opt.gui_type.compare("select_open") == 0) { - m_fields.emplace(id, STDMOVE(Choice::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(Choice::Create(parent(), opt, id))); } else if (opt.gui_type.compare("color") == 0) { - m_fields.emplace(id, STDMOVE(ColourPicker::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(ColourPicker::Create(parent(), opt, id))); } else if (opt.gui_type.compare("f_enum_open") == 0 || opt.gui_type.compare("i_enum_open") == 0 || opt.gui_type.compare("i_enum_closed") == 0) { - m_fields.emplace(id, STDMOVE(Choice::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(Choice::Create(parent(), opt, id))); } else if (opt.gui_type.compare("slider") == 0) { - m_fields.emplace(id, STDMOVE(SliderCtrl::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(SliderCtrl::Create(parent(), opt, id))); } else if (opt.gui_type.compare("i_spin") == 0) { // Spinctrl } else if (opt.gui_type.compare("legend") == 0) { // StaticText - m_fields.emplace(id, STDMOVE(StaticText::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(StaticText::Create(parent(), opt, id))); } else { switch (opt.type) { case coFloatOrPercent: @@ -43,21 +43,21 @@ const t_field& OptionsGroup::build_field(const t_config_option_key& id, const Co case coPercents: case coString: case coStrings: - m_fields.emplace(id, STDMOVE(TextCtrl::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(TextCtrl::Create(parent(), opt, id))); break; case coBool: case coBools: - m_fields.emplace(id, STDMOVE(CheckBox::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(CheckBox::Create(parent(), opt, id))); break; case coInt: case coInts: - m_fields.emplace(id, STDMOVE(SpinCtrl::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(SpinCtrl::Create(parent(), opt, id))); break; case coEnum: - m_fields.emplace(id, STDMOVE(Choice::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(Choice::Create(parent(), opt, id))); break; case coPoints: - m_fields.emplace(id, STDMOVE(PointCtrl::Create(parent(), opt, id))); + m_fields.emplace(id, std::move(PointCtrl::Create(parent(), opt, id))); break; case coNone: break; default: