Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Managing Custom Data Elements: VarSet #12135

Merged
merged 3 commits into from Feb 21, 2024

Conversation

pieterhijma
Copy link
Contributor

This PR proposes part of a solution to issue #12120

We will introduce a simple App::VarSet property container modeled after Dynamic Data, Path PropertyBags, and Assembly 4 Variables but implemented in C++ as a core feature.

The fact that it is implemented in C++ makes it accessible to C++ workbenches as well and allows us to add features that interact with other core logic.

The VarSet allows users to define properties that can be used in models just as with spreadsheet aliases and the other three property containers described above.

The hope is that the functionality that the VarSet provides will motivate users of FreeCAD to use it as their main way to define custom data.

Context and future plans

This PR will be followed by other PRs that add more functionality. Since this PR is for a new data structure in core and it currently may seem as a minor contribution, the following text describes the context and future plans:

Application Geometry Interface

The main workflow for VarSets is adding them to an App::Part object with the idea that the design of the part makes use of the properties in the VarSets. However, in this case where the VarSet has an App::Part as parent, the VarSet becomes special. In that case, the VarSet will automatically obtain a boolean property Exposed in group Base that is initially false. Users can simply continue to use the variables defined in the VarSet in their design. If they toggle Exposed to true, the parent App::Part will automatically obtain an XLink property with the same name as the VarSet that points to the exposed VarSet. This XLink property will be CopyOnChange allowing users to change the link to the exposed VarSet to an equivalent VarSet outside of the App::Part. Expressions used in App::Part that refer to the exposed VarSet will automatically be redirected to the equivalent VarSet.

This functionality allows Parts to have something similar to an API, let's call it an Application Geometry Interface. The designer of geometry expresses the parameters in terms of variables in VarSets. For example, suppose a designer creates a box with three parameters: Width, Height, and Depth. The designer creates an App::Part, with a VarSet Parameters and a PartDesign Body. The VarSet contains the three parameters with default values and the boolean Exposed. The PartDesign body has been designed in terms of these three parameters.

By setting Exposed to true, the designer of the box communicates to users of the box that the design can be varied by means of Width, Height, and Depth. The part will automatically obtain an XLink property with the same name as the VarSet, in this case Parameters.

Suppose there is a user of the box that wants to change the default dimensions of the box. The user can copy the VarSet within the part to a VarSet outside of the design, change the values of the three dimensions in this new VarSet, and change the XLink property Parameters of the Part to point to this newly created VarSet. The geometry will change accordingly.

This solution gives designers and users of designs a well-defined interface with which the geometry can be changed from outside of the Part. This provides FreeCAD and its design much improved modularity because parameters for geometry can be "injected" from outside into geometry via the interface that the designer defined for the geometry.

For example, consider an assembly of wooden parts created from the same sheet of wood resulting in subcomponents with the same thickness of wood. The assembly could have a VarSet that defines parameters that are shared among its subcomponents such as the thickness of the wood. Then for each piece, you could define a VarSet with that thickness of wood, ensuring that the user has only one place where the thickness of wood needs to be changed, applying a new value for the wooden thickness to all parts of the assembly.

In conclusion, having a core property container allows us to define an interface between a design and users of the design, typically an assembly.

Variant Links

With the above functionality, it is possible to create a link to the part, and change the Part property link to point to a different equivalent VarSet, essentially providing a convenient way to create variant links.

Suppose the box above is assembled from a simple rectangular piece of wood with a common thickness. The box has a long side, a short side, and a base. With a simple rectangular sketch and a pad we create one of those sides with a VarSet that exposes the length, height, and thickness.

We can then define VarSets for each of these three variants and change the parameters accordingly. We then use one copy of the part 'side', make two links to this part (or alternatively two subshapebinders), change the property Link Copy on Change to Enabled and have the Parts point to their respective VarSets.

Configuration management

By means of defining VarSets for a configuration, managing configurations becomes essentially changing a link from one configuration to another. In the example above, the assembly uses three configurations defined by VarSets. What is especially powerful is that the configurations can come from outside.

GUI / Workflow

Having a core property container allows us to optimize access to it in the GUI and add general workflow improvements. One possibility is to have a panel with all the variables sorted by equivalent VarSets (where equivalent VarSets means VarSets that have the same set of properties). This can help designers to refactor their design, for example deciding how variables should be grouped into VarSets and which VarSets to expose.

A second possibility would be to extend the panel to insert an expression with a field for a name of a property, similar to what the Sketcher workbench has. Besides this input field, there is a dropdown menu called "Scopes" where the user can indicate where (1) the values in an expression come from and (2) where the resulting expression should be written to.

