From d38dadaf58e193a53cb002b4960e922e92123b3b Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Fri, 6 Apr 2018 09:34:00 -0700 Subject: [PATCH] possible fix for qml texture leak --- .../src/RenderableEntityItem.cpp | 4 +--- .../src/RenderableWebEntityItem.cpp | 24 ++++++++++++++----- libraries/qml/src/qml/impl/SharedObject.cpp | 14 ++++++----- libraries/qml/src/qml/impl/TextureCache.cpp | 22 +++++++---------- libraries/qml/src/qml/impl/TextureCache.h | 1 - 5 files changed, 36 insertions(+), 29 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableEntityItem.cpp b/libraries/entities-renderer/src/RenderableEntityItem.cpp index c33b87e5cf..b61f24972a 100644 --- a/libraries/entities-renderer/src/RenderableEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableEntityItem.cpp @@ -135,10 +135,8 @@ void EntityRenderer::makeStatusGetters(const EntityItemPointer& entity, Item::St template std::shared_ptr make_renderer(const EntityItemPointer& entity) { - T* rawResult = new T(entity); - // We want to use deleteLater so that renderer destruction gets pushed to the main thread - return std::shared_ptr(rawResult, std::bind(&QObject::deleteLater, rawResult)); + return std::shared_ptr(new T(entity), [](T* ptr) { ptr->deleteLater(); }); } EntityRenderer::EntityRenderer(const EntityItemPointer& entity) : _entity(entity) { diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index bd00ded12d..f31ed4e238 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -34,7 +34,7 @@ static const QString WEB_ENTITY_QML = "controls/WebEntityView.qml"; const float METERS_TO_INCHES = 39.3701f; static uint32_t _currentWebCount{ 0 }; -// Don't allow more than 100 concurrent web views +// Don't allow more than 20 concurrent web views static const uint32_t MAX_CONCURRENT_WEB_VIEWS = 20; // If a web-view hasn't been rendered for 30 seconds, de-allocate the framebuffer static uint64_t MAX_NO_RENDER_INTERVAL = 30 * USECS_PER_SECOND; @@ -88,8 +88,14 @@ bool WebEntityRenderer::needsRenderUpdateFromTypedEntity(const TypedEntityPointe return true; } - if (uvec2(getWindowSize(entity)) != toGlm(_webSurface->size())) { - return true; + { + QSharedPointer webSurface; + withReadLock([&] { + webSurface = _webSurface; + }); + if (webSurface && uvec2(getWindowSize(entity)) != toGlm(webSurface->size())) { + return true; + } } if (_lastSourceUrl != entity->getSourceUrl()) { @@ -108,9 +114,15 @@ bool WebEntityRenderer::needsRenderUpdateFromTypedEntity(const TypedEntityPointe } bool WebEntityRenderer::needsRenderUpdate() const { - if (!_webSurface) { - // If we have rendered recently, and there is no web surface, we're going to create one - return true; + { + QSharedPointer webSurface; + withReadLock([&] { + webSurface = _webSurface; + }); + if (!webSurface) { + // If we have rendered recently, and there is no web surface, we're going to create one + return true; + } } return Parent::needsRenderUpdate(); diff --git a/libraries/qml/src/qml/impl/SharedObject.cpp b/libraries/qml/src/qml/impl/SharedObject.cpp index d66f0f1dab..d593169d94 100644 --- a/libraries/qml/src/qml/impl/SharedObject.cpp +++ b/libraries/qml/src/qml/impl/SharedObject.cpp @@ -37,9 +37,8 @@ static const int MIN_TIMER_MS = 5; using namespace hifi::qml; using namespace hifi::qml::impl; -TextureCache offscreenTextures; - TextureCache& SharedObject::getTextureCache() { + static TextureCache offscreenTextures; return offscreenTextures; } @@ -243,7 +242,7 @@ void SharedObject::releaseTextureAndFence() { QMutexLocker lock(&_mutex); // If the most recent texture was unused, we can directly recycle it if (_latestTextureAndFence.first) { - offscreenTextures.releaseTexture(_latestTextureAndFence); + getTextureCache().releaseTexture(_latestTextureAndFence); _latestTextureAndFence = TextureAndFence{ 0, 0 }; } } @@ -307,7 +306,10 @@ bool SharedObject::preRender() { void SharedObject::shutdownRendering(OffscreenGLCanvas& canvas, const QSize& size) { QMutexLocker locker(&_mutex); if (size != QSize(0, 0)) { - offscreenTextures.releaseSize(size); + getTextureCache().releaseSize(size); + if (_latestTextureAndFence.first) { + getTextureCache().releaseTexture(_latestTextureAndFence); + } } _renderControl->invalidate(); canvas.doneCurrent(); @@ -403,7 +405,7 @@ void SharedObject::onRender() { } void SharedObject::onTimer() { - offscreenTextures.report(); + getTextureCache().report(); if (!_renderRequested) { return; } @@ -436,7 +438,7 @@ void SharedObject::updateTextureAndFence(const TextureAndFence& newTextureAndFen QMutexLocker locker(&_mutex); // If the most recent texture was unused, we can directly recycle it if (_latestTextureAndFence.first) { - offscreenTextures.releaseTexture(_latestTextureAndFence); + getTextureCache().releaseTexture(_latestTextureAndFence); _latestTextureAndFence = { 0, 0 }; } diff --git a/libraries/qml/src/qml/impl/TextureCache.cpp b/libraries/qml/src/qml/impl/TextureCache.cpp index c649a36594..7af8fa1ac9 100644 --- a/libraries/qml/src/qml/impl/TextureCache.cpp +++ b/libraries/qml/src/qml/impl/TextureCache.cpp @@ -11,10 +11,6 @@ using namespace hifi::qml::impl; -#if defined(Q_OS_ANDROID) -#define USE_GLES 1 -#endif - uint64_t uvec2ToUint64(const QSize& size) { uint64_t result = size.width(); result <<= 32; @@ -31,26 +27,23 @@ void TextureCache::acquireSize(const QSize& size) { void TextureCache::releaseSize(const QSize& size) { auto sizeKey = uvec2ToUint64(size); - ValueList texturesToDelete; { Lock lock(_mutex); assert(_textures.count(sizeKey)); auto& textureSet = _textures[sizeKey]; if (0 == --textureSet.clientCount) { - texturesToDelete.swap(textureSet.returnedTextures); + for (const auto& textureAndFence : textureSet.returnedTextures) { + destroy(textureAndFence); + } _textures.erase(sizeKey); } } - for (const auto& textureAndFence : texturesToDelete) { - destroy(textureAndFence); - } } uint32_t TextureCache::acquireTexture(const QSize& size) { Lock lock(_mutex); recycle(); - ++_activeTextureCount; auto sizeKey = uvec2ToUint64(size); assert(_textures.count(sizeKey)); @@ -83,7 +76,12 @@ void TextureCache::report() { } size_t TextureCache::getUsedTextureMemory() { - return _totalTextureUsage; + size_t toReturn; + { + Lock lock(_mutex); + toReturn = _totalTextureUsage; + } + return toReturn; } size_t TextureCache::getMemoryForSize(const QSize& size) { @@ -122,8 +120,6 @@ uint32_t TextureCache::createTexture(const QSize& size) { glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_R, GL_CLAMP_TO_EDGE); - glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_R, GL_CLAMP_TO_EDGE); - glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAX_ANISOTROPY_EXT, 8.0f); #if !defined(USE_GLES) glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_LOD_BIAS, -0.2f); #endif diff --git a/libraries/qml/src/qml/impl/TextureCache.h b/libraries/qml/src/qml/impl/TextureCache.h index 572e1cadea..c146d0bdbf 100644 --- a/libraries/qml/src/qml/impl/TextureCache.h +++ b/libraries/qml/src/qml/impl/TextureCache.h @@ -41,7 +41,6 @@ public: ValueList returnedTextures; }; - void releaseSize(const QSize& size); void acquireSize(const QSize& size); uint32_t acquireTexture(const QSize& size);