Merge pull request #14898 from bgbsww/bgbsww-toponamingSaveRestoreElementMaps

Toponaming: Fix save and restore of elementmaps

src/App/ComplexGeoData.cpp

@@ -457,7 +457,7 @@ void ComplexGeoData::Restore(Base::XMLReader& reader)
 
     const char* file = "";
     if (reader.hasAttribute("file")) {
-        reader.getAttribute("file");
+        file = reader.getAttribute("file");
     }
     if (*file != 0) {
         reader.addFile(file, this);

src/App/Property.cpp

@@ -88,6 +88,31 @@ std::string Property::getFullName() const {
     return name;
 }
 
+std::string Property::getFileName(const char* postfix, const char* prefix) const
+{
+    std::ostringstream ss;
+    if (prefix) {
+        ss << prefix;
+    }
+    if (!myName) {
+        ss << "Property";
+    }
+    else {
+        std::string name = getFullName();
+        auto pos = name.find('#');
+        if (pos == std::string::npos) {
+            ss << name;
+        }
+        else {
+            ss << (name.c_str() + pos + 1);
+        }
+    }
+    if (postfix) {
+        ss << postfix;
+    }
+    return ss.str();
+}
+
 short Property::getType() const
 {
     short type = 0;

src/App/Property.h

@@ -288,6 +288,9 @@ protected:
     /// Verify a path for the current property
     virtual void verifyPath(const App::ObjectIdentifier & p) const;
 
+    /// Return a file name suitable for saving this property
+    std::string getFileName(const char *postfix=0, const char *prefix=0) const;
+
 public:
     // forbidden
     Property(const Property&) = delete;

src/Base/Reader.cpp

@@ -264,8 +264,8 @@ bool Base::XMLReader::isEndOfDocument() const
 void Base::XMLReader::readEndElement(const char* ElementName, int level)
 {
     // if we are already at the end of the current element
-    if (ReadType == EndElement && ElementName && LocalName == ElementName
-        && (level < 0 || level == Level)) {
+    if ((ReadType == EndElement || ReadType == StartEndElement) && ElementName
+        && LocalName == ElementName && (level < 0 || level == Level)) {
         return;
     }
     if (ReadType == EndDocument) {

src/Base/Writer.cpp

@@ -320,6 +320,11 @@ void Writer::decInd()
     indBuf[indent] = '\0';
 }
 
+void Writer::putNextEntry(const char* file, const char* obj)
+{
+    ObjectName = obj ? obj : file;
+}
+
 // ----------------------------------------------------------------------------
 
 ZipWriter::ZipWriter(const char* FileName)
@@ -346,6 +351,13 @@ ZipWriter::ZipWriter(std::ostream& os)
     ZipStream.setf(ios::fixed, ios::floatfield);
 }
 
+void ZipWriter::putNextEntry(const char* file, const char* obj)
+{
+    Writer::putNextEntry(file, obj);
+
+    ZipStream.putNextEntry(file);
+}
+
 void ZipWriter::writeFiles()
 {
     // use a while loop because it is possible that while
@@ -353,7 +365,9 @@ void ZipWriter::writeFiles()
     size_t index = 0;
     while (index < FileList.size()) {
         FileEntry entry = FileList[index];
-        ZipStream.putNextEntry(entry.FileName);
+        putNextEntry(entry.FileName.c_str());
+        indent = 0;
+        indBuf[0] = 0;
         entry.Object->SaveDocFile(*this);
         index++;
     }
@@ -372,8 +386,10 @@ FileWriter::FileWriter(const char* DirName)
 
 FileWriter::~FileWriter() = default;
 
-void FileWriter::putNextEntry(const char* file)
+void FileWriter::putNextEntry(const char* file, const char* obj)
 {
+    Writer::putNextEntry(file, obj);
+
     std::string fileName = DirName + "/" + file;
     this->FileStream.open(fileName.c_str(), std::ios::out | std::ios::binary);
 }
@@ -402,8 +418,9 @@ void FileWriter::writeFiles()
                 fi.createDirectory();
             }
 
-            std::string fileName = DirName + "/" + entry.FileName;
-            this->FileStream.open(fileName.c_str(), std::ios::out | std::ios::binary);
+            putNextEntry(entry.FileName.c_str());
+            indent = 0;
+            indBuf[0] = 0;
             entry.Object->SaveDocFile(*this);
             this->FileStream.close();
         }

src/Base/Writer.h

@@ -68,6 +68,9 @@ public:
     void setFileVersion(int);
     int getFileVersion() const;
 
+    /// put the next entry with a give name
+    virtual void putNextEntry(const char* filename, const char* objName = nullptr);
+
     /// insert a file as CDATA section in the XML file
     void insertAsciiFile(const char* FileName);
     /// insert a binary file BASE64 coded as CDATA section in the XML file
@@ -206,10 +209,7 @@ public:
     {
         ZipStream.setLevel(level);
     }
-    void putNextEntry(const char* str)
-    {
-        ZipStream.putNextEntry(str);
-    }
+    void putNextEntry(const char* filename, const char* objName = nullptr) override;
 
     ZipWriter(const ZipWriter&) = delete;
     ZipWriter(ZipWriter&&) = delete;
@@ -256,7 +256,7 @@ public:
     explicit FileWriter(const char* DirName);
     ~FileWriter() override;
 
-    void putNextEntry(const char* file);
+    void putNextEntry(const char* filename, const char* objName = nullptr) override;
     void writeFiles() override;
 
     std::ostream& Stream() override

src/Mod/Part/App/PropertyTopoShape.cpp

@@ -260,7 +260,7 @@ void PropertyPartShape::beforeSave() const
         _Shape.beforeSave();
     }
 }