As an example, suppose we have a VarSet called Parameters that we are defining and variables that we use come from VarSets Tolerances and Constants. The dropdown menu could look like this:

[w] Parameters
[r] Tolerances
[ ] constants

Let's say we are defining a Pad and want to enter the Length, we could enter an expression like this:

Result: 30 mm
Name Variable: MyNewName
Expression: 29 + GapBetweenSheets

This will read and autocomplete GapBetweenSheets from the Tolerances, and create a new property MyNewName in VarSet Parameters.

This workflow addition allows users to define variables as they are designing.

VarSet relations

A potential feature for VarSets is creating relations between them. For example, suppose we have a VarSet Parameters with let's say a length, width, and height parameter. We could define two more equivalent VarSets (having the same properties), one called Tolerances and one Variables. For Variables we define it to be in relation with Tolerances and Parameters via relation "summing". Because of this, each value in Variables should be the sum of the corresponding property in Parameters and Tolerances. In this case, we could make Parameters and Tolerances to be exposed. The values of Parameters are such that it defines the geometry and the values of Tolerances can be zero initially. The values of Variables will automatically be summed, so initially they will reflect the values of Parameters.

Since Parameters and Tolerances are exposed to the user, the user can modify the Parameters to end up with the requested design and if the design needs to be manufactured, the user can replace the Tolerances with a version specific to the project or machine. Suppose the project is CNC-milled, the user can try to find tolerances such that the project ends up having the requested dimensions.

Other potential relations between VarSets are maximum, minimum, for example to constrain parameters between maximum and minimum values. Another potential relation is enumeration, to define lists of values, for example [10, 20, 30] for a parameter.

@github-actions github-actions bot added the Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD label Jan 26, 2024
@adrianinsaval
Copy link
Member

Your plans for the future seem like their duplicating the functionality of configuration tables. What's the reason for this duplication? (What's the advantage over existing functionality?)

@kadet1090
Copy link
Contributor

I also don't quite understand, this introduces yet another way to implement the same thing. It should be wise probably to start with designing common API for this kind of features and move the spreadsheet, Path parameter bag etc to it so it would use the same underlaying solution with simply different UIs that could depend on what is needed. Otherwise we will end up with famous xkcd situation:

@adrianinsaval
Copy link
Member

This is a good move in terms of replacing dynamicdata, propery bags an variables container from asm4 at least. There's clearly a need for something simpler than spreadsheets

@joshualibrarian
Copy link

@pieterhijma Are you envisioning that the variables themselves would be implemented as custom Properties of the VarSet objects? Or first class sub-objects?

@pieterhijma
Copy link
Contributor Author

Your plans for the future seem like their duplicating the functionality of configuration tables. What's the reason for this duplication? (What's the advantage over existing functionality?

Thank you for taking a look.

It is true that it duplicates the functionality. There is no particular reason for it but it is a consequence of the overall functionality the future plans offer. The functionality of configuration tables simply emerges from having the described functionality. Note that both spreadsheets and Dynamic Data offer configuration tables, so there are already two implementations.

The advantage over the existing functionality is that in my humble opinion spreadsheet configuration tables are a bit brittle, requiring hiddenref and cell bindings, while in the above plans, configuration tables are more naturally incorporated in an improved "data management workflow". I think we should mainly focus on the concept to have an API for geometry, an improved workflow (in my humble opinion, the spreadsheet workflow is suboptimal) and the relations.

@pieterhijma
Copy link
Contributor Author

I also don't quite understand, this introduces yet another way to implement the same thing. It should be wise probably to start with designing common API for this kind of features and move the spreadsheet, Path parameter bag etc to it so it would use the same underlaying solution with simply different UIs that could depend on what is needed. Otherwise we will end up with famous xkcd situation:

Thank you for looking into it. I was wondering how soon anyone would bring the XKCD up. Within 12 hours! I had the pleasure of doing a couple of projects with and having lectures from Andy Tanenbaum, so I would answer with his quote 😄: "The good thing about standards is that there are so many to choose from".

All joking aside, I'm very aware of this. Of course there is a difference with at least the PropertyBag, Assembly 4 Variables, and Dynamic Data. It would become a core feature. So, one way to consider the proposal is the following: We've seen a need for a simple document object that maintains data because there are 3 implementations in FreeCAD addons. Let's simply acknowledge this and react to this need by making it a core feature. And while we're doing that, let's learn from all those implementations, let's try to capture the good things, and let's try to think of the possibilities and potential that it would offer being a core feature, such as the API for geometry.

