Preview: Link, stage two, API groundwork

Merged, abandoned or rejected pull requests are moved here to clear the main Pull Requests forum.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Preview: Link, stage two, API groundwork

Post by realthunder »

ickby wrote: Sun Jul 30, 2017 12:16 pm So Every GeoFeature has a unambiguous location, it exists one time at one place only. That means every document object that uses geometric input from other objects need to know this unambiguous location to calculate the correct result. And this is only possible when the calculating object lives within the same GeoFeatureGroup than the objects it uses. Only than it uses the correct location information, and only then the calculated result is correct.
With you approach, the only thing the object needs to know is that all the base object it operates on is in the same coordinate system. It makes no difference which coordinate system it is in, right? So why not assume it is in the global CS? If we follow the same 'tool and its bases must be in the same coordinate system'. It makes no difference right? Why will this be a 'placement mess'.

Is it because if we move the base objects to another group, we'll see the tool's claimed children moved as well? Is that why you say it is wrong and must not be allowed? Well, my approach always shows the claimed the children at global CS, so it will not move. Then I guess you think my approach will be a mess because, say if I add only the fusion to some group, the fusion object's distance from its children will change, and that is wrong? But, when adding any object into a group, that object will jump as well. It is expected, right? And the distance between the fusion and its children is changeable in the first place, because it too has a placement of its own. So the user is obviously accustomed to the fact that the fusion and its children are not always at the same place.

With my approach, the workflow is much more flexible. You can follow the old way, by doing geometry modeling with objects all inside global CS, and then choose to add some final result objects (but not necessary its base objects) into whatever group, nested whatever you want. If you want to check the original relationship between the the model and its bases, just toggle its visibility in the global CS. If you work solely in some local CS, just make sure you add all objects and the tool output to that group. You don't need any special adjustment of the placement.

Toll and base in same Group: Yes, that will be enforced. Our solution to this is the instance object. If you want to use something in a different GeoFeatureGroup you simply make a instance of it and move that instance to the other Group. You have the object at two places, they are different. This is perfectly captured by the instance.
Is this 'instance' you are referring to somehow similar to my Link? Because, that is what Link is originally designed for. But, then I always wanted to see if it is possible to make this 'instance' behavior implicit for geo group, meaning that, any object added to it is implicitly made into an 'instance', so that it does not interfere with the real object in global CS. And that is kind of my whole idea of this post, not exactly the same, but pretty close (it has independent visibility already, but not placement, yet). And I am sure you realize that this 'instance' object must be treated specially, and be excluded from re-grouping. I am NOT sure, however, whether or not you realize that if you still insist on one object inside one group concept, than adding an 'instance' of another geo group will force you to make all of its nested children 'instance' as well. Trying to maintain synchronization is going to be a nightmare, I am afraid. Or, do you want to simply forbid instance of geo group as well? In summary, your 'local' approach put too much restrictions on the group member, which makes it not very 'nesting' friendly. But, being able to nest is a very important requirement for assembly, if not the most important.

Breaking Models: Definitely not. GeoFeatureGroup or the whole concept has not been available before, we do not break anything. Not all constructions may be splittable into different GeoFeatureGroups, but that is a minor Issue in my opinion. All old workflows are still supportet, Everything can live in the global CS as they do now, just don't put it in a GeoFeatureGroup.
Well, breaking is not the right word. I should have said incompatible, because existing model may not be addable to GeoFeatureGroup. Imagine how I would feel, if after all that hard work, and my own gigantic models can't be used in the new assembly WB?
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
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Preview: Link, stage two, API groundwork

Post by realthunder »

DeepSOIC wrote: Sun Jul 30, 2017 12:19 pmI was kind of nudging you to make a distinction by using a different separator in front of subelement, to be specific, and to allow to find out the type of shape returned from the link string alone. For example, "Part/Body.Face2" for Body.Shape.Face2, and "Part/Body/Face2" for Face2.Shape.
That is not a bad idea. I'll see if I can add it somehow. But to be compatible with current Gui.Selection display, may be something like an ending '.' for all object, but not the subelement. So Part.Body.Face2. for Body.Face2.Shape, and Part.Body.Face2 for Body.Shape.Face2
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
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Preview: Link, stage two, API groundwork

