Refactor: Added LoopCallback for making deferred calls in main thread

skyjake · Dec 29, 2015 · abd61c8 · abd61c8
1 parent 9ae0d10
commit abd61c8
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 54 deletions.
diff --git a/doomsday/apps/client/src/network/serverlink.cpp b/doomsday/apps/client/src/network/serverlink.cpp
@@ -13,7 +13,7 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
  * Public License for more details. You should have received a copy of the GNU
  * General Public License along with this program; if not, see:
- * http://www.gnu.org/licenses</small> 
+ * http://www.gnu.org/licenses</small>
  */
 
 #include "de_platform.h"
@@ -49,26 +49,21 @@ enum LinkState
 };
 
 DENG2_PIMPL(ServerLink)
-, DENG2_OBSERVES(Loop, Iteration)
 {
     shell::ServerFinder finder; ///< Finding local servers.
     LinkState state;
     bool fetching;
     typedef QMap<Address, serverinfo_t> Servers;
     Servers discovered;
     Servers fromMaster;
+    LoopCallback mainCall;
 
     Instance(Public *i)
         : Base(i)
         , state(None)
         , fetching(false)
     {}
 
-    ~Instance()
-    {
-        Loop::get().audienceForIteration() -= this;
-    }
-
     void notifyDiscoveryUpdate()
     {
         DENG2_FOR_PUBLIC_AUDIENCE(DiscoveryUpdate, i) i->linkDiscoveryUpdate(self);
@@ -183,17 +178,16 @@ DENG2_PIMPL(ServerLink)
         fetching = true;
         N_MAPost(MAC_REQUEST);
         N_MAPost(MAC_WAIT);
-        Loop::get().audienceForIteration() += this;
+        mainCall.enqueue([this] () { checkMasterReply(); });
     }
 
-    void loopIteration()
+    void checkMasterReply()
     {
         DENG2_ASSERT(fetching);
 
         if(N_MADone())
         {
             fetching = false;
-            Loop::get().audienceForIteration() -= this;
 
             fromMaster.clear();
             int const count = N_MasterGet(0, 0);
@@ -206,6 +200,11 @@ DENG2_PIMPL(ServerLink)
 
             notifyDiscoveryUpdate();
         }
+        else
+        {
+            // Check again later.
+            mainCall.enqueue([this] () { checkMasterReply(); });
+        }
     }
 
     Servers allFound(FoundMask const &mask) const

diff --git a/doomsday/apps/client/src/resource/resourcesystem.cpp b/doomsday/apps/client/src/resource/resourcesystem.cpp
@@ -2196,6 +2196,8 @@ DENG2_PIMPL(ResourceSystem)
 
     void loopIteration()
     {
+        /// @todo Refactor: TaskPool has a signal (or audience) when all tasks are complete.
+        /// No need to check on every loop iteration.
         if(convertSavegameTasks.isDone())
         {
             LOG_AS("ResourceSystem");

diff --git a/doomsday/apps/client/src/ui/widgets/savedsessionmenuwidget.cpp b/doomsday/apps/client/src/ui/widgets/savedsessionmenuwidget.cpp
@@ -38,7 +38,6 @@ using de::game::SavedSession;
 DENG_GUI_PIMPL(SavedSessionMenuWidget)
 , DENG2_OBSERVES(Games,               Readiness)
 , DENG2_OBSERVES(Session::SavedIndex, AvailabilityUpdate)
-, DENG2_OBSERVES(Loop,                Iteration) // deferred refresh
 {
     /**
      * Action for loading a saved session.
@@ -170,6 +169,8 @@ DENG_GUI_PIMPL(SavedSessionMenuWidget)
         }
     };
 
+    LoopCallback mainCall;
+
     Instance(Public *i) : Base(i)
     {
         App_Games().audienceForReadiness() += this;
@@ -178,7 +179,6 @@ DENG_GUI_PIMPL(SavedSessionMenuWidget)
 
     ~Instance()
     {
-        Loop::get().audienceForIteration() -= this;
         App_Games().audienceForReadiness() -= this;
         game::Session::savedIndex().audienceForAvailabilityUpdate() -= this;
     }
@@ -277,24 +277,7 @@ DENG_GUI_PIMPL(SavedSessionMenuWidget)
 
     void savedIndexAvailabilityUpdate(Session::SavedIndex const &)
     {
-        if(!App::inMainThread())
-        {
-            // We'll have to defer the update for now.
-            deferUpdate();
-            return;
-        }
-        updateItemsFromSavedIndex();
-    }
-
-    void deferUpdate()
-    {
-        Loop::get().audienceForIteration() += this;
-    }
-
-    void loopIteration()
-    {
-        Loop::get().audienceForIteration() -= this;
-        updateItemsFromSavedIndex();
+        mainCall.enqueue([this] () { updateItemsFromSavedIndex(); });
     }
 };
 

diff --git a/doomsday/apps/client/src/ui/widgets/singleplayersessionmenuwidget.cpp b/doomsday/apps/client/src/ui/widgets/singleplayersessionmenuwidget.cpp
@@ -30,7 +30,6 @@ using namespace de;
 DENG_GUI_PIMPL(SingleplayerSessionMenuWidget)
 , DENG2_OBSERVES(Games, Addition)
 , DENG2_OBSERVES(Games, Readiness)
-, DENG2_OBSERVES(Loop, Iteration) // deferred updates
 , DENG2_OBSERVES(App, GameChange)
 {
     /// ActionItem with a Game member, for loading a particular game.
@@ -66,6 +65,7 @@ DENG_GUI_PIMPL(SingleplayerSessionMenuWidget)
 
     Mode mode;
     FIFO<Game> pendingGames;
+    LoopCallback mainCall;
 
     Instance(Public *i) : Base(i)
     {
@@ -76,8 +76,6 @@ DENG_GUI_PIMPL(SingleplayerSessionMenuWidget)
 
     ~Instance()
     {
-        Loop::get().audienceForIteration() -= this;
-
         App_Games().audienceForAddition() -= this;
         App_Games().audienceForReadiness() -= this;
         App::app().audienceForGameChange() -= this;
@@ -89,7 +87,10 @@ DENG_GUI_PIMPL(SingleplayerSessionMenuWidget)
         pendingGames.put(&game);
 
         // Update from main thread later.
-        Loop::get().audienceForIteration() += this;
+        mainCall.enqueue([this] () {
+            addPendingGames();
+            updateGameAvailability();
+        });
     }
 
     void addExistingGames()
@@ -107,13 +108,6 @@ DENG_GUI_PIMPL(SingleplayerSessionMenuWidget)
                 (mode == ShowGamesWithMissingResources && !isReady));
     }
 
-    void loopIteration()
-    {
-        Loop::get().audienceForIteration() -= this;
-        addPendingGames();
-        updateGameAvailability();
-    }
-
     void addPendingGames()
     {
         if(pendingGames.isEmpty()) return;
@@ -178,7 +172,7 @@ DENG_GUI_PIMPL(SingleplayerSessionMenuWidget)
 
     void currentGameChanged(game::Game const &)
     {
-        Loop::get().audienceForIteration() += this;
+        mainCall.enqueue([this] () { updateGameAvailability(); });
     }
 };
 

diff --git a/doomsday/sdk/libcore/include/de/core/loop.h b/doomsday/sdk/libcore/include/de/core/loop.h
@@ -13,16 +13,19 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
  * General Public License for more details. You should have received a copy of
  * the GNU Lesser General Public License along with this program; if not, see:
- * http://www.gnu.org/licenses</small> 
+ * http://www.gnu.org/licenses</small>
  */
 
 #ifndef LIBDENG2_LOOP_H
 #define LIBDENG2_LOOP_H
 
 #include <QObject>
+#include <QList>
 #include <de/Observers>
 #include <de/Time>
 
+#include <functional>
+
 namespace de {
 
 /**
@@ -88,6 +91,24 @@ public slots:
     DENG2_PRIVATE(d)
 };
 
+/**
+ * Utility for deferring callbacks via the Loop.
+ */
+class DENG2_PUBLIC LoopCallback : public Lockable, DENG2_OBSERVES(Loop, Iteration)
+{
+public:
+    typedef std::function<void ()> Callback;
+
+    LoopCallback();
+    ~LoopCallback();
+
+    void enqueue(Callback func);
+    void loopIteration();
+
+private:
+    QList<Callback> _funcs;
+};
+
 } // namespace de
 
 #endif // LIBDENG2_LOOP_H
diff --git a/doomsday/sdk/libcore/src/core/loop.cpp b/doomsday/sdk/libcore/src/core/loop.cpp
@@ -13,7 +13,7 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
  * General Public License for more details. You should have received a copy of
  * the GNU Lesser General Public License along with this program; if not, see:
- * http://www.gnu.org/licenses</small> 
+ * http://www.gnu.org/licenses</small>
  */
 
 #include "de/Loop"
@@ -119,4 +119,35 @@ void Loop::nextLoopIteration()
     }
 }
 
+LoopCallback::LoopCallback()
+{}
+
+LoopCallback::~LoopCallback()
+{
+    Loop::get().audienceForIteration() -= this;
+}
+
+void LoopCallback::enqueue(Callback func)
+{
+    DENG2_GUARD(this);
+
+    _funcs << func;
+    Loop::get().audienceForIteration() += this;
+}
+
+void LoopCallback::loopIteration()
+{
+    DENG2_GUARD(this);
+
+    Loop::get().audienceForIteration() -= this;
+
+    // Make a copy of the list if new callbacks get enqueued in the callback.
+    QList<Callback> const funcs = _funcs;
+    _funcs.clear();
+    for(Callback const &cb : funcs)
+    {
+        cb();
+    }
+}
+
 } // namespace de
diff --git a/doomsday/sdk/libcore/src/data/bank.cpp b/doomsday/sdk/libcore/src/data/bank.cpp
@@ -111,8 +111,7 @@ class Cache : public Lockable
 
 } // namespace internal
 
