From ca3572f991660352ebd14d467513f03902322b88 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 3 Oct 2016 14:13:10 -0700 Subject: [PATCH 1/2] Support external GL textures with proper fencing --- interface/src/ui/ApplicationOverlay.cpp | 43 ++++---- interface/src/ui/ApplicationOverlay.h | 2 +- interface/src/ui/overlays/Web3DOverlay.cpp | 22 +++-- interface/src/ui/overlays/Web3DOverlay.h | 2 +- .../src/RenderableWebEntityItem.cpp | 20 ++-- .../src/RenderableWebEntityItem.h | 4 +- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 99 ++++++++++++++++--- libraries/gl/src/gl/OffscreenQmlSurface.h | 12 ++- libraries/gpu-gl/src/gpu/gl/GLBackend.cpp | 28 ++++-- libraries/gpu-gl/src/gpu/gl/GLBackend.h | 4 +- libraries/gpu-gl/src/gpu/gl/GLTexture.cpp | 35 ++++++- libraries/gpu-gl/src/gpu/gl/GLTexture.h | 39 ++++++++ libraries/gpu-gl/src/gpu/gl41/GL41Backend.h | 1 + .../src/gpu/gl41/GL41BackendTexture.cpp | 8 +- libraries/gpu-gl/src/gpu/gl45/GL45Backend.h | 1 + .../src/gpu/gl45/GL45BackendTexture.cpp | 9 +- libraries/gpu/src/gpu/Batch.cpp | 19 ++-- libraries/gpu/src/gpu/Batch.h | 5 - libraries/gpu/src/gpu/Texture.cpp | 23 +++++ libraries/gpu/src/gpu/Texture.h | 21 +++- 20 files changed, 308 insertions(+), 89 deletions(-) diff --git a/interface/src/ui/ApplicationOverlay.cpp b/interface/src/ui/ApplicationOverlay.cpp index 197fb5b58d..f4937c4459 100644 --- a/interface/src/ui/ApplicationOverlay.cpp +++ b/interface/src/ui/ApplicationOverlay.cpp @@ -40,15 +40,6 @@ ApplicationOverlay::ApplicationOverlay() auto geometryCache = DependencyManager::get(); _domainStatusBorder = geometryCache->allocateID(); _magnifierBorder = geometryCache->allocateID(); - - // Once we move UI rendering and screen rendering to different - // threads, we need to use a sync object to deteremine when - // the current UI texture is no longer being read from, and only - // then release it back to the UI for re-use - auto offscreenUi = DependencyManager::get(); - connect(offscreenUi.data(), &OffscreenUi::textureUpdated, this, [&](GLuint textureId) { - _uiTexture = textureId; - }); } ApplicationOverlay::~ApplicationOverlay() { @@ -96,18 +87,32 @@ void ApplicationOverlay::renderOverlay(RenderArgs* renderArgs) { void ApplicationOverlay::renderQmlUi(RenderArgs* renderArgs) { PROFILE_RANGE(__FUNCTION__); - if (_uiTexture) { - gpu::Batch& batch = *renderArgs->_batch; - auto geometryCache = DependencyManager::get(); - geometryCache->useSimpleDrawPipeline(batch); - batch.setProjectionTransform(mat4()); - batch.setModelTransform(Transform()); - batch.resetViewTransform(); - batch._glActiveBindTexture(GL_TEXTURE0, GL_TEXTURE_2D, _uiTexture); - - geometryCache->renderUnitQuad(batch, glm::vec4(1)); + if (!_uiTexture) { + _uiTexture = gpu::TexturePointer(gpu::Texture::createExternal2D([](uint32_t recycleTexture, void* recycleFence){ + DependencyManager::get()->releaseTexture({ recycleTexture, recycleFence }); + })); + _uiTexture->setSource(__FUNCTION__); } + // Once we move UI rendering and screen rendering to different + // threads, we need to use a sync object to deteremine when + // the current UI texture is no longer being read from, and only + // then release it back to the UI for re-use + auto offscreenUi = DependencyManager::get(); + + OffscreenQmlSurface::TextureAndFence newTextureAndFence; + bool newTextureAvailable = offscreenUi->fetchTexture(newTextureAndFence); + if (newTextureAvailable) { + _uiTexture->setExternalTexture(newTextureAndFence.first, newTextureAndFence.second); + } + auto geometryCache = DependencyManager::get(); + gpu::Batch& batch = *renderArgs->_batch; + geometryCache->useSimpleDrawPipeline(batch); + batch.setProjectionTransform(mat4()); + batch.setModelTransform(Transform()); + batch.resetViewTransform(); + batch.setResourceTexture(0, _uiTexture); + geometryCache->renderUnitQuad(batch, glm::vec4(1)); } void ApplicationOverlay::renderAudioScope(RenderArgs* renderArgs) { diff --git a/interface/src/ui/ApplicationOverlay.h b/interface/src/ui/ApplicationOverlay.h index b7a0529f92..d20b569457 100644 --- a/interface/src/ui/ApplicationOverlay.h +++ b/interface/src/ui/ApplicationOverlay.h @@ -40,13 +40,13 @@ private: float _alpha{ 1.0f }; float _trailingAudioLoudness{ 0.0f }; - uint32_t _uiTexture{ 0 }; int _domainStatusBorder; int _magnifierBorder; ivec2 _previousBorderSize{ -1 }; + gpu::TexturePointer _uiTexture; gpu::TexturePointer _overlayDepthTexture; gpu::TexturePointer _overlayColorTexture; gpu::FramebufferPointer _overlayFramebuffer; diff --git a/interface/src/ui/overlays/Web3DOverlay.cpp b/interface/src/ui/overlays/Web3DOverlay.cpp index 46fc2dfc36..8f213e7740 100644 --- a/interface/src/ui/overlays/Web3DOverlay.cpp +++ b/interface/src/ui/overlays/Web3DOverlay.cpp @@ -80,9 +80,6 @@ void Web3DOverlay::render(RenderArgs* args) { _webSurface->resume(); _webSurface->getRootItem()->setProperty("url", _url); _webSurface->resize(QSize(_resolution.x, _resolution.y)); - _connection = QObject::connect(_webSurface, &OffscreenQmlSurface::textureUpdated, [&](GLuint textureId) { - _texture = textureId; - }); currentContext->makeCurrent(currentSurface); } @@ -97,14 +94,21 @@ void Web3DOverlay::render(RenderArgs* args) { transform.postScale(vec3(getDimensions(), 1.0f)); } - Q_ASSERT(args->_batch); - gpu::Batch& batch = *args->_batch; - if (_texture) { - batch._glActiveBindTexture(GL_TEXTURE0, GL_TEXTURE_2D, _texture); - } else { - batch.setResourceTexture(0, DependencyManager::get()->getWhiteTexture()); + if (!_texture) { + _texture = gpu::TexturePointer(gpu::Texture::createExternal2D([this](uint32_t recycleTexture, void* recycleFence) { + _webSurface->releaseTexture({ recycleTexture, recycleFence }); + })); + _texture->setSource(__FUNCTION__); + } + OffscreenQmlSurface::TextureAndFence newTextureAndFence; + bool newTextureAvailable = _webSurface->fetchTexture(newTextureAndFence); + if (newTextureAvailable) { + _texture->setExternalTexture(newTextureAndFence.first, newTextureAndFence.second); } + Q_ASSERT(args->_batch); + gpu::Batch& batch = *args->_batch; + batch.setResourceTexture(0, _texture); batch.setModelTransform(transform); auto geometryCache = DependencyManager::get(); if (color.a < OPAQUE_ALPHA_THRESHOLD) { diff --git a/interface/src/ui/overlays/Web3DOverlay.h b/interface/src/ui/overlays/Web3DOverlay.h index a828626715..0bb9e5e030 100644 --- a/interface/src/ui/overlays/Web3DOverlay.h +++ b/interface/src/ui/overlays/Web3DOverlay.h @@ -43,7 +43,7 @@ public: private: OffscreenQmlSurface* _webSurface{ nullptr }; QMetaObject::Connection _connection; - uint32_t _texture{ 0 }; + gpu::TexturePointer _texture; QString _url; float _dpi; vec2 _resolution{ 640, 480 }; diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index 86bce87ba2..3c339b7b7d 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -140,9 +140,6 @@ bool RenderableWebEntityItem::buildWebSurface(EntityTreeRenderer* renderer) { _webSurface->getRootItem()->setProperty("url", _sourceUrl); _webSurface->getRootContext()->setContextProperty("desktop", QVariant()); _webSurface->getRootContext()->setContextProperty("webEntity", _webEntityAPIHelper); - _connection = QObject::connect(_webSurface, &OffscreenQmlSurface::textureUpdated, [&](GLuint textureId) { - _texture = textureId; - }); // Restore the original GL context currentContext->makeCurrent(currentSurface); @@ -217,20 +214,31 @@ void RenderableWebEntityItem::render(RenderArgs* args) { // without worrying about excessive overhead. _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 }); + })); + _texture->setSource(__FUNCTION__); + } + OffscreenQmlSurface::TextureAndFence newTextureAndFence; + bool newTextureAvailable = _webSurface->fetchTexture(newTextureAndFence); + if (newTextureAvailable) { + _texture->setExternalTexture(newTextureAndFence.first, newTextureAndFence.second); + } + PerformanceTimer perfTimer("RenderableWebEntityItem::render"); Q_ASSERT(getType() == EntityTypes::Web); static const glm::vec2 texMin(0.0f), texMax(1.0f), topLeft(-0.5f), bottomRight(0.5f); Q_ASSERT(args->_batch); gpu::Batch& batch = *args->_batch; + bool success; batch.setModelTransform(getTransformToCenter(success)); if (!success) { return; } - if (_texture) { - batch._glActiveBindTexture(GL_TEXTURE0, GL_TEXTURE_2D, _texture); - } + batch.setResourceTexture(0, _texture); float fadeRatio = _isFading ? Interpolate::calculateFadeRatio(_fadeStartTime) : 1.0f; diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.h b/libraries/entities-renderer/src/RenderableWebEntityItem.h index 47808c4262..ea9ddd0c12 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.h +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.h @@ -80,8 +80,8 @@ private: OffscreenQmlSurface* _webSurface{ nullptr }; QMetaObject::Connection _connection; - uint32_t _texture{ 0 }; - ivec2 _lastPress{ INT_MIN }; + gpu::TexturePointer _texture; + ivec2 _lastPress { INT_MIN }; bool _pressed{ false }; QTouchEvent _lastTouchEvent { QEvent::TouchUpdate }; uint64_t _lastRenderTime{ 0 }; diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index d1c884f264..797f297488 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -34,7 +34,6 @@ #include #include "OffscreenGLCanvas.h" -#include "GLEscrow.h" #include "GLHelpers.h" #include "GLLogging.h" @@ -265,6 +264,14 @@ private: // Helper methods void setupFbo(); bool allowNewFrame(uint8_t fps); + bool fetchTexture(OffscreenQmlSurface::TextureAndFence& textureAndFence); + void releaseTexture(const OffscreenQmlSurface::TextureAndFence& textureAndFence); + + // Texture management + std::mutex _textureMutex; + GLuint _latestTexture { 0 }; + GLsync _latestTextureFence { 0 }; + std::list _returnedTextures; // Rendering members OffscreenGLCanvas _canvas; @@ -274,7 +281,6 @@ private: GLuint _fbo { 0 }; GLuint _depthStencil { 0 }; RawTextureRecycler _textures { true }; - GLTextureEscrow _escrow; uint64_t _lastRenderTime{ 0 }; uvec2 _size{ 1920, 1080 }; @@ -406,9 +412,6 @@ void OffscreenQmlRenderThread::init() { _renderControl->initialize(_canvas.getContext()); setupFbo(); - _escrow.setRecycler([this](GLuint texture){ - _textures.recycleTexture(texture); - }); } void OffscreenQmlRenderThread::cleanup() { @@ -485,27 +488,93 @@ void OffscreenQmlRenderThread::render() { _quickWindow->setRenderTarget(_fbo, QSize(_size.x, _size.y)); + // Clear out any pending textures to be returned + { + std::list returnedTextures; + { + std::unique_lock lock(_textureMutex); + returnedTextures.swap(_returnedTextures); + } + if (!returnedTextures.empty()) { + for (const auto& textureAndFence : returnedTextures) { + GLsync fence = static_cast(textureAndFence.second); + if (fence) { + glWaitSync(fence, 0, GL_TIMEOUT_IGNORED); + glDeleteSync(fence); + } + _textures.recycleTexture(textureAndFence.first); + } + } + } + try { GLuint texture = _textures.getNextTexture(); glBindFramebuffer(GL_DRAW_FRAMEBUFFER, _fbo); glFramebufferTexture(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, texture, 0); - PROFILE_RANGE("qml_render->rendercontrol") - _renderControl->render(); - + PROFILE_RANGE("qml_render->rendercontrol") + _renderControl->render(); glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); glBindTexture(GL_TEXTURE_2D, texture); glGenerateMipmap(GL_TEXTURE_2D); glBindTexture(GL_TEXTURE_2D, 0); + { + std::unique_lock lock(_textureMutex); + // If the most recent texture was unused, we can directly recycle it + if (_latestTextureFence) { + } + if (_latestTexture) { + _textures.recycleTexture(_latestTexture); + glDeleteSync(_latestTextureFence); + _latestTexture = 0; + _latestTextureFence = 0; + } + + _latestTextureFence = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); + _latestTexture = texture; + // Fence will be used in another thread / context, so a flush is required + glFlush(); + } + _quickWindow->resetOpenGLState(); - _escrow.submit(texture); _lastRenderTime = usecTimestampNow(); } catch (std::runtime_error& error) { qWarning() << "Failed to render QML: " << error.what(); } } +bool OffscreenQmlRenderThread::fetchTexture(OffscreenQmlSurface::TextureAndFence& textureAndFence) { + textureAndFence = { 0, 0 }; + + std::unique_lock lock(_textureMutex); + if (0 == _latestTexture) { + return false; + } + + // Ensure writes to the latest texture are complete before before returning it for reading + Q_ASSERT(0 != _latestTextureFence); + textureAndFence = { _latestTexture, _latestTextureFence }; + _latestTextureFence = 0; + _latestTexture = 0; + return true; +} + +void OffscreenQmlRenderThread::releaseTexture(const OffscreenQmlSurface::TextureAndFence& textureAndFence) { + std::unique_lock lock(_textureMutex); + _returnedTextures.push_back(textureAndFence); +} + bool OffscreenQmlRenderThread::allowNewFrame(uint8_t fps) { + // If we already have a pending texture, don't render another one + // i.e. don't render faster than the consumer context, since it wastes + // GPU cycles on producing output that will never be seen + { + std::unique_lock lock(_textureMutex); + if (0 != _latestTexture) { + return false; + } + } + auto minRenderInterval = USECS_PER_SECOND / fps; auto lastInterval = usecTimestampNow() - _lastRenderTime; return (lastInterval > minRenderInterval); @@ -726,13 +795,18 @@ void OffscreenQmlSurface::updateQuick() { // Lock the GUI size while syncing QMutexLocker locker(&(_renderer->_mutex)); _renderer->_queue.add(RENDER); + // FIXME need to find a better way to handle the render lockout than this locking of the main thread _renderer->_waitCondition.wait(&(_renderer->_mutex)); _render = false; } +} - if (_renderer->_escrow.fetchSignaledAndRelease(_currentTexture)) { - emit textureUpdated(_currentTexture); - } +bool OffscreenQmlSurface::fetchTexture(TextureAndFence& texture) { + return _renderer->fetchTexture(texture); +} + +void OffscreenQmlSurface::releaseTexture(const TextureAndFence& texture) { + _renderer->releaseTexture(texture); } QPointF OffscreenQmlSurface::mapWindowToUi(const QPointF& sourcePosition, QObject* sourceObject) { @@ -752,7 +826,6 @@ QPointF OffscreenQmlSurface::mapToVirtualScreen(const QPointF& originalPoint, QO return _mouseTranslator(originalPoint); } - /////////////////////////////////////////////////////// // // Event handling customization diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.h b/libraries/gl/src/gl/OffscreenQmlSurface.h index a9a77f2941..fa2346dd2f 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.h +++ b/libraries/gl/src/gl/OffscreenQmlSurface.h @@ -71,8 +71,17 @@ public: QPointF mapToVirtualScreen(const QPointF& originalPoint, QObject* originalWidget); bool eventFilter(QObject* originalDestination, QEvent* event) override; + using TextureAndFence = std::pair; + // Checks to see if a new texture is available. If one is, the function returns true and + // textureAndFence will be populated with the texture ID and a fence which will be signalled + // when the texture is safe to read. + // Returns false if no new texture is available + bool fetchTexture(TextureAndFence& textureAndFence); + // Release a previously acquired texture, along with a fence which indicates when reads from the + // texture have completed. + void releaseTexture(const TextureAndFence& textureAndFence); + signals: - void textureUpdated(unsigned int texture); void focusObjectChanged(QObject* newFocus); void focusTextChanged(bool focusText); @@ -100,7 +109,6 @@ private: QQmlComponent* _qmlComponent{ nullptr }; QQuickItem* _rootItem{ nullptr }; QTimer _updateTimer; - uint32_t _currentTexture{ 0 }; bool _render{ false }; bool _polish{ true }; bool _paused{ true }; diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp index f070dfe637..c082c00609 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp @@ -119,8 +119,6 @@ GLBackend::CommandCall GLBackend::_commandCalls[Batch::NUM_COMMANDS] = (&::gpu::gl::GLBackend::do_startNamedCall), (&::gpu::gl::GLBackend::do_stopNamedCall), - (&::gpu::gl::GLBackend::do_glActiveBindTexture), - (&::gpu::gl::GLBackend::do_glUniform1i), (&::gpu::gl::GLBackend::do_glUniform1f), (&::gpu::gl::GLBackend::do_glUniform2f), @@ -388,14 +386,6 @@ void GLBackend::do_popProfileRange(const Batch& batch, size_t paramOffset) { // As long as we don;t use several versions of shaders we can avoid this more complex code path // #define GET_UNIFORM_LOCATION(shaderUniformLoc) _pipeline._programShader->getUniformLocation(shaderUniformLoc, isStereo()); #define GET_UNIFORM_LOCATION(shaderUniformLoc) shaderUniformLoc -void GLBackend::do_glActiveBindTexture(const Batch& batch, size_t paramOffset) { - glActiveTexture(batch._params[paramOffset + 2]._uint); - glBindTexture( - GET_UNIFORM_LOCATION(batch._params[paramOffset + 1]._uint), - batch._params[paramOffset + 0]._uint); - - (void)CHECK_GL_ERROR(); -} void GLBackend::do_glUniform1i(const Batch& batch, size_t paramOffset) { if (_pipeline._program == 0) { @@ -568,6 +558,11 @@ void GLBackend::releaseBuffer(GLuint id, Size size) const { _buffersTrash.push_back({ id, size }); } +void GLBackend::releaseExternalTexture(GLuint id, const Texture::ExternalRecycler& recycler) const { + Lock lock(_trashMutex); + _externalTexturesTrash.push_back({ id, recycler }); +} + void GLBackend::releaseTexture(GLuint id, Size size) const { Lock lock(_trashMutex); _texturesTrash.push_back({ id, size }); @@ -662,6 +657,19 @@ void GLBackend::recycle() const { } } + { + std::list> externalTexturesTrash; + { + Lock lock(_trashMutex); + std::swap(_externalTexturesTrash, externalTexturesTrash); + } + for (auto pair : externalTexturesTrash) { + auto fence = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); + pair.second(pair.first, fence); + decrementTextureGPUCount(); + } + } + { std::list programsTrash; { diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.h b/libraries/gpu-gl/src/gpu/gl/GLBackend.h index af4851a1b0..f99d34393c 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.h @@ -130,8 +130,6 @@ public: // TODO: As long as we have gl calls explicitely issued from interface // code, we need to be able to record and batch these calls. THe long // term strategy is to get rid of any GL calls in favor of the HIFI GPU API - virtual void do_glActiveBindTexture(const Batch& batch, size_t paramOffset) final; - virtual void do_glUniform1i(const Batch& batch, size_t paramOffset) final; virtual void do_glUniform1f(const Batch& batch, size_t paramOffset) final; virtual void do_glUniform2f(const Batch& batch, size_t paramOffset) final; @@ -170,6 +168,7 @@ public: virtual bool isTextureReady(const TexturePointer& texture); virtual void releaseBuffer(GLuint id, Size size) const; + virtual void releaseExternalTexture(GLuint id, const Texture::ExternalRecycler& recycler) const; virtual void releaseTexture(GLuint id, Size size) const; virtual void releaseFramebuffer(GLuint id) const; virtual void releaseShader(GLuint id) const; @@ -194,6 +193,7 @@ protected: mutable Mutex _trashMutex; mutable std::list> _buffersTrash; mutable std::list> _texturesTrash; + mutable std::list> _externalTexturesTrash; mutable std::list _framebuffersTrash; mutable std::list _shadersTrash; mutable std::list _programsTrash; diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp index 56ff4166ea..b39bc58bac 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp @@ -136,6 +136,7 @@ float GLTexture::getMemoryPressure() { // Create the texture and allocate storage GLTexture::GLTexture(const std::weak_ptr& backend, const Texture& texture, GLuint id, bool transferrable) : GLObject(backend, texture, id), + _external(false), _source(texture.source()), _storageStamp(texture.getStamp()), _target(getGLTextureType(texture)), @@ -152,10 +153,38 @@ GLTexture::GLTexture(const std::weak_ptr& backend, const Texture& tex Backend::setGPUObject(texture, this); } +GLTexture::GLTexture(const std::weak_ptr& backend, const Texture& texture, GLuint id) : + GLObject(backend, texture, id), + _external(true), + _source(texture.source()), + _storageStamp(0), + _target(getGLTextureType(texture)), + _internalFormat(GL_RGBA8), + // FIXME force mips to 0? + _maxMip(texture.maxMip()), + _minMip(texture.minMip()), + _virtualSize(0), + _transferrable(false) +{ + Backend::setGPUObject(texture, this); + + // FIXME Is this necessary? + //withPreservedTexture([this] { + // syncSampler(); + // if (_gpuObject.isAutogenerateMips()) { + // generateMips(); + // } + //}); +} + GLTexture::~GLTexture() { - if (_id) { - auto backend = _backend.lock(); - if (backend) { + auto backend = _backend.lock(); + if (backend) { + if (_external) { + auto recycler = _gpuObject.getExternalRecycler(); + assert(recycler); + backend->releaseExternalTexture(_id, recycler); + } else if (_id) { backend->releaseTexture(_id, _size); } } diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.h b/libraries/gpu-gl/src/gpu/gl/GLTexture.h index 4375d0644f..9091a63086 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.h +++ b/libraries/gpu-gl/src/gpu/gl/GLTexture.h @@ -32,6 +32,42 @@ public: template static GLTextureType* sync(GLBackend& backend, const TexturePointer& texturePointer, bool needTransfer) { const Texture& texture = *texturePointer; + + // Special case external textures + if (texture.getUsage().isExternal()) { + Texture::ExternalUpdates updates = texture.getUpdates(); + if (!updates.empty()) { + Texture::ExternalRecycler recycler = texture.getExternalRecycler(); + Q_ASSERT(recycler); + // Discard any superfluous updates + while (updates.size() > 1) { + const auto& update = updates.front(); + // Superfluous updates will never have been read, but we want to ensure the previous + // writes to them are complete before they're written again, so return them with the + // same fences they arrived with. This can happen on any thread because no GL context + // work is involved + recycler(update.first, update.second); + updates.pop_front(); + } + + // The last texture remaining is the one we'll use to create the GLTexture + const auto& update = updates.front(); + // Check for a fence, and if it exists, inject a wait into the command stream, then destroy the fence + if (update.second) { + GLsync fence = static_cast(update.second); + glWaitSync(fence, 0, GL_TIMEOUT_IGNORED); + glDeleteSync(fence); + } + + // Create the new texture object (replaces any previous texture object) + new GLTextureType(backend.shared_from_this(), texture, update.first); + } + + // Return the texture object (if any) associated with the texture, without extensive logic + // (external textures are + return Backend::getGPUObject(texture); + } + if (!texture.isDefined()) { // NO texture definition yet so let's avoid thinking return nullptr; @@ -110,6 +146,8 @@ public: ~GLTexture(); + // Is this texture generated outside the GPU library? + const bool _external; const GLuint& _texture { _id }; const std::string _source; const Stamp _storageStamp; @@ -159,6 +197,7 @@ protected: std::atomic _syncState { GLSyncState::Idle }; GLTexture(const std::weak_ptr& backend, const Texture& texture, GLuint id, bool transferrable); + GLTexture(const std::weak_ptr& backend, const Texture& texture, GLuint id); void setSyncState(GLSyncState syncState) { _syncState = syncState; } diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h b/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h index 5d03997b44..3f1f45624a 100644 --- a/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h +++ b/libraries/gpu-gl/src/gpu/gl41/GL41Backend.h @@ -42,6 +42,7 @@ public: using Parent = GLTexture; GLuint allocate(); public: + GL41Texture(const std::weak_ptr& backend, const Texture& buffer, GLuint externalId); GL41Texture(const std::weak_ptr& backend, const Texture& buffer, bool transferrable); protected: diff --git a/libraries/gpu-gl/src/gpu/gl41/GL41BackendTexture.cpp b/libraries/gpu-gl/src/gpu/gl41/GL41BackendTexture.cpp index 3fb729711d..5594be36b6 100644 --- a/libraries/gpu-gl/src/gpu/gl41/GL41BackendTexture.cpp +++ b/libraries/gpu-gl/src/gpu/gl41/GL41BackendTexture.cpp @@ -38,7 +38,13 @@ GLTexture* GL41Backend::syncGPUObject(const TexturePointer& texture, bool transf return GL41Texture::sync(*this, texture, transfer); } -GL41Texture::GL41Texture(const std::weak_ptr& backend, const Texture& texture, bool transferrable) : GLTexture(backend, texture, allocate(), transferrable) {} +GL41Texture::GL41Texture(const std::weak_ptr& backend, const Texture& texture, GLuint externalId) + : GLTexture(backend, texture, externalId) { +} + +GL41Texture::GL41Texture(const std::weak_ptr& backend, const Texture& texture, bool transferrable) + : GLTexture(backend, texture, allocate(), transferrable) { +} void GL41Texture::generateMips() const { withPreservedTexture([&] { diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h b/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h index f1c30b9382..059156b4a3 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h +++ b/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h @@ -33,6 +33,7 @@ public: static const uint32_t DEFAULT_PAGE_DIMENSION = 128; static const uint32_t DEFAULT_MAX_SPARSE_LEVEL = 0xFFFF; public: + GL45Texture(const std::weak_ptr& backend, const Texture& texture, GLuint externalId); GL45Texture(const std::weak_ptr& backend, const Texture& texture, bool transferrable); ~GL45Texture(); diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendTexture.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendTexture.cpp index 36b7b4886f..c59c945531 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendTexture.cpp +++ b/libraries/gpu-gl/src/gpu/gl45/GL45BackendTexture.cpp @@ -243,6 +243,10 @@ GLuint GL45Backend::getTextureID(const TexturePointer& texture, bool transfer) { return GL45Texture::getId(*this, texture, transfer); } +GL45Texture::GL45Texture(const std::weak_ptr& backend, const Texture& texture, GLuint externalId) + : GLTexture(backend, texture, externalId), _sparseInfo(*this), _transferState(*this) { +} + GL45Texture::GL45Texture(const std::weak_ptr& backend, const Texture& texture, bool transferrable) : GLTexture(backend, texture, allocate(texture), transferrable), _sparseInfo(*this), _transferState(*this) { @@ -252,7 +256,10 @@ GL45Texture::GL45Texture(const std::weak_ptr& backend, const Texture& } GL45Texture::~GL45Texture() { - qCDebug(gpugl45logging) << "Destroying texture " << _id << " from source " << _source.c_str(); + // External textures cycle very quickly, so don't spam the log with messages about them. + if (!_gpuObject.getUsage().isExternal()) { + qCDebug(gpugl45logging) << "Destroying texture " << _id << " from source " << _source.c_str(); + } if (_sparseInfo.sparse) { // Remove this texture from the candidate list of derezzable textures { diff --git a/libraries/gpu/src/gpu/Batch.cpp b/libraries/gpu/src/gpu/Batch.cpp index 8d3f019168..c15da61800 100644 --- a/libraries/gpu/src/gpu/Batch.cpp +++ b/libraries/gpu/src/gpu/Batch.cpp @@ -294,6 +294,11 @@ void Batch::setUniformBuffer(uint32 slot, const BufferView& view) { void Batch::setResourceTexture(uint32 slot, const TexturePointer& texture) { + if (texture && texture->getUsage().isExternal()) { + auto recycler = texture->getExternalRecycler(); + Q_ASSERT(recycler); + } + ADD_COMMAND(setResourceTexture); _params.emplace_back(_textures.cache(texture)); @@ -506,18 +511,6 @@ void Batch::popProfileRange() { #endif } -#define GL_TEXTURE0 0x84C0 - -void Batch::_glActiveBindTexture(uint32 unit, uint32 target, uint32 texture) { - // clean the cache on the texture unit we are going to use so the next call to setResourceTexture() at the same slot works fine - setResourceTexture(unit - GL_TEXTURE0, nullptr); - - ADD_COMMAND(glActiveBindTexture); - _params.emplace_back(texture); - _params.emplace_back(target); - _params.emplace_back(unit); -} - void Batch::_glUniform1i(int32 location, int32 v0) { if (location < 0) { return; @@ -680,4 +673,4 @@ void Batch::flush() { } buffer->flush(); } -} \ No newline at end of file +} diff --git a/libraries/gpu/src/gpu/Batch.h b/libraries/gpu/src/gpu/Batch.h index 8a52eef4ea..ad8155774d 100644 --- a/libraries/gpu/src/gpu/Batch.h +++ b/libraries/gpu/src/gpu/Batch.h @@ -229,9 +229,6 @@ public: // term strategy is to get rid of any GL calls in favor of the HIFI GPU API // For now, instead of calling the raw gl Call, use the equivalent call on the batch so the call is beeing recorded // THe implementation of these functions is in GLBackend.cpp - - void _glActiveBindTexture(unsigned int unit, unsigned int target, unsigned int texture); - void _glUniform1i(int location, int v0); void _glUniform1f(int location, float v0); void _glUniform2f(int location, float v0, float v1); @@ -314,8 +311,6 @@ public: // TODO: As long as we have gl calls explicitely issued from interface // code, we need to be able to record and batch these calls. THe long // term strategy is to get rid of any GL calls in favor of the HIFI GPU API - COMMAND_glActiveBindTexture, - COMMAND_glUniform1i, COMMAND_glUniform1f, COMMAND_glUniform2f, diff --git a/libraries/gpu/src/gpu/Texture.cpp b/libraries/gpu/src/gpu/Texture.cpp index 44804abebe..924f5145b9 100755 --- a/libraries/gpu/src/gpu/Texture.cpp +++ b/libraries/gpu/src/gpu/Texture.cpp @@ -238,6 +238,16 @@ bool Texture::Storage::assignMipFaceData(uint16 level, const Element& format, Si return allocated == size; } +Texture* Texture::createExternal2D(const ExternalRecycler& recycler, const Sampler& sampler) { + Texture* tex = new Texture(); + tex->_type = TEX_2D; + tex->_maxMip = 0; + tex->_sampler = sampler; + tex->setUsage(Usage::Builder().withExternal().withColor()); + tex->setExternalRecycler(recycler); + return tex; +} + Texture* Texture::create1D(const Element& texelFormat, uint16 width, const Sampler& sampler) { return create(TEX_1D, texelFormat, width, 1, 1, 1, 1, sampler); } @@ -925,3 +935,16 @@ Vec3u Texture::evalMipDimensions(uint16 level) const { return glm::max(dimensions, Vec3u(1)); } +void Texture::setExternalTexture(uint32 externalId, void* externalFence) { + Lock lock(_externalMutex); + _externalUpdates.push_back({ externalId, externalFence }); +} + +Texture::ExternalUpdates Texture::getUpdates() const { + Texture::ExternalUpdates result; + { + Lock lock(_externalMutex); + _externalUpdates.swap(result); + } + return result; +} \ No newline at end of file diff --git a/libraries/gpu/src/gpu/Texture.h b/libraries/gpu/src/gpu/Texture.h index 61d03c070c..9c3e88c67a 100755 --- a/libraries/gpu/src/gpu/Texture.h +++ b/libraries/gpu/src/gpu/Texture.h @@ -163,6 +163,10 @@ public: static void setEnableSparseTextures(bool enabled); static void setEnableIncrementalTextureTransfers(bool enabled); + using ExternalRecycler = std::function; + using ExternalIdAndFence = std::pair; + using ExternalUpdates = std::list; + class Usage { public: enum FlagBit { @@ -170,7 +174,7 @@ public: NORMAL, // Texture is a normal map ALPHA, // Texture has an alpha channel ALPHA_MASK, // Texture alpha channel is a Mask 0/1 - + EXTERNAL, NUM_FLAGS, }; typedef std::bitset Flags; @@ -196,6 +200,7 @@ public: Builder& withNormal() { _flags.set(NORMAL); return (*this); } Builder& withAlpha() { _flags.set(ALPHA); return (*this); } Builder& withAlphaMask() { _flags.set(ALPHA_MASK); return (*this); } + Builder& withExternal() { _flags.set(EXTERNAL); return (*this); } }; Usage(const Builder& builder) : Usage(builder._flags) {} @@ -204,6 +209,7 @@ public: bool isAlpha() const { return _flags[ALPHA]; } bool isAlphaMask() const { return _flags[ALPHA_MASK]; } + bool isExternal() const { return _flags[EXTERNAL]; } bool operator==(const Usage& usage) { return (_flags == usage._flags); } @@ -293,6 +299,7 @@ public: static Texture* create2D(const Element& texelFormat, uint16 width, uint16 height, const Sampler& sampler = Sampler()); static Texture* create3D(const Element& texelFormat, uint16 width, uint16 height, uint16 depth, const Sampler& sampler = Sampler()); static Texture* createCube(const Element& texelFormat, uint16 width, const Sampler& sampler = Sampler()); + static Texture* createExternal2D(const ExternalRecycler& recycler, const Sampler& sampler = Sampler()); Texture(); Texture(const Texture& buf); // deep copy of the sysmem texture @@ -458,9 +465,21 @@ public: // Only callable by the Backend void notifyMipFaceGPULoaded(uint16 level, uint8 face = 0) const { return _storage->notifyMipFaceGPULoaded(level, face); } + void setExternalTexture(uint32 externalId, void* externalFence); + void setExternalRecycler(const ExternalRecycler& recycler) { _externalRecycler = recycler; } + ExternalRecycler getExternalRecycler() const { return _externalRecycler; } + const GPUObjectPointer gpuObject {}; + ExternalUpdates getUpdates() const; + protected: + // Should only be accessed internally or by the backend sync function + mutable Mutex _externalMutex; + mutable std::list _externalUpdates; + ExternalRecycler _externalRecycler; + + // Not strictly necessary, but incredibly useful for debugging std::string _source; std::unique_ptr< Storage > _storage; From 8c5028158fe8a3e874f2da09e1d2825c992ffa2a Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 3 Oct 2016 16:42:50 -0700 Subject: [PATCH 2/2] 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); }