put all FBXBaker on same thread for bad FBX SDK

libraries/model-baking/src/FBXBaker.cpp

@@ -50,39 +50,59 @@ QString FBXBaker::pathToCopyOfOriginal() const {
     return _uniqueOutputPath + ORIGINAL_OUTPUT_SUBFOLDER + _fbxURL.fileName();
 }
 
+void FBXBaker::handleError(const QString& error) {
+    qCCritical(model_baking) << error;
+    _errorList << error;
+    emit finished();
+}
+
 void FBXBaker::bake() {
     qCDebug(model_baking) << "Baking" << _fbxURL;
 
     // setup the output folder for the results of this bake
     setupOutputFolder();
 
+    if (hasErrors()) {
+        return;
+    }
+
+    connect(this, &FBXBaker::sourceCopyReadyToLoad, this, &FBXBaker::bakeSourceCopy);
+
     // make a local copy of the FBX file
     loadSourceFBX();
+}
 
+void FBXBaker::bakeSourceCopy() {
     // load the scene from the FBX file
     importScene();
 
+    if (hasErrors()) {
+        return;
+    }
+
     // enumerate the textures found in the scene and start a bake for them
     rewriteAndBakeSceneTextures();
 
+    if (hasErrors()) {
+        return;
+    }
+
     // export the FBX with re-written texture references
     exportScene();
 
+    if (hasErrors()) {
+        return;
+    }
+
     // 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
+    if (hasErrors()) {
-    possiblyCleanupOriginals();
+        return;
-
-    // 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();
+    // cleanup the originals if we weren't asked to keep them around
+    possiblyCleanupOriginals();
 }
 
 void FBXBaker::setupOutputFolder() {
@@ -100,16 +120,14 @@ void FBXBaker::setupOutputFolder() {
 
     // attempt to make the output folder
     if (!QDir().mkdir(_uniqueOutputPath)) {
-        qCCritical(model_baking) << "Failed to create FBX output folder" << _uniqueOutputPath;
+        handleError("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;
+        handleError("Failed to create baked/original subfolders in " + _uniqueOutputPath);
-
         return;
     }
 }
@@ -123,8 +141,8 @@ void FBXBaker::loadSourceFBX() {
         // make a copy in the output folder
         localFBX.copy(pathToCopyOfOriginal());
 
-        // start the bake now that we have everything in place
+        // emit our signal to start the import of the FBX source copy
-        bake();
+        emit sourceCopyReadyToLoad();
     } else {
         // remote file, kick off a download
         auto& networkAccessManager = NetworkAccessManager::getInstance();
@@ -140,16 +158,13 @@ void FBXBaker::loadSourceFBX() {
         qCDebug(model_baking) << "Downloading" << _fbxURL;
         auto networkReply = networkAccessManager.get(networkRequest);
 
-        // start a QEventLoop so we process events while waiting for the request to complete
+        connect(networkReply, &QNetworkReply::finished, this, &FBXBaker::handleFBXNetworkReply);
-        QEventLoop eventLoop;
-        connect(networkReply, &QNetworkReply::finished, &eventLoop, &QEventLoop::quit);
-        eventLoop.exec();
-
-        handleFBXNetworkReply(networkReply);
     }
 }
 
-void FBXBaker::handleFBXNetworkReply(QNetworkReply* requestReply) {
+void FBXBaker::handleFBXNetworkReply() {
+    auto requestReply = qobject_cast<QNetworkReply*>(sender());
+
     if (requestReply->error() == QNetworkReply::NoError) {
         qCDebug(model_baking) << "Downloaded" << _fbxURL;
 
@@ -159,21 +174,23 @@ void FBXBaker::handleFBXNetworkReply(QNetworkReply* requestReply) {
         qDebug(model_baking) << "Writing copy of original FBX to" << copyOfOriginal.fileName();
 
         if (!copyOfOriginal.open(QIODevice::WriteOnly) || (copyOfOriginal.write(requestReply->readAll()) == -1)) {
-
             // add an error to the error list for this FBX stating that a duplicate of the original FBX could not be made
-            emit finished();
+            handleError("Could not create copy of " + _fbxURL.toString());
             return;
         }
 
         // close that file now that we are done writing to it
         copyOfOriginal.close();
+
+        // emit our signal to start the import of the FBX source copy
+        emit sourceCopyReadyToLoad();
     } else {
         // add an error to our list stating that the FBX could not be downloaded
-
+        handleError("Failed to download " + _fbxURL.toString());
     }
 }
 
-bool FBXBaker::importScene() {
+void FBXBaker::importScene() {
     // create an FBX SDK importer
     FbxImporter* importer = FbxImporter::Create(_sdkManager, "");
 
@@ -183,10 +200,8 @@ bool FBXBaker::importScene() {
 
     if (!importStatus) {
         // failed to initialize importer, print an error and return
-        qCCritical(model_baking) << "Failed to import FBX file at" << _fbxURL
+        handleError("Failed to import FBX file at" + _fbxURL.toString() + " - error:" + importer->GetStatus().GetErrorString());
-            << "- error:" << importer->GetStatus().GetErrorString();
+        return;
-
-        return false;
     } else {
         qCDebug(model_baking) << "Imported" << _fbxURL << "to FbxScene";
     }
@@ -199,8 +214,6 @@ bool FBXBaker::importScene() {
 
     // destroy the importer that is no longer needed
     importer->Destroy();
-
-    return true;
 }
 
 static const QString BAKED_TEXTURE_EXT = ".ktx";
@@ -344,7 +357,7 @@ TextureType textureTypeForMaterialProperty(FbxProperty& property, FbxSurfaceMate
     return UNUSED_TEXTURE;
 }
 
-bool FBXBaker::rewriteAndBakeSceneTextures() {
+void FBXBaker::rewriteAndBakeSceneTextures() {
 
     // enumerate the surface materials to find the textures used in the scene
     int numMaterials = _scene->GetMaterialCount();
@@ -406,8 +419,6 @@ bool FBXBaker::rewriteAndBakeSceneTextures() {
             }
         }
     }
-
-    return true;
 }
 
 void FBXBaker::bakeTexture(const QUrl& textureURL) {
@@ -449,22 +460,24 @@ void FBXBaker::handleBakedTexture() {
 
         if (originalTextureFile.open(QIODevice::WriteOnly) && originalTextureFile.write(bakedTexture->getOriginalTexture()) != -1) {
             qCDebug(model_baking) << "Saved original texture file" << originalTextureFile.fileName()
-            << "for" << _fbxURL;
+                << "for" << _fbxURL;
         } else {
-            qCWarning(model_baking) << "Could not save original external texture" << originalTextureFile.fileName()
+            handleError("Could not save original external texture " + originalTextureFile.fileName()
-            << "for" << _fbxURL;
+                       + " for " + _fbxURL.toString());
+            return;
         }
     }
 
     // now that this texture has been baked and handled, we can remove that TextureBaker from our list
     _unbakedTextures.remove(bakedTexture->getTextureURL());
 
+    // check if we're done everything we need to do for this FBX
     if (_unbakedTextures.isEmpty()) {
-        emit allTexturesBaked();
+        emit finished();
     }
 }
 
-bool FBXBaker::exportScene() {
+void FBXBaker::exportScene() {
     // setup the exporter
     FbxExporter* exporter = FbxExporter::Create(_sdkManager, "");
 
@@ -478,18 +491,14 @@ bool FBXBaker::exportScene() {
 
     if (!exportStatus) {
         // failed to initialize exporter, print an error and return
-         qCCritical(model_baking) << "Failed to export FBX file at" << _fbxURL
+        handleError("Failed to export FBX file at " + _fbxURL.toString() + " to " + rewrittenFBXPath
-            << "to" << rewrittenFBXPath << "- error:" << exporter->GetStatus().GetErrorString();
+                    + "- error: " + exporter->GetStatus().GetErrorString());
-
-        return false;
     }
 
     // export the scene
     exporter->Export(_scene);
 
     qCDebug(model_baking) << "Exported" << _fbxURL << "with re-written paths to" << rewrittenFBXPath;
-
-    return true;
 }
 
 

libraries/model-baking/src/FBXBaker.h

@@ -53,7 +53,9 @@ public:
     FBXBaker(const QUrl& fbxURL, const QString& baseOutputPath, bool copyOriginals = true);
     ~FBXBaker();
 
-    void bake();
+    Q_INVOKABLE void bake();
+
+    bool hasErrors() const { return !_errorList.isEmpty(); }
 
     QUrl getFBXUrl() const { return _fbxURL; }
     QString getBakedFBXRelativePath() const { return _bakedFBXRelativePath; }
@@ -62,18 +64,23 @@ signals:
     void finished();
     void allTexturesBaked();
 
+    void sourceCopyReadyToLoad();
+
 private slots:
+    void bakeSourceCopy();
+    void handleFBXNetworkReply();
     void handleBakedTexture();
-    
+
 private:
     void setupOutputFolder();
 
     void loadSourceFBX();
-    void handleFBXNetworkReply(QNetworkReply* requestReply);
 
-    bool importScene();
+    void bakeCopiedFBX();
-    bool rewriteAndBakeSceneTextures();
+
-    bool exportScene();
+    void importScene();
+    void rewriteAndBakeSceneTextures();
+    void exportScene();
     void removeEmbeddedMediaFolder();
     void possiblyCleanupOriginals();
 
@@ -84,6 +91,8 @@ private:
 
     QString pathToCopyOfOriginal() const;
 
+    void handleError(const QString& error);
+
     QUrl _fbxURL;
     QString _fbxName;
     
@@ -104,6 +113,8 @@ private:
     QFutureSynchronizer<void> _textureBakeSynchronizer;
 
     bool _copyOriginals { true };
+
+    bool _finishedNonTextureOperations { false };
 };
 
 #endif // hifi_FBXBaker_h

libraries/model-baking/src/TextureBaker.cpp

@@ -79,9 +79,6 @@ void TextureBaker::handleTextureNetworkReply(QNetworkReply* requestReply) {
 
         // store the original texture so it can be passed along for the bake
         _originalTexture = requestReply->readAll();
-
-        // kickoff the texture bake now that everything is ready to go
-        bake();
     } else {
         // add an error to our list stating that this texture could not be downloaded
         qCDebug(model_baking) << "Error downloading texture" << requestReply->errorString();

tools/oven/src/DomainBaker.cpp

@@ -37,6 +37,7 @@ DomainBaker::DomainBaker(const QUrl& localModelFileURL, const QString& domainNam
 void DomainBaker::bake() {
     setupOutputFolder();
     loadLocalFile();
+    setupBakerThread();
     enumerateEntities();
 
     if (!_entitiesNeedingRewrite.isEmpty()) {
@@ -48,6 +49,9 @@ void DomainBaker::bake() {
 
     writeNewEntitiesFile();
 
+    // stop the FBX baker thread now that all our bakes have completed
+    _fbxBakerThread->quit();
+
     // we've now written out our new models file - time to say that we are finished up
     emit finished();
 }
@@ -126,6 +130,15 @@ void DomainBaker::loadLocalFile() {
     }
 }
 
+void DomainBaker::setupBakerThread() {
+    // This is a real bummer, but the FBX SDK is not thread safe - even with separate FBXManager objects.
+    // This means that we need to put all of the FBX importing/exporting on the same thread.
+    // We'll setup that thread now and then move the FBXBaker objects to the thread later when enumerating entities.
+    _fbxBakerThread = std::unique_ptr<QThread>(new QThread);
+    _fbxBakerThread->setObjectName("Domain FBX Baker Thread");
+    _fbxBakerThread->start();
+}
+
 static const QString ENTITY_MODEL_URL_KEY = "modelURL";
 
 void DomainBaker::enumerateEntities() {
@@ -162,8 +175,10 @@ void DomainBaker::enumerateEntities() {
                         // 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
+                        // move the baker to the baker thread
-                        QtConcurrent::run(baker.data(), &FBXBaker::bake);
+                        // and kickoff the bake
+                        baker->moveToThread(_fbxBakerThread.get());
+                        QMetaObject::invokeMethod(baker.data(), "bake");
                     }
 
                     // add this QJsonValueRef to our multi hash so that we can easily re-write

tools/oven/src/DomainBaker.h

@@ -15,6 +15,7 @@
 #include <QtCore/QJsonArray>
 #include <QtCore/QObject>
 #include <QtCore/QUrl>
+#include <QtCore/QThread>
 
 #include <FBXBaker.h>
 
@@ -37,6 +38,7 @@ private slots:
 private:
     void setupOutputFolder();
     void loadLocalFile();
+    void setupBakerThread();
     void enumerateEntities();
     void writeNewEntitiesFile();
 
@@ -49,6 +51,7 @@ private:
 
     QJsonArray _entities;
 
+    std::unique_ptr<QThread> _fbxBakerThread;
     QHash<QUrl, QSharedPointer<FBXBaker>> _bakers;
     QMultiHash<QUrl, QJsonValueRef> _entitiesNeedingRewrite;
 };