diff --git a/libraries/model-baking/src/Baker.h b/libraries/model-baking/src/Baker.h index ab9c22ac53..8f73819dfe 100644 --- a/libraries/model-baking/src/Baker.h +++ b/libraries/model-baking/src/Baker.h @@ -18,14 +18,15 @@ class Baker : public QObject { Q_OBJECT public: - virtual void bake() = 0; - bool hasErrors() const { return !_errorList.isEmpty(); } QStringList getErrors() const { return _errorList; } bool hasWarnings() const { return !_warningList.isEmpty(); } QStringList getWarnings() const { return _warningList; } +public slots: + virtual void bake() = 0; + signals: void finished(); diff --git a/libraries/model-baking/src/FBXBaker.cpp b/libraries/model-baking/src/FBXBaker.cpp index ddf68b74c7..5b47f5023b 100644 --- a/libraries/model-baking/src/FBXBaker.cpp +++ b/libraries/model-baking/src/FBXBaker.cpp @@ -30,9 +30,11 @@ std::once_flag onceFlag; FBXSDKManagerUniquePointer FBXBaker::_sdkManager { nullptr }; -FBXBaker::FBXBaker(const QUrl& fbxURL, const QString& baseOutputPath, bool copyOriginals) : +FBXBaker::FBXBaker(const QUrl& fbxURL, const QString& baseOutputPath, + TextureBakerThreadGetter textureThreadGetter, bool copyOriginals) : _fbxURL(fbxURL), _baseOutputPath(baseOutputPath), + _textureThreadGetter(textureThreadGetter), _copyOriginals(copyOriginals) { std::call_once(onceFlag, [](){ @@ -402,8 +404,9 @@ void FBXBaker::bakeTexture(const QUrl& textureURL, gpu::TextureType textureType, // keep a shared pointer to the baking texture _bakingTextures.insert(bakingTexture); - // start baking the texture on our thread pool - QtConcurrent::run(bakingTexture.data(), &TextureBaker::bake); + // start baking the texture on one of our available worker threads + bakingTexture->moveToThread(_textureThreadGetter()); + QMetaObject::invokeMethod(bakingTexture.data(), "bake"); } void FBXBaker::handleBakedTexture() { diff --git a/libraries/model-baking/src/FBXBaker.h b/libraries/model-baking/src/FBXBaker.h index cca4878308..8ad42d6de8 100644 --- a/libraries/model-baking/src/FBXBaker.h +++ b/libraries/model-baking/src/FBXBaker.h @@ -32,21 +32,23 @@ namespace fbxsdk { static const QString BAKED_FBX_EXTENSION = ".baked.fbx"; using FBXSDKManagerUniquePointer = std::unique_ptr>; +using TextureBakerThreadGetter = std::function; + class FBXBaker : public Baker { Q_OBJECT public: - FBXBaker(const QUrl& fbxURL, const QString& baseOutputPath, bool copyOriginals = true); - - // all calls to bake must be from the same thread, because the Autodesk SDK will cause - // a crash if it is called from multiple threads - Q_INVOKABLE virtual void bake() override; + FBXBaker(const QUrl& fbxURL, const QString& baseOutputPath, + TextureBakerThreadGetter textureThreadGetter, bool copyOriginals = true); QUrl getFBXUrl() const { return _fbxURL; } QString getBakedFBXRelativePath() const { return _bakedFBXRelativePath; } -signals: - void allTexturesBaked(); +public slots: + // all calls to FBXBaker::bake for FBXBaker instances must be from the same thread + // because the Autodesk SDK will cause a crash if it is called from multiple threads + virtual void bake() override; +signals: void sourceCopyReadyToLoad(); private slots: @@ -92,6 +94,8 @@ private: QSet> _bakingTextures; QFutureSynchronizer _textureBakeSynchronizer; + TextureBakerThreadGetter _textureThreadGetter; + bool _copyOriginals { true }; bool _pendingErrorEmission { false }; diff --git a/libraries/model-baking/src/TextureBaker.cpp b/libraries/model-baking/src/TextureBaker.cpp index 82f42a5776..0ebde00bde 100644 --- a/libraries/model-baking/src/TextureBaker.cpp +++ b/libraries/model-baking/src/TextureBaker.cpp @@ -33,24 +33,11 @@ TextureBaker::TextureBaker(const QUrl& textureURL, gpu::TextureType textureType, } void TextureBaker::bake() { + // once our texture is loaded, kick off a the processing + connect(this, &TextureBaker::originalTextureLoaded, this, &TextureBaker::processTexture); + // first load the texture (either locally or remotely) loadTexture(); - - if (hasErrors()) { - return; - } - - qCDebug(model_baking) << "Baking texture at" << _textureURL; - - processTexture(); - - if (hasErrors()) { - return; - } - - qCDebug(model_baking) << "Baked texture at" << _textureURL; - - emit finished(); } void TextureBaker::loadTexture() { @@ -65,6 +52,8 @@ void TextureBaker::loadTexture() { } _originalTexture = localTexture.readAll(); + + emit originalTextureLoaded(); } else { // remote file, kick off a download auto& networkAccessManager = NetworkAccessManager::getInstance(); @@ -79,23 +68,22 @@ void TextureBaker::loadTexture() { qCDebug(model_baking) << "Downloading" << _textureURL; + // kickoff the download, wait for slot to tell us it is done auto networkReply = networkAccessManager.get(networkRequest); - - // 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); + connect(networkReply, &QNetworkReply::finished, this, &TextureBaker::handleTextureNetworkReply); } } -void TextureBaker::handleTextureNetworkReply(QNetworkReply* requestReply) { +void TextureBaker::handleTextureNetworkReply() { + auto requestReply = qobject_cast(sender()); + if (requestReply->error() == QNetworkReply::NoError) { - qCDebug(model_baking) << "Downloaded texture at" << _textureURL; + qCDebug(model_baking) << "Downloaded texture" << _textureURL; // store the original texture so it can be passed along for the bake _originalTexture = requestReply->readAll(); + + emit originalTextureLoaded(); } else { // add an error to our list stating that this texture could not be downloaded handleError("Error downloading " + _textureURL.toString() + " - " + requestReply->errorString()); @@ -111,6 +99,13 @@ void TextureBaker::processTexture() { return; } + // the baked textures need to have the source hash added for cache checks in Interface + // so we add that to the processed texture before handling it off to be serialized + QCryptographicHash hasher(QCryptographicHash::Md5); + hasher.addData(_originalTexture); + std::string hash = hasher.result().toHex().toStdString(); + processedTexture->setSourceHash(hash); + auto memKTX = gpu::Texture::serialize(*processedTexture); if (!memKTX) { @@ -127,4 +122,7 @@ void TextureBaker::processTexture() { if (!bakedTextureFile.open(QIODevice::WriteOnly) || bakedTextureFile.write(data, length) == -1) { handleError("Could not write baked texture for " + _textureURL.toString()); } + + qCDebug(model_baking) << "Baked texture" << _textureURL; + emit finished(); } diff --git a/libraries/model-baking/src/TextureBaker.h b/libraries/model-baking/src/TextureBaker.h index 17c725b57d..c0cb8a377a 100644 --- a/libraries/model-baking/src/TextureBaker.h +++ b/libraries/model-baking/src/TextureBaker.h @@ -27,18 +27,23 @@ class TextureBaker : public Baker { public: TextureBaker(const QUrl& textureURL, gpu::TextureType textureType, const QString& destinationFilePath); - - void bake(); const QByteArray& getOriginalTexture() const { return _originalTexture; } const QUrl& getTextureURL() const { return _textureURL; } +public slots: + virtual void bake() override; + +signals: + void originalTextureLoaded(); + +private slots: + void processTexture(); + private: void loadTexture(); - void handleTextureNetworkReply(QNetworkReply* requestReply); - - void processTexture(); + void handleTextureNetworkReply(); QUrl _textureURL; QByteArray _originalTexture; diff --git a/tools/oven/src/DomainBaker.cpp b/tools/oven/src/DomainBaker.cpp index 5589df1645..a8fd464790 100644 --- a/tools/oven/src/DomainBaker.cpp +++ b/tools/oven/src/DomainBaker.cpp @@ -18,6 +18,8 @@ #include "Gzip.h" +#include "Oven.h" + #include "DomainBaker.h" DomainBaker::DomainBaker(const QUrl& localModelFileURL, const QString& domainName, @@ -47,40 +49,14 @@ void DomainBaker::bake() { return; } - setupBakerThread(); - - if (hasErrors()) { - return; - } - enumerateEntities(); if (hasErrors()) { return; } - 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(); - } - - if (hasErrors()) { - return; - } - - writeNewEntitiesFile(); - - if (hasErrors()) { - return; - } - - // 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(); + // in case we've baked and re-written all of our entities already, check if we're done + checkIfRewritingComplete(); } void DomainBaker::setupOutputFolder() { @@ -158,15 +134,6 @@ 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() { @@ -190,12 +157,15 @@ void DomainBaker::enumerateEntities() { if (BAKEABLE_MODEL_EXTENSIONS.contains(completeLowerExtension)) { // grab a clean version of the URL without a query or fragment - modelURL.setFragment(QString()); - modelURL.setQuery(QString()); + modelURL = modelURL.adjusted(QUrl::RemoveQuery | QUrl::RemoveFragment); // setup an FBXBaker for this URL, as long as we don't already have one if (!_bakers.contains(modelURL)) { - QSharedPointer baker { new FBXBaker(modelURL, _contentOutputPath), &FBXBaker::deleteLater }; + QSharedPointer baker { + new FBXBaker(modelURL, _contentOutputPath, []() -> QThread* { + return qApp->getNextWorkerThread(); + }), &FBXBaker::deleteLater + }; // make sure our handler is called when the baker is done connect(baker.data(), &Baker::finished, this, &DomainBaker::handleFinishedBaker); @@ -205,7 +175,7 @@ void DomainBaker::enumerateEntities() { // move the baker to the baker thread // and kickoff the bake - baker->moveToThread(_fbxBakerThread.get()); + baker->moveToThread(qApp->getFBXBakerThread()); QMetaObject::invokeMethod(baker.data(), "bake"); } @@ -228,6 +198,7 @@ void DomainBaker::handleFinishedBaker() { if (baker) { if (!baker->hasErrors()) { // this FBXBaker is done and everything went according to plan + qDebug() << "Re-writing entity references to" << baker->getFBXUrl(); // enumerate the QJsonRef values for the URL of this FBX from our multi hash of // entity objects needing a URL re-write @@ -242,12 +213,42 @@ void DomainBaker::handleFinishedBaker() { // setup a new URL using the prefix we were passed QUrl newModelURL = _destinationPath.resolved(baker->getBakedFBXRelativePath()); - // copy the fragment and query from the old model URL + // copy the fragment and query, and user info from the old model URL newModelURL.setQuery(oldModelURL.query()); newModelURL.setFragment(oldModelURL.fragment()); + newModelURL.setUserInfo(oldModelURL.userInfo()); // set the new model URL as the value in our temp QJsonObject entity[ENTITY_MODEL_URL_KEY] = newModelURL.toString(); + + // check if the entity also had an animation at the same URL + // in which case it should be replaced with our baked model URL too + const QString ENTITY_ANIMATION_KEY = "animation"; + const QString ENTITIY_ANIMATION_URL_KEY = "url"; + + if (entity.contains(ENTITY_ANIMATION_KEY) + && entity[ENTITY_ANIMATION_KEY].toObject().contains(ENTITIY_ANIMATION_URL_KEY)) { + auto animationValue = entity[ENTITY_ANIMATION_KEY]; + auto animationObject = animationValue.toObject(); + + // grab the old animation URL + QUrl oldAnimationURL { animationObject[ENTITIY_ANIMATION_URL_KEY].toString() }; + + // check if its stripped down version matches our stripped down model URL + if (oldAnimationURL.matches(oldModelURL, QUrl::RemoveUserInfo | QUrl::RemoveQuery | QUrl::RemoveFragment)) { + // the animation URL matched the old model URL, so make the animation URL point to the baked FBX + // with its original query and fragment + auto newAnimationURL = _destinationPath.resolved(baker->getBakedFBXRelativePath()); + newAnimationURL.setQuery(oldAnimationURL.query()); + newAnimationURL.setFragment(oldAnimationURL.fragment()); + newAnimationURL.setUserInfo(oldAnimationURL.userInfo()); + + animationObject[ENTITIY_ANIMATION_URL_KEY] = newAnimationURL.toString(); + + // replace the animation object referenced by the QJsonValueRef + animationValue = animationObject; + } + } // replace our temp object with the value referenced by our QJsonValueRef entityValue = entity; @@ -267,9 +268,21 @@ void DomainBaker::handleFinishedBaker() { // emit progress to tell listeners how many models we have baked emit bakeProgress(_totalNumberOfEntities - _entitiesNeedingRewrite.keys().size(), _totalNumberOfEntities); - if (_entitiesNeedingRewrite.isEmpty()) { - emit allModelsFinished(); + // check if this was the last model we needed to re-write and if we are done now + checkIfRewritingComplete(); + } +} + +void DomainBaker::checkIfRewritingComplete() { + if (_entitiesNeedingRewrite.isEmpty()) { + writeNewEntitiesFile(); + + if (hasErrors()) { + return; } + + // we've now written out our new models file - time to say that we are finished up + emit finished(); } } @@ -308,6 +321,5 @@ void DomainBaker::writeNewEntitiesFile() { } qDebug() << "Exported entities file with baked model URLs to" << bakedEntitiesFilePath; - qDebug() << "WARNINGS:" << _warningList; } diff --git a/tools/oven/src/DomainBaker.h b/tools/oven/src/DomainBaker.h index 23c332abab..ddcb3cd006 100644 --- a/tools/oven/src/DomainBaker.h +++ b/tools/oven/src/DomainBaker.h @@ -23,24 +23,25 @@ class DomainBaker : public Baker { Q_OBJECT public: + // 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 from the same process on the same thread. + // That means you must pass a usable running QThread when constructing a domain baker. DomainBaker(const QUrl& localEntitiesFileURL, const QString& domainName, const QString& baseOutputPath, const QUrl& destinationPath); -public: - virtual void bake() override; - signals: void allModelsFinished(); void bakeProgress(int modelsBaked, int modelsTotal); private slots: + virtual void bake() override; void handleFinishedBaker(); private: void setupOutputFolder(); void loadLocalFile(); - void setupBakerThread(); void enumerateEntities(); + void checkIfRewritingComplete(); void writeNewEntitiesFile(); QUrl _localEntitiesFileURL; @@ -52,11 +53,10 @@ private: QJsonArray _entities; - std::unique_ptr _fbxBakerThread; QHash> _bakers; QMultiHash _entitiesNeedingRewrite; - int _totalNumberOfEntities; + int _totalNumberOfEntities { 0 }; }; #endif // hifi_DomainBaker_h diff --git a/tools/oven/src/Oven.cpp b/tools/oven/src/Oven.cpp index 60754759f4..ac8ef505ba 100644 --- a/tools/oven/src/Oven.cpp +++ b/tools/oven/src/Oven.cpp @@ -10,6 +10,7 @@ // #include +#include #include @@ -33,5 +34,67 @@ Oven::Oven(int argc, char* argv[]) : // setup the GUI _mainWindow = new OvenMainWindow; _mainWindow->show(); + + // setup our worker threads + setupWorkerThreads(QThread::idealThreadCount() - 1); + + // Autodesk's SDK means that we need a single thread for all FBX importing/exporting in the same process + // setup the FBX Baker thread + setupFBXBakerThread(); +} + +Oven::~Oven() { + // cleanup the worker threads + for (auto i = 0; i < _workerThreads.size(); ++i) { + _workerThreads[i]->quit(); + _workerThreads[i]->wait(); + } + + // cleanup the FBX Baker thread + _fbxBakerThread->quit(); + _fbxBakerThread->wait(); +} + +void Oven::setupWorkerThreads(int numWorkerThreads) { + for (auto i = 0; i < numWorkerThreads; ++i) { + // setup a worker thread yet and add it to our concurrent vector + auto newThread = new QThread(this); + newThread->setObjectName("Oven Worker Thread " + QString::number(i + 1)); + + _workerThreads.push_back(newThread); + } +} + +void Oven::setupFBXBakerThread() { + // we're being asked for the FBX baker thread, but we don't have one yet + // so set that up now + _fbxBakerThread = new QThread(this); + _fbxBakerThread->setObjectName("Oven FBX Baker Thread"); +} + +QThread* Oven::getFBXBakerThread() { + if (!_fbxBakerThread->isRunning()) { + // start the FBX baker thread if it isn't running yet + _fbxBakerThread->start(); + } + + return _fbxBakerThread; +} + +QThread* Oven::getNextWorkerThread() { + // Here we replicate some of the functionality of QThreadPool by giving callers an available worker thread to use. + // We can't use QThreadPool because we want to put QObjects with signals/slots on these threads. + // So instead we setup our own list of threads, up to one less than the ideal thread count + // (for the FBX Baker Thread to have room), and cycle through them to hand a usable running thread back to our callers. + + auto nextIndex = ++_nextWorkerThreadIndex; + auto nextThread = _workerThreads[nextIndex % _workerThreads.size()]; + + // start the thread if it isn't running yet + if (!nextThread->isRunning()) { + nextThread->start(); + } + + return nextThread; } diff --git a/tools/oven/src/Oven.h b/tools/oven/src/Oven.h index 3fc9a4c0f6..bf7f478b83 100644 --- a/tools/oven/src/Oven.h +++ b/tools/oven/src/Oven.h @@ -14,6 +14,8 @@ #include +#include + #if defined(qApp) #undef qApp #endif @@ -26,11 +28,23 @@ class Oven : public QApplication { public: Oven(int argc, char* argv[]); + ~Oven(); OvenMainWindow* getMainWindow() const { return _mainWindow; } + QThread* getFBXBakerThread(); + QThread* getNextWorkerThread(); + private: + void setupWorkerThreads(int numWorkerThreads); + void setupFBXBakerThread(); + OvenMainWindow* _mainWindow; + QThread* _fbxBakerThread; + QList _workerThreads; + + std::atomic _nextWorkerThreadIndex; + int _numWorkerThreads; }; diff --git a/tools/oven/src/ui/DomainBakeWidget.cpp b/tools/oven/src/ui/DomainBakeWidget.cpp index 9f33dbcc29..cd2d9f8e3c 100644 --- a/tools/oven/src/ui/DomainBakeWidget.cpp +++ b/tools/oven/src/ui/DomainBakeWidget.cpp @@ -216,8 +216,12 @@ void DomainBakeWidget::bakeButtonClicked() { // watch the baker's progress so that we can put its progress in the results table connect(domainBaker.get(), &DomainBaker::bakeProgress, this, &DomainBakeWidget::handleBakerProgress); - // run the baker in our thread pool - QtConcurrent::run(domainBaker.get(), &DomainBaker::bake); + // move the baker to the next available Oven worker thread + auto nextThread = qApp->getNextWorkerThread(); + domainBaker->moveToThread(nextThread); + + // kickoff the domain baker on its thread + QMetaObject::invokeMethod(domainBaker.get(), "bake"); // add a pending row to the results window to show that this bake is in process auto resultsWindow = qApp->getMainWindow()->showResultsWindow(); @@ -232,7 +236,7 @@ void DomainBakeWidget::bakeButtonClicked() { void DomainBakeWidget::handleBakerProgress(int modelsBaked, int modelsTotal) { if (auto baker = qobject_cast(sender())) { // add the results of this bake to the results window - auto it = std::remove_if(_bakers.begin(), _bakers.end(), [baker](const BakerRowPair& value) { + auto it = std::find_if(_bakers.begin(), _bakers.end(), [baker](const BakerRowPair& value) { return value.first.get() == baker; }); @@ -251,7 +255,7 @@ void DomainBakeWidget::handleBakerProgress(int modelsBaked, int modelsTotal) { void DomainBakeWidget::handleFinishedBaker() { if (auto baker = qobject_cast(sender())) { // add the results of this bake to the results window - auto it = std::remove_if(_bakers.begin(), _bakers.end(), [baker](const BakerRowPair& value) { + auto it = std::find_if(_bakers.begin(), _bakers.end(), [baker](const BakerRowPair& value) { return value.first.get() == baker; }); @@ -260,12 +264,21 @@ void DomainBakeWidget::handleFinishedBaker() { auto resultsWindow = qApp->getMainWindow()->showResultsWindow(); if (baker->hasErrors()) { - resultsWindow->changeStatusForRow(resultRow, baker->getErrors().join("\n")); + auto errors = baker->getErrors(); + errors.removeDuplicates(); + + resultsWindow->changeStatusForRow(resultRow, errors.join("\n")); } else if (baker->hasWarnings()) { - resultsWindow->changeStatusForRow(resultRow, baker->getWarnings().join("\n")); + auto warnings = baker->getWarnings(); + warnings.removeDuplicates(); + + resultsWindow->changeStatusForRow(resultRow, warnings.join("\n")); } else { resultsWindow->changeStatusForRow(resultRow, "Success"); } + + // remove the DomainBaker now that it has completed + _bakers.erase(it); } } } diff --git a/tools/oven/src/ui/ModelBakeWidget.cpp b/tools/oven/src/ui/ModelBakeWidget.cpp index 08b5402821..2841262fee 100644 --- a/tools/oven/src/ui/ModelBakeWidget.cpp +++ b/tools/oven/src/ui/ModelBakeWidget.cpp @@ -183,7 +183,11 @@ void ModelBakeWidget::bakeButtonClicked() { } // everything seems to be in place, kick off a bake for this model now - auto baker = std::unique_ptr { new FBXBaker(modelToBakeURL, outputDirectory.absolutePath(), false) }; + auto baker = std::unique_ptr { + new FBXBaker(modelToBakeURL, outputDirectory.absolutePath(), []() -> QThread* { + return qApp->getNextWorkerThread(); + }, false) + }; // move the baker to the baker thread baker->moveToThread(_bakerThread);