Follow-up to dc3931ec1f:

Fix mutable priority queue being wiped when moving out of function
    Without move constructor, the clean() gets called when returning an instance from a function.
The above fix was applied also to MutableSkipHeapPriorityQueue.

Follow-up to 15a082b80b:
Fixed TEST_CASE("Mutable priority queue - first pop", "[MutableSkipHeapPriorityQueue]")

src/libslic3r/MutablePriorityQueue.hpp

@@ -24,7 +24,7 @@ public:
 	MutablePriorityQueue& operator=(MutablePriorityQueue &&) = default;
 
 	// This class modifies the outside data through the m_index_setter
-	// and thus it should not be copied. The semantics are similar to std::unique_ptr
+	// and thus it should not be copied. The semantics is similar to std::unique_ptr
 	MutablePriorityQueue(const MutablePriorityQueue &) = delete;
 	MutablePriorityQueue& operator=(const MutablePriorityQueue &) = delete;
 
@@ -267,6 +267,14 @@ public:
 		{}
 	~MutableSkipHeapPriorityQueue()	{ clear(); }
 
+	MutableSkipHeapPriorityQueue(MutableSkipHeapPriorityQueue &&) = default;
+    MutableSkipHeapPriorityQueue &operator=(MutableSkipHeapPriorityQueue &&) = default;
+
+    // This class modifies the outside data through the m_index_setter
+    // and thus it should not be copied. The semantics is similar to std::unique_ptr
+    MutableSkipHeapPriorityQueue(const MutableSkipHeapPriorityQueue &) = delete;
+    MutableSkipHeapPriorityQueue &operator=(const MutableSkipHeapPriorityQueue &) = delete;
+
 	void		clear();
 	// Reserve one unused element per miniheap.
 	void		reserve(size_t cnt) 				{ m_heap.reserve(cnt + ((cnt + (address::block_size - 1)) / (address::block_size - 1))); }
@@ -278,6 +286,8 @@ public:
 	void		update(size_t idx) 					{ assert(! address::is_padding(idx)); T item = m_heap[idx]; remove(idx); push(item); }
 	// There is one padding element storead at each miniheap, thus lower the number of elements by the number of miniheaps.
 	size_t 		size() const noexcept 				{ return m_heap.size() - (m_heap.size() + address::block_size - 1) / address::block_size; }
+	// Number of heap elements including padding. heap_size() >= size().
+	size_t      heap_size() const noexcept          { return m_heap.size(); }
 	bool		empty() const						{ return m_heap.empty(); }
 	T&			operator[](std::size_t idx) noexcept { assert(! address::is_padding(idx)); return m_heap[idx]; }
 	const T&    operator[](std::size_t idx) const noexcept { assert(! address::is_padding(idx)); return m_heap[idx]; }

tests/libslic3r/test_mutable_priority_queue.cpp

@@ -347,21 +347,35 @@ TEST_CASE("Mutable priority queue - first pop", "[MutableSkipHeapPriorityQueue]"
         float val;
     };
     static constexpr const size_t count = 50000;
+    std::vector<size_t> idxs(count, {0});
     auto q = make_miniheap_mutable_priority_queue<MyValue, 16, true>(
-        [](MyValue &v, size_t idx) { v.id = idx; },
+        [&idxs](MyValue &v, size_t idx) { idxs[v.id] = idx; },
         [](MyValue &l, MyValue &r) { return l.val < r.val; });
     using QueueType = decltype(q);
+    THEN("Skip queue has 0th element unused, 1st element is the top of the queue.") {
+        CHECK(QueueType::address::is_padding(0));
+        CHECK(!QueueType::address::is_padding(1));
+    }
     q.reserve(count);
     for (size_t id = 0; id < count; ++ id)
         q.push({ id, rand() / 100.f });
     MyValue v = q.top(); // copy
