From 056c46d01fd9a1d33d6aa9fa81a5d0e6175ca679 Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Wed, 23 Sep 2020 12:18:39 +0200 Subject: [PATCH 1/3] Undo/Redo serialization extension: If an object indicates a valid timestamp, then the timestamp is relied upon to not serialize the object data if the timestamp of the same object on the undo/redo stack matches. --- src/libslic3r/ObjectID.hpp | 9 +++++-- src/slic3r/Utils/UndoRedo.cpp | 48 ++++++++++++++++++++++++++++++----- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/libslic3r/ObjectID.hpp b/src/libslic3r/ObjectID.hpp index 920f512de..fd5cc80ae 100644 --- a/src/libslic3r/ObjectID.hpp +++ b/src/libslic3r/ObjectID.hpp @@ -49,7 +49,12 @@ private: class ObjectBase { public: - ObjectID id() const { return m_id; } + ObjectID id() const { return m_id; } + // Return an optional timestamp of this object. + // If the timestamp returned is non-zero, then the serialization framework will + // only save this object on the Undo/Redo stack if the timestamp is different + // from the timestmap of the object at the top of the Undo / Redo stack. + virtual uint64_t timestamp() const { return 0; } protected: // Constructors to be only called by derived classes. @@ -59,7 +64,7 @@ protected: // by an existing ID copied from elsewhere. ObjectBase(int) : m_id(ObjectID(0)) {} // The class tree will have virtual tables and type information. - virtual ~ObjectBase() {} + virtual ~ObjectBase() = default; // Use with caution! void set_new_unique_id() { m_id = generate_new_id(); } diff --git a/src/slic3r/Utils/UndoRedo.cpp b/src/slic3r/Utils/UndoRedo.cpp index 10b8062f7..79cf3e82e 100644 --- a/src/slic3r/Utils/UndoRedo.cpp +++ b/src/slic3r/Utils/UndoRedo.cpp @@ -307,7 +307,11 @@ private: size_t size; char data[1]; + // The serialized data matches the data stored here. bool matches(const std::string& rhs) { return this->size == rhs.size() && memcmp(this->data, rhs.data(), this->size) == 0; } + + // The timestamp matches the timestamp serialized in the data stored here. + bool matches_timestamp(uint64_t timestamp) { assert(timestamp > 0); assert(this->size > 8); return memcmp(this->data, ×tamp, 8) == 0; } }; Interval m_interval; @@ -350,7 +354,8 @@ public: size_t size() const { return m_data->size; } size_t refcnt() const { return m_data->refcnt; } bool matches(const std::string& data) { return m_data->matches(data); } - size_t memsize() const { + bool matches_timestamp(uint64_t timestamp) { return m_data->matches_timestamp(timestamp); } + size_t memsize() const { return m_data->refcnt == 1 ? // Count just the size of the snapshot data. m_data->size : @@ -398,6 +403,27 @@ public: return memsize; } + // If an object provides a reliable timestamp and the object serializes the timestamp first, + // then we may just check the validity of the timestamp against the last snapshot without + // having to serialize the whole object. This reduces the amount of serialization and memcmp + // when taking a snapshot. + bool try_save_timestamp(size_t active_snapshot_time, size_t current_time, uint64_t timestamp) { + assert(m_history.empty() || m_history.back().end() <= active_snapshot_time); + if (! m_history.empty() && m_history.back().matches_timestamp(timestamp)) { + if (m_history.back().end() < active_snapshot_time) + // Share the previous data by reference counting. + m_history.emplace_back(Interval(current_time, current_time + 1), m_history.back()); + else { + assert(m_history.back().end() == active_snapshot_time); + // Just extend the last interval using the old data. + m_history.back().extend_end(current_time + 1); + } + return true; + } + // The timestamp is not valid, the caller has to call this->save() with the serialized data. + return false; + } + void save(size_t active_snapshot_time, size_t current_time, const std::string &data) { assert(m_history.empty() || m_history.back().end() <= active_snapshot_time); if (m_history.empty() || m_history.back().end() < active_snapshot_time) { @@ -749,13 +775,23 @@ template ObjectID StackImpl::save_mutable_object(const T &object) if (it_object_history == m_objects.end()) it_object_history = m_objects.insert(it_object_history, std::make_pair(object.id(), std::unique_ptr>(new MutableObjectHistory()))); auto *object_history = static_cast*>(it_object_history->second.get()); - // Then serialize the object into a string. - std::ostringstream oss; + bool needs_to_save = true; { - Slic3r::UndoRedo::OutputArchive archive(*this, oss); - archive(object); + // If the timestamp returned is non zero, then it is considered reliable. + // The caller is supposed to serialize the timestamp first. + uint64_t timestamp = object.timestamp(); + if (timestamp > 0) + needs_to_save = ! object_history->try_save_timestamp(m_active_snapshot_time, m_current_time, timestamp); + } + if (needs_to_save) { + // Serialize the object into a string. + std::ostringstream oss; + { + Slic3r::UndoRedo::OutputArchive archive(*this, oss); + archive(object); + } + object_history->save(m_active_snapshot_time, m_current_time, oss.str()); } - object_history->save(m_active_snapshot_time, m_current_time, oss.str()); return object.id(); } From 0dad7adfa1ecd42a429a97a583cf908078386372 Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Wed, 23 Sep 2020 12:58:58 +0200 Subject: [PATCH 2/3] "There is an object with no extrusions on the first layer." should throw SlicingError, not RuntimeError. --- src/libslic3r/GCode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index 431ad3830..bed8b1dba 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -629,7 +629,7 @@ std::vector GCode::collect_layers_to_print(const PrintObjec // Check that there are extrusions on the very first layer. if (layers_to_print.size() == 1u) { if (!has_extrusions) - throw Slic3r::RuntimeError(_(L("There is an object with no extrusions on the first layer."))); + throw Slic3r::SlicingError(_(L("There is an object with no extrusions on the first layer."))); } // In case there are extrusions on this layer, check there is a layer to lay it on. From dde64d361b58247516acc8f69f33dea33466edec Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Wed, 23 Sep 2020 12:59:15 +0200 Subject: [PATCH 3/3] Tiny polishing and documentation. --- src/libslic3r/Model.hpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libslic3r/Model.hpp b/src/libslic3r/Model.hpp index b9045e28b..c6b7c6030 100644 --- a/src/libslic3r/Model.hpp +++ b/src/libslic3r/Model.hpp @@ -448,7 +448,11 @@ public: Vec3d mesh_offset{ Vec3d::Zero() }; Geometry::Transformation transform; - template void serialize(Archive& ar) { ar(input_file, object_idx, volume_idx, mesh_offset, transform); } + template void serialize(Archive& ar) { + //FIXME Vojtech: Serialize / deserialize only if the Source is set. + // likely testing input_file or object_idx would be sufficient. + ar(input_file, object_idx, volume_idx, mesh_offset, transform); + } }; Source source; @@ -467,7 +471,7 @@ public: FacetsAnnotation m_supported_facets; // List of seam enforcers/blockers. - FacetsAnnotation m_seam_facets; + FacetsAnnotation m_seam_facets; // A parent object owning this modifier volume. ModelObject* get_object() const { return this->object; }