Realthunder Link implementation: Architecture discussion

Discussion about the development of the Assembly workbench.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Realthunder Link implementation: Architecture discussion

Post by ickby »

Hello,

I like to discuss realthunders link implementation from an architecture point of view. As this is a very important piece of functionality in FreeCAD we should be all aligned if this is the way to go, with all advantages and disadvantages it has. Note that I may have missunderstood some things from the code, so please feel free to correct me (Many changes, little time to review). I try to paint a high-level picture of the architecture for the discussion.

So it was already decided to have each Item in FreeCAD only once and make duplicates via instance objects, or link objects. There are IMHO two general possible ways to do this:

Create special link objects like done by realthunder
No changes to the current object/selection mechanism in FreeCAD, a Link is just a new object that would appear in the tree, as well as the selection and all PropertyLinks that uses the link object. The LinkObject than allows has an API to retreive the original object and all needed placement infos.
Advantage:
  • Basically no base system changes
  • No changes to tools nesseccary
  • Lots of functionality can be added to the link object to ease the usage of links
Disadvantage:
  • Tools cannot directly use selected links
  • To not adjust tools, and make them work with links directly, a special link object type has to be created for each object (like currently done for Part::Link). This than results in quite some data duplication
Make a reference to an object only valid together with instance information.
This means a link to an object would not be valid by only holding the object itself, but only with the pair {object, InstancePath}. One could easily extend the PropertyLink properties to hold both, and also extend selection mechanism to return both on selection. So the Instance Object would be a new object in the tree, but when selected it returns the pair {object, InstancePath} with the original object and just a different instancePath. Than all tools would need to be adjusted to handle the Instance Path.
Advantage:
  • All links are still to the object of correct type, features using them would only need to do some placement calculation via instance path. This is a small change.
  • Very coherent architecture, e.g. no real special cases
Disadvantage:
  • Some changes in the base system
  • Every tool in FreeCAD would need to be adjusted to work with InstancePath

So the question is if we are ok with this kind of implementation. I personally see the problem of interoperability with the chossen approach, the Part::Link for example looks like a hack solution, and making this for other objects as well gives me some headaches. The second approach seems to be more streamlined, even though more laboursome to implement. Do you see more advantages or dificulties than described? Do you think I'm wrong in my assumption? Thats an open discussion and I'm fine with been proven wrong, lets go :)
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

Re: Realthunder Link implementation: Architecture discussion

Post by yorik »

ickby wrote: Mon Jun 19, 2017 2:03 pmEvery tool in FreeCAD would need to be adjusted
For me the problem is just that line :D
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Realthunder Link implementation: Architecture discussion

Post by realthunder »

Hi, ickby, I am very appreciated that you actually take time to look at the code. I was going to start a discussion like this, but without someone knowing the details, I felt like talking to myself most of the time.

My branch actually kind of addressed both your approaches in certain way. There are two important API added to DocumentObject.

1) getLinkedObject, which is what you mentioned in the first approach, to get the linked object and accumulated placement along a path. This path only consists of other intermediate Links. Because of the placement override (or accumulate) effect, link creates a new coordinate system, similar to a geo group with only one child, except that geo group can only accumulate but not override.

2) getSubObject, which allows you to retrieve an object deep nested in some container, with accumulated placement as well. Right now, only geo group has meaningful implementation of this function. This is somewhat similar to your second approach. Difference is that your approach holds the object at the tail of the path, but getSubObject is called from the head (the top level group) of the path. Both having the same path. When the path is traversed using getSubObject, all intermediate links are automatically resolved, and their placement accumulated as expected.

I think your description didn't distinguish these two type of path, or you didn't account for the possibility of multiple-link-chain to some object. The later actually has very practical usage.