Post by DeepSOIC »

realthunder wrote: Sun Jul 30, 2017 5:05 pm"Part.Body.Face2." for Body.Face2.Shape
Sounds good to me :mrgreen:
ickby
Veteran
Posts: 3116
Joined: Wed Oct 05, 2011 7:36 am

Re: Preview: Link, stage two, API groundwork

Post by ickby »

With you approach, the only thing the object needs to know is that all the base object it operates on is in the same coordinate system. It makes no difference which coordinate system it is in, right? So why not assume it is in the global CS? If we follow the same 'tool and its bases must be in the same coordinate system'. It makes no difference right? Why will this be a 'placement mess'.
Because this assumption is not what the user sees. Consider your Boy and Cone are in two different groups. Then for the user they may appear in totally different relation to each other than to the tool that assumes they are both in Global CS. The result can be something considerably different than what the user expects when he selects the objects for the fusion. Thats what I mean with mess: You cannot relate 3D view to tool outcome anymore if all tools work on the Global version only. You can forbid to select anything in groups for tools at all as a solution, but this sounds really bad.
With my approach, the workflow is much more flexible
I question that. Your workflow basically means you create a instance everytime you add a object to a group. The original stays in the global CS and the instance is in the Group. That means all objects of the whole file are crowded at the toplevel tree. The advantage you gain is that you can move everything into groups, as it will be a instance only. What you cannot do is to really move things. So for the sole purpose of automatically creating instances you loose the ability to move objects completely. But creating a instance in the current approach is only one click away. So with the current workflow you still can build everything in the global CS as you propose and than simply make instances of the results and move them to the GeoFeatureGroups. It is what you try to achieve just with one click more. However, your way makes the other workflow completely impossible, that is moving objects around and having everything in Parts so that they are away from the global CS (which is how I work for example). You try to imitate that with the property to not show things in root tree, but this only works on a group basis, not on a individual object one. Your approach hence is less flexible.
Is this 'instance' you are referring to somehow similar to my Link
Yes, it basically is the same. And you did a good job implementing the link features. I don't see why it should not work for whole groups just as you have it now? The only difference is that GeoFeatureGroups do not create "links" or references.
Well, breaking is not the right word. I should have said incompatible, because existing model may not be addable to GeoFeatureGroup. Imagine how I would feel, if after all that hard work, and my own gigantic models can't be used in the new assembly WB?
You would simply make a instance/link to the results and put that into a geofeaturegroup. That is what you want anyway, model everything toplevel and only put instances into the groups. There is no difference to what you try to achieve.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Preview: Link, stage two, API groundwork

Post by realthunder »

ickby wrote: Sun Jul 30, 2017 10:12 pm Because this assumption is not what the user sees. Consider your Boy and Cone are in two different groups. Then for the user they may appear in totally different relation to each other than to the tool that assumes they are both in Global CS. The result can be something considerably different than what the user expects when he selects the objects for the fusion. Thats what I mean with mess: You cannot relate 3D view to tool outcome anymore if all tools work on the Global version only. You can forbid to select anything in groups for tools at all as a solution, but this sounds really bad.
The case you just described is a cross coordinate system operation, which is difficult to handle in both our approach. If those objects stay in the same group, there is no real difference to our approach, except that we show claimed the children at different places. But the relationship between children stays true. Like I said, the fusion itself can have placement, so the user shall expect the possibility of fusion in another place. Besides, with my approach, the user can always toggle the visibility at global CS at will.

