use QtConcurrent to cleanup threading of bakers

libraries/model-baking/CMakeLists.txt

@@ -1,6 +1,6 @@
 set(TARGET_NAME model-baking)
 
-setup_hifi_library()
+setup_hifi_library(Concurrent)
 
 link_hifi_libraries(networking)
 

libraries/model-baking/src/FBXBaker.cpp

@@ -11,8 +11,12 @@
 
 #include <fbxsdk.h>
 
+#include <QtConcurrent>
+#include <QtCore/QCoreApplication>
 #include <QtCore/QDir>
+#include <QtCore/QEventLoop>
 #include <QtCore/QFileInfo>
+#include <QtCore/QThread>
 
 #include <NetworkAccessManager.h>
 
@@ -46,14 +50,71 @@ QString FBXBaker::pathToCopyOfOriginal() const {
     return _uniqueOutputPath + ORIGINAL_OUTPUT_SUBFOLDER + _fbxURL.fileName();
 }
 
-void FBXBaker::start() {
+void FBXBaker::bake() {
     qCDebug(model_baking) << "Baking" << _fbxURL;
 
     // setup the output folder for the results of this bake
-    if (!setupOutputFolder()) {
+    setupOutputFolder();
+
+    // make a local copy of the FBX file
+    loadSourceFBX();
+
+    // load the scene from the FBX file
+    importScene();
+
+    // enumerate the textures found in the scene and start a bake for them
+    rewriteAndBakeSceneTextures();
+
+    // export the FBX with re-written texture references
+    exportScene();
+
+    // remove the embedded media folder that the FBX SDK produces when reading the original
+    removeEmbeddedMediaFolder();
+
+    // cleanup the originals if we weren't asked to keep them around
+    possiblyCleanupOriginals();
+
+    // if the texture baking operations are not complete
+    // use a QEventLoop to process events until texture bake operations are complete
+    if (!_unbakedTextures.isEmpty()) {
+        QEventLoop eventLoop;
+        connect(this, &FBXBaker::allTexturesBaked, &eventLoop, &QEventLoop::quit);
+        eventLoop.exec();
+    }
+
+    emit finished();
+}
+
+void FBXBaker::setupOutputFolder() {
+    // construct the output path using the name of the fbx and the base output path
+    _uniqueOutputPath =  _baseOutputPath + "/" + _fbxName + "/";
+
+    // make sure there isn't already an output directory using the same name
+    int iteration = 0;
+
+    while (QDir(_uniqueOutputPath).exists()) {
+        _uniqueOutputPath = _baseOutputPath + "/" + _fbxName + "-" + QString::number(++iteration) + "/";
+    }
+
+    qCDebug(model_baking) << "Creating FBX output folder" << _uniqueOutputPath;
+
+    // attempt to make the output folder
+    if (!QDir().mkdir(_uniqueOutputPath)) {
+        qCCritical(model_baking) << "Failed to create FBX output folder" << _uniqueOutputPath;
+
         return;
     }
 
+    // make the baked and original sub-folders used during export
+    QDir uniqueOutputDir = _uniqueOutputPath;
+    if (!uniqueOutputDir.mkdir(BAKED_OUTPUT_SUBFOLDER) || !uniqueOutputDir.mkdir(ORIGINAL_OUTPUT_SUBFOLDER)) {
+        qCCritical(model_baking) << "Failed to create baked/original subfolders in" << _uniqueOutputPath;
+
+        return;
+    }
+}
+
+void FBXBaker::loadSourceFBX() {
     // check if the FBX is local or first needs to be downloaded
     if (_fbxURL.isLocalFile()) {
         // load up the local file
@@ -77,48 +138,18 @@ void FBXBaker::start() {
         networkRequest.setUrl(_fbxURL);
 
         qCDebug(model_baking) << "Downloading" << _fbxURL;
-
         auto networkReply = networkAccessManager.get(networkRequest);
-        connect(networkReply, &QNetworkReply::finished, this, &FBXBaker::handleFBXNetworkReply);
+
+        // start a QEventLoop so we process events while waiting for the request to complete
+        QEventLoop eventLoop;
+        connect(networkReply, &QNetworkReply::finished, &eventLoop, &QEventLoop::quit);
+        eventLoop.exec();
+
+        handleFBXNetworkReply(networkReply);
     }
 }
 
-bool FBXBaker::setupOutputFolder() {
+void FBXBaker::handleFBXNetworkReply(QNetworkReply* requestReply) {
-    // construct the output path using the name of the fbx and the base output path
-    _uniqueOutputPath =  _baseOutputPath + "/" + _fbxName + "/";
-
-    // make sure there isn't already an output directory using the same name
-    int iteration = 0;
-
-    while (QDir(_uniqueOutputPath).exists()) {
-        _uniqueOutputPath = _baseOutputPath + "/" + _fbxName + "-" + QString::number(++iteration) + "/";
-    }
-
-    qCDebug(model_baking) << "Creating FBX output folder" << _uniqueOutputPath;
-
-    // attempt to make the output folder
-    if (!QDir().mkdir(_uniqueOutputPath)) {
-        qCCritical(model_baking) << "Failed to create FBX output folder" << _uniqueOutputPath;
-
-        emit finished();
-        return false;
-    }
-
-    // make the baked and original sub-folders used during export
-    QDir uniqueOutputDir = _uniqueOutputPath;
-    if (!uniqueOutputDir.mkdir(BAKED_OUTPUT_SUBFOLDER) || !uniqueOutputDir.mkdir(ORIGINAL_OUTPUT_SUBFOLDER)) {
-        qCCritical(model_baking) << "Failed to create baked/original subfolders in" << _uniqueOutputPath;
-
-        emit finished();
-        return false;
-    }
-
-    return true;
-}
-
-void FBXBaker::handleFBXNetworkReply() {
-    QNetworkReply* requestReply = qobject_cast<QNetworkReply*>(sender());
-
     if (requestReply->error() == QNetworkReply::NoError) {
         qCDebug(model_baking) << "Downloaded" << _fbxURL;
 
@@ -136,36 +167,12 @@ void FBXBaker::handleFBXNetworkReply() {
 
         // close that file now that we are done writing to it
         copyOfOriginal.close();
-
-        // kick off the bake process now that everything is ready to go
-        bake();
     } else {
         // add an error to our list stating that the FBX could not be downloaded
 
-
-        emit finished();
     }
 }
 
-void FBXBaker::bake() {
-    // (1) load the scene from the FBX file
-    // (2) enumerate the textures found in the scene and start a bake for them
-    // (3) export the FBX with re-written texture references
-
-    importScene();
-    rewriteAndBakeSceneTextures();
-    exportScene();
-
-    removeEmbeddedMediaFolder();
-    possiblyCleanupOriginals();
-
-    // at this point we are sure that we've finished everything that does not relate to textures
-    // so set that flag now
-    _finishedNonTextureOperations = true;
-
-    checkIfFinished();
-}
-
 bool FBXBaker::importScene() {
     // create an FBX SDK importer
     FbxImporter* importer = FbxImporter::Create(_sdkManager, "");
@@ -235,8 +242,6 @@ QString FBXBaker::createBakedTextureFileName(const QFileInfo& textureFileInfo) {
 QUrl FBXBaker::getTextureURL(const QFileInfo& textureFileInfo, FbxFileTexture* fileTexture) {
     QUrl urlToTexture;
 
-    qDebug() << "Looking at" << textureFileInfo.absoluteFilePath();
-
     if (textureFileInfo.exists() && textureFileInfo.isFile()) {
         // set the texture URL to the local texture that we have confirmed exists
         urlToTexture = QUrl::fromLocalFile(textureFileInfo.absoluteFilePath());
@@ -411,7 +416,7 @@ void FBXBaker::bakeTexture(const QUrl& textureURL) {
 
     connect(bakingTexture, &TextureBaker::finished, this, &FBXBaker::handleBakedTexture);
 
-    bakingTexture->start();
+    QtConcurrent::run(bakingTexture, &TextureBaker::bake);
 
     _bakingTextures.emplace_back(bakingTexture);
 }
@@ -454,9 +459,9 @@ void FBXBaker::handleBakedTexture() {
     // now that this texture has been baked and handled, we can remove that TextureBaker from our list
     _unbakedTextures.remove(bakedTexture->getTextureURL());
 
-    // since this could have been the last texture we were waiting for
+    if (_unbakedTextures.isEmpty()) {
-    // we should perform a quick check now to see if we are done baking this model
+        emit allTexturesBaked();
-    checkIfFinished();
+    }
 }
 
 bool FBXBaker::exportScene() {
@@ -500,9 +505,3 @@ void FBXBaker::possiblyCleanupOriginals() {
         QDir(_uniqueOutputPath + ORIGINAL_OUTPUT_SUBFOLDER).removeRecursively();
     }
 }
-
-void FBXBaker::checkIfFinished() {
-    if (_unbakedTextures.isEmpty() && _finishedNonTextureOperations) {
-        emit finished();
-    }
-}

libraries/model-baking/src/FBXBaker.h

@@ -12,6 +12,7 @@
 #ifndef hifi_FBXBaker_h
 #define hifi_FBXBaker_h
 
+#include <QtCore/QFutureSynchronizer>
 #include <QtCore/QDir>
 #include <QtCore/QUrl>
 #include <QtNetwork/QNetworkReply>
@@ -52,28 +53,29 @@ public:
     FBXBaker(const QUrl& fbxURL, const QString& baseOutputPath, bool copyOriginals = true);
     ~FBXBaker();
 
-    void start();
+    void bake();
 
     QUrl getFBXUrl() const { return _fbxURL; }
     QString getBakedFBXRelativePath() const { return _bakedFBXRelativePath; }
 
 signals:
     void finished();
+    void allTexturesBaked();
 
 private slots:
-    void handleFBXNetworkReply();
     void handleBakedTexture();
     
 private:
-    void bake();
+    void setupOutputFolder();
+
+    void loadSourceFBX();
+    void handleFBXNetworkReply(QNetworkReply* requestReply);
 
-    bool setupOutputFolder();
     bool importScene();
     bool rewriteAndBakeSceneTextures();
     bool exportScene();
     void removeEmbeddedMediaFolder();
     void possiblyCleanupOriginals();
-    void checkIfFinished();
 
     QString createBakedTextureFileName(const QFileInfo& textureFileInfo);
     QUrl getTextureURL(const QFileInfo& textureFileInfo, fbxsdk::FbxFileTexture* fileTexture);
@@ -99,10 +101,9 @@ private:
     QHash<uint64_t, TextureType> _textureTypes;
 
     std::list<std::unique_ptr<TextureBaker>> _bakingTextures;
+    QFutureSynchronizer<void> _textureBakeSynchronizer;
 
     bool _copyOriginals { true };
-
-    bool _finishedNonTextureOperations { false };
 };
 
 #endif // hifi_FBXBaker_h

libraries/model-baking/src/TextureBaker.cpp

@@ -9,6 +9,7 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
+#include <QtCore/QEventLoop>
 #include <QtCore/QFile>
 #include <QtNetwork/QNetworkReply>
 
@@ -24,8 +25,16 @@ TextureBaker::TextureBaker(const QUrl& textureURL) :
     
 }
 
-void TextureBaker::start() {
+void TextureBaker::bake() {
+    // first load the texture (either locally or remotely)
+    loadTexture();
 
+    qCDebug(model_baking) << "Baking texture at" << _textureURL;
+
+    emit finished();
+}
+
+void TextureBaker::loadTexture() {
     // check if the texture is local or first needs to be downloaded
     if (_textureURL.isLocalFile()) {
         // load up the local file
@@ -39,9 +48,6 @@ void TextureBaker::start() {
         }
 
         _originalTexture = localTexture.readAll();
-
-        // start the bake now that we have everything in place
-        bake();
     } else {
         // remote file, kick off a download
         auto& networkAccessManager = NetworkAccessManager::getInstance();
@@ -57,13 +63,17 @@ void TextureBaker::start() {
         qCDebug(model_baking) << "Downloading" << _textureURL;
 
         auto networkReply = networkAccessManager.get(networkRequest);
-        connect(networkReply, &QNetworkReply::finished, this, &TextureBaker::handleTextureNetworkReply);
+
+        // use an event loop to process events while we wait for the network reply
+        QEventLoop eventLoop;
+        connect(networkReply, &QNetworkReply::finished, &eventLoop, &QEventLoop::quit);
+        eventLoop.exec();
+
+        handleTextureNetworkReply(networkReply);
     }
 }
 
-void TextureBaker::handleTextureNetworkReply() {
+void TextureBaker::handleTextureNetworkReply(QNetworkReply* requestReply) {
-    QNetworkReply* requestReply = qobject_cast<QNetworkReply*>(sender());
-
     if (requestReply->error() == QNetworkReply::NoError) {
         qCDebug(model_baking) << "Downloaded texture at" << _textureURL;
 
@@ -75,14 +85,5 @@ void TextureBaker::handleTextureNetworkReply() {
     } else {
         // add an error to our list stating that this texture could not be downloaded
         qCDebug(model_baking) << "Error downloading texture" << requestReply->errorString();
-
-        emit finished();
     }
 }
-
-void TextureBaker::bake() {
-    qCDebug(model_baking) << "Baking texture at" << _textureURL;
-
-    // call image library to asynchronously bake this texture
-    emit finished();
-}

libraries/model-baking/src/TextureBaker.h

@@ -21,7 +21,7 @@ class TextureBaker : public QObject {
 public:
     TextureBaker(const QUrl& textureURL);
 
-    void start();
+    void bake();
 
     const QByteArray& getOriginalTexture() const { return _originalTexture; }
 
@@ -30,11 +30,9 @@ public:
 signals:
     void finished();
 
-private slots:
-    void handleTextureNetworkReply();
-
 private:
-    void bake();
+    void loadTexture();
+    void handleTextureNetworkReply(QNetworkReply* requestReply);
 
     QUrl _textureURL;
     QByteArray _originalTexture;

tools/oven/CMakeLists.txt

@@ -1,5 +1,5 @@
 set(TARGET_NAME oven)
 
-setup_hifi_project(Widgets Gui)
+setup_hifi_project(Widgets Gui Concurrent)
 
 link_hifi_libraries(model-baking shared)

tools/oven/src/DomainBaker.cpp

@@ -9,10 +9,10 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
-#include <QtCore/QDebug>
+#include <QtConcurrent>
+#include <QtCore/QEventLoop>
 #include <QtCore/QFile>
 #include <QtCore/QFileInfo>
-
 #include <QtCore/QJsonDocument>
 #include <QtCore/QJsonObject>
 
@@ -34,10 +34,22 @@ DomainBaker::DomainBaker(const QUrl& localModelFileURL, const QString& domainNam
     }
 }
 
-void DomainBaker::start() {
+void DomainBaker::bake() {
     setupOutputFolder();
     loadLocalFile();
     enumerateEntities();
+
+    if (!_entitiesNeedingRewrite.isEmpty()) {
+        // use a QEventLoop to wait for all entity rewrites to be completed before writing the final models file
+        QEventLoop eventLoop;
+        connect(this, &DomainBaker::allModelsFinished, &eventLoop, &QEventLoop::quit);
+        eventLoop.exec();
+    }
+
+    writeNewEntitiesFile();
+
+    // we've now written out our new models file - time to say that we are finished up
+    emit finished();
 }
 
 void DomainBaker::setupOutputFolder() {
@@ -147,11 +159,11 @@ void DomainBaker::enumerateEntities() {
                         // make sure our handler is called when the baker is done
                         connect(baker.data(), &FBXBaker::finished, this, &DomainBaker::handleFinishedBaker);
 
-                        // start the baker
-                        baker->start();
-
                         // insert it into our bakers hash so we hold a strong pointer to it
                         _bakers.insert(modelURL, baker);
+
+                        // send the FBXBaker to the thread pool
+                        QtConcurrent::run(baker.data(), &FBXBaker::bake);
                     }
 
                     // add this QJsonValueRef to our multi hash so that we can easily re-write
@@ -161,11 +173,6 @@ void DomainBaker::enumerateEntities() {
             }
         }
     }
-
-    _enumeratedAllEntities = true;
-
-    // check if it's time to write out the final entities file with re-written URLs
-    possiblyOutputEntitiesFile();
 }
 
 void DomainBaker::handleFinishedBaker() {
@@ -201,49 +208,45 @@ void DomainBaker::handleFinishedBaker() {
         // remove the baked URL from the multi hash of entities needing a re-write
         _entitiesNeedingRewrite.remove(baker->getFBXUrl());
 
-        // check if it's time to write out the final entities file with re-written URLs
+        if (_entitiesNeedingRewrite.isEmpty()) {
-        possiblyOutputEntitiesFile();
+            emit allModelsFinished();
-    }
-}
-
-void DomainBaker::possiblyOutputEntitiesFile() {
-    if (_enumeratedAllEntities && _entitiesNeedingRewrite.isEmpty()) {
-        // we've enumerated all of our entities and re-written all the URLs we'll be able to re-write
-        // time to write out a main models.json.gz file
-
-        // first setup a document with the entities array below the entities key
-        QJsonDocument entitiesDocument;
-
-        QJsonObject rootObject;
-        rootObject[ENTITIES_OBJECT_KEY] = _entities;
-
-        entitiesDocument.setObject(rootObject);
-
-        // turn that QJsonDocument into a byte array ready for compression
-        QByteArray jsonByteArray = entitiesDocument.toJson();
-
-        // compress the json byte array using gzip
-        QByteArray compressedJson;
-        gzip(jsonByteArray, compressedJson);
-
-        // write the gzipped json to a new models file
-        static const QString MODELS_FILE_NAME = "models.json.gz";
-
-        auto bakedEntitiesFilePath = QDir(_uniqueOutputPath).filePath(MODELS_FILE_NAME);
-        QFile compressedEntitiesFile { bakedEntitiesFilePath };
-
-        if (!compressedEntitiesFile.open(QIODevice::WriteOnly)
-            || (compressedEntitiesFile.write(compressedJson) == -1)) {
-            qWarning() << "Failed to export baked entities file to" << bakedEntitiesFilePath;
-            // add an error to our list to state that the output models file could not be created or could not be written to
-
-            return;
         }
-
-        qDebug() << "Exported entities file with baked model URLs to" << bakedEntitiesFilePath;
-
-        // we've now written out our new models file - time to say that we are finished up
-        emit finished();
     }
 }
 
+void DomainBaker::writeNewEntitiesFile() {
+    // we've enumerated all of our entities and re-written all the URLs we'll be able to re-write
+    // time to write out a main models.json.gz file
+
+    // first setup a document with the entities array below the entities key
+    QJsonDocument entitiesDocument;
+
+    QJsonObject rootObject;
+    rootObject[ENTITIES_OBJECT_KEY] = _entities;
+
+    entitiesDocument.setObject(rootObject);
+
+    // turn that QJsonDocument into a byte array ready for compression
+    QByteArray jsonByteArray = entitiesDocument.toJson();
+
+    // compress the json byte array using gzip
+    QByteArray compressedJson;
+    gzip(jsonByteArray, compressedJson);
+
+    // write the gzipped json to a new models file
+    static const QString MODELS_FILE_NAME = "models.json.gz";
+
+    auto bakedEntitiesFilePath = QDir(_uniqueOutputPath).filePath(MODELS_FILE_NAME);
+    QFile compressedEntitiesFile { bakedEntitiesFilePath };
+
+    if (!compressedEntitiesFile.open(QIODevice::WriteOnly)
+        || (compressedEntitiesFile.write(compressedJson) == -1)) {
+        qWarning() << "Failed to export baked entities file to" << bakedEntitiesFilePath;
+        // add an error to our list to state that the output models file could not be created or could not be written to
+
+        return;
+    }
+
+    qDebug() << "Exported entities file with baked model URLs to" << bakedEntitiesFilePath;
+}
+

tools/oven/src/DomainBaker.h

@@ -24,11 +24,12 @@ public:
     DomainBaker(const QUrl& localEntitiesFileURL, const QString& domainName,
                 const QString& baseOutputPath, const QUrl& destinationPath);
 
-public slots:
+public:
-    void start();
+    void bake();
 
 signals:
     void finished();
+    void allModelsFinished();
 
 private slots:
     void handleFinishedBaker();
@@ -37,7 +38,7 @@ private:
     void setupOutputFolder();
     void loadLocalFile();
     void enumerateEntities();
-    void possiblyOutputEntitiesFile();
+    void writeNewEntitiesFile();
 
     QUrl _localEntitiesFileURL;
     QString _domainName;
@@ -50,8 +51,6 @@ private:
 
     QHash<QUrl, QSharedPointer<FBXBaker>> _bakers;
     QMultiHash<QUrl, QJsonValueRef> _entitiesNeedingRewrite;
-
-    bool _enumeratedAllEntities { false };
 };
 
 #endif // hifi_DomainBaker_h

tools/oven/src/Oven.cpp

@@ -9,6 +9,8 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
+#include <QtCore/QDebug>
+
 #include <SettingInterface.h>
 
 #include "ui/OvenMainWindow.h"

tools/oven/src/ui/DomainBakeWidget.cpp

@@ -9,6 +9,8 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
+#include <QtConcurrent>
+
 #include <QtWidgets/QFileDialog>
 #include <QtWidgets/QGridLayout>
 #include <QtWidgets/QLabel>
@@ -207,7 +209,9 @@ void DomainBakeWidget::bakeButtonClicked() {
                 new DomainBaker(fileToBakeURL, _domainNameLineEdit->text(),
                                 outputDirectory.absolutePath(), _destinationPathLineEdit->text())
         };
-        _baker->start();
+
+        // run the baker in our thread pool
+        QtConcurrent::run(_baker.get(), &DomainBaker::bake);
 
         return;
     }

tools/oven/src/ui/ModelBakeWidget.cpp

@@ -176,7 +176,7 @@ void ModelBakeWidget::bakeButtonClicked() {
 
         // everything seems to be in place, kick off a bake for this model now
         auto baker = new FBXBaker(modelToBakeURL, outputDirectory.absolutePath(), false);
-        baker->start();
+        baker->bake();
         _bakers.emplace_back(baker);
     }
 }