App::Part question

[DeepSOIC]

Ok, another smaller change about Parts to discuss (assuming the current state) :
Make Origin a separate Link property inside the App::Part.

Sounds like a good idea to me, although there might be some extra complications because of this.

[ickby]

Fat-Zer wrote:
Ok, another smaller change about Parts to discuss (assuming the current state) :
Make Origin a separate Link property inside the App::Part.
Sounds like a good idea to me, although there might be some extra complications because of this.

But wouldn't this be completely useless in the current structure? I mean searching a origin within a part is a super simple Part item iteration, and one can simply add a function which is doing it to the part. Adding a property for this seems way over the top for actually no real benefit (need to save it in files, need to keep it up tp date, increase object size in redo stack). Or are there any usecases I'm not aware of?

EDIT: I think in the feature many other things will have origins, at least all assembly objects next to part. But I also suppose that other workbenches are going to have their own geo group objects with origins.

[Fat-Zer]

Adding a property for this seems way over the top for actually no real benefit (need to save it in files, need to keep it up tp date, increase object size in redo stack).

Now the Origin link is stored with others in the GeoFeatureGroup::Items Property, so it's getting saved, keeped-up-to-date and present in redo stack anyway... But IMHO this is quite messy and error-prone because it's not meant to be Part's content and have slightly different meaning... Also it's really unclear and takes a lot of time to dig-out how part really references it...
I've run into the fact that I have to apply some actions to the origin or to any object in the part but the origin then playing around with Drag-n-drop and fixing up some bugs related to removing from the parts and undo... Also I had thoughts about some other applications.

EDIT: I think in the feature many other things will have origins, at least all assembly objects next to part. But I also suppose that other workbenches are going to have their own geo group objects with origins.

Ok, so a different intermediate class... (If no objections on the previous paragraph of course)


Yet another portion of food for thought...
I don't like that GeoFeatureGroup and DocumentObjectGroup have the same code... But yet I haven't found any sane way to overcome it except next two:
- making the GeoFeatureGroup based on the DocumentObjectGroup rather than GeoFeature and reinplemt stuff from GeoFeature (may break stuff)
- move code from them to some none-DocumentObject-derived class and inherit it as well in both *Group, but this got a lot of problems... including what to do with python API, with ViewProviders and others...

Also If this workout somehow I've got thoughts about making Part::BodyBase provide the same API as App::GeoFeatureGroup and App::DocumentObjectGroup, and rename it to Part::FeatureGroup for consistency... So the class will receive some meaning rather than just dangling around...

[DeepSOIC]

What's the purpose of the origins in the first place?
* attachment for sketches
* planes for mirroring and axes of rotations
* they look cool
* anything else?
If nothing else, I see no reason having a special property for them...

[ickby]

Now the Origin link is stored with others in the GeoFeatureGroup::Items Property, so it's getting saved, keeped-up-to-date and present in redo stack anyway... But IMHO this is quite messy and error-prone because it's not meant to be Part's content and have slightly different meaning... Also it's really unclear and takes a lot of time to dig-out how part really references it...
I've run into the fact that I have to apply some actions to the origin or to any object in the part but the origin then playing around with Drag-n-drop and fixing up some bugs related to removing from the parts and undo... Also I had thoughts about some other applications.

Ah now I see your intend. That sounds good, yes. If it is worth the trouble implementing it and chaning the coed everywhere, I don't know,

Yet another portion of food for thought...
I don't like that GeoFeatureGroup and DocumentObjectGroup have the same code... But yet I haven't found any sane way to overcome it except next two:
- making the GeoFeatureGroup based on the DocumentObjectGroup rather than GeoFeature and reinplemt stuff from GeoFeature (may break stuff)
- move code from them to some none-DocumentObject-derived class and inherit it as well in both *Group, but this got a lot of problems... including what to do with python API, with ViewProviders and others...

I currently don't know wihch parts are douplicate, but both your solutions sound actually worse than a simple code duplication Is the amount of duplication that large that a redesign is justified?

Also If this workout somehow I've got thoughts about making Part::BodyBase provide the same API as App::GeoFeatureGroup and App::DocumentObjectGroup, and rename it to Part::FeatureGroup for consistency... So the class will receive some meaning rather than just dangling around...

Wouldn't it then be best to simply make BodyBase Inherit from FeatureGroup? Don't know, If you think it is better to change it do it.


Just one more general thought. Please don't feel critisised, this is just and observation : It seems you already have come from a migration workflow implementation to a large structure redesign. Please make sure your task is not getting too large to handle. Of course many things may not be perfect, and often enough the work of others seems faulty in ones one view. However, changing everything is just too much work to bring to a successfull end. So please mke sure you keep your task in a managable size. It is always better to keep some things imperfect than not bringing the perfect thing to an end.

[Fat-Zer]

