From 270c076e77663895e69d975e276e3edf6643cc1a Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Mon, 30 Aug 2021 17:23:44 +0200 Subject: [PATCH] Fixed undo/redo issue when clear method of FacetsAnnotation and ModelConfig reset timestamp to 1. This led to a bug where e.g. deleting painted facets through the respective item in object list followed by possible other actions and undo restored the painted facets from the time when the project was loaded. I'm not sure if there was any other situation where this problem manifested. --- src/libslic3r/Model.cpp | 10 +++++----- src/libslic3r/Model.hpp | 5 ++++- src/libslic3r/ObjectID.hpp | 3 --- src/libslic3r/PrintConfig.hpp | 6 ++++-- src/slic3r/GUI/GUI_ObjectList.cpp | 12 ++++++------ src/slic3r/GUI/Plater.cpp | 6 +++--- 6 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/libslic3r/Model.cpp b/src/libslic3r/Model.cpp index 6654d3a13..6f9e4fd4c 100644 --- a/src/libslic3r/Model.cpp +++ b/src/libslic3r/Model.cpp @@ -1198,9 +1198,9 @@ ModelObjectPtrs ModelObject::cut(size_t instance, coordf_t z, ModelObjectCutAttr for (ModelVolume *volume : volumes) { const auto volume_matrix = volume->get_matrix(); - volume->supported_facets.clear(); - volume->seam_facets.clear(); - volume->mmu_segmentation_facets.clear(); + volume->supported_facets.reset(); + volume->seam_facets.reset(); + volume->mmu_segmentation_facets.reset(); if (! volume->is_model_part()) { // Modifiers are not cut, but we still need to add the instance transformation @@ -2019,11 +2019,11 @@ bool FacetsAnnotation::set(const TriangleSelector& selector) return false; } -void FacetsAnnotation::clear() +void FacetsAnnotation::reset() { m_data.first.clear(); m_data.second.clear(); - this->reset_timestamp(); + this->touch(); } // Following function takes data from a triangle and encodes it as string diff --git a/src/libslic3r/Model.hpp b/src/libslic3r/Model.hpp index b89dd5aa1..ec6fac821 100644 --- a/src/libslic3r/Model.hpp +++ b/src/libslic3r/Model.hpp @@ -541,7 +541,10 @@ public: indexed_triangle_set get_facets_strict(const ModelVolume& mv, EnforcerBlockerType type) const; bool has_facets(const ModelVolume& mv, EnforcerBlockerType type) const; bool empty() const { return m_data.first.empty(); } - void clear(); + + // Following method clears the config and increases its timestamp, so the deleted + // state is considered changed from perspective of the undo/redo stack. + void reset(); // Serialize triangle into string, for serialization into 3MF/AMF. std::string get_triangle_as_string(int i) const; diff --git a/src/libslic3r/ObjectID.hpp b/src/libslic3r/ObjectID.hpp index ea7c748a5..1030171e7 100644 --- a/src/libslic3r/ObjectID.hpp +++ b/src/libslic3r/ObjectID.hpp @@ -105,9 +105,6 @@ protected: // The class tree will have virtual tables and type information. virtual ~ObjectWithTimestamp() = default; - // Resetting timestamp to 1 indicates the object is in its initial (cleared) state. - // To be called by the derived class's clear() method. - void reset_timestamp() { m_timestamp = 1; } // The timestamp uniquely identifies content of the derived class' data, therefore it makes sense to copy the timestamp if the content data was copied. void copy_timestamp(const ObjectWithTimestamp& rhs) { m_timestamp = rhs.m_timestamp; } diff --git a/src/libslic3r/PrintConfig.hpp b/src/libslic3r/PrintConfig.hpp index b15651ba7..64c8445f8 100644 --- a/src/libslic3r/PrintConfig.hpp +++ b/src/libslic3r/PrintConfig.hpp @@ -1064,7 +1064,9 @@ Points get_bed_shape(const SLAPrinterConfig &cfg); class ModelConfig { public: - void clear() { m_data.clear(); m_timestamp = 1; } + // Following method clears the config and increases its timestamp, so the deleted + // state is considered changed from perspective of the undo/redo stack. + void reset() { m_data.clear(); touch(); } void assign_config(const ModelConfig &rhs) { if (m_timestamp != rhs.m_timestamp) { @@ -1076,7 +1078,7 @@ public: if (m_timestamp != rhs.m_timestamp) { m_data = std::move(rhs.m_data); m_timestamp = rhs.m_timestamp; - rhs.clear(); + rhs.reset(); } } diff --git a/src/slic3r/GUI/GUI_ObjectList.cpp b/src/slic3r/GUI/GUI_ObjectList.cpp index b85219c0b..6be9a7acc 100644 --- a/src/slic3r/GUI/GUI_ObjectList.cpp +++ b/src/slic3r/GUI/GUI_ObjectList.cpp @@ -1803,21 +1803,21 @@ void ObjectList::del_info_item(const int obj_idx, InfoItemType type) cnv->get_gizmos_manager().reset_all_states(); Plater::TakeSnapshot(plater, _L("Remove paint-on supports")); for (ModelVolume* mv : (*m_objects)[obj_idx]->volumes) - mv->supported_facets.clear(); + mv->supported_facets.reset(); break; case InfoItemType::CustomSeam: cnv->get_gizmos_manager().reset_all_states(); Plater::TakeSnapshot(plater, _L("Remove paint-on seam")); for (ModelVolume* mv : (*m_objects)[obj_idx]->volumes) - mv->seam_facets.clear(); + mv->seam_facets.reset(); break; case InfoItemType::MmuSegmentation: cnv->get_gizmos_manager().reset_all_states(); Plater::TakeSnapshot(plater, _L("Remove Multi Material painting")); for (ModelVolume* mv : (*m_objects)[obj_idx]->volumes) - mv->mmu_segmentation_facets.clear(); + mv->mmu_segmentation_facets.reset(); break; case InfoItemType::Sinking: @@ -1856,7 +1856,7 @@ void ObjectList::del_settings_from_config(const wxDataViewItem& parent_item) if (is_layer_settings) layer_height = m_config->opt_float("layer_height"); - m_config->clear(); + m_config->reset(); if (extruder >= 0) m_config->set_key_value("extruder", new ConfigOptionInt(extruder)); @@ -1932,7 +1932,7 @@ bool ObjectList::del_subobject_from_object(const int obj_idx, const int idx, con const auto last_volume = object->volumes[0]; if (!last_volume->config.empty()) { object->config.apply(last_volume->config); - last_volume->config.clear(); + last_volume->config.reset(); // update extruder color in ObjectList wxDataViewItem obj_item = m_objects_model->GetItemById(obj_idx); @@ -3769,7 +3769,7 @@ void ObjectList::last_volume_is_deleted(const int obj_idx) auto volume = (*m_objects)[obj_idx]->volumes.front(); // clear volume's config values - volume->config.clear(); + volume->config.reset(); // set a default extruder value, since user can't add it manually volume->config.set_key_value("extruder", new ConfigOptionInt(0)); diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 4e9adab62..e8ce90516 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -6217,9 +6217,9 @@ void Plater::clear_before_change_mesh(int obj_idx) bool paint_removed = false; for (ModelVolume* mv : mo->volumes) { paint_removed |= ! mv->supported_facets.empty() || ! mv->seam_facets.empty() || ! mv->mmu_segmentation_facets.empty(); - mv->supported_facets.clear(); - mv->seam_facets.clear(); - mv->mmu_segmentation_facets.clear(); + mv->supported_facets.reset(); + mv->seam_facets.reset(); + mv->mmu_segmentation_facets.reset(); } if (paint_removed) { // snapshot_time is captured by copy so the lambda knows where to undo/redo to.