Old Workflow refactoring

[ickby]

I'm talking about just adding a first-solid-feature property to body which required to be Part::Feature without changing any class hierarchy. And setting the first solid's BaseObject property inside the Body's Model to it...

Yes I understood it like this. With using a body like a feature i did not mean the class Part::Feature but the general concept of a feature as a modeling step. Like a pad is a feature added to something else, this property then allows to add a body to something else.

I'd rather follow the previous consent here: either to use old workflow or to migrate and put all PD features inside bodies... But I suppose now mostly any file will be possible to migrate without breaking anything... So even unconditional migration or to forbid to add features outside bodies would be an option for me...

I would be fine with both, as long as PartDesign features only can be added to bodies I'm good One could go a split route: allow old workflow in old files without bodies and offer a migration to move everything to a part and then work with the new body workflow. (move everything to a body seems problematic for mixed part - partdesign - draft models)

Oh... Body actually is a Part::Feature... So I suppose that can be considered as "yes"... =))
But I'd rather double check that it has correctly overloaded all required feature methods...

Making that fully workable is a nice addition too!

[Fat-Zer]

i did not mean the class Part::Feature but the general concept of a feature as a modeling step

In general, I still don't get what that «Feature» abstraction is...


By the word I want to hear your opinion on some more things I want to change...

Is there any special sense in making a non-solid feature a Tip of a Body?
The only purpose I can think about is to get a bit more manual control on the insertion of non-solids, but it's now organized tricky and I don't like it...

I really dislike the concept of Active body and Active part, I suppose where possible we should determine bodies from associated features, and if it's impossible guess upon selection and if we couldn't then get it from the user....

IMO it would be nice to mark a tip of the body graphically in the TreeView with a small icon like for broken/recomputed features. Won't be hard but will require modification to base Gui code and an icon (I suspect better have both 8x8 and scalable)...

[ickby]

Is there any special sense in making a non-solid feature a Tip of a Body?
The only purpose I can think about is to get a bit more manual control on the insertion of non-solids, but it's now organized tricky and I don't like it...

I'm not really sure, I suppose it was done the way it is now to keep always the newest feature and therefore have a strict order of things inside the body from the tip on. But for the outside world of the body it would indeed make more sense if it is always a solid. Then the outside access does not need to happen with the "preSolid" functions. But I think we can go here the route that fits the implementation best.

IMO it would be nice to mark a tip of the body graphically in the TreeView with a small icon like for broken/recomputed features. Won't be hard but will require modification to base Gui code and an icon (I suspect better have both 8x8 and scalable)...

That sounds like a good idea. But I think changing Gui code is not necessary, changing icons on the fly is possible within the normal code, so we would only have to implement some stuff in the viewprovider to overlay the normal icon with a marker and use this for the tree. I would need to search how this chaning icon stuff was done...

I really dislike the concept of Active body and Active part, I suppose where possible we should determine bodies from associated features, and if it's impossible guess upon selection and if we couldn't then get it from the user....

Hm I actually like it and think it is usefull. For example the active Part is also needed for the Part Workbench, there you cannot associate it. I also think it is a nice way for clear modeling, it reduces the possibility of accidently working on different thinks tremeandously. I highly appreciate this when working with multiple parts/bodies. I furthermore think the "active object" will become a way more used paradigm in freecad in the future, for exmaple FEM with a active analysis or Path with a active path object. So the user will be used to it.
--> I think we should keep it because it is a usefull thing. If it turns out to be a large hassle in modeling practice and we get a large negative feedback from users we can decide to drop it in the future, as it is only a GUI feature with no real impact on models/files.

[Fat-Zer]

I'm not really sure, I suppose it was done the way it is now to keep always the newest feature and therefore have a strict order of things inside the body from the tip on. But for the outside world of the body it would indeed make more sense if it is always a solid. Then the outside access does not need to happen with the "preSolid" functions. But I think we can go here the route that fits the implementation best.