I completely agree that it would be best to have it extensible to ensure that the three addons can adapt the functionality to their specific needs.

@pieterhijma
Copy link
Contributor Author

@pieterhijma Are you envisioning that the variables themselves would be implemented as custom Properties of the VarSet objects? Or first class sub-objects?

Thanks for your reaction. My idea was to simply have them as custom properties, similar to the three property containers in the addons.

@Zolko-123
Copy link
Contributor

Can this Ondsel trainwreck be stopped somehow ?

@joshualibrarian
Copy link

joshualibrarian commented Jan 27, 2024

I love the notion of FreeCAD objects having a sort of "API", by announcing what parameters they accept. However, I don't see what's gained by creating a new object type to do so, especially since we can already put custom properties on any DocumentObject. What if we added a boolean value to the Property class called something like isParameter? This way, ANY geometry (or even non-geometry objects) could have parameters, this in effect acting the same way as your Exposed, but being on a per-property basis.

These could then be exposed in enclosing Part much as you describe, and the parameters themselves can actually be on the relevant geometry objects in question, or on any object at all (such as an arbitrary Group), since they're just custom parameters. Then when creating a link, that link could automatically show the parameter properties (as their default values), which then when changed, turns that link into a variant link.

When we add new types of objects, we create more complication for the user and more things to learn about, I think it is a far better approach to add functionality to existing objects, rather than creating new ones, hence I still like better the idea of special handling for Part where the objects inside it can reference it and it's properties directly, giving a perfect place already to store variables relevant to it, but either way the idea of an isParameter flag on Property is a separate notion.

@pieterhijma
Copy link
Contributor Author

However, I don't see what's gained by creating a new object type to do so, especially since we can already put custom properties on any DocumentObject.

This is a good point, thanks for bringing it up.

One reason for this design is technical. Referring to a parent property within a design leads to a cyclic dependency. This can be prevented by using a hiddenref, but to use this always in the design is cumbersome.

Another reason for this design is in terms of workflow. If you have many properties to expose and you want to reuse multiple copies/links of the same geometry, you will have to set all those values one by one. Having a VarSet allows you to do this by creating a VarSet for the variant and simply linking one or more variants to the VarSet.

Another reason is related but not completely the same and is more organizational: You can have different VarSets with parameters / constants / tolerances. I noticed when making things, you often need various categories for the same set of variables. I described that in this forum post in section "week 5". This essentially where the idea originates from, recognizing that the pattern of some kind of property container had arisen already three times in different contexts. The idea to then define relations between these VarSets, makes it very powerful in my humble opinion.

This organizational aspect can also help make the geometry/model/design more clear. For example, with VarSets, the variants are expressed in terms of the VarSets and not in terms of the actual (links to parts). Suppose you make use of aluminum profiles and your projects makes use of 600, 300, and 200 mm profiles, then there are three VarSets for those variants. The (links to parts) can then link to either one of those and refactoring is really easy. Suppose the 600 has to become a 500, you only need to change that there.

Finally, a benefit is that it is easy to verify whether geometry is used in its default form (the link points to the internal VarSet with the same name) or in a variant form (the link points to an external VarSet).

@pieterhijma
Copy link
Contributor Author

What if we added a boolean value to the Property class called something like isParameter? This way, ANY geometry (or even non-geometry objects) could have parameters, this in effect acting the same way as your Exposed, but being on a per-property basis.

In a sense, we already have that and that would then be the CopyOnChange flag. A drawback of this approach is that you need to change variants per property (as discussed above). Additionally, it is more difficult to understand whether a value has changed for the designer. Perhaps not with links, but for copies of objects, it would be difficult to see. Finally, cyclic dependencies would be a problem.

@pieterhijma
Copy link
Contributor Author

These could then be exposed in enclosing Part much as you describe...

When we add new types of objects, we create more complication for the user... ---8<--- ...flag on Property is a separate notion.

I hope the above benefits are convincing. It is not a very complicated object and I would like to note that there are already three similar implementations of the same thing in the addons.

Regarding applying exposing things from different kinds of geometry. I considered that and that could easily be implemented as well, but I think it would be safe to start with App::Part as that is the most relevant use-case.

@yorikvanhavre
Copy link
Member