I currently don't know wihch parts are douplicate, but both your solutions sound actually worse than a simple code duplication Is the amount of duplication that large that a redesign is justified?

It's not really... but both GeoFeatureGroup and ViewProviderGeoFeatureGroup becomes copy-paste of the group's ones with small changes in the last one...

- move code from them to some none-DocumentObject-derived class and inherit it as well in both *Group, but this got a lot of problems... including what to do with python API, with ViewProviders and others...

This one won't workout without major changes..

- making the GeoFeatureGroup based on the DocumentObjectGroup rather than GeoFeature and reinplemt stuff from GeoFeature (may break stuff)

This one actually may work...
GeoFeature have only one property nothing more... I've greped the code and it seams objects are not very often referred as GeoFeature in context making sense in Part's context (only some free transformation related lines in base Gui)...
ViewProviderGeometryObject on the other side have a lot of complicated code, but half of it doesn't work for Part now:

• Material/Color/Transparency related stuff — doesn't work, also App::Part have it's own material property, which doesn't works either... Will need reimplementation anyway
• Selectable — doesn't make sense for Part because it doesn't displays anything by itself
• BoundingBox — doesn't work and I suppose will require to be reimplemented anyway
• Placement — works, not so big and generally some code may be shared with some static methodes...
• Free Transformation (with Orbits) — that one will be lost (IMHO it's in general doesn't makes a lot of sense for a CAD, I suppose it some kind of legacy), may be reimplemented or shared some way if needed...

To be noted there are not a lot of places in current code there referring a Part as a DocumentObjectGroup will do some stuff, although It won't break anything and I believe it will help to avoid some mistakes in future...
Also if so, assembly will allow to handle _only_ Parts...

In the underlaying sense way of speaking that will makes the Part primary a group rather, and only secondly a placeable object...

Wouldn't it then be best to simply make BodyBase Inherit from FeatureGroup? Don't know, If you think it is better to change it do it.

I thought about that too, but if so body won't be a solid (Part::Feature) as well, and loose the functionality it recently received... I'm afraid there will be too much work here.
I'm nearly left this idea anyway...

Just one more general thought. Please don't feel critisised, this is just and observation : It seems you already have come from a migration workflow implementation to a large structure redesign.

Yes, seems you are right, I'm going too far now... =)

PS: I'll take a shot with changing GeoFeatureGroup's base if I won't receive nice working result with reduce of code by today, I'll drop the idea...

[Fat-Zer]

Yesterday's experiment about rebasing Part on a DocumentObjectGroup was successfull enough: 168 additions and 440 deletions. And now it's possible to add features to parts with drug&drop... May still require some polish...
https://github.com/Fat-Zer/FreeCAD_sf_m ... partRework

Now I'm working on a "Group with origin entity" and rewriting the Origin to make all control and initialization dedicated to it inside it rather than inside a Part... going to finish it on weekends...

[Fat-Zer]

I'm a bit stuck with how to encapsulate the creation of Origin object and all it's axises/planes into one action.
IMO the situation when it's a caller responsibility to call some setup() method after the creation of an object to create it's childrens that are required to operate correctly is unacceptable...

The only place I found is to call it is during the view provider's attach(), but it seems a bit hacky (And won't work correctly in some cases I suppose)...
Another variant is to subscribe on App::Document's signalNewObject(), but this sounds a bit silly too...

A good way is to add a virtual setup() callback to App::DocumentObject() and to call it from the addObject()

Before I start to hacking into some basic stuff, are there any alternatives?

===
I'm going to revert 6ab2002 due to
1) deletion by the user must be handled in view providers... otherwise it may result in nasty situations when a user copied his origin/plane/axis to head and can't delete it afterwards (I see no way to handle it in)...
2) Seems there is no more usecases for the bit... are there?

[ickby]

A good way is to add a virtual setup() callback to App::DocumentObject() and to call it from the addObject()
Before I start to hacking into some basic stuff, are there any alternatives?

This seems like a good way of doing it.

I'm going to revert 6ab2002 due to
1) deletion by the user must be handled in view providers... otherwise it may result in nasty situations when a user copied his origin/plane/axis to head and can't delete it afterwards (I see no way to handle it in)...
2) Seems there is no more usecases for the bit... are there?

But how would you prevent the deletion in a viewprovider context? And if you change things to allow the prevention of a deletion, than you have the exact same problem after copying, don't you? IMHO it makes more sense to forbid copying of origins too, as it is a wrongs operation from th point of view that a origin always belong to a certain coordinate system. Either make the flag a non-deleatable/non-copyable one or add annother non-copyable flag.

[Fat-Zer]

But how would you prevent the deletion in a viewprovider context?

return false from onDelete(), haven't tried yet, but should work fine

And if you change things to allow the prevention of a deletion, than you have the exact same problem after copying, don't you?

just check if plane/axis/origin is part of origin/origin/part and delete if not...