From 1d4b4431130e8d5899d4d0cf1227b472b58dde9d Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Sat, 17 Jun 2017 14:32:59 -0700 Subject: [PATCH] Prevent cache ejection of textures in use --- libraries/gpu-gl/src/gpu/gl/GLTexture.cpp | 15 ++- libraries/gpu/src/gpu/Texture.h | 8 +- libraries/gpu/src/gpu/Texture_ktx.cpp | 95 +++++++++++++------ .../src/model-networking/KTXCache.h | 2 +- .../src/model-networking/TextureCache.cpp | 11 ++- .../src => shared/src/shared}/FileCache.cpp | 13 ++- .../src => shared/src/shared}/FileCache.h | 2 +- .../src/FileCacheTests.cpp | 0 .../src/FileCacheTests.h | 0 9 files changed, 102 insertions(+), 44 deletions(-) rename libraries/{networking/src => shared/src/shared}/FileCache.cpp (97%) rename libraries/{networking/src => shared/src/shared}/FileCache.h (98%) rename tests/{networking => shared}/src/FileCacheTests.cpp (100%) rename tests/{networking => shared}/src/FileCacheTests.h (100%) diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp index 88e5cde330..4161242a24 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp @@ -223,12 +223,21 @@ TransferJob::TransferJob(const GLTexture& parent, uint16_t sourceMip, uint16_t t // Buffering can invoke disk IO, so it should be off of the main and render threads _bufferingLambda = [=] { - _mipData = _parent._gpuObject.accessStoredMipFace(sourceMip, face)->createView(_transferSize, _transferOffset); + auto mipStorage = _parent._gpuObject.accessStoredMipFace(sourceMip, face); + if (mipStorage) { + _mipData = mipStorage->createView(_transferSize, _transferOffset); + } else { + qCWarning(gpugllogging) << "Buffering failed because mip could not be retrieved from texture " << _parent._source.c_str() ; + } }; _transferLambda = [=] { - _parent.copyMipFaceLinesFromTexture(targetMip, face, transferDimensions, lineOffset, internalFormat, format, type, _mipData->size(), _mipData->readData()); - _mipData.reset(); + if (_mipData) { + _parent.copyMipFaceLinesFromTexture(targetMip, face, transferDimensions, lineOffset, internalFormat, format, type, _mipData->size(), _mipData->readData()); + _mipData.reset(); + } else { + qCWarning(gpugllogging) << "Transfer failed because mip could not be retrieved from texture " << _parent._source.c_str(); + } }; } diff --git a/libraries/gpu/src/gpu/Texture.h b/libraries/gpu/src/gpu/Texture.h index 211dc7b8ce..1877b494cf 100755 --- a/libraries/gpu/src/gpu/Texture.h +++ b/libraries/gpu/src/gpu/Texture.h @@ -18,7 +18,7 @@ #include #include - +#include #include "Forward.h" #include "Resource.h" #include "Metric.h" @@ -311,6 +311,7 @@ public: class KtxStorage : public Storage { public: KtxStorage(const std::string& filename); + KtxStorage(const cache::FilePointer& file); PixelsPointer getMipFace(uint16 level, uint8 face = 0) const override; Size getMipFaceSize(uint16 level, uint8 face = 0) const override; bool isMipAvailable(uint16 level, uint8 face = 0) const override; @@ -328,6 +329,7 @@ public: mutable std::weak_ptr _cacheFile; std::string _filename; + cache::FilePointer _cacheEntry; std::atomic _minMipLevelAvailable; size_t _offsetToMinMipKV; @@ -499,6 +501,7 @@ public: void setStorage(std::unique_ptr& newStorage); void setKtxBacking(const std::string& filename); + void setKtxBacking(const cache::FilePointer& cacheEntry); // Usage is a a set of flags providing Semantic about the usage of the Texture. void setUsage(const Usage& usage) { _usage = usage; } @@ -529,8 +532,9 @@ public: // Serialize a texture into a KTX file static ktx::KTXUniquePointer serialize(const Texture& texture); + static TexturePointer build(const ktx::KTXDescriptor& descriptor); static TexturePointer unserialize(const std::string& ktxFile); - static TexturePointer unserialize(const std::string& ktxFile, const ktx::KTXDescriptor& descriptor); + static TexturePointer unserialize(const cache::FilePointer& cacheEntry); static bool evalKTXFormat(const Element& mipFormat, const Element& texelFormat, ktx::Header& header); static bool evalTextureFormat(const ktx::Header& header, Element& mipFormat, Element& texelFormat); diff --git a/libraries/gpu/src/gpu/Texture_ktx.cpp b/libraries/gpu/src/gpu/Texture_ktx.cpp index f455fde009..14fd983ec2 100644 --- a/libraries/gpu/src/gpu/Texture_ktx.cpp +++ b/libraries/gpu/src/gpu/Texture_ktx.cpp @@ -154,6 +154,10 @@ struct IrradianceKTXPayload { }; const std::string IrradianceKTXPayload::KEY{ "hifi.irradianceSH" }; +KtxStorage::KtxStorage(const cache::FilePointer& cacheEntry) : KtxStorage(cacheEntry->getFilepath()) { + _cacheEntry = cacheEntry; +} + KtxStorage::KtxStorage(const std::string& filename) : _filename(filename) { { // We are doing a lot of work here just to get descriptor data @@ -295,20 +299,35 @@ void KtxStorage::assignMipFaceData(uint16 level, uint8 face, const storage::Stor throw std::runtime_error("Invalid call"); } +bool validKtx(const std::string& filename) { + ktx::StoragePointer storage { new storage::FileStorage(filename.c_str()) }; + auto ktxPointer = ktx::KTX::create(storage); + if (!ktxPointer) { + return false; + } + return true; +} + void Texture::setKtxBacking(const std::string& filename) { // Check the KTX file for validity before using it as backing storage - { - ktx::StoragePointer storage { new storage::FileStorage(filename.c_str()) }; - auto ktxPointer = ktx::KTX::create(storage); - if (!ktxPointer) { - return; - } + if (!validKtx(filename)) { + return; } auto newBacking = std::unique_ptr(new KtxStorage(filename)); setStorage(newBacking); } +void Texture::setKtxBacking(const cache::FilePointer& cacheEntry) { + // Check the KTX file for validity before using it as backing storage + if (!validKtx(cacheEntry->getFilepath())) { + return; + } + + auto newBacking = std::unique_ptr(new KtxStorage(cacheEntry)); + setStorage(newBacking); +} + ktx::KTXUniquePointer Texture::serialize(const Texture& texture) { ktx::Header header; @@ -442,21 +461,10 @@ ktx::KTXUniquePointer Texture::serialize(const Texture& texture) { return ktxBuffer; } -TexturePointer Texture::unserialize(const std::string& ktxfile) { - std::unique_ptr ktxPointer = ktx::KTX::create(std::make_shared(ktxfile.c_str())); - if (!ktxPointer) { - return nullptr; - } - - ktx::KTXDescriptor descriptor { ktxPointer->toDescriptor() }; - return unserialize(ktxfile, ktxPointer->toDescriptor()); -} - -TexturePointer Texture::unserialize(const std::string& ktxfile, const ktx::KTXDescriptor& descriptor) { - const auto& header = descriptor.header; - +TexturePointer Texture::build(const ktx::KTXDescriptor& descriptor) { Format mipFormat = Format::COLOR_BGRA_32; Format texelFormat = Format::COLOR_SRGBA_32; + const auto& header = descriptor.header; if (!Texture::evalTextureFormat(header, mipFormat, texelFormat)) { return nullptr; @@ -485,20 +493,19 @@ TexturePointer Texture::unserialize(const std::string& ktxfile, const ktx::KTXDe } auto texture = create(gpuktxKeyValue._usageType, - type, - texelFormat, - header.getPixelWidth(), - header.getPixelHeight(), - header.getPixelDepth(), - 1, // num Samples - header.getNumberOfSlices(), - header.getNumberOfLevels(), - gpuktxKeyValue._samplerDesc); + type, + texelFormat, + header.getPixelWidth(), + header.getPixelHeight(), + header.getPixelDepth(), + 1, // num Samples + header.getNumberOfSlices(), + header.getNumberOfLevels(), + gpuktxKeyValue._samplerDesc); texture->setUsage(gpuktxKeyValue._usage); // Assing the mips availables texture->setStoredMipFormat(mipFormat); - texture->setKtxBacking(ktxfile); IrradianceKTXPayload irradianceKtxKeyValue; if (IrradianceKTXPayload::findInKeyValues(descriptor.keyValues, irradianceKtxKeyValue)) { @@ -508,6 +515,36 @@ TexturePointer Texture::unserialize(const std::string& ktxfile, const ktx::KTXDe return texture; } + + +TexturePointer Texture::unserialize(const cache::FilePointer& cacheEntry) { + std::unique_ptr ktxPointer = ktx::KTX::create(std::make_shared(cacheEntry->getFilepath().c_str())); + if (!ktxPointer) { + return nullptr; + } + + auto texture = build(ktxPointer->toDescriptor()); + if (texture) { + texture->setKtxBacking(cacheEntry); + } + + return texture; +} + +TexturePointer Texture::unserialize(const std::string& ktxfile) { + std::unique_ptr ktxPointer = ktx::KTX::create(std::make_shared(ktxfile.c_str())); + if (!ktxPointer) { + return nullptr; + } + + auto texture = build(ktxPointer->toDescriptor()); + if (texture) { + texture->setKtxBacking(ktxfile); + } + + return texture; +} + bool Texture::evalKTXFormat(const Element& mipFormat, const Element& texelFormat, ktx::Header& header) { if (texelFormat == Format::COLOR_RGBA_32 && mipFormat == Format::COLOR_BGRA_32) { header.setUncompressed(ktx::GLType::UNSIGNED_BYTE, 1, ktx::GLFormat::BGRA, ktx::GLInternalFormat::RGBA8, ktx::GLBaseInternalFormat::RGBA); diff --git a/libraries/model-networking/src/model-networking/KTXCache.h b/libraries/model-networking/src/model-networking/KTXCache.h index 5617019c52..a919c88bd7 100644 --- a/libraries/model-networking/src/model-networking/KTXCache.h +++ b/libraries/model-networking/src/model-networking/KTXCache.h @@ -14,7 +14,7 @@ #include -#include +#include namespace ktx { class KTX; diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index 8683d56b6b..15eb4be839 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -612,9 +612,10 @@ void NetworkTexture::maybeHandleFinishedInitialLoad() { if (!texture) { KTXFilePointer ktxFile = textureCache->_ktxCache.getFile(hash); if (ktxFile) { - texture = gpu::Texture::unserialize(ktxFile->getFilepath()); + texture = gpu::Texture::unserialize(ktxFile); if (texture) { texture = textureCache->cacheTextureByHash(hash, texture); + _file = ktxFile; } } } @@ -644,8 +645,8 @@ void NetworkTexture::maybeHandleFinishedInitialLoad() { auto newKtxDescriptor = memKtx->toDescriptor(); - texture = gpu::Texture::unserialize(_file->getFilepath(), newKtxDescriptor); - texture->setKtxBacking(file->getFilepath()); + texture = gpu::Texture::build(newKtxDescriptor); + texture->setKtxBacking(file); texture->setSource(filename); auto& images = _originalKtxDescriptor->images; @@ -796,7 +797,7 @@ void ImageReader::read() { if (!texture) { KTXFilePointer ktxFile = textureCache->_ktxCache.getFile(hash); if (ktxFile) { - texture = gpu::Texture::unserialize(ktxFile->getFilepath()); + texture = gpu::Texture::unserialize(ktxFile); if (texture) { texture = textureCache->cacheTextureByHash(hash, texture); } else { @@ -848,7 +849,7 @@ void ImageReader::read() { if (!networkTexture->_file) { qCWarning(modelnetworking) << _url << "file cache failed"; } else { - texture->setKtxBacking(networkTexture->_file->getFilepath()); + texture->setKtxBacking(networkTexture->_file); } } else { qCWarning(modelnetworking) << "Unable to serialize texture to KTX " << _url; diff --git a/libraries/networking/src/FileCache.cpp b/libraries/shared/src/shared/FileCache.cpp similarity index 97% rename from libraries/networking/src/FileCache.cpp rename to libraries/shared/src/shared/FileCache.cpp index 43bab67ec7..801c68e302 100644 --- a/libraries/networking/src/FileCache.cpp +++ b/libraries/shared/src/shared/FileCache.cpp @@ -21,8 +21,8 @@ #include #include -#include -#include +#include "../PathUtils.h" +#include "../NumericalConstants.h" #ifdef Q_OS_WIN #include @@ -132,9 +132,16 @@ FilePointer FileCache::addFile(Metadata&& metadata, const std::string& filepath) } FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata, bool overwrite) { + FilePointer file; + + if (0 == metadata.length) { + qCWarning(file_cache) << "Cannot store empty files in the cache"; + return file; + } + + Lock lock(_mutex); - FilePointer file; if (!_initialized) { qCWarning(file_cache) << "File cache used before initialization"; return file; diff --git a/libraries/networking/src/FileCache.h b/libraries/shared/src/shared/FileCache.h similarity index 98% rename from libraries/networking/src/FileCache.h rename to libraries/shared/src/shared/FileCache.h index 089d99273a..25210af1da 100644 --- a/libraries/networking/src/FileCache.h +++ b/libraries/shared/src/shared/FileCache.h @@ -143,7 +143,7 @@ public: const Key& getKey() const { return _key; } const size_t& getLength() const { return _length; } - std::string getFilepath() const { return _filepath; } + const std::string& getFilepath() const { return _filepath; } virtual ~File(); /// overrides should call File::deleter to maintain caching behavior diff --git a/tests/networking/src/FileCacheTests.cpp b/tests/shared/src/FileCacheTests.cpp similarity index 100% rename from tests/networking/src/FileCacheTests.cpp rename to tests/shared/src/FileCacheTests.cpp diff --git a/tests/networking/src/FileCacheTests.h b/tests/shared/src/FileCacheTests.h similarity index 100% rename from tests/networking/src/FileCacheTests.h rename to tests/shared/src/FileCacheTests.h