My 2 cents here:

  • We have document properties, and we should finish the work started by @AjinkyaDahale to have them fully working (basically, be able to use them in expressions). There is little question that's the most obvious, simple, direct way to have properties defined at document level. I think that's still the main direction we should put our efforts into. This is fundamentally a workaround.
  • Below document level, everything has custom properties already, and they are working, and usable in expressions
  • If something is missing there (exposed/hidden status, etc...) that should be fixed at property level
  • There is no problem IMHO to have SEVERAL ways to store data in a document, object, part or whatever. There are, already. So I don't see any real inherent problem in having one more and have this PR merged. If it's useful to some, and the code is properly self-contained and compartment-tight (which it is), then why not
  • That said, I agree that fundamentally such a feature is basically relying on a new workflow yet to be created, together with a UI, etc. And it provides fundamentally the same thing we already have. So IMHO it cannot pretend to become a new standard. But if we're OK with that, I have nothing against it.

@pieterhijma
Copy link
Contributor Author

My 2 cents here:

  • We have document properties, and we should finish the work started by @AjinkyaDahale to have them fully working (basically, be able to use them in expressions). There is little question that's the most obvious, simple, direct way to have properties defined at document level. I think that's still the main direction we should put our efforts into. This is fundamentally a workaround.

  • Below document level, everything has custom properties already, and they are working, and usable in expressions

  • If something is missing there (exposed/hidden status, etc...) that should be fixed at property level

Thank you for your reaction. It is a bit unclear to which "This" in "This is fundamentally a workaround" refers to. I assume for the discussion it is referring to this PR, because that helps me to make the argument below.

Issue #12120 discusses the problem of managing custom data. I consider the work by @AjinkyaDahale on document-wide variables, so fully integrated properties at the root level as one of the solutions to this problem. Although I think it is good to have the capability to refer to the properties in the document root from expressions, I don't think it is a good or full solution to #12120.

One reason is that referencing a document-wide property goes "against" the direction of the hierarchy. Because of this, it is prone to cause dependency cycles.

Another reason is that it is not good for modularity. If a document object refers to a document property, it is hidden as soon as the document object is linked to. This may be what the designer of the document object wants, but it may also be what you don't want. This is also related to #11742.

Thirdly, it seems to assume that the file (represented by the file root in the tree) is an important entity in FreeCAD. However, there are more important entities / abstractions. It could be made important if we would allow only one document object per file, but currently we don't and therefore it is better to store part-related information (for example) in the properties of the part itself.

Another argument for why document roots are not so useful is that it also impossible to create links to document roots. So, we cannot create a link to document 1 in document 2. You cán create a link to a document object within document 1 in document 2. In other words, document objects are a more important abstraction for FreeCAD than document roots and therefore, I would suggest to use document-wide properties sparingly. The only use I see for them is to capture file-related properties, such as the modification date, hash, and license properties. This is related to #12136.

So, in my humble opinion document-wide variables are good to have for (sparingly) using them for file-related things in CAD files (for example emboss the date of file modification on a part), but we should not consider them as a good solution for the problem defined in #12120. So, in that light, this PR is not a work-around for the fact that we don't have document-wide variables, but in my opinion a better solution than document-wide variables that accounts for the tree structure that FreeCAD currently has. In that light, I consider the document-wide properties orthogonal to this PR.

@pieterhijma
Copy link
Contributor Author

My 2 cents here:

  • There is no problem IMHO to have SEVERAL ways to store data in a document, object, part or whatever. There are, already. So I don't see any real inherent problem in having one more and have this PR merged. If it's useful to some, and the code is properly self-contained and compartment-tight (which it is), then why not

Great.

  • That said, I agree that fundamentally such a feature is basically relying on a new workflow yet to be created, together with a UI, etc. And it provides fundamentally the same thing we already have. So IMHO it cannot pretend to become a new standard. But if we're OK with that, I have nothing against it.

Of course, it is not removing functionality or other options. I simply hope that the new workflow with the additional feature of bringing modularity provides so many benefits for managing data, that it becomes the de-facto standard.

@sliptonic
Copy link
Member

Holding on this for this week. @pieterhijma, can you plan to attend merge meeting next week to walk through the plans?

@pieterhijma
Copy link
Contributor Author

Yes, I will make the plans more concrete then.

@WandererFan
Copy link
Contributor

VarSets could be useful in objects other than App::Part.

@yorikvanhavre
Copy link
Member

Reminder to @pieterhijma : update the icon to work on dark backgrounds

@pieterhijma
Copy link
Contributor Author

@yorikvanhavre, I've just pushed a commit with an outlined icon and rebased. Thanks!

@yorikvanhavre
Copy link
Member

Looks perfect, thanks! Waiting for the CI tests to finish and I'll merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants