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

@@ -0,0 +1,130 @@
+#ifndef ANYPTR_HPP
+#define ANYPTR_HPP
+
+#include <memory>
+#include <type_traits>
+#include <boost/variant.hpp>
+
+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 T>
+class AnyPtr {
+    enum { RawPtr, UPtr, ShPtr, WkPtr };
+
+    boost::variant<T*, std::unique_ptr<T>, std::shared_ptr<T>, std::weak_ptr<T>> ptr;
+
+    template<class Self> static T *get_ptr(Self &&s)
+    {
+        switch (s.ptr.which()) {
+        case RawPtr: return boost::get<T *>(s.ptr);
+        case UPtr: return boost::get<std::unique_ptr<T>>(s.ptr).get();
+        case ShPtr: return boost::get<std::shared_ptr<T>>(s.ptr).get();
+        case WkPtr: {
+            auto shptr = boost::get<std::weak_ptr<T>>(s.ptr).lock();
+            return shptr.get();
+        }
+        }
+
+        return nullptr;
+    }
+
+public:
+    template<class TT = T, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
+    AnyPtr(TT *p = nullptr) : ptr{p}
+    {}
+    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
+    AnyPtr(std::unique_ptr<TT> p) : ptr{std::unique_ptr<T>(std::move(p))}
+    {}
+    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
+    AnyPtr(std::shared_ptr<TT> p) : ptr{std::shared_ptr<T>(std::move(p))}
+    {}
+    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
+    AnyPtr(std::weak_ptr<TT> p) : ptr{std::weak_ptr<T>(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<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
+    AnyPtr &operator=(TT *p) { ptr = p; return *this; }
+
+    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
+    AnyPtr &operator=(std::unique_ptr<TT> p) { ptr = std::move(p); return *this; }
+
+    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
+    AnyPtr &operator=(std::shared_ptr<TT> p) { ptr = p; return *this; }
+
+    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
+    AnyPtr &operator=(std::weak_ptr<TT> 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<T *>(ptr));
+        case UPtr: return bool(boost::get<std::unique_ptr<T>>(ptr));
+        case ShPtr: return bool(boost::get<std::shared_ptr<T>>(ptr));
+        case WkPtr: {
+            auto shptr = boost::get<std::weak_ptr<T>>(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<T> get_shared_cpy() const
+    {
+        std::shared_ptr<T> ret;
+
+        switch (ptr.which()) {
+        case ShPtr: ret = boost::get<std::shared_ptr<T>>(ptr); break;
+        case WkPtr: ret = boost::get<std::weak_ptr<T>>(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<T>{std::move(boost::get<std::unique_ptr<T>>(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

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

src/libslic3r/CSGMesh/CSGMesh.hpp

@@ -1,7 +1,7 @@
 #ifndef CSGMESH_HPP
 #define CSGMESH_HPP
 
-#include <libslic3r/MTUtils.hpp> // for AnyPtr
+#include <libslic3r/AnyPtr.hpp>
 #include <admesh/stl.h>
 
 namespace Slic3r { namespace csg {

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<class It, class OutIt>
 void copy_csgrange_shallow(const Range<It> &csgrange, OutIt out)
 {
     for (const auto &part : csgrange) {
-        CSGPart cpy{AnyPtr<const indexed_triangle_set>{get_mesh(part)},
+        CSGPart cpy{{},
                     get_operation(part),
                     get_transform(part)};
 
         cpy.stack_operation = get_stack_operation(part);
 
+        if constexpr (std::is_convertible_v<decltype(part), const CSGPart&>) {
+            if (auto shptr = part.its_ptr.get_shared_cpy()) {
+                cpy.its_ptr = shptr;
+            }
+        }
+
+        if (!cpy.its_ptr)
+            cpy.its_ptr = AnyPtr<const indexed_triangle_set>{get_mesh(part)};
+
         *out = std::move(cpy);
         ++out;
     }

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<class OutIt>

src/libslic3r/MTUtils.hpp

@@ -8,7 +8,6 @@
 #include <vector>
 #include <algorithm>
 #include <cmath>
-#include <boost/variant.hpp>
 
 #include "libslic3r.h"
 
@@ -137,88 +136,6 @@ inline std::vector<ArithmeticOnly<T>> 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 T>
-class AnyPtr {
-    enum { RawPtr, UPtr, ShPtr, WkPtr };
-
-    boost::variant<T*, std::unique_ptr<T>, std::shared_ptr<T>, std::weak_ptr<T>> ptr;
-
-    template<class Self> static T *get_ptr(Self &&s)
-    {
-        switch (s.ptr.which()) {
-        case RawPtr: return boost::get<T *>(s.ptr);
-        case UPtr: return boost::get<std::unique_ptr<T>>(s.ptr).get();
-        case ShPtr: return boost::get<std::shared_ptr<T>>(s.ptr).get();
-        case WkPtr: {
-            auto shptr = boost::get<std::weak_ptr<T>>(s.ptr).lock();
-            return shptr.get();
-        }
-        }
-
-        return nullptr;
-    }
-
-public:
-    template<class TT = T, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
-    AnyPtr(TT *p = nullptr) : ptr{p}
-    {}
-    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
-    AnyPtr(std::unique_ptr<TT> p) : ptr{std::unique_ptr<T>(std::move(p))}
-    {}
-    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
-    AnyPtr(std::shared_ptr<TT> p) : ptr{std::shared_ptr<T>(std::move(p))}
-    {}
-    template<class TT, class = std::enable_if_t<std::is_convertible_v<TT, T>>>
-    AnyPtr(std::weak_ptr<TT> p) : ptr{std::weak_ptr<T>(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<T> p) { ptr = std::move(p); return *this; }
-    AnyPtr &operator=(std::shared_ptr<T> p) { ptr = p; return *this; }
-    AnyPtr &operator=(std::weak_ptr<T> 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<T *>(ptr));
-        case UPtr: return bool(boost::get<std::unique_ptr<T>>(ptr));
-        case ShPtr: return bool(boost::get<std::shared_ptr<T>>(ptr));
-        case WkPtr: {
-            auto shptr = boost::get<std::weak_ptr<T>>(ptr).lock();
-            return bool(shptr);
-        }
-        }
-
-        return false;
-    }
-};
-
 } // namespace Slic3r
 
 #endif // MTUTILS_HPP

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) {}
 

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<csg::CSGPart> SLAPrintObject::get_parts_to_slice() const
+{
+    return get_parts_to_slice(slaposCount);
+}
+
+std::vector<csg::CSGPart>
+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<csg::CSGPart> 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);

src/libslic3r/SLAPrint.hpp

@@ -123,16 +123,9 @@ public:
     // like hollowing and drilled holes.
     const TriangleMesh & get_mesh_to_print() const;
 
-    const Range<CSGContainer::const_iterator> get_parts_to_slice() const
-    {
-        return range(m_mesh_to_slice);
-    }
+    std::vector<csg::CSGPart> get_parts_to_slice() const;
 
-    const Range<CSGContainer::const_iterator> get_parts_to_slice(SLAPrintObjectStep step) const
-    {
-        auto r = m_mesh_to_slice.equal_range(step);
-        return {r.first, r.second};
-    }
+    std::vector<csg::CSGPart> 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

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<indexed_triangle_set>(std::move(m)), csg::CSGType::Difference});
+                     csg::CSGPart{std::make_shared<indexed_triangle_set>(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;

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<MeshClipper>();
-            mc->set_mesh(partstoslice);
+            mc->set_mesh(range(partstoslice));
             mc_tr = Geometry::Transformation{po->trafo().inverse().cast<double>()};
         }
     }