There is another crucial part I added to the Gui namespace for 3D selection, which is realized using two new API in ViewProvider, getElementPicked(), and getDetailPath(). These two functions allow FC to correctly disambiguate the same Coin node picked using its Coin node path. getElementPicked translates the Coin path to the DocumentObject hierarchy path (your second approach), and getDetailPath() does the reverse translation so that a selection in the tree view (a selection of the tree node in fact determines the object path) can be correctly synced to 3D view. Currently, only ViewProviderLink implemented these two function, and thus, only linked geo group shows the full object path when the child of a linked geo group is selected. I have later added Gui::Document::getViewProvidersByPath() which can obtain the object path from coin path for plain geo group, as well. But Gui::Selection is not using that at the moment for compatibility reason, because I want to limit this path thing to Link at the moment, in order to not affect too much of the existing FC system.

I also modified the Gui::Selection so that normal tools can work with object referenced by path. The path is reflected in SelObject.SubName. Selection::countObjectsOfType/getObjectsOfType() will by default automatically resolve the object using getSubObject API, and thus, as I mentioned early, automatically resolve intermediate Links as well. Most of tools calls these API to obtain the selected object, and operate on them. So they should work as before. Although, in order to get the user desired result, the output object should be put into the same container of the selected object as well. But this 'smart active container' feature is out of the scope of this branch, and @DeepSOIC has done quite some work on that I believe.

So, in summary, my Link branch is not all about Link, it also introduced the object path concept to FC, in both App and Gui namespace.

The reason why Part::Link exists is actually very fundamental. Considering that the user selects a Link instance in 3D view, It doesn't matter whether this instance is deep nested inside a container or not, Gui::Selection will resolve to this link instance, not the actual object it links to, because, unlike container object, where the tool can put the resulting object back to the container to be with the correct coordinate system, Links can't. So the tools will be operating on the link instance, instead of the linked object. And Part::Link is there to make that work. I'd like to know if you have any solution for this fundamental problem.

Or, put in another way, let's say, we don't want placement override and link-chaining feature, then Link object is no longer needed. We can achieve the linking effect by using geo group only. I can easily modify geo group to implement getElementPicked() and getDetailPath() function to "link" its children just like a linked geo group, so that the same object can now correctly appear inside more than one group. I actually intend to do that once Link branch is matured. Maybe that is a partial solution, i.e. use this enhanced geo group for linking purpose, and all tools can work as expected as long as they put the result back to the container.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: Realthunder Link implementation: Architecture discussion

Post by ickby »

Hello,

you implemented the ObjectPath, that is very good and definitely something along of what I imagined in my second architecture (calle dnstancePath). Basically a description how to reach the object with resolving all links and containers. So lets call it ObjectPath from now on.
The reason why Part::Link exists is actually very fundamental.
Yes, and this is excactly where I see the problem. When the link is selected it is used in the tools, and Part Link is used to do the translation so that the tool must not be adjusted. I see why you introduced it, and in your current architecture it must work like that. You had two possibilities, to either make every tool aware of App::Link and handle it with the getObject/getSubObject API or build a middle man that emulates a Part::Feature and does all the link resolving. you chose the second and I see two main issues with your choice here:
1. You need such an adapter for basically every object type in FreeCAD to not adopt the tools. In Part this worked ok, as TopoShapes have a memory sharing functionality, but in other workbenches like mesh this may result in much data douplication. Also it feels a bit like a hack.
2. The user needs to make a different link for each object type by hand, as the general link tool can't access the workbench specific implementations. This is very unintuitive and cumbersome. This may be worked around with some clever API/extension code, but currently workes like that.

Now what you actually did is IMHO trade to not change tools for lots of adoption code. That resultet in a somewhat blurry and hard to grasp architecture, it makes Links not a first class citicen but some additional feature somehow added on top of everything. I personally don't think this trade is a good choice and that the other option is actually less work.

Just imagine you chose to make every tool aware of App::Link and enforced it to handle it instead of the actually required DocumentObject type. You would need to add some type checking code and retreving of the linked object if it is a link. Than when accessing the properties of the object you would need to make sure that the different placement is applied correctly when needed. However, only very few properties are actually interested in placement, Shape and Mesh for example, but not Length or Angle etc. This boilderplate code is not that much (and helper functions can be provided) and I personally think it is simpler to handle than what is implemented now.

