Return existing textures when matching hash for a previously found live texture

libraries/model-networking/src/model-networking/KTXCache.cpp

@@ -40,5 +40,8 @@ KTXFile::KTXFile(Metadata&& metadata, const std::string& filepath) :
 
 std::unique_ptr<ktx::KTX> KTXFile::getKTX() const {
     ktx::StoragePointer storage = std::make_shared<storage::FileStorage>(getFilepath().c_str());
-    return ktx::KTX::create(storage);
+    if (*storage) {
+        return ktx::KTX::create(storage);
+    }
+    return {};
 }

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<NetworkTexture>();
 }
 
+gpu::TexturePointer TextureCache::getTextureByHash(const std::string& hash) {
+    std::weak_ptr<gpu::Texture> weakPointer;
+    {
+        std::unique_lock<std::mutex> 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<std::mutex> 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<TextureCache>();
@@ -386,15 +415,6 @@ private:
     int _maxNumPixels;
 };
 
-class KTXReader : public Reader {
-public:
-    KTXReader(const QWeakPointer<Resource>& 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> reader;
+    auto textureCache = static_cast<TextureCache*>(_cache.data());
-    KTXFilePointer ktxFile;
+
-    if (!_cache.isNull() && (ktxFile = static_cast<TextureCache*>(_cache.data())->_ktxCache.getFile(hash))) {
+    if (textureCache != nullptr) {
-        reader.reset(new KTXReader(_self, _url, ktxFile));
+        // If we already have a live texture with the same hash, use it
-    } else {
+        auto texture = textureCache->getTextureByHash(hash);
-        reader.reset(new ImageReader(_self, _url, content, hash, _maxNumPixels));
+
+        // 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>& resource, const QUrl& url) :
@@ -526,22 +570,34 @@ void ImageReader::read() {
             texture->setFallbackTexture(networkTexture->getFallbackTexture());
         }
 
+        auto textureCache = DependencyManager::get<TextureCache>();
         // Save the image into a KTXFile
-        auto ktx = gpu::Texture::serialize(*texture);
+        auto memKtx = gpu::Texture::serialize(*texture);
-        if (ktx) {
+        if (!memKtx) {
-            const char* data = reinterpret_cast<const char*>(ktx->_storage->data());
+            qCWarning(modelnetworking) << "Unable to serialize texture to KTX " << _url;
-            size_t length = ktx->_storage->size();
+        }
+
+        if (memKtx && textureCache) {
+            const char* data = reinterpret_cast<const char*>(memKtx->_storage->data());
+            size_t length = memKtx->_storage->size();
             KTXFilePointer file;
-            auto& ktxCache = DependencyManager::get<TextureCache>()->_ktxCache;
+            auto& ktxCache = textureCache->_ktxCache;
-            if (!ktx || !(file = ktxCache.writeFile(data, KTXCache::Metadata(_hash, length)))) {
+            if (!memKtx || !(file = ktxCache.writeFile(data, KTXCache::Metadata(_hash, length)))) {
                 qCWarning(modelnetworking) << _url << "file cache failed";
             } else {
                 resource.staticCast<NetworkTexture>()->_file = file;
-                auto ktx = file->getKTX();
+                auto fileKtx = file->getKTX();
-                texture->setKtxBacking(ktx);
+                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>& 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<NetworkTexture>()->_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";
-    }
-
-}

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<std::string, std::weak_ptr<gpu::Texture>> _texturesByHashes;
+    std::mutex _texturesByHashesMutex;
 
     gpu::TexturePointer _permutationNormalTexture;
     gpu::TexturePointer _whiteTexture;

libraries/shared/src/shared/Storage.cpp

@@ -8,7 +8,11 @@
 
 #include "Storage.h"
 
-#include <QFileInfo>
+#include <QtCore/QFileInfo>
+#include <QtCore/QDebug>
+#include <QtCore/QLoggingCategory>
+
+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)) {
+    if (_file.open(QFile::ReadOnly)) {
-        throw std::runtime_error("Unable to open file");
+        _mapped = _file.map(0, _file.size());
-    }
+        if (_mapped) {
-    _mapped = _file.map(0, _file.size());
+            _valid = true;
-    if (!_mapped) {
+        } else {
-        throw std::runtime_error("Unable to map file");
+            qCWarning(storagelogging) << "Failed to map file " << filename;
+        }
+    } else {
+        qCWarning(storagelogging) << "Failed to open file " << filename;
     }
 }
 

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<uint8_t> _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;