[merged] PR #5357 - add feature to create tapered Pads / Pockets

Post here if you have re-based and finalised code to integrate into master, which was discussed, agreed to and tested in other forums. You can also submit your PR directly on github.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
User avatar
-alex-
Veteran
Posts: 1856
Joined: Wed Feb 13, 2019 9:42 pm
Location: France

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by -alex- »

realthunder wrote: Mon Jan 10, 2022 4:14 am I got the edge part working, but not the face, although it seems to be enough in many use cases, such as the sheet workbench.
No it is not, unforyunately unfold algorithm expects both linear edges (cylindrical bends) and planar faces.
Change the nature of geometric elements is not a good behavior. IMO it's too bad that OCC handles elements this way when it is not necessary (or maybe FC is the culprit?).
Such change can break sketches mapping, Xref. etc...
Hope ones could find a way to fully linearize such tapered features or help @realthunder to improve his code.
Thanks for your attention.
I stop now to not hijack this thread maybe.
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by uwestoehr »

realthunder wrote: Mon Jan 10, 2022 4:14 am I have actually implemented this taper feature with support of inner wires on both Part and PartDesign. The code is in Part::Extrusion::makeDraft().

Thanks. But why did you do it this way? I like the way PD loft does this and you did not change the basic build method used by part Extrude and PD Loft - building shells through the wires. To cut out the inner sections, the PartWB has the Part::FaceMakerCheese::makeFace() method. Loft puts in there al wires of a section and the method does the rest (cutouts) for you.

My approach is to change as little as necessary since I fear side-effects. Methods like Part::FaceMakerCheese::makeFace() are apparently successfully used for a long time therefore I would only go away from this when there are problems.

Regarding the B-Spline edge/face generated by the loft operation, I need to understand this. looking in your code you did not change the way the shells are build. So I don't see a difference with Extrude and Loft since bot use the same OCC method.

What exactly is the problem with the PD Loft code? I use it in almost all my real-life documents, 3D-pring these parts, make technical drawings, generate DXF with laser cutting paths etc. I did not yet any problem with lofts. B-Splines are state of the art since years. All §D-slicing software I know can handle this, same as for the laser cutters, milling machines etc.
User avatar
-alex-
Veteran
Posts: 1856
Joined: Wed Feb 13, 2019 9:42 pm
Location: France

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by -alex- »

uwestoehr wrote: Mon Jan 10, 2022 2:23 pm All §D-slicing software I know can handle this, same as for the laser cutters, milling machines etc.
Sure, there is no problem for 3D printing, with any tesselated workflow neither.
But with B-rep workflows it can be an issue sometimes.
For example the unfold algorithm of Sheetmetal WB doesn't handle B-spline shapes.
Another issue is not related to B-spline itself, but related to the change of elements nature, from linear to B-spline when you modify the geometry of a Loft, or sweep.
See my previous link for examples.
This behavior involves some instability (this is not TNP), map mod failure, Xref. failure, because previous planar faces and linear edges become sometimes B-splines, even for non-twisted shapes I mean.
Flat faces should be planar faces, not B-spline.
The same for linear edges.
IMO this behavior is not acceptable for a CAD software.
B-splines should be used only for twisted shapes, not linear ones.
You can use the information tool of Curves WB to check then see the instability.
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by adrianinsaval »

how does this feature behave with custom directions? no issue? Are the shapes generated by PartDesign_Draft bsplines too?
chrisb
Veteran
Posts: 53930
Joined: Tue Mar 17, 2015 9:14 am

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by chrisb »

adrianinsaval wrote: Mon Jan 10, 2022 5:19 pm how does this feature behave with custom directions? no issue? Are the shapes generated by PartDesign_Draft bsplines too?
I pointed this out in the parallel topic (should I merge these??):
https://forum.freecadweb.org/viewtopic. ... 87#p560413
https://forum.freecadweb.org/viewtopic. ... 87#p560487

With PartDesign Draft we have planes and cones. The faces are as simple as can be, if we can agree that plane faces or cones are simpler than BSplineCurves.
I agree with alex, that we shouldn't use BSplineCurves, if planes suffice.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by uwestoehr »

