From 4298149e49db89d7afd8fd2dbfd0ee6656c5cb38 Mon Sep 17 00:00:00 2001
From: tamasmeszaros <meszaros.q@gmail.com>
Date: Fri, 3 Dec 2021 14:09:54 +0100
Subject: [PATCH] Clarify comments for thread safe queue

Cleanup
---
 src/slic3r/GUI/Jobs/PlaterWorker.hpp    | 24 ++++++------------------
 src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp | 13 ++++++++++---
 2 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/src/slic3r/GUI/Jobs/PlaterWorker.hpp b/src/slic3r/GUI/Jobs/PlaterWorker.hpp
index 186506193f..5735902728 100644
--- a/src/slic3r/GUI/Jobs/PlaterWorker.hpp
+++ b/src/slic3r/GUI/Jobs/PlaterWorker.hpp
@@ -21,19 +21,8 @@ class PlaterWorker: public Worker {
     WorkerSubclass m_w;
     Plater *m_plater;
 
-    struct JobHolder : Job {
-        std::unique_ptr<Job> m_job;
-        void process(Ctl &ctl) override { m_job->process(ctl); };
-        void finalize(bool canceled, std::exception_ptr &e) override
-        {
-            m_job->finalize(canceled, e);
-        }
-        JobHolder(std::unique_ptr<Job> &&j) : m_job{std::move(j)} {}
-    };
-
-    template<class JobSubclass>
     class PlaterJob : public Job {
-        JobSubclass m_job;
+        std::unique_ptr<Job> m_job;
         Plater *m_plater;
 
     public:
@@ -64,12 +53,12 @@ class PlaterWorker: public Worker {
             } wctl{c};
 
             CursorSetterRAII busycursor{wctl};
-            m_job.process(wctl);
+            m_job->process(wctl);
         }
 
         void finalize(bool canceled, std::exception_ptr &eptr) override
         {
-            m_job.finalize(canceled, eptr);
+            m_job->finalize(canceled, eptr);
 
             if (eptr) try {
                 std::rethrow_exception(eptr);
@@ -79,9 +68,8 @@ class PlaterWorker: public Worker {
             }
         }
 
-        template<class...Args>
-        PlaterJob(Plater *p, Args&&...args)
-            : m_job{std::forward<Args>(args)...}, m_plater{p}
+        PlaterJob(Plater *p, std::unique_ptr<Job> j)
+            : m_job{std::move(j)}, m_plater{p}
         {
             // TODO: decide if disabling slice button during UI job is what we
             // want.
@@ -117,7 +105,7 @@ public:
     // Always package the job argument into a PlaterJob
     bool push(std::unique_ptr<Job> job) override
     {
-        return m_w.push(std::make_unique<PlaterJob<JobHolder>>(m_plater, std::move(job)));
+        return m_w.push(std::make_unique<PlaterJob>(m_plater, std::move(job)));
     }
 
     bool is_idle() const override { return m_w.is_idle(); }
diff --git a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp
index 46b4d87b6c..d400490137 100644
--- a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp
+++ b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp
@@ -9,14 +9,20 @@
 
 namespace Slic3r { namespace GUI {
 
+// Helper structure for overloads of ThreadSafeQueueSPSC::consume_one()
+// to block if the queue is empty.
 struct BlockingWait
 {
+    // Timeout to wait for the arrival of new element into the queue.
     unsigned timeout_ms = 0;
+
+    // An optional atomic flag to set true if an incoming element gets
+    // consumed. The flag will be atomically set to true when popping the
+    // front of the queue.
     std::atomic<bool> *pop_flag = nullptr;
 };
 
-// A thread safe queue for one producer and one consumer. Use consume_one_blk
-// to block on an empty queue.
+// A thread safe queue for one producer and one consumer.
 template<class T,
          template<class, class...> class Container = std::deque,
          class... ContainerArgs>
@@ -54,7 +60,8 @@ public:
 
             m_queue.pop();
 
-            if (blkw.pop_flag) // The optional atomic is set before the lock us unlocked
+            if (blkw.pop_flag)
+                // The optional flag is set before the lock us unlocked.
                 blkw.pop_flag->store(true);
         }