Now, with cross coordinate operation, I do have an answer. Gui.Selection knows the selected full object path, so it will know exactly which 'instance' of the object is selected, and can easily detect if all selected objects are in the same group or not. Right now, with my implementation, Gui.Selection will by default automatically resolve to the selected objects in the global coordinate space. It can easily be modified so that when detects cross coordinate selection, create a Link pointing to the object in local coordinate instead, with a possible user warning if you want. This behavior is not implemented in this preview, because there is no Link class built-in yet. But the Python Link in the demo is already capable of link into object inside a group with synchronized local coordinates, which I've shown in the demo, the second screencast. It is not easy to handle the coordinate synchronization, but certainly not a mess, because it is automatic and not visible to user.
With my approach, the workflow is much more flexible
I question that. Your workflow basically means you create a instance every time you add a object to a group. The original stays in the global CS and the instance is in the Group. That means all objects of the whole file are crowded at the toplevel tree. The advantage you gain is that you can move everything into groups, as it will be a instance only. What you cannot do is to really move things. So for the sole purpose of automatically creating instances you loose the ability to move objects completely. But creating a instance in the current approach is only one click away. So with the current workflow you still can build everything in the global CS as you propose and than simply make instances of the results and move them to the GeoFeatureGroups. It is what you try to achieve just with one click more. However, your way makes the other workflow completely impossible, that is moving objects around and having everything in Parts so that they are away from the global CS (which is how I work for example). You try to imitate that with the property to not show things in root tree, but this only works on a group basis, not on a individual object one. Your approach hence is less flexible.
Good, it is argument like these make the discussion more productive. This is exactly my purpose of this preview. You list the things you want or don't want. And I can show you how it is done, can be done or avoid with concrete code and demo. As for your concern here, my first screencast shows how to avoid that. By default, any object added to my LinkGroup is removed from tree root. That doesn't mean the object is removed from global CS, it is still there, by default hidden, and not shown in the tree root either. ViewProviderLinkGroup has a property 'ChildAtRoot' to reveal the children at tree root. Now, this is for demo purpose only, we can easily add a menu action to reveal object in global space without the item at root, in fact, you can simply toggle the object's Visibility property to reveal it in global CS. The object's local visibility status is stored in its owner group, and can be access by group.ViewObject.setElementVisible(name, visible)

When I said I'd like to make the 'instance' behavior implicit for group, I didn't mean to actually create an 'instance' object for each added object. The 'instance' thing is internal to group, no actual document object is created (see my Python LinkGroup implementation). The crucial difference between an actual 'instance' object, and an implicit one in the group is that, 'instance' object has independent placement from the real object, while a child object in my LinkGroup can appear in more than one local coordinates, all of them has the same placement relative to their group. So when you move the object, it moves in all its group's local coordinate. And that may be an actual requirement for some use case. If the user wants independent placement, then they can manually create the 'instance' and add to group just like you described.
Is this 'instance' you are referring to somehow similar to my Link
Yes, it basically is the same. And you did a good job implementing the link features. I don't see why it should not work for whole groups just as you have it now? The only difference is that GeoFeatureGroups do not create "links" or references.
Because, when you have an 'instance' of a geo group, and put it into other groups, the child objects for that group 'instance' now physically exists in more than one local coordinates. It breaks the geo group's assumption of one object one owner group, meaning that, given an object, you can no longer determine which one the user is referring without the full object path. Unless, of course, like I said, you automatically make every nested child to be an instance, as well, which is going to be very hard to maintain.
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: Preview: Link, stage two, API groundwork

Post by triplus »

One observation why are we calling it a group in the first place if we actually talk about coordinate system after to make things clear? If it looks like a duck...
realthunder wrote: Mon Jul 31, 2017 3:51 am Now, this is for demo purpose only, we can easily add a menu action to reveal object in global space without the item at root ...
Such things usually make much sense from developer point of view. Where the developer has a good overview and understanding about everything. From an average user point of view such items in menu are usually comprehended as Sci-Fi.

But as for the important question. I feel we are there now and yes forget about everything else for now and just focus on coordinate system debate. And bear in mind consensus is what we are after and therefore after some argumentation that hopefully should be the result. In addition there are countless developers waiting to start supporting all this global/local coordinate system power with their commands once FreeCAD 0.17 is out. Therefore achieving such task should be rather painless thing to do in my opinion.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Preview: Link, stage two, API groundwork

Post by realthunder »

triplus wrote: Mon Jul 31, 2017 12:21 pm One observation why are we calling it a group in the first place if we actually talk about coordinate system after to make things clear? If it looks like a duck...
realthunder wrote: Mon Jul 31, 2017 3:51 am Now, this is for demo purpose only, we can easily add a menu action to reveal object in global space without the item at root ...
Such things usually make much sense from developer point of view. Where the developer has a good overview and understanding about everything. From an average user point of view such items in menu are usually comprehended as Sci-Fi.
Well, adding the local coordinate concept into the mix is going to make everything complex for everyone, even the developers, look how long it took to get where we are. After the developers reaches some consensus, we'll definitely need a few pages in the wiki to thoroughly explain everything in and out, probably touching deep into 3D rendering (Coin3D node tree) vs. geometry modeling (placement in FC, or TopLoc_Location in OCC stuff).
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
Jee-Bee
Veteran
Posts: 2566
Joined: Tue Jun 16, 2015 10:32 am
Location: Netherlands