-
+#ifndef FC_USE_TNP_FIX
 void PropertyPartShape::Save (Base::Writer &writer) const
 {
     if(!writer.isForceXML()) {
@@ -278,7 +278,7 @@ void PropertyPartShape::Save (Base::Writer &writer) const
     }
 }
 
-#ifdef NOT_YET_AND_MAYBE_NEVER
+#else
 void PropertyPartShape::Save (Base::Writer &writer) const
 {
     //See SaveDocFile(), RestoreDocFile()
@@ -304,17 +304,17 @@ void PropertyPartShape::Save (Base::Writer &writer) const
     bool toXML = writer.getFileVersion()>1 && writer.isForceXML()>=(binary?3:2);
     if(!toXML) {
         writer.Stream() << " file=\""
-                        << writer.addFile(getFileName(binary?".bin":".brp"), this)
+                        << writer.addFile(getFileName(binary?".bin":".brp").c_str(), this)
                         << "\"/>\n";
     } else if(binary) {
         writer.Stream() << " binary=\"1\">\n";
         TopoShape shape;
         shape.setShape(_Shape.getShape());
-        shape.exportBinary(writer.beginCharStream(true));
+        shape.exportBinary(writer.beginCharStream());
         writer.endCharStream() <<  writer.ind() << "</Part>\n";
     } else {
         writer.Stream() << " brep=\"1\">\n";
-        _Shape.exportBrep(writer.beginCharStream(false)<<'\n');
+        _Shape.exportBrep(writer.beginCharStream()<<'\n');
         writer.endCharStream() << '\n' << writer.ind() << "</Part>\n";
     }
 
@@ -341,6 +341,7 @@ std::string PropertyPartShape::getElementMapVersion(bool restored) const {
     return PropertyComplexGeoData::getElementMapVersion(false);
 }
 
+#ifndef FC_USE_TNP_FIX
 void PropertyPartShape::Restore(Base::XMLReader &reader)
 {
     reader.readElement("Part");
@@ -352,7 +353,7 @@ void PropertyPartShape::Restore(Base::XMLReader &reader)
     }
 }
 
