From 7680872e41810b222cca5cc37f7f12ec58fa92f2 Mon Sep 17 00:00:00 2001 From: Eric Price <32105268+AIRCAP@users.noreply.github.com> Date: Mon, 11 Nov 2024 17:44:27 +0100 Subject: [PATCH] increase tolerance on helical shapes to avoid crash / broken geometry (#17356) * increase tolerance on helical shapes to avoid crash fix #17113 fix #9377 fix #11083 * added test cases for additive and subtractive helix * fixed whitespace issue, reduced turncount for speed * relaxed large scale testcases for OCCT 7.3 and below, work around windows CI error #17394 * Further reduce max size in test for buggy Ubuntu OCCT --- src/Mod/Part/App/TopoShape.cpp | 2 +- src/Mod/PartDesign/App/FeatureHelix.cpp | 18 +++ src/Mod/PartDesign/App/FeatureHelix.h | 2 + .../PartDesign/PartDesignTests/TestHelix.py | 141 ++++++++++++++++++ 4 files changed, 162 insertions(+), 1 deletion(-) diff --git a/src/Mod/Part/App/TopoShape.cpp b/src/Mod/Part/App/TopoShape.cpp index 6f7c6a95ab..2259e3798b 100644 --- a/src/Mod/Part/App/TopoShape.cpp +++ b/src/Mod/Part/App/TopoShape.cpp @@ -2333,7 +2333,7 @@ TopoDS_Shape TopoShape::makeSpiralHelix(Standard_Real radiusbottom, Standard_Rea } TopoDS_Wire wire = mkWire.Wire(); - BRepLib::BuildCurves3d(wire, Precision::Confusion(), GeomAbs_Shape::GeomAbs_C1, 14, 10000); + BRepLib::BuildCurves3d(wire, Precision::Confusion() * 1e-6 * (radiusbottom+radiustop), GeomAbs_Shape::GeomAbs_C1, 14, 10000); // precision must be reduced for very large parts to avoid #17113 return TopoDS_Shape(std::move(wire)); } diff --git a/src/Mod/PartDesign/App/FeatureHelix.cpp b/src/Mod/PartDesign/App/FeatureHelix.cpp index fa0e82139b..6739db60b7 100644 --- a/src/Mod/PartDesign/App/FeatureHelix.cpp +++ b/src/Mod/PartDesign/App/FeatureHelix.cpp @@ -34,6 +34,7 @@ # include # include # include +# include # include # include # include @@ -61,6 +62,7 @@ PROPERTY_SOURCE(PartDesign::Helix, PartDesign::ProfileBased) // we purposely use not FLT_MAX because this would not be computable const App::PropertyFloatConstraint::Constraints Helix::floatTurns = { Precision::Confusion(), INT_MAX, 1.0 }; +const App::PropertyFloatConstraint::Constraints Helix::floatTolerance = { 0.1, INT_MAX, 1.0 }; const App::PropertyAngle::Constraints Helix::floatAngle = { -89.0, 89.0, 1.0 }; Helix::Helix() @@ -104,6 +106,9 @@ Helix::Helix() ADD_PROPERTY_TYPE(HasBeenEdited, (false), group, App::Prop_Hidden, QT_TRANSLATE_NOOP("App::Property", "If false, the tool will propose an initial value for the pitch based on the profile bounding box,\n" "so that self intersection is avoided.")); + ADD_PROPERTY_TYPE(Tolerance, (0.1), group, App::Prop_None, + QT_TRANSLATE_NOOP("App::Property", "Fusion Tolerance for the Helix, increase if helical shape does not merge nicely with part.")); + Tolerance.setConstraints(&floatTolerance); setReadWriteStatusForMode(initialMode); } @@ -229,6 +234,17 @@ App::DocumentObjectExecReturn* Helix::execute() TopoDS_Shape face = sketchshape; face.Move(invObjLoc); + + Bnd_Box bounds; + BRepBndLib::Add(path, bounds); + double size=sqrt(bounds.SquareExtent()); + ShapeFix_ShapeTolerance fix; + fix.LimitTolerance(path, Precision::Confusion() * 1e-6 * size ); // needed to produce valid Pipe for very big parts + // We introduce final part tolerance with the second call to LimitTolerance below, however + // OCCT has a bug where the side-walls of the Pipe disappear with very large (km range) pieces + // increasing a tiny bit of extra tolerance to the path fixes this. This will in any case + // be less than the tolerance lower limit below, but sufficient to avoid the bug + BRepOffsetAPI_MakePipe mkPS(TopoDS::Wire(path), face, GeomFill_Trihedron::GeomFill_IsFrenet, Standard_False); result = mkPS.Shape(); @@ -237,6 +253,8 @@ App::DocumentObjectExecReturn* Helix::execute() if (SC.State() == TopAbs_IN) { result.Reverse(); } + + fix.LimitTolerance(result, Precision::Confusion() * size * Tolerance.getValue() ); // significant precision reduction due to helical approximation - needed to allow fusion to succeed AddSubShape.setValue(result); diff --git a/src/Mod/PartDesign/App/FeatureHelix.h b/src/Mod/PartDesign/App/FeatureHelix.h index 3b01150b05..32e41c41fa 100644 --- a/src/Mod/PartDesign/App/FeatureHelix.h +++ b/src/Mod/PartDesign/App/FeatureHelix.h @@ -56,6 +56,7 @@ public: App::PropertyEnumeration Mode; App::PropertyBool Outside; App::PropertyBool HasBeenEdited; + App::PropertyFloatConstraint Tolerance; /** if this property is set to a valid link, both Axis and Base properties * are calculated according to the linked line @@ -95,6 +96,7 @@ protected: static const App::PropertyFloatConstraint::Constraints floatTurns; static const App::PropertyAngle::Constraints floatAngle; + static const App::PropertyFloatConstraint::Constraints floatTolerance; private: static const char* ModeEnums[]; diff --git a/src/Mod/PartDesign/PartDesignTests/TestHelix.py b/src/Mod/PartDesign/PartDesignTests/TestHelix.py index fb5c917928..9ae8b7cd74 100644 --- a/src/Mod/PartDesign/PartDesignTests/TestHelix.py +++ b/src/Mod/PartDesign/PartDesignTests/TestHelix.py @@ -129,6 +129,147 @@ class TestHelix(unittest.TestCase): expected = pi * 25 * 5 * 3 self.assertAlmostEqual(helix.Shape.Volume, expected, places=2) + def testGiantHelix(self): + """ Test giant helix """ + _OCC_VERSION=[ int(v) for v in Part.OCC_VERSION.split('.') ] + if _OCC_VERSION[0]>7 or (_OCC_VERSION[0]==7 and _OCC_VERSION[1]>3): + mine=-1 + maxe=10 + else: + mine=-1 + maxe=9 + for iexponent in range(mine,maxe): + exponent = float(iexponent) + body = self.Doc.addObject('PartDesign::Body','GearBody') + gearSketch = self.Doc.addObject('Sketcher::SketchObject', 'GearSketch') + body.addObject(gearSketch) + TestSketcherApp.CreateRectangleSketch(gearSketch, (10*(10**exponent), 0), (1*(10**exponent), 1*(10**exponent))) + xz_plane = body.Origin.OriginFeatures[4] + gearSketch.AttachmentSupport = xz_plane + gearSketch.MapMode = 'FlatFace' + self.Doc.recompute() + + helix = self.Doc.addObject("PartDesign::AdditiveHelix","AdditiveHelix") + body.addObject(helix) + helix.Profile = gearSketch + helix.ReferenceAxis = (gearSketch,"V_Axis") + helix.Placement = FreeCAD.Placement(FreeCAD.Vector(0,0,0), FreeCAD.Rotation(FreeCAD.Vector(0,0,1),0), FreeCAD.Vector(0,0,0)) + + helix.Pitch = 2*(10**exponent) + helix.Turns = 2 + helix.Height = helix.Turns* helix.Pitch + helix.Angle = 0 + helix.Mode = 0 + self.Doc.recompute() + + self.assertTrue(helix.Shape.isValid()) + bbox = helix.Shape.BoundBox + self.assertAlmostEqual(bbox.ZMin/((10**exponent)**3),0,places=4) + # Computed exact value + # with r = radius, l = length of square, t = turns + # pi * r**2 * l * t + expected = pi * ( ((11*(10**exponent))**2) - ((10*(10**exponent))**2) ) * 1*(10**exponent) * helix.Turns + self.assertAlmostEqual(helix.Shape.Volume/ ((10**exponent)**3),expected/ ((10**exponent)**3),places=2) + + def testGiantHelixAdditive(self): + """ Test giant helix added to Cylinder """ + _OCC_VERSION=[ int(v) for v in Part.OCC_VERSION.split('.') ] + if _OCC_VERSION[0]>7 or (_OCC_VERSION[0]==7 and _OCC_VERSION[1]>3): + mine=-1 + maxe=8 + else: + mine=-1 + maxe=6 + for iexponent in range(mine,maxe): + exponent = float(iexponent) + body = self.Doc.addObject('PartDesign::Body','GearBody') + gearSketch = self.Doc.addObject('Sketcher::SketchObject', 'GearSketch') + body.addObject(gearSketch) + TestSketcherApp.CreateRectangleSketch(gearSketch, (10*(10**exponent), 0), (1*(10**exponent), 1*(10**exponent))) + xz_plane = body.Origin.OriginFeatures[4] + gearSketch.AttachmentSupport = xz_plane + gearSketch.MapMode = 'FlatFace' + self.Doc.recompute() + + cylinder = self.Doc.addObject('PartDesign::AdditiveCylinder','Cylinder') + cylinder.Radius = 10*(10**exponent) + cylinder.Height = 8*(10**exponent) + cylinder.Angle = 360 + body.addObject(cylinder) + self.Doc.recompute() + + helix = self.Doc.addObject("PartDesign::AdditiveHelix","AdditiveHelix") + body.addObject(helix) + helix.Profile = gearSketch + helix.ReferenceAxis = (gearSketch,"V_Axis") + helix.Placement = FreeCAD.Placement(FreeCAD.Vector(0,0,0), FreeCAD.Rotation(FreeCAD.Vector(0,0,1),0), FreeCAD.Vector(0,0,0)) + + helix.Pitch = 2*(10**exponent) + helix.Turns = 2.5 # workaround for OCCT bug with very large helices - full turns truncate the cylinder due to seam alignment + helix.Height = helix.Turns* helix.Pitch + helix.Angle = 0 + helix.Mode = 0 + self.Doc.recompute() + + self.assertTrue(helix.Shape.isValid()) + bbox = helix.Shape.BoundBox + self.assertAlmostEqual(bbox.ZMin/((10**exponent)**3),0,places=4) + # Computed exact value + # with r = radius, l = length of square, t = turns + # pi * r**2 * l * t + cyl = pi * ((10*(10**exponent))**2) * 8*(10**exponent) + expected = cyl + (pi * ( ((11*(10**exponent))**2) - ((10*(10**exponent))**2) ) * 1*(10**exponent) * helix.Turns ) + self.assertAlmostEqual(helix.Shape.Volume/ ((10**exponent)**3),expected/ ((10**exponent)**3),places=2) + + def testGiantHelixSubtractive(self): + """ Test giant helix subtracted from Cylinder """ + _OCC_VERSION=[ int(v) for v in Part.OCC_VERSION.split('.') ] + if _OCC_VERSION[0]>7 or (_OCC_VERSION[0]==7 and _OCC_VERSION[1]>3): + mine=-1 + maxe=8 + else: + mine=-1 + maxe=6 + for iexponent in range(mine,maxe): + exponent = float(iexponent) + body = self.Doc.addObject('PartDesign::Body','GearBody') + gearSketch = self.Doc.addObject('Sketcher::SketchObject', 'GearSketch') + body.addObject(gearSketch) + TestSketcherApp.CreateRectangleSketch(gearSketch, (10*(10**exponent), 0), (1*(10**exponent), 1*(10**exponent))) + xz_plane = body.Origin.OriginFeatures[4] + gearSketch.AttachmentSupport = xz_plane + gearSketch.MapMode = 'FlatFace' + self.Doc.recompute() + + cylinder = self.Doc.addObject('PartDesign::AdditiveCylinder','Cylinder') + cylinder.Radius = 11*(10**exponent) + cylinder.Height = 8*(10**exponent) + cylinder.Angle = 360 + body.addObject(cylinder) + self.Doc.recompute() + + helix = self.Doc.addObject("PartDesign::SubtractiveHelix","SubtractiveHelix") + body.addObject(helix) + helix.Profile = gearSketch + helix.ReferenceAxis = (gearSketch,"V_Axis") + helix.Placement = FreeCAD.Placement(FreeCAD.Vector(0,0,0), FreeCAD.Rotation(FreeCAD.Vector(0,0,1),0), FreeCAD.Vector(0,0,0)) + + helix.Pitch = 2*(10**exponent) + helix.Turns = 2.5 # workaround for OCCT bug with very large helices - full turns truncate the cylinder due to seam alignment + helix.Height = helix.Turns* helix.Pitch + helix.Angle = 0 + helix.Mode = 0 + self.Doc.recompute() + + self.assertTrue(helix.Shape.isValid()) + bbox = helix.Shape.BoundBox + self.assertAlmostEqual(bbox.ZMin/((10**exponent)**3),0,places=4) + # Computed exact value + # with r = radius, l = length of square, t = turns + # pi * r**2 * l * t + cyl = pi * ((11*(10**exponent))**2) * 8*(10**exponent) + expected = cyl - (pi * ( ((11*(10**exponent))**2) - ((10*(10**exponent))**2) ) * 1*(10**exponent) * helix.Turns ) + self.assertAlmostEqual(helix.Shape.Volume/ ((10**exponent)**3),expected/ ((10**exponent)**3),places=2) def testCone(self): """ Test helix following a cone """