Gui: Add Part variant logic #19735

pieterhijma · 2025-02-20T13:49:27Z

This PR is an outcome of FPA grant proposal Research Variant Parts.

This PR mainly adds GUI support and improves the Part::Variant introduced in #19733.

This builds on (and currently also includes) #19733.

This PR is not meant to be merged as is but it contains a prototype with a potential option to have Variant Parts in FreeCAD. I discuss the implementation in detail in this forum post and beyond.

How it can be used is explained in this forum post.

This commit does not do anything but just adds logic to intercept getters and setters to properties. This is used to compute shapes from document objects. In particular for document objects, it creates a stack of document objects that is used to write and read properties to and from when executing a document object that is the target of a variant.

This commit makes use of the logic from the previous commit to intercept access to properties by context document objects. This is used to compute shapes from document objects. Since there is no logic to actually set a context, all calls to acquire a context will fail, so effectively all is a no-op currently.

This adds Part::Variant based on an App::VariantExtension that makes use of DocumentObject contexts to compute shapes from document objects.

In the Property View, exposed properties are colored purple and there is a menu action to expose or unexpose properties.

- add a command and button for variants - adopt support icon for a variant with an overlay - hide context document objects and add them to the group

benj5378 · 2025-02-20T14:29:50Z

src/App/DocumentObject.cpp

+            for (const auto& id : exp->getIdentifiers()) {
+                Property* prop = id.first.getProperty();
+                if (prop) {
+                    DocumentObject* obj = dynamic_cast<DocumentObject*>(prop->getContainer());


dynamic_cast is slow. I don't believe we can use that here for a function that might be called very often. Consider using isDerivedFrom

Maybe Base::freecad_dynamic_cast would be fine there? It essentially is doing the isDerivedFrom call and static cast so it should be pretty cheap.

benj5378 · 2025-02-20T14:30:35Z

src/App/DocumentObject.cpp

+    std::vector<Property*> propsObj;
+    obj->getPropertyList(propsObj);
+    for (auto prop : propsObj) {
+        auto link = dynamic_cast<PropertyLinkBase*>(prop);


dynamic_cast :(

benj5378 · 2025-02-20T14:32:32Z

src/App/DocumentObject.cpp

+bool DocumentObject::isExposed(const Property* prop) const
+{
+    return prop->testStatus(Property::Exposed);
+}


Why would one call DocumentObject::isExposed(prop) and not just prop->testStatus(Exposed) directly?

benj5378 · 2025-02-20T14:35:31Z

src/App/Expression.cpp

+                bool include = true;
+                if (excludeExposed) {
+                    auto propNames = dep.second;
+                    include = !std::all_of(propNames.begin(), propNames.end(),


Will this cause performance issues? Hmmm

benj5378 · 2025-02-20T14:37:29Z

src/App/Property.h

+            throw NoContextException();
+        }
+
+        auto derivedProp = dynamic_cast<const DerivedType*>(prop);


dynamic_cast :(

benj5378 · 2025-02-20T14:37:50Z

src/App/Property.h

+        static_assert(std::is_member_function_pointer_v<FuncType>,
+                      "Func must be a member function pointer.");
+
+        auto* derivedProp = dynamic_cast<DerivedType*>(getContextProperty(CreatePropOption::Create));


dynamic_cast :(

benj5378 · 2025-02-20T14:38:06Z

src/App/Property.h

+    {
+        Property* prop = this->getContextProperty(option);
+        if (prop) {
+            return dynamic_cast<PropertyListsT<T, ListT, ParentT>*>(prop);


dynamic_cast :(

benj5378 · 2025-02-20T14:41:21Z

src/App/VariantExtension.cpp

+    set_difference(currentAdoptedExposedProps,
+                   adoptedExposedProps,
+                   std::inserter(propsToRemove, propsToRemove.end()));
+    for_each(propsToRemove, [this](const App::Property* prop) {


use normal for(const auto& x : xs)?

benj5378 · 2025-02-20T14:43:02Z

src/Gui/CommandStructure.cpp

@@ -176,6 +177,56 @@ bool StdCmdVarSet::isActive()
    return hasActiveDocument();
 }

+
+//===========================================================================
+// Std_VarSet


Suggested change

// Std_VarSet

// Std_Variant

benj5378 · 2025-02-20T14:44:25Z

src/Gui/propertyeditor/PropertyEditor.cpp

@@ -24,6 +24,7 @@
 #include "PreCompiled.h"

 #ifndef _PreComp_
+#include <boost/algorithm/cxx11/all_of.hpp>


marcuspollio · 2025-02-20T15:22:49Z

@pieterhijma Great !
Adding the video you made on the PR description could help for user testing =D
Thanks for your work !

pieterhijma · 2025-02-20T15:36:14Z

@pieterhijma Great ! Adding the video you made on the PR description could help for user testing =D Thanks for your work !

Yes, sorry, cyclic dependencies (the forum refers to this and this should refer to the forum).... The description now points to the forum in which I describe the implementation. I've also added a link to the video there (for quick reference).

pieterhijma · 2025-02-20T15:38:36Z

@benj5378, Thanks for the review. As this is a prototype and a draft, I haven't looked very much into your comments but I think you have a point in all your cases. I think for now, it is much more helpful to look at the bigger picture, such as the design and the implication of adding this to FreeCAD. See the forum topic that I link to in the discussion. Would you be able to comment on that as well? Would be appreciated!

pieterhijma added 8 commits February 15, 2025 13:28

Core: Add an "Exposed" flag to properties

71a7d18

Core: Add functions for "exposed" dependencies

8f84f8c

Core: Enable dependency checking for "Exposed"

f26b5a8

Core: Add Variant logic to compute shapes

1da91f1

This adds Part::Variant based on an App::VariantExtension that makes use of DocumentObject contexts to compute shapes from document objects.

Gui: Support exposed properties in PropertyView

2e1ba9f

In the Property View, exposed properties are colored purple and there is a menu action to expose or unexpose properties.

Core: Improve GUI support for variants

4d97a0f

- add a command and button for variants - adopt support icon for a variant with an overlay - hide context document objects and add them to the group

github-actions bot added Mod: Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD Mod: Part Related to the Part Workbench Mod: Sketcher Related to the Sketcher Workbench labels Feb 20, 2025

benj5378 reviewed Feb 20, 2025

View reviewed changes

pieterhijma mentioned this pull request Feb 20, 2025

Core: Allow exposing properties #18412

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gui: Add Part variant logic #19735

Gui: Add Part variant logic #19735

pieterhijma commented Feb 20, 2025 •

edited

Loading

benj5378 Feb 20, 2025

kadet1090 Feb 20, 2025

benj5378 Feb 20, 2025

benj5378 Feb 20, 2025

benj5378 Feb 20, 2025

benj5378 Feb 20, 2025

benj5378 Feb 20, 2025

benj5378 Feb 20, 2025

benj5378 Feb 20, 2025

benj5378 Feb 20, 2025

benj5378 Feb 20, 2025

marcuspollio commented Feb 20, 2025

pieterhijma commented Feb 20, 2025

pieterhijma commented Feb 20, 2025

Gui: Add Part variant logic #19735

Are you sure you want to change the base?

Gui: Add Part variant logic #19735

Conversation

pieterhijma commented Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcuspollio commented Feb 20, 2025

pieterhijma commented Feb 20, 2025

pieterhijma commented Feb 20, 2025

pieterhijma commented Feb 20, 2025 •

edited

Loading