-#ifdef NOT_YET_AND_MAYBE_NEVER
+#else
 void PropertyPartShape::Restore(Base::XMLReader &reader)
 {
     reader.readElement("Part");
@@ -363,9 +364,14 @@ void PropertyPartShape::Restore(Base::XMLReader &reader)
     if(has_ver)
         _Ver = reader.getAttribute("ElementMap");
 
-    int hasher_idx = reader.getAttributeAsInteger("HasherIndex","-1");
-    int save_hasher = reader.getAttributeAsInteger("SaveHasher","");
-
+    int hasher_idx = -1;
+    int save_hasher = 0;
+    if ( reader.hasAttribute("HasherIndex") ) {
+        reader.getAttributeAsInteger("HasherIndex");
+    }
+    if ( reader.hasAttribute("SaveHasher") ) {
+        save_hasher = reader.getAttributeAsInteger("SaveHasher");
+    }
     TopoDS_Shape sh;
 
     if(reader.hasAttribute("file")) {
@@ -374,13 +380,13 @@ void PropertyPartShape::Restore(Base::XMLReader &reader)
             // initiate a file read
             reader.addFile(file.c_str(),this);
         }
-    } else if(reader.getAttributeAsInteger("binary","")) {
+    } else if( reader.hasAttribute(("binary")) && reader.getAttributeAsInteger("binary")) {
         TopoShape shape;
-        shape.importBinary(reader.beginCharStream(true));
+        shape.importBinary(reader.beginCharStream());
         sh = shape.getShape();
-    } else if(reader.getAttributeAsInteger("brep","")) {
+    } else if( reader.hasAttribute("brep") && reader.getAttributeAsInteger("brep")) {
         BRep_Builder builder;
-        BRepTools::Read(sh, reader.beginCharStream(false), builder);
+        BRepTools::Read(sh, reader.beginCharStream(), builder);
     }
 
     reader.readEndElement("Part");
@@ -425,7 +431,8 @@ void PropertyPartShape::Restore(Base::XMLReader &reader)
             }
         }
     } else if(owner && !owner->getDocument()->testStatus(App::Document::PartialDoc)) {
-        if(App::DocumentParams::getWarnRecomputeOnRestore()) {
+        // if(App::DocumentParams::getWarnRecomputeOnRestore()) {
+        if( true ) {
             FC_WARN("Pending recompute for generating element map: " << owner->getFullName());
             owner->getDocument()->addRecomputeObject(owner);
         }
@@ -438,18 +445,18 @@ void PropertyPartShape::Restore(Base::XMLReader &reader)
     }
 }
 
-void PropertyPartShape::afterRestore()
-{
-    if (_Shape.isRestoreFailed()) {
-        // this cause GeoFeature::updateElementReference() to call
-        // PropertyLinkBase::updateElementReferences() with reverse = true, in
-        // order to try to regenerate the element map
-        _Ver = "?";
-    }
-    else if (_Shape.getElementMapSize() == 0)
-        _Shape.Hasher->clear(); //reset();
-    PropertyComplexGeoData::afterRestore();
-}
+// void PropertyPartShape::afterRestore()
+// {
+//     if (_Shape.isRestoreFailed()) {
+//         // this cause GeoFeature::updateElementReference() to call
+//         // PropertyLinkBase::updateElementReferences() with reverse = true, in
+//         // order to try to regenerate the element map
+//         _Ver = "?";
+//     }
+//     else if (_Shape.getElementMapSize() == 0)
+//         _Shape.Hasher->clear(); //reset();
+//     PropertyComplexGeoData::afterRestore();
+// }
 #endif
 
 // The following function is copied from OCCT BRepTools.cxx and modified

src/Mod/Part/App/PropertyTopoShapeList.cpp

@@ -154,6 +154,104 @@ void PropertyTopoShapeList::setPyObject(PyObject *value)
     }
 }
 
