From 8c5028158fe8a3e874f2da09e1d2825c992ffa2a Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 3 Oct 2016 16:42:50 -0700 Subject: [PATCH] Fix crash on destroying web entities and overlays --- interface/src/ui/overlays/Web3DOverlay.cpp | 12 +++++-- interface/src/ui/overlays/Web3DOverlay.h | 2 +- .../src/RenderableWebEntityItem.cpp | 33 +++++++++++-------- .../src/RenderableWebEntityItem.h | 2 +- libraries/gpu-gl/src/gpu/gl/GLTexture.cpp | 7 ++-- 5 files changed, 35 insertions(+), 21 deletions(-) diff --git a/interface/src/ui/overlays/Web3DOverlay.cpp b/interface/src/ui/overlays/Web3DOverlay.cpp index 8f213e7740..be564a768e 100644 --- a/interface/src/ui/overlays/Web3DOverlay.cpp +++ b/interface/src/ui/overlays/Web3DOverlay.cpp @@ -73,7 +73,12 @@ void Web3DOverlay::render(RenderArgs* args) { QOpenGLContext * currentContext = QOpenGLContext::currentContext(); QSurface * currentSurface = currentContext->surface(); if (!_webSurface) { - _webSurface = new OffscreenQmlSurface(); + auto deleter = [](OffscreenQmlSurface* webSurface) { + AbstractViewStateInterface::instance()->postLambdaEvent([webSurface] { + webSurface->deleteLater(); + }); + }; + _webSurface = QSharedPointer(new OffscreenQmlSurface(), deleter); _webSurface->create(currentContext); _webSurface->setBaseUrl(QUrl::fromLocalFile(PathUtils::resourcesPath() + "/qml/controls/")); _webSurface->load("WebView.qml"); @@ -95,8 +100,9 @@ void Web3DOverlay::render(RenderArgs* args) { } if (!_texture) { - _texture = gpu::TexturePointer(gpu::Texture::createExternal2D([this](uint32_t recycleTexture, void* recycleFence) { - _webSurface->releaseTexture({ recycleTexture, recycleFence }); + auto webSurface = _webSurface; + _texture = gpu::TexturePointer(gpu::Texture::createExternal2D([webSurface](uint32_t recycleTexture, void* recycleFence) { + webSurface->releaseTexture({ recycleTexture, recycleFence }); })); _texture->setSource(__FUNCTION__); } diff --git a/interface/src/ui/overlays/Web3DOverlay.h b/interface/src/ui/overlays/Web3DOverlay.h index 0bb9e5e030..1e75bbbb06 100644 --- a/interface/src/ui/overlays/Web3DOverlay.h +++ b/interface/src/ui/overlays/Web3DOverlay.h @@ -41,7 +41,7 @@ public: virtual Web3DOverlay* createClone() const override; private: - OffscreenQmlSurface* _webSurface{ nullptr }; + QSharedPointer _webSurface; QMetaObject::Connection _connection; gpu::TexturePointer _texture; QString _url; diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index 3c339b7b7d..e415062e5c 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -129,7 +129,19 @@ bool RenderableWebEntityItem::buildWebSurface(EntityTreeRenderer* renderer) { // Save the original GL context, because creating a QML surface will create a new context QOpenGLContext * currentContext = QOpenGLContext::currentContext(); QSurface * currentSurface = currentContext->surface(); - _webSurface = new OffscreenQmlSurface(); + + auto deleter = [](OffscreenQmlSurface* webSurface) { + AbstractViewStateInterface::instance()->postLambdaEvent([webSurface] { + webSurface->deleteLater(); + }); + }; + _webSurface = QSharedPointer(new OffscreenQmlSurface(), deleter); + + // The lifetime of the QML surface MUST be managed by the main thread + // Additionally, we MUST use local variables copied by value, rather than + // member variables, since they would implicitly refer to a this that + // is no longer valid + _webSurface->create(currentContext); _webSurface->setBaseUrl(QUrl::fromLocalFile(PathUtils::resourcesPath() + "/qml/controls/")); _webSurface->load("WebView.qml", [&](QQmlContext* context, QObject* obj) { @@ -215,9 +227,11 @@ void RenderableWebEntityItem::render(RenderArgs* args) { _webSurface->resize(QSize(windowSize.x, windowSize.y)); if (!_texture) { - _texture = gpu::TexturePointer(gpu::Texture::createExternal2D([this](uint32_t recycleTexture, void* recycleFence) { - _webSurface->releaseTexture({ recycleTexture, recycleFence }); - })); + auto webSurface = _webSurface; + auto recycler = [webSurface] (uint32_t recycleTexture, void* recycleFence) { + webSurface->releaseTexture({ recycleTexture, recycleFence }); + }; + _texture = gpu::TexturePointer(gpu::Texture::createExternal2D(recycler)); _texture->setSource(__FUNCTION__); } OffscreenQmlSurface::TextureAndFence newTextureAndFence; @@ -352,16 +366,7 @@ void RenderableWebEntityItem::destroyWebSurface() { _mouseMoveConnection = QMetaObject::Connection(); QObject::disconnect(_hoverLeaveConnection); _hoverLeaveConnection = QMetaObject::Connection(); - - // The lifetime of the QML surface MUST be managed by the main thread - // Additionally, we MUST use local variables copied by value, rather than - // member variables, since they would implicitly refer to a this that - // is no longer valid - auto webSurface = _webSurface; - AbstractViewStateInterface::instance()->postLambdaEvent([webSurface] { - webSurface->deleteLater(); - }); - _webSurface = nullptr; + _webSurface.reset(); } } diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.h b/libraries/entities-renderer/src/RenderableWebEntityItem.h index ea9ddd0c12..b7caaae68c 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.h +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.h @@ -78,7 +78,7 @@ private: void destroyWebSurface(); glm::vec2 getWindowSize() const; - OffscreenQmlSurface* _webSurface{ nullptr }; + QSharedPointer _webSurface; QMetaObject::Connection _connection; gpu::TexturePointer _texture; ivec2 _lastPress { INT_MIN }; diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp index b39bc58bac..649065ab84 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp @@ -182,8 +182,11 @@ GLTexture::~GLTexture() { if (backend) { if (_external) { auto recycler = _gpuObject.getExternalRecycler(); - assert(recycler); - backend->releaseExternalTexture(_id, recycler); + if (recycler) { + backend->releaseExternalTexture(_id, recycler); + } else { + qWarning() << "No recycler available for texture " << _id << " possible leak"; + } } else if (_id) { backend->releaseTexture(_id, _size); }