#5127: Migrate ModelPopulator to derive from ThreadedResourceTreePopu…

…lator. Move progress message handling to ResourceTreeView class - we need more control about when we need to clear that item.
codereader · Jan 3, 2021 · c316105 · c316105
1 parent a803743
commit c316105
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 148 deletions.
diff --git a/libs/wxutil/dataview/ResourceTreeView.cpp b/libs/wxutil/dataview/ResourceTreeView.cpp
@@ -42,6 +42,7 @@ ResourceTreeView::ResourceTreeView(wxWindow* parent, const TreeModel::Ptr& model
 
     Bind(wxEVT_DATAVIEW_ITEM_CONTEXT_MENU, &ResourceTreeView::_onContextMenu, this);
     Bind(EV_TREEMODEL_POPULATION_FINISHED, &ResourceTreeView::_onTreeStorePopulationFinished, this);
+    Bind(EV_TREEMODEL_POPULATION_PROGRESS, &ResourceTreeView::_onTreeStorePopulationProgress, this);
 }
 
 ResourceTreeView::~ResourceTreeView()
@@ -280,11 +281,31 @@ void ResourceTreeView::AddCustomMenuItem(const ui::IMenuItemPtr& item)
     _customMenuItems.push_back(item);
 }
 
+void ResourceTreeView::_onTreeStorePopulationProgress(TreeModel::PopulationProgressEvent& ev)
+{
+    if (!_progressItem.IsOk())
+    {
+        wxutil::TreeModel::Row row = GetTreeModel()->AddItem();
+
+        row[_columns.iconAndName] = wxVariant(wxDataViewIconText(_("Loading...")));
+        row[_columns.isFolder] = false;
+        row[_columns.isFavourite] = false;
+
+        row.SendItemAdded();
+        _progressItem = row.getItem();
+    }
+
+    wxutil::TreeModel::Row row(_progressItem, *GetModel());
+    row[_columns.iconAndName] = wxVariant(wxDataViewIconText(ev.GetMessage()));
+    row.SendItemChanged();
+}
+
 void ResourceTreeView::_onTreeStorePopulationFinished(TreeModel::PopulationFinishedEvent& ev)
 {
     UnselectAll();
     SetTreeModel(ev.GetTreeModel());
     _populator.reset();
+    _progressItem = wxDataViewItem();
 
     // Trigger a column size event on the first-level row
     TriggerColumnSizeEvent();

diff --git a/libs/wxutil/dataview/ResourceTreeView.h b/libs/wxutil/dataview/ResourceTreeView.h
@@ -64,6 +64,7 @@ class ResourceTreeView :
     TreeModel::Ptr _treeStore;
     TreeModelFilter::Ptr _treeModelFilter;
     wxDataViewItem _emptyFavouritesLabel;
+    wxDataViewItem _progressItem;
 
     // The currently active populator object
     IResourceTreePopulator::Ptr _populator;
@@ -123,6 +124,7 @@ class ResourceTreeView :
 
 private:
     void _onContextMenu(wxDataViewEvent& ev);
+    void _onTreeStorePopulationProgress(TreeModel::PopulationProgressEvent& ev);
     void _onTreeStorePopulationFinished(TreeModel::PopulationFinishedEvent& ev);
 
     bool _testAddToFavourites();

diff --git a/libs/wxutil/dataview/ThreadedResourceTreePopulator.cpp b/libs/wxutil/dataview/ThreadedResourceTreePopulator.cpp
@@ -63,6 +63,11 @@ wxThread::ExitCode ThreadedResourceTreePopulator::Entry()
     return static_cast<ExitCode>(0);
 }
 