+#ifdef FC_USE_TNP_FIX
+
+void PropertyTopoShapeList::Save(Writer& writer) const
+{
+    writer.Stream() << writer.ind() << "<ShapeList count=\"" << getSize() << "\">" << endl;
+    writer.incInd();
+    for (int i = 0; i < getSize(); i++) {
+        bool binary = writer.getMode("BinaryBrep");
+        writer.Stream() << writer.ind() << "<TopoShape";
+        if (!writer.isForceXML()) {
+            // See SaveDocFile(), RestoreDocFile()
+            //  add a filename to the writer's list.  Each file on the list is eventually
+            //  processed by SaveDocFile().
+            std::string ext(".");
+            ext += std::to_string(i);
+            if (binary) {
+                ext += ".bin";
+            }
+            else {
+                ext += ".brp";
+            }
+            writer.Stream() << writer.ind() << " file=\""
+                            << writer.addFile(getFileName(ext.c_str()).c_str(), this) << "\"/>\n";
+        }
+        else if (binary) {
+            writer.Stream() << " binary=\"1\">\n";
+            _lValueList[i].exportBinary(writer.beginCharStream());
+            writer.endCharStream() << writer.ind() << "</TopoShape>\n";
+        }
+        else {
+            writer.Stream() << " brep=\"1\">\n";
+            _lValueList[i].exportBrep(writer.beginCharStream() << '\n');
+            writer.endCharStream() << '\n' << writer.ind() << "</TopoShape>\n";
+        }
+    }
+    writer.decInd();
+    writer.Stream() << writer.ind() << "</ShapeList>" << endl;
+}
+
+void PropertyTopoShapeList::SaveDocFile(Base::Writer& writer) const
+{
+    Base::FileInfo finfo(writer.ObjectName);
+    bool binary = finfo.hasExtension("bin");
+    int index = atoi(Base::FileInfo(finfo.fileNamePure()).extension().c_str());
+    if (index < 0 || index >= static_cast<int>(_lValueList.size())) {
+        return;
+    }
+
+    const TopoShape& shape = _lValueList[index];
+    if (binary) {
+        shape.exportBinary(writer.Stream());
+    }
+    else {
+        shape.exportBrep(writer.Stream());
+    }
+}
+
+void PropertyTopoShapeList::Restore(Base::XMLReader& reader)
+{
+    reader.readElement("ShapeList");
+    int count = reader.getAttributeAsInteger("count");
+    m_restorePointers.clear();  // just in case
+    m_restorePointers.reserve(count);
+    for (int i = 0; i < count; i++) {
+        auto newShape = std::make_shared<TopoShape>();
+        reader.readElement("TopoShape");
+        std::string file(reader.getAttribute("file"));
+        if (!file.empty()) {
+            reader.addFile(file.c_str(), this);
+        }
+        else if (reader.hasAttribute("binary") && reader.getAttributeAsInteger("binary")) {
+            newShape->importBinary(reader.beginCharStream());
+        }
+        else if (reader.hasAttribute("brep") && reader.getAttributeAsInteger("brep")) {
+            newShape->importBrep(reader.beginCharStream());
+        }
+        m_restorePointers.push_back(newShape);
+    }
+    reader.readEndElement("ShapeList");
+}
+
+void PropertyTopoShapeList::RestoreDocFile(Base::Reader& reader)
+{
+    Base::FileInfo finfo(reader.getFileName());
+    bool binary = finfo.hasExtension("bin");
+    int index = atoi(Base::FileInfo(finfo.fileNamePure()).extension().c_str());
+    if (index < 0 || index >= static_cast<int>(m_restorePointers.size())) {
+        return;
+    }
+    if (binary) {
+        m_restorePointers[index]->importBinary(reader);
+    }
+    else {
+        m_restorePointers[index]->importBrep(reader);
+    }
+}
+
+#else
 void PropertyTopoShapeList::Save(Writer &writer) const
 {
     writer.Stream() << writer.ind() << "<ShapeList count=\"" << getSize() <<"\">" << endl;
@@ -178,7 +276,7 @@ void PropertyTopoShapeList::Restore(Base::XMLReader &reader)
     }
     reader.readEndElement("ShapeList");
 }
