From 52ed6eb8487fb6d68ff0af6c2d3170755ae2a1c3 Mon Sep 17 00:00:00 2001 From: bgbsww <120601209+bgbsww@users.noreply.github.com> Date: Wed, 15 May 2024 19:43:30 -0400 Subject: [PATCH] Toponaming: Bring in Chamfer, Fillet code and add tests (#14035) * Toponaming: bring in missing code fragments in Sketcher * Toponaming: Fix infinite recursion, remove debug cruft, rough in fillet test * Bring in missing code; fix chamfers * Toponaming: Add code for fillets and test --- src/Mod/Part/App/AttachExtension.cpp | 17 - src/Mod/Part/App/PartFeature.cpp | 482 +++++++++--------- src/Mod/PartDesign/App/FeatureExtrude.cpp | 18 +- src/Mod/PartDesign/App/FeatureExtrude.h | 2 +- src/Mod/PartDesign/App/FeatureFillet.cpp | 62 +++ src/Mod/PartDesign/App/FeatureLoft.cpp | 8 +- src/Mod/PartDesign/App/FeaturePad.cpp | 2 +- src/Mod/PartDesign/App/FeaturePocket.cpp | 13 +- src/Mod/PartDesign/App/FeaturePocket.h | 4 +- src/Mod/PartDesign/App/FeatureRevolution.cpp | 3 +- src/Mod/PartDesign/App/FeatureSketchBased.cpp | 83 ++- src/Mod/PartDesign/App/FeatureSketchBased.h | 2 +- .../TestTopologicalNamingProblem.py | 110 +++- 13 files changed, 523 insertions(+), 283 deletions(-) diff --git a/src/Mod/Part/App/AttachExtension.cpp b/src/Mod/Part/App/AttachExtension.cpp index 910c8e592b..53ddf40fa6 100644 --- a/src/Mod/Part/App/AttachExtension.cpp +++ b/src/Mod/Part/App/AttachExtension.cpp @@ -342,21 +342,6 @@ App::DocumentObjectExecReturn* AttachExtension::extensionExecute() } return App::DocumentObjectExtension::extensionExecute(); } -void dumpAttacher(std::string name, App::PropertyLinkSubList& a) -{ - std::cerr << name << ": "; - for (auto& sh : a.getShadowSubs()) { - std::cerr << "(" << sh.first << " : " << sh.second << ")"; - } - for (auto& v : a.getValues()) { - std::cerr << v->getNameInDocument(); - } - for (auto& sv : a.getSubValues()) { - std::cerr << "[" << sv << "]"; - } - std::cerr << std::endl; -} - void AttachExtension::extensionOnChanged(const App::Property* prop) { @@ -375,7 +360,6 @@ void AttachExtension::extensionOnChanged(const App::Property* prop) App::Property::User3, &Support); Support.Paste(AttachmentSupport); - dumpAttacher("PasteSupport", AttachmentSupport); } _active = -1; updateAttacherVals(/*base*/ false); @@ -472,7 +456,6 @@ void AttachExtension::updateAttacherVals(bool base) const if (!props.attachment) { return; } - dumpAttacher("updateAttacherVals", *props.attachment); attacher(base).setUp(*props.attachment, eMapMode(props.mapMode->getValue()), props.mapReversed->getValue(), diff --git a/src/Mod/Part/App/PartFeature.cpp b/src/Mod/Part/App/PartFeature.cpp index aae5563c10..ab22149a38 100644 --- a/src/Mod/Part/App/PartFeature.cpp +++ b/src/Mod/Part/App/PartFeature.cpp @@ -125,12 +125,245 @@ PyObject *Feature::getPyObject() return Py::new_reference_to(PythonObject); } +/** + * Override getElementName to support the Export type. Other calls are passed to the original + * method + * @param name The name to search for, or if non existent, name of current Feature is returned + * @param type An element type name. + * @return The element name located, of + */ +std::pair Feature::getElementName(const char* name, + ElementNameType type) const +{ + if (type != ElementNameType::Export) { + return App::GeoFeature::getElementName(name, type); + } + + // This function is overridden to provide higher level shape topo names that + // are generated on demand, e.g. Wire, Shell, Solid, etc. + + auto prop = Base::freecad_dynamic_cast(getPropertyOfGeometry()); + if (!prop) { + return App::GeoFeature::getElementName(name, type); + } + + TopoShape shape = prop->getShape(); + Data::MappedElement mapped = shape.getElementName(name); + auto res = shape.shapeTypeAndIndex(mapped.index); + static const int MinLowerTopoNames = 3; + static const int MaxLowerTopoNames = 10; + if (res.second && !mapped.name) { + // Here means valid index name, but no mapped name, check to see if + // we shall generate the high level topo name. + // + // The general idea of the algorithm is to find the minimum number of + // lower elements that can identify the given higher element, and + // combine their names to generate the name for the higher element. + // + // In theory, all it takes to find one lower element that only appear + // in the given higher element. To make the algorithm more robust + // against model changes, we shall take minimum MinLowerTopoNames lower + // elements. + // + // On the other hand, it may be possible to take too many elements for + // disambiguation. We shall limit to maximum MaxLowerTopoNames. If the + // chosen elements are not enough to disambiguate the higher element, + // we'll include an index for disambiguation. + + auto subshape = shape.getSubTopoShape(res.first, res.second, true); + TopAbs_ShapeEnum lower; + Data::IndexedName idxName; + if (!subshape.isNull()) { + switch (res.first) { + case TopAbs_WIRE: + lower = TopAbs_EDGE; + idxName = Data::IndexedName::fromConst("Edge", 1); + break; + case TopAbs_SHELL: + case TopAbs_SOLID: + case TopAbs_COMPOUND: + case TopAbs_COMPSOLID: + lower = TopAbs_FACE; + idxName = Data::IndexedName::fromConst("Face", 1); + break; + default: + lower = TopAbs_SHAPE; + } + if (lower != TopAbs_SHAPE) { + typedef std::pair> NameEntry; + std::vector indices; + std::vector names; + std::vector ancestors; + int count = 0; + for (auto& ss : subshape.getSubTopoShapes(lower)) { + auto name = ss.getMappedName(idxName); + if (!name) { + continue; + } + indices.emplace_back(name.size(), + shape.findAncestors(ss.getShape(), res.first)); + names.push_back(name); + if (indices.back().second.size() == 1 && ++count >= MinLowerTopoNames) { + break; + } + } + + if (names.size() >= MaxLowerTopoNames) { + std::stable_sort(indices.begin(), + indices.end(), + [](const NameEntry& a, const NameEntry& b) { + return a.second.size() < b.second.size(); + }); + std::vector sorted; + auto pos = 0; + sorted.reserve(names.size()); + for (auto& v : indices) { + size_t size = ancestors.size(); + if (size == 0) { + ancestors = v.second; + } + else if (size > 1) { + for (auto it = ancestors.begin(); it != ancestors.end();) { + if (std::find(v.second.begin(), v.second.end(), *it) + == v.second.end()) { + it = ancestors.erase(it); + if (ancestors.size() == 1) { + break; + } + } + else { + ++it; + } + } + } + auto itPos = sorted.end(); + if (size == 1 || size != ancestors.size()) { + itPos = sorted.begin() + pos; + ++pos; + } + sorted.insert(itPos, names[v.first]); + if (size == 1 && sorted.size() >= MinLowerTopoNames) { + break; + } + } + } + + names.resize(std::min((int)names.size(), MaxLowerTopoNames)); + if (names.size()) { + std::string op; + if (ancestors.size() > 1) { + // The current chosen elements are not enough to + // identify the higher element, generate an index for + // disambiguation. + auto it = std::find(ancestors.begin(), ancestors.end(), res.second); + if (it == ancestors.end()) { + assert(0 && "ancestor not found"); // this shouldn't happened + } + else { + op = Data::POSTFIX_TAG + std::to_string(it - ancestors.begin()); + } + } + + // Note: setting names to shape will change its underlying + // shared element name table. This actually violates the + // const'ness of this function. + // + // To be const correct, we should have made the element + // name table to be implicit sharing (i.e. copy on change). + // + // Not sure if there is any side effect of indirectly + // change the element map inside the Shape property without + // recording the change in undo stack. + // + mapped.name = shape.setElementComboName(mapped.index, + names, + mapped.index.getType(), + op.c_str()); + } + } + } + return App::GeoFeature::_getElementName(name, mapped); + } + + if (!res.second && mapped.name) { + const char* dot = strchr(name, '.'); + if (dot) { + ++dot; + // Here means valid mapped name, but cannot find the corresponding + // indexed name. This usually means the model has been changed. The + // original indexed name is usually appended to the mapped name + // separated by a dot. We use it as a clue to decode the combo name + // set above, and try to single out one sub shape that has all the + // lower elements encoded in the combo name. But since we don't + // always use all the lower elements for encoding, this can only be + // consider a heuristics. + if (Data::hasMissingElement(dot)) { + dot += strlen(Data::MISSING_PREFIX); + } + std::pair occindex = shape.shapeTypeAndIndex(dot); + if (occindex.second > 0) { + auto idxName = Data::IndexedName::fromConst(shape.shapeName(occindex.first).c_str(), + occindex.second); + std::string postfix; + auto names = + shape.decodeElementComboName(idxName, mapped.name, idxName.getType(), &postfix); + std::vector ancestors; + for (auto& name : names) { + auto index = shape.getIndexedName(name); + if (!index) { + ancestors.clear(); + break; + } + auto oidx = shape.shapeTypeAndIndex(index); + auto subshape = shape.getSubShape(oidx.first, oidx.second); + if (subshape.IsNull()) { + ancestors.clear(); + break; + } + auto current = shape.findAncestors(subshape, occindex.first); + if (ancestors.empty()) { + ancestors = std::move(current); + } + else { + for (auto it = ancestors.begin(); it != ancestors.end();) { + if (std::find(current.begin(), current.end(), *it) == current.end()) { + it = ancestors.erase(it); + } + else { + ++it; + } + } + if (ancestors.empty()) { // model changed beyond recognition, bail! + break; + } + } + } + if (ancestors.size() > 1 && boost::starts_with(postfix, Data::POSTFIX_INDEX)) { + std::istringstream iss(postfix.c_str() + strlen(Data::POSTFIX_INDEX)); + int idx; + if (iss >> idx && idx >= 0 && idx < (int)ancestors.size()) { + ancestors.resize(1, ancestors[idx]); + } + } + if (ancestors.size() == 1) { + idxName.setIndex(ancestors.front()); + mapped.index = idxName; + return App::GeoFeature::_getElementName(name, mapped); + } + } + } + } + return App::GeoFeature::_getElementName(name, mapped); +} + App::DocumentObject* Feature::getSubObject(const char* subname, PyObject** pyObj, Base::Matrix4D* pmat, bool transform, int depth) const { + while(subname && *subname=='.') ++subname; // skip leading . + // having '.' inside subname means it is referencing some children object, // instead of any sub-element from ourself if (subname && !Data::isMappedElement(subname) && strchr(subname, '.')) { @@ -423,7 +656,6 @@ QVector Feature::getElementFromSource(App::DocumentObject* if (name == element.name) { std::pair objElement; std::size_t len = sub.size(); -// checkingSubname.toString(sub); checkingSubname.appendToStringBuffer(sub); GeoFeature::resolveElement(obj, sub.c_str(), objElement); sub.resize(len); @@ -467,7 +699,8 @@ QVector Feature::getElementFromSource(App::DocumentObject* // a compound operation), then take a shortcut and assume the element index // remains the same. But we still need to trace the shape history to // confirm. - if (element.name && shape.countSubShapes(type) == srcShape.countSubShapes(type)) { + if (type != TopAbs_SHAPE && element.name + && shape.countSubShapes(type) == srcShape.countSubShapes(type)) { tagChanges = 0; checkingSubname = element.index; auto mapped = shape.getMappedName(element.index); @@ -494,7 +727,7 @@ QVector Feature::getElementFromSource(App::DocumentObject* return res; } - if (!element.name) { + if (!element.name || type == TopAbs_SHAPE) { return res; } @@ -1188,11 +1421,15 @@ void Feature::onChanged(const App::Property* prop) if (prop == &this->Placement) { #ifdef FC_USE_TNP_FIX TopoShape shape = this->Shape.getShape(); - shape.setTransform(this->Placement.getValue().toMatrix()); + auto oldTransform = shape.getTransform(); + auto newTransform = this->Placement.getValue().toMatrix(); + shape.setTransform(newTransform); Base::ObjectStatusLocker guard( App::Property::NoRecompute, &this->Shape); - this->Shape.setValue(shape); + if ( oldTransform != newTransform) { + this->Shape.setValue(shape); + } #else this->Shape.setTransform(this->Placement.getValue().toMatrix()); @@ -1201,7 +1438,11 @@ void Feature::onChanged(const App::Property* prop) // if the point data has changed check and adjust the transformation as well else if (prop == &this->Shape) { if (this->isRecomputing()) { +#ifdef FC_USE_TNP_FIX + this->Shape._Shape.setTransform(this->Placement.getValue().toMatrix()); +#else this->Shape.setTransform(this->Placement.getValue().toMatrix()); +#endif } else { Base::Placement p; @@ -1643,234 +1884,3 @@ bool Part::checkIntersection(const TopoDS_Shape& first, const TopoDS_Shape& seco } } - -/** - * Override getElementName to support the Export type. Other calls are passed to the original - * method - * @param name The name to search for, or if non existent, name of current Feature is returned - * @param type An element type name. - * @return The element name located, of - */ -std::pair Feature::getElementName(const char* name, - ElementNameType type) const -{ - if (type != ElementNameType::Export) { - return App::GeoFeature::getElementName(name, type); - } - - // This function is overridden to provide higher level shape topo names that - // are generated on demand, e.g. Wire, Shell, Solid, etc. - - auto prop = Base::freecad_dynamic_cast(getPropertyOfGeometry()); - if (!prop) { - return App::GeoFeature::getElementName(name, type); - } - - TopoShape shape = prop->getShape(); - Data::MappedElement mapped = shape.getElementName(name); - auto res = shape.shapeTypeAndIndex(mapped.index); - static const int MinLowerTopoNames = 3; - static const int MaxLowerTopoNames = 10; - if (res.second && !mapped.name) { - // Here means valid index name, but no mapped name, check to see if - // we shall generate the high level topo name. - // - // The general idea of the algorithm is to find the minimum number of - // lower elements that can identify the given higher element, and - // combine their names to generate the name for the higher element. - // - // In theory, all it takes to find one lower element that only appear - // in the given higher element. To make the algorithm more robust - // against model changes, we shall take minimum MinLowerTopoNames lower - // elements. - // - // On the other hand, it may be possible to take too many elements for - // disambiguation. We shall limit to maximum MaxLowerTopoNames. If the - // chosen elements are not enough to disambiguate the higher element, - // we'll include an index for disambiguation. - - auto subshape = shape.getSubTopoShape(res.first, res.second, true); - TopAbs_ShapeEnum lower; - Data::IndexedName idxName; - if (!subshape.isNull()) { - switch (res.first) { - case TopAbs_WIRE: - lower = TopAbs_EDGE; - idxName = Data::IndexedName::fromConst("Edge", 1); - break; - case TopAbs_SHELL: - case TopAbs_SOLID: - case TopAbs_COMPOUND: - case TopAbs_COMPSOLID: - lower = TopAbs_FACE; - idxName = Data::IndexedName::fromConst("Face", 1); - break; - default: - lower = TopAbs_SHAPE; - } - if (lower != TopAbs_SHAPE) { - typedef std::pair> NameEntry; - std::vector indices; - std::vector names; - std::vector ancestors; - int count = 0; - for (auto& ss : subshape.getSubTopoShapes(lower)) { - auto name = ss.getMappedName(idxName); - if (!name) { - continue; - } - indices.emplace_back(name.size(), - shape.findAncestors(ss.getShape(), res.first)); - names.push_back(name); - if (indices.back().second.size() == 1 && ++count >= MinLowerTopoNames) { - break; - } - } - - if (names.size() >= MaxLowerTopoNames) { - std::stable_sort(indices.begin(), - indices.end(), - [](const NameEntry& a, const NameEntry& b) { - return a.second.size() < b.second.size(); - }); - std::vector sorted; - auto pos = 0; - sorted.reserve(names.size()); - for (auto& v : indices) { - size_t size = ancestors.size(); - if (size == 0) { - ancestors = v.second; - } - else if (size > 1) { - for (auto it = ancestors.begin(); it != ancestors.end();) { - if (std::find(v.second.begin(), v.second.end(), *it) - == v.second.end()) { - it = ancestors.erase(it); - if (ancestors.size() == 1) { - break; - } - } - else { - ++it; - } - } - } - auto itPos = sorted.end(); - if (size == 1 || size != ancestors.size()) { - itPos = sorted.begin() + pos; - ++pos; - } - sorted.insert(itPos, names[v.first]); - if (size == 1 && sorted.size() >= MinLowerTopoNames) { - break; - } - } - } - - names.resize(std::min((int)names.size(), MaxLowerTopoNames)); - if (names.size()) { - std::string op; - if (ancestors.size() > 1) { - // The current chosen elements are not enough to - // identify the higher element, generate an index for - // disambiguation. - auto it = std::find(ancestors.begin(), ancestors.end(), res.second); - if (it == ancestors.end()) { - assert(0 && "ancestor not found"); // this shouldn't happened - } - else { - op = Data::POSTFIX_TAG + std::to_string(it - ancestors.begin()); - } - } - - // Note: setting names to shape will change its underlying - // shared element name table. This actually violates the - // const'ness of this function. - // - // To be const correct, we should have made the element - // name table to be implicit sharing (i.e. copy on change). - // - // Not sure if there is any side effect of indirectly - // change the element map inside the Shape property without - // recording the change in undo stack. - // - mapped.name = shape.setElementComboName(mapped.index, - names, - mapped.index.getType(), - op.c_str()); - } - } - } - return App::GeoFeature::_getElementName(name, mapped); - } - - if (!res.second && mapped.name) { - const char* dot = strchr(name, '.'); - if (dot) { - ++dot; - // Here means valid mapped name, but cannot find the corresponding - // indexed name. This usually means the model has been changed. The - // original indexed name is usually appended to the mapped name - // separated by a dot. We use it as a clue to decode the combo name - // set above, and try to single out one sub shape that has all the - // lower elements encoded in the combo name. But since we don't - // always use all the lower elements for encoding, this can only be - // consider a heuristics. - if (Data::hasMissingElement(dot)) { - dot += strlen(Data::MISSING_PREFIX); - } - std::pair occindex = shape.shapeTypeAndIndex(dot); - if (occindex.second > 0) { - auto idxName = Data::IndexedName::fromConst(shape.shapeName(occindex.first).c_str(), - occindex.second); - std::string postfix; - auto names = - shape.decodeElementComboName(idxName, mapped.name, idxName.getType(), &postfix); - std::vector ancestors; - for (auto& name : names) { - auto index = shape.getIndexedName(name); - if (!index) { - ancestors.clear(); - break; - } - auto oidx = shape.shapeTypeAndIndex(index); - auto subshape = shape.getSubShape(oidx.first, oidx.second); - if (subshape.IsNull()) { - ancestors.clear(); - break; - } - auto current = shape.findAncestors(subshape, occindex.first); - if (ancestors.empty()) { - ancestors = std::move(current); - } - else { - for (auto it = ancestors.begin(); it != ancestors.end();) { - if (std::find(current.begin(), current.end(), *it) == current.end()) { - it = ancestors.erase(it); - } - else { - ++it; - } - } - if (ancestors.empty()) { // model changed beyond recognition, bail! - break; - } - } - } - if (ancestors.size() > 1 && boost::starts_with(postfix, Data::POSTFIX_INDEX)) { - std::istringstream iss(postfix.c_str() + strlen(Data::POSTFIX_INDEX)); - int idx; - if (iss >> idx && idx >= 0 && idx < (int)ancestors.size()) { - ancestors.resize(1, ancestors[idx]); - } - } - if (ancestors.size() == 1) { - idxName.setIndex(ancestors.front()); - mapped.index = idxName; - return App::GeoFeature::_getElementName(name, mapped); - } - } - } - } - return App::GeoFeature::_getElementName(name, mapped); -} diff --git a/src/Mod/PartDesign/App/FeatureExtrude.cpp b/src/Mod/PartDesign/App/FeatureExtrude.cpp index 0f51b74881..02f73792a1 100644 --- a/src/Mod/PartDesign/App/FeatureExtrude.cpp +++ b/src/Mod/PartDesign/App/FeatureExtrude.cpp @@ -73,8 +73,9 @@ short FeatureExtrude::mustExecute() const return ProfileBased::mustExecute(); } -Base::Vector3d FeatureExtrude::computeDirection(const Base::Vector3d& sketchVector) +Base::Vector3d FeatureExtrude::computeDirection(const Base::Vector3d& sketchVector, bool inverse) { + (void) inverse; Base::Vector3d extrudeDirection; if (!UseCustomVector.getValue()) { @@ -91,12 +92,12 @@ Base::Vector3d FeatureExtrude::computeDirection(const Base::Vector3d& sketchVect Base::Vector3d dir; getAxis(pcReferenceAxis, subReferenceAxis, base, dir, ForbiddenAxis::NotPerpendicularWithNormal); switch (addSubType) { - case Type::Additive: - extrudeDirection = dir; - break; - case Type::Subtractive: - extrudeDirection = -dir; - break; + case Type::Additive: + extrudeDirection = dir; + break; + case Type::Subtractive: + extrudeDirection = -dir; + break; } } } @@ -431,6 +432,7 @@ App::DocumentObjectExecReturn* FeatureExtrude::buildExtrusion(ExtrudeOptions opt bool makeface = options.testFlag(ExtrudeOption::MakeFace); bool fuse = options.testFlag(ExtrudeOption::MakeFuse); bool legacyPocket = options.testFlag(ExtrudeOption::LegacyPocket); + bool inverseDirection = options.testFlag(ExtrudeOption::InverseDirection); std::string method(Type.getValueAsString()); @@ -513,7 +515,7 @@ App::DocumentObjectExecReturn* FeatureExtrude::buildExtrusion(ExtrudeOptions opt base.move(invObjLoc); - Base::Vector3d paddingDirection = computeDirection(SketchVector); + Base::Vector3d paddingDirection = computeDirection(SketchVector, inverseDirection); // create vector in padding direction with length 1 gp_Dir dir(paddingDirection.x, paddingDirection.y, paddingDirection.z); diff --git a/src/Mod/PartDesign/App/FeatureExtrude.h b/src/Mod/PartDesign/App/FeatureExtrude.h index 22ed2741e6..088c7ce8ea 100644 --- a/src/Mod/PartDesign/App/FeatureExtrude.h +++ b/src/Mod/PartDesign/App/FeatureExtrude.h @@ -64,7 +64,7 @@ public: //@} protected: - Base::Vector3d computeDirection(const Base::Vector3d& sketchVector); + Base::Vector3d computeDirection(const Base::Vector3d& sketchVector, bool inverse); bool hasTaperedAngle() const; /// Options for buildExtrusion() diff --git a/src/Mod/PartDesign/App/FeatureFillet.cpp b/src/Mod/PartDesign/App/FeatureFillet.cpp index 4b91088023..e38f88b168 100644 --- a/src/Mod/PartDesign/App/FeatureFillet.cpp +++ b/src/Mod/PartDesign/App/FeatureFillet.cpp @@ -65,6 +65,67 @@ short Fillet::mustExecute() const App::DocumentObjectExecReturn *Fillet::execute() { +#ifdef FC_USE_TNP_FIX + Part::TopoShape baseShape; + try { + baseShape = getBaseTopoShape(); + } + catch (Base::Exception& e) { + return new App::DocumentObjectExecReturn(e.what()); + } + baseShape.setTransform(Base::Matrix4D()); + + auto edges = UseAllEdges.getValue() ? baseShape.getSubTopoShapes(TopAbs_EDGE) + : getContinuousEdges(baseShape); + if (edges.empty()) { + return new App::DocumentObjectExecReturn( + QT_TRANSLATE_NOOP("Exception", "Fillet not possible on selected shapes")); + } + + double radius = Radius.getValue(); + + if (radius <= 0) { + return new App::DocumentObjectExecReturn( + QT_TRANSLATE_NOOP("Exception", "Fillet radius must be greater than zero")); + } + + this->positionByBaseFeature(); + + try { + TopoShape shape(0); //,getDocument()->getStringHasher()); + shape.makeElementFillet(baseShape, edges, Radius.getValue(), Radius.getValue()); + if (shape.isNull()) { + return new App::DocumentObjectExecReturn( + QT_TRANSLATE_NOOP("Exception", "Resulting shape is null")); + } + + TopTools_ListOfShape aLarg; + aLarg.Append(baseShape.getShape()); + bool failed = false; + if (!BRepAlgo::IsValid(aLarg, shape.getShape(), Standard_False, Standard_False)) { + ShapeFix_ShapeTolerance aSFT; + aSFT.LimitTolerance(shape.getShape(), + Precision::Confusion(), + Precision::Confusion(), + TopAbs_SHAPE); + } + + if (!failed) { + shape = refineShapeIfActive(shape); + shape = getSolid(shape); + } + this->Shape.setValue(shape); + + if (failed) { + return new App::DocumentObjectExecReturn("Resulting shape is invalid"); + } + return App::DocumentObject::StdReturn; + } + catch (Standard_Failure& e) { + return new App::DocumentObjectExecReturn(e.GetMessageString()); + } + +#else Part::TopoShape TopShape; try { TopShape = getBaseTopoShape(); @@ -144,6 +205,7 @@ App::DocumentObjectExecReturn *Fillet::execute() catch (Standard_Failure& e) { return new App::DocumentObjectExecReturn(e.GetMessageString()); } +#endif } void Fillet::Restore(Base::XMLReader &reader) diff --git a/src/Mod/PartDesign/App/FeatureLoft.cpp b/src/Mod/PartDesign/App/FeatureLoft.cpp index b067af4f4f..46958c5b43 100644 --- a/src/Mod/PartDesign/App/FeatureLoft.cpp +++ b/src/Mod/PartDesign/App/FeatureLoft.cpp @@ -390,6 +390,8 @@ App::DocumentObjectExecReturn *Loft::execute(void) } catch (const Base::Exception&) { } + auto hasher = getDocument()->getStringHasher(); + try { //setup the location this->positionByPrevious(); @@ -413,7 +415,7 @@ App::DocumentObjectExecReturn *Loft::execute(void) wiresections[i++].push_back(s); } - TopoShape result(0); + TopoShape result(0,hasher); std::vector shapes; // if (SplitProfile.getValue()) { @@ -429,14 +431,14 @@ App::DocumentObjectExecReturn *Loft::execute(void) for (auto &wires : wiresections) { for(auto& wire : wires) wire.move(invObjLoc); - shells.push_back(TopoShape(0).makeElementLoft( + shells.push_back(TopoShape(0, hasher).makeElementLoft( wires, Part::IsSolid::notSolid, Ruled.getValue()? Part::IsRuled::ruled : Part::IsRuled::notRuled, Closed.getValue() ? Part::IsClosed::closed : Part::IsClosed::notClosed)); // } //build the top and bottom face, sew the shell and build the final solid TopoShape front; if (wiresections[0].front().shapeType() != TopAbs_VERTEX) { - front = getVerifiedFace(); + front = getTopoShapeVerifiedFace(); if (front.isNull()) return new App::DocumentObjectExecReturn( QT_TRANSLATE_NOOP("Exception", "Loft: Creating a face from sketch failed")); diff --git a/src/Mod/PartDesign/App/FeaturePad.cpp b/src/Mod/PartDesign/App/FeaturePad.cpp index cab6b2f754..cbff42e895 100644 --- a/src/Mod/PartDesign/App/FeaturePad.cpp +++ b/src/Mod/PartDesign/App/FeaturePad.cpp @@ -116,7 +116,7 @@ App::DocumentObjectExecReturn *Pad::execute() base.Move(invObjLoc); - Base::Vector3d paddingDirection = computeDirection(SketchVector); + Base::Vector3d paddingDirection = computeDirection(SketchVector, false); // create vector in padding direction with length 1 gp_Dir dir(paddingDirection.x, paddingDirection.y, paddingDirection.z); diff --git a/src/Mod/PartDesign/App/FeaturePocket.cpp b/src/Mod/PartDesign/App/FeaturePocket.cpp index b772216189..aaae33f138 100644 --- a/src/Mod/PartDesign/App/FeaturePocket.cpp +++ b/src/Mod/PartDesign/App/FeaturePocket.cpp @@ -119,7 +119,7 @@ App::DocumentObjectExecReturn *Pocket::execute() base.move(invObjLoc); - Base::Vector3d pocketDirection = computeDirection(SketchVector); + Base::Vector3d pocketDirection = computeDirection(SketchVector, false); // create vector in pocketing direction with length 1 gp_Dir dir(pocketDirection.x, pocketDirection.y, pocketDirection.z); @@ -258,3 +258,14 @@ App::DocumentObjectExecReturn *Pocket::execute() } #endif } + +Base::Vector3d Pocket::getProfileNormal() const +{ + auto res = FeatureExtrude::getProfileNormal(); + // turn around for pockets +#ifdef FC_USE_TNP_FIX + return res * -1; +#else + return res; +#endif +} diff --git a/src/Mod/PartDesign/App/FeaturePocket.h b/src/Mod/PartDesign/App/FeaturePocket.h index 8d5f1e6102..c1bf5f3a22 100644 --- a/src/Mod/PartDesign/App/FeaturePocket.h +++ b/src/Mod/PartDesign/App/FeaturePocket.h @@ -53,7 +53,9 @@ public: const char* getViewProviderName() const override { return "PartDesignGui::ViewProviderPocket"; } - //@} + + Base::Vector3d getProfileNormal() const override; + private: static const char* TypeEnums[]; }; diff --git a/src/Mod/PartDesign/App/FeatureRevolution.cpp b/src/Mod/PartDesign/App/FeatureRevolution.cpp index 9f884a752d..9c9903b441 100644 --- a/src/Mod/PartDesign/App/FeatureRevolution.cpp +++ b/src/Mod/PartDesign/App/FeatureRevolution.cpp @@ -161,7 +161,7 @@ App::DocumentObjectExecReturn* Revolution::execute() // Create a fresh support even when base exists so that it can be used for patterns #ifdef FC_USE_TNP_FIX - TopoShape result; + TopoShape result(0); #else TopoDS_Shape result; #endif @@ -375,6 +375,7 @@ void Revolution::generateRevolution(TopoDS_Shape& revol, #ifdef FC_USE_TNP_FIX revol = TopoShape(from).makeElementRevolve(revolAx,angleTotal); + revol.Tag = -getID(); #else // revolve the face to a solid // BRepPrimAPI is the only option that allows use of this shape for patterns. diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.cpp b/src/Mod/PartDesign/App/FeatureSketchBased.cpp index 197ddfe9f4..08cebf0dcf 100644 --- a/src/Mod/PartDesign/App/FeatureSketchBased.cpp +++ b/src/Mod/PartDesign/App/FeatureSketchBased.cpp @@ -60,6 +60,7 @@ #include "FeatureSketchBased.h" #include "DatumLine.h" #include "DatumPlane.h" +#include "Mod/Part/App/Geometry.h" FC_LOG_LEVEL_INIT("PartDesign",true,true); @@ -1472,6 +1473,86 @@ Base::Vector3d ProfileBased::getProfileNormal() const { Base::Placement SketchPos = obj->Placement.getValue(); Base::Rotation SketchOrientation = SketchPos.getRotation(); SketchOrientation.multVec(SketchVector, SketchVector); +#ifdef FC_USE_TNP_FIX + return SketchVector; + } + + // For newer version, do not do fitting, as it may flip the face normal for + // some reason. + TopoShape shape = getVerifiedFace(true); //, _ProfileBasedVersion.getValue() <= 0); + + gp_Pln pln; + if (shape.findPlane(pln)) { + gp_Dir dir = pln.Axis().Direction(); + return Base::Vector3d(dir.X(), dir.Y(), dir.Z()); + } + + if (shape.hasSubShape(TopAbs_EDGE)) { + // Find the first planar face that contains the edge, and return the plane normal + TopoShape objShape = Part::Feature::getTopoShape(obj); + for (int idx : objShape.findAncestors(shape.getSubShape(TopAbs_EDGE, 1), TopAbs_FACE)) { + if (objShape.getSubTopoShape(TopAbs_FACE, idx).findPlane(pln)) { + gp_Dir dir = pln.Axis().Direction(); + return Base::Vector3d(dir.X(), dir.Y(), dir.Z()); + } + } + } + + // If no planar face, try to use the normal of the center of the first face. + if (shape.hasSubShape(TopAbs_FACE)) { + TopoDS_Face face = TopoDS::Face(shape.getSubShape(TopAbs_FACE, 1)); + BRepAdaptor_Surface adapt(face); + double u = + adapt.FirstUParameter() + (adapt.LastUParameter() - adapt.FirstUParameter()) / 2.; + double v = + adapt.FirstVParameter() + (adapt.LastVParameter() - adapt.FirstVParameter()) / 2.; + BRepLProp_SLProps prop(adapt, u, v, 2, Precision::Confusion()); + if (prop.IsNormalDefined()) { + gp_Pnt pnt; + gp_Vec vec; + // handles the orientation state of the shape + BRepGProp_Face(face).Normal(u, v, pnt, vec); + return Base::Vector3d(vec.X(), vec.Y(), vec.Z()); + } + } + + if (!shape.hasSubShape(TopAbs_EDGE)) { + return SketchVector; + } + + // If the shape is a line, then return an arbitrary direction that is perpendicular to the line + auto geom = Part::Geometry::fromShape(shape.getSubShape(TopAbs_EDGE, 1), true); + auto geomLine = Base::freecad_dynamic_cast(geom.get()); + if (geomLine) { + Base::Vector3d dir = geomLine->getDir(); + double x = std::fabs(dir.x); + double y = std::fabs(dir.y); + double z = std::fabs(dir.z); + if (x > y && x > z && x > 1e-7) { + if (y + z < 1e-7) { + return Base::Vector3d(0, 0, 1); + } + dir.x = -(dir.z + dir.y) / dir.x; + } + else if (y > x && y > z && y > 1e-7) { + if (x + z < 1e-7) { + return Base::Vector3d(0, 0, 1); + } + dir.y = -(dir.z + dir.x) / dir.y; + } + else if (z > 1e-7) { + if (x + y < 1e-7) { + return Base::Vector3d(1, 0, 0); + } + dir.z = -(dir.x + dir.y) / dir.z; + } + else { + return SketchVector; + } + return dir.Normalize(); + } + +#else } else { TopoDS_Shape shape = getVerifiedFace(true); @@ -1494,7 +1575,7 @@ Base::Vector3d ProfileBased::getProfileNormal() const { } } } - +#endif return SketchVector; } diff --git a/src/Mod/PartDesign/App/FeatureSketchBased.h b/src/Mod/PartDesign/App/FeatureSketchBased.h index 4db8246c6c..0c1ceab95a 100644 --- a/src/Mod/PartDesign/App/FeatureSketchBased.h +++ b/src/Mod/PartDesign/App/FeatureSketchBased.h @@ -127,7 +127,7 @@ public: // TODO: Toponaming April 2024 Deprecated in favor of TopoShape method. Remove when possible. TopoShape getTopoShapeSupportFace() const; - Base::Vector3d getProfileNormal() const; + virtual Base::Vector3d getProfileNormal() const; TopoShape getProfileShape() const; diff --git a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py index 8ea9260656..ceaa68163e 100644 --- a/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py +++ b/src/Mod/PartDesign/PartDesignTests/TestTopologicalNamingProblem.py @@ -944,7 +944,7 @@ class TestTopologicalNamingProblem(unittest.TestCase): 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.assertAlmostEqual(self.Body.Shape.BoundBox.YMax,25.2) self.assertEqual(self.Body.Shape.BoundBox.ZMax,20) self.assertNotEquals(area1, area2) @@ -1091,9 +1091,9 @@ class TestTopologicalNamingProblem(unittest.TestCase): 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) + self.assertEqual(self.Body.Shape.BoundBox.XMax, 35) + self.assertEqual(self.Body.Shape.BoundBox.YMax, 25) + self.assertEqual(self.Body.Shape.BoundBox.ZMax, 10) def testSubShapeBinder(self): doc = self.Doc @@ -1233,9 +1233,9 @@ class TestTopologicalNamingProblem(unittest.TestCase): 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) + self.assertEqual(self.Body.Shape.BoundBox.XMax, 35) + self.assertEqual(self.Body.Shape.BoundBox.YMax, 25) + self.assertEqual(self.Body.Shape.BoundBox.ZMax, 10) def testPartDesignTNPChamfer(self): """ Test Chamfer """ @@ -1258,9 +1258,6 @@ class TestTopologicalNamingProblem(unittest.TestCase): doc.Body.newObject('Sketcher::SketchObject', 'Sketch') doc.Sketch.AttachmentSupport = (chamfer, "Face8") - # doc.Sketch.AttachmentOffset = App.Placement( - # App.Vector(0.0000000000, 2.0000000000, 0.0000000000), - # App.Rotation(0.0000000000, 0.0000000000, 0.0000000000)) doc.Sketch.MapMode = 'FlatFace' doc.recompute() @@ -1296,11 +1293,9 @@ class TestTopologicalNamingProblem(unittest.TestCase): pocket = self.Doc.addObject('PartDesign::Pocket', 'Pocket') pocket.Type = "Length" - # pocket.Length2 = 2 pocket.Length = 3 pocket.Direction = App.Vector(-0.710000000,0.7100000000, 0.0000000000) pocket.Profile = doc.Sketch - pocket.Reversed = True body.addObject(pocket) self.Doc.recompute() volume3 = body.Shape.Volume @@ -1333,6 +1328,97 @@ class TestTopologicalNamingProblem(unittest.TestCase): self.assertAlmostEqual(volume3, boxVolume - 3 * chamferVolume - cutVolume, 4) self.assertAlmostEqual(volume4, boxVolume - 2 * chamferVolume - cutVolume, 4) + def testPartDesignTNPFillet(self): + """ Test Fillet """ + # Arrange + doc = self.Doc + body = self.Doc.addObject('PartDesign::Body', 'Body') + box = self.Doc.addObject('PartDesign::AdditiveBox', 'Box') + body.addObject(box) + self.Doc.recompute() + volume1 = body.Shape.Volume + fillet = self.Doc.addObject('PartDesign::Fillet', 'Fillet') + fillet.Base = (box, ['Edge1', + 'Edge5', + 'Edge7', + ]) + # fillet.Size = 1 + body.addObject(fillet) + self.Doc.recompute() + volume2 = body.Shape.Volume + + doc.Body.newObject('Sketcher::SketchObject', 'Sketch') + doc.Sketch.AttachmentSupport = (fillet, "Face2") + doc.Sketch.MapMode = 'FlatFace' + doc.recompute() + + x1, x2, y1, y2 = 4,6 , 6, 11 + geoList = [] + geoList.append( + Part.LineSegment(App.Vector(x1, y1, 0.0 ), + App.Vector(x1, y2, 0.0 ))) + geoList.append( + Part.LineSegment(App.Vector(x1, y2, 0.0), + App.Vector(x2, y2, 0.0))) + geoList.append( + Part.LineSegment(App.Vector(x2, y2, 0.0), + App.Vector(x2, y1, 0.0))) + geoList.append( + Part.LineSegment(App.Vector(x2, y1, 0.0), + App.Vector(x1, y1, 0.0))) + doc.Sketch.addGeometry(geoList, False) + del geoList + + constraintList = [] + constraintList.append(Sketcher.Constraint('Coincident', 0, 2, 1, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 1, 2, 2, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 2, 2, 3, 1)) + constraintList.append(Sketcher.Constraint('Coincident', 3, 2, 0, 1)) + constraintList.append(Sketcher.Constraint('Horizontal', 0)) + constraintList.append(Sketcher.Constraint('Horizontal', 2)) + constraintList.append(Sketcher.Constraint('Vertical', 1)) + constraintList.append(Sketcher.Constraint('Vertical', 3)) + doc.Sketch.addConstraint(constraintList) + del constraintList + body.addObject(doc.Sketch) + + pocket = self.Doc.addObject('PartDesign::Pocket', 'Pocket') + pocket.Type = "Length" + pocket.Length = 3 + pocket.Direction = App.Vector(-0.710000000,0.7100000000, 0.0000000000) + pocket.Profile = doc.Sketch + # pocket.Reversed = False + body.addObject(pocket) + self.Doc.recompute() + volume3 = body.Shape.Volume + # Change the filleted edges, potentially triggering TNP + fillet.Base = (box, ['Edge5', + 'Edge7', + ]) + self.Doc.recompute() + volume4 = body.Shape.Volume + # Assert + if body.Shape.ElementMapVersion == "": # Skip without element maps. + return + reverseMap = body.Shape.childShapes()[0].ElementReverseMap + faces = [name for name in reverseMap.keys() if name.startswith("Face")] + edges = [name for name in reverseMap.keys() if name.startswith("Edge")] + vertexes = [name for name in reverseMap.keys() if name.startswith("Vertex")] + self.assertEqual(len(body.Shape.childShapes()), 1) + self.assertEqual(body.Shape.childShapes()[0].ElementMapSize, 62) + self.assertEqual(len(reverseMap),62) + self.assertEqual(len(faces),12) + self.assertEqual(len(edges),30) + self.assertEqual(len(vertexes),20) + boxVolume = 10 * 10 * 10 + # Full prism minus the rounded triangle prism. + filletVolume = 1 * 1 * 10 - 1 * 1 * math.pi / 4 * 10 #0.5 * 10 + cutVolume = 24 + self.assertAlmostEqual(volume1, boxVolume ) + self.assertAlmostEqual(volume2, boxVolume - 3 * filletVolume) + self.assertAlmostEqual(volume3, boxVolume - 3 * filletVolume - cutVolume, 4) + self.assertAlmostEqual(volume4, boxVolume - 2 * filletVolume - cutVolume, 4) + def create_t_sketch(self): self.Doc.getObject('Body').newObject('Sketcher::SketchObject', 'Sketch') geo_list = [