chrisb wrote: Mon Jan 10, 2022 6:43 pm I pointed this out in the parallel topic (should I merge these??):
This thread is about the PR and this PR will not get support for inner wires. Inner wires will be stage 2 thus a separate thread for this.
realthunder
Veteran
Posts: 2190
Joined: Tue Jan 03, 2017 10:55 am

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by realthunder »

uwestoehr wrote: Mon Jan 10, 2022 2:23 pm Thanks. But why did you do it this way? I like the way PD loft does this and you did not change the basic build method used by part Extrude and PD Loft - building shells through the wires. To cut out the inner sections, the PartWB has the Part::FaceMakerCheese::makeFace() method. Loft puts in there al wires of a section and the method does the rest (cutouts) for you.
I am not sure if I get what you mean here. The current upstream Part::Extrusion::makeDraft() simply discards the inner wire. I modified it to split out the inner ones and optionally apply another taper angle on them. It offers user more options. The user may not want to touch inner holes, or he/she may want a different draft angle for the holes, while makeOffset() (necessary if not splitting the wires and use face directly) does not allow that.

Regarding the B-Spline edge/face generated by the loft operation, I need to understand this. looking in your code you did not change the way the shells are build. So I don't see a difference with Extrude and Loft since bot use the same OCC method.
Yes, it is an additional call to TopoShape::linearize() to try to replace BSpline geometry after the draft shape is produced. It's a separable step just like 'Refine'.
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
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by uwestoehr »

realthunder wrote: Tue Jan 11, 2022 1:32 am I am not sure if I get what you mean here. The current upstream Part::Extrusion::makeDraft() simply discards the inner wire...
That is clear. I asked why you don't use the Part::FaceMakerCheese::makeFace() method to handle the inner wires. This method is used by PD loft. Therefore we can treat this method as being stable and I thought we should use existing methods preferably than to create new ones.
So the question is what drawback of Part::FaceMakerCheese::makeFace() do you see that you haven't used it for your rewrite?

Another, more general question: Your work is absolutely amazing, but I don't see discussions about this nor PRs. Therefore I cannot follow a discussion, join the development or learn things. For example for the tapered padding, this was often requested by users but I never saw a PR. What was the reason?
Now I spend a lot of time to create a PR and we discuss how it can be implemented. But in effect, you already did some work. Can you in future please make directly a PR and start discussions? We have now more mergers and manpower to handle them. I know that debates need time and take energy but in the end we benefit from the different user application input and thus can setup solutions that suit the most.
User avatar
adrianinsaval
Veteran
Posts: 5541
Joined: Thu Apr 05, 2018 5:15 pm

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by adrianinsaval »

Yeah I would love to see PRs for the other smaller stuff. I tried rt's branch a few days ago and the navi cube was looking better than master, would be nice to have that PR'd and I think it would be much easier to merge than the TN stuff. And the UI stuff is just awesome. I think the lack of PRs comes from the initial plan to give the TN stuff priority and then start merging the rest afterwards, of course this never happened and many of rt's PRs stalled.
drmacro
Veteran
Posts: 8868
Joined: Sun Mar 02, 2014 4:35 pm

Re: PR #5357 - add feature to create tapered Pads / Pockets

Post by drmacro »

adrianinsaval wrote: Thu Jan 13, 2022 12:38 pm Yeah I would love to see PRs for the other smaller stuff. I tried rt's branch a few days ago and the navi cube was looking better than master, would be nice to have that PR'd and I think it would be much easier to merge than the TN stuff. And the UI stuff is just awesome. I think the lack of PRs comes from the initial plan to give the TN stuff priority and then start merging the rest afterwards, of course this never happened and many of rt's PRs stalled.
The UI stuff being awesome is personal opinion. There are definitely some functionality that I think would be nice to get into master. But, I find no particular UI differences that appealing.

I'm not convinced, about the priority of the TNP mitigation over other things. The rt branch has a working model that has been used extensively by many people worldwide. And that model has been merged master>rt a few times since the initial PR last April. So, why does it take 10 months and counting to go the other way?

And, "ooh it's being worked on in secret back room", really? Umm...secret open source...oxymoron maybe? For 10 months.

Sorry, just me being skeptical. :mrgreen:
Star Trek II: The Wrath of Khan: Spock: "...His pattern indicates two-dimensional thinking."
Post Reply