Dissallowing cross coordinate-system links

[DeepSOIC]

@ickby: I'm curious to know, why didn't you go for cross-reference prevention by altering the behavior of getSelection()/getSelectionEx()?

[ickby]

@ickby: I'm curious to know, why didn't you go for cross-reference prevention by altering the behavior of getSelection()/getSelectionEx()?

Mostly because this only helps for tools and is not a general solution, you can mes sthings easily from the python or c++ API. I suppose doing it that way we still would end up having any kind of crazy graphs.

Yesterday I realized that my code doesn't work very well, as it only checks for new links. However, if one removes a object from a group, but not the objects it links to, it still creates an invalid linking without my code recognizing it.

With this issue and the already mentioned ones bfrom deepsoic it seems I need go back to the drawing board, the current solution is not very satisfying...

[DeepSOIC]

Hi!
I have a couple thoughts to drop in.

I don't really like the term "coordinate system" referring to a collection of objects sharing a coordinate space. I'd suggest replace the term with the word "workspace".


Another thought is about caching InLists. I think this is a little bit of an overkill. And it doesn't knock the performance of looking up a container of something down to absolute nothing. You still need to filter the inlist to pick up containers from it, and then check if the feature is actually contained by the container.
Making the object know which container it is in seems to be easier, as you only need to alter GroupExtension to take care of keeping the stuff synchronized. Yet it gives absolute maximum performance for the problem.
One thing that benefits from inlist caching is TempoVis, which has a feature to collect up all dependent objects to make sure they are hidden. But this kind of operation can be re-implemented to pre-cache the graph as needed, especially to constrain itself to only work within one container. And even in current implementation I haven't noticed any slowdown yet.

[kkremitzki]

I don't really like the term "coordinate system" referring to a collection of objects sharing a coordinate space. I'd suggest replace the term with the word "workspace".

Why not reference frame? Isn't that exactly what they are?

[DeepSOIC]

Why not reference frame? Isn't that exactly what they are?

Because
a) reference frame, as in physics, does not define a coordinate system. In this case, we are talking of a container that does define a coordinate system.
b) in our case, we are talking that geometric objects belong to a coordinate system, and it requires additional effort to use the object from within another coordinate system (transformation between them needs to be taken into account).

Workspace seems to do a better job, because I can say:
"Object1 is in Workspace1"
"Objects within a workspace can link freely to each other", e.g. "I can Part Fuse a sphere and a cube because they are both in Workspace1"
"Objects in different workspaces cannot simply link to each other, they must use special link type and optionally account for coordinate system difference"
"Placement defines a coordinate system" (I mean, placement alone by itself, App.Placement)
"Coordinate system doesn't define placement, since coordinate system can be in general left handed, skewed and otherwise distorted"
"Placement doesn't define a workspace. Workspace is defined by a moveable container (e.g. PartDesign Part). Objects in non-moveable containers (like Group) within Part are in the same workspace, Part."

[kkremitzki]

Why not reference frame? Isn't that exactly what they are?

Because
a) reference frame, as in physics, does not define a coordinate system. In this case, we are talking of a container that does define a coordinate system.
b) in our case, we are talking that geometric objects belong to a coordinate system, and it requires additional effort to use the object from within another coordinate system (transformation between them needs to be taken into account).

Workspace seems to do a better job, because I can say:
"Object1 is in Workspace1"
"Objects within a workspace can link freely to each other", e.g. "I can Part Fuse a sphere and a cube because they are both in Workspace1"
"Objects in different workspaces cannot simply link to each other, they must use special link type and optionally account for coordinate system difference"
"Placement defines a coordinate system" (I mean, placement alone by itself, App.Placement)
"Coordinate system doesn't define placement, since coordinate system can be in general left handed, skewed and otherwise distorted"
"Placement doesn't define a workspace. Workspace is defined by a moveable container (e.g. PartDesign Part). Objects in non-moveable containers (like Group) within Part are in the same workspace, Part."

Are you using a very particular definition of reference frame? A reference frame does not require a particular coordinate system (e.g. Cartesian, spherical), but a reference frame from my understanding is a coordinate system/orthonormal basis equipped with an origin, so while it doesn't "define" a coordinate system, it certainly "has" one, although which one is arbitrary. Then, the geometric objects you mention in b) "have" that reference frame, or are defined in terms of it, and in order to use objects in another reference frame, you have to start expressing things using a combination of the vectors that define the origins of your reference frames plus their basis vectors.

Reference frames seem to do everything you're describing so it's quite confusing to hear you say otherwise.

However, as far as the actual implementation, what you're saying makes sense; using "Workspace" for a movable container of objects that are share the same reference frame (as I use the term) seems useful, especially as a way to differentiate between fixed containers like Group.

[ickby]

I changed the code a bit to make a pull request. Due to the discussed problems the implementation of cross CS link handling need to be changed, that will be done in annother PR. For now I bundled all the Group and GeoFeatureGroup handling code that makes the group handling easier. So no changes to link properties.

https://github.com/FreeCAD/FreeCAD/pull/803

[ickby]

I further worked on this topic and I think the now found solution addresses all problems you guys brought up: https://github.com/FreeCAD/FreeCAD/pull/916

It introduces "Scope" for links, specifying what it is allowed to link to in respect to GeoFeatureGroups. In is either "local" (only within the same group as the object that holds the link property), child(if the object is a GeoFeatureGroup it is allowed to link to things inside it) or Global (link to everything). All links are "Local" by default.

The scope is only checked on recompute. This means it is allowed to have all kind of link combinations while altering the document and it must only work out once recompute is called.

[triplus]

Sounds reasonable.

P.S. Is there any support or plans to expose link scope to GUI workflow? I am guessing currently only local coordinate system links can be created from the GUI?

[ickby]

Is there any support or plans to expose link scope to GUI workflow? I am guessing currently only local coordinate system links can be created from the GUI?

There won't be any. This is only relevant for tool developers. They must decide how there tool works, and which kind of links they need. The default "local" scope is how FreeCAD currently behaves, so no tool needs to be adopted. Only special ones that want to work with GeoFeatureGroups in a special way need the other scopes, and that is than up to the developer.

There must be some GUI adoptions to streamline the workflow. DeepSOIC has done some work on this in his "container branch". Don't know how far that currently is.