From 916cecb8ec147c3d9e2227743ea78f7e0b915059 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 13 Apr 2017 16:17:06 -0700 Subject: [PATCH] use QtConcurrent to cleanup threading of bakers --- libraries/model-baking/CMakeLists.txt | 2 +- libraries/model-baking/src/FBXBaker.cpp | 151 ++++++++++---------- libraries/model-baking/src/FBXBaker.h | 15 +- libraries/model-baking/src/TextureBaker.cpp | 35 ++--- libraries/model-baking/src/TextureBaker.h | 8 +- tools/oven/CMakeLists.txt | 2 +- tools/oven/src/DomainBaker.cpp | 109 +++++++------- tools/oven/src/DomainBaker.h | 9 +- tools/oven/src/Oven.cpp | 2 + tools/oven/src/ui/DomainBakeWidget.cpp | 6 +- tools/oven/src/ui/ModelBakeWidget.cpp | 2 +- 11 files changed, 174 insertions(+), 167 deletions(-) diff --git a/libraries/model-baking/CMakeLists.txt b/libraries/model-baking/CMakeLists.txt index 487b8536c9..a0774cdcc1 100644 --- a/libraries/model-baking/CMakeLists.txt +++ b/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) diff --git a/libraries/model-baking/src/FBXBaker.cpp b/libraries/model-baking/src/FBXBaker.cpp index f798a137e4..ac72fb152c 100644 --- a/libraries/model-baking/src/FBXBaker.cpp +++ b/libraries/model-baking/src/FBXBaker.cpp @@ -11,8 +11,12 @@ #include +#include +#include #include +#include #include +#include #include @@ -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() { - // 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(sender()); - +void FBXBaker::handleFBXNetworkReply(QNetworkReply* requestReply) { 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 - // we should perform a quick check now to see if we are done baking this model - checkIfFinished(); + if (_unbakedTextures.isEmpty()) { + emit allTexturesBaked(); + } } 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(); - } -} diff --git a/libraries/model-baking/src/FBXBaker.h b/libraries/model-baking/src/FBXBaker.h index 71ded0c44a..458723dab0 100644 --- a/libraries/model-baking/src/FBXBaker.h +++ b/libraries/model-baking/src/FBXBaker.h @@ -12,6 +12,7 @@ #ifndef hifi_FBXBaker_h #define hifi_FBXBaker_h +#include #include #include #include @@ -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 _textureTypes; std::list> _bakingTextures; + 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 367ae5f463..b8c49c6d50 100644 --- a/libraries/model-baking/src/TextureBaker.cpp +++ b/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 #include #include @@ -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() { - QNetworkReply* requestReply = qobject_cast(sender()); - +void TextureBaker::handleTextureNetworkReply(QNetworkReply* requestReply) { 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(); -} diff --git a/libraries/model-baking/src/TextureBaker.h b/libraries/model-baking/src/TextureBaker.h index 5544352005..7394a0652e 100644 --- a/libraries/model-baking/src/TextureBaker.h +++ b/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; diff --git a/tools/oven/CMakeLists.txt b/tools/oven/CMakeLists.txt index 2e40cc9de4..2c5d0b98e5 100644 --- a/tools/oven/CMakeLists.txt +++ b/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) diff --git a/tools/oven/src/DomainBaker.cpp b/tools/oven/src/DomainBaker.cpp index 7cd675de3f..c8e03e1224 100644 --- a/tools/oven/src/DomainBaker.cpp +++ b/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 +#include +#include #include #include - #include #include @@ -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 - possiblyOutputEntitiesFile(); - } -} - -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; + if (_entitiesNeedingRewrite.isEmpty()) { + emit allModelsFinished(); } - - 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; +} + diff --git a/tools/oven/src/DomainBaker.h b/tools/oven/src/DomainBaker.h index 17419b736a..e3585ba64f 100644 --- a/tools/oven/src/DomainBaker.h +++ b/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: - void start(); +public: + 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> _bakers; QMultiHash _entitiesNeedingRewrite; - - bool _enumeratedAllEntities { false }; }; #endif // hifi_DomainBaker_h diff --git a/tools/oven/src/Oven.cpp b/tools/oven/src/Oven.cpp index ede1b7c1e4..60754759f4 100644 --- a/tools/oven/src/Oven.cpp +++ b/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 + #include #include "ui/OvenMainWindow.h" diff --git a/tools/oven/src/ui/DomainBakeWidget.cpp b/tools/oven/src/ui/DomainBakeWidget.cpp index 2194f17a39..01bc6110cb 100644 --- a/tools/oven/src/ui/DomainBakeWidget.cpp +++ b/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 + #include #include #include @@ -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; } diff --git a/tools/oven/src/ui/ModelBakeWidget.cpp b/tools/oven/src/ui/ModelBakeWidget.cpp index ff95e521c5..5f31ed2673 100644 --- a/tools/oven/src/ui/ModelBakeWidget.cpp +++ b/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); } }