From 5639728e8ae72896be55ce89aa7dc4ac855ccdc0 Mon Sep 17 00:00:00 2001 From: PaddleStroke Date: Mon, 7 Apr 2025 17:34:13 +0200 Subject: [PATCH] PartDesign: Fix use of App::Planes to create sketches (#20453) --- src/App/Datums.cpp | 35 ++++----- src/App/Datums.h | 35 ++++----- src/App/Document.cpp | 15 +++- src/App/Document.h | 1 + src/Gui/Selection/Selection.cpp | 95 +++++++++++++++++++++++ src/Gui/Selection/Selection.h | 20 +++++ src/Gui/Selection/SelectionFilter.cpp | 13 ++-- src/Gui/Selection/SelectionFilter.h | 5 +- src/Mod/PartDesign/Gui/SketchWorkflow.cpp | 33 +++++--- 9 files changed, 190 insertions(+), 62 deletions(-) diff --git a/src/App/Datums.cpp b/src/App/Datums.cpp index 0d315ed859..b0279766d9 100644 --- a/src/App/Datums.cpp +++ b/src/App/Datums.cpp @@ -39,7 +39,7 @@ PROPERTY_SOURCE(App::DatumElement, App::GeoFeature) PROPERTY_SOURCE(App::Plane, App::DatumElement) PROPERTY_SOURCE(App::Line, App::DatumElement) PROPERTY_SOURCE(App::Point, App::DatumElement) -PROPERTY_SOURCE(App::LocalCoordinateSystem, App::GeoFeature) +PROPERTY_SOURCE_WITH_EXTENSIONS(App::LocalCoordinateSystem, App::GeoFeature) DatumElement::DatumElement(bool hideRole) : baseDir{0.0, 0.0, 1.0} @@ -125,7 +125,6 @@ Point::Point() // ---------------------------------------------------------------------------- LocalCoordinateSystem::LocalCoordinateSystem() - : extension(this) { ADD_PROPERTY_TYPE(OriginFeatures, (nullptr), @@ -133,8 +132,11 @@ LocalCoordinateSystem::LocalCoordinateSystem() App::Prop_Hidden, "Axis and baseplanes controlled by the LCS"); + Group.setStatus(Property::Transient, true); + setStatus(App::NoAutoExpand, true); - extension.initExtension(this); + + GroupExtension::initExtension(this); } @@ -201,12 +203,6 @@ App::Point* LocalCoordinateSystem::getPoint(const char* role) const throw Base::RuntimeError(err.str().c_str()); } -bool LocalCoordinateSystem::hasObject(const DocumentObject* obj) const -{ - const auto& features = OriginFeatures.getValues(); - return std::ranges::find(features, obj) != features.end(); -} - short LocalCoordinateSystem::mustExecute() const { if (OriginFeatures.isTouched()) { @@ -340,18 +336,7 @@ void LocalCoordinateSystem::migrateOriginPoint() // ---------------------------------------------------------------------------- -LocalCoordinateSystem::LCSExtension::LCSExtension(LocalCoordinateSystem* obj) - : obj(obj) -{ - Group.setStatus(Property::Transient, true); -} - -void LocalCoordinateSystem::LCSExtension::initExtension(ExtensionContainer* obj) -{ - App::GroupExtension::initExtension(obj); -} - -bool LocalCoordinateSystem::LCSExtension::extensionGetSubObject(DocumentObject*& ret, +bool LocalCoordinateSystem::extensionGetSubObject(DocumentObject*& ret, const char* subname, PyObject** pyobj, Base::Matrix4D* mat, @@ -380,7 +365,7 @@ bool LocalCoordinateSystem::LCSExtension::extensionGetSubObject(DocumentObject*& } try { - ret = obj->getDatumElement(name.c_str()); + ret = getDatumElement(name.c_str()); if (!ret) { return false; } @@ -399,3 +384,9 @@ bool LocalCoordinateSystem::LCSExtension::extensionGetSubObject(DocumentObject*& return false; } } + +bool LocalCoordinateSystem::hasObject(const DocumentObject* obj, [[maybe_unused]] bool recursive) const +{ + const auto& features = OriginFeatures.getValues(); + return std::ranges::find(features, obj) != features.end(); +} diff --git a/src/App/Datums.h b/src/App/Datums.h index 9426f4afc5..e59b854728 100644 --- a/src/App/Datums.h +++ b/src/App/Datums.h @@ -102,10 +102,9 @@ public: } }; - -class AppExport LocalCoordinateSystem: public App::GeoFeature +class AppExport LocalCoordinateSystem: public App::GeoFeature, public App::GeoFeatureGroupExtension { - PROPERTY_HEADER_WITH_OVERRIDE(App::LocalCoordinateSystem); + PROPERTY_HEADER_WITH_EXTENSIONS(App::LocalCoordinateSystem); Q_DECLARE_TR_FUNCTIONS(App::LocalCoordinateSystem) public: @@ -195,9 +194,6 @@ public: App::Point* getPoint(const char* role) const; ///@} - /// Returns true if the given object is part of the origin - bool hasObject(const DocumentObject* obj) const; - /// Returns true on changing DatumElement set short mustExecute() const override; @@ -216,6 +212,17 @@ public: // Axis links PropertyLinkList OriginFeatures; + // GeoFeatureGroupExtension overrides: + bool extensionGetSubObject(DocumentObject*& ret, + const char* subname, + PyObject**, + Base::Matrix4D*, + bool, + int) const override; + + // Reimplement the hasObject because LCS doesn't use Group but stores objects in OriginFeatures for whatever reason. + bool hasObject(const DocumentObject* obj, bool recursive = false) const override; + protected: /// Checks integrity of the LCS App::DocumentObjectExecReturn* execute() override; @@ -229,22 +236,6 @@ private: struct SetupData; void setupDatumElement(App::PropertyLink& featProp, const SetupData& data); - class LCSExtension: public GeoFeatureGroupExtension - { - LocalCoordinateSystem* obj; - - public: - explicit LCSExtension(LocalCoordinateSystem* obj); - void initExtension(ExtensionContainer* obj) override; - bool extensionGetSubObject(DocumentObject*& ret, - const char* subname, - PyObject**, - Base::Matrix4D*, - bool, - int) const override; - }; - LCSExtension extension; - struct SetupData { Base::Type type; diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 1f51d9d925..ae3da65da5 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -4154,7 +4154,6 @@ const std::vector& Document::getObjects() const return d->objectArray; } - std::vector Document::getObjectsOfType(const Base::Type& typeId) const { std::vector Objects; @@ -4166,6 +4165,20 @@ std::vector Document::getObjectsOfType(const Base::Type& typeId return Objects; } +std::vector Document::getObjectsOfType(const std::vector& types) const +{ + std::vector Objects; + for (auto it : d->objectArray) { + for (auto& typeId : types) { + if (it->isDerivedFrom(typeId)) { + Objects.push_back(it); + break; // Prevent adding several times the same object. + } + } + } + return Objects; +} + std::vector Document::getObjectsWithExtension(const Base::Type& typeId, bool derived) const { diff --git a/src/App/Document.h b/src/App/Document.h index dce5b1a553..a7f9574df0 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -357,6 +357,7 @@ public: /// Returns a list of all Objects const std::vector& getObjects() const; std::vector getObjectsOfType(const Base::Type& typeId) const; + std::vector getObjectsOfType(const std::vector& types) const; /// Returns all object with given extensions. If derived=true also all objects with extensions /// derived from the given one std::vector getObjectsWithExtension(const Base::Type& typeId, diff --git a/src/Gui/Selection/Selection.cpp b/src/Gui/Selection/Selection.cpp index 9aa562fcf2..112edc9e3b 100644 --- a/src/Gui/Selection/Selection.cpp +++ b/src/Gui/Selection/Selection.cpp @@ -311,6 +311,101 @@ std::vector SelectionSingleton::getPickedList(const return temp; } +std::vector SelectionSingleton::getSelectionIn(App::DocumentObject* container, + Base::Type typeId, bool single) const +{ + if (!container) { + return getSelectionEx(nullptr, typeId, ResolveMode::NoResolve, single); + } + + std::vector sels = getSelectionEx(nullptr, App::DocumentObject::getClassTypeId(), ResolveMode::NoResolve, single); + + std::vector ret; + std::map SortMap; + + for (auto& sel : sels) { + auto* rootObj = sel.getObject(); + App::Document* doc = rootObj->getDocument(); + std::vector subs = sel.getSubNames(); + bool objPassed = false; + + for (size_t i = 0; i < subs.size(); ++i) { + auto& sub = subs[i]; + App::DocumentObject* newRootObj = nullptr; + std::string newSub = ""; + + std::vector names = Base::Tools::splitSubName(sub); + + if (container == rootObj) { + objPassed = true; + } + + if (rootObj->isLink()) { + // Update doc in case its an external link. + doc = rootObj->getLinkedObject()->getDocument(); + } + + for (auto& name : names) { + App::DocumentObject* obj = doc->getObject(name.c_str()); + if (!obj) { // We reached the element name (for example 'edge1') + newSub += name; + break; + } + + if (objPassed) { + if (!newRootObj) { + // We are the first object after the container is passed. + newRootObj = obj; + } + else { + newSub += name + "."; + } + } + + if (obj == container) { + objPassed = true; + } + if (obj->isLink()) { + // Update doc in case its an external link. + doc = obj->getLinkedObject()->getDocument(); + } + } + + if (newRootObj) { + // Make sure selected object is of correct type + auto* lastObj = newRootObj->resolve(newSub.c_str()); + if (!lastObj || !lastObj->isDerivedFrom(typeId)) { + continue; + } + + auto it = SortMap.find(newRootObj); + if (it != SortMap.end()) { + // only add sub-element + if (newSub != "") { + ret[it->second].SubNames.emplace_back(newSub); + ret[it->second].SelPoses.emplace_back(sel.SelPoses[i]); + } + } + else { + if (single && !ret.empty()) { + ret.clear(); + break; + } + // create a new entry + ret.emplace_back(newRootObj); + if (newSub != "") { + ret.back().SubNames.emplace_back(newSub); + ret.back().SelPoses.emplace_back(sel.SelPoses[i]); + } + SortMap.insert(std::make_pair(newRootObj, ret.size() - 1)); + } + } + } + } + + return ret; +} + std::vector SelectionSingleton::getSelectionEx(const char* pDocName, Base::Type typeId, ResolveMode resolve, bool single) const { diff --git a/src/Gui/Selection/Selection.h b/src/Gui/Selection/Selection.h index f5e594c249..f207474f26 100644 --- a/src/Gui/Selection/Selection.h +++ b/src/Gui/Selection/Selection.h @@ -445,6 +445,26 @@ public: */ std::vector getSelectionEx(const char* pDocName=nullptr, Base::Type typeId=App::DocumentObject::getClassTypeId(), ResolveMode resolve = ResolveMode::OldStyleElement, bool single=false) const; + /** Returns a vector of selection objects children of an object + * + * @param obj: Object within which you want to find a selection. + * The selection objects returned will be direct children of Obj. The rest of the subname will be unresolved. + * So this is equivalent to ResolveMode::NoResolve, but starting from obj. + * For instance if you have : Assembly.Part.Body.LCS.PlaneXY + * - If obj = Assembly : SelectionObject = Part, subname = "Body.LCS.PlaneXY." + * - If obj = Part : SelectionObject = Body, subname = "LCS.PlaneXY." + * - If obj = Body : SelectionObject = LCS, subname = "PlaneXY." + * The docname used is the document of obj. + * if obj is nullptr, this acts as getSelectionEx with empty docName + * + * @param typeId: specify the type of object to be returned. + * @param single: if set to true, then it will return an empty vector if + * there is more than one selection. + * + * @return The returned vector reflects the sequence of selection. + */ + std::vector getSelectionIn(App::DocumentObject* obj=nullptr, + Base::Type typeId=App::DocumentObject::getClassTypeId(), bool single=false) const; /** * @brief getAsPropertyLinkSubList fills PropertyLinkSubList with current selection. diff --git a/src/Gui/Selection/SelectionFilter.cpp b/src/Gui/Selection/SelectionFilter.cpp index b189ba02cd..75fab0f327 100644 --- a/src/Gui/Selection/SelectionFilter.cpp +++ b/src/Gui/Selection/SelectionFilter.cpp @@ -131,14 +131,16 @@ bool SelectionFilterGatePython::allow(App::Document*, App::DocumentObject* obj, // ---------------------------------------------------------------------------- -SelectionFilter::SelectionFilter(const char* filter) - : Ast(nullptr) +SelectionFilter::SelectionFilter(const char* filter, App::DocumentObject* container) + : Ast(nullptr) + , container(container) { setFilter(filter); } -SelectionFilter::SelectionFilter(const std::string& filter) - : Ast(nullptr) +SelectionFilter::SelectionFilter(const std::string& filter, App::DocumentObject* container) + : Ast(nullptr) + , container(container) { setFilter(filter.c_str()); } @@ -172,8 +174,7 @@ bool SelectionFilter::match() min = it->Slice->Min; max = it->Slice->Max; } - - std::vector temp = Gui::Selection().getSelectionEx(nullptr, it->ObjectType); + std::vector temp = container ? Gui::Selection().getSelectionIn(container, it->ObjectType) : Gui::Selection().getSelectionEx(nullptr, it->ObjectType); // test if subnames present if (it->SubName.empty()) { diff --git a/src/Gui/Selection/SelectionFilter.h b/src/Gui/Selection/SelectionFilter.h index 13981dabbc..c4faf33357 100644 --- a/src/Gui/Selection/SelectionFilter.h +++ b/src/Gui/Selection/SelectionFilter.h @@ -53,8 +53,8 @@ class GuiExport SelectionFilter public: /** Constructs a SelectionFilter object. */ - explicit SelectionFilter(const char* filter); - explicit SelectionFilter(const std::string& filter); + explicit SelectionFilter(const char* filter, App::DocumentObject* container = nullptr); + explicit SelectionFilter(const std::string& filter, App::DocumentObject* container = nullptr); virtual ~SelectionFilter(); /// Set a new filter string @@ -88,6 +88,7 @@ protected: std::string Filter; std::string Errors; bool parse(); + App::DocumentObject* container; std::shared_ptr Ast; }; diff --git a/src/Mod/PartDesign/Gui/SketchWorkflow.cpp b/src/Mod/PartDesign/Gui/SketchWorkflow.cpp index 945bdc5126..7d68c46b76 100644 --- a/src/Mod/PartDesign/Gui/SketchWorkflow.cpp +++ b/src/Mod/PartDesign/Gui/SketchWorkflow.cpp @@ -181,7 +181,7 @@ public: std::string getSupport() const { - return Gui::Command::getObjectCmd(getObject(), "(",",'')"); + return faceSelection.getAsPropertyLinkSubString(); } App::DocumentObject* getObject() const @@ -387,11 +387,17 @@ public: void findDatumPlanes() { App::GeoFeatureGroupExtension *geoGroup = getGroupExtensionOfBody(); - auto datumPlanes( appdocument->getObjectsOfType(PartDesign::Plane::getClassTypeId()) ); + const std::vector types = { PartDesign::Plane::getClassTypeId(), App::Plane::getClassTypeId() }; + auto datumPlanes = appdocument->getObjectsOfType(types); + for (auto plane : datumPlanes) { + if (std::find(planes.begin(), planes.end(), plane) != planes.end()) { + continue; // Skip if already in planes (for base planes) + } + planes.push_back ( plane ); // Check whether this plane belongs to the active body - if ( activeBody->hasObject(plane) ) { + if ( activeBody->hasObject(plane, true) ) { if ( !activeBody->isAfterInsertPoint ( plane ) ) { validPlaneCount++; status.push_back(PartDesignGui::TaskFeaturePick::validFeature); @@ -675,11 +681,20 @@ private: { // may happen when the user switched to an empty document while the // dialog is open - if (features.empty()) + if (features.empty()) { return; - App::Plane* plane = static_cast(features.front()); + } std::string FeatName = documentOfBody->getUniqueObjectName("Sketch"); - std::string supportString = Gui::Command::getObjectCmd(plane,"(",",[''])"); + auto* plane = static_cast(features.front()); + auto* lcs = plane->getLCS(); + + std::string supportString; + if (lcs) { + supportString = Gui::Command::getObjectCmd(lcs, "(") + ",['" + plane->getNameInDocument() + "'])"; + } + else { + supportString = Gui::Command::getObjectCmd(plane, "(", ",[''])"); + } App::Document* doc = partDesignBody->getDocument(); if (!doc->hasPendingTransaction()) { @@ -797,9 +812,9 @@ std::tuple SketchWorkflow::getFaceAn // a new sketch. // See https://forum.freecad.org/viewtopic.php?f=3&t=44070 - Gui::SelectionFilter FaceFilter ("SELECT Part::Feature SUBELEMENT Face COUNT 1"); - Gui::SelectionFilter PlaneFilter ("SELECT App::Plane COUNT 1"); - Gui::SelectionFilter PlaneFilter2("SELECT PartDesign::Plane COUNT 1"); + Gui::SelectionFilter FaceFilter ("SELECT Part::Feature SUBELEMENT Face COUNT 1", activeBody); + Gui::SelectionFilter PlaneFilter ("SELECT App::Plane COUNT 1", activeBody); + Gui::SelectionFilter PlaneFilter2("SELECT PartDesign::Plane COUNT 1", activeBody); if (PlaneFilter2.match()) { PlaneFilter = PlaneFilter2;