I more and more certaining that it will really make things easier and more straightforward if Tip will be always a solid... On the other hand we will have to introduce some "insertion point" abstraction I see 2 ways here: To add an insertion point property or to insert before the next solid after the tip. I love the second one more...
So the Tip meaning will be "the final feature which body provides to the external world".

That sounds like a good idea. But I think changing Gui code is not necessary, changing icons on the fly is possible within the normal code, so we would only have to implement some stuff in the viewprovider to overlay the normal icon with a marker and use this for the tree. I would need to search how this chaning icon stuff was done...

I thought about that, but AFAIK the overlaying of icons in done in raster, and it's in general a bad practice to return prescaled icon from a view provider.

Hm I actually like it and think it is usefull. For example the active Part is also needed for the Part Workbench, there you cannot associate it. I also think it is a nice way for clear modeling, it reduces the possibility of accidently working on different thinks tremeandously. I highly appreciate this when working with multiple parts/bodies. I furthermore think the "active object" will become a way more used paradigm in freecad in the future, for exmaple FEM with a active analysis or Path with a active path object. So the user will be used to it.
--> I think we should keep it because it is a usefull thing. If it turns out to be a large hassle in modeling practice and we get a large negative feedback from users we can decide to drop it in the future, as it is only a GUI feature with no real impact on models/files.

I'll think about it... I can't say about parts, I'm not very competent with them, so I speak only for bodies... At least we may prompt a user if his selection is not conforms with operations in active body if he wants to switch it... also in some cases it makes absolutely no sense to require active body e.g. just to change the Tip...

Also there are some refactoring jobs pending I suppose:
There are a lot of utility code in Workflow.cpp (I suspect that most of it is temporary) may I move it somewhere else? Utils.cpp? some ViewProviders? some *Commands.cpp?
Commands.cpp is getting quite big... I want at least to split new workflow commands dedicated to creating a part and managing a body to a separate file... Any thoughts? May be the same for Datum's...

[ickby]

I more and more certaining that it will really make things easier and more straightforward if Tip will be always a solid... On the other hand we will have to introduce some "insertion point" abstraction I see 2 ways here: To add an insertion point property or to insert before the next solid after the tip. I love the second one more...
So the Tip meaning will be "the final feature which body provides to the external world".

What is the hassle with the current implementation and using the "getNextSolid" functions? I think a extra propertie is an rather ugly solution, I would opt for your second method too.

I thought about that, but AFAIK the overlaying of icons in done in raster, and it's in general a bad practice to return prescaled icon from a view provider.

I can't look it up now, but does the viewprovider function "getIcon" not return a QIcon, which is a non-scaled Icon? But of course if a general implementation in GUI core makes more sense than this is also possible. I just always try to prevent that as stuff in GUI must be well thought through and be implemented very general and complete, this is often way more work.

also in some cases it makes absolutely no sense to require active body e.g. just to change the Tip...

Why would a user change the Tip? Is this not a internal property where the (normal) user has no real use for?

Also there are some refactoring jobs pending I suppose:
There are a lot of utility code in Workflow.cpp (I suspect that most of it is temporary) may I move it somewhere else? Utils.cpp? some ViewProviders? some *Commands.cpp?
Commands.cpp is getting quite big... I want at least to split new workflow commands dedicated to creating a part and managing a body to a separate file... Any thoughts? May be the same for Datum's...

Yes sure, split commands at will if it gets too large. For utility stuff I suppose you mean Workbench.cpp? What would be the intend to move it? But I don't see any reason why not.

[Fat-Zer]

What is the hassle with the current implementation and using the "getNextSolid" functions? I think a extra propertie is an rather ugly solution, I would opt for your second method too.

There are two reasons:
First one is about the meaning of the Tip and intention of the body class: as I see it and as sad in the comment to BodyBase the body aggregates some modeling features and provides to outside only one of them, the Tip... For PartDesign It's really confusing IMHO to provide to outside something other but solid...
I've run into some problems implementing the BaseFeature, e.g. for consistency either the getPrevSolid should be able to return the link to it or the tip should be allowed to point to it... but with the first strategy the code became really messed up...

To be noted, I've already nearly done with both, but haven't committed yet... cleaning up some bugs in Gui now...

Why would a user change the Tip? Is this not a internal property where the (normal) user has no real use for?

IMHO, no... I see at least one use case personally I will appreciate: if the user wants to see what the whole part will look like if he will switch to some previous feature...
Also because of that I proposed to mark it in the tree view...

I can't look it up now, but does the viewprovider function "getIcon" not return a QIcon, which is a non-scaled Icon?

Yes, the viewprovider's getIcon() a QIcon as non-scaled Icon, and because AFAIK the overlaying of icons is done in raster (without getting deep into QIcon), we shouldn't do it in a viewprovider, if I understand it right...

For utility stuff I suppose you mean Workbench.cpp? What would be the intend to move it? But I don't see any reason why not.

yes, I'm talking about getBody(), getPart(), migration stuff, body setting up etc which is currently in the Workbench.cpp... I don't yet decided where to move it... may be a separate file CommandUtils.cpp or something like that...

[ickby]

I see. As said feel free to make the proposed changes, nothing preventing us here from chaning things internally.

[Fat-Zer]

I see. As said feel free to make the proposed changes, nothing preventing us here from chaning things internally.

It's better to discuss changing of key features first... I'm not an overmind, I may be mistaken or just wrong after all
The same is for non-necessary massive refactoring changes that will taint the git blame...

[Fat-Zer]

Ok, here is an initial for the BaseFeature work:
https://github.com/Fat-Zer/FreeCAD_sf_m ... aseFeature
Branch name: AMM-bodyBaseFeature

The branch is based on DeepSOIC's AMM-oldWokrflow6-rebaseMaster.

What should work:

• All old PartDesign features (Pad/pocket, Revolution/Groove, transformations, ) etc...
• Creation of a body with base feature (just select a part::feature when creating a body)
• Body management features moving features inside the body/setting the tip
• Split of some body-related commands to a separate file

Unfinished so far:

• Workflow selection.
• Migration

What is pending to be done:

• Forbid to create PartDesign features outside of bodies in the new workflow
• PartDesign boolean features
• Addictive/subtractive Loft/Sweep
• Lots of bugfixes

What wasn't tested/doesn't work:

• Addictive/subtractive primitives
• There were some crashes somewhere inside occ and coin, I'm not sure if they were introduced by me or were there before that...

A couple of testcases/examples to play with:
Just some abstract bunch of shapes based on the Part cube: (Notice the changes in shapes when editing the Cube's properties )

testcase-basefeature-1.1.png (214.39 KiB) Viewed 2108 times

testcase-basefeature-1.1.fcstd

(55.17 KiB) Downloaded 56 times

A pyramid build in the triplus's way(viewtopic.php?t=11155#p89745)

testcase-basefeature-pyramid.png (162.9 KiB) Viewed 2108 times

testcase-basefeature-pyramid.fcstd

(24.14 KiB) Downloaded 57 times

[triplus]

A pyramid build in the triplus's way(viewtopic.php?t=11155#p89745)

testcase-basefeature-pyramid.png

I didn't compile and test the branch yet as i feel this is something developers need to sort out. But from looking at it and what was said. Body container in this example actually is some sort of fork? If you have 2 body containers in the Part you actually have 2 forks (two branches)? And in this Pyramid example they got merged. But if i see correctly Cube, Common, Sketch002, Pocket and Pyramid are all outside the body containers and as i understood this shouldn't be allowed in new PartDesign WB? But indeed once you do Common between (features in) 2 body containers will you be allowed to add/remove any new features to the result? As i am not all that sure how does this sort of operation fits in the idea "Forbid to create PartDesign features outside of bodies in the new workflow"? Isn't Common exactly that? A feature outside any body container?