Re: Preview: Link, stage two, API groundwork

Post by Jee-Bee »

First i hope the linking stuff i hope this will become in master very soon. As i see a lot of power in it related to assembly.

But I see the discussion becomes more or less a discussion about Local or global coordinate system. I have the feeling (from user perspective) that the discussion is going in the wrong direction...
For me as user the local coordinate system is what i use for modelling my parts. The global coordinate system is the one of the assembly with my coordinate system related to the assembly.

For what i read it is a discussion about an extra coordinate transformation.
I'm curious what would change so that the current PDN is usable with the linking too? I would say that, that is a far more important discussion... ;)
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: Preview: Link, stage two, API groundwork

Post by realthunder »

Jee-Bee wrote: Mon Jul 31, 2017 6:28 pm First i hope the linking stuff i hope this will become in master very soon. As i see a lot of power in it related to assembly.

But I see the discussion becomes more or less a discussion about Local or global coordinate system. I have the feeling (from user perspective) that the discussion is going in the wrong direction...
For me as user the local coordinate system is what i use for modelling my parts. The global coordinate system is the one of the assembly with my coordinate system related to the assembly.
Your understanding of the local vs global is kind of reversed in terms of this discussion thread, not necessarily wrong, because FC doesn't have any official definition of the terms yet. The real reason of the discussion here is that, in the process of working on links, I too, find it can do much more than I expected. in fact, it has too much power, so much so that it becomes incompatible with the current design of geo group. I'll try to explain more without bring in the confusing local vs global coordinate stuff.

You see, I started working on Link to solve one problem with geo group, that is, one object can only belong to one group at any time. That is going to seriously affect the assembly use case, just imaging how a lego assembly will look like. The reason of this restriction is that FC has trouble determine which object is selected, if the very same object appears in multiple places. I've worked very hard to make FC capable of doing that, and that's how Link can come into existence. When linked to an object, it shares its geometry data, but has separate visibility and placement, and appears to be a distinct object to geo group, so everything is fine, until I realize that Link can really be linked to anything. What if it links to another group. Now the actual object can really be everywhere. In my branch, I added extra logic for linking into a geo group, and it works very similar to the LinkGroup in my Python demo here. So, one of the purposes of this discussion is to check with ickby if he is willing to incorporate the same concept into geo group. And to a larger extent, whether it is really necessary to still hang on to the one object one group restriction. It is my feeling that, this restriction starts out as a compromise to FC's lack of ability to distinguish object in different places, but is later found to have benefits in other respect, so the restriction becomes a requirement and gets enforced more and more along the development process. But now, my stage one patch has given FC that ability, I'd hope we can look back and rethink our decisions.
I'm curious what would change so that the current PDN is usable with the linking too?
Err... what is PDN?
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
Jee-Bee
Veteran
Posts: 2566
Joined: Tue Jun 16, 2015 10:32 am
Location: Netherlands

Re: Preview: Link, stage two, API groundwork

Post by Jee-Bee »

realthunder wrote: Mon Jul 31, 2017 7:19 pm In my branch, I added extra logic for linking into a geo group, and it works very similar to the LinkGroup in my Python demo here. So, one of the purposes of this discussion is to check with ickby if he is willing to incorporate the same concept into geo group. And to a larger extent, whether it is really necessary to still hang on to the one object one group restriction. It is my feeling that, this restriction starts out as a compromise to FC's lack of ability to distinguish object in different places, but is later found to have benefits in other respect, so the restriction becomes a requirement and gets enforced more and more along the development process. But now, my stage one patch has given FC that ability, I'd hope we can look back and rethink our decisions.
I think this is a much better starting point for the discussion as what was before :D

realthunder wrote: Mon Jul 31, 2017 7:19 pm Err... what is PDN?
Part design next or better what is in fact current part design of 0.17 ;)
Post Reply