Merge pull request #7722 from zzmp/fix/load-lock

Fix deadlock on shutdown from blocking texture/model loads

libraries/animation/src/AnimationCache.cpp

@@ -64,9 +64,9 @@ void AnimationReader::run() {
 
         if (urlValid) {
             // Parse the FBX directly from the QNetworkReply
-            FBXGeometry* fbxgeo = nullptr;
+            FBXGeometry::Pointer fbxgeo;
             if (_url.path().toLower().endsWith(".fbx")) {
-                fbxgeo = readFBX(_data, QVariantHash(), _url.path());
+                fbxgeo.reset(readFBX(_data, QVariantHash(), _url.path()));
             } else {
                 QString errorStr("usupported format");
                 emit onError(299, errorStr);
@@ -117,16 +117,16 @@ const QVector<FBXAnimationFrame>& Animation::getFramesReference() const {
 void Animation::downloadFinished(const QByteArray& data) {
     // parse the animation/fbx file on a background thread.
     AnimationReader* animationReader = new AnimationReader(_url, data);
-    connect(animationReader, SIGNAL(onSuccess(FBXGeometry*)), SLOT(animationParseSuccess(FBXGeometry*)));
+    connect(animationReader, SIGNAL(onSuccess(FBXGeometry::Pointer)), SLOT(animationParseSuccess(FBXGeometry::Pointer)));
     connect(animationReader, SIGNAL(onError(int, QString)), SLOT(animationParseError(int, QString)));
     QThreadPool::globalInstance()->start(animationReader);
 }
 
-void Animation::animationParseSuccess(FBXGeometry* geometry) {
+void Animation::animationParseSuccess(FBXGeometry::Pointer geometry) {
 
     qCDebug(animation) << "Animation parse success" << _url.toDisplayString();
 
-    _geometry.reset(geometry);
+    _geometry = geometry;
     finishedLoading(true);
 }
 

libraries/animation/src/AnimationCache.h

@@ -68,12 +68,12 @@ protected:
     virtual void downloadFinished(const QByteArray& data) override;
 
 protected slots:
-    void animationParseSuccess(FBXGeometry* geometry);
+    void animationParseSuccess(FBXGeometry::Pointer geometry);
     void animationParseError(int error, QString str);
 
 private:
     
-    std::unique_ptr<FBXGeometry> _geometry;
+    FBXGeometry::Pointer _geometry;
 };
 
 /// Reads geometry in a worker thread.
@@ -85,7 +85,7 @@ public:
     virtual void run();
 
 signals:
-    void onSuccess(FBXGeometry* geometry);
+    void onSuccess(FBXGeometry::Pointer geometry);
     void onError(int error, QString str);
 
 private:

libraries/fbx/src/FBXReader.cpp

@@ -39,6 +39,8 @@
 
 using namespace std;
 
+static int FBXGeometryPointerMetaTypeId = qRegisterMetaType<FBXGeometry::Pointer>();
+
 QStringList FBXGeometry::getJointNames() const {
     QStringList names;
     foreach (const FBXJoint& joint, joints) {

libraries/fbx/src/FBXReader.h

@@ -270,6 +270,7 @@ inline bool operator!=(const SittingPoint& lhs, const SittingPoint& rhs)
 /// A set of meshes extracted from an FBX document.
 class FBXGeometry {
 public:
+    using Pointer = std::shared_ptr<FBXGeometry>;
 
     QString author;
     QString applicationName; ///< the name of the application that generated the model
@@ -330,6 +331,7 @@ public:
 };
 
 Q_DECLARE_METATYPE(FBXGeometry)
+Q_DECLARE_METATYPE(FBXGeometry::Pointer)
 
 /// Reads FBX geometry from the supplied model and mapping data.
 /// \exception QString if an error occurs in parsing

libraries/gpu/src/gpu/Texture.cpp

@@ -17,6 +17,7 @@
 
 using namespace gpu;
 
+static int TexturePointerMetaTypeId = qRegisterMetaType<TexturePointer>();
 
 std::atomic<uint32_t> Texture::_textureCPUCount{ 0 };
 std::atomic<Texture::Size> Texture::_textureCPUMemoryUsage{ 0 };
@@ -857,8 +858,8 @@ void TextureSource::reset(const QUrl& url) {
     _imageUrl = url;
 }
 
-void TextureSource::resetTexture(gpu::Texture* texture) {
+void TextureSource::resetTexture(gpu::TexturePointer texture) {
-    _gpuTexture.reset(texture);
+    _gpuTexture = texture;
 }
 
 bool TextureSource::isDefined() const {

libraries/gpu/src/gpu/Texture.h

@@ -16,6 +16,7 @@
 #include <algorithm> //min max and more
 #include <bitset>
 
+#include <QMetaType>
 #include <QUrl>
 
 namespace gpu {
@@ -469,7 +470,6 @@ protected:
 typedef std::shared_ptr<Texture> TexturePointer;
 typedef std::vector< TexturePointer > Textures;
 
-
  // TODO: For now TextureView works with Texture as a place holder for the Texture.
  // The overall logic should be about the same except that the Texture will be a real GL Texture under the hood
 class TextureView {
@@ -526,7 +526,7 @@ public:
 
     void reset(const QUrl& url);
 
-    void resetTexture(gpu::Texture* texture);
+    void resetTexture(gpu::TexturePointer texture);
 
     bool isDefined() const;
 
@@ -538,5 +538,6 @@ typedef std::shared_ptr< TextureSource > TextureSourcePointer;
 
 };
 
+Q_DECLARE_METATYPE(gpu::TexturePointer)
 
 #endif

libraries/model-networking/src/model-networking/ModelCache.cpp

@@ -143,40 +143,38 @@ void GeometryReader::run() {
         QString urlname = _url.path().toLower();
         if (!urlname.isEmpty() && !_url.path().isEmpty() &&
             (_url.path().toLower().endsWith(".fbx") || _url.path().toLower().endsWith(".obj"))) {
-            FBXGeometry* fbxGeometry = nullptr;
+            FBXGeometry::Pointer fbxGeometry;
 
             if (_url.path().toLower().endsWith(".fbx")) {
-                fbxGeometry = readFBX(_data, _mapping, _url.path());
+                fbxGeometry.reset(readFBX(_data, _mapping, _url.path()));
                 if (fbxGeometry->meshes.size() == 0 && fbxGeometry->joints.size() == 0) {
                     throw QString("empty geometry, possibly due to an unsupported FBX version");
                 }
             } else if (_url.path().toLower().endsWith(".obj")) {
-                fbxGeometry = OBJReader().readOBJ(_data, _mapping, _url);
+                fbxGeometry.reset(OBJReader().readOBJ(_data, _mapping, _url));
             } else {
                 throw QString("unsupported format");
             }
 
-            // Ensure the resource has not been deleted, and won't be while invokeMethod is in flight.
+            // Ensure the resource has not been deleted
             auto resource = _resource.toStrongRef();
             if (!resource) {
                 qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
-                delete fbxGeometry;
             } else {
-                QMetaObject::invokeMethod(resource.data(), "setGeometryDefinition", Qt::BlockingQueuedConnection, Q_ARG(void*, fbxGeometry));
+                QMetaObject::invokeMethod(resource.data(), "setGeometryDefinition",
+                    Q_ARG(FBXGeometry::Pointer, fbxGeometry));
             }
         } else {
             throw QString("url is invalid");
         }
     } catch (const QString& error) {
 
-        qCDebug(modelnetworking) << "Error reading " << _url << ": " << error;
+        qCDebug(modelnetworking) << "Error parsing model for" << _url << ":" << error;
 
         auto resource = _resource.toStrongRef();
-        // Ensure the resoruce has not been deleted, and won't be while invokeMethod is in flight.
+        if (resource) {
-        if (!resource) {
+            QMetaObject::invokeMethod(resource.data(), "finishedLoading",
-            qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
+                Q_ARG(bool, false));
-        } else {
-            QMetaObject::invokeMethod(resource.data(), "finishedLoading", Qt::BlockingQueuedConnection, Q_ARG(bool, false));
         }
     }
 }
@@ -190,7 +188,7 @@ public:
     virtual void downloadFinished(const QByteArray& data) override;
 
 protected:
-    Q_INVOKABLE void setGeometryDefinition(void* fbxGeometry);
+    Q_INVOKABLE void setGeometryDefinition(FBXGeometry::Pointer fbxGeometry);
 
 private:
     QVariantHash _mapping;
@@ -200,9 +198,9 @@ void GeometryDefinitionResource::downloadFinished(const QByteArray& data) {
     QThreadPool::globalInstance()->start(new GeometryReader(_self, _url, _mapping, data));
 }
 
-void GeometryDefinitionResource::setGeometryDefinition(void* fbxGeometry) {
+void GeometryDefinitionResource::setGeometryDefinition(FBXGeometry::Pointer fbxGeometry) {
     // Assume ownership of the geometry pointer
-    _geometry.reset(static_cast<FBXGeometry*>(fbxGeometry));
+    _geometry = fbxGeometry;
 
     // Copy materials
     QHash<QString, size_t> materialIDAtlas;

libraries/model-networking/src/model-networking/TextureCache.cpp

@@ -330,7 +330,7 @@ void ImageReader::run() {
         return;
     }
 
-    gpu::Texture* texture = nullptr;
+    gpu::TexturePointer texture = nullptr;
     {
         // Double-check the resource still exists between long operations.
         auto resource = _resource.toStrongRef();
@@ -342,36 +342,32 @@ void ImageReader::run() {
         auto url = _url.toString().toStdString();
 
         PROFILE_RANGE_EX(__FUNCTION__"::textureLoader", 0xffffff00, nullptr);
-        texture = resource.dynamicCast<NetworkTexture>()->getTextureLoader()(image, url);
+        texture.reset(resource.dynamicCast<NetworkTexture>()->getTextureLoader()(image, url));
     }
 
-    // Ensure the resource has not been deleted, and won't be while invokeMethod is in flight.
+    // Ensure the resource has not been deleted
     auto resource = _resource.toStrongRef();
     if (!resource) {
         qCWarning(modelnetworking) << "Abandoning load of" << _url << "; could not get strong ref";
-        delete texture;
     } else {
-        QMetaObject::invokeMethod(resource.data(), "setImage", Qt::BlockingQueuedConnection,
+        QMetaObject::invokeMethod(resource.data(), "setImage",
-            Q_ARG(void*, texture),
+            Q_ARG(gpu::TexturePointer, texture),
             Q_ARG(int, originalWidth), Q_ARG(int, originalHeight));
     }
 }
 
-void NetworkTexture::setImage(void* voidTexture, int originalWidth,
+void NetworkTexture::setImage(gpu::TexturePointer texture, int originalWidth,
                               int originalHeight) {
     _originalWidth = originalWidth;
     _originalHeight = originalHeight;
 
-    gpu::Texture* texture = static_cast<gpu::Texture*>(voidTexture);
-
     // Passing ownership
     _textureSource->resetTexture(texture);
-    auto gpuTexture = _textureSource->getGPUTexture();
 
-    if (gpuTexture) {
+    if (texture) {
-        _width = gpuTexture->getWidth();
+        _width = texture->getWidth();
-        _height = gpuTexture->getHeight();
+        _height = texture->getHeight();
-        setSize(gpuTexture->getStoredSize());
+        setSize(texture->getStoredSize());
     } else {
         // FIXME: If !gpuTexture, we failed to load!
         _width = _height = 0;

libraries/model-networking/src/model-networking/TextureCache.h

@@ -128,7 +128,6 @@ public:
 signals:
     void networkTextureCreated(const QWeakPointer<NetworkTexture>& self);
 
-
 protected:
 
     virtual bool isCacheable() const override { return _loaded; }
@@ -136,9 +135,7 @@ protected:
     virtual void downloadFinished(const QByteArray& data) override;
           
     Q_INVOKABLE void loadContent(const QByteArray& content);
-    // FIXME: This void* should be a gpu::Texture* but i cannot get it to work for now, moving on...
+    Q_INVOKABLE void setImage(gpu::TexturePointer texture, int originalWidth, int originalHeight);
-    Q_INVOKABLE void setImage(void* texture, int originalWidth, int originalHeight);
-
 
 private:
     TextureType _type;