diff --git a/src/libslic3r/MutablePriorityQueue.hpp b/src/libslic3r/MutablePriorityQueue.hpp index 81f3cb782..b45b8cfff 100644 --- a/src/libslic3r/MutablePriorityQueue.hpp +++ b/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]; } diff --git a/tests/libslic3r/test_mutable_priority_queue.cpp b/tests/libslic3r/test_mutable_priority_queue.cpp index 5ea3db276..626b0388d 100644 --- a/tests/libslic3r/test_mutable_priority_queue.cpp +++ b/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 idxs(count, {0}); auto q = make_miniheap_mutable_priority_queue( - [](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) - CHECK(QueueType::address::is_padding(0)); - CHECK(!QueueType::address::is_padding(1)); + THEN("Element at the top of the queue has a valid ID.") { + CHECK(v.id >= 0); + 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); - // is first item in queue valid value - CHECK(q.top().id == 1); + THEN("Element removed from the queue has its position in idxs reset to invalid.") { + CHECK(idxs[v.id] == q.invalid_id()); + } + 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++) { - MyValue mv; - mv.id = ord++; - mv.val = rand_val(); - q.push(mv); - } + for (size_t id = 0; id < count; ++ id) + q.push({ id, rand_val() }); + auto check = [&]()->bool{ for (size_t i = 0; i < idxs.size(); ++i) { - if (dels[i]) continue; - size_t qid = idxs[i]; - if (qid > 3*count) { - return false; - } - MyValue &mv = q[qid]; - if (mv.id != i) { - return false; // ERROR + if (dels[i]) { + if (idxs[i] != q.invalid_id()) + return false; // ERROR + } else { + size_t qid = idxs[i]; + if (qid >= q.heap_size()) { + 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(); - q.push(it); - dels[it.id] = false; + v.val = rand_val(); + q.push(v); + 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];