From 14c8c3f4db766ad3f7f09a3955bfea565cd8406c Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 20 Mar 2017 09:39:29 -0700 Subject: [PATCH] Return existing textures when matching hash for a previously found live texture --- .../src/model-networking/KTXCache.cpp | 5 +- .../src/model-networking/TextureCache.cpp | 140 ++++++++++-------- .../src/model-networking/TextureCache.h | 7 + libraries/shared/src/shared/Storage.cpp | 21 ++- libraries/shared/src/shared/Storage.h | 5 + 5 files changed, 112 insertions(+), 66 deletions(-) diff --git a/libraries/model-networking/src/model-networking/KTXCache.cpp b/libraries/model-networking/src/model-networking/KTXCache.cpp index 908d3245ce..63d35fe4a4 100644 --- a/libraries/model-networking/src/model-networking/KTXCache.cpp +++ b/libraries/model-networking/src/model-networking/KTXCache.cpp @@ -40,5 +40,8 @@ KTXFile::KTXFile(Metadata&& metadata, const std::string& filepath) : std::unique_ptr KTXFile::getKTX() const { ktx::StoragePointer storage = std::make_shared(getFilepath().c_str()); - return ktx::KTX::create(storage); + if (*storage) { + return ktx::KTX::create(storage); + } + return {}; } diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index e51b016904..5dfaddd471 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -188,6 +188,35 @@ NetworkTexturePointer TextureCache::getTexture(const QUrl& url, Type type, const return ResourceCache::getResource(url, QUrl(), &extra).staticCast(); } +gpu::TexturePointer TextureCache::getTextureByHash(const std::string& hash) { + std::weak_ptr weakPointer; + { + std::unique_lock lock(_texturesByHashesMutex); + weakPointer = _texturesByHashes[hash]; + } + auto result = weakPointer.lock(); + if (result) { + qCWarning(modelnetworking) << "QQQ Returning live texture for hash " << hash.c_str(); + } + return result; +} + +gpu::TexturePointer TextureCache::cacheTextureByHash(const std::string& hash, const gpu::TexturePointer& texture) { + gpu::TexturePointer result; + { + std::unique_lock lock(_texturesByHashesMutex); + result = _texturesByHashes[hash].lock(); + if (!result) { + _texturesByHashes[hash] = texture; + result = texture; + } else { + qCWarning(modelnetworking) << "QQQ Swapping out texture with previous live texture in hash " << hash.c_str(); + } + } + return result; +} + + gpu::TexturePointer getFallbackTextureForType(NetworkTexture::Type type) { gpu::TexturePointer result; auto textureCache = DependencyManager::get(); @@ -386,15 +415,6 @@ private: int _maxNumPixels; }; -class KTXReader : public Reader { -public: - KTXReader(const QWeakPointer& resource, const QUrl& url, const KTXFilePointer& ktxFile); - void read() override final; - -private: - KTXFilePointer _file; -}; - void NetworkTexture::downloadFinished(const QByteArray& data) { loadContent(data); } @@ -408,15 +428,39 @@ void NetworkTexture::loadContent(const QByteArray& content) { hash = hasher.result().toHex().toStdString(); } - std::unique_ptr reader; - KTXFilePointer ktxFile; - if (!_cache.isNull() && (ktxFile = static_cast(_cache.data())->_ktxCache.getFile(hash))) { - reader.reset(new KTXReader(_self, _url, ktxFile)); - } else { - reader.reset(new ImageReader(_self, _url, content, hash, _maxNumPixels)); + auto textureCache = static_cast(_cache.data()); + + if (textureCache != nullptr) { + // If we already have a live texture with the same hash, use it + auto texture = textureCache->getTextureByHash(hash); + + // If there is no live texture, check if there's an existing KTX file + if (!texture) { + KTXFilePointer ktxFile = textureCache->_ktxCache.getFile(hash); + if (ktxFile) { + // Ensure that the KTX deserialization worked + auto ktx = ktxFile->getKTX(); + if (ktx) { + texture.reset(gpu::Texture::unserialize(ktx)); + // Ensure that the texture population worked + if (texture) { + texture->setKtxBacking(ktx); + texture = textureCache->cacheTextureByHash(hash, texture); + } + } + } + } + + // If we found the texture either because it's in use or via KTX deserialization, + // set the image and return immediately. + if (texture) { + setImage(texture, texture->getWidth(), texture->getHeight()); + return; + } } - QThreadPool::globalInstance()->start(reader.release()); + // We failed to find an existing live or KTX texture, so trigger an image reader + QThreadPool::globalInstance()->start(new ImageReader(_self, _url, content, hash, _maxNumPixels)); } Reader::Reader(const QWeakPointer& resource, const QUrl& url) : @@ -526,22 +570,34 @@ void ImageReader::read() { texture->setFallbackTexture(networkTexture->getFallbackTexture()); } + auto textureCache = DependencyManager::get(); // Save the image into a KTXFile - auto ktx = gpu::Texture::serialize(*texture); - if (ktx) { - const char* data = reinterpret_cast(ktx->_storage->data()); - size_t length = ktx->_storage->size(); + auto memKtx = gpu::Texture::serialize(*texture); + if (!memKtx) { + qCWarning(modelnetworking) << "Unable to serialize texture to KTX " << _url; + } + + if (memKtx && textureCache) { + const char* data = reinterpret_cast(memKtx->_storage->data()); + size_t length = memKtx->_storage->size(); KTXFilePointer file; - auto& ktxCache = DependencyManager::get()->_ktxCache; - if (!ktx || !(file = ktxCache.writeFile(data, KTXCache::Metadata(_hash, length)))) { + auto& ktxCache = textureCache->_ktxCache; + if (!memKtx || !(file = ktxCache.writeFile(data, KTXCache::Metadata(_hash, length)))) { qCWarning(modelnetworking) << _url << "file cache failed"; } else { resource.staticCast()->_file = file; - auto ktx = file->getKTX(); - texture->setKtxBacking(ktx); + auto fileKtx = file->getKTX(); + if (fileKtx) { + texture->setKtxBacking(fileKtx); + } } - } else { - qCWarning(modelnetworking) << "Unable to serialize texture to KTX " << _url; + } + + // We replace the texture with the one stored in the cache. This deals with the possible race condition of two different + // images with the same hash being loaded concurrently. Only one of them will make it into the cache by hash first and will + // be the winner + if (textureCache) { + texture = textureCache->cacheTextureByHash(_hash, texture); } } @@ -554,35 +610,3 @@ void ImageReader::read() { qCDebug(modelnetworking) << _url << "loading stopped; resource out of scope"; } } - -KTXReader::KTXReader(const QWeakPointer& resource, const QUrl& url, - const KTXFilePointer& ktxFile) : - Reader(resource, url), _file(ktxFile) {} - -void KTXReader::read() { - PROFILE_RANGE_EX(resource_parse_image_ktx, __FUNCTION__, 0xffff0000, 0, { { "url", _url.toString() } }); - - gpu::TexturePointer texture; - { - auto resource = _resource.lock(); // to ensure the resource is still needed - if (!resource) { - qCDebug(modelnetworking) << _url << "loading stopped; resource out of scope"; - return; - } - - resource.staticCast()->_file = _file; - auto ktx = _file->getKTX(); - texture.reset(gpu::Texture::unserialize(ktx)); - texture->setKtxBacking(ktx); - } - - auto resource = _resource.lock(); // to ensure the resource is still needed - if (resource) { - QMetaObject::invokeMethod(resource.data(), "setImage", - Q_ARG(gpu::TexturePointer, texture), - Q_ARG(int, texture->getWidth()), Q_ARG(int, texture->getHeight())); - } else { - qCDebug(modelnetworking) << _url << "loading stopped; resource out of scope"; - } - -} diff --git a/libraries/model-networking/src/model-networking/TextureCache.h b/libraries/model-networking/src/model-networking/TextureCache.h index 8ee3f59416..6005cc1226 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.h +++ b/libraries/model-networking/src/model-networking/TextureCache.h @@ -137,6 +137,10 @@ public: NetworkTexturePointer getTexture(const QUrl& url, Type type = Type::DEFAULT_TEXTURE, const QByteArray& content = QByteArray(), int maxNumPixels = ABSOLUTE_MAX_TEXTURE_NUM_PIXELS); + + gpu::TexturePointer getTextureByHash(const std::string& hash); + gpu::TexturePointer cacheTextureByHash(const std::string& hash, const gpu::TexturePointer& texture); + protected: // Overload ResourceCache::prefetch to allow specifying texture type for loads Q_INVOKABLE ScriptableResource* prefetch(const QUrl& url, int type, int maxNumPixels = ABSOLUTE_MAX_TEXTURE_NUM_PIXELS); @@ -155,6 +159,9 @@ private: static const std::string KTX_DIRNAME; static const std::string KTX_EXT; KTXCache _ktxCache; + // Map from image hashes to texture weak pointers + std::unordered_map> _texturesByHashes; + std::mutex _texturesByHashesMutex; gpu::TexturePointer _permutationNormalTexture; gpu::TexturePointer _whiteTexture; diff --git a/libraries/shared/src/shared/Storage.cpp b/libraries/shared/src/shared/Storage.cpp index 6754854c40..3c46347a49 100644 --- a/libraries/shared/src/shared/Storage.cpp +++ b/libraries/shared/src/shared/Storage.cpp @@ -8,7 +8,11 @@ #include "Storage.h" -#include +#include +#include +#include + +Q_LOGGING_CATEGORY(storagelogging, "hifi.core.storage") using namespace storage; @@ -64,12 +68,15 @@ StoragePointer FileStorage::create(const QString& filename, size_t size, const u } FileStorage::FileStorage(const QString& filename) : _file(filename) { - if (!_file.open(QFile::ReadOnly)) { - throw std::runtime_error("Unable to open file"); - } - _mapped = _file.map(0, _file.size()); - if (!_mapped) { - throw std::runtime_error("Unable to map file"); + if (_file.open(QFile::ReadOnly)) { + _mapped = _file.map(0, _file.size()); + if (_mapped) { + _valid = true; + } else { + qCWarning(storagelogging) << "Failed to map file " << filename; + } + } else { + qCWarning(storagelogging) << "Failed to open file " << filename; } } diff --git a/libraries/shared/src/shared/Storage.h b/libraries/shared/src/shared/Storage.h index 4b97e14178..306984040f 100644 --- a/libraries/shared/src/shared/Storage.h +++ b/libraries/shared/src/shared/Storage.h @@ -25,6 +25,7 @@ namespace storage { virtual ~Storage() {} virtual const uint8_t* data() const = 0; virtual size_t size() const = 0; + virtual operator bool() const { return true; } StoragePointer createView(size_t size = 0, size_t offset = 0) const; StoragePointer toFileStorage(const QString& filename) const; @@ -41,6 +42,7 @@ namespace storage { const uint8_t* data() const override { return _data.data(); } uint8_t* data() { return _data.data(); } size_t size() const override { return _data.size(); } + operator bool() const override { return true; } private: std::vector _data; }; @@ -56,7 +58,9 @@ namespace storage { const uint8_t* data() const override { return _mapped; } size_t size() const override { return _file.size(); } + operator bool() const override { return _valid; } private: + bool _valid { false }; QFile _file; uint8_t* _mapped { nullptr }; }; @@ -66,6 +70,7 @@ namespace storage { ViewStorage(const storage::StoragePointer& owner, size_t size, const uint8_t* data); const uint8_t* data() const override { return _data; } size_t size() const override { return _size; } + operator bool() const override { return *_owner; } private: const storage::StoragePointer _owner; const size_t _size;