-DENG2_PIMPL(Bank),
-DENG2_OBSERVES(Loop, Iteration) // notifications from other threads sent via main Loop
+DENG2_PIMPL(Bank)
 {
     /**
      * Data item. Has ownership of the in-memory cached data and the source
@@ -535,6 +534,7 @@ DENG2_OBSERVES(Loop, Iteration) // notifications from other threads sent via mai
     DataTree items;
     TaskPool jobs;
     NotifyQueue notifications;
+    LoopCallback mainCall;
 
     Instance(Public *i, char const *name, Flags const &flg)
         : Base(i)
@@ -550,7 +550,6 @@ DENG2_OBSERVES(Loop, Iteration) // notifications from other threads sent via mai
 
     ~Instance()
     {
-        Loop::get().audienceForIteration() -= this;
         destroySerialCache();
     }
 
@@ -674,16 +673,10 @@ DENG2_OBSERVES(Loop, Iteration) // notifications from other threads sent via mai
         notifications.put(new Notification(notif));
         if(isThreaded())
         {
-            Loop::get().audienceForIteration() += this;
+            mainCall.enqueue([this] () { performNotifications(); });
         }
     }
 
-    void loopIteration()
-    {
-        Loop::get().audienceForIteration() -= this;
-        performNotifications();
-    }
-
     void performNotifications()
     {
         forever