From c5fbd28ecf3425bb2f110aa59078dc9b0fee36c5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 13 Apr 2017 18:25:27 -0700 Subject: [PATCH] put all FBXBaker on same thread for bad FBX SDK --- libraries/model-baking/src/FBXBaker.cpp | 103 +++++++++++--------- libraries/model-baking/src/FBXBaker.h | 23 +++-- libraries/model-baking/src/TextureBaker.cpp | 3 - tools/oven/src/DomainBaker.cpp | 19 +++- tools/oven/src/DomainBaker.h | 3 + 5 files changed, 93 insertions(+), 58 deletions(-) diff --git a/libraries/model-baking/src/FBXBaker.cpp b/libraries/model-baking/src/FBXBaker.cpp index ac72fb152c..5b5a8b7c0a 100644 --- a/libraries/model-baking/src/FBXBaker.cpp +++ b/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 - 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(); + if (hasErrors()) { + return; } - 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 - bake(); + // emit our signal to start the import of the FBX source copy + 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 - QEventLoop eventLoop; - connect(networkReply, &QNetworkReply::finished, &eventLoop, &QEventLoop::quit); - eventLoop.exec(); - - handleFBXNetworkReply(networkReply); + connect(networkReply, &QNetworkReply::finished, this, &FBXBaker::handleFBXNetworkReply); } } -void FBXBaker::handleFBXNetworkReply(QNetworkReply* requestReply) { +void FBXBaker::handleFBXNetworkReply() { + auto requestReply = qobject_cast(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 - << "- error:" << importer->GetStatus().GetErrorString(); - - return false; + handleError("Failed to import FBX file at" + _fbxURL.toString() + " - error:" + importer->GetStatus().GetErrorString()); + return; } 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() - << "for" << _fbxURL; + handleError("Could not save original external texture " + originalTextureFile.fileName() + + " 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 - << "to" << rewrittenFBXPath << "- error:" << exporter->GetStatus().GetErrorString(); - - return false; + handleError("Failed to export FBX file at " + _fbxURL.toString() + " to " + rewrittenFBXPath + + "- error: " + exporter->GetStatus().GetErrorString()); } // export the scene exporter->Export(_scene); qCDebug(model_baking) << "Exported" << _fbxURL << "with re-written paths to" << rewrittenFBXPath; - - return true; } diff --git a/libraries/model-baking/src/FBXBaker.h b/libraries/model-baking/src/FBXBaker.h index 458723dab0..10c8a5c359 100644 --- a/libraries/model-baking/src/FBXBaker.h +++ b/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(); - bool rewriteAndBakeSceneTextures(); - bool exportScene(); + void bakeCopiedFBX(); + + 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 _textureBakeSynchronizer; bool _copyOriginals { true }; + + bool _finishedNonTextureOperations { false }; }; #endif // hifi_FBXBaker_h diff --git a/libraries/model-baking/src/TextureBaker.cpp b/libraries/model-baking/src/TextureBaker.cpp index b8c49c6d50..a717835ed7 100644 --- a/libraries/model-baking/src/TextureBaker.cpp +++ b/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(); diff --git a/tools/oven/src/DomainBaker.cpp b/tools/oven/src/DomainBaker.cpp index c8e03e1224..c779fd3a40 100644 --- a/tools/oven/src/DomainBaker.cpp +++ b/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(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 - QtConcurrent::run(baker.data(), &FBXBaker::bake); + // move the baker to the baker thread + // 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 diff --git a/tools/oven/src/DomainBaker.h b/tools/oven/src/DomainBaker.h index e3585ba64f..f949ddba9c 100644 --- a/tools/oven/src/DomainBaker.h +++ b/tools/oven/src/DomainBaker.h @@ -15,6 +15,7 @@ #include #include #include +#include #include @@ -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 _fbxBakerThread; QHash> _bakers; QMultiHash _entitiesNeedingRewrite; };