Simplify serializer registration process

interface/src/Application.cpp

@@ -101,7 +101,7 @@
 #include <MainWindow.h>
 #include <MappingRequest.h>
 #include <MessagesClient.h>
-#include <model-networking/ModelFormatRegistry.h>
+#include <hfm/ModelFormatRegistry.h>
 #include <model-networking/ModelCacheScriptingInterface.h>
 #include <model-networking/TextureCacheScriptingInterface.h>
 #include <ModelEntityItem.h>
@@ -884,7 +884,7 @@ bool setupEssentials(int& argc, char** argv, bool runningMarkerExisted) {
     DependencyManager::set<NodeList>(NodeType::Agent, listenPort);
     DependencyManager::set<recording::ClipCache>();
     DependencyManager::set<GeometryCache>();
-    DependencyManager::set<ModelFormatRegistry>();
+    DependencyManager::set<ModelFormatRegistry>(); // ModelFormatRegistry must be defined before ModelCache. See the ModelCache constructor.
     DependencyManager::set<ModelCache>();
     DependencyManager::set<ModelCacheScriptingInterface>();
     DependencyManager::set<ScriptCache>();
@@ -2747,9 +2747,9 @@ Application::~Application() {
     DependencyManager::destroy<FramebufferCache>();
     DependencyManager::destroy<TextureCacheScriptingInterface>();
     DependencyManager::destroy<TextureCache>();
-    DependencyManager::destroy<ModelFormatRegistry>();
     DependencyManager::destroy<ModelCacheScriptingInterface>();
     DependencyManager::destroy<ModelCache>();
+    DependencyManager::destroy<ModelFormatRegistry>();
     DependencyManager::destroy<ScriptCache>();
     DependencyManager::destroy<SoundCacheScriptingInterface>();
     DependencyManager::destroy<SoundCache>();

libraries/fbx/src/FBXSerializer.cpp

@@ -33,7 +33,6 @@
 #include <gpu/Format.h>
 #include <LogHandler.h>
 
-#include <hfm/HFMSimpleFormat.h>
 #include <hfm/ModelFormatLogging.h>
 
 // TOOL: Uncomment the following line to enable the filtering of all the unkwnon fields of a node so we can break point easily while loading a model with problems...
@@ -1834,14 +1833,16 @@ HFMModel* FBXSerializer::extractHFMModel(const QVariantHash& mapping, const QStr
     return hfmModelPtr;
 }
 
-MediaType getFBXMediaType() {
+MediaType FBXSerializer::getMediaType() const {
     MediaType mediaType("fbx");
     mediaType.extensions.push_back("fbx");
     mediaType.fileSignatures.emplace_back("Kaydara FBX Binary  \x00", 0);
     return mediaType;
 }
 
-std::shared_ptr<hfm::Format> FBXSerializer::FORMAT = std::make_shared<hfm::SimpleFormat<FBXSerializer>>(getFBXMediaType());
+std::unique_ptr<hfm::Serializer::Factory> FBXSerializer::getFactory() const {
+    return std::make_unique<hfm::Serializer::SimpleFactory<FBXSerializer>>();
+}
 
 HFMModel::Pointer FBXSerializer::read(const QByteArray& data, const QVariantHash& mapping, const QUrl& url) {
     QBuffer buffer(const_cast<QByteArray*>(&data));

libraries/fbx/src/FBXSerializer.h

@@ -28,7 +28,6 @@
 
 #include "FBX.h"
 #include <hfm/HFMSerializer.h>
-#include <hfm/HFMFormat.h>
 
 #include <graphics/Geometry.h>
 #include <graphics/Material.h>
@@ -97,7 +96,8 @@ class ExtractedMesh;
 
 class FBXSerializer : public HFMSerializer {
 public:
-    static std::shared_ptr<hfm::Format> FORMAT;
+    MediaType getMediaType() const override;
+    std::unique_ptr<hfm::Serializer::Factory> getFactory() const override;
 
     HFMModel* _hfmModel;
     /// Reads HFMModel from the supplied model and mapping data.

libraries/fbx/src/GLTFSerializer.cpp

@@ -34,7 +34,6 @@
 #include <PathUtils.h>
 
 #include "FBXSerializer.h"
-#include <hfm/HFMSimpleFormat.h>
 
 bool GLTFSerializer::getStringVal(const QJsonObject& object, const QString& fieldname,
                               QString& value, QMap<QString, bool>&  defined) {
@@ -906,14 +905,16 @@ bool GLTFSerializer::buildGeometry(HFMModel& hfmModel, const QUrl& url) {
     return true;
 }
 
-MediaType getGLTFMediaType() {
+MediaType GLTFSerializer::getMediaType() const {
     MediaType mediaType("gltf");
     mediaType.extensions.push_back("gltf");
     mediaType.webMediaTypes.push_back("model/gltf+json");
     return mediaType;
 }
 
-std::shared_ptr<hfm::Format> GLTFSerializer::FORMAT = std::make_shared<hfm::SimpleFormat<GLTFSerializer>>(getGLTFMediaType());
+std::unique_ptr<hfm::Serializer::Factory> GLTFSerializer::getFactory() const {
+    return std::make_unique<hfm::Serializer::SimpleFactory<GLTFSerializer>>();
+}
 
 HFMModel::Pointer GLTFSerializer::read(const QByteArray& data, const QVariantHash& mapping, const QUrl& url) {
     

libraries/fbx/src/GLTFSerializer.h

@@ -16,8 +16,6 @@
 #include <QtNetwork/QNetworkReply>
 #include <hfm/ModelFormatLogging.h>
 #include <hfm/HFMSerializer.h>
-#include <hfm/HFMFormat.h>
-#include "FBXSerializer.h"
 
 
 struct GLTFAsset {
@@ -704,7 +702,8 @@ struct GLTFFile {
 class GLTFSerializer : public QObject, public HFMSerializer {
     Q_OBJECT
 public:
-    static std::shared_ptr<hfm::Format> FORMAT;
+    MediaType getMediaType() const override;
+    std::unique_ptr<hfm::Serializer::Factory> getFactory() const override;
 
     HFMModel::Pointer read(const QByteArray& data, const QVariantHash& mapping, const QUrl& url = QUrl()) override;
 private:

libraries/fbx/src/OBJSerializer.cpp

@@ -28,7 +28,6 @@
 #include <ResourceManager.h>
 
 #include "FBXSerializer.h"
-#include <hfm/HFMSimpleFormat.h>
 #include <hfm/ModelFormatLogging.h>
 #include <shared/PlatformHacks.h>
 
@@ -652,13 +651,15 @@ done:
     return result;
 }
 
-MediaType getOBJMediaType() {
+MediaType OBJSerializer::getMediaType() const {
     MediaType mediaType("obj");
     mediaType.extensions.push_back("obj");
     return mediaType;
 }
 
-std::shared_ptr<hfm::Format> OBJSerializer::FORMAT = std::make_shared<hfm::SimpleFormat<OBJSerializer>>(getOBJMediaType());
+std::unique_ptr<hfm::Serializer::Factory> OBJSerializer::getFactory() const {
+    return std::make_unique<hfm::Serializer::SimpleFactory<OBJSerializer>>();
+}
 
 HFMModel::Pointer OBJSerializer::read(const QByteArray& data, const QVariantHash& mapping, const QUrl& url) {
     PROFILE_RANGE_EX(resource_parse, __FUNCTION__, 0xffff0000, nullptr);

libraries/fbx/src/OBJSerializer.h

@@ -14,8 +14,6 @@
 
 #include <QtNetwork/QNetworkReply>
 #include <hfm/HFMSerializer.h>
-#include <hfm/HFMFormat.h>
-#include "FBXSerializer.h"
 
 class OBJTokenizer {
 public:
@@ -93,7 +91,8 @@ public:
 class OBJSerializer: public QObject, public HFMSerializer { // QObject so we can make network requests.
     Q_OBJECT
 public:
-    static std::shared_ptr<hfm::Format> FORMAT;
+    MediaType getMediaType() const override;
+    std::unique_ptr<hfm::Serializer::Factory> getFactory() const;
     
     typedef QVector<OBJFace> FaceGroup;
     QVector<glm::vec3> vertices;

libraries/hfm/src/hfm/FormatSerializerRegister.cpp

@@ -0,0 +1,29 @@
+//
+//  FormatSerializerRegister.cpp
+//  libraries/hfm/src/hfm
+//
+//  Created by Sabrina Shanman on 2018/12/07.
+//  Copyright 2018 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#include "FormatSerializerRegister.h"
+
+#include "ModelFormatRegistry.h"
+
+namespace hfm {
+
+DoFormatSerializerRegister::DoFormatSerializerRegister(const MediaType& mediaType, std::unique_ptr<Serializer::Factory> factory) {
+    auto registry = DependencyManager::get<ModelFormatRegistry>();
+    _mediaTypeID = registry->_hfmFormatRegistry.registerMediaType(mediaType, std::move(factory));
+}
+
+void DoFormatSerializerRegister::unregisterFormat() {
+    auto registry = DependencyManager::get<ModelFormatRegistry>();
+    registry->_hfmFormatRegistry.unregisterMediaType(_mediaTypeID);
+    _mediaTypeID = hfm::FormatRegistry::INVALID_MEDIA_TYPE_ID;
+}
+
+};

libraries/hfm/src/hfm/FormatSerializerRegister.h

@@ -0,0 +1,35 @@
+//
+//  FormatSerializerRegister.h
+//  libraries/hfm/src/hfm
+//
+//  Created by Sabrina Shanman on 2018/11/30.
+//  Copyright 2018 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#ifndef hifi_HFMFormat_h
+#define hifi_HFMFormat_h
+
+#include "HFMFormatRegistry.h"
+#include "HFMSerializer.h"
+
+namespace hfm {
+    // A helper class which allows early de-registration of a Serializer from ModelFormatRegistry
+    class FormatSerializerRegister {
+    public:
+        virtual void unregisterFormat() = 0;
+    };
+
+    class DoFormatSerializerRegister : public FormatSerializerRegister {
+    public:
+        DoFormatSerializerRegister(const MediaType& mediaType, std::unique_ptr<Serializer::Factory> factory);
+
+        void unregisterFormat() override;
+    protected:
+        FormatRegistry::MediaTypeID _mediaTypeID { FormatRegistry::INVALID_MEDIA_TYPE_ID };
+    };
+};
+
+#endif // hifi_HFMFormat_h

libraries/hfm/src/hfm/HFMFormat.h

@@ -1,25 +0,0 @@
-//
-//  HFMFormat.h
-//  libraries/hfm/src/hfm
-//
-//  Created by Sabrina Shanman on 2018/11/30.
-//  Copyright 2018 High Fidelity, Inc.
-//
-//  Distributed under the Apache License, Version 2.0.
-//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
-//
-
-#ifndef hifi_HFMFormat_h
-#define hifi_HFMFormat_h
-
-#include "HFMFormatRegistry.h"
-
-namespace hfm {
-    class Format {
-    public:
-        virtual void registerFormat(FormatRegistry& registry) = 0;
-        virtual void unregisterFormat(FormatRegistry& registry) = 0;
-    };
-};
-
-#endif // hifi_HFMFormat_h

libraries/hfm/src/hfm/HFMFormatRegistry.h

@@ -27,9 +27,10 @@ public:
     void unregisterMediaType(const MediaTypeID& id);
 
     std::shared_ptr<Serializer> getSerializerForMediaType(const hifi::ByteArray& data, const hifi::URL& url, const std::string& webMediaType) const;
-    std::shared_ptr<Serializer> getSerializerForMediaTypeID(MediaTypeID id) const;
 
 protected:
+    std::shared_ptr<Serializer> getSerializerForMediaTypeID(MediaTypeID id) const;
+
     MediaTypeLibrary _mediaTypeLibrary;
     std::mutex _libraryLock;
     class SupportedFormat {

libraries/hfm/src/hfm/HFMSerializer.h

@@ -15,6 +15,7 @@
 #include <shared/HifiTypes.h>
 
 #include "HFM.h"
+#include <shared/MediaTypeLibrary.h>
 
 namespace hfm {
 
@@ -25,6 +26,16 @@ public:
         virtual std::shared_ptr<Serializer> get() = 0;
     };
 
+    template<typename T>
+    class SimpleFactory : public Factory {
+        std::shared_ptr<Serializer> get() override {
+            return std::make_shared<T>();
+        }
+    };
+
+    virtual MediaType getMediaType() const = 0;
+    virtual std::unique_ptr<Factory> getFactory() const = 0;
+
     virtual Model::Pointer read(const hifi::ByteArray& data, const hifi::VariantHash& mapping, const hifi::URL& url = hifi::URL()) = 0;
 };
 

libraries/hfm/src/hfm/ModelFormatRegistry.cpp

@@ -0,0 +1,20 @@
+//
+//  ModelFormatRegistry.cpp
+//  libraries/model-networking/src/model-networking
+//
+//  Created by Sabrina Shanman on 2018/11/30.
+//  Copyright 2018 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#include "ModelFormatRegistry.h"
+
+std::unique_ptr<hfm::FormatSerializerRegister> ModelFormatRegistry::addFormat(const hfm::Serializer& serializer) {
+    return std::make_unique<hfm::DoFormatSerializerRegister>(serializer.getMediaType(), serializer.getFactory());
+}
+
+std::shared_ptr<hfm::Serializer> ModelFormatRegistry::getSerializerForMediaType(const hifi::ByteArray& data, const hifi::URL& url, const std::string& webMediaType) const {
+    return _hfmFormatRegistry.getSerializerForMediaType(data, url, webMediaType);
+}

libraries/hfm/src/hfm/ModelFormatRegistry.h

@@ -0,0 +1,30 @@
+//
+//  ModelFormatRegistry.h
+//  libraries/hfm/src/hfm
+//
+//  Created by Sabrina Shanman on 2018/11/30.
+//  Copyright 2018 High Fidelity, Inc.
+//
+//  Distributed under the Apache License, Version 2.0.
+//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
+//
+
+#ifndef hifi_ModelFormatRegistry_h
+#define hifi_ModelFormatRegistry_h
+
+#include "FormatSerializerRegister.h"
+#include "HFMFormatRegistry.h"
+
+#include <DependencyManager.h>
+
+class ModelFormatRegistry : public Dependency {
+public:
+    std::unique_ptr<hfm::FormatSerializerRegister> addFormat(const hfm::Serializer& serializer);
+    std::shared_ptr<hfm::Serializer> getSerializerForMediaType(const hifi::ByteArray& data, const hifi::URL& url, const std::string& webMediaType) const;
+
+protected:
+    friend class hfm::DoFormatSerializerRegister;
+    hfm::FormatRegistry _hfmFormatRegistry;
+};
+
+#endif // hifi_ModelFormatRegistry_h

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

@@ -23,6 +23,10 @@
 #include "ModelNetworkingLogging.h"
 #include <Trace.h>
 #include <StatTracker.h>
+#include <hfm/ModelFormatRegistry.h>
+#include <FBXSerializer.h>
+#include <OBJSerializer.h>
+#include <GLTFSerializer.h>
 
 Q_LOGGING_CATEGORY(trace_resource_parse_geometry, "trace.resource.parse.geometry")
 
@@ -308,6 +312,11 @@ ModelCache::ModelCache() {
     const qint64 GEOMETRY_DEFAULT_UNUSED_MAX_SIZE = DEFAULT_UNUSED_MAX_SIZE;
     setUnusedResourceCacheSize(GEOMETRY_DEFAULT_UNUSED_MAX_SIZE);
     setObjectName("ModelCache");
+
+    auto modelFormatRegistry = DependencyManager::get<ModelFormatRegistry>();
+    modelFormatRegistry->addFormat(FBXSerializer());
+    modelFormatRegistry->addFormat(OBJSerializer());
+    modelFormatRegistry->addFormat(GLTFSerializer());
 }
 
 QSharedPointer<Resource> ModelCache::createResource(const QUrl& url, const QSharedPointer<Resource>& fallback,

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

@@ -21,6 +21,7 @@
 #include "FBXSerializer.h"
 #include "TextureCache.h"
 #include "ModelLoader.h"
+#include "hfm/FormatSerializerRegister.h"
 
 // Alias instead of derive to avoid copying
 

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

@@ -1,41 +0,0 @@
-//
-//  ModelFormatRegistry.cpp
-//  libraries/model-networking/src/model-networking
-//
-//  Created by Sabrina Shanman on 2018/11/30.
-//  Copyright 2018 High Fidelity, Inc.
-//
-//  Distributed under the Apache License, Version 2.0.
-//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
-//
-
-#include "ModelFormatRegistry.h"
-
-#include "FBXSerializer.h"
-#include "OBJSerializer.h"
-#include "GLTFSerializer.h"
-
-ModelFormatRegistry::ModelFormatRegistry() : hfm::FormatRegistry() {
-    addDefaultFormats();
-}
-
-void ModelFormatRegistry::addDefaultFormats() {
-    addFormat(FBXSerializer::FORMAT);
-    addFormat(OBJSerializer::FORMAT);
-    addFormat(GLTFSerializer::FORMAT);
-}
-
-void ModelFormatRegistry::addFormat(const std::shared_ptr<hfm::Format>& format) {
-    format->registerFormat(*this);
-    {
-        std::lock_guard<std::mutex> lock(_formatsLock);
-        formats.push_back(format);
-    }
-}
-
-ModelFormatRegistry::~ModelFormatRegistry() {
-    for (auto& format : formats) {
-        format->unregisterFormat(*this);
-    }
-    formats.clear();
-}

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

@@ -1,32 +0,0 @@
-//
-//  ModelFormatRegistry.h
-//  libraries/model-networking/src/model-networking
-//
-//  Created by Sabrina Shanman on 2018/11/30.
-//  Copyright 2018 High Fidelity, Inc.
-//
-//  Distributed under the Apache License, Version 2.0.
-//  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
-//
-
-#ifndef hifi_ModelFormatRegistry_h
-#define hifi_ModelFormatRegistry_h
-
-#include <hfm/HFMFormat.h>
-#include <hfm/HFMFormatRegistry.h>
-
-#include <DependencyManager.h>
-
-class ModelFormatRegistry : public hfm::FormatRegistry, public Dependency {
-public:
-    ModelFormatRegistry();
-    ~ModelFormatRegistry();
-    void addFormat(const std::shared_ptr<hfm::Format>& format);
-
-protected:
-    void addDefaultFormats();
-    std::vector<std::shared_ptr<hfm::Format>> formats;
-    std::mutex _formatsLock;
-};
-
-#endif // hifi_ModelFormatRegistry_h

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

@@ -12,7 +12,7 @@
 #include "ModelLoader.h"
 
 #include <DependencyManager.h>
-#include "ModelFormatRegistry.h"
+#include <hfm/ModelFormatRegistry.h>
 
 
 hfm::Model::Pointer ModelLoader::load(const hifi::ByteArray& data, const hifi::VariantHash& mapping, const hifi::URL& url, const std::string& webMediaType) const {