From f9b65e563dd3f64fd06bcdd63e1ee72d5541c4ea Mon Sep 17 00:00:00 2001 From: bgbsww Date: Wed, 1 May 2024 18:03:00 -0400 Subject: [PATCH] Test for changed Sketches avoiding TNP --- src/App/GeoFeature.cpp | 41 -------- src/App/GeoFeature.h | 23 ----- src/App/PropertyLinks.cpp | 30 ------ .../TestTopologicalNamingProblem.py | 94 ++++++++++++++++++- 4 files changed, 93 insertions(+), 95 deletions(-) diff --git a/src/App/GeoFeature.cpp b/src/App/GeoFeature.cpp index fcb13566ef..391928818f 100644 --- a/src/App/GeoFeature.cpp +++ b/src/App/GeoFeature.cpp @@ -155,11 +155,6 @@ DocumentObject *GeoFeature::resolveElement(DocumentObject *obj, const char *subn return nullptr; auto linked = sobj->getLinkedObject(true); auto geo = Base::freecad_dynamic_cast(linked); -// if(!geo && linked) { -// auto ext = linked->getExtensionByType(true); -// if(ext) -// geo = Base::freecad_dynamic_cast(ext->getTrueLinkedObject(true)); -// } #else auto sobj = obj->getSubObject(subname); if(!sobj) @@ -223,13 +218,6 @@ void GeoFeature::updateElementReference() { auto geo = prop->getComplexData(); if(!geo) return; bool reset = false; -// auto version = getElementMapVersion(prop); -// if(_ElementMapVersion.getStrValue().empty()) -// _ElementMapVersion.setValue(version); -// else if(_ElementMapVersion.getStrValue()!=version) { -// reset = true; -// _ElementMapVersion.setValue(version); -// } PropertyLinkBase::updateElementReferences(this,reset); } @@ -244,35 +232,6 @@ void GeoFeature::onChanged(const Property *prop) { DocumentObject::onChanged(prop); } -//void GeoFeature::onDocumentRestored() { -// if(!getDocument()->testStatus(Document::Status::Importing)) -// _ElementMapVersion.setValue(getElementMapVersion(getPropertyOfGeometry(),true)); -// DocumentObject::onDocumentRestored(); -//} - -//const std::vector& -//GeoFeature::searchElementCache(const std::string &element, -// Data::SearchOptions options, -// double tol, -// double atol) const -//{ -// static std::vector none; -// (void)element; -// (void)options; -// (void)tol; -// (void)atol; -// return none; -//} - -//const std::vector& -//GeoFeature::getElementTypes(bool /*all*/) const -//{ -// static std::vector nil; -// auto prop = getPropertyOfGeometry(); -// if (!prop) -// return nil; -// return prop->getComplexData()->getElementTypes(); -//} std::vector GeoFeature::getHigherElements(const char *element, bool silent) const diff --git a/src/App/GeoFeature.h b/src/App/GeoFeature.h index 038daeb2de..3dcdffd6eb 100644 --- a/src/App/GeoFeature.h +++ b/src/App/GeoFeature.h @@ -143,33 +143,10 @@ public: #ifdef FC_USE_TNP_FIX static bool hasMissingElement(const char *subname); - /** Search sub element using internal cached geometry - * - * @param element: element name - * @param options: search options - * @param tol: coordinate tolerance - * @param atol: angle tolerance - * - * @return Returns a list of found element reference to the new goemetry. - * The returned value will be invalidated when the geometry is changed. - * - * Before changing the property of geometry, GeoFeature will internally - * make a snapshot of all referenced element geometry. After change, user - * code may call this function to search for the new element name that - * reference to the same geometry of the old element. - */ -// virtual const std::vector& searchElementCache(const std::string &element, -// Data::SearchOptions options = Data::SearchOption::CheckGeometry, -// double tol = 1e-7, -// double atol = 1e-10) const; - - /// Return the object that owns the shape that contains the give element name virtual DocumentObject *getElementOwner(const Data::MappedName & /*name*/) const {return nullptr;} -// virtual const std::vector& getElementTypes(bool all=true) const; - /// Return the higher level element names of the given element virtual std::vector getHigherElements(const char *name, bool silent=false) const; diff --git a/src/App/PropertyLinks.cpp b/src/App/PropertyLinks.cpp index 61a28ecf86..312dec3497 100644 --- a/src/App/PropertyLinks.cpp +++ b/src/App/PropertyLinks.cpp @@ -400,36 +400,6 @@ bool PropertyLinkBase::_updateElementReference(DocumentObject *feature, << oldName << " -> " << newName); } } - // Note: the following code proves to be too risky. There is no way - // (so far) to ensure the recompute do not change the geometry. If - // the geometry does remain the same, the above geometry search - // should be able to find the new reference any way! -#if 0 - else if (missing && reverse && shadow.first.size()) { - // reverse means we are trying to either generate the element - // name for the first time, or upgrade to a new map version. In - // case of upgrading, we still consult the original mapped name - // in first try. Here means the first try failed, and the - // geometry search cannot find any match, so we try the - // non-mapped name as a last resort. - // - // WARNING! We are assuming the recomputation is done with no - // actual property change, and the resulting geometry remains - // the same. If this condition is not met, the result may be - // undesirable. TODO: find a way to ensure this condition. - - GeoFeature::resolveElement(obj, shadow.second.c_str(), elementName, true, - GeoFeature::ElementNameType::Export,feature); - if(!elementName.second.empty()) { - missing = Data::ComplexGeoData::hasMissingElement(elementName.second.c_str()); - if (!missing) { - FC_WARN(propertyName(this) - << " element reference changed " << ret->getFullName() << " " - << shadow.first << " -> " << elementName.first); - } - } - } -#endif } } diff --git a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py index a027aca68b..907e604ee0 100644 --- a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py +++ b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py @@ -789,6 +789,97 @@ class TestTopologicalNamingProblem(unittest.TestCase): self.assertEqual(plane.Shape.ElementMapSize, 0) self.assertEqual(pad.Shape.ElementMapSize, 26) + def testChangeSketch(self): + # Arrange + self.Body = self.Doc.addObject('PartDesign::Body', 'Body') + # Make first offset cube Pad + self.PadSketch = self.Doc.addObject('Sketcher::SketchObject', 'Sketch') + self.Body.addObject(self.PadSketch) + TestSketcherApp.CreateRectangleSketch(self.PadSketch, (0, 0), (31.37, 25.2)) + self.Doc.recompute() + self.Pad = self.Doc.addObject("PartDesign::Pad", "Pad") + self.Body.addObject(self.Pad) + self.Pad.Profile = self.PadSketch + self.Pad.Length = 10 + self.Doc.recompute() + + self.Sketch001 = self.Body.newObject('Sketcher::SketchObject','Sketch001') + self.Sketch001.AttachmentSupport = (self.Doc.getObject('Pad'),['Face6',]) + self.Sketch001.MapMode = 'FlatFace' + App.ActiveDocument.recompute() + + self.Sketch001.addExternal("Pad","Edge10") + self.Sketch001.addExternal("Pad","Edge7") + + geoList = [] + geoList.append(Part.Circle(App.Vector(15.093666, 13.036922, 0.000000), + App.Vector(0.000000, 0.000000, 1.000000), 5.000000)) + self.Sketch001.addGeometry(geoList,False) + del geoList + self.Sketch001.addConstraint(Sketcher.Constraint('Radius',0,5.000000)) + self.Sketch001.addConstraint(Sketcher.Constraint('Symmetric',-3,2,-4,1,0,3)) + App.ActiveDocument.recompute() + self.Doc.recompute() + + self.Pad001 = self.Body.newObject('PartDesign::Pad','Pad001') + self.Pad001.Profile = self.Doc.getObject('Sketch001') + self.Pad001.Length = 10 + App.ActiveDocument.recompute() + self.Pad001.ReferenceAxis = (self.Doc.getObject('Sketch001'),['N_Axis']) + self.Sketch001.Visibility = False + App.ActiveDocument.recompute() + + self.Pad001.Length = 10.000000 + self.Pad001.TaperAngle = 0.000000 + self.Pad001.UseCustomVector = 0 + self.Pad001.Direction = (0, 0, 1) + self.Pad001.ReferenceAxis = (self.Doc.getObject('Sketch001'), ['N_Axis']) + self.Pad001.AlongSketchNormal = 1 + self.Pad001.Type = 0 + self.Pad001.UpToFace = None + self.Pad001.Reversed = 0 + self.Pad001.Midplane = 0 + self.Pad001.Offset = 0 + self.Doc.recompute() + self.Doc.getObject('Pad').Visibility = False + + self.Doc.getObject('Sketch001').Visibility = False + + # Modify the original sketch to generate TNP issue + geoList = [] + geoList.append(Part.LineSegment(App.Vector(2.510468, 22.837425, 0.000000), + App.Vector(2.510468, 19.933617, 0.000000))) + geoList.append(Part.LineSegment(App.Vector(2.510468, 19.933617, 0.000000), + App.Vector(4.869811, 19.933617, 0.000000))) + geoList.append(Part.LineSegment(App.Vector(4.869811, 19.933617, 0.000000), + App.Vector(4.869811, 22.837425, 0.000000))) + geoList.append(Part.LineSegment(App.Vector(4.869811, 22.837425, 0.000000), + App.Vector(2.510468, 22.837425, 0.000000))) + self.PadSketch.addGeometry(geoList,False) + del geoList + + constraintList = [] + constraintList.append(Sketcher.Constraint('Coincident', 4, 2, 5, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 5, 2, 6, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 6, 2, 7, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 7, 2, 4, 1)) + constraintList.append(Sketcher.Constraint('Vertical', 4)) + constraintList.append(Sketcher.Constraint('Vertical', 6)) + constraintList.append(Sketcher.Constraint('Horizontal', 5)) + constraintList.append(Sketcher.Constraint('Horizontal', 7)) + self.PadSketch.addConstraint(constraintList) + del constraintList + self.Doc.recompute() + # Assert + if self.Body.Shape.ElementMapVersion == "": # Should be '4' as of Mar 2023. + return + self.assertEqual(self.Body.Shape.BoundBox.XMin,0) + self.assertEqual(self.Body.Shape.BoundBox.YMin,0) + self.assertEqual(self.Body.Shape.BoundBox.ZMin,0) + self.assertEqual(self.Body.Shape.BoundBox.XMax,31.37) + self.assertEqual(self.Body.Shape.BoundBox.YMax,25.2) + self.assertEqual(self.Body.Shape.BoundBox.ZMax,20) + def create_t_sketch(self): self.Doc.getObject('Body').newObject('Sketcher::SketchObject', 'Sketch') geo_list = [ @@ -818,4 +909,5 @@ class TestTopologicalNamingProblem(unittest.TestCase): def tearDown(self): """ Close our test document """ - App.closeDocument("PartDesignTestTNP") + # App.closeDocument("PartDesignTestTNP") + pass \ No newline at end of file