From 513138ca9809cbdef7b263a8ca5c9cb4e733ae15 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Mon, 21 Mar 2016 18:28:26 -0700 Subject: [PATCH 01/15] one more crack at fixing the blender --- libraries/render-utils/src/Model.cpp | 18 ++++++++++++------ libraries/render-utils/src/Model.h | 4 ++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index 16db6532da..df186802f6 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -1277,20 +1277,26 @@ void ModelBlender::noteRequiresBlend(ModelPointer model) { } return; } - _modelsRequiringBlends.insert(model); + + { + Lock lock(_mutex); + _modelsRequiringBlends.insert(model); + } } void ModelBlender::setBlendedVertices(const QPointer& model, int blendNumber, const QWeakPointer& geometry, const QVector& vertices, const QVector& normals) { + if (!model.isNull()) { model->setBlendedVertices(blendNumber, geometry, vertices, normals); } _pendingBlenders--; - while (!_modelsRequiringBlends.empty()) { - auto firstItem = _modelsRequiringBlends.begin(); - if (firstItem != _modelsRequiringBlends.end()) { - _modelsRequiringBlends.erase(firstItem); - ModelPointer nextModel = firstItem->lock(); + { + Lock lock(_mutex); + for (auto i = _modelsRequiringBlends.begin(); i != _modelsRequiringBlends.end();) { + auto weakPtr = *i; + _modelsRequiringBlends.erase(i++); // remove front of the set + ModelPointer nextModel = weakPtr.lock(); if (nextModel && nextModel->maybeStartBlender()) { _pendingBlenders++; return; diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index 1e8b3f2af6..53ed82f418 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -402,11 +402,15 @@ public slots: const QVector& vertices, const QVector& normals); private: + using Mutex = std::mutex; + using Lock = std::unique_lock; + ModelBlender(); virtual ~ModelBlender(); std::set> _modelsRequiringBlends; int _pendingBlenders; + Mutex _mutex; }; From 28f25489f888abbdfbe02f20cdb215410b73834d Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 21 Mar 2016 18:46:04 -0700 Subject: [PATCH 02/15] Fix occasional crash on creating overlay tool windows --- interface/resources/qml/ToolWindow.qml | 68 +++++++++++++++----------- libraries/ui/src/QmlWindowClass.cpp | 3 -- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/interface/resources/qml/ToolWindow.qml b/interface/resources/qml/ToolWindow.qml index 404d1c1e78..09fbf78077 100644 --- a/interface/resources/qml/ToolWindow.qml +++ b/interface/resources/qml/ToolWindow.qml @@ -17,7 +17,6 @@ Windows.Window { visible: false width: 384; height: 640; title: "Tools" - property string newTabSource property alias tabView: tabView onParentChanged: { if (parent) { @@ -32,26 +31,21 @@ Windows.Window { property alias y: toolWindow.y } - property var webTabCreator: Component { - Controls.WebView { - id: webView - property string originalUrl; - - // Both toolWindow.newTabSource and url can change, so we need - // to store the original url here, without creating any bindings - Component.onCompleted: { - originalUrl = toolWindow.newTabSource; - url = originalUrl; - } - } - } - TabView { anchors.fill: parent id: tabView; - onCountChanged: { - if (0 == count) { - toolWindow.visible = false + Repeater { + model: 4 + Tab { + active: true + enabled: false; + // we need to store the original url here for future identification + property string originalUrl: ""; + onEnabledChanged: toolWindow.updateVisiblity(); + Controls.WebView { + id: webView; + anchors.fill: parent + } } } } @@ -68,8 +62,7 @@ Windows.Window { function findIndexForUrl(source) { for (var i = 0; i < tabView.count; ++i) { var tab = tabView.getTab(i); - if (tab && tab.item && tab.item.originalUrl && - tab.item.originalUrl === source) { + if (tab.originalUrl === source) { return i; } } @@ -101,21 +94,32 @@ Windows.Window { } } + function findFreeTab() { + for (var i = 0; i < tabView.count; ++i) { + var tab = tabView.getTab(i); + if (tab && !tab.originalUrl || tab.originalUrl === "") { + return i; + } + } + console.warn("Could not find free tab"); + return -1; + } + function removeTabForUrl(source) { var index = findIndexForUrl(source); if (index < 0) { return; } + var tab = tabView.getTab(index); - tab.enabledChanged.disconnect(updateVisiblity); - tabView.removeTab(index); - console.log("Updating visibility based on child tab removed"); - updateVisiblity(); + tab.title = ""; + tab.originalUrl = ""; + tab.enabled = false; } function addWebTab(properties) { if (!properties.source) { - console.warn("Attempted to open Web Tool Pane without URl") + console.warn("Attempted to open Web Tool Pane without URL") return; } @@ -125,11 +129,17 @@ Windows.Window { return tabView.getTab(existingTabIndex); } - var title = properties.title || "Unknown"; - newTabSource = properties.source; - var newTab = tabView.addTab(title, webTabCreator); + var freeTabIndex = findFreeTab(); + if (freeTabIndex === -1) { + console.warn("Unable to add new tab"); + return; + } + + var newTab = tabView.getTab(freeTabIndex); + newTab.title = properties.title || "Unknown"; + newTab.originalUrl = properties.source; + newTab.item.url = properties.source; newTab.active = true; - newTab.enabled = false; if (properties.width) { tabView.width = Math.min(Math.max(tabView.width, properties.width), diff --git a/libraries/ui/src/QmlWindowClass.cpp b/libraries/ui/src/QmlWindowClass.cpp index 5bd40b10a9..e986ea0441 100644 --- a/libraries/ui/src/QmlWindowClass.cpp +++ b/libraries/ui/src/QmlWindowClass.cpp @@ -172,7 +172,6 @@ QScriptValue QmlWindowClass::internalConstructor(const QString& qmlSource, setupServer(); retVal = builder(newTab); retVal->_toolWindow = true; - offscreenUi->getRootContext()->engine()->setObjectOwnership(retVal->_qmlWindow, QQmlEngine::CppOwnership); registerObject(url.toLower(), retVal); return QVariant(); }); @@ -330,10 +329,8 @@ void QmlWindowClass::close() { if (_qmlWindow) { if (_toolWindow) { auto offscreenUi = DependencyManager::get(); - auto qmlWindow = _qmlWindow; offscreenUi->executeOnUiThread([=] { auto toolWindow = offscreenUi->getToolWindow(); - offscreenUi->getRootContext()->engine()->setObjectOwnership(qmlWindow, QQmlEngine::JavaScriptOwnership); auto invokeResult = QMetaObject::invokeMethod(toolWindow, "removeTabForUrl", Qt::DirectConnection, Q_ARG(QVariant, _source)); Q_ASSERT(invokeResult); From a30df07d53443e89aaf37680eb5dc6535c3b69c0 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 22 Mar 2016 09:15:29 -0700 Subject: [PATCH 03/15] PR feeback, precendence --- interface/resources/qml/ToolWindow.qml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/resources/qml/ToolWindow.qml b/interface/resources/qml/ToolWindow.qml index 09fbf78077..75aa50aa34 100644 --- a/interface/resources/qml/ToolWindow.qml +++ b/interface/resources/qml/ToolWindow.qml @@ -97,7 +97,7 @@ Windows.Window { function findFreeTab() { for (var i = 0; i < tabView.count; ++i) { var tab = tabView.getTab(i); - if (tab && !tab.originalUrl || tab.originalUrl === "") { + if (tab && (!tab.originalUrl || tab.originalUrl === "")) { return i; } } From 47fb85eaeb4057edbe2f302a4957c080923e3910 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 22 Mar 2016 11:24:12 -0700 Subject: [PATCH 04/15] be more defensive in FBXGeometry::convexHullContains --- libraries/fbx/src/FBXReader.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/fbx/src/FBXReader.cpp b/libraries/fbx/src/FBXReader.cpp index b2ede33e01..de8c001566 100644 --- a/libraries/fbx/src/FBXReader.cpp +++ b/libraries/fbx/src/FBXReader.cpp @@ -78,10 +78,14 @@ bool FBXGeometry::convexHullContains(const glm::vec3& point) const { auto checkEachPrimitive = [=](FBXMesh& mesh, QVector indices, int primitiveSize) -> bool { // Check whether the point is "behind" all the primitives. + int verticesSize = mesh.vertices.size(); for (int j = 0; j < indices.size() - 2; // -2 in case the vertices aren't the right size -- we access j + 2 below j += primitiveSize) { - if (!isPointBehindTrianglesPlane(point, + if (indices[j] < verticesSize && + indices[j + 1] < verticesSize && + indices[j + 2] < verticesSize && + !isPointBehindTrianglesPlane(point, mesh.vertices[indices[j]], mesh.vertices[indices[j + 1]], mesh.vertices[indices[j + 2]])) { From 691ee87c737fbbd9a56fe7db32052157ea313edd Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 22 Mar 2016 11:32:38 -0700 Subject: [PATCH 05/15] fix to default avatar model --- libraries/avatars/src/AvatarData.cpp | 3 ++- libraries/avatars/src/AvatarData.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 93d2ebb5da..290aaff820 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -968,9 +968,10 @@ bool AvatarData::hasIdentityChangedAfterParsing(const QByteArray& data) { bool hasIdentityChanged = false; - if (skeletonModelURL != _skeletonModelURL) { + if (_firstSkeletonCheck || (skeletonModelURL != _skeletonModelURL)) { setSkeletonModelURL(skeletonModelURL); hasIdentityChanged = true; + _firstSkeletonCheck = false; } if (displayName != _displayName) { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 8a6b558383..900da38ffa 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -368,7 +368,8 @@ protected: HeadData* _headData; - QUrl _skeletonModelURL; // These need to be empty so that on first time setting them they will not short circuit + QUrl _skeletonModelURL; + bool _firstSkeletonCheck { true }; QUrl _skeletonFBXURL; QVector _attachmentData; QString _displayName; From 64ced6ee049c96e0c473e4bccd5419438878a49a Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 12:04:24 -0700 Subject: [PATCH 06/15] Add comment explaining bare glGetError --- libraries/gpu/src/gpu/GLBackend.cpp | 7 ++----- plugins/oculus/src/OculusBaseDisplayPlugin.cpp | 2 +- plugins/openvr/src/OpenVrDisplayPlugin.cpp | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/libraries/gpu/src/gpu/GLBackend.cpp b/libraries/gpu/src/gpu/GLBackend.cpp index 14e4397b83..2c25255a80 100644 --- a/libraries/gpu/src/gpu/GLBackend.cpp +++ b/libraries/gpu/src/gpu/GLBackend.cpp @@ -87,18 +87,15 @@ void GLBackend::init() { static std::once_flag once; std::call_once(once, [] { qCDebug(gpulogging) << "GL Version: " << QString((const char*) glGetString(GL_VERSION)); - qCDebug(gpulogging) << "GL Shader Language Version: " << QString((const char*) glGetString(GL_SHADING_LANGUAGE_VERSION)); - qCDebug(gpulogging) << "GL Vendor: " << QString((const char*) glGetString(GL_VENDOR)); - qCDebug(gpulogging) << "GL Renderer: " << QString((const char*) glGetString(GL_RENDERER)); glewExperimental = true; GLenum err = glewInit(); - glGetError(); + glGetError(); // clear the potential error from glewExperimental if (GLEW_OK != err) { - /* Problem: glewInit failed, something is seriously wrong. */ + // glewInit failed, something is seriously wrong. qCDebug(gpulogging, "Error: %s\n", glewGetErrorString(err)); } qCDebug(gpulogging, "Status: Using GLEW %s\n", glewGetString(GLEW_VERSION)); diff --git a/plugins/oculus/src/OculusBaseDisplayPlugin.cpp b/plugins/oculus/src/OculusBaseDisplayPlugin.cpp index 9d0838606e..29fc014a64 100644 --- a/plugins/oculus/src/OculusBaseDisplayPlugin.cpp +++ b/plugins/oculus/src/OculusBaseDisplayPlugin.cpp @@ -30,7 +30,7 @@ bool OculusBaseDisplayPlugin::isSupported() const { void OculusBaseDisplayPlugin::customizeContext() { glewExperimental = true; GLenum err = glewInit(); - glGetError(); + glGetError(); // clear the potential error from glewExperimental Parent::customizeContext(); } diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.cpp b/plugins/openvr/src/OpenVrDisplayPlugin.cpp index 843b70807d..0cd9bac15f 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.cpp +++ b/plugins/openvr/src/OpenVrDisplayPlugin.cpp @@ -101,7 +101,7 @@ void OpenVrDisplayPlugin::customizeContext() { std::call_once(once, []{ glewExperimental = true; GLenum err = glewInit(); - glGetError(); + glGetError(); // clear the potential error from glewExperimental }); Parent::customizeContext(); } From 23e3b7e4e3bf93dca28f06b2fb6ac248edc0460d Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 12:04:58 -0700 Subject: [PATCH 07/15] Rm extra GL logging --- libraries/gl/src/gl/GLWindow.cpp | 8 -------- libraries/gl/src/gl/OffscreenGLCanvas.cpp | 11 ----------- 2 files changed, 19 deletions(-) diff --git a/libraries/gl/src/gl/GLWindow.cpp b/libraries/gl/src/gl/GLWindow.cpp index 78c7666d25..bde33ed170 100644 --- a/libraries/gl/src/gl/GLWindow.cpp +++ b/libraries/gl/src/gl/GLWindow.cpp @@ -39,14 +39,6 @@ GLWindow::~GLWindow() { bool GLWindow::makeCurrent() { bool makeCurrentResult = _context->makeCurrent(this); Q_ASSERT(makeCurrentResult); - - std::call_once(_reportOnce, []{ - qDebug() << "GL Version: " << QString((const char*) glGetString(GL_VERSION)); - qDebug() << "GL Shader Language Version: " << QString((const char*) glGetString(GL_SHADING_LANGUAGE_VERSION)); - qDebug() << "GL Vendor: " << QString((const char*) glGetString(GL_VENDOR)); - qDebug() << "GL Renderer: " << QString((const char*) glGetString(GL_RENDERER)); - }); - Q_ASSERT(_context == QOpenGLContext::currentContext()); return makeCurrentResult; diff --git a/libraries/gl/src/gl/OffscreenGLCanvas.cpp b/libraries/gl/src/gl/OffscreenGLCanvas.cpp index 7406577814..ddca53d2f4 100644 --- a/libraries/gl/src/gl/OffscreenGLCanvas.cpp +++ b/libraries/gl/src/gl/OffscreenGLCanvas.cpp @@ -44,10 +44,6 @@ bool OffscreenGLCanvas::create(QOpenGLContext* sharedContext) { return true; } - qWarning() << "GL Version: " << QString((const char*) glGetString(GL_VERSION)); - qWarning() << "GL Shader Language Version: " << QString((const char*) glGetString(GL_SHADING_LANGUAGE_VERSION)); - qWarning() << "GL Vendor: " << QString((const char*) glGetString(GL_VENDOR)); - qWarning() << "GL Renderer: " << QString((const char*) glGetString(GL_RENDERER)); qWarning() << "Failed to create OffscreenGLCanvas"; return false; @@ -57,13 +53,6 @@ bool OffscreenGLCanvas::makeCurrent() { bool result = _context->makeCurrent(_offscreenSurface); Q_ASSERT(result); - std::call_once(_reportOnce, []{ - qDebug() << "GL Version: " << QString((const char*) glGetString(GL_VERSION)); - qDebug() << "GL Shader Language Version: " << QString((const char*) glGetString(GL_SHADING_LANGUAGE_VERSION)); - qDebug() << "GL Vendor: " << QString((const char*) glGetString(GL_VENDOR)); - qDebug() << "GL Renderer: " << QString((const char*) glGetString(GL_RENDERER)); - }); - #ifdef DEBUG if (result && !_logger) { _logger = new QOpenGLDebugLogger(this); From 362e288be8ef18d466e13db1dbea4bb6a00a7b8b Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 12:05:24 -0700 Subject: [PATCH 08/15] Rm mystery GL error before qml fbo --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index 0a3598e840..0f7a30a7d8 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -187,6 +187,15 @@ bool OffscreenQmlRenderThread::event(QEvent *e) { void OffscreenQmlRenderThread::setupFbo() { using namespace oglplus; _textures.setSize(_size); + + // Before making any ogl calls, clear any outstanding errors + // FIXME: Something upstream is polluting the context with a GL_INVALID_ENUM, + // likely from glewExperimental = true + GLenum err = glGetError(); + if (err != GLEW_OK) { + qDebug() << "Clearing outstanding GL error to set up QML FBO:" << glewGetErrorString(err); + } + _depthStencil.reset(new Renderbuffer()); Context::Bound(Renderbuffer::Target::Renderbuffer, *_depthStencil) .Storage( From 471ac80135cd6d46a33acfd9ee4bb73012c3ad1b Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 13:24:29 -0700 Subject: [PATCH 09/15] Revert most of "Rm extra GL logging" --- libraries/gl/src/gl/GLWindow.cpp | 8 ++++++++ libraries/gl/src/gl/OffscreenGLCanvas.cpp | 9 +++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/libraries/gl/src/gl/GLWindow.cpp b/libraries/gl/src/gl/GLWindow.cpp index bde33ed170..78c7666d25 100644 --- a/libraries/gl/src/gl/GLWindow.cpp +++ b/libraries/gl/src/gl/GLWindow.cpp @@ -39,6 +39,14 @@ GLWindow::~GLWindow() { bool GLWindow::makeCurrent() { bool makeCurrentResult = _context->makeCurrent(this); Q_ASSERT(makeCurrentResult); + + std::call_once(_reportOnce, []{ + qDebug() << "GL Version: " << QString((const char*) glGetString(GL_VERSION)); + qDebug() << "GL Shader Language Version: " << QString((const char*) glGetString(GL_SHADING_LANGUAGE_VERSION)); + qDebug() << "GL Vendor: " << QString((const char*) glGetString(GL_VENDOR)); + qDebug() << "GL Renderer: " << QString((const char*) glGetString(GL_RENDERER)); + }); + Q_ASSERT(_context == QOpenGLContext::currentContext()); return makeCurrentResult; diff --git a/libraries/gl/src/gl/OffscreenGLCanvas.cpp b/libraries/gl/src/gl/OffscreenGLCanvas.cpp index ddca53d2f4..31bbc84cb2 100644 --- a/libraries/gl/src/gl/OffscreenGLCanvas.cpp +++ b/libraries/gl/src/gl/OffscreenGLCanvas.cpp @@ -44,8 +44,6 @@ bool OffscreenGLCanvas::create(QOpenGLContext* sharedContext) { return true; } - qWarning() << "Failed to create OffscreenGLCanvas"; - return false; } @@ -53,6 +51,13 @@ bool OffscreenGLCanvas::makeCurrent() { bool result = _context->makeCurrent(_offscreenSurface); Q_ASSERT(result); + std::call_once(_reportOnce, []{ + qDebug() << "GL Version: " << QString((const char*) glGetString(GL_VERSION)); + qDebug() << "GL Shader Language Version: " << QString((const char*) glGetString(GL_SHADING_LANGUAGE_VERSION)); + qDebug() << "GL Vendor: " << QString((const char*) glGetString(GL_VENDOR)); + qDebug() << "GL Renderer: " << QString((const char*) glGetString(GL_RENDERER)); + }); + #ifdef DEBUG if (result && !_logger) { _logger = new QOpenGLDebugLogger(this); From e28a876629636c74dbfcd8d27444847146d3ef3a Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 13:25:43 -0700 Subject: [PATCH 10/15] Compare gl to GL_NO_ERROR, not GLEW_OK --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index 0f7a30a7d8..563c590874 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -192,7 +192,7 @@ void OffscreenQmlRenderThread::setupFbo() { // FIXME: Something upstream is polluting the context with a GL_INVALID_ENUM, // likely from glewExperimental = true GLenum err = glGetError(); - if (err != GLEW_OK) { + if (err != GL_NO_ERROR) { qDebug() << "Clearing outstanding GL error to set up QML FBO:" << glewGetErrorString(err); } From 9f0084dbb13343bebef975f3d00f4dc6431c2731 Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Tue, 22 Mar 2016 16:01:31 -0700 Subject: [PATCH 11/15] make Blender keep a shared pointer to the model it's blending --- libraries/render-utils/src/Model.cpp | 19 +++++++++---------- libraries/render-utils/src/Model.h | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/libraries/render-utils/src/Model.cpp b/libraries/render-utils/src/Model.cpp index df186802f6..0a483914df 100644 --- a/libraries/render-utils/src/Model.cpp +++ b/libraries/render-utils/src/Model.cpp @@ -29,7 +29,7 @@ using namespace std; -static int modelPointerTypeId = qRegisterMetaType >(); +static int nakedModelPointerTypeId = qRegisterMetaType(); static int weakNetworkGeometryPointerTypeId = qRegisterMetaType >(); static int vec3VectorTypeId = qRegisterMetaType >(); float Model::FAKE_DIMENSION_PLACEHOLDER = -1.0f; @@ -821,21 +821,21 @@ QStringList Model::getJointNames() const { class Blender : public QRunnable { public: - Blender(Model* model, int blendNumber, const QWeakPointer& geometry, + Blender(ModelPointer model, int blendNumber, const QWeakPointer& geometry, const QVector& meshes, const QVector& blendshapeCoefficients); virtual void run(); private: - QPointer _model; + ModelPointer _model; int _blendNumber; QWeakPointer _geometry; QVector _meshes; QVector _blendshapeCoefficients; }; -Blender::Blender(Model* model, int blendNumber, const QWeakPointer& geometry, +Blender::Blender(ModelPointer model, int blendNumber, const QWeakPointer& geometry, const QVector& meshes, const QVector& blendshapeCoefficients) : _model(model), _blendNumber(blendNumber), @@ -847,7 +847,7 @@ Blender::Blender(Model* model, int blendNumber, const QWeakPointer vertices, normals; - if (!_model.isNull()) { + if (_model) { int offset = 0; foreach (const FBXMesh& mesh, _meshes) { if (mesh.blendshapes.isEmpty()) { @@ -877,7 +877,7 @@ void Blender::run() { } // post the result to the geometry cache, which will dispatch to the model if still alive QMetaObject::invokeMethod(DependencyManager::get().data(), "setBlendedVertices", - Q_ARG(const QPointer&, _model), Q_ARG(int, _blendNumber), + Q_ARG(ModelPointer, _model), Q_ARG(int, _blendNumber), Q_ARG(const QWeakPointer&, _geometry), Q_ARG(const QVector&, vertices), Q_ARG(const QVector&, normals)); } @@ -1088,7 +1088,7 @@ float Model::getLimbLength(int jointIndex) const { bool Model::maybeStartBlender() { const FBXGeometry& fbxGeometry = _geometry->getFBXGeometry(); if (fbxGeometry.hasBlendedMeshes()) { - QThreadPool::globalInstance()->start(new Blender(this, ++_blendNumber, _geometry, + QThreadPool::globalInstance()->start(new Blender(getThisPointer(), ++_blendNumber, _geometry, fbxGeometry.meshes, _blendshapeCoefficients)); return true; } @@ -1284,10 +1284,9 @@ void ModelBlender::noteRequiresBlend(ModelPointer model) { } } -void ModelBlender::setBlendedVertices(const QPointer& model, int blendNumber, +void ModelBlender::setBlendedVertices(ModelPointer model, int blendNumber, const QWeakPointer& geometry, const QVector& vertices, const QVector& normals) { - - if (!model.isNull()) { + if (model) { model->setBlendedVertices(blendNumber, geometry, vertices, normals); } _pendingBlenders--; diff --git a/libraries/render-utils/src/Model.h b/libraries/render-utils/src/Model.h index 53ed82f418..4e51dc4f33 100644 --- a/libraries/render-utils/src/Model.h +++ b/libraries/render-utils/src/Model.h @@ -384,7 +384,7 @@ protected: RigPointer _rig; }; -Q_DECLARE_METATYPE(QPointer) +Q_DECLARE_METATYPE(ModelPointer) Q_DECLARE_METATYPE(QWeakPointer) /// Handle management of pending models that need blending @@ -398,7 +398,7 @@ public: void noteRequiresBlend(ModelPointer model); public slots: - void setBlendedVertices(const QPointer& model, int blendNumber, const QWeakPointer& geometry, + void setBlendedVertices(ModelPointer model, int blendNumber, const QWeakPointer& geometry, const QVector& vertices, const QVector& normals); private: From 9c11474dd78785eb8811b1084847534c480acbcf Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 15:39:03 -0700 Subject: [PATCH 12/15] Expose qApp->updateHeartbeat --- interface/src/Application.cpp | 24 ++++++++++++------------ interface/src/Application.h | 4 ++++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 7e88ea28dc..65a8f83871 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -239,11 +239,7 @@ class DeadlockWatchdogThread : public QThread { public: static const unsigned long HEARTBEAT_CHECK_INTERVAL_SECS = 1; static const unsigned long HEARTBEAT_UPDATE_INTERVAL_SECS = 1; -#ifdef DEBUG - static const unsigned long MAX_HEARTBEAT_AGE_USECS = 600 * USECS_PER_SECOND; -#else static const unsigned long MAX_HEARTBEAT_AGE_USECS = 10 * USECS_PER_SECOND; -#endif // Set the heartbeat on launch DeadlockWatchdogThread() { @@ -511,8 +507,8 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : auto nodeList = DependencyManager::get(); // Set up a watchdog thread to intentionally crash the application on deadlocks - auto deadlockWatchdog = new DeadlockWatchdogThread(); - deadlockWatchdog->start(); + _deadlockWatchdogThread = new DeadlockWatchdogThread(); + _deadlockWatchdogThread->start(); qCDebug(interfaceapp) << "[VERSION] Build sequence:" << qPrintable(applicationVersion()); @@ -586,7 +582,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : ResourceManager::init(); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); // Setup MessagesClient auto messagesClient = DependencyManager::get(); @@ -734,7 +730,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : initializeGL(); _offscreenContext->makeCurrent(); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); // Tell our entity edit sender about our known jurisdictions _entityEditSender.setServerJurisdictions(&_entityServerJurisdictions); @@ -746,7 +742,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : _overlays.init(); // do this before scripts load // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); connect(this, SIGNAL(aboutToQuit()), this, SLOT(aboutToQuit())); @@ -900,11 +896,11 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : // do this as late as possible so that all required subsystems are initialized scriptEngines->loadScripts(); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); loadSettings(); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); int SAVE_SETTINGS_INTERVAL = 10 * MSECS_PER_SECOND; // Let's save every seconds for now connect(&_settingsTimer, &QTimer::timeout, this, &Application::saveSettings); @@ -1019,7 +1015,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : }); // Make sure we don't time out during slow operations at startup - deadlockWatchdog->updateHeartbeat(); + updateHeartbeat(); connect(this, &Application::applicationStateChanged, this, &Application::activeChanged); qCDebug(interfaceapp, "Startup time: %4.2f seconds.", (double)startupTimer.elapsed() / 1000.0); @@ -1060,6 +1056,10 @@ void Application::showCursor(const QCursor& cursor) { _cursorNeedsChanging = true; } +void Application::updateHeartbeat() { + static_cast(_deadlockWatchdogThread)->updateHeartbeat(); +} + void Application::aboutToQuit() { emit beforeAboutToQuit(); diff --git a/interface/src/Application.h b/interface/src/Application.h index 4c12164a5f..3b906a7d17 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -271,6 +271,8 @@ public slots: void reloadResourceCaches(); + void updateHeartbeat(); + void crashApplication(); void deadlockApplication(); @@ -505,6 +507,8 @@ private: mutable QMutex _changeCursorLock { QMutex::Recursive }; QCursor _desiredCursor{ Qt::BlankCursor }; bool _cursorNeedsChanging { false }; + + QThread* _deadlockWatchdogThread; }; #endif // hifi_Application_h From 745438148a760b10260d832dcfbeb99f5851c112 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Tue, 22 Mar 2016 17:17:36 -0700 Subject: [PATCH 13/15] Reset deadlock watch before qml load --- libraries/gl/src/gl/OffscreenQmlSurface.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index 563c590874..4cfd2527ea 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/libraries/gl/src/gl/OffscreenQmlSurface.cpp @@ -426,7 +426,11 @@ void OffscreenQmlSurface::setBaseUrl(const QUrl& baseUrl) { } QObject* OffscreenQmlSurface::load(const QUrl& qmlSource, std::function f) { - _qmlComponent->loadUrl(qmlSource); + // Synchronous loading may take a while; restart the deadlock timer + QMetaObject::invokeMethod(qApp, "updateHeartbeat", Qt::DirectConnection); + + _qmlComponent->loadUrl(qmlSource, QQmlComponent::PreferSynchronous); + if (_qmlComponent->isLoading()) { connect(_qmlComponent, &QQmlComponent::statusChanged, this, [this, f](QQmlComponent::Status){ From 2fdb92e8cded5675ad64c64db4f218b9728f03ed Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 22 Mar 2016 22:19:10 -0700 Subject: [PATCH 14/15] Move oglplus dependency from sourceforge to s3 --- cmake/externals/oglplus/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/externals/oglplus/CMakeLists.txt b/cmake/externals/oglplus/CMakeLists.txt index 43730129a0..089ee5bb95 100644 --- a/cmake/externals/oglplus/CMakeLists.txt +++ b/cmake/externals/oglplus/CMakeLists.txt @@ -4,7 +4,7 @@ string(TOUPPER ${EXTERNAL_NAME} EXTERNAL_NAME_UPPER) include(ExternalProject) ExternalProject_Add( ${EXTERNAL_NAME} - URL http://iweb.dl.sourceforge.net/project/oglplus/oglplus-0.63.x/oglplus-0.63.0.zip + URL http://hifi-public.s3.amazonaws.com/dependencies/oglplus-0.63.0.zip URL_MD5 de984ab245b185b45c87415c0e052135 CONFIGURE_COMMAND "" BUILD_COMMAND "" From 252a49eea4ea92c88d1baf4d021a840bd246e0e7 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 22 Mar 2016 20:21:36 -0700 Subject: [PATCH 15/15] Add a tracker and logging of memory allocated by the GPU library --- libraries/gpu/src/gpu/Resource.cpp | 64 ++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/libraries/gpu/src/gpu/Resource.cpp b/libraries/gpu/src/gpu/Resource.cpp index 1788f6ece5..197263f392 100644 --- a/libraries/gpu/src/gpu/Resource.cpp +++ b/libraries/gpu/src/gpu/Resource.cpp @@ -10,11 +10,74 @@ // #include "Resource.h" +#include + +#include +#include #include using namespace gpu; +class AllocationDebugger { +public: + void operator+=(size_t size) { + _allocatedMemory += size; + maybeReport(); + } + + void operator-=(size_t size) { + _allocatedMemory -= size; + maybeReport(); + } + +private: + QString formatSize(size_t size) { + float num = size; + QStringList list; + list << "KB" << "MB" << "GB" << "TB"; + + QStringListIterator i(list); + QString unit("bytes"); + + while (num >= K && i.hasNext()) { + unit = i.next(); + num /= K; + } + return QString().setNum(num, 'f', 2) + " " + unit; + } + + void maybeReport() { + auto now = usecTimestampNow(); + if (now - _lastReportTime < MAX_REPORT_FREQUENCY) { + return; + } + size_t current = _allocatedMemory; + size_t last = _lastReportedMemory; + size_t delta = (current > last) ? (current - last) : (last - current); + if (delta > MIN_REPORT_DELTA) { + _lastReportTime = now; + _lastReportedMemory = current; + qDebug() << "Total allocation " << formatSize(current); + } + } + + std::atomic _allocatedMemory; + std::atomic _lastReportedMemory; + std::atomic _lastReportTime; + + static const float K; + // Report changes of 5 megabytes + static const size_t MIN_REPORT_DELTA = 1024 * 1024 * 5; + // Report changes no more frequently than every 15 seconds + static const uint64_t MAX_REPORT_FREQUENCY = USECS_PER_SECOND * 15; +}; + +const float AllocationDebugger::K = 1024.0f; + +static AllocationDebugger allocationDebugger; + Resource::Size Resource::Sysmem::allocateMemory(Byte** dataAllocated, Size size) { + allocationDebugger += size; if ( !dataAllocated ) { qWarning() << "Buffer::Sysmem::allocateMemory() : Must have a valid dataAllocated pointer."; return NOT_ALLOCATED; @@ -38,6 +101,7 @@ Resource::Size Resource::Sysmem::allocateMemory(Byte** dataAllocated, Size size) } void Resource::Sysmem::deallocateMemory(Byte* dataAllocated, Size size) { + allocationDebugger -= size; if (dataAllocated) { delete[] dataAllocated; }