+void ThreadedResourceTreePopulator::PostEvent(wxEvent* ev)
+{
+    wxQueueEvent(_finishedHandler, ev);
+}
+
 void ThreadedResourceTreePopulator::SetFinishedHandler(wxEvtHandler* finishedHandler)
 {
     _finishedHandler = finishedHandler;

diff --git a/libs/wxutil/dataview/ThreadedResourceTreePopulator.h b/libs/wxutil/dataview/ThreadedResourceTreePopulator.h
@@ -56,6 +56,9 @@ class ThreadedResourceTreePopulator :
     // to PopulateModel/SortModel as well as the exception handling
     wxThread::ExitCode Entry() override final;
 
+    // Queues an event to the attached finished handler
+    void PostEvent(wxEvent* ev);
+
 public:
     // Construct and initialise variables
     ThreadedResourceTreePopulator(const TreeModel::ColumnRecord& columns);

diff --git a/radiant/ui/modelselector/ModelPopulator.h b/radiant/ui/modelselector/ModelPopulator.h
@@ -1,8 +1,7 @@
 #pragma once
 
 #include "wxutil/dataview/VFSTreePopulator.h"
-#include "wxutil/ModalProgressDialog.h"
-#include "imainframe.h"
+#include "wxutil/dataview/ThreadedResourceTreePopulator.h"
 #include "iregistry.h"
 #include "igame.h"
 #include "EventRateLimiter.h"
@@ -27,17 +26,12 @@ namespace ui
  * to a new TreeModel object. Fires a PopulationFinished event once
  * its work is done.
  */
-class ModelPopulator :
-    public wxThread
+class ModelPopulator final :
+    public wxutil::ThreadedResourceTreePopulator
 {
+private:
     const ModelTreeView::TreeColumns& _columns;
 
-    // The working copy to populate
-    wxutil::TreeModel::Ptr _treeStore;
-
-	// VFSTreePopulator to populate
-	wxutil::VFSTreePopulator _populator;
-
 	// Progress dialog and model count
 	std::size_t _count;
 
@@ -46,92 +40,63 @@ class ModelPopulator :
 
 	std::set<std::string> _allowedExtensions;
 
-    // The event handler to notify on completion
-    wxEvtHandler* _finishedHandler;
-
-    class ThreadAbortedException : public std::exception
-    {};
-
 public:
 
 	// Constructor sets the populator
     ModelPopulator(const ModelTreeView::TreeColumns& columns,
                    wxEvtHandler* finishedHandler) :
-        wxThread(wxTHREAD_JOINABLE),
+        ThreadedResourceTreePopulator(columns),
         _columns(columns),
-        _treeStore(new wxutil::TreeModel(_columns)),
-		_populator(_treeStore),
-		//_progress(_("Loading models")),
 		_count(0),
-		_evLimiter(50),
-        _finishedHandler(finishedHandler)
+        _evLimiter(50)
 	{
-		//_progress.setText(_("Searching"));
-
 		// Load the allowed extensions
 		std::string extensions = GlobalGameManager().currentGame()->getKeyValue("modeltypes");
 		string::split(_allowedExtensions, extensions, " ");
 	}
 
     ~ModelPopulator()
     {
-        // We might have a running thread, wait for it
-        if (IsRunning())
-        {
-            Delete();
-        }
+        EnsureStopped();
     }
 
-    // Thread entry point
-    ExitCode Entry()
+protected:
+    void PopulateModel(const wxutil::TreeModel::Ptr& model) override
     {
-        try
-        {
-            // Search for model files
-            GlobalFileSystem().forEachFile(
-                MODELS_FOLDER, "*",
-                [&](const vfs::FileInfo& fileInfo)
-                {
-                    // Only add visible models
-                    if (fileInfo.visibility == vfs::Visibility::NORMAL)
-                        visitModelFile(fileInfo.name);
-                },
-                0
-            );
+        wxutil::VFSTreePopulator populator(model);
 
-            if (TestDestroy()) return static_cast<wxThread::ExitCode>(0);
-
-            reportProgress(_("Building tree..."));
+        // Search for model files
+        GlobalFileSystem().forEachFile(
+            MODELS_FOLDER, "*",
+            [&](const vfs::FileInfo& fileInfo)
+            {
+                // Only add visible models
+                if (fileInfo.visibility == vfs::Visibility::NORMAL)
+                {
+                    visitModelFile(fileInfo.name, populator);
+                }
+            },
+            0
+        );
 
-            // Fill in the column data (TRUE = including skins)
-            ModelDataInserter inserterSkins(_columns, true);
-            _populator.forEachNode(inserterSkins);
+        ThrowIfCancellationRequested();
 
-            if (TestDestroy()) return static_cast<wxThread::ExitCode>(0);
+        reportProgress(_("Building tree..."));
 
-            // Sort the model before returning it
-            _treeStore->SortModelFoldersFirst(_columns.iconAndName, _columns.isFolder);
+        // Fill in the column data (TRUE = including skins)
+        ModelDataInserter inserterSkins(_columns, true);
+        populator.forEachNode(inserterSkins);
+    }
 
-            if (!TestDestroy())
-            {
-                // Send the event to our listener, only if we are not forced to finish
-                wxQueueEvent(_finishedHandler, new wxutil::TreeModel::PopulationFinishedEvent(_treeStore));
-            }
-
-            return static_cast<wxThread::ExitCode>(0);
-        }
-        catch (const ThreadAbortedException&)
-        {
-            return static_cast<wxThread::ExitCode>(0);
-        }
+    void SortModel(const wxutil::TreeModel::Ptr& model) override
+    {
+        // Sort the model before returning it
+        model->SortModelFoldersFirst(_columns.iconAndName, _columns.isFolder);
     }
 
-    void visitModelFile(const std::string& file)
+    void visitModelFile(const std::string& file, wxutil::VFSTreePopulator& populator)
 	{
-        if (TestDestroy())
-        {
-            throw ThreadAbortedException();
-        }
+        ThrowIfCancellationRequested();
 
 		std::string ext = os::getExtension(file);
 		string::to_lower(ext);
@@ -142,7 +107,7 @@ class ModelPopulator :
 		{
 			_count++;
 
-			_populator.addPath(file);
+			populator.addPath(file);
 
 			if (_evLimiter.readyForEvent())
             {
@@ -153,8 +118,7 @@ class ModelPopulator :
 
     void reportProgress(const std::string& message)
     {
-        wxQueueEvent(_finishedHandler, new wxutil::TreeModel::PopulationProgressEvent(
-            fmt::format(_("{0:d} models loaded"), _count)));
+        PostEvent(new wxutil::TreeModel::PopulationProgressEvent(fmt::format(_("{0:d} models loaded"), _count)));
     }
 };
 

diff --git a/radiant/ui/modelselector/ModelSelector.cpp b/radiant/ui/modelselector/ModelSelector.cpp
@@ -52,7 +52,6 @@ ModelSelector::ModelSelector() :
     _materialsList(nullptr),
 	_lastModel(""),
 	_lastSkin(""),
-	_populated(false),
 	_showOptions(true)
 {
     // Set the default size of the window
@@ -178,28 +177,6 @@ void ModelSelector::onTreeStorePopulationFinished(wxutil::TreeModel::PopulationF
 	findNamedObject<wxButton>(this, "ModelSelectorReloadSkinsButton")->Enable(true);
 }
 
-void ModelSelector::preSelectModel()
-{
-    // If an empty string was passed for the current model, use the last selected one
-    std::string previouslySelected = (!_preselectedModel.empty()) ? _preselectedModel : _lastModel;
-
-    if (!previouslySelected.empty())
-    {
-        wxutil::TreeModel* model = static_cast<wxutil::TreeModel*>(_treeView->GetModel());
-
-        // Lookup the model path in the treemodel
-        wxDataViewItem found = model->FindString(previouslySelected, _columns.fullName);
-
-        if (found.IsOk())
-        {
-            _treeView->Select(found);
-            _treeView->EnsureVisible(found);
-
-            showInfoForSelectedModel();
-        }
-    }
-}
-
 // Show the dialog and enter recursive main loop
 ModelSelectorResult ModelSelector::showAndBlock(const std::string& curModel,
                                                 bool showOptions,
@@ -249,9 +226,6 @@ ModelSelectorResult ModelSelector::chooseModel(const std::string& curModel,
 
 void ModelSelector::onSkinsOrModelsReloaded()
 {
-    // Clear the flag, this triggers a new population next time the dialog is shown
-    _populated = false;
-
     GetUserInterfaceModule().dispatch([this] ()
     {
         populateModels();
@@ -303,9 +277,7 @@ void ModelSelector::setupTreeView(wxWindow* parent)
 
 void ModelSelector::populateModels()
 {
-    // Spawn the population thread
-    _populator.reset(new ModelPopulator(_columns, _treeView));
-    _populator->Run();
+    _treeView->Populate(std::make_shared<ModelPopulator>(_columns, _treeView));
 }
 
 void ModelSelector::Populate()

diff --git a/radiant/ui/modelselector/ModelSelector.h b/radiant/ui/modelselector/ModelSelector.h
@@ -77,13 +77,6 @@ class ModelSelector :
 	std::string _lastModel;
 	std::string _lastSkin;
 
-	// TRUE if the treeview has been populated
-	bool _populated;
-    std::unique_ptr<ModelPopulator> _populator;
-
-    // The model to highlight on show
-    std::string _preselectedModel;
-
     // Whether to show advanced options panel
     bool _showOptions;
 
@@ -108,7 +101,6 @@ class ModelSelector :
 	// Helper functions to configure GUI components
     void setupAdvancedPanel(wxWindow* parent);
     void setupTreeView(wxWindow* parent);
-    void preSelectModel();
 
 	// Populate the tree view with models
 	void populateModels();

diff --git a/radiant/ui/modelselector/ModelTreeView.h b/radiant/ui/modelselector/ModelTreeView.h
@@ -38,8 +38,6 @@ class ModelTreeView :
         _columns(columns),
         _showSkins(true)
     {
-        constexpr const char* MODEL_ICON = "model16green.png";
-
         // Single visible column, containing the directory/shader name and the icon
         AppendIconTextColumn(
             _("Model Path"), _columns.iconAndName.getColumnIndex(),
@@ -50,10 +48,6 @@ class ModelTreeView :
         // Use the TreeModel's full string search function
         AddSearchColumn(_columns.iconAndName);
         EnableFavouriteManagement(decl::Type::Model);
-
-        Bind(wxutil::EV_TREEMODEL_POPULATION_PROGRESS, &ModelTreeView::onTreeStorePopulationProgress, this);
-
-        _modelIcon.CopyFromBitmap(wxArtProvider::GetBitmap(GlobalUIManager().ArtIdPrefix() + MODEL_ICON));
     }
 
     void SetShowSkins(bool showSkins)
@@ -72,22 +66,6 @@ class ModelTreeView :
         return row[_columns.skin];
     }
 
-    void Populate(const wxutil::IResourceTreePopulator::Ptr& populator) override
-    {
-        wxutil::TreeModel::Row row = GetTreeModel()->AddItem();
-
-        row[_columns.iconAndName] = wxVariant(wxDataViewIconText(_("Loading..."), _modelIcon));
-        row[_columns.isSkin] = false;
-        row[_columns.isFolder] = false;
-        row[_columns.isFolder] = std::string();
-        row[_columns.isFavourite] = false;
-
-        row.SendItemAdded();
-        _progressItem = row.getItem();
-
-        ResourceTreeView::Populate(populator);
-    }
-
 protected:
     bool IsTreeModelRowVisible(wxutil::TreeModel::Row& row) override
     {
@@ -99,22 +77,6 @@ class ModelTreeView :
         // Pass to the base class
         return ResourceTreeView::IsTreeModelRowVisible(row);
     }
-
-private:
-    void onTreeStorePopulationProgress(wxutil::TreeModel::PopulationProgressEvent& ev)
-    {
-        if (!_progressItem.IsOk()) return;
-
-#if 0
-        if (_populator && !_populator->IsAlive())
-        {
-            return; // we might be in the process of being destructed
-        }
-#endif
-        wxutil::TreeModel::Row row(_progressItem, *GetModel());
-        row[_columns.iconAndName] = wxVariant(wxDataViewIconText(ev.GetMessage(), _modelIcon));
-        row.SendItemChanged();
-    }
 };
 
 }