3D Surface - Face Selection (New feature) [Merged]

Here's the place for discussion related to CAM/CNC and the development of the Path module.
User avatar
freman
Posts: 1231
Joined: Tue Nov 27, 2018 10:30 pm

Re: 3D Surface - Face Selection (New feature)

Postby freman » Tue Feb 25, 2020 12:29 am

Sorry I was not clear. I did take down all three files Russ had under the images in the headline post.

You will get the same error if you are reprocessing a path you created earlier. I have noted this before working on dressups. Delete the path op and recreate it now you have the current 3D code. That should get more coherent results.
Russ4262
Posts: 490
Joined: Sat Jun 30, 2018 3:22 pm

Re: 3D Surface - Face Selection (New feature)

Postby Russ4262 » Tue Feb 25, 2020 3:57 pm

First off, Welcome to the forum, @jffmichi! I think you will find the forum to be a great resource for using FreeCAD.

As for your request,
jffmichi wrote:
Sun Feb 23, 2020 11:41 pm
... I would like to suggest the possibility to choose/invert the cutting/sampling direction. The possibility to choose between circular and lines is already great but I would like to be able to select something like "cirular inward" or "lines in x direction but starting with the line at y=10 and ending with the line at y=0". ...
.

This is done. New property added: `CutPatternReversed`. At the moment I don't think I have pushed this update to the PR. I also have fixed the code so that `CutMode` (climb / conventional) is carried out with fidelity. I might need to verify the Gcode output to ensure I tended to the Gcode side as well as the geometric side of the operation.

Thanks,
Russ
User avatar
freman
Posts: 1231
Joined: Tue Nov 27, 2018 10:30 pm

Re: 3D Surface - Face Selection (New feature)

Postby freman » Tue Feb 25, 2020 5:13 pm

Hi Russ.

taking my last uploaded model and setting Respect Boundary = false , I see some unnecessary drops machining the edge of the boundary it is supposed to be ignoring.

I can kinda see why this would be happening but maybe it could be avoided. If it is cutting air ( and presumably knows it is ) could this be optimised out?

Not a big issue but maybe something which can be improved in terms of machine time. Pointless ups and downs can be costly.

Also, I'm wondering what limits the range of the overshoot. It looks like >tool rad outside all machinable points which if great to ensure surface is fully cleared. One other reason I may want to run outside is to avoid all the lifting above safe space and back. Often not "cutting" can take longer than cutting, with all the fannying around. Maybe a prop to set an over-run greater than tool rad would be useful to improve machine time in some cases.

BTW this is not something I would want to machine using circular but it's just pushing the code around a bit to see what happens. It does look pretty solid so far. Nice work.
Attachments
slope-reverse-noboundary-view2.png
slope-reverse-noboundary-view2.png (5.77 KiB) Viewed 328 times
slope-reverse-noboundary.png
slope-reverse-noboundary.png (40.44 KiB) Viewed 328 times
Russ4262
Posts: 490
Joined: Sat Jun 30, 2018 3:22 pm

Re: 3D Surface - Face Selection (New feature)

Postby Russ4262 » Tue Feb 25, 2020 5:57 pm

freman wrote:
Mon Feb 24, 2020 5:37 pm
... I now set start depth= -10 ; finish depth=0 ; step down= -1 and the paths are done in reverse order. ...

chrisb wrote:
Mon Feb 24, 2020 10:47 pm
... a full recompute shows this in report view:

Code: Select all

...
Traceback (most recent call last):
  File "/Users/cb/Desktop/FreeCAD.app/Contents/Resources/Mod/Path/PathScripts/PathUtils.py", line 59, in new_function
    res = function(*args, **kwargs)
  File "/Users/cb/Desktop/FreeCAD.app/Contents/Resources/Mod/Path/PathScripts/PathOp.py", line 513, in execute
    result = self.opExecute(obj) # pylint: disable=assignment-from-no-return
  File "/Users/cb/Desktop/FreeCAD.app/Contents/Resources/Mod/Path/PathScripts/PathSurface.py", line 368, in opExecute
    final = self._planarDropCutOp(obj, stl, bb)
  File "/Users/cb/Desktop/FreeCAD.app/Contents/Resources/Mod/Path/PathScripts/PathSurface.py", line 438, in _planarDropCutOp
    dep_par = PathUtils.depth_params(obj.ClearanceHeight.Value, obj.SafeHeight.Value, obj.StartDepth.Value, obj.StepDown.Value, 0.0, obj.FinalDepth.Value)
  File "/Users/cb/Desktop/FreeCAD.app/Contents/Resources/Mod/Path/PathScripts/PathUtils.py", line 734, in __init__
    raise ValueError('z_finish_step must be less than step_down')
<class 'ValueError'>: z_finish_step must be less than step_down
4.1e-08 <App> Document.cpp(3464): Failed to recompute slope_reverse#Surface: z_finish_step must be less than step_down
Recompute failed! Please check report view.
freman wrote:
Mon Feb 24, 2020 5:31 pm
... This really needs to be in UI, but I'm sure Russ will take care of that when the basics are finished. ...

