From 440df505b47086e30272547b429b2bd75db99334 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Wed, 11 Jan 2023 18:24:44 +0100 Subject: [PATCH] SLA backend thread-safety improvements - Put AnyPtr into separate header, it deserves one - Add means to handle shared pointers inside AnyPtr - Use shared pointers in sla csg collection for meshes not owned by Model - Add method to get the last completed step in PrintObjectBase - Make SLAPrintObject::get_parts_to_slice() safe to call from UI thread against background thread activity. --- src/libslic3r/AnyPtr.hpp | 130 +++++++++++++++++++++++ src/libslic3r/CMakeLists.txt | 1 + src/libslic3r/CSGMesh/CSGMesh.hpp | 2 +- src/libslic3r/CSGMesh/CSGMeshCopy.hpp | 15 ++- src/libslic3r/CSGMesh/ModelToCSGMesh.hpp | 8 +- src/libslic3r/MTUtils.hpp | 83 --------------- src/libslic3r/PrintBase.hpp | 15 +++ src/libslic3r/SLAPrint.cpp | 37 ++++++- src/libslic3r/SLAPrint.hpp | 20 ++-- src/libslic3r/SLAPrintSteps.cpp | 14 ++- src/slic3r/GUI/Gizmos/GLGizmosCommon.cpp | 2 +- 11 files changed, 218 insertions(+), 109 deletions(-) create mode 100644 src/libslic3r/AnyPtr.hpp diff --git a/src/libslic3r/AnyPtr.hpp b/src/libslic3r/AnyPtr.hpp new file mode 100644 index 0000000000..c40d10093e --- /dev/null +++ b/src/libslic3r/AnyPtr.hpp @@ -0,0 +1,130 @@ +#ifndef ANYPTR_HPP +#define ANYPTR_HPP + +#include +#include +#include + +namespace Slic3r { + +// A general purpose pointer holder that can hold any type of smart pointer +// or raw pointer which can own or not own any object they point to. +// In case a raw pointer is stored, it is not destructed so ownership is +// assumed to be foreign. +// +// The stored pointer is not checked for being null when dereferenced. +// +// This is a movable only object due to the fact that it can possibly hold +// a unique_ptr which a non-copy. +template +class AnyPtr { + enum { RawPtr, UPtr, ShPtr, WkPtr }; + + boost::variant, std::shared_ptr, std::weak_ptr> ptr; + + template static T *get_ptr(Self &&s) + { + switch (s.ptr.which()) { + case RawPtr: return boost::get(s.ptr); + case UPtr: return boost::get>(s.ptr).get(); + case ShPtr: return boost::get>(s.ptr).get(); + case WkPtr: { + auto shptr = boost::get>(s.ptr).lock(); + return shptr.get(); + } + } + + return nullptr; + } + +public: + template>> + AnyPtr(TT *p = nullptr) : ptr{p} + {} + template>> + AnyPtr(std::unique_ptr p) : ptr{std::unique_ptr(std::move(p))} + {} + template>> + AnyPtr(std::shared_ptr p) : ptr{std::shared_ptr(std::move(p))} + {} + template>> + AnyPtr(std::weak_ptr p) : ptr{std::weak_ptr(std::move(p))} + {} + + ~AnyPtr() = default; + + AnyPtr(AnyPtr &&other) noexcept : ptr{std::move(other.ptr)} {} + AnyPtr(const AnyPtr &other) = delete; + + AnyPtr &operator=(AnyPtr &&other) noexcept { ptr = std::move(other.ptr); return *this; } + AnyPtr &operator=(const AnyPtr &other) = delete; + + template>> + AnyPtr &operator=(TT *p) { ptr = p; return *this; } + + template>> + AnyPtr &operator=(std::unique_ptr p) { ptr = std::move(p); return *this; } + + template>> + AnyPtr &operator=(std::shared_ptr p) { ptr = p; return *this; } + + template>> + AnyPtr &operator=(std::weak_ptr p) { ptr = std::move(p); return *this; } + + const T &operator*() const { return *get_ptr(*this); } + T &operator*() { return *get_ptr(*this); } + + T *operator->() { return get_ptr(*this); } + const T *operator->() const { return get_ptr(*this); } + + T *get() { return get_ptr(*this); } + const T *get() const { return get_ptr(*this); } + + operator bool() const + { + switch (ptr.which()) { + case RawPtr: return bool(boost::get(ptr)); + case UPtr: return bool(boost::get>(ptr)); + case ShPtr: return bool(boost::get>(ptr)); + case WkPtr: { + auto shptr = boost::get>(ptr).lock(); + return bool(shptr); + } + } + + return false; + } + + // If the stored pointer is a shared or weak pointer, returns a reference + // counted copy. Empty shared pointer is returned otherwise. + std::shared_ptr get_shared_cpy() const + { + std::shared_ptr ret; + + switch (ptr.which()) { + case ShPtr: ret = boost::get>(ptr); break; + case WkPtr: ret = boost::get>(ptr).lock(); break; + default: + ; + } + + return ret; + } + + // If the underlying pointer is unique, convert to shared pointer + void convert_unique_to_shared() + { + if (ptr.which() == UPtr) + ptr = std::shared_ptr{std::move(boost::get>(ptr))}; + } + + // Returns true if the data is owned by this AnyPtr instance + bool is_owned() const noexcept + { + return ptr.which() == UPtr || ptr.which() == ShPtr; + } +}; + +} // namespace Slic3r + +#endif // ANYPTR_HPP diff --git a/src/libslic3r/CMakeLists.txt b/src/libslic3r/CMakeLists.txt index da561d411a..e9aeb68e27 100644 --- a/src/libslic3r/CMakeLists.txt +++ b/src/libslic3r/CMakeLists.txt @@ -22,6 +22,7 @@ set(SLIC3R_SOURCES AABBTreeLines.hpp AABBMesh.hpp AABBMesh.cpp + AnyPtr.hpp BoundingBox.cpp BoundingBox.hpp BridgeDetector.cpp diff --git a/src/libslic3r/CSGMesh/CSGMesh.hpp b/src/libslic3r/CSGMesh/CSGMesh.hpp index ef555a38b3..d14ed76595 100644 --- a/src/libslic3r/CSGMesh/CSGMesh.hpp +++ b/src/libslic3r/CSGMesh/CSGMesh.hpp @@ -1,7 +1,7 @@ #ifndef CSGMESH_HPP #define CSGMESH_HPP -#include // for AnyPtr +#include #include namespace Slic3r { namespace csg { diff --git a/src/libslic3r/CSGMesh/CSGMeshCopy.hpp b/src/libslic3r/CSGMesh/CSGMeshCopy.hpp index b296ecbfa7..78800f9bbb 100644 --- a/src/libslic3r/CSGMesh/CSGMeshCopy.hpp +++ b/src/libslic3r/CSGMesh/CSGMeshCopy.hpp @@ -5,17 +5,28 @@ namespace Slic3r { namespace csg { -// Copy a csg range but for the meshes, only copy the pointers. +// Copy a csg range but for the meshes, only copy the pointers. If the copy +// is made from a CSGPart compatible object, and the pointer is a shared one, +// it will be copied with reference counting. template void copy_csgrange_shallow(const Range &csgrange, OutIt out) { for (const auto &part : csgrange) { - CSGPart cpy{AnyPtr{get_mesh(part)}, + CSGPart cpy{{}, get_operation(part), get_transform(part)}; cpy.stack_operation = get_stack_operation(part); + if constexpr (std::is_convertible_v) { + if (auto shptr = part.its_ptr.get_shared_cpy()) { + cpy.its_ptr = shptr; + } + } + + if (!cpy.its_ptr) + cpy.its_ptr = AnyPtr{get_mesh(part)}; + *out = std::move(cpy); ++out; } diff --git a/src/libslic3r/CSGMesh/ModelToCSGMesh.hpp b/src/libslic3r/CSGMesh/ModelToCSGMesh.hpp index ecf9d8168b..9e413594ed 100644 --- a/src/libslic3r/CSGMesh/ModelToCSGMesh.hpp +++ b/src/libslic3r/CSGMesh/ModelToCSGMesh.hpp @@ -12,10 +12,10 @@ namespace Slic3r { namespace csg { // Flags to select which parts to export from Model into a csg part collection. // These flags can be chained with the | operator enum ModelParts { - mpartsPositive = 1, // Include positive parts - mpartsNegative = 2, // Include negative parts - mpartsDrillHoles = 4, // Include drill holes - mpartsDoSplits = 8 // Split each splitable mesh and export as a union of csg parts + mpartsPositive = 1, // Include positive parts + mpartsNegative = 2, // Include negative parts + mpartsDrillHoles = 4, // Include drill holes + mpartsDoSplits = 8, // Split each splitable mesh and export as a union of csg parts }; template diff --git a/src/libslic3r/MTUtils.hpp b/src/libslic3r/MTUtils.hpp index 74c44aa5e5..ab99ea5f68 100644 --- a/src/libslic3r/MTUtils.hpp +++ b/src/libslic3r/MTUtils.hpp @@ -8,7 +8,6 @@ #include #include #include -#include #include "libslic3r.h" @@ -137,88 +136,6 @@ inline std::vector> grid(const T &start, return vals; } -// A general purpose pointer holder that can hold any type of smart pointer -// or raw pointer which can own or not own any object they point to. -// In case a raw pointer is stored, it is not destructed so ownership is -// assumed to be foreign. -// -// The stored pointer is not checked for being null when dereferenced. -// -// This is a movable only object due to the fact that it can possibly hold -// a unique_ptr which a non-copy. -template -class AnyPtr { - enum { RawPtr, UPtr, ShPtr, WkPtr }; - - boost::variant, std::shared_ptr, std::weak_ptr> ptr; - - template static T *get_ptr(Self &&s) - { - switch (s.ptr.which()) { - case RawPtr: return boost::get(s.ptr); - case UPtr: return boost::get>(s.ptr).get(); - case ShPtr: return boost::get>(s.ptr).get(); - case WkPtr: { - auto shptr = boost::get>(s.ptr).lock(); - return shptr.get(); - } - } - - return nullptr; - } - -public: - template>> - AnyPtr(TT *p = nullptr) : ptr{p} - {} - template>> - AnyPtr(std::unique_ptr p) : ptr{std::unique_ptr(std::move(p))} - {} - template>> - AnyPtr(std::shared_ptr p) : ptr{std::shared_ptr(std::move(p))} - {} - template>> - AnyPtr(std::weak_ptr p) : ptr{std::weak_ptr(std::move(p))} - {} - - ~AnyPtr() = default; - - AnyPtr(AnyPtr &&other) noexcept : ptr{std::move(other.ptr)} {} - AnyPtr(const AnyPtr &other) = delete; - - AnyPtr &operator=(AnyPtr &&other) noexcept { ptr = std::move(other.ptr); return *this; } - AnyPtr &operator=(const AnyPtr &other) = delete; - - AnyPtr &operator=(T *p) { ptr = p; return *this; } - AnyPtr &operator=(std::unique_ptr p) { ptr = std::move(p); return *this; } - AnyPtr &operator=(std::shared_ptr p) { ptr = p; return *this; } - AnyPtr &operator=(std::weak_ptr p) { ptr = std::move(p); return *this; } - - const T &operator*() const { return *get_ptr(*this); } - T &operator*() { return *get_ptr(*this); } - - T *operator->() { return get_ptr(*this); } - const T *operator->() const { return get_ptr(*this); } - - T *get() { return get_ptr(*this); } - const T *get() const { return get_ptr(*this); } - - operator bool() const - { - switch (ptr.which()) { - case RawPtr: return bool(boost::get(ptr)); - case UPtr: return bool(boost::get>(ptr)); - case ShPtr: return bool(boost::get>(ptr)); - case WkPtr: { - auto shptr = boost::get>(ptr).lock(); - return bool(shptr); - } - } - - return false; - } -}; - } // namespace Slic3r #endif // MTUTILS_HPP diff --git a/src/libslic3r/PrintBase.hpp b/src/libslic3r/PrintBase.hpp index 13796abbad..08268c04b9 100644 --- a/src/libslic3r/PrintBase.hpp +++ b/src/libslic3r/PrintBase.hpp @@ -690,6 +690,21 @@ public: PrintStateBase::StateWithTimeStamp step_state_with_timestamp(PrintObjectStepEnum step) const { return m_state.state_with_timestamp(step, PrintObjectBase::state_mutex(m_print)); } PrintStateBase::StateWithWarnings step_state_with_warnings(PrintObjectStepEnum step) const { return m_state.state_with_warnings(step, PrintObjectBase::state_mutex(m_print)); } + auto last_completed_step() const + { + static_assert(COUNT > 0, "Step count should be > 0"); + auto s = int(COUNT) - 1; + + std::lock_guard lk(state_mutex(m_print)); + while (s >= 0 && ! is_step_done_unguarded(PrintObjectStepEnum(s))) + --s; + + if (s < 0) + s = COUNT; + + return PrintObjectStepEnum(s); + } + protected: PrintObjectBaseWithState(PrintType *print, ModelObject *model_object) : PrintObjectBase(model_object), m_print(print) {} diff --git a/src/libslic3r/SLAPrint.cpp b/src/libslic3r/SLAPrint.cpp index 0e1f8b4308..79cc2605ff 100644 --- a/src/libslic3r/SLAPrint.cpp +++ b/src/libslic3r/SLAPrint.cpp @@ -1,5 +1,6 @@ #include "SLAPrint.hpp" #include "SLAPrintSteps.hpp" +#include "CSGMesh/CSGMeshCopy.hpp" #include "CSGMesh/PerformCSGMeshBooleans.hpp" #include "Geometry.hpp" @@ -1017,12 +1018,16 @@ const TriangleMesh &SLAPrintObject::get_mesh_to_print() const { const TriangleMesh *ret = nullptr; - int s = SLAPrintObjectStep::slaposCount; + int s = last_completed_step(); - while (s > 0 && !ret) { - --s; - if (is_step_done(SLAPrintObjectStep(s)) && !m_preview_meshes[s].empty()) + if (s == slaposCount) + ret = &EMPTY_MESH; + + while (s >= 0 && !ret) { + if (!m_preview_meshes[s].empty()) ret = &m_preview_meshes[s]; + + --s; } if (!ret) @@ -1031,6 +1036,30 @@ const TriangleMesh &SLAPrintObject::get_mesh_to_print() const return *ret; } +std::vector SLAPrintObject::get_parts_to_slice() const +{ + return get_parts_to_slice(slaposCount); +} + +std::vector +SLAPrintObject::get_parts_to_slice(SLAPrintObjectStep untilstep) const +{ + auto laststep = last_completed_step(); + SLAPrintObjectStep s = std::min(untilstep, laststep); + + if (s == slaposCount) + return {}; + + std::vector ret; + + for (int step = 0; step < s; ++step) { + auto r = m_mesh_to_slice.equal_range(SLAPrintObjectStep(step)); + csg::copy_csgrange_shallow(Range{r.first, r.second}, std::back_inserter(ret)); + } + + return ret; +} + sla::SupportPoints SLAPrintObject::transformed_support_points() const { assert(m_model_object != nullptr); diff --git a/src/libslic3r/SLAPrint.hpp b/src/libslic3r/SLAPrint.hpp index 0cf796d07c..e6a4286eaa 100644 --- a/src/libslic3r/SLAPrint.hpp +++ b/src/libslic3r/SLAPrint.hpp @@ -123,16 +123,9 @@ public: // like hollowing and drilled holes. const TriangleMesh & get_mesh_to_print() const; - const Range get_parts_to_slice() const - { - return range(m_mesh_to_slice); - } + std::vector get_parts_to_slice() const; - const Range get_parts_to_slice(SLAPrintObjectStep step) const - { - auto r = m_mesh_to_slice.equal_range(step); - return {r.first, r.second}; - } + std::vector get_parts_to_slice(SLAPrintObjectStep step) const; sla::SupportPoints transformed_support_points() const; sla::DrainHoles transformed_drainhole_points() const; @@ -373,6 +366,15 @@ private: // Holds CSG operations for the printed object, prioritized by print steps. CSGContainer m_mesh_to_slice; + auto mesh_to_slice(SLAPrintObjectStep s) const + { + auto r = m_mesh_to_slice.equal_range(s); + + return Range{r.first, r.second}; + } + + auto mesh_to_slice() const { return range(m_mesh_to_slice); } + // Holds the preview of the object to be printed (as it will look like with // all its holes and cavities, negatives and positive volumes unified. // Essentially this should be a m_mesh_to_slice after the CSG operations diff --git a/src/libslic3r/SLAPrintSteps.cpp b/src/libslic3r/SLAPrintSteps.cpp index f0e2e5db08..f3824f7515 100644 --- a/src/libslic3r/SLAPrintSteps.cpp +++ b/src/libslic3r/SLAPrintSteps.cpp @@ -212,7 +212,7 @@ void SLAPrint::Steps::generate_preview(SLAPrintObject &po, SLAPrintObjectStep st // If that fails for any of the drillholes, the voxelization fallback is // used. - bool is_pure_model = is_all_positive(po.get_parts_to_slice(slaposAssembly)); + bool is_pure_model = is_all_positive(po.mesh_to_slice(slaposAssembly)); bool can_hollow = po.m_hollowing_data && po.m_hollowing_data->interior && !sla::get_mesh(*po.m_hollowing_data->interior).empty(); @@ -317,7 +317,11 @@ struct csg_inserter { SLAPrintObjectStep key; csg_inserter &operator*() { return *this; } - void operator=(csg::CSGPart &&part) { m.emplace(key, std::move(part)); } + void operator=(csg::CSGPart &&part) + { + part.its_ptr.convert_unique_to_shared(); + m.emplace(key, std::move(part)); + } csg_inserter& operator++() { return *this; } }; @@ -356,7 +360,7 @@ void SLAPrint::Steps::hollow_model(SLAPrintObject &po) ctl.cancelfn = [this]() { throw_if_canceled(); }; sla::InteriorPtr interior = - generate_interior(range(po.m_mesh_to_slice), hlwcfg, ctl); + generate_interior(po.mesh_to_slice(), hlwcfg, ctl); if (!interior || sla::get_mesh(*interior).empty()) BOOST_LOG_TRIVIAL(warning) << "Hollowed interior is empty!"; @@ -378,7 +382,7 @@ void SLAPrint::Steps::hollow_model(SLAPrintObject &po) // Put the interior into the target mesh as a negative po.m_mesh_to_slice .emplace(slaposHollowing, - csg::CSGPart{std::make_unique(std::move(m)), csg::CSGType::Difference}); + csg::CSGPart{std::make_shared(std::move(m)), csg::CSGType::Difference}); generate_preview(po, slaposHollowing); } @@ -518,7 +522,7 @@ void SLAPrint::Steps::slice_model(SLAPrintObject &po) auto thr = [this]() { m_print->throw_if_canceled(); }; auto &slice_grid = po.m_model_height_levels; - po.m_model_slices = slice_csgmesh_ex(range(po.m_mesh_to_slice), slice_grid, params, thr); + po.m_model_slices = slice_csgmesh_ex(po.mesh_to_slice(), slice_grid, params, thr); auto mit = slindex_it; for (size_t id = 0; diff --git a/src/slic3r/GUI/Gizmos/GLGizmosCommon.cpp b/src/slic3r/GUI/Gizmos/GLGizmosCommon.cpp index 705a13b902..62f7eb5fe3 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmosCommon.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmosCommon.cpp @@ -324,7 +324,7 @@ void ObjectClipper::on_update() auto partstoslice = po->get_parts_to_slice(); if (! partstoslice.empty()) { mc = std::make_unique(); - mc->set_mesh(partstoslice); + mc->set_mesh(range(partstoslice)); mc_tr = Geometry::Transformation{po->trafo().inverse().cast()}; } }