-
+#endif
 App::Property *PropertyTopoShapeList::Copy() const
 {
     PropertyTopoShapeList *p = new PropertyTopoShapeList();

src/Mod/Part/App/PropertyTopoShapeList.h

@@ -84,6 +84,9 @@ public:
     void Save(Base::Writer &writer) const override;
     void Restore(Base::XMLReader &reader) override;
 
+    void SaveDocFile (Base::Writer &writer) const override;
+    void RestoreDocFile(Base::Reader &reader) override;
+
     App::Property *Copy() const override;
     void Paste(const App::Property &from) override;
 

src/Mod/Part/App/TopoShape.cpp

@@ -1199,7 +1199,29 @@ bool TopoShape::getCenterOfGravity(Base::Vector3d& center) const
 
     return false;
 }
+#ifdef FC_USE_TNP_FIX
+void TopoShape::Save (Base::Writer &writer ) const
+{
+    Data::ComplexGeoData::Save(writer);
+}
 
+void TopoShape::Restore(Base::XMLReader &reader)
+{
+    Data::ComplexGeoData::Restore(reader);
+}
+
+void TopoShape::SaveDocFile (Base::Writer &writer) const
+{
+    Data::ComplexGeoData::SaveDocFile(writer);
+}
+
+void TopoShape::RestoreDocFile(Base::Reader &reader)
+{
+    Data::ComplexGeoData::RestoreDocFile(reader);
+}
+
+
+#else
 void TopoShape::Save (Base::Writer& writer) const
 {
     if(!writer.isForceXML()) {
@@ -1252,7 +1274,7 @@ void TopoShape::RestoreDocFile(Base::Reader& reader)
         importBrep(reader);
     }
 }
-
+#endif
 unsigned int TopoShape_RefCountShapes(const TopoDS_Shape& aShape)
 {
     unsigned int size = 1; // this shape

src/Mod/Part/parttests/TopoShapeTest.py

@@ -90,10 +90,10 @@ class TopoShapeTest(unittest.TestCase, TopoShapeAssertions):
             ["CenterOfMass", App.Vector(1, 0.5, 1)],
             ["CompSolids", []],
             ["Compounds", []],
-            ["Content", ""],
+            ["Content", "<ElementMap/>\n"], # Our element map is empty, or there would be more here.
             ["ElementMap", {}],
             ["ElementReverseMap", {}],
-            # ['Hasher', {}],    # Todo:  Should this exist?  Different implementation?
+            ['Hasher', None],
             [
                 "MatrixOfInertia",
                 App.Matrix(

src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py

@@ -1463,6 +1463,17 @@ class TestTopologicalNamingProblem(unittest.TestCase):
         else:
             self.assertEqual(App.Gui.Selection.getSelectionEx("", 0)[0].SubElementNames[0][-8:],",F.Face2")
 
+    def testFileSaveRestore(self):
+        self.Body = self.Doc.addObject('PartDesign::Body', 'Body')
+        self.create_t_sketch()
+        self.assertEqual(self.Doc.Sketch.Shape.ElementMapSize, 18)
+        filename = self.Doc.Name
+        self.Doc.saveAs(filename)
+        App.closeDocument(filename)
+        self.Doc = App.openDocument(filename+".FCStd")
+        self.Doc.recompute()
+        self.assertEqual(self.Doc.Sketch.Shape.ElementMapSize, 18)
+
     def create_t_sketch(self):
         self.Doc.getObject('Body').newObject('Sketcher::SketchObject', 'Sketch')
         geo_list = [
@@ -1492,4 +1503,4 @@ class TestTopologicalNamingProblem(unittest.TestCase):
 
     def tearDown(self):
         """ Close our test document """
-        App.closeDocument("PartDesignTestTNP")
+        App.closeDocument(self.Doc.Name)