[merged] PR #4118 The section cut feature

[user1234]

Hello!

This would be an nice feature. For example if you had to repetitive inspect always the same section while constructing or assembling. Like an object with source (child objects), directions, and settings (like TechDraw). Other CAD progams have also similar functions.

But an boolean cut is the overkill. A visual cut is more then enough.

Greetings
user1234

[wmayer]

I would like if you are less upset

I am not upset (maybe a little bit )

since you are a master and I am a beginner

Well, this depends on how you define master. I consider myself as a beginner with some experiences.

and you have seen in the past that I followed your advises

I know and appreciate it. That's why I was a bit surprised.

I will carefully read you feedback later this evening.

I think the easiest is to move your additions to the PartGui module. Then fix the issues I described in my second post.

I understand that I am not objective when I collect my personal use cases but I think I am not the only user having such use cases.

This is absolute valid and is how FreeCAD had grown over the years.

I took the largest real-life STEP I could find. Some months ago I had to cut it already and did it manually using a box too. So cutting works, but of course not with a slider. However, in most cases you know where to cut in advance and can enter the position and save recomputes.

With one of its 7.x versions OCCT has introduced the fuzzy boolean operations and we have to re-check this possibility to get faster results. Btw, how big is your STEP file and how long does it take for cutting?

Interesting. The question is if their result can lead to a feature in the document tree that one can modify further. I cannot figure that out.

No, I don't think so -- at least not very easily.

The volume rendering is an option we also need because there is an open ticket for a feature request. When we render a solid then we only display the boundary faces but as soon as you use the clipping plane you will see that the solids are hollow. The volume rendering then shows a face at the position of the clipping plane so that the user has the impression that the object is really solid.

[uwestoehr]

I think the easiest is to move your additions to the PartGui module. Then fix the issues I described in my second post.

Yes. I am however still reluctant to keep the combination clipping/cutting in one dialog. I would also like to keep the feature accessible via a main menu that one can easily access it for different WBs.

With one of its 7.x versions OCCT has introduced the fuzzy boolean operations and we have to re-check this possibility to get faster results. Btw, how big is your STEP file and how long does it take for cutting?

It has 20.8 MB and consists of 17 parts. The interesting thing is that when I create a link to it, put the link into a compound and then cut out a box it takes 2:06 min. But when taking a boolean fragments instead of the compound, it only takes 0:26 min. That is why I am already working on a cut using Boolean fragments. But this if of course only possible for more than one part/object. However, I don't have a big STEP consisting only of one part/object.
To be fair, calculating the Boolean fragments needs time too.

As I stated in my initial posts, OCC 7.4 is a big improvement in terms of speed for boolean operations compared to OCC 7.3.

No, I don't think so -- at least not very easily.

OK. So they are no solution for posterior modification, creation of technical drawings, meshes etc.

[uwestoehr]

Your function however also modifies the document and thus I strongly recommend to implement it as a separate function and realize it as task dialog so that no other function can be opened at the same time.

OK. My initial plan was to have a dialog that acts on the currently active document so you can jump between documents and make cuts. Meanwhile I realized that this is not a very clever idea.

This is quite inefficient because the call of getObject() internally has to search inside a std::map which has a complexity of O(log n) and doing this in a loop can cause a complexity of O(n log n). Access it only once and store the pointer.

OK.

When you add an object with

Code: Select all

auto CutFeature = doc->addObject("Part::Cut", "CutViewCutZ");

there is no guarantee that its name will be "CutViewCutZ" and thus it's not safe to try to access it this way as this can point to the wrong object.

That is what I mean with I am a beginner. Why is there no such guarantee? I chose this name, created the object and immediately after its creation I access it. Why can the name be different? I see in the code that Python WBs create objects this way too.

Code: Select all

Part::Compound* pcCompoundDel = static_cast<Part::Compound*>(compoundObject);

Same as above and thus the static_cast is dangerous.

I have this code:

Code: Select all

if (doc->getObject("CutViewCompound")) {
        auto compoundObject = doc->getObject("CutViewCompound");
        Part::Compound* pcCompoundDel = static_cast<Part::Compound*>(compoundObject);
        

so I check of the compound exists, then I get it and cast it. Do you mean that the user might have renamed another object to "CutViewCompound" that is not a compound? What should I do instead other than catch the cast exception? I cannot use a pointer because one of the features is that you can open a file with an existing cut and the cutting feature removes/refreshes it when you invoke a cut operation. Since the objects existed before the Cutting dialog was opened, there is no pointer available.

Code: Select all

if ((*it)->getTypeId() == Base::Type::fromName("App::Part")) {

Use

Code: Select all

if ((*it)->getTypeId().isDerivedFrom(App::Part::getClassTypeId())) {

But I don't want all types that are derived (maybe in future) from App::Part, only the App::Parts.

Code smell: Assembly4 is an add-on. So, do not make any assumptions of how things are implemented.

The point is that I can only take links from App::Part objects when I know it is an assembly 4 object. Otherwise I must take all links from all App::Parts then subsequently filter out of the links link to objects that can be cut. This involves a lot of code to be run.

Unsafe check. A feature can be created with a property called Shape that is not a Part::Feature. So, you will pick it up with unexpected behaviour afterwards.

OK, but what can I do? I already asked in the forum and anly got the reply to use Python's "hasattr(Shape)" but this method is the same I did.

Code: Select all

    // sort out objects that are part of Part::Boolean, Part::MultiCommon, Part::MultiFuse,
    // Part::Thickness and Part::FilletBase

What is the intention?

I am unhappy with this too. All I need is a way to sort out the subobjects of cuttable objects. For example Part::Boolean is a cuttable object, then only this one may appear in the compound, not also its Base and Tool objects.

When you know that an object is a Part::Feature then use its OutList to remove all objects you don't want to have.

So "->getOutList()"? Seems this is what I was searching for. Thanks!

[chrisb]

When you add an object with

Code: Select all

auto CutFeature = doc->addObject("Part::Cut", "CutViewCutZ");

there is no guarantee that its name will be "CutViewCutZ" and thus it's not safe to try to access it this way as this can point to the wrong object.

That is what I mean with I am a beginner. Why is there no such guarantee?

If there exists an object wit that name the newly created object will probably get the name CutViewCutZ001. Unlikely, but possible.

[wmayer]

I am however still reluctant to keep the combination clipping/cutting in one dialog

Technically they are too different and thus doesn't make much sense to combine them in a common dialog.

It has 20.8 MB and consists of 17 parts.

So, it's not that huge. When Jean-Marie improved the STEP importer a few years ago he posted a link to a 100MB STEP file.

Why is there no such guarantee?

Because it's not possible. The internal name of an object must be unique and with this snippet two objects are created:

Code: Select all

auto CutFeature1 = doc->addObject("Part::Cut", "CutViewCutZ");
auto CutFeature2 = doc->addObject("Part::Cut", "CutViewCutZ");

So, at least the second object must have a different name than passed to addObject.

Why can the name be different?

Because the user can have created an object with this name before opening the dialog. Although quite unlikely it's not impossible that it happens and robust code must consider such eventualities.

With

if (doc->getObject("CutViewCompound")) {
auto compoundObject = doc->getObject("CutViewCompound");

you know that this object exists but you have no guarantee of what type it is. It's the same as above that the user can create an object with this name that is not a Part::Compound.

Then an additional remark: do not hard-code the object name over and over again because once the name will be changed you have to do it everywhere and in case you forget it or by accident change it incorrectly the code won't work correctly any more. So, as said above safe the internal name of the returned object of addObject to a std::string and use this string in all other cases.

But instead of using a std::string a more elegant way is to use a DocumentObjectWeakPtrT instead. This has an additional benefit that the access to the object is much faster because internally it keeps a pointer. This class is also able to handle the case that the object has been deleted in the meantime which can be checked with the method expired().

What should I do instead other than catch the cast exception?

A static_cast never throws an exception but in case it's invalid returns a dangling pointer. So either double-check with the object's getTypeId() function or directly use a dynamic_cast. The latter returns a null pointer if the cast cannot be performed.

I cannot use a pointer because one of the features is that you can open a file with an existing cut and the cutting feature removes/refreshes it when you invoke a cut operation. Since the objects existed before the Cutting dialog was opened, there is no pointer available.

Sorry, but I didn't get the point.

But I don't want all types that are derived (maybe in future) from App::Part, only the App::Parts.

But then still use App::Part::getClassTypeId() instead of Base::Type::fromName("App::Part") because then the compiler can assist in case the class name is incorrect.

The point is that I can only take links from App::Part objects when I know it is an assembly 4 object. Otherwise I must take all links from all App::Parts then subsequently filter out of the links link to objects that can be cut. This involves a lot of code to be run.

As soon as the author of Assembly4 changes the name your code will break. It's in general bad programming style to rely on any implementation details as this makes your code quite fragile.

OK, but what can I do? I already asked in the forum and anly got the reply to use Python's "hasattr(Shape)" but this method is the same I did.

The whole functionality is already very focused on Part::Features. So why a less restriction in this case? Check if the object is a Part::Feature do a cast to this type and directly access its Shape property.

[M4x]

I can't say anything about how this should be integrated into the FreeCAD software architecture / structure but here is what I think from a user perspective:

First of all: Thank you! This behavior is exactly what I've expected to see the first time I've used the clipping plane feature. I'm very happy to see that someone is working on it. I'm confident that you'll sort out the "details" regarding the correct implementation (I know that these are quiet big "details" ). The discussion is going on, you'll receive valuable information regarding the software part and the user part.

I like the sliders too and I'd also like to have something to "drag" directly in the 3D view. Something that looks like FEM_ClippingPlaneAdd and/or Std_Transform. I think that's what you've in mind too.

[...]
However, in any case support for custom cuts can and even should be done as next step.
[...]

The sliders could follow. This way it would be easy to "read" the exact position and directly access it again as a user. Or maybe we could safe it as some kind of object in the tree and access / activate it again? That could be especially useful if you're discussing something and have to switch back and forth.

Volumetric rendering and boolean cut - best of both worlds?
Regarding the use of more suitable ways to tackle this problem (volumetric rendering), I've a proposal which could satisfy what wmayer said

[...]
For volumetric rendering you usually use OpenGL techniques like the stencil buffer: https://tech.metail.com/the-stencil-buf ... rsections/
Coin already provides a library for this purpose https://github.com/coin3d/simvoleon and it's worth to have a look at it.

and your goal to be able to really cut objects and refer to them for drawings and stuff like that (which I'd like very much too!).

I'm thinking about a behavior like what you've shown us but using what wmayer has provided = fast section cuts which are using less ressources and additionally a button to make a cut if needed. So the relatively resource hungry boolean cut is only applied if you really need it.

• Go through your file (assembly for example) using OpenGL/Coin library for visual checks, presentations and so on
• Make a boolean cut if you'd like to work with the cutted object (drawings, brochures and so on). The information regarding where the cut should be made / what should be cutted should match what you're seeing in the 3D view using the OpenGL/Coin library of course.

What do you think? I'm looking forward to following this thread.

[uwestoehr]

Technically they are too different and thus doesn't make much sense to combine them in a common dialog.

OK.

Code: Select all

auto CutFeature1 = doc->addObject("Part::Cut", "CutViewCutZ");
auto CutFeature2 = doc->addObject("Part::Cut", "CutViewCutZ");

So, at least the second object must have a different name than passed to addObject.

Yes, but it is me who adds the "CutViewCutZ" object. I already catched the case that it existed before the dialog was opened, because this is a typical use case - you can start with a file having already a cut. So at the beginning of the routine I delete the object "CutViewCutZ" if it exists.
My plan was that when the user wants to keep the existing cut and make a new one, he must rename the one that should be kept in advance. I will document this of course.

Code: Select all

if (doc->getObject("CutViewCompound")) {
        auto compoundObject = doc->getObject("CutViewCompound");

you know that this object exists but you have no guarantee of what type it is. It's the same as above that the user can create an object with this name that is not a Part::Compound.

As for the cuts, also the "CutViewCompound" is deleted when the new cut is created. So I can be sure there is no such object.

But instead of using a std::string a more elegant way is to use a DocumentObjectWeakPtrT instead.

Yes, and I was wrong in my last post. Since I create these objects new, I can set a pointer to them.

A static_cast never throws an exception but in case it's invalid returns a dangling pointer. So either double-check with the object's getTypeId() function or directly use a dynamic_cast. The latter returns a null pointer if the cast cannot be performed.

Thanks, Another beginner thing - I did not know that a static cast does not throw an exception.

But then still use App::Part::getClassTypeId() instead of Base::Type::fromName("App::Part") because then the compiler can assist in case the class name is incorrect.

I will do so.

As soon as the author of Assembly4 changes the name your code will break. It's in general bad programming style to rely on any implementation details as this makes your code quite fragile.

I know. but it is really hard to check otherwise if the links I collect for the compound link to cuttable objects. So isn't it OK to start with this for now? I mean when Zolko changes the App::Part name, then no links will be collected anymore. This is better than collecting uncuttable links. So then the cutting feature would no longer work for assembly 4 files, but for all others.
But of course I'll try to find a solution.

The whole functionality is already very focused on Part::Features. So why a less restriction in this case? Check if the object is a Part::Feature do a cast to this type and directly access its Shape property.

In principle yes, I need to re-think if App::Part could be an exception.

[uwestoehr]

fast section cuts which are using less resources and additionally a button to make a cut if needed. So the relatively resource hungry boolean cut is only applied if you really need it.

That would be helpful. Personally I find it important to store the section view somehow. Also if you don't need a cut object to modify this later on, you often need to safe different cut views. I used this in Solidworks a lot and it was very helpful when presenting the work.

For example I find FEM's clip feature amazing (should become part of Std_Clipping in my opinion) but it is not persistent. Assume you found a nice cut to demonstrate a certain feature/topic etc but when I reopen FC the next day I cannot get it back.

I can predict that I won't find time to learn Coin soon. Since I do more and more FC stuff at work, I try to improve the things I need the most for real-life files. Thus I have other things in the pipeline to be done besides the cut feature: finish the hole dialog overhaul, add a radius measurement to Part's measure toolbar etc.

[uwestoehr]

I think the easiest is to move your additions to the PartGui module. Then fix the issues I described in my second post.

Here it is: https://github.com/FreeCAD/FreeCAD/pull/4118

I think I ironed out all issues so far.
(Note that I delete at first all objects with the names with which I recreate the objects, so I can be sure the created objects will get the name I give them.)
Since the cutting feature is designed to be used for assemblies too (there it becomes really useful for me), it belongs to different WBs, thus I listed it in the main "View" menu.
The cutting feature is designed to stay open as a side bar to cut different documents if you like/need. So it is purposely no task dialog.

I need a further review because there is at least one issue I cannot resolve:
- while the cutting dialog is open, the user is free to delete an object. Therefore I need to check that. My attempt is in line 262 of cutting.cpp:

Code: Select all

(*it)->isValid()

But despite (*it) points to an object that is no longer part of the file, isValid() gives me 'true'.
What is the/a proper way to check if an App::DocumentObject* still points to an existing object?

p.s. I tried my best which doesn't mean that's enough