Gentlemen,
I do appreciate the creativity here. Please respectfully note a few things:
  • I do acknowledge that the two of you are power-users.
  • The error presented here is not 3D Surface related. It will occur in any other op with a `Finish Depth`, when the same data set is entered. Maybe the original author of the depth_params() function did not intend for bottom up machining. Or, perhaps he simply didn't have the time to include additional code to check for direction when checking that `Finish Depth` is less than `Step Down`.
  • A recent feature request to this effect by @jffmichi has been honored, reversing the cut pattern.
  • True, the error presented here could be presented in a milder manner, by wrapping the depth_params() call into a `try: except:` test case, with only the simple, "z_finish_step must be less than step_down" error phrase returned.
  • The UI editor window (duplicate data entry access) is secondary or tertiary priority in my book, when expanding functionality. Yes, it is important and deserves some attention, but I don't have a time frame for updating it. I would rather update the wiki with useful support and instruction on how to use the new features.
  • I am learning a lot from you all! Thanks for helping me help you.
chrisb
Posts: 28098
Joined: Tue Mar 17, 2015 9:14 am

Re: 3D Surface - Face Selection (New feature)

Postby chrisb » Tue Feb 25, 2020 7:07 pm

As fancy as it looked to me to mill upside down (or better downside up) the more I think about it, the more sense it makes. If the GCodes should ever be used for 3D printing, they should naturally start at the bottom.
Russ4262
Posts: 490
Joined: Sat Jun 30, 2018 3:22 pm

Re: 3D Surface - Face Selection (New feature)

Postby Russ4262 » Tue Feb 25, 2020 8:40 pm

Afternoon Freman,
Thanks for the testing and feedback.
freman wrote:
Tue Feb 25, 2020 5:13 pm
Hi Russ.

taking my last uploaded model and setting Respect Boundary = false , I see some unnecessary drops machining the edge of the boundary it is supposed to be ignoring.

I can kinda see why this would be happening but maybe it could be avoided. If it is cutting air ( and presumably knows it is ) could this be optimised out? ...
The answer falls into to categories depending on the geometry.
  1. Faces/regions that have a coplanar perimeter wire can be easily offset and handled as your inquiry suggests.
    If I understand you correctly, you would like the area beyond the face boundary to be treated like a milling op - the cutter would simply extend beyond the selected face, in a coplanar fashion, for the purpose of clearing all material at the edges.
  2. Other faces/regions that do not have a coplanar perimeter wire, for which 3D Surface was designed to tackle. In these cases the perimeter wire is not (with my skill set) easily offset to allow the cutter to cut at correct depth at any point beyond the boundary of the face/region.
freman wrote:
Tue Feb 25, 2020 5:13 pm
Also, I'm wondering what limits the range of the overshoot. It looks like >tool rad outside all machinable points which if great to ensure surface is fully cleared. One other reason I may want to run outside is to avoid all the lifting above safe space and back. Often not "cutting" can take longer than cutting, with all the fannying around. Maybe a prop to set an over-run greater than tool rad would be useful to improve machine time in some cases. ...
There is a `Boundary Adjustment` property. Positive pushes beyond the boundary, negative withdraws. Setting this to less than cutter diameter should remove the drops. Also, this property exists in other ops by a different name. I see `OffsetExtra` used. Perhaps I should adopt that name.

Answering your questions here makes me think I need to type up a list of new properties, along with their intended purposes as coded. Doing so would allow for some great feedback on terminology and standardization, as well as guide the purpose and effect of the feature. I will work on putting this together and posting it here. Such a list would roll over to the wiki fairly well.

Thanks,
Russ
User avatar
freman
Posts: 1231
Joined: Tue Nov 27, 2018 10:30 pm

Re: 3D Surface - Face Selection (New feature)

Postby freman » Tue Feb 25, 2020 8:45 pm

chrisb wrote:
Tue Feb 25, 2020 7:07 pm
As fancy as it looked to me to mill upside down (or better downside up) the more I think about it, the more sense it makes. If the GCodes should ever be used for 3D printing, they should naturally start at the bottom.
That's an interesting twist but if you allow that matrial removal techniques such as milling you'll get a nasty tool crash on the first pass. I would be wary of trying to combine two such different techniques in the same code. I think the two situations are too different, and with fundamentally conflicting needs, such that they need to be clearly separated. Mixing the two will lead to compromised choices which will be detrimental to both.

There may be scope for a 3D printer WB which uses modified versions of the PathWB tools. That may be relatively easy to do given the existing capabilities.
User avatar
freman
Posts: 1231
Joined: Tue Nov 27, 2018 10:30 pm

Re: 3D Surface - Face Selection (New feature)

Postby freman » Tue Feb 25, 2020 8:46 pm

Answering your questions here makes me think I need to type up a list of new properties, along with their intended purposes as coded. Doing so would allow for some great feedback on terminology and standardization, as well as guide the purpose and effect of the feature. I will work on putting this together and posting it here. Such a list would roll over to the wiki fairly well.
Yes, some doc would be helpful and possibly avoid silly questions and suggesting things that are already there but have not been discovered.
There is a `Boundary Adjustment` property. Positive pushes beyond the boundary, negative withdraws. Setting this to less than cutter diameter should remove the drops. Also, this property exists in other ops by a different name. I see `OffsetExtra` used. Perhaps I should adopt that name.
Boundary Adjustment is at least clearer about what it does. OffsetExtra does not say what is 'offset', nor what it does. For a long time I just skipped that feature because I could not work out what it was about and had too much else to learn and digest.

Some of these things seem to have been thrown a working name as a draft and it just got stuck there by inertia. Maybe there could be a discussion about adopting your terminology across all tools.
jffmichi
Posts: 3
Joined: Sun Feb 23, 2020 11:25 pm

Re: 3D Surface - Face Selection (New feature)

Postby jffmichi » Thu Feb 27, 2020 3:13 pm

@Russ4262,

thank you very much for the fast implementation of my feature request. Unfortunately I can't test it at the moment because I'm getting a "list index out of range" error. I compiled the "PathSurfaceWork" branch on Arch Linux a few minutes ago. (This is of course no complaint but rather an additional test case for your code).

Code: Select all

Traceback (most recent call last):
  File "/usr/lib/freecad/Mod/Path/PathScripts/PathUtils.py", line 59, in new_function
    res = function(*args, **kwargs)
  File "/usr/lib/freecad/Mod/Path/PathScripts/PathOp.py", line 513, in execute
    result = self.opExecute(obj) # pylint: disable=assignment-from-no-return
  File "/usr/lib/freecad/Mod/Path/PathScripts/PathSurface.py", line 341, in opExecute
    (faceShapesList, voidShapesList) = self._preProcessModel(JOB, obj)
  File "/usr/lib/freecad/Mod/Path/PathScripts/PathSurface.py", line 616, in _preProcessModel
    clsdAreas = self._extractFaceLoopFeatures(M, fcshp, faceIdx)  # returns all closed loops
  File "/usr/lib/freecad/Mod/Path/PathScripts/PathSurface.py", line 967, in _extractFaceLoopFeatures
    fcShp = self._wireToFlatCrsSctn(mBB, edge)
  File "/usr/lib/freecad/Mod/Path/PathScripts/PathSurface.py", line 1252, in _wireToFlatCrsSctn
    if not wire.Wires[0].isClosed():
<class 'IndexError'>: list index out of range
6e-08 <App> Document.cpp(3715): Failed to recompute Unnamed#Surface: list index out of range
Recompute failed! Please check report view.
Thank you for the great work,
jffmichi
Attachments
3dsurface-test-case.FCStd
(21.11 KiB) Downloaded 8 times
Russ4262
Posts: 490
Joined: Sat Jun 30, 2018 3:22 pm

Re: 3D Surface - Face Selection (New feature)

Postby Russ4262 » Thu Feb 27, 2020 9:39 pm

Good afternoon,
Thanks for the support.j
The bug you encountered here is one of the last barriers I have to remove to get this upgraded version of 3D Surface ready for serious consideration in the PR. The function mentioned in the code snippet you quoted (below) is the bottleneck that is behaving with great inconsistency. Today, I think I have identified the two best strategies, considering performance as a big factor, to implement in this problematic function.
jffmichi wrote:
Thu Feb 27, 2020 3:13 pm
@Russ4262,

thank you very much for the fast implementation of my feature request. Unfortunately I can't test it at the moment because I'm getting a "list index out of range" error. I compiled the "PathSurfaceWork" branch on Arch Linux a few minutes ago. (This is of course no complaint but rather an additional test case for your code).

Code: Select all

Traceback (most recent call last):
  ...
  File "/usr/lib/freecad/Mod/Path/PathScripts/PathSurface.py", line 1252, in _wireToFlatCrsSctn
    if not wire.Wires[0].isClosed():
<class 'IndexError'>: list index out of range
6e-08 <App> Document.cpp(3715): Failed to recompute Unnamed#Surface: list index out of range
Recompute failed! Please check report view.

Tests so far are yielding good results, but I need to test more. My first priority is to generate correct paths, my second is to make it run faster! ... Which I am doing.

A partial to-do list:
  • I need to verify I have reasonably sufficient error handling in place for logical use cases that would not produce a path.
  • I need to fix a reversed-direction artifact in the `Circular` based cut patterns that occurs sometimes when the path is broken to avoid a feature.
  • I need to check compatibility with your original file that had multiple models in the Job.
  • Fix a few other bugs found in testing with certain settings.
  • Spend some time with my wife and children.
The path in the screenshot took about 170 seconds to calculate. It looks good.

Russ
Attachments
MouleCabane3 - lower collective faces.png
MouleCabane3 - lower collective faces.png (224.83 KiB) Viewed 196 times