From e1d79ee8f4a8ed0e1f605d57771897366314d94c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 19 Sep 2017 11:04:01 -0700 Subject: [PATCH 1/2] add abort to Baker and subclasses --- assignment-client/src/assets/AssetServer.cpp | 68 +++++------- assignment-client/src/assets/AssetServer.h | 23 +--- .../src/assets/BakeAssetTask.cpp | 79 ++++++++++++++ assignment-client/src/assets/BakeAssetTask.h | 50 +++++++++ libraries/baking/src/Baker.cpp | 33 +++++- libraries/baking/src/Baker.h | 16 +++ libraries/baking/src/FBXBaker.cpp | 79 +++++++++++--- libraries/baking/src/FBXBaker.h | 6 +- libraries/baking/src/TextureBaker.cpp | 21 +++- libraries/baking/src/TextureBaker.h | 5 + libraries/image/src/image/Image.cpp | 101 +++++++++++------- libraries/image/src/image/Image.h | 54 ++++++---- .../src/model-networking/TextureCache.cpp | 2 +- 13 files changed, 403 insertions(+), 134 deletions(-) create mode 100644 assignment-client/src/assets/BakeAssetTask.cpp create mode 100644 assignment-client/src/assets/BakeAssetTask.h diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index f1e921c768..17aae74dfd 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -35,6 +35,7 @@ #include #include "AssetServerLogging.h" +#include "BakeAssetTask.h" #include "SendAssetTask.h" #include "UploadAssetTask.h" @@ -53,45 +54,6 @@ static QStringList BAKEABLE_TEXTURE_EXTENSIONS; static const QString BAKED_MODEL_SIMPLE_NAME = "asset.fbx"; static const QString BAKED_TEXTURE_SIMPLE_NAME = "texture.ktx"; -BakeAssetTask::BakeAssetTask(const AssetHash& assetHash, const AssetPath& assetPath, const QString& filePath) - : _assetHash(assetHash), _assetPath(assetPath), _filePath(filePath) { -} - -void BakeAssetTask::run() { - _isBaking.store(true); - - qRegisterMetaType >("QVector"); - TextureBakerThreadGetter fn = []() -> QThread* { return QThread::currentThread(); }; - - std::unique_ptr baker; - - if (_assetPath.endsWith(".fbx")) { - baker = std::unique_ptr { - new FBXBaker(QUrl("file:///" + _filePath), fn, PathUtils::generateTemporaryDir()) - }; - } else { - baker = std::unique_ptr { - new TextureBaker(QUrl("file:///" + _filePath), image::TextureUsage::CUBE_TEXTURE, - PathUtils::generateTemporaryDir()) - }; - } - - QEventLoop loop; - connect(baker.get(), &Baker::finished, &loop, &QEventLoop::quit); - QMetaObject::invokeMethod(baker.get(), "bake", Qt::QueuedConnection); - loop.exec(); - - if (baker->hasErrors()) { - qDebug() << "Failed to bake: " << _assetHash << _assetPath << baker->getErrors(); - auto errors = baker->getErrors().join('\n'); // Join error list into a single string for convenience - emit bakeFailed(_assetHash, _assetPath, errors); - } else { - auto vectorOutputFiles = QVector::fromStdVector(baker->getOutputFiles()); - qDebug() << "Finished baking: " << _assetHash << _assetPath << vectorOutputFiles; - emit bakeComplete(_assetHash, _assetPath, vectorOutputFiles); - } -} - void AssetServer::bakeAsset(const AssetHash& assetHash, const AssetPath& assetPath, const QString& filePath) { qDebug() << "Starting bake for: " << assetPath << assetHash; auto it = _pendingBakes.find(assetHash); @@ -102,6 +64,7 @@ void AssetServer::bakeAsset(const AssetHash& assetHash, const AssetPath& assetPa connect(task.get(), &BakeAssetTask::bakeComplete, this, &AssetServer::handleCompletedBake); connect(task.get(), &BakeAssetTask::bakeFailed, this, &AssetServer::handleFailedBake); + connect(task.get(), &BakeAssetTask::bakeAborted, this, &AssetServer::handleAbortedBake); _bakingTaskPool.start(task.get()); } else { @@ -310,6 +273,28 @@ AssetServer::AssetServer(ReceivedMessage& message) : } void AssetServer::aboutToFinish() { + + // remove pending transfer tasks + _transferTaskPool.clear(); + + // abort each of our still running bake tasks, remove pending bakes that were never put on the thread pool + auto it = _pendingBakes.begin(); + while (it != _pendingBakes.end()) { + auto pendingRunnable = _bakingTaskPool.tryTake(it.value().get()); + + if (pendingRunnable) { + _pendingBakes.erase(it); + } else { + it.value()->abort(); + ++it; + } + } + + // make sure all bakers are finished or aborted + while (_pendingBakes.size() > 0) { + QCoreApplication::processEvents(); + } + // re-set defaults in image library image::setColorTexturesCompressionEnabled(_wasCubeTextureCompressionEnabled); image::setGrayscaleTexturesCompressionEnabled(_wasGrayscaleTextureCompressionEnabled); @@ -1247,6 +1232,11 @@ void AssetServer::handleCompletedBake(QString originalAssetHash, QString origina _pendingBakes.remove(originalAssetHash); } +void AssetServer::handleAbortedBake(QString originalAssetHash, QString assetPath) { + // for an aborted bake we don't do anything but remove the BakeAssetTask from our pending bakes + _pendingBakes.remove(originalAssetHash); +} + static const QString BAKE_VERSION_KEY = "bake_version"; static const QString APP_VERSION_KEY = "app_version"; static const QString FAILED_LAST_BAKE_KEY = "failed_last_bake"; diff --git a/assignment-client/src/assets/AssetServer.h b/assignment-client/src/assets/AssetServer.h index 8641f5498f..10ea067ee5 100644 --- a/assignment-client/src/assets/AssetServer.h +++ b/assignment-client/src/assets/AssetServer.h @@ -29,26 +29,6 @@ namespace std { }; } -class BakeAssetTask : public QObject, public QRunnable { - Q_OBJECT -public: - BakeAssetTask(const AssetHash& assetHash, const AssetPath& assetPath, const QString& filePath); - - bool isBaking() { return _isBaking.load(); } - - void run() override; - -signals: - void bakeComplete(QString assetHash, QString assetPath, QVector outputFiles); - void bakeFailed(QString assetHash, QString assetPath, QString errors); - -private: - std::atomic _isBaking { false }; - AssetHash _assetHash; - AssetPath _assetPath; - QString _filePath; -}; - struct AssetMeta { AssetMeta() { } @@ -59,6 +39,8 @@ struct AssetMeta { QString lastBakeErrors; }; +class BakeAssetTask; + class AssetServer : public ThreadedAssignment { Q_OBJECT public: @@ -121,6 +103,7 @@ private: /// Move baked content for asset to baked directory and update baked status void handleCompletedBake(QString originalAssetHash, QString assetPath, QVector bakedFilePaths); void handleFailedBake(QString originalAssetHash, QString assetPath, QString errors); + void handleAbortedBake(QString originalAssetHash, QString assetPath); /// Create meta file to describe baked content for original asset std::pair readMetaFile(AssetHash hash); diff --git a/assignment-client/src/assets/BakeAssetTask.cpp b/assignment-client/src/assets/BakeAssetTask.cpp new file mode 100644 index 0000000000..9073510f79 --- /dev/null +++ b/assignment-client/src/assets/BakeAssetTask.cpp @@ -0,0 +1,79 @@ +// +// BakeAssetTask.cpp +// assignment-client/src/assets +// +// Created by Stephen Birarda on 9/18/17. +// Copyright 2017 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "BakeAssetTask.h" + +#include + +#include +#include + +BakeAssetTask::BakeAssetTask(const AssetHash& assetHash, const AssetPath& assetPath, const QString& filePath) : + _assetHash(assetHash), + _assetPath(assetPath), + _filePath(filePath) +{ + +} + +void BakeAssetTask::run() { + _isBaking.store(true); + + qRegisterMetaType >("QVector"); + TextureBakerThreadGetter fn = []() -> QThread* { return QThread::currentThread(); }; + + if (_assetPath.endsWith(".fbx")) { + _baker = std::unique_ptr { + new FBXBaker(QUrl("file:///" + _filePath), fn, PathUtils::generateTemporaryDir()) + }; + } else { + _baker = std::unique_ptr { + new TextureBaker(QUrl("file:///" + _filePath), image::TextureUsage::CUBE_TEXTURE, + PathUtils::generateTemporaryDir()) + }; + } + + QEventLoop loop; + connect(_baker.get(), &Baker::finished, &loop, &QEventLoop::quit); + connect(_baker.get(), &Baker::aborted, &loop, &QEventLoop::quit); + QMetaObject::invokeMethod(_baker.get(), "bake", Qt::QueuedConnection); + loop.exec(); + + if (_baker->wasAborted()) { + qDebug() << "Aborted baking: " << _assetHash << _assetPath; + + _wasAborted.store(true); + + emit bakeAborted(_assetHash, _assetPath); + } else if (_baker->hasErrors()) { + qDebug() << "Failed to bake: " << _assetHash << _assetPath << _baker->getErrors(); + + auto errors = _baker->getErrors().join('\n'); // Join error list into a single string for convenience + + _didFinish.store(true); + + emit bakeFailed(_assetHash, _assetPath, errors); + } else { + auto vectorOutputFiles = QVector::fromStdVector(_baker->getOutputFiles()); + + qDebug() << "Finished baking: " << _assetHash << _assetPath << vectorOutputFiles; + + _didFinish.store(true); + + emit bakeComplete(_assetHash, _assetPath, vectorOutputFiles); + } +} + +void BakeAssetTask::abort() { + if (_baker) { + _baker->abort(); + } +} diff --git a/assignment-client/src/assets/BakeAssetTask.h b/assignment-client/src/assets/BakeAssetTask.h new file mode 100644 index 0000000000..5c456c4521 --- /dev/null +++ b/assignment-client/src/assets/BakeAssetTask.h @@ -0,0 +1,50 @@ +// +// BakeAssetTask.h +// assignment-client/src/assets +// +// Created by Stephen Birarda on 9/18/17. +// Copyright 2017 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_BakeAssetTask_h +#define hifi_BakeAssetTask_h + +#include +#include +#include + +#include +#include + +class BakeAssetTask : public QObject, public QRunnable { + Q_OBJECT +public: + BakeAssetTask(const AssetHash& assetHash, const AssetPath& assetPath, const QString& filePath); + + bool isBaking() { return _isBaking.load(); } + + void run() override; + + void abort(); + bool wasAborted() const { return _wasAborted.load(); } + bool didFinish() const { return _didFinish.load(); } + +signals: + void bakeComplete(QString assetHash, QString assetPath, QVector outputFiles); + void bakeFailed(QString assetHash, QString assetPath, QString errors); + void bakeAborted(QString assetHash, QString assetPath); + +private: + std::atomic _isBaking { false }; + AssetHash _assetHash; + AssetPath _assetPath; + QString _filePath; + std::unique_ptr _baker; + std::atomic _wasAborted { false }; + std::atomic _didFinish { false }; +}; + +#endif // hifi_BakeAssetTask_h diff --git a/libraries/baking/src/Baker.cpp b/libraries/baking/src/Baker.cpp index c0cbd8d124..2adedf08a1 100644 --- a/libraries/baking/src/Baker.cpp +++ b/libraries/baking/src/Baker.cpp @@ -13,20 +13,49 @@ #include "Baker.h" +bool Baker::shouldStop() { + if (_shouldAbort) { + setWasAborted(true); + return true; + } + + if (hasErrors()) { + return true; + } + + return false; +} + void Baker::handleError(const QString& error) { qCCritical(model_baking).noquote() << error; _errorList.append(error); - emit finished(); + setIsFinished(true); } void Baker::handleErrors(const QStringList& errors) { // we're appending errors, presumably from a baking operation we called // add those to our list and emit that we are finished _errorList.append(errors); - emit finished(); + setIsFinished(true); } void Baker::handleWarning(const QString& warning) { qCWarning(model_baking).noquote() << warning; _warningList.append(warning); } + +void Baker::setIsFinished(bool isFinished) { + _isFinished.store(isFinished); + + if (isFinished) { + emit finished(); + } +} + +void Baker::setWasAborted(bool wasAborted) { + _wasAborted.store(wasAborted); + + if (wasAborted) { + emit aborted(); + } +} diff --git a/libraries/baking/src/Baker.h b/libraries/baking/src/Baker.h index fd76246497..2da315c9fc 100644 --- a/libraries/baking/src/Baker.h +++ b/libraries/baking/src/Baker.h @@ -18,6 +18,8 @@ class Baker : public QObject { Q_OBJECT public: + bool shouldStop(); + bool hasErrors() const { return !_errorList.isEmpty(); } QStringList getErrors() const { return _errorList; } @@ -26,11 +28,20 @@ public: std::vector getOutputFiles() const { return _outputFiles; } + virtual void setIsFinished(bool isFinished); + bool isFinished() const { return _isFinished.load(); } + + virtual void setWasAborted(bool wasAborted); + + bool wasAborted() const { return _wasAborted.load(); } + public slots: virtual void bake() = 0; + virtual void abort() { _shouldAbort.store(true); } signals: void finished(); + void aborted(); protected: void handleError(const QString& error); @@ -44,6 +55,11 @@ protected: QStringList _errorList; QStringList _warningList; + + std::atomic _isFinished { false }; + + std::atomic _shouldAbort { false }; + std::atomic _wasAborted { false }; }; #endif // hifi_Baker_h diff --git a/libraries/baking/src/FBXBaker.cpp b/libraries/baking/src/FBXBaker.cpp index 0f31b878ee..b5b6570599 100644 --- a/libraries/baking/src/FBXBaker.cpp +++ b/libraries/baking/src/FBXBaker.cpp @@ -56,7 +56,19 @@ FBXBaker::FBXBaker(const QUrl& fbxURL, TextureBakerThreadGetter textureThreadGet } +void FBXBaker::abort() { + Baker::abort(); + + // tell our underlying TextureBaker instances to abort + // the FBXBaker will wait until all are aborted before emitting its own abort signal + for (auto& textureBaker : _bakingTextures) { + textureBaker->abort(); + } +} + void FBXBaker::bake() { + qDebug() << "FBXBaker" << _fbxURL << "bake starting"; + auto tempDir = PathUtils::generateTemporaryDir(); if (tempDir.isEmpty()) { @@ -73,7 +85,7 @@ void FBXBaker::bake() { // setup the output folder for the results of this bake setupOutputFolder(); - if (hasErrors()) { + if (shouldStop()) { return; } @@ -87,22 +99,27 @@ void FBXBaker::bakeSourceCopy() { // load the scene from the FBX file importScene(); - if (hasErrors()) { + if (shouldStop()) { return; } // enumerate the models and textures found in the scene and start a bake for them - rewriteAndBakeSceneModels(); rewriteAndBakeSceneTextures(); - if (hasErrors()) { + if (shouldStop()) { + return; + } + + rewriteAndBakeSceneModels(); + + if (shouldStop()) { return; } // export the FBX with re-written texture references exportScene(); - if (hasErrors()) { + if (shouldStop()) { return; } @@ -165,7 +182,6 @@ void FBXBaker::loadSourceFBX() { networkRequest.setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::AlwaysNetwork); networkRequest.setHeader(QNetworkRequest::UserAgentHeader, HIGH_FIDELITY_USER_AGENT); - networkRequest.setUrl(_fbxURL); qCDebug(model_baking) << "Downloading" << _fbxURL; @@ -553,6 +569,11 @@ void FBXBaker::rewriteAndBakeSceneTextures() { while (object != rootChild.children.end()) { if (object->name == "Texture") { + // double check that we didn't get an abort while baking the last texture + if (shouldStop()) { + return; + } + // enumerate the texture children for (FBXNode& textureChild : object->children) { @@ -630,8 +651,9 @@ void FBXBaker::bakeTexture(const QUrl& textureURL, image::TextureUsage::Type tex &TextureBaker::deleteLater }; - // make sure we hear when the baking texture is done + // make sure we hear when the baking texture is done or aborted connect(bakingTexture.data(), &Baker::finished, this, &FBXBaker::handleBakedTexture); + connect(bakingTexture.data(), &TextureBaker::aborted, this, &FBXBaker::handleAbortedTexture); // keep a shared pointer to the baking texture _bakingTextures.insert(textureURL, bakingTexture); @@ -646,7 +668,7 @@ void FBXBaker::handleBakedTexture() { // make sure we haven't already run into errors, and that this is a valid texture if (bakedTexture) { - if (!hasErrors()) { + if (!shouldStop()) { if (!bakedTexture->hasErrors()) { if (!_originalOutputDir.isEmpty()) { // we've been asked to make copies of the originals, so we need to make copies of this if it is a linked texture @@ -698,6 +720,11 @@ void FBXBaker::handleBakedTexture() { // now that this texture has been baked, even though it failed, we can remove that TextureBaker from our list _bakingTextures.remove(bakedTexture->getTextureURL()); + // abort any other ongoing texture bakes since we know we'll end up failing + for (auto& bakingTexture : _bakingTextures) { + bakingTexture->abort(); + } + checkIfTexturesFinished(); } } else { @@ -711,6 +738,25 @@ void FBXBaker::handleBakedTexture() { } } +void FBXBaker::handleAbortedTexture() { + // grab the texture bake that was aborted and remove it from our hash since we don't need to track it anymore + TextureBaker* bakedTexture = qobject_cast(sender()); + + if (bakedTexture) { + _bakingTextures.remove(bakedTexture->getTextureURL()); + } + + // since a texture we were baking aborted, our status is also aborted + _shouldAbort.store(true); + + // abort any other ongoing texture bakes since we know we'll end up failing + for (auto& bakingTexture : _bakingTextures) { + bakingTexture->abort(); + } + + checkIfTexturesFinished(); +} + void FBXBaker::exportScene() { // save the relative path to this FBX inside our passed output folder auto fileName = _fbxURL.fileName(); @@ -735,25 +781,34 @@ void FBXBaker::exportScene() { qCDebug(model_baking) << "Exported" << _fbxURL << "with re-written paths to" << _bakedFBXFilePath; } - void FBXBaker::checkIfTexturesFinished() { // check if we're done everything we need to do for this FBX // and emit our finished signal if we're done if (_bakingTextures.isEmpty()) { - if (hasErrors()) { + if (shouldStop()) { // if we're checking for completion but we have errors // that means one or more of our texture baking operations failed if (_pendingErrorEmission) { - emit finished(); + setIsFinished(true); } return; } else { qCDebug(model_baking) << "Finished baking, emitting finsihed" << _fbxURL; - emit finished(); + setIsFinished(true); + } + } +} + +void FBXBaker::setWasAborted(bool wasAborted) { + if (wasAborted != _wasAborted.load()) { + Baker::setWasAborted(wasAborted); + + if (wasAborted) { + qCDebug(model_baking) << "Aborted baking" << _fbxURL; } } } diff --git a/libraries/baking/src/FBXBaker.h b/libraries/baking/src/FBXBaker.h index c611fb712f..26471a29b3 100644 --- a/libraries/baking/src/FBXBaker.h +++ b/libraries/baking/src/FBXBaker.h @@ -39,10 +39,11 @@ public: QUrl getFBXUrl() const { return _fbxURL; } QString getBakedFBXFilePath() const { return _bakedFBXFilePath; } + virtual void setWasAborted(bool wasAborted) override; + 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; + virtual void abort() override; signals: void sourceCopyReadyToLoad(); @@ -51,6 +52,7 @@ private slots: void bakeSourceCopy(); void handleFBXNetworkReply(); void handleBakedTexture(); + void handleAbortedTexture(); private: void setupOutputFolder(); diff --git a/libraries/baking/src/TextureBaker.cpp b/libraries/baking/src/TextureBaker.cpp index cdf21a0290..febc7ea092 100644 --- a/libraries/baking/src/TextureBaker.cpp +++ b/libraries/baking/src/TextureBaker.cpp @@ -50,6 +50,13 @@ void TextureBaker::bake() { } } +void TextureBaker::abort() { + Baker::abort(); + + // flip our atomic bool so any ongoing texture processing is stopped + _abortProcessing.store(true); +} + const QStringList TextureBaker::getSupportedFormats() { auto formats = QImageReader::supportedImageFormats(); QStringList stringFormats; @@ -111,7 +118,11 @@ void TextureBaker::handleTextureNetworkReply() { void TextureBaker::processTexture() { auto processedTexture = image::processImage(_originalTexture, _textureURL.toString().toStdString(), - ABSOLUTE_MAX_TEXTURE_NUM_PIXELS, _textureType); + ABSOLUTE_MAX_TEXTURE_NUM_PIXELS, _textureType, _abortProcessing); + + if (shouldStop()) { + return; + } if (!processedTexture) { handleError("Could not process texture " + _textureURL.toString()); @@ -145,5 +156,11 @@ void TextureBaker::processTexture() { } qCDebug(model_baking) << "Baked texture" << _textureURL; - emit finished(); + setIsFinished(true); +} + +void TextureBaker::setWasAborted(bool wasAborted) { + Baker::setWasAborted(wasAborted); + + qCDebug(model_baking) << "Aborted baking" << _textureURL; } diff --git a/libraries/baking/src/TextureBaker.h b/libraries/baking/src/TextureBaker.h index 5ea696dda0..e5bd41cf0d 100644 --- a/libraries/baking/src/TextureBaker.h +++ b/libraries/baking/src/TextureBaker.h @@ -39,8 +39,11 @@ public: QString getDestinationFilePath() const { return _outputDirectory.absoluteFilePath(_bakedTextureFileName); } QString getBakedTextureFileName() const { return _bakedTextureFileName; } + virtual void setWasAborted(bool wasAborted) override; + public slots: virtual void bake() override; + virtual void abort() override; signals: void originalTextureLoaded(); @@ -58,6 +61,8 @@ private: QDir _outputDirectory; QString _bakedTextureFileName; + + std::atomic _abortProcessing { false }; }; #endif // hifi_TextureBaker_h diff --git a/libraries/image/src/image/Image.cpp b/libraries/image/src/image/Image.cpp index 30299663de..9d49efcba0 100644 --- a/libraries/image/src/image/Image.cpp +++ b/libraries/image/src/image/Image.cpp @@ -97,52 +97,64 @@ TextureUsage::TextureLoader TextureUsage::getTextureLoaderForType(Type type, con } } -gpu::TexturePointer TextureUsage::createStrict2DTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureColorFromImage(srcImage, srcImageName, true); +gpu::TexturePointer TextureUsage::createStrict2DTextureFromImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureColorFromImage(srcImage, srcImageName, true, abortProcessing); } -gpu::TexturePointer TextureUsage::create2DTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureColorFromImage(srcImage, srcImageName, false); +gpu::TexturePointer TextureUsage::create2DTextureFromImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureColorFromImage(srcImage, srcImageName, false, abortProcessing); } -gpu::TexturePointer TextureUsage::createAlbedoTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureColorFromImage(srcImage, srcImageName, false); +gpu::TexturePointer TextureUsage::createAlbedoTextureFromImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureColorFromImage(srcImage, srcImageName, false, abortProcessing); } -gpu::TexturePointer TextureUsage::createEmissiveTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureColorFromImage(srcImage, srcImageName, false); +gpu::TexturePointer TextureUsage::createEmissiveTextureFromImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureColorFromImage(srcImage, srcImageName, false, abortProcessing); } -gpu::TexturePointer TextureUsage::createLightmapTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureColorFromImage(srcImage, srcImageName, false); +gpu::TexturePointer TextureUsage::createLightmapTextureFromImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureColorFromImage(srcImage, srcImageName, false, abortProcessing); } -gpu::TexturePointer TextureUsage::createNormalTextureFromNormalImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureNormalMapFromImage(srcImage, srcImageName, false); +gpu::TexturePointer TextureUsage::createNormalTextureFromNormalImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureNormalMapFromImage(srcImage, srcImageName, false, abortProcessing); } -gpu::TexturePointer TextureUsage::createNormalTextureFromBumpImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureNormalMapFromImage(srcImage, srcImageName, true); +gpu::TexturePointer TextureUsage::createNormalTextureFromBumpImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureNormalMapFromImage(srcImage, srcImageName, true, abortProcessing); } -gpu::TexturePointer TextureUsage::createRoughnessTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureGrayscaleFromImage(srcImage, srcImageName, false); +gpu::TexturePointer TextureUsage::createRoughnessTextureFromImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureGrayscaleFromImage(srcImage, srcImageName, false, abortProcessing); } -gpu::TexturePointer TextureUsage::createRoughnessTextureFromGlossImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureGrayscaleFromImage(srcImage, srcImageName, true); +gpu::TexturePointer TextureUsage::createRoughnessTextureFromGlossImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureGrayscaleFromImage(srcImage, srcImageName, true, abortProcessing); } -gpu::TexturePointer TextureUsage::createMetallicTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { - return process2DTextureGrayscaleFromImage(srcImage, srcImageName, false); +gpu::TexturePointer TextureUsage::createMetallicTextureFromImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return process2DTextureGrayscaleFromImage(srcImage, srcImageName, false, abortProcessing); } -gpu::TexturePointer TextureUsage::createCubeTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { - return processCubeTextureColorFromImage(srcImage, srcImageName, true); +gpu::TexturePointer TextureUsage::createCubeTextureFromImage(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return processCubeTextureColorFromImage(srcImage, srcImageName, true, abortProcessing); } -gpu::TexturePointer TextureUsage::createCubeTextureFromImageWithoutIrradiance(const QImage& srcImage, const std::string& srcImageName) { - return processCubeTextureColorFromImage(srcImage, srcImageName, false); +gpu::TexturePointer TextureUsage::createCubeTextureFromImageWithoutIrradiance(const QImage& srcImage, const std::string& srcImageName, + const std::atomic& abortProcessing) { + return processCubeTextureColorFromImage(srcImage, srcImageName, false, abortProcessing); } @@ -194,8 +206,9 @@ void setCubeTexturesCompressionEnabled(bool enabled) { compressCubeTextures.store(enabled); } - -gpu::TexturePointer processImage(const QByteArray& content, const std::string& filename, int maxNumPixels, TextureUsage::Type textureType) { +gpu::TexturePointer processImage(const QByteArray& content, const std::string& filename, + int maxNumPixels, TextureUsage::Type textureType, + const std::atomic& abortProcessing) { // Help the QImage loader by extracting the image file format from the url filename ext. // Some tga are not created properly without it. auto filenameExtension = filename.substr(filename.find_last_of('.') + 1); @@ -245,7 +258,7 @@ gpu::TexturePointer processImage(const QByteArray& content, const std::string& f } auto loader = TextureUsage::getTextureLoaderForType(textureType); - auto texture = loader(image, filename); + auto texture = loader(image, filename, abortProcessing); return texture; } @@ -321,7 +334,7 @@ struct MyErrorHandler : public nvtt::ErrorHandler { } }; -void generateMips(gpu::Texture* texture, QImage& image, int face = -1) { +void generateMips(gpu::Texture* texture, QImage& image, const std::atomic& abortProcessing = false, int face = -1) { #if CPU_MIPMAPS PROFILE_RANGE(resource_parse, "generateMips"); @@ -439,14 +452,20 @@ void generateMips(gpu::Texture* texture, QImage& image, int face = -1) { class SequentialTaskDispatcher : public nvtt::TaskDispatcher { public: + SequentialTaskDispatcher(const std::atomic& abortProcessing) : _abortProcessing(abortProcessing) {}; + + const std::atomic& _abortProcessing; + virtual void dispatch(nvtt::Task* task, void* context, int count) override { for (int i = 0; i < count; i++) { - task(context, i); + if (!_abortProcessing.load()) { + task(context, i); + } } } }; - SequentialTaskDispatcher dispatcher; + SequentialTaskDispatcher dispatcher(abortProcessing); nvtt::Compressor compressor; compressor.setTaskDispatcher(&dispatcher); compressor.process(inputOptions, compressionOptions, outputOptions); @@ -482,7 +501,8 @@ void processTextureAlpha(const QImage& srcImage, bool& validAlpha, bool& alphaAs validAlpha = (numOpaques != NUM_PIXELS); } -gpu::TexturePointer TextureUsage::process2DTextureColorFromImage(const QImage& srcImage, const std::string& srcImageName, bool isStrict) { +gpu::TexturePointer TextureUsage::process2DTextureColorFromImage(const QImage& srcImage, const std::string& srcImageName, + bool isStrict, const std::atomic& abortProcessing) { PROFILE_RANGE(resource_parse, "process2DTextureColorFromImage"); QImage image = processSourceImage(srcImage, false); bool validAlpha = image.hasAlphaChannel(); @@ -530,7 +550,7 @@ gpu::TexturePointer TextureUsage::process2DTextureColorFromImage(const QImage& s } theTexture->setUsage(usage.build()); theTexture->setStoredMipFormat(formatMip); - generateMips(theTexture.get(), image); + generateMips(theTexture.get(), image, abortProcessing); } return theTexture; @@ -606,7 +626,8 @@ QImage processBumpMap(QImage& image) { return result; } -gpu::TexturePointer TextureUsage::process2DTextureNormalMapFromImage(const QImage& srcImage, const std::string& srcImageName, bool isBumpMap) { +gpu::TexturePointer TextureUsage::process2DTextureNormalMapFromImage(const QImage& srcImage, const std::string& srcImageName, + bool isBumpMap, const std::atomic& abortProcessing) { PROFILE_RANGE(resource_parse, "process2DTextureNormalMapFromImage"); QImage image = processSourceImage(srcImage, false); @@ -631,13 +652,15 @@ gpu::TexturePointer TextureUsage::process2DTextureNormalMapFromImage(const QImag theTexture = gpu::Texture::create2D(formatGPU, image.width(), image.height(), gpu::Texture::MAX_NUM_MIPS, gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_MIP_LINEAR)); theTexture->setSource(srcImageName); theTexture->setStoredMipFormat(formatMip); - generateMips(theTexture.get(), image); + generateMips(theTexture.get(), image, abortProcessing); } return theTexture; } -gpu::TexturePointer TextureUsage::process2DTextureGrayscaleFromImage(const QImage& srcImage, const std::string& srcImageName, bool isInvertedPixels) { +gpu::TexturePointer TextureUsage::process2DTextureGrayscaleFromImage(const QImage& srcImage, const std::string& srcImageName, + bool isInvertedPixels, + const std::atomic& abortProcessing) { PROFILE_RANGE(resource_parse, "process2DTextureGrayscaleFromImage"); QImage image = processSourceImage(srcImage, false); @@ -665,7 +688,7 @@ gpu::TexturePointer TextureUsage::process2DTextureGrayscaleFromImage(const QImag theTexture = gpu::Texture::create2D(formatGPU, image.width(), image.height(), gpu::Texture::MAX_NUM_MIPS, gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_MIP_LINEAR)); theTexture->setSource(srcImageName); theTexture->setStoredMipFormat(formatMip); - generateMips(theTexture.get(), image); + generateMips(theTexture.get(), image, abortProcessing); } return theTexture; @@ -927,7 +950,9 @@ const CubeLayout CubeLayout::CUBEMAP_LAYOUTS[] = { }; const int CubeLayout::NUM_CUBEMAP_LAYOUTS = sizeof(CubeLayout::CUBEMAP_LAYOUTS) / sizeof(CubeLayout); -gpu::TexturePointer TextureUsage::processCubeTextureColorFromImage(const QImage& srcImage, const std::string& srcImageName, bool generateIrradiance) { +gpu::TexturePointer TextureUsage::processCubeTextureColorFromImage(const QImage& srcImage, const std::string& srcImageName, + bool generateIrradiance, + const std::atomic& abortProcessing) { PROFILE_RANGE(resource_parse, "processCubeTextureColorFromImage"); gpu::TexturePointer theTexture = nullptr; @@ -986,7 +1011,7 @@ gpu::TexturePointer TextureUsage::processCubeTextureColorFromImage(const QImage& theTexture->setStoredMipFormat(formatMip); for (uint8 face = 0; face < faces.size(); ++face) { - generateMips(theTexture.get(), faces[face], face); + generateMips(theTexture.get(), faces[face], abortProcessing, face); } // Generate irradiance while we are at it diff --git a/libraries/image/src/image/Image.h b/libraries/image/src/image/Image.h index 3bf45ace98..856dc009cf 100644 --- a/libraries/image/src/image/Image.h +++ b/libraries/image/src/image/Image.h @@ -41,26 +41,42 @@ enum Type { UNUSED_TEXTURE }; -using TextureLoader = std::function; +using TextureLoader = std::function&)>; TextureLoader getTextureLoaderForType(Type type, const QVariantMap& options = QVariantMap()); -gpu::TexturePointer create2DTextureFromImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createStrict2DTextureFromImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createAlbedoTextureFromImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createEmissiveTextureFromImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createNormalTextureFromNormalImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createNormalTextureFromBumpImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createRoughnessTextureFromImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createRoughnessTextureFromGlossImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createMetallicTextureFromImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createCubeTextureFromImage(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createCubeTextureFromImageWithoutIrradiance(const QImage& image, const std::string& srcImageName); -gpu::TexturePointer createLightmapTextureFromImage(const QImage& image, const std::string& srcImageName); +gpu::TexturePointer create2DTextureFromImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createStrict2DTextureFromImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createAlbedoTextureFromImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createEmissiveTextureFromImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createNormalTextureFromNormalImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createNormalTextureFromBumpImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createRoughnessTextureFromImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createRoughnessTextureFromGlossImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createMetallicTextureFromImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createCubeTextureFromImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createCubeTextureFromImageWithoutIrradiance(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); +gpu::TexturePointer createLightmapTextureFromImage(const QImage& image, const std::string& srcImageName, + const std::atomic& abortProcessing); -gpu::TexturePointer process2DTextureColorFromImage(const QImage& srcImage, const std::string& srcImageName, bool isStrict); -gpu::TexturePointer process2DTextureNormalMapFromImage(const QImage& srcImage, const std::string& srcImageName, bool isBumpMap); -gpu::TexturePointer process2DTextureGrayscaleFromImage(const QImage& srcImage, const std::string& srcImageName, bool isInvertedPixels); -gpu::TexturePointer processCubeTextureColorFromImage(const QImage& srcImage, const std::string& srcImageName, bool generateIrradiance); +gpu::TexturePointer process2DTextureColorFromImage(const QImage& srcImage, const std::string& srcImageName, bool isStrict, + const std::atomic& abortProcessing); +gpu::TexturePointer process2DTextureNormalMapFromImage(const QImage& srcImage, const std::string& srcImageName, bool isBumpMap, + const std::atomic& abortProcessing); +gpu::TexturePointer process2DTextureGrayscaleFromImage(const QImage& srcImage, const std::string& srcImageName, bool isInvertedPixels, + const std::atomic& abortProcessing); +gpu::TexturePointer processCubeTextureColorFromImage(const QImage& srcImage, const std::string& srcImageName, bool generateIrradiance, + const std::atomic& abortProcessing); } // namespace TextureUsage @@ -74,7 +90,9 @@ void setNormalTexturesCompressionEnabled(bool enabled); void setGrayscaleTexturesCompressionEnabled(bool enabled); void setCubeTexturesCompressionEnabled(bool enabled); -gpu::TexturePointer processImage(const QByteArray& content, const std::string& url, int maxNumPixels, TextureUsage::Type textureType); +gpu::TexturePointer processImage(const QByteArray& content, const std::string& url, + int maxNumPixels, TextureUsage::Type textureType, + const std::atomic& abortProcessing = false); } // namespace image diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index cc29e841ce..fed5c69ce2 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -265,7 +265,7 @@ gpu::TexturePointer getFallbackTextureForType(image::TextureUsage::Type type) { gpu::TexturePointer TextureCache::getImageTexture(const QString& path, image::TextureUsage::Type type, QVariantMap options) { QImage image = QImage(path); auto loader = image::TextureUsage::getTextureLoaderForType(type, options); - return gpu::TexturePointer(loader(image, QUrl::fromLocalFile(path).fileName().toStdString())); + return gpu::TexturePointer(loader(image, QUrl::fromLocalFile(path).fileName().toStdString(), false)); } QSharedPointer TextureCache::createResource(const QUrl& url, const QSharedPointer& fallback, From a8ac015bc4e350ffc768699633f757af51a1f03c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 19 Sep 2017 13:56:42 -0700 Subject: [PATCH 2/2] address code review comments --- assignment-client/src/assets/AssetServer.cpp | 4 ++-- libraries/image/src/image/Image.cpp | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index 17aae74dfd..fea9d8329d 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -280,10 +280,10 @@ void AssetServer::aboutToFinish() { // abort each of our still running bake tasks, remove pending bakes that were never put on the thread pool auto it = _pendingBakes.begin(); while (it != _pendingBakes.end()) { - auto pendingRunnable = _bakingTaskPool.tryTake(it.value().get()); + auto pendingRunnable = _bakingTaskPool.tryTake(it->get()); if (pendingRunnable) { - _pendingBakes.erase(it); + it = _pendingBakes.erase(it); } else { it.value()->abort(); ++it; diff --git a/libraries/image/src/image/Image.cpp b/libraries/image/src/image/Image.cpp index 9d49efcba0..6081355eaa 100644 --- a/libraries/image/src/image/Image.cpp +++ b/libraries/image/src/image/Image.cpp @@ -460,6 +460,8 @@ void generateMips(gpu::Texture* texture, QImage& image, const std::atomic& for (int i = 0; i < count; i++) { if (!_abortProcessing.load()) { task(context, i); + } else { + break; } } }