From e912500cc2ac5dd0892c9ab9f67ec390bb150054 Mon Sep 17 00:00:00 2001 From: Clement Date: Fri, 12 Apr 2019 09:10:12 -0700 Subject: [PATCH 1/9] Fix oven integration in the asset server --- assignment-client/src/assets/AssetServer.cpp | 134 +++++++++++------- assignment-client/src/assets/AssetServer.h | 7 +- .../src/assets/BakeAssetTask.cpp | 34 +++-- assignment-client/src/assets/BakeAssetTask.h | 2 +- 4 files changed, 111 insertions(+), 66 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index c2aec9b058..88f81f639b 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -1326,12 +1326,40 @@ void AssetServer::handleFailedBake(QString originalAssetHash, QString assetPath, } void AssetServer::handleCompletedBake(QString originalAssetHash, QString originalAssetPath, - QString bakedTempOutputDir, QVector bakedFilePaths) { + QString bakedTempOutputDir) { bool errorCompletingBake { false }; QString errorReason; qDebug() << "Completing bake for " << originalAssetHash; + + + QDir outputDir(bakedTempOutputDir); + auto directories = outputDir.entryList(QDir::Dirs | QDir::NoDotAndDotDot); + assert(directories.size() == 1); + QString bakedDirectoryPath; + for (const auto& dirName : directories) { + outputDir.cd(dirName); + if (outputDir.exists("baked") && outputDir.exists("original")) { + bakedDirectoryPath = outputDir.filePath("baked"); + } + outputDir.cdUp(); + } + + assert(!bakedDirectoryPath.isEmpty()); + + QDirIterator it(bakedDirectoryPath, QDirIterator::Subdirectories); + QVector bakedFilePaths; + while (it.hasNext()) { + it.next(); + if (it.fileInfo().isFile()) { + bakedFilePaths.push_back(it.filePath()); + } + } + + QDir bakedDirectory(bakedDirectoryPath); + QString redirectTarget; + for (auto& filePath : bakedFilePaths) { // figure out the hash for the contents of this file QFile file(filePath); @@ -1340,62 +1368,59 @@ void AssetServer::handleCompletedBake(QString originalAssetHash, QString origina AssetUtils::AssetHash bakedFileHash; - if (file.open(QIODevice::ReadOnly)) { - QCryptographicHash hasher(QCryptographicHash::Sha256); - - if (hasher.addData(&file)) { - bakedFileHash = hasher.result().toHex(); - } else { - // stop handling this bake, couldn't hash the contents of the file - errorCompletingBake = true; - errorReason = "Failed to finalize bake"; - break; - } - - // first check that we don't already have this bake file in our list - auto bakeFileDestination = _filesDirectory.absoluteFilePath(bakedFileHash); - if (!QFile::exists(bakeFileDestination)) { - // copy each to our files folder (with the hash as their filename) - if (!file.copy(_filesDirectory.absoluteFilePath(bakedFileHash))) { - // stop handling this bake, couldn't copy the bake file into our files directory - errorCompletingBake = true; - errorReason = "Failed to copy baked assets to asset server"; - break; - } - } - - // setup the mapping for this bake file - auto relativeFilePath = QUrl(filePath).fileName(); - qDebug() << "Relative file path is: " << relativeFilePath; - if (relativeFilePath.endsWith(".fbx", Qt::CaseInsensitive)) { - // for an FBX file, we replace the filename with the simple name - // (to handle the case where two mapped assets have the same hash but different names) - relativeFilePath = BAKED_ASSET_SIMPLE_FBX_NAME; - } else if (relativeFilePath.endsWith(".js", Qt::CaseInsensitive)) { - relativeFilePath = BAKED_ASSET_SIMPLE_JS_NAME; - } else if (!originalAssetPath.endsWith(".fbx", Qt::CaseInsensitive)) { - relativeFilePath = BAKED_ASSET_SIMPLE_TEXTURE_NAME; - } - - QString bakeMapping = getBakeMapping(originalAssetHash, relativeFilePath); - - // add a mapping (under the hidden baked folder) for this file resulting from the bake - if (setMapping(bakeMapping, bakedFileHash)) { - qDebug() << "Added" << bakeMapping << "for bake file" << bakedFileHash << "from bake of" << originalAssetHash; - } else { - qDebug() << "Failed to set mapping"; - // stop handling this bake, couldn't add a mapping for this bake file - errorCompletingBake = true; - errorReason = "Failed to finalize bake"; - break; - } - } else { + if (!file.open(QIODevice::ReadOnly)) { qDebug() << "Failed to open baked file: " << filePath; // stop handling this bake, we couldn't open one of the files for reading errorCompletingBake = true; errorReason = "Failed to finalize bake"; break; } + + QCryptographicHash hasher(QCryptographicHash::Sha256); + + if (!hasher.addData(&file)) { + // stop handling this bake, couldn't hash the contents of the file + errorCompletingBake = true; + errorReason = "Failed to finalize bake"; + break; + } + + bakedFileHash = hasher.result().toHex(); + + // first check that we don't already have this bake file in our list + auto bakeFileDestination = _filesDirectory.absoluteFilePath(bakedFileHash); + if (!QFile::exists(bakeFileDestination)) { + // copy each to our files folder (with the hash as their filename) + if (!file.copy(_filesDirectory.absoluteFilePath(bakedFileHash))) { + // stop handling this bake, couldn't copy the bake file into our files directory + errorCompletingBake = true; + errorReason = "Failed to copy baked assets to asset server"; + break; + } + } + + // setup the mapping for this bake file + auto relativeFilePath = bakedDirectory.relativeFilePath(filePath); + qDebug() << "Relative file path is: " << relativeFilePath; + + QString bakeMapping = getBakeMapping(originalAssetHash, relativeFilePath); + + // Check if this is the file we should redirect to when someone asks for the original asset + if ((relativeFilePath.endsWith(".baked.fst", Qt::CaseInsensitive) && originalAssetPath.endsWith(".fbx")) || + (relativeFilePath.endsWith(".texmeta.json", Qt::CaseInsensitive) && !originalAssetPath.endsWith(".fbx"))) { + redirectTarget = bakeMapping; + } + + // add a mapping (under the hidden baked folder) for this file resulting from the bake + if (!setMapping(bakeMapping, bakedFileHash)) { + qDebug() << "Failed to set mapping"; + // stop handling this bake, couldn't add a mapping for this bake file + errorCompletingBake = true; + errorReason = "Failed to finalize bake"; + break; + } + + qDebug() << "Added" << bakeMapping << "for bake file" << bakedFileHash << "from bake of" << originalAssetHash; } for (auto& filePath : bakedFilePaths) { @@ -1411,9 +1436,12 @@ void AssetServer::handleCompletedBake(QString originalAssetHash, QString origina auto type = assetTypeForFilename(originalAssetPath); auto currentTypeVersion = currentBakeVersionForAssetType(type); + assert(!redirectTarget.isEmpty()); + AssetMeta meta; meta.bakeVersion = currentTypeVersion; meta.failedLastBake = errorCompletingBake; + meta.redirectTarget = redirectTarget; if (errorCompletingBake) { qWarning() << "Could not complete bake for" << originalAssetHash; @@ -1435,6 +1463,7 @@ void AssetServer::handleAbortedBake(QString originalAssetHash, QString assetPath static const QString BAKE_VERSION_KEY = "bake_version"; static const QString FAILED_LAST_BAKE_KEY = "failed_last_bake"; static const QString LAST_BAKE_ERRORS_KEY = "last_bake_errors"; +static const QString REDIRECT_TARGET_KEY = "redirect_target"; std::pair AssetServer::readMetaFile(AssetUtils::AssetHash hash) { auto metaFilePath = AssetUtils::HIDDEN_BAKED_CONTENT_FOLDER + hash + "/" + "meta.json"; @@ -1461,6 +1490,7 @@ std::pair AssetServer::readMetaFile(AssetUtils::AssetHash hash) auto bakeVersion = root[BAKE_VERSION_KEY]; auto failedLastBake = root[FAILED_LAST_BAKE_KEY]; auto lastBakeErrors = root[LAST_BAKE_ERRORS_KEY]; + auto redirectTarget = root[REDIRECT_TARGET_KEY]; if (bakeVersion.isDouble() && failedLastBake.isBool() @@ -1470,6 +1500,7 @@ std::pair AssetServer::readMetaFile(AssetUtils::AssetHash hash) meta.bakeVersion = bakeVersion.toInt(); meta.failedLastBake = failedLastBake.toBool(); meta.lastBakeErrors = lastBakeErrors.toString(); + meta.redirectTarget = redirectTarget.toString(); return { true, meta }; } else { @@ -1488,6 +1519,7 @@ bool AssetServer::writeMetaFile(AssetUtils::AssetHash originalAssetHash, const A metaFileObject[BAKE_VERSION_KEY] = (int)meta.bakeVersion; metaFileObject[FAILED_LAST_BAKE_KEY] = meta.failedLastBake; metaFileObject[LAST_BAKE_ERRORS_KEY] = meta.lastBakeErrors; + metaFileObject[REDIRECT_TARGET_KEY] = meta.redirectTarget; QJsonDocument metaFileDoc; metaFileDoc.setObject(metaFileObject); diff --git a/assignment-client/src/assets/AssetServer.h b/assignment-client/src/assets/AssetServer.h index b3d0f18a8f..fe84df5141 100644 --- a/assignment-client/src/assets/AssetServer.h +++ b/assignment-client/src/assets/AssetServer.h @@ -62,12 +62,10 @@ enum class ScriptBakeVersion : BakeVersion { }; struct AssetMeta { - AssetMeta() { - } - BakeVersion bakeVersion { INITIAL_BAKE_VERSION }; bool failedLastBake { false }; QString lastBakeErrors; + QString redirectTarget; }; class BakeAssetTask; @@ -139,8 +137,7 @@ private: void bakeAsset(const AssetUtils::AssetHash& assetHash, const AssetUtils::AssetPath& assetPath, const QString& filePath); /// Move baked content for asset to baked directory and update baked status - void handleCompletedBake(QString originalAssetHash, QString assetPath, QString bakedTempOutputDir, - QVector bakedFilePaths); + void handleCompletedBake(QString originalAssetHash, QString assetPath, QString bakedTempOutputDir); void handleFailedBake(QString originalAssetHash, QString assetPath, QString errors); void handleAbortedBake(QString originalAssetHash, QString assetPath); diff --git a/assignment-client/src/assets/BakeAssetTask.cpp b/assignment-client/src/assets/BakeAssetTask.cpp index ecb4ede5d8..7ca87fe4e3 100644 --- a/assignment-client/src/assets/BakeAssetTask.cpp +++ b/assignment-client/src/assets/BakeAssetTask.cpp @@ -57,12 +57,19 @@ void BakeAssetTask::run() { return; } + // Make a new temporary directory for the Oven to work in QString tempOutputDir = PathUtils::generateTemporaryDir(); + + // Copy file to bake the temporary dir and give a name the oven can work with + auto assetName = _assetPath.split("/").last(); + auto tempAssetPath = tempOutputDir + "/" + assetName; + QFile::copy(_filePath, tempAssetPath); + auto base = QFileInfo(QCoreApplication::applicationFilePath()).absoluteDir(); QString path = base.absolutePath() + "/oven"; QString extension = _assetPath.mid(_assetPath.lastIndexOf('.') + 1); QStringList args { - "-i", _filePath, + "-i", tempAssetPath, "-o", tempOutputDir, "-t", extension, }; @@ -72,7 +79,7 @@ void BakeAssetTask::run() { QEventLoop loop; connect(_ovenProcess.get(), static_cast(&QProcess::finished), - this, [&loop, this, tempOutputDir](int exitCode, QProcess::ExitStatus exitStatus) { + this, [&loop, this, tempOutputDir, tempAssetPath](int exitCode, QProcess::ExitStatus exitStatus) { qDebug() << "Baking process finished: " << exitCode << exitStatus; if (exitStatus == QProcess::CrashExit) { @@ -82,18 +89,20 @@ void BakeAssetTask::run() { QString errors = "Fatal error occurred while baking"; emit bakeFailed(_assetHash, _assetPath, errors); } - } else if (exitCode == OVEN_STATUS_CODE_SUCCESS) { - QDir outputDir = tempOutputDir; - auto files = outputDir.entryInfoList(QDir::Files); - QVector outputFiles; - for (auto& file : files) { - outputFiles.push_back(file.absoluteFilePath()); + if (!QDir(tempOutputDir).rmdir(".")) { + qWarning() << "Failed to remove temporary directory:" << tempOutputDir; } + } else if (exitCode == OVEN_STATUS_CODE_SUCCESS) { + // Remove temp copy of the original asset + QFile::remove(tempAssetPath); - emit bakeComplete(_assetHash, _assetPath, tempOutputDir, outputFiles); + emit bakeComplete(_assetHash, _assetPath, tempOutputDir); } else if (exitStatus == QProcess::NormalExit && exitCode == OVEN_STATUS_CODE_ABORT) { _wasAborted.store(true); emit bakeAborted(_assetHash, _assetPath); + if (!QDir(tempOutputDir).rmdir(".")) { + qWarning() << "Failed to remove temporary directory:" << tempOutputDir; + } } else { QString errors; if (exitCode == OVEN_STATUS_CODE_FAIL) { @@ -108,6 +117,9 @@ void BakeAssetTask::run() { } } emit bakeFailed(_assetHash, _assetPath, errors); + if (!QDir(tempOutputDir).rmdir(".")) { + qWarning() << "Failed to remove temporary directory:" << tempOutputDir; + } } loop.quit(); @@ -115,9 +127,13 @@ void BakeAssetTask::run() { qDebug() << "Starting oven for " << _assetPath; _ovenProcess->start(path, args, QIODevice::ReadOnly); + qDebug() << "Running:" << path << args; if (!_ovenProcess->waitForStarted(-1)) { QString errors = "Oven process failed to start"; emit bakeFailed(_assetHash, _assetPath, errors); + if (!QDir(tempOutputDir).rmdir(".")) { + qWarning() << "Failed to remove temporary directory:" << tempOutputDir; + } return; } diff --git a/assignment-client/src/assets/BakeAssetTask.h b/assignment-client/src/assets/BakeAssetTask.h index 24b070d08a..2d50a26bc1 100644 --- a/assignment-client/src/assets/BakeAssetTask.h +++ b/assignment-client/src/assets/BakeAssetTask.h @@ -37,7 +37,7 @@ public slots: void abort(); signals: - void bakeComplete(QString assetHash, QString assetPath, QString tempOutputDir, QVector outputFiles); + void bakeComplete(QString assetHash, QString assetPath, QString tempOutputDir); void bakeFailed(QString assetHash, QString assetPath, QString errors); void bakeAborted(QString assetHash, QString assetPath); From dc41fea46e66e4764d748825a8892f4f0f3d4ae0 Mon Sep 17 00:00:00 2001 From: Clement Date: Fri, 12 Apr 2019 11:26:40 -0700 Subject: [PATCH 2/9] Update Asset Server redirect and baking conditions --- assignment-client/src/assets/AssetServer.cpp | 132 +++++++++---------- 1 file changed, 60 insertions(+), 72 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 88f81f639b..4805d2fe4c 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -107,6 +107,10 @@ BakeVersion currentBakeVersionForAssetType(BakedAssetType type) { } } +QString getBakeMapping(const AssetUtils::AssetHash& hash, const QString& relativeFilePath) { + return AssetUtils::HIDDEN_BAKED_CONTENT_FOLDER + hash + "/" + relativeFilePath; +} + const QString ASSET_SERVER_LOGGING_TARGET_NAME = "asset-server"; void AssetServer::bakeAsset(const AssetUtils::AssetHash& assetHash, const AssetUtils::AssetPath& assetPath, const QString& filePath) { @@ -141,26 +145,22 @@ std::pair AssetServer::getAssetStatus(const A return { AssetUtils::Baked, "" }; } - auto dotIndex = path.lastIndexOf("."); - if (dotIndex == -1) { + BakedAssetType type = assetTypeForFilename(path); + + if (type == BakedAssetType::Undefined) { return { AssetUtils::Irrelevant, "" }; } - auto extension = path.mid(dotIndex + 1); + bool loaded; + AssetMeta meta; + std::tie(loaded, meta) = readMetaFile(hash); - QString bakedFilename; - - if (BAKEABLE_MODEL_EXTENSIONS.contains(extension)) { - bakedFilename = BAKED_MODEL_SIMPLE_NAME; - } else if (BAKEABLE_TEXTURE_EXTENSIONS.contains(extension.toLocal8Bit()) && hasMetaFile(hash)) { - bakedFilename = BAKED_TEXTURE_SIMPLE_NAME; - } else if (BAKEABLE_SCRIPT_EXTENSIONS.contains(extension)) { - bakedFilename = BAKED_SCRIPT_SIMPLE_NAME; - } else { - return { AssetUtils::Irrelevant, "" }; + QString bakedFilename = bakedFilenameForAssetType(type); + auto bakedPath = getBakeMapping(hash, bakedFilename); + if (loaded && !meta.redirectTarget.isEmpty()) { + bakedPath = meta.redirectTarget; } - auto bakedPath = AssetUtils::HIDDEN_BAKED_CONTENT_FOLDER + hash + "/" + bakedFilename; auto jt = _fileMappings.find(bakedPath); if (jt != _fileMappings.end()) { if (jt->second == hash) { @@ -168,14 +168,8 @@ std::pair AssetServer::getAssetStatus(const A } else { return { AssetUtils::Baked, "" }; } - } else { - bool loaded; - AssetMeta meta; - - std::tie(loaded, meta) = readMetaFile(hash); - if (loaded && meta.failedLastBake) { - return { AssetUtils::Error, meta.lastBakeErrors }; - } + } else if (loaded && meta.failedLastBake) { + return { AssetUtils::Error, meta.lastBakeErrors }; } return { AssetUtils::Pending, "" }; @@ -227,8 +221,16 @@ bool AssetServer::needsToBeBaked(const AssetUtils::AssetPath& path, const AssetU return false; } + bool loaded; + AssetMeta meta; + std::tie(loaded, meta) = readMetaFile(assetHash); + QString bakedFilename = bakedFilenameForAssetType(type); - auto bakedPath = AssetUtils::HIDDEN_BAKED_CONTENT_FOLDER + assetHash + "/" + bakedFilename; + auto bakedPath = getBakeMapping(assetHash, bakedFilename); + if (loaded && !meta.redirectTarget.isEmpty()) { + bakedPath = meta.redirectTarget; + } + auto mappingIt = _fileMappings.find(bakedPath); bool bakedMappingExists = mappingIt != _fileMappings.end(); @@ -238,10 +240,6 @@ bool AssetServer::needsToBeBaked(const AssetUtils::AssetPath& path, const AssetU return false; } - bool loaded; - AssetMeta meta; - std::tie(loaded, meta) = readMetaFile(assetHash); - if (type == BakedAssetType::Texture && !loaded) { return false; } @@ -633,36 +631,33 @@ void AssetServer::handleGetMappingOperation(ReceivedMessage& message, NLPacketLi if (it != _fileMappings.end()) { // check if we should re-direct to a baked asset - - // first, figure out from the mapping extension what type of file this is - auto assetPathExtension = assetPath.mid(assetPath.lastIndexOf('.') + 1).toLower(); - - auto type = assetTypeForFilename(assetPath); - QString bakedRootFile = bakedFilenameForAssetType(type); - auto originalAssetHash = it->second; QString redirectedAssetHash; - QString bakedAssetPath; quint8 wasRedirected = false; bool bakingDisabled = false; - if (!bakedRootFile.isEmpty()) { - // we ran into an asset for which we could have a baked version, let's check if it's ready - bakedAssetPath = AssetUtils::HIDDEN_BAKED_CONTENT_FOLDER + originalAssetHash + "/" + bakedRootFile; - auto bakedIt = _fileMappings.find(bakedAssetPath); + bool loaded; + AssetMeta meta; + std::tie(loaded, meta) = readMetaFile(originalAssetHash); - if (bakedIt != _fileMappings.end()) { - if (bakedIt->second != originalAssetHash) { - qDebug() << "Did find baked version for: " << originalAssetHash << assetPath; - // we found a baked version of the requested asset to serve, redirect to that - redirectedAssetHash = bakedIt->second; - wasRedirected = true; - } else { - qDebug() << "Did not find baked version for: " << originalAssetHash << assetPath << " (disabled)"; - bakingDisabled = true; - } + auto type = assetTypeForFilename(assetPath); + QString bakedRootFile = bakedFilenameForAssetType(type); + QString bakedAssetPath = getBakeMapping(originalAssetHash, bakedRootFile); + + if (loaded && !meta.redirectTarget.isEmpty()) { + bakedAssetPath = meta.redirectTarget; + } + + auto bakedIt = _fileMappings.find(bakedAssetPath); + if (bakedIt != _fileMappings.end()) { + if (bakedIt->second != originalAssetHash) { + qDebug() << "Did find baked version for: " << originalAssetHash << assetPath; + // we found a baked version of the requested asset to serve, redirect to that + redirectedAssetHash = bakedIt->second; + wasRedirected = true; } else { - qDebug() << "Did not find baked version for: " << originalAssetHash << assetPath; + qDebug() << "Did not find baked version for: " << originalAssetHash << assetPath << " (disabled)"; + bakingDisabled = true; } } @@ -684,20 +679,13 @@ void AssetServer::handleGetMappingOperation(ReceivedMessage& message, NLPacketLi auto query = QUrlQuery(url.query()); bool isSkybox = query.hasQueryItem("skybox"); - if (isSkybox) { - bool loaded; - AssetMeta meta; - std::tie(loaded, meta) = readMetaFile(originalAssetHash); - - if (!loaded) { - AssetMeta needsBakingMeta; - needsBakingMeta.bakeVersion = NEEDS_BAKING_BAKE_VERSION; - - writeMetaFile(originalAssetHash, needsBakingMeta); - if (!bakingDisabled) { - maybeBake(assetPath, originalAssetHash); - } + if (isSkybox && !loaded) { + AssetMeta needsBakingMeta; + needsBakingMeta.bakeVersion = NEEDS_BAKING_BAKE_VERSION; + writeMetaFile(originalAssetHash, needsBakingMeta); + if (!bakingDisabled) { + maybeBake(assetPath, originalAssetHash); } } } @@ -1297,14 +1285,6 @@ bool AssetServer::renameMapping(AssetUtils::AssetPath oldPath, AssetUtils::Asset } } -static const QString BAKED_ASSET_SIMPLE_FBX_NAME = "asset.fbx"; -static const QString BAKED_ASSET_SIMPLE_TEXTURE_NAME = "texture.ktx"; -static const QString BAKED_ASSET_SIMPLE_JS_NAME = "asset.js"; - -QString getBakeMapping(const AssetUtils::AssetHash& hash, const QString& relativeFilePath) { - return AssetUtils::HIDDEN_BAKED_CONTENT_FOLDER + hash + "/" + relativeFilePath; -} - void AssetServer::handleFailedBake(QString originalAssetHash, QString assetPath, QString errors) { qDebug() << "Failed to bake: " << originalAssetHash << assetPath << "(" << errors << ")"; @@ -1553,10 +1533,18 @@ bool AssetServer::setBakingEnabled(const AssetUtils::AssetPathList& paths, bool if (type == BakedAssetType::Undefined) { continue; } - QString bakedFilename = bakedFilenameForAssetType(type); auto hash = it->second; + + bool loaded; + AssetMeta meta; + std::tie(loaded, meta) = readMetaFile(hash); + + QString bakedFilename = bakedFilenameForAssetType(type); auto bakedMapping = getBakeMapping(hash, bakedFilename); + if (loaded && !meta.redirectTarget.isEmpty()) { + bakedMapping = meta.redirectTarget; + } auto it = _fileMappings.find(bakedMapping); bool currentlyDisabled = (it != _fileMappings.end() && it->second == hash); From 77d8a3691412733f10fa9f0be87752abc518581a Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Thu, 11 Apr 2019 16:41:31 -0700 Subject: [PATCH 3/9] attempt to handle atp redirects from .fbx to .baked.fst --- .../src/material-networking/TextureCache.cpp | 6 +- .../src/model-networking/ModelCache.cpp | 315 ++++++++---------- .../src/model-networking/ModelCache.h | 43 ++- 3 files changed, 171 insertions(+), 193 deletions(-) diff --git a/libraries/material-networking/src/material-networking/TextureCache.cpp b/libraries/material-networking/src/material-networking/TextureCache.cpp index 6af59930fa..8ae5b12286 100644 --- a/libraries/material-networking/src/material-networking/TextureCache.cpp +++ b/libraries/material-networking/src/material-networking/TextureCache.cpp @@ -630,11 +630,9 @@ void NetworkTexture::makeLocalRequest() { } bool NetworkTexture::handleFailedRequest(ResourceRequest::Result result) { - if (_currentlyLoadingResourceType != ResourceType::KTX - && result == ResourceRequest::Result::RedirectFail) { - + if (_shouldFailOnRedirect && result == ResourceRequest::Result::RedirectFail) { auto newPath = _request->getRelativePathUrl(); - if (newPath.fileName().endsWith(".ktx")) { + if (newPath.fileName().toLower().endsWith(".ktx")) { _currentlyLoadingResourceType = ResourceType::KTX; _activeUrl = newPath; _shouldFailOnRedirect = false; diff --git a/libraries/model-networking/src/model-networking/ModelCache.cpp b/libraries/model-networking/src/model-networking/ModelCache.cpp index 23b365dd03..fe242335e7 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.cpp +++ b/libraries/model-networking/src/model-networking/ModelCache.cpp @@ -31,8 +31,6 @@ Q_LOGGING_CATEGORY(trace_resource_parse_geometry, "trace.resource.parse.geometry") -class GeometryReader; - class GeometryExtra { public: const GeometryMappingPair& mapping; @@ -87,113 +85,6 @@ namespace std { }; } -QUrl resolveTextureBaseUrl(const QUrl& url, const QUrl& textureBaseUrl) { - return textureBaseUrl.isValid() ? textureBaseUrl : url; -} - -class GeometryMappingResource : public GeometryResource { - Q_OBJECT -public: - GeometryMappingResource(const QUrl& url) : GeometryResource(url) {}; - - QString getType() const override { return "GeometryMapping"; } - - virtual void downloadFinished(const QByteArray& data) override; - -private slots: - void onGeometryMappingLoaded(bool success); - -private: - GeometryResource::Pointer _geometryResource; - QMetaObject::Connection _connection; -}; - -void GeometryMappingResource::downloadFinished(const QByteArray& data) { - PROFILE_ASYNC_BEGIN(resource_parse_geometry, "GeometryMappingResource::downloadFinished", _url.toString(), - { { "url", _url.toString() } }); - - // store parsed contents of FST file - _mapping = FSTReader::readMapping(data); - - QString filename = _mapping.value("filename").toString(); - - if (filename.isNull()) { - finishedLoading(false); - } else { - const QString baseURL = _mapping.value("baseURL").toString(); - const QUrl base = _effectiveBaseURL.resolved(baseURL); - QUrl url = base.resolved(filename); - - QString texdir = _mapping.value(TEXDIR_FIELD).toString(); - if (!texdir.isNull()) { - if (!texdir.endsWith('/')) { - texdir += '/'; - } - _textureBaseUrl = resolveTextureBaseUrl(url, base.resolved(texdir)); - } else { - _textureBaseUrl = url.resolved(QUrl(".")); - } - - auto scripts = FSTReader::getScripts(base, _mapping); - if (scripts.size() > 0) { - _mapping.remove(SCRIPT_FIELD); - for (auto &scriptPath : scripts) { - _mapping.insertMulti(SCRIPT_FIELD, scriptPath); - } - } - - auto animGraphVariant = _mapping.value("animGraphUrl"); - - if (animGraphVariant.isValid()) { - QUrl fstUrl(animGraphVariant.toString()); - if (fstUrl.isValid()) { - _animGraphOverrideUrl = base.resolved(fstUrl); - } else { - _animGraphOverrideUrl = QUrl(); - } - } else { - _animGraphOverrideUrl = QUrl(); - } - - auto modelCache = DependencyManager::get(); - GeometryExtra extra { GeometryMappingPair(base, _mapping), _textureBaseUrl, false }; - - // Get the raw GeometryResource - _geometryResource = modelCache->getResource(url, QUrl(), &extra, std::hash()(extra)).staticCast(); - // Avoid caching nested resources - their references will be held by the parent - _geometryResource->_isCacheable = false; - - if (_geometryResource->isLoaded()) { - onGeometryMappingLoaded(!_geometryResource->getURL().isEmpty()); - } else { - if (_connection) { - disconnect(_connection); - } - - _connection = connect(_geometryResource.data(), &Resource::finished, - this, &GeometryMappingResource::onGeometryMappingLoaded); - } - } -} - -void GeometryMappingResource::onGeometryMappingLoaded(bool success) { - if (success && _geometryResource) { - _hfmModel = _geometryResource->_hfmModel; - _materialMapping = _geometryResource->_materialMapping; - _meshParts = _geometryResource->_meshParts; - _meshes = _geometryResource->_meshes; - _materials = _geometryResource->_materials; - - // Avoid holding onto extra references - _geometryResource.reset(); - // Make sure connection will not trigger again - disconnect(_connection); // FIXME Should not have to do this - } - - PROFILE_ASYNC_END(resource_parse_geometry, "GeometryMappingResource::downloadFinished", _url.toString()); - finishedLoading(success); -} - class GeometryReader : public QRunnable { public: GeometryReader(const ModelLoader& modelLoader, QWeakPointer& resource, const QUrl& url, const GeometryMappingPair& mapping, @@ -300,47 +191,137 @@ void GeometryReader::run() { } } -class GeometryDefinitionResource : public GeometryResource { - Q_OBJECT -public: - GeometryDefinitionResource(const ModelLoader& modelLoader, const QUrl& url) : GeometryResource(url), _modelLoader(modelLoader) {} - GeometryDefinitionResource(const GeometryDefinitionResource& other) : - GeometryResource(other), - _modelLoader(other._modelLoader), - _mapping(other._mapping), - _combineParts(other._combineParts) {} +QUrl resolveTextureBaseUrl(const QUrl& url, const QUrl& textureBaseUrl) { + return textureBaseUrl.isValid() ? textureBaseUrl : url; +} - QString getType() const override { return "GeometryDefinition"; } +GeometryResource::GeometryResource(const GeometryResource& other) : + Resource(other), + Geometry(other), + _modelLoader(other._modelLoader), + _mappingPair(other._mappingPair), + _textureBaseURL(other._textureBaseURL), + _combineParts(other._combineParts), + _isCacheable(other._isCacheable) +{ + if (other._geometryResource) { + _startedLoading = false; + } +} - virtual void downloadFinished(const QByteArray& data) override; +void GeometryResource::downloadFinished(const QByteArray& data) { + if (_activeUrl.fileName().toLower().endsWith(".fst")) { + PROFILE_ASYNC_BEGIN(resource_parse_geometry, "GeometryResource::downloadFinished", _url.toString(), { { "url", _url.toString() } }); - void setExtra(void* extra) override; + // store parsed contents of FST file + _mapping = FSTReader::readMapping(data); -protected: - Q_INVOKABLE void setGeometryDefinition(HFMModel::Pointer hfmModel, const GeometryMappingPair& mapping); + QString filename = _mapping.value("filename").toString(); -private: - ModelLoader _modelLoader; - GeometryMappingPair _mapping; - bool _combineParts; -}; + if (filename.isNull()) { + finishedLoading(false); + } else { + const QString baseURL = _mapping.value("baseURL").toString(); + const QUrl base = _effectiveBaseURL.resolved(baseURL); + QUrl url = base.resolved(filename); -void GeometryDefinitionResource::setExtra(void* extra) { + QString texdir = _mapping.value(TEXDIR_FIELD).toString(); + if (!texdir.isNull()) { + if (!texdir.endsWith('/')) { + texdir += '/'; + } + _textureBaseURL = resolveTextureBaseUrl(url, base.resolved(texdir)); + } else { + _textureBaseURL = url.resolved(QUrl(".")); + } + + auto scripts = FSTReader::getScripts(base, _mapping); + if (scripts.size() > 0) { + _mapping.remove(SCRIPT_FIELD); + for (auto &scriptPath : scripts) { + _mapping.insertMulti(SCRIPT_FIELD, scriptPath); + } + } + + auto animGraphVariant = _mapping.value("animGraphUrl"); + + if (animGraphVariant.isValid()) { + QUrl fstUrl(animGraphVariant.toString()); + if (fstUrl.isValid()) { + _animGraphOverrideUrl = base.resolved(fstUrl); + } else { + _animGraphOverrideUrl = QUrl(); + } + } else { + _animGraphOverrideUrl = QUrl(); + } + + auto modelCache = DependencyManager::get(); + GeometryExtra extra { GeometryMappingPair(base, _mapping), _textureBaseURL, false }; + + // Get the raw GeometryResource + _geometryResource = modelCache->getResource(url, QUrl(), &extra, std::hash()(extra)).staticCast(); + // Avoid caching nested resources - their references will be held by the parent + _geometryResource->_isCacheable = false; + + if (_geometryResource->isLoaded()) { + onGeometryMappingLoaded(!_geometryResource->getURL().isEmpty()); + } else { + if (_connection) { + disconnect(_connection); + } + + _connection = connect(_geometryResource.data(), &Resource::finished, this, &GeometryResource::onGeometryMappingLoaded); + } + } + } else { + if (_url != _effectiveBaseURL) { + _url = _effectiveBaseURL; + _textureBaseURL = _effectiveBaseURL; + } + QThreadPool::globalInstance()->start(new GeometryReader(_modelLoader, _self, _effectiveBaseURL, _mappingPair, data, _combineParts, _request->getWebMediaType())); + } +} + +bool GeometryResource::handleFailedRequest(ResourceRequest::Result result) { + if (_shouldFailOnRedirect && result == ResourceRequest::Result::RedirectFail) { + auto newPath = _request->getRelativePathUrl(); + if (newPath.fileName().toLower().endsWith(".fst")) { + _activeUrl = newPath; + _shouldFailOnRedirect = false; + makeRequest(); + return true; + } + } + return Resource::handleFailedRequest(result); +} + +void GeometryResource::onGeometryMappingLoaded(bool success) { + if (success && _geometryResource) { + _hfmModel = _geometryResource->_hfmModel; + _materialMapping = _geometryResource->_materialMapping; + _meshParts = _geometryResource->_meshParts; + _meshes = _geometryResource->_meshes; + _materials = _geometryResource->_materials; + + // Avoid holding onto extra references + _geometryResource.reset(); + // Make sure connection will not trigger again + disconnect(_connection); // FIXME Should not have to do this + } + + PROFILE_ASYNC_END(resource_parse_geometry, "GeometryResource::downloadFinished", _url.toString()); + finishedLoading(success); +} + +void GeometryResource::setExtra(void* extra) { const GeometryExtra* geometryExtra = static_cast(extra); - _mapping = geometryExtra ? geometryExtra->mapping : GeometryMappingPair(QUrl(), QVariantHash()); - _textureBaseUrl = geometryExtra ? resolveTextureBaseUrl(_url, geometryExtra->textureBaseUrl) : QUrl(); + _mappingPair = geometryExtra ? geometryExtra->mapping : GeometryMappingPair(QUrl(), QVariantHash()); + _textureBaseURL = geometryExtra ? resolveTextureBaseUrl(_url, geometryExtra->textureBaseUrl) : QUrl(); _combineParts = geometryExtra ? geometryExtra->combineParts : true; } -void GeometryDefinitionResource::downloadFinished(const QByteArray& data) { - if (_url != _effectiveBaseURL) { - _url = _effectiveBaseURL; - _textureBaseUrl = _effectiveBaseURL; - } - QThreadPool::globalInstance()->start(new GeometryReader(_modelLoader, _self, _effectiveBaseURL, _mapping, data, _combineParts, _request->getWebMediaType())); -} - -void GeometryDefinitionResource::setGeometryDefinition(HFMModel::Pointer hfmModel, const GeometryMappingPair& mapping) { +void GeometryResource::setGeometryDefinition(HFMModel::Pointer hfmModel, const GeometryMappingPair& mapping) { // Do processing on the model baker::Baker modelBaker(hfmModel, mapping.second, mapping.first); modelBaker.run(); @@ -353,7 +334,7 @@ void GeometryDefinitionResource::setGeometryDefinition(HFMModel::Pointer hfmMode QHash materialIDAtlas; for (const HFMMaterial& material : _hfmModel->materials) { materialIDAtlas[material.materialID] = _materials.size(); - _materials.push_back(std::make_shared(material, _textureBaseUrl)); + _materials.push_back(std::make_shared(material, _textureBaseURL)); } std::shared_ptr meshes = std::make_shared(); @@ -376,6 +357,23 @@ void GeometryDefinitionResource::setGeometryDefinition(HFMModel::Pointer hfmMode finishedLoading(true); } +void GeometryResource::deleter() { + resetTextures(); + Resource::deleter(); +} + +void GeometryResource::setTextures() { + if (_hfmModel) { + for (const HFMMaterial& material : _hfmModel->materials) { + _materials.push_back(std::make_shared(material, _textureBaseURL)); + } + } +} + +void GeometryResource::resetTextures() { + _materials.clear(); +} + ModelCache::ModelCache() { const qint64 GEOMETRY_DEFAULT_UNUSED_MAX_SIZE = DEFAULT_UNUSED_MAX_SIZE; setUnusedResourceCacheSize(GEOMETRY_DEFAULT_UNUSED_MAX_SIZE); @@ -388,26 +386,14 @@ ModelCache::ModelCache() { } QSharedPointer ModelCache::createResource(const QUrl& url) { - Resource* resource = nullptr; - if (url.path().toLower().endsWith(".fst")) { - resource = new GeometryMappingResource(url); - } else { - resource = new GeometryDefinitionResource(_modelLoader, url); - } - - return QSharedPointer(resource, &Resource::deleter); + return QSharedPointer(new GeometryResource(url, _modelLoader), &GeometryResource::deleter); } QSharedPointer ModelCache::createResourceCopy(const QSharedPointer& resource) { - if (resource->getURL().path().toLower().endsWith(".fst")) { - return QSharedPointer(new GeometryMappingResource(*resource.staticCast()), &Resource::deleter); - } else { - return QSharedPointer(new GeometryDefinitionResource(*resource.staticCast()), &Resource::deleter); - } + return QSharedPointer(new GeometryResource(*resource.staticCast()), &GeometryResource::deleter); } -GeometryResource::Pointer ModelCache::getGeometryResource(const QUrl& url, - const GeometryMappingPair& mapping, const QUrl& textureBaseUrl) { +GeometryResource::Pointer ModelCache::getGeometryResource(const QUrl& url, const GeometryMappingPair& mapping, const QUrl& textureBaseUrl) { bool combineParts = true; GeometryExtra geometryExtra = { mapping, textureBaseUrl, combineParts }; GeometryResource::Pointer resource = getResource(url, QUrl(), &geometryExtra, std::hash()(geometryExtra)).staticCast(); @@ -513,23 +499,6 @@ const std::shared_ptr Geometry::getShapeMaterial(int partID) co return nullptr; } -void GeometryResource::deleter() { - resetTextures(); - Resource::deleter(); -} - -void GeometryResource::setTextures() { - if (_hfmModel) { - for (const HFMMaterial& material : _hfmModel->materials) { - _materials.push_back(std::make_shared(material, _textureBaseUrl)); - } - } -} - -void GeometryResource::resetTextures() { - _materials.clear(); -} - void GeometryResourceWatcher::startWatching() { connect(_resource.data(), &Resource::finished, this, &GeometryResourceWatcher::resourceFinished); connect(_resource.data(), &Resource::onRefresh, this, &GeometryResourceWatcher::resourceRefreshed); diff --git a/libraries/model-networking/src/model-networking/ModelCache.h b/libraries/model-networking/src/model-networking/ModelCache.h index ca1ceaff16..93a3d811b3 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.h +++ b/libraries/model-networking/src/model-networking/ModelCache.h @@ -24,8 +24,6 @@ class MeshPart; -class GeometryMappingResource; - using GeometryMappingPair = std::pair; Q_DECLARE_METATYPE(GeometryMappingPair) @@ -60,8 +58,6 @@ public: const QVariantHash& getMapping() const { return _mapping; } protected: - friend class GeometryMappingResource; - // Shared across all geometries, constant throughout lifetime std::shared_ptr _hfmModel; MaterialMapping _materialMapping; @@ -80,23 +76,30 @@ private: /// A geometry loaded from the network. class GeometryResource : public Resource, public Geometry { + Q_OBJECT public: using Pointer = QSharedPointer; - GeometryResource(const QUrl& url) : Resource(url) {} - GeometryResource(const GeometryResource& other) : - Resource(other), - Geometry(other), - _textureBaseUrl(other._textureBaseUrl), - _isCacheable(other._isCacheable) {} + GeometryResource(const QUrl& url, const ModelLoader& modelLoader) : Resource(url), _modelLoader(modelLoader) { _shouldFailOnRedirect = !url.fileName().toLower().endsWith(".fst"); } + GeometryResource(const GeometryResource& other); - virtual bool areTexturesLoaded() const override { return isLoaded() && Geometry::areTexturesLoaded(); } + QString getType() const override { return "Geometry"; } virtual void deleter() override; + virtual void downloadFinished(const QByteArray& data) override; + bool handleFailedRequest(ResourceRequest::Result result) override; + void setExtra(void* extra) override; + + virtual bool areTexturesLoaded() const override { return isLoaded() && Geometry::areTexturesLoaded(); } + +private slots: + void onGeometryMappingLoaded(bool success); + protected: friend class ModelCache; - friend class GeometryMappingResource; + + Q_INVOKABLE void setGeometryDefinition(HFMModel::Pointer hfmModel, const GeometryMappingPair& mapping); // Geometries may not hold onto textures while cached - that is for the texture cache // Instead, these methods clear and reset textures from the geometry when caching/loading @@ -104,10 +107,18 @@ protected: void setTextures(); void resetTextures(); - QUrl _textureBaseUrl; - virtual bool isCacheable() const override { return _loaded && _isCacheable; } - bool _isCacheable { true }; + +private: + ModelLoader _modelLoader; + GeometryMappingPair _mappingPair; + QUrl _textureBaseURL; + bool _combineParts; + + GeometryResource::Pointer _geometryResource; + QMetaObject::Connection _connection; + + bool _isCacheable{ true }; }; class GeometryResourceWatcher : public QObject { @@ -158,7 +169,7 @@ public: const QUrl& textureBaseUrl = QUrl()); protected: - friend class GeometryMappingResource; + friend class GeometryResource; virtual QSharedPointer createResource(const QUrl& url) override; QSharedPointer createResourceCopy(const QSharedPointer& resource) override; From 794cedaa6070c87dc7fd45d11af959384d72974d Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Fri, 12 Apr 2019 15:49:34 -0700 Subject: [PATCH 4/9] Patch interface redirect code --- .../src/model-networking/ModelCache.cpp | 15 +-------------- .../src/model-networking/ModelCache.h | 3 +-- libraries/networking/src/ResourceCache.cpp | 1 + 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/libraries/model-networking/src/model-networking/ModelCache.cpp b/libraries/model-networking/src/model-networking/ModelCache.cpp index fe242335e7..a837599b57 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.cpp +++ b/libraries/model-networking/src/model-networking/ModelCache.cpp @@ -210,7 +210,7 @@ GeometryResource::GeometryResource(const GeometryResource& other) : } void GeometryResource::downloadFinished(const QByteArray& data) { - if (_activeUrl.fileName().toLower().endsWith(".fst")) { + if (_effectiveBaseURL.fileName().toLower().endsWith(".fst")) { PROFILE_ASYNC_BEGIN(resource_parse_geometry, "GeometryResource::downloadFinished", _url.toString(), { { "url", _url.toString() } }); // store parsed contents of FST file @@ -283,19 +283,6 @@ void GeometryResource::downloadFinished(const QByteArray& data) { } } -bool GeometryResource::handleFailedRequest(ResourceRequest::Result result) { - if (_shouldFailOnRedirect && result == ResourceRequest::Result::RedirectFail) { - auto newPath = _request->getRelativePathUrl(); - if (newPath.fileName().toLower().endsWith(".fst")) { - _activeUrl = newPath; - _shouldFailOnRedirect = false; - makeRequest(); - return true; - } - } - return Resource::handleFailedRequest(result); -} - void GeometryResource::onGeometryMappingLoaded(bool success) { if (success && _geometryResource) { _hfmModel = _geometryResource->_hfmModel; diff --git a/libraries/model-networking/src/model-networking/ModelCache.h b/libraries/model-networking/src/model-networking/ModelCache.h index 93a3d811b3..b14dcd7199 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.h +++ b/libraries/model-networking/src/model-networking/ModelCache.h @@ -80,7 +80,7 @@ class GeometryResource : public Resource, public Geometry { public: using Pointer = QSharedPointer; - GeometryResource(const QUrl& url, const ModelLoader& modelLoader) : Resource(url), _modelLoader(modelLoader) { _shouldFailOnRedirect = !url.fileName().toLower().endsWith(".fst"); } + GeometryResource(const QUrl& url, const ModelLoader& modelLoader) : Resource(url), _modelLoader(modelLoader) {} GeometryResource(const GeometryResource& other); QString getType() const override { return "Geometry"; } @@ -88,7 +88,6 @@ public: virtual void deleter() override; virtual void downloadFinished(const QByteArray& data) override; - bool handleFailedRequest(ResourceRequest::Result result) override; void setExtra(void* extra) override; virtual bool areTexturesLoaded() const override { return isLoaded() && Geometry::areTexturesLoaded(); } diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index d5abb27a27..746c28a306 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -577,6 +577,7 @@ Resource::Resource(const Resource& other) : Resource::Resource(const QUrl& url) : _url(url), _activeUrl(url), + _effectiveBaseURL(url), _requestID(++requestID) { init(); } From d19b1c18a05c29612961b90f74474df71e95b3ae Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Wed, 27 Mar 2019 15:31:22 -0700 Subject: [PATCH 5/9] Fix MaterialBaker not including names in baked multi-materials --- libraries/graphics/src/graphics/Material.h | 1 + .../src/material-networking/MaterialCache.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/libraries/graphics/src/graphics/Material.h b/libraries/graphics/src/graphics/Material.h index d24e906f98..80b247bed0 100755 --- a/libraries/graphics/src/graphics/Material.h +++ b/libraries/graphics/src/graphics/Material.h @@ -318,6 +318,7 @@ public: void setTextureTransforms(const Transform& transform, MaterialMappingMode mode, bool repeat); const std::string& getName() const { return _name; } + void setName(const std::string& name) { _name = name; } const std::string& getModel() const { return _model; } void setModel(const std::string& model) { _model = model; } diff --git a/libraries/material-networking/src/material-networking/MaterialCache.cpp b/libraries/material-networking/src/material-networking/MaterialCache.cpp index 9eef89d5c9..5a5f4ab54b 100644 --- a/libraries/material-networking/src/material-networking/MaterialCache.cpp +++ b/libraries/material-networking/src/material-networking/MaterialCache.cpp @@ -184,6 +184,7 @@ std::pair> NetworkMaterialResource auto nameJSON = materialJSON.value(key); if (nameJSON.isString()) { name = nameJSON.toString().toStdString(); + material->setName(name); } } else if (key == "model") { auto modelJSON = materialJSON.value(key); From c5b7bdc802096d9264f731cddd422a2e7f339fc1 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Fri, 12 Apr 2019 17:51:03 -0700 Subject: [PATCH 6/9] Fix unix warning --- libraries/networking/src/ResourceCache.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 746c28a306..44d3d1ee4d 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -576,8 +576,8 @@ Resource::Resource(const Resource& other) : Resource::Resource(const QUrl& url) : _url(url), - _activeUrl(url), _effectiveBaseURL(url), + _activeUrl(url), _requestID(++requestID) { init(); } From ea8bc72bbf25762ae4f274c9cfc06586df57c46e Mon Sep 17 00:00:00 2001 From: Clement Date: Mon, 15 Apr 2019 19:08:44 -0700 Subject: [PATCH 7/9] Fix Asset Server reporting bad status for textures --- assignment-client/src/assets/AssetServer.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 4805d2fe4c..297a622c25 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -146,7 +146,6 @@ std::pair AssetServer::getAssetStatus(const A } BakedAssetType type = assetTypeForFilename(path); - if (type == BakedAssetType::Undefined) { return { AssetUtils::Irrelevant, "" }; } @@ -155,6 +154,12 @@ std::pair AssetServer::getAssetStatus(const A AssetMeta meta; std::tie(loaded, meta) = readMetaFile(hash); + // We create a meta file for Skyboxes at runtime when they get requested + // Otherwise, textures don't get baked by themselves. + if (type == BakedAssetType::Texture && !loaded) { + return { AssetUtils::Irrelevant, "" }; + } + QString bakedFilename = bakedFilenameForAssetType(type); auto bakedPath = getBakeMapping(hash, bakedFilename); if (loaded && !meta.redirectTarget.isEmpty()) { @@ -240,6 +245,8 @@ bool AssetServer::needsToBeBaked(const AssetUtils::AssetPath& path, const AssetU return false; } + // We create a meta file for Skyboxes at runtime when they get requested + // Otherwise, textures don't get baked by themselves. if (type == BakedAssetType::Texture && !loaded) { return false; } From 03b28b3dfaabbcfe18724b4f62d980a0f9f09e13 Mon Sep 17 00:00:00 2001 From: Clement Date: Tue, 16 Apr 2019 19:40:59 -0700 Subject: [PATCH 8/9] Add error reporting + Make temp dir erase safer --- assignment-client/src/assets/AssetServer.cpp | 91 ++++++++++++------- .../src/assets/BakeAssetTask.cpp | 54 +++++------ libraries/shared/src/PathUtils.cpp | 24 +++++ libraries/shared/src/PathUtils.h | 1 + 4 files changed, 102 insertions(+), 68 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 297a622c25..502cf15aa2 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -1314,27 +1314,57 @@ void AssetServer::handleFailedBake(QString originalAssetHash, QString assetPath, void AssetServer::handleCompletedBake(QString originalAssetHash, QString originalAssetPath, QString bakedTempOutputDir) { + auto reportCompletion = [this, originalAssetPath, originalAssetHash](bool errorCompletingBake, + QString errorReason, + QString redirectTarget) { + auto type = assetTypeForFilename(originalAssetPath); + auto currentTypeVersion = currentBakeVersionForAssetType(type); + + AssetMeta meta; + meta.bakeVersion = currentTypeVersion; + meta.failedLastBake = errorCompletingBake; + meta.redirectTarget = redirectTarget; + + if (errorCompletingBake) { + qWarning() << "Could not complete bake for" << originalAssetHash; + meta.lastBakeErrors = errorReason; + } + + writeMetaFile(originalAssetHash, meta); + + _pendingBakes.remove(originalAssetHash); + }; + bool errorCompletingBake { false }; QString errorReason; + QString redirectTarget; qDebug() << "Completing bake for " << originalAssetHash; - - + // Find the directory containing the baked content QDir outputDir(bakedTempOutputDir); + QString outputDirName = outputDir.dirName(); auto directories = outputDir.entryList(QDir::Dirs | QDir::NoDotAndDotDot); - assert(directories.size() == 1); QString bakedDirectoryPath; for (const auto& dirName : directories) { outputDir.cd(dirName); if (outputDir.exists("baked") && outputDir.exists("original")) { bakedDirectoryPath = outputDir.filePath("baked"); + break; } outputDir.cdUp(); } + if (bakedDirectoryPath.isEmpty()) { + errorCompletingBake = true; + errorReason = "Failed to find baking output"; - assert(!bakedDirectoryPath.isEmpty()); + // Cleanup temporary output directory + PathUtils::deleteMyTemporaryDir(outputDirName); + reportCompletion(errorCompletingBake, errorReason, redirectTarget); + return; + } + // Compile list of all the baked files QDirIterator it(bakedDirectoryPath, QDirIterator::Subdirectories); QVector bakedFilePaths; while (it.hasNext()) { @@ -1343,9 +1373,17 @@ void AssetServer::handleCompletedBake(QString originalAssetHash, QString origina bakedFilePaths.push_back(it.filePath()); } } + if (bakedFilePaths.isEmpty()) { + errorCompletingBake = true; + errorReason = "Baking output has no files"; + + // Cleanup temporary output directory + PathUtils::deleteMyTemporaryDir(outputDirName); + reportCompletion(errorCompletingBake, errorReason, redirectTarget); + return; + } QDir bakedDirectory(bakedDirectoryPath); - QString redirectTarget; for (auto& filePath : bakedFilePaths) { // figure out the hash for the contents of this file @@ -1359,7 +1397,7 @@ void AssetServer::handleCompletedBake(QString originalAssetHash, QString origina qDebug() << "Failed to open baked file: " << filePath; // stop handling this bake, we couldn't open one of the files for reading errorCompletingBake = true; - errorReason = "Failed to finalize bake"; + errorReason = "Could not open baked file " + file.fileName(); break; } @@ -1368,7 +1406,7 @@ void AssetServer::handleCompletedBake(QString originalAssetHash, QString origina if (!hasher.addData(&file)) { // stop handling this bake, couldn't hash the contents of the file errorCompletingBake = true; - errorReason = "Failed to finalize bake"; + errorReason = "Could not hash data for " + file.fileName(); break; } @@ -1388,13 +1426,15 @@ void AssetServer::handleCompletedBake(QString originalAssetHash, QString origina // setup the mapping for this bake file auto relativeFilePath = bakedDirectory.relativeFilePath(filePath); - qDebug() << "Relative file path is: " << relativeFilePath; QString bakeMapping = getBakeMapping(originalAssetHash, relativeFilePath); // Check if this is the file we should redirect to when someone asks for the original asset if ((relativeFilePath.endsWith(".baked.fst", Qt::CaseInsensitive) && originalAssetPath.endsWith(".fbx")) || (relativeFilePath.endsWith(".texmeta.json", Qt::CaseInsensitive) && !originalAssetPath.endsWith(".fbx"))) { + if (!redirectTarget.isEmpty()) { + qWarning() << "Found multiple baked redirect target for" << originalAssetPath; + } redirectTarget = bakeMapping; } @@ -1403,41 +1443,22 @@ void AssetServer::handleCompletedBake(QString originalAssetHash, QString origina qDebug() << "Failed to set mapping"; // stop handling this bake, couldn't add a mapping for this bake file errorCompletingBake = true; - errorReason = "Failed to finalize bake"; + errorReason = "Failed to set mapping for baked file " + file.fileName(); break; } qDebug() << "Added" << bakeMapping << "for bake file" << bakedFileHash << "from bake of" << originalAssetHash; } - for (auto& filePath : bakedFilePaths) { - QFile file(filePath); - if (!file.remove()) { - qWarning() << "Failed to remove temporary file:" << filePath; - } - } - if (!QDir(bakedTempOutputDir).rmdir(".")) { - qWarning() << "Failed to remove temporary directory:" << bakedTempOutputDir; + + if (redirectTarget.isEmpty()) { + errorCompletingBake = true; + errorReason = "Could not find root file for baked output"; } - auto type = assetTypeForFilename(originalAssetPath); - auto currentTypeVersion = currentBakeVersionForAssetType(type); - - assert(!redirectTarget.isEmpty()); - - AssetMeta meta; - meta.bakeVersion = currentTypeVersion; - meta.failedLastBake = errorCompletingBake; - meta.redirectTarget = redirectTarget; - - if (errorCompletingBake) { - qWarning() << "Could not complete bake for" << originalAssetHash; - meta.lastBakeErrors = errorReason; - } - - writeMetaFile(originalAssetHash, meta); - - _pendingBakes.remove(originalAssetHash); + // Cleanup temporary output directory + PathUtils::deleteMyTemporaryDir(outputDirName); + reportCompletion(errorCompletingBake, errorReason, redirectTarget); } void AssetServer::handleAbortedBake(QString originalAssetHash, QString assetPath) { diff --git a/assignment-client/src/assets/BakeAssetTask.cpp b/assignment-client/src/assets/BakeAssetTask.cpp index 7ca87fe4e3..99fff4f180 100644 --- a/assignment-client/src/assets/BakeAssetTask.cpp +++ b/assignment-client/src/assets/BakeAssetTask.cpp @@ -36,21 +36,6 @@ BakeAssetTask::BakeAssetTask(const AssetUtils::AssetHash& assetHash, const Asset }); } -void cleanupTempFiles(QString tempOutputDir, std::vector files) { - for (const auto& filename : files) { - QFile f { filename }; - if (!f.remove()) { - qDebug() << "Failed to remove:" << filename; - } - } - if (!tempOutputDir.isEmpty()) { - QDir dir { tempOutputDir }; - if (!dir.rmdir(".")) { - qDebug() << "Failed to remove temporary directory:" << tempOutputDir; - } - } -}; - void BakeAssetTask::run() { if (_isBaking.exchange(true)) { qWarning() << "Tried to start bake asset task while already baking"; @@ -59,11 +44,24 @@ void BakeAssetTask::run() { // Make a new temporary directory for the Oven to work in QString tempOutputDir = PathUtils::generateTemporaryDir(); + QString tempOutputDirName = QDir(tempOutputDir).dirName(); + if (tempOutputDir.isEmpty()) { + QString errors = "Could not create temporary working directory"; + emit bakeFailed(_assetHash, _assetPath, errors); + PathUtils::deleteMyTemporaryDir(tempOutputDirName); + return; + } // Copy file to bake the temporary dir and give a name the oven can work with auto assetName = _assetPath.split("/").last(); auto tempAssetPath = tempOutputDir + "/" + assetName; - QFile::copy(_filePath, tempAssetPath); + auto sucess = QFile::copy(_filePath, tempAssetPath); + if (!sucess) { + QString errors = "Couldn't copy file to bake to temporary directory"; + emit bakeFailed(_assetHash, _assetPath, errors); + PathUtils::deleteMyTemporaryDir(tempOutputDirName); + return; + } auto base = QFileInfo(QCoreApplication::applicationFilePath()).absoluteDir(); QString path = base.absolutePath() + "/oven"; @@ -79,30 +77,23 @@ void BakeAssetTask::run() { QEventLoop loop; connect(_ovenProcess.get(), static_cast(&QProcess::finished), - this, [&loop, this, tempOutputDir, tempAssetPath](int exitCode, QProcess::ExitStatus exitStatus) { + this, [&loop, this, tempOutputDir, tempAssetPath, tempOutputDirName](int exitCode, QProcess::ExitStatus exitStatus) { qDebug() << "Baking process finished: " << exitCode << exitStatus; if (exitStatus == QProcess::CrashExit) { + PathUtils::deleteMyTemporaryDir(tempOutputDirName); if (_wasAborted) { emit bakeAborted(_assetHash, _assetPath); } else { QString errors = "Fatal error occurred while baking"; emit bakeFailed(_assetHash, _assetPath, errors); } - if (!QDir(tempOutputDir).rmdir(".")) { - qWarning() << "Failed to remove temporary directory:" << tempOutputDir; - } } else if (exitCode == OVEN_STATUS_CODE_SUCCESS) { - // Remove temp copy of the original asset - QFile::remove(tempAssetPath); - emit bakeComplete(_assetHash, _assetPath, tempOutputDir); } else if (exitStatus == QProcess::NormalExit && exitCode == OVEN_STATUS_CODE_ABORT) { _wasAborted.store(true); + PathUtils::deleteMyTemporaryDir(tempOutputDirName); emit bakeAborted(_assetHash, _assetPath); - if (!QDir(tempOutputDir).rmdir(".")) { - qWarning() << "Failed to remove temporary directory:" << tempOutputDir; - } } else { QString errors; if (exitCode == OVEN_STATUS_CODE_FAIL) { @@ -116,10 +107,8 @@ void BakeAssetTask::run() { errors = "Unknown error occurred while baking"; } } + PathUtils::deleteMyTemporaryDir(tempOutputDirName); emit bakeFailed(_assetHash, _assetPath, errors); - if (!QDir(tempOutputDir).rmdir(".")) { - qWarning() << "Failed to remove temporary directory:" << tempOutputDir; - } } loop.quit(); @@ -128,12 +117,11 @@ void BakeAssetTask::run() { qDebug() << "Starting oven for " << _assetPath; _ovenProcess->start(path, args, QIODevice::ReadOnly); qDebug() << "Running:" << path << args; - if (!_ovenProcess->waitForStarted(-1)) { + if (!_ovenProcess->waitForStarted()) { + PathUtils::deleteMyTemporaryDir(tempOutputDirName); + QString errors = "Oven process failed to start"; emit bakeFailed(_assetHash, _assetPath, errors); - if (!QDir(tempOutputDir).rmdir(".")) { - qWarning() << "Failed to remove temporary directory:" << tempOutputDir; - } return; } diff --git a/libraries/shared/src/PathUtils.cpp b/libraries/shared/src/PathUtils.cpp index 60b426e46d..be60533406 100644 --- a/libraries/shared/src/PathUtils.cpp +++ b/libraries/shared/src/PathUtils.cpp @@ -185,6 +185,30 @@ QString PathUtils::generateTemporaryDir() { return ""; } +bool PathUtils::deleteMyTemporaryDir(QString dirName) { + QDir rootTempDir = QDir::tempPath(); + + QString appName = qApp->applicationName(); + QRegularExpression re { "^" + QRegularExpression::escape(appName) + "\\-(?\\d+)\\-(?\\d+)$" }; + + auto match = re.match(dirName); + auto pid = match.capturedRef("pid").toLongLong(); + + if (match.hasMatch() && rootTempDir.exists(dirName) && pid == qApp->applicationPid()) { + auto absoluteDirPath = QDir(rootTempDir.absoluteFilePath(dirName)); + + bool success = absoluteDirPath.removeRecursively(); + if (success) { + qDebug() << " Removing temporary directory: " << absoluteDirPath.absolutePath(); + } else { + qDebug() << " Failed to remove temporary directory: " << absoluteDirPath.absolutePath(); + } + return success; + } + + return false; +} + // Delete all temporary directories for an application int PathUtils::removeTemporaryApplicationDirs(QString appName) { if (appName.isNull()) { diff --git a/libraries/shared/src/PathUtils.h b/libraries/shared/src/PathUtils.h index 0096cb6c90..cac5c58ca4 100644 --- a/libraries/shared/src/PathUtils.h +++ b/libraries/shared/src/PathUtils.h @@ -56,6 +56,7 @@ public: static QString getAppLocalDataFilePath(const QString& filename); static QString generateTemporaryDir(); + static bool deleteMyTemporaryDir(QString dirName); static int removeTemporaryApplicationDirs(QString appName = QString::null); From f371b7b2d9a408b9bcf86c5563dfd8636d1b0f27 Mon Sep 17 00:00:00 2001 From: Clement Date: Wed, 17 Apr 2019 12:57:15 -0700 Subject: [PATCH 9/9] Fix typo --- assignment-client/src/assets/BakeAssetTask.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/assets/BakeAssetTask.cpp b/assignment-client/src/assets/BakeAssetTask.cpp index 99fff4f180..7c845f9b38 100644 --- a/assignment-client/src/assets/BakeAssetTask.cpp +++ b/assignment-client/src/assets/BakeAssetTask.cpp @@ -55,8 +55,8 @@ void BakeAssetTask::run() { // Copy file to bake the temporary dir and give a name the oven can work with auto assetName = _assetPath.split("/").last(); auto tempAssetPath = tempOutputDir + "/" + assetName; - auto sucess = QFile::copy(_filePath, tempAssetPath); - if (!sucess) { + auto success = QFile::copy(_filePath, tempAssetPath); + if (!success) { QString errors = "Couldn't copy file to bake to temporary directory"; emit bakeFailed(_assetHash, _assetPath, errors); PathUtils::deleteMyTemporaryDir(tempOutputDirName);