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()