-    // is valid id (no initial value default value)
+    THEN("Element at the top of the queue has a valid ID.") {
-    CHECK(QueueType::address::is_padding(0));
+        CHECK(v.id >= 0);
-    CHECK(!QueueType::address::is_padding(1));
+        CHECK(v.id < count);
+    }
+    THEN("Element at the top of the queue has its position stored in idxs.") {
+        CHECK(idxs[v.id] == 1);
+    }
     q.pop();
-    CHECK(v.id == 1);
+    THEN("Element removed from the queue has its position in idxs reset to invalid.") {
-    // is first item in queue valid value
+        CHECK(idxs[v.id] == q.invalid_id());
-    CHECK(q.top().id == 1);
+    }
+    THEN("Element was removed from the queue, new top of the queue has its index set correctly.") {
+        CHECK(q.top().id >= 0);
+        CHECK(q.top().id < count);
+        CHECK(idxs[q.top().id] == 1);
+    }
 }
 
 TEST_CASE("Mutable priority queue complex", "[MutableSkipHeapPriorityQueue]")
@@ -379,23 +393,23 @@ TEST_CASE("Mutable priority queue complex", "[MutableSkipHeapPriorityQueue]")
     q.reserve(count);
 
     auto rand_val = [&]()->float { return (rand() % 53) / 10.f; };
-    size_t ord = 0;
+    for (size_t id = 0; id < count; ++ id)
-    for (size_t id = 0; id < count; id++) {
+        q.push({ id, rand_val() });
-        MyValue mv;
+
-        mv.id = ord++;
-        mv.val = rand_val();
-        q.push(mv);
-    }
     auto check = [&]()->bool{
         for (size_t i = 0; i < idxs.size(); ++i) {
-            if (dels[i]) continue;
+            if (dels[i]) {
-            size_t   qid = idxs[i];
+                if (idxs[i] != q.invalid_id())
-            if (qid > 3*count) { 
+                    return false; // ERROR 
-                return false;
+            } else {
-            }
+                size_t   qid = idxs[i];
-            MyValue &mv  = q[qid]; 
+                if (qid >= q.heap_size()) { 
-            if (mv.id != i) { 
+                    return false; // ERROR 
-                return false; // ERROR 
+                }
+                MyValue &mv  = q[qid]; 
+                if (mv.id != i) { 
+                    return false; // ERROR 
+                }
             }
         }
         return true;
@@ -403,6 +417,7 @@ TEST_CASE("Mutable priority queue complex", "[MutableSkipHeapPriorityQueue]")
 
     CHECK(check()); // initial check
 
+    // Generate an element ID of an elmenet, which was not yet deleted, thus it is still valid.
     auto get_valid_id = [&]()->int { 
         int id = 0;
         do {
@@ -410,23 +425,28 @@ TEST_CASE("Mutable priority queue complex", "[MutableSkipHeapPriorityQueue]")
         } while (dels[id]);
         return id;
     };
+
+    // Remove first 100 elements from the queue of 5000 elements, cross-validate indices.
+    // Re-enter every 20th element back to the queue.
     for (size_t i = 0; i < 100; i++) {
-        MyValue it = q.top(); // copy
+        MyValue v = q.top(); // copy
         q.pop();
-        dels[it.id] = true;
+        dels[v.id] = true;
         CHECK(check());
         if (i % 20 == 0) {
-            it.val = rand_val();
+            v.val = rand_val();
-            q.push(it);
+            q.push(v);
-            dels[it.id] = false;
+            dels[v.id] = false;
             CHECK(check());
             continue;
         }
-
+        // Remove some valid element from the queue.
         int id = get_valid_id();
+        CHECK(idxs[id] != q.invalid_id());
         q.remove(idxs[id]);
         dels[id] = true;
         CHECK(check());
+        // and change 5 random elements and reorder them in the queue.
         for (size_t j = 0; j < 5; j++) { 
             int id = get_valid_id();
             size_t   qid = idxs[id];