supported link to model in serialize/deserialize #5240

pashem · 2025-10-15T12:03:26Z

No description provided.

adalisk-emikhaylov · 2025-10-16T09:11:38Z

source/MRMesh/MRObjectMeshHolder.cpp

+    if ( const auto objectMeshHolder = dynamic_cast< const ObjectMeshHolder* >( &other ) )
+    {
+        // can we compare vert colors ?
+        return data_.mesh == objectMeshHolder->data_.mesh && data_.vertColors == objectMeshHolder->data_.vertColors;


Not all mesh formats include vertex colors, need to check MeshSaverCapabilities first, I think.

Fedr · 2025-10-16T09:48:09Z

source/MRMesh/MRObjectSave.cpp

+        std::error_code ec;
+        if ( !std::filesystem::is_directory( sharedFolder, ec ) )
+            if ( !std::filesystem::create_directories( sharedFolder, ec ) )
+                return MR::unexpected( "Cannot create directories " + MR::utf8string( sharedFolder ) );


Do we need MR:: here?

Fedr · 2025-10-16T09:51:35Z

source/MRMesh/MRObjectSave.cpp

+        if ( node->isAncillary() )
+            continue; // consider ancillary_ objects as temporary, not requiring saving
+
+        auto it = links.find( { node } );


you can use insert instead of find + [], insert returns indication whether new item was added or already was in map

Fedr · 2025-10-16T09:54:42Z

source/MRMesh/MRObject.h


    /// Reads model from file
    MRMESH_API virtual Expected<void> deserializeModel_( const std::filesystem::path& path, ProgressCallback progressCb = {} );
+    /// reads model from oter object


oter -> other, reads - > shares

Fedr · 2025-10-16T09:56:06Z

source/MRMesh/MRObject.h

    [[nodiscard]] MRMESH_API virtual size_t heapBytes() const;

+    // return true if model of current object equals to model of other
+    MRMESH_API virtual bool isModelEqual( const Object& other ) const;


better name something like sameModels or areModelsEqual

Fedr · 2025-10-16T10:00:56Z

source/MRMesh/MRObject.h


 /// \}

+struct KeyObjectModel


why do we need it?

Because we need key for comapring objectA and objectB and in this logic of shared models we consider them equal when equal (the same) them models

Fedr · 2025-10-16T10:01:56Z

source/MRMesh/MRObject.h

+
+struct LinkData
+{
+    std::string link; // relative path (link) to shared model file


if it is a path, then better use std::filesystem::path

Grantim · 2025-10-16T09:41:25Z

source/MRMesh/MRObject.h

+// map for mapping Objects to relative path (link) to shared model (used while serialization)
+using MapSharedObjectModelToLinkData = std::unordered_map<KeyObjectModel, LinkData, KeyObjectModelHasher>;
+// map for mapping relative path (link) to shared model file to first deserialized Object (used while deserialization)
+using MapLinkToSharedObjectModel = std::unordered_map<std::string, const Object*>;


why do we need both?

I don't understund

first map for serialization
we need key for comparing ObjectA and ObjectB and in this logic of shared models we consider them equal when equal (the same) them models. After collecting all links to shared objects we have map
with pairs <ObjectA, SharedModels/N_model> and while serializing we map objectA to SharedModels/N_model and save model file and save SharedModels/N_model to root["Link"], ObjectB equals (have the same model as ObjectA ), and we no need to serialize his model we just save link to already saved model file to root["Link"]

2.second map for deserialization
This map empty before deserialization
after deserialize model of object ObjectA, we save to map values <SharedModels/N_model, ObjectA> and while deserializing ObjectB we get the same link to file via root["Link"], and find in map that SharedModels/N_model have already deserialized by ObjectA and we get model from ObjectA an set it to ObjectB via ObjectB.setSharedModel_(ObjectA)

Grantim · 2025-10-16T09:41:48Z

source/MRMesh/MRObject.h

+    MRMESH_API MR_BIND_IGNORE Expected<std::vector<std::future<Expected<void>>>> serializeRecursive( const std::filesystem::path& path, Json::Value& root,
+        int childId, const std::filesystem::path& rootFolder, MapSharedObjectModelToLinkData* mapSharedObjectModelToLinkData = nullptr ) const;


why do we need to pass rootFolder?

we have relative path in Link struct we need known physical path for directory while serialization
PS we can save rootFolder to LinkData but i made the same as in deserializeRecursive()

Grantim · 2025-10-16T09:42:07Z

source/MRMesh/MRObject.h

+    MRMESH_API Expected<void> deserializeRecursive( const std::filesystem::path& path, const Json::Value& root, const std::filesystem::path& rootFolder,
+        ProgressCallback progressCb = {}, int* objCounter = nullptr, MapLinkToSharedObjectModel* mapLinkToSharedObjectModel = nullptr );


we have relative path in Link field of json and we need known physical path for directory while deserialization

Grantim · 2025-10-16T09:52:40Z

source/MRMesh/MRObject.cpp

 }

-Expected<std::vector<std::future<Expected<void>>>> Object::serializeRecursive( const std::filesystem::path& path, Json::Value& root, int childId ) const
+Expected<std::vector<std::future<Expected<void>>>> Object::serializeRecursive( const std::filesystem::path& path, Json::Value& root, int childId, const std::filesystem::path& rootFolder, MapSharedObjectModelToLinkData* mapSharedObjectModelToLinkData ) const


what if we move this implementation into protected serializeRecursive_ function with input parameters structure and implement this one like this:

void serializeRecursive_(...) { // current implementation serializeRecursive_(); } void serialize( ) { // build map with links serializeRecursive_(); // will all extra parameters, so we don't need to change ObjectSave/ObjectLoad }

Grantim · 2025-10-16T09:53:39Z

source/MRMesh/MRObject.cpp

-    if ( !res.has_value() )
-        return res;
+    std::string link;
+    if ( !root["Link"].empty() )


better to check root["Link"].isString() too

Grantim · 2025-10-16T10:03:01Z

source/MRMesh/MRObject.h

+struct LinkData
+{
+    std::string link; // relative path (link) to shared model file
+    bool serialized = false; // set true after first serialization to avoid other model copies
+};


can we use const Object* instead of this structure? (so it will be single MapType for both serialization and deserialization)

auto it = map.find( this ); if ( it != map.end() ) // add link from it->second->createLink() else // serialize as not-shared model if ( it != map.end() && it->second == this ) // serialize as shared model

Grantim · 2025-10-16T10:06:44Z

source/MRMesh/MRObject.h

+    /// reads model from oter object
+    MRMESH_API virtual Expected<void> setModelFromObject_( const Object& other );


lets rename it to something like setSharedModel_, and add comment that it differs form simple setModel because some Object types consider different fields as model

Grantim · 2025-10-16T10:08:49Z

source/MRMesh/MRObjectMeshHolder.cpp

+Expected<void> ObjectMeshHolder::setModelFromObject_( const Object& other )
+{
+    if ( const auto objectMeshHolder = dynamic_cast< const ObjectMeshHolder* >( &other ) )
+    {
+        data_.mesh = objectMeshHolder->data_.mesh;
+        data_.vertColors = objectMeshHolder->data_.vertColors;
+        return{};
+    }
+    return unexpected("Invalid object type");
+}


can we use setData here?

No, because we get some fields of data_ from json, may be in future all model data saved to file move to separate struct?

Than we should validate format capabilities in this check, because if we cannot save vert map in model we will save it in json (for example for mrmesh format), and same logic will be applicable for it

Grantim · 2025-10-16T10:10:10Z

source/MRMesh/MRObjectSave.cpp

+    // collect links to shared models
+    MapSharedObjectModelToLinkData links;
+    std::filesystem::path sharedFolder;
+    const auto countSharedFiles = collectLinks(object, scenePath, links, sharedFolder );
+    if ( countSharedFiles > 0 )
+    {
+        std::error_code ec;
+        if ( !std::filesystem::is_directory( sharedFolder, ec ) )
+            if ( !std::filesystem::create_directories( sharedFolder, ec ) )
+                return MR::unexpected( "Cannot create directories " + MR::utf8string( sharedFolder ) );
+    }


i think we should move this logic inside Object::serialize function

it can be done once with small refactoring #5240 (comment)

Grantim · 2025-10-16T10:11:08Z

source/MRMesh/MRObjectSave.cpp

+        auto it = links.find( { node } );
+        if ( it == links.end() )
+            links[{ node }] = {};


links.insert

Grantim · 2025-10-17T08:34:38Z

source/MRMesh/MRObject.cpp

+// return shared models count
+int collectLinks( const Object& rootObject, MapSharedObjects& links )


detailed comment would be better

pashem requested a review from gorbovskoy-evg October 15, 2025 12:03

pashem force-pushed the 5238-de-serialization-of-objects-with-same-shared-model-mesh-support branch 2 times, most recently from f1e04d2 to 29e9bc8 Compare October 15, 2025 14:37

adalisk-emikhaylov requested changes Oct 16, 2025

View reviewed changes

Fedr reviewed Oct 16, 2025

View reviewed changes

Grantim reviewed Oct 16, 2025

View reviewed changes

pashem force-pushed the 5238-de-serialization-of-objects-with-same-shared-model-mesh-support branch 2 times, most recently from da913d5 to 09af1e4 Compare October 16, 2025 23:15

adalisk-emikhaylov approved these changes Oct 17, 2025

View reviewed changes

Grantim approved these changes Oct 17, 2025

View reviewed changes

Fedr approved these changes Oct 17, 2025

View reviewed changes

gorbovskoy-evg approved these changes Oct 17, 2025

View reviewed changes

pashem added 3 commits October 17, 2025 13:20

supported link to shared model in serialize/deserialize

e7ae47c

fix after review

97fe76c

fix after review (add comment)

99313fb

pashem force-pushed the 5238-de-serialization-of-objects-with-same-shared-model-mesh-support branch from 09af1e4 to 99313fb Compare October 17, 2025 09:27

pashem merged commit 3f36c2b into master Oct 17, 2025
35 checks passed

pashem deleted the 5238-de-serialization-of-objects-with-same-shared-model-mesh-support branch October 17, 2025 10:22

		MRMESH_API MR_BIND_IGNORE Expected<std::vector<std::future<Expected<void>>>> serializeRecursive( const std::filesystem::path& path, Json::Value& root,
		int childId, const std::filesystem::path& rootFolder, MapSharedObjectModelToLinkData* mapSharedObjectModelToLinkData = nullptr ) const;

		MRMESH_API Expected<void> deserializeRecursive( const std::filesystem::path& path, const Json::Value& root, const std::filesystem::path& rootFolder,
		ProgressCallback progressCb = {}, int* objCounter = nullptr, MapLinkToSharedObjectModel* mapLinkToSharedObjectModel = nullptr );

		/// reads model from oter object
		MRMESH_API virtual Expected<void> setModelFromObject_( const Object& other );

		// return shared models count
		int collectLinks( const Object& rootObject, MapSharedObjects& links )

supported link to model in serialize/deserialize #5240

supported link to model in serialize/deserialize #5240

Uh oh!

Conversation

pashem commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pashem Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pashem Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Grantim Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Grantim Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pashem Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pashem Oct 16, 2025 •

edited

Loading

pashem Oct 16, 2025 •

edited

Loading

Grantim Oct 16, 2025 •

edited

Loading

Grantim Oct 16, 2025 •

edited

Loading

pashem Oct 16, 2025 •

edited

Loading