So with this needed adoptions in mind one could think of a different way of doing things. If a normal PropertyLink would be extended to hold the pair {DocumentObject, ObjectPath} it would unambiguisly describe the linked object, even with multiple links to the same DocumentObject. When the default DocumentObject is selected than ObjectPath is zero. Gui::Selection would also always return such a {DocumentObject, ObjectPath} pair, and if the user selects a link it would created this pair just with a different ObjectPath. Than a tool would like now just access the linked object and all its properties, no change required. Only for the properties that are dependend on placement a small API change is done. E.g. instead of shape.getValue() it will be shape.getValue(ObjectPath). There the transformations could happen (thats simplified for now). Than the compiler tells you excactly where one needs to make the slight code adoption in the tools for the correct property handling. This would even further reduce the needed tool changes, but makes a way cleaner API and implementation. this includes links as first class citizen in FreeCAD,no workaround code needed.
ickby wrote: ↑
Mon Jun 19, 2017 2:03 pm
Every tool in FreeCAD would need to be adjusted
For me the problem is just that line :D
I know, it sounds horrible. But Links will be a extremely often used feature, and if we start in the beginning with unclean implementations we will have way more spagetthi code to write than tools to slightly adjust.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Realthunder Link implementation: Architecture discussion

Post by realthunder »

ickby wrote: Tue Jun 20, 2017 6:08 am Just imagine you chose to make every tool aware of App::Link and enforced it to handle it instead of the actually required DocumentObject type. You would need to add some type checking code and retrieving of the linked object if it is a link. Than when accessing the properties of the object you would need to make sure that the different placement is applied correctly when needed. However, only very few properties are actually interested in placement
That is actually quite doable, and better, we don't need to "enforce" a foreigner type onto any tools. The tools just need to remember to call DocumentObject::getLinkedObject() on its operating objects, which will resolve to the linked object if it is a link, and obtain the correct placement. Actually, various PartGui view providers are modified this way to call Gui counterpart API ViewProvider::getLinkedView(). I have to add this modification because most of PartGui view provider is hardcoded to work with objects having PartGui::ViewProviderPart derived view provider only, which neither App::ViewProviderLink nor Part::ViewProviderLink is. I could have done the same with various Part feature tools, but I wanted to limit the extent of modification at current stage. I guess you are right in this aspect, that it won't take much code to enable other tools to work directly with App::Link.

I would argue aginst modifying PropertyLink the way you described, though. It is both redundant and actually harmful to encode the actual ObjectPath into the link property. Let's consider an example,

Code: Select all

PartA
  |--ChildA1
  |--ChildA2(link) -> PartB
  |--ChildA3           |--ChildB1(link) -> Shape1
                       |--ChildB2(link) -> Shape2
When the user selects the ChildB1 (which is a link) in the 3D view, the current implementation of Gui::Selection will give you a SelObj with (ObjName='PartA', SubName='ChildA2.ChildB1'). ChildA2 here is also a Link, but Gui::Selection, in general, does not take link path into consideration. It relies on getSubObject() API to internally call getLinkedObject() to resolve link implicitly. So, there is no explicit reference to 'PartB' here.

The problem I see with your idea is that, for the modified PropertyLink, what shall we store for the ObjectName and ObjectPath? Shall we set ObjectName='ChildB1', and ObjectPath='PartA.ChildA2'? If so, what if user changed ChildA2 later to point to another object? the stored ObjectPath become invalid. Or, store the complete path 'PartA.ChildA2.PartB.ChildB1.Shape'. Well, this has the same problem as before, and additionally, the tail path is redundant because it is already stored inside ChildB1, and is better queried at runtime with getLinkedObject() because it might change as well. Now, say, the user ctrl select an additional ChildB2, and tries to create a FuseB. My current implementation relies on Gui::Selection to automatically resolve the ObjectPath to obtain ChildB1 and ChildB2. So the fuse will be applied on to ChildB1 and ChildB2 without needing to know the ObjectPath leading to them. In other word, their containers' placements are not taken into consideration. That's why I said, in order to obtain the user desired result, the tool shall put the output object back to the immediate container of the selected object. In this case, put FuseB back to PartB. This way, any changes to parent containers won't have unexpected side-effect on the output object, and all these can be done without modifying PropertyLink.

So in summary, I agree that in the long run, it is better to modify every tool to be aware of link, by calling getLinkedObject(). And, the tools shall put the result back to the owner container of the selected object, which can be obtained by calling GeoFeatureGroupExtension::getGroupOfObject().

All that being said, Link does have the capability to store ObjectPath, somewhat similar to your idea of PropertyLink modification. Link stores the head of the ObjectPath into its PropertyLink, and the rest of the ObjectPath in SubList, a PropertyStringList. This enables the user to flatten the object hierarchy and perform operation across container.

Considering the previous example, in order to fuse ChildA1 and ChildB1 and without putting the result FuseC into any container, the user can create a LinkC1 to PartA, and add 'ChildA1' to SubList, then a second LinkC2 to PartA, and add 'ChildA1.ChildB1' to SubList. I consider this to be advanced usage, and the user needs to be aware of all kinds of gotcha involved, and a new dedicated discussion thread is probably required for this topic.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
User avatar
yorik
Founder
Posts: 13640
Joined: Tue Feb 17, 2009 9:16 pm
Location: Brussels
Contact:

Re: Realthunder Link implementation: Architecture discussion

Post by yorik »

ickby wrote: Tue Jun 20, 2017 6:08 amI know, it sounds horrible. But Links will be a extremely often used feature, and if we start in the beginning with unclean implementations we will have way more spagetthi code to write than tools to slightly adjust.
Yes I agree of course. I was just gratuitously complaining :) Such a Link object is something I feel the need too, and all the complexity of interdependent placements and coin nodes is something that would be really nice to have a clean way to work with.
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: Realthunder Link implementation: Architecture discussion

Post by ickby »

The problem I see with your idea is that, for the modified PropertyLink, what shall we store for the ObjectName and ObjectPath?
I would not have used the object name sheme, but the PropertyLink would have stored th edocument object pointer just as now. In your example, when the user selects the ChildB1 Link, the stored information would be "Shape1" pointer and the ObjectPath "ChildA2.ChildB1". So you are right, this is the other way around as your idea. But It is not redundant, you need the object and the path to it to make it unambiguous. See also the next paragraph:
If so, what if user changed ChildA2 later to point to another object?
Very good point. I always thought that in this case the PropertyLink of the using object must become unresolvable. The user selects a certain, well defined instance of an object. If the link changes, this object does not exist anymore, hence the PropertyLink using it must become invalid. But I think here we have a difference in thinking. Where I see this whole stuff as a mean to douplicate objects and to make the douplciates just like normal objects I think you are going more for a real link, in the sense of "link in whatever you like and change that later", with the purpose of excactly that, chaning the linked object. That is maybe the most fundamental differences why we also come to different architectures.

I need to think about that a bit more :)
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Realthunder Link implementation: Architecture discussion

Post by realthunder »

Hmm, now I can see clearer the difference in our view point, too. I think assembly is meant to capture the placement relationship among various objects. The actual instance (or type) of object is not all that important. And Link is here to act as an abstract placeholder in the container, which is why I specifically allowed multiple link chain to a final object. Each level of linking (indirection) is an abstraction. Considering an example of screw in an assembly. You'll create individual Link for each screw and place thim in an assembly. Instead of pointing those screw Link to the actual screw object, you point those Links to a second level Links, each of which links to a different type of screws. Now, you can easily change the actual type of screw without needing to manually change each individual screw link in the assembly.

This abstraction can also help a lot with the infamous topological naming problem. Instead of directly referencing the volatile topological names, like edges and faces, etc, you can Link to those feature, which effectively give it a name of your choice, and then reference this Link in other part of the assembly through others Links. When the topological name changes, as it will inevitably, manual modification will be drastically limited and becomes manageable. Imaging we create a special container for assembly Part, with a predefined sub container for "Connection Point", and put Links to the connection edges or faces. Whenever an assembly is modified, the author only needs to check for children inside this sub container, and manually adjust the Links inside if necessary. Higher level assemblies will not be affected. If the abstraction is done right at the design stage, the Part and Assembly author can easily be of different person, or even from different vendors. It becomes Object Oriented, so to speak.

For the first example in my previous post. The user selects ChildB1, and want to modify it. What is his real intention? If he is the author of the assembly, then he deliberately Link PartB into PartA through ChildA2, which indicates that his real intention is to make change in PartB, not just the particular linked instance under PartA. So, that change should be reflected into PartB, and all other links to PartB shall be able to see the change as well. If his real intention is to modify that particular instance of ChildB1 under ChildA2, then as the assembly author, he should realize that he is making a new specialized Part derived from PartB, and then use extra Link, Copy, or other means to adjust the assembly structure first before doing the modification to avoid affecting the other links to PartB. In this sense, I don't think it is necessary to store the leading ObjectPath for disambiguation. With the current implementation, given an object, one can unambiguously determine the direct parent and other ancestor geo group(s). That object can appear in multiple places through other Links. And since it is referenced through Linking, it indicates the user's intention to reflect all changes of the object to those Links as well.

This is exactly why I named it as Link, not Clone. We can later add a Clone object (maybe enhance Draft.Clone), that takes advantage of Link's coin node sharing, but with something like Copy on Change behavior.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
triplus
Veteran
Posts: 9471
Joined: Mon Dec 12, 2011 4:45 pm

Re: Realthunder Link implementation: Architecture discussion

Post by triplus »

When PartDesign NEXT effort started we mixed Assembly, local coordinate system, topological naming ... areas heavily. That turned out to be a mistake as it lead to a lot of confusion. This are separate areas/projects. Therefore we should learn something from that and shouldn't do that again when it comes to Links. Links are links and lets focus on that for now. Other areas are not all that related. As for topology i asked you that question and you didn't provide a clear answer.

Link has topology (vertex, edge, face) and topology of a link can change? If that is true i wouldn't mix topology discussion in Link discussion all that much for now. As Link doesn't solve it therefore that is a separate and unrelated effort in the end.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Realthunder Link implementation: Architecture discussion

Post by realthunder »

triplus wrote: Wed Jun 21, 2017 12:04 pm Link has topology (vertex, edge, face) and topology of a link can change? If that is true i wouldn't mix topology discussion in Link discussion all that much for now. As Link doesn't solve it therefore that is a separate and unrelated effort in the end.
Well, you have to realize that there is no way to solve the topological naming thing completely automatically by the program. You can have the smartest hash algorithm in the world, and there still won't be any guarantee that the topological name will stay the same. So, human intervention is bound to happen at some point. What we can do, is to contain that change to a limited scope. When doing assembly, there are all kinds of hierarchies involved. The bottom hierarchy involves the actual models that are the direct cause of topological changes. We can use link (it doesn't have to by my Link object, some other object with PropertyLinkSub works, too, although less flexible) to isolate that changes. Those links define the connection interface of the model to other parts, and we can use a specialized container to group the bottom level model with all its connection interfaces. The author must check those links after changing the model, and manually correct any undesired topological naming changes. The higher assembly hierarchies can then interconnect together through links to those connection interfaces. Again, those links can be other type of object with simple PropertyLink. The key is that, at this level, there is no need to reference topological names any more.

Is that a clear answer to your question? This concept is not specific to my Link object, but it is one of the main design goal of Link, that is, to provide abstraction.
Try Assembly3 with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
Post Reply