Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Path] Fix open edge zero value start point #5343

Merged
merged 1 commit into from Feb 17, 2022

Conversation

Russ4262
Copy link
Contributor

@Russ4262 Russ4262 commented Jan 6, 2022

forum mention at https://forum.freecadweb.org/viewtopic.php?f=15&t=64882
This fix applies a small value to start points for open edges when either the x or y start point value is roughly zero.

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, we ask you to conform to the following items. Pull requests which don't satisfy all the items below might be rejected. If you are in doubt with any of the items below, don't hesitate to ask for help in the FreeCAD forum!

  • Your pull request is confined strictly to a single module. That is, all the files changed by your pull request are either in App, Base, Gui or one of the Mod subfolders. If you need to make changes in several locations, make several pull requests and wait for the first one to be merged before submitting the next ones
  • In case your pull request does more than just fixing small bugs, make sure you discussed your ideas with other developers on the FreeCAD forum
  • Your branch is rebased on latest master git pull --rebase upstream master
  • All FreeCAD unit tests are confirmed to pass by running ./bin/FreeCAD --run-test 0
  • All commit messages are well-written ex: Fixes typo in Draft Move command text
  • Your pull request is well written and has a good description, and its title starts with the module name, ex: Draft: Fixed typos
  • Commit messages include issue #<id> or fixes #<id> where <id> is the FreeCAD bug tracker issue number in case a particular commit solves or is related to an existing issue on the tracker. Ex: Draft: fix typos - fixes #0004805

And please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 0.20 Changelog Forum Thread.


@github-actions github-actions bot added the WB CAM Related to the CAM/Path Workbench label Jan 6, 2022
@freecadci
Copy link

pipeline status for feature branch PR_5343. Pipeline 443190374 was triggered at 9c83410. All CI branches and pipelines.

@Russ4262 Russ4262 changed the title Path: Fix open edge zero value start point [Path] Fix open edge zero value start point Jan 7, 2022
@benj5378
Copy link
Contributor

The code looks fine. I haven't tested the functionality

idx = len(verts) - 1
x = verts[idx].X
y = verts[idx].Y
# Zero start value adjustments for Path.fromShapes() bug
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be mistaken but I'm not sure this is a bug in Path.fromShapes().
If you do a Path.show(pp, "pp") after line 290. You can see that fromShapes() return same shaped paths irregardless of the value of x.

If the bug was in fromShapes i would expect fromShapes to return different-shaped paths when x=0 and x=1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an image that hopefully clarifies what I try to say.
profile-results

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I grabbed a recent weekly build:
OS: Windows 10 Version 2009
Word size of FreeCAD: 64-bit
Version: 0.20.27428 (Git)
Build type: Release
Branch: master
Hash: 2746035
Python version: 3.8.6+
Qt version: 5.15.2
Coin version: 4.0.1
OCC version: 7.5.3
Locale: English/United States (en_US)

... and this is the result with what appears to be the same test file as you present above in the image.
Snip macro screenshot-f5e464

Applying the proposed fix in this PR corrects the path generated, as seen in the following image, and is applied in the following FreeCAD version:
OS: Windows 10 Version 2009
Word size of FreeCAD: 64-bit
Version: 0.20.27496 +2 (Git)
Build type: Release
Branch: fix/3D_pocket_overcut
Hash: f1a223e
Python version: 3.8.6+
Qt version: 5.15.2
Coin version: 4.0.1
OCC version: 7.5.3
Locale: English/United States (en_US)

Snip macro screenshot-2e9f48

I have not looked at code for Path.fromShapes() to determine if a bug exists there. As presented in the original test file by Lupin in the forum, the zero passed in the pathParams["start"] vector parameter are causing, or related to, the problem. This PR intervenes within the python to make those zeros a very small non-zero number, which seems to correct the issue within the gcode output.

Thanks for reviewing and collaborating on this bug.

Russell

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I could have been clearer. I was not doubting your fix, it works. I was only doubting that the bug is in fromShapes. But I'm not so sure anymore.

Looking closer at the output generated by fromShapes does indeed show that the x feed is omitted when the X=0 in the start param.
Rapids between layers with model x=0:

Command G0 [ Z:5 ],
Command G0 [ Y:13 ],
Command G0 [ Z:3 ]

Rapids between layers with model x=1:

Command G0 [ Z:5 ],
Command G0 [ X:1 Y:13 ],
Command G0 [ Z:3 ]

So long story short. Ignore what I said :)

forum mention at https://forum.freecadweb.org/viewtopic.php?f=15&t=64882
This fix applies a small value to start points for open edges when either the x or y start point value is roughly zero.
@freecadci
Copy link

pipeline status for feature branch PR_5343. Pipeline 470441361 was triggered at 9227b4a. All CI branches and pipelines.

@sliptonic sliptonic merged commit 74371fd into FreeCAD:master Feb 17, 2022
donovaly pushed a commit that referenced this pull request Feb 17, 2022
[Path] Fix open edge zero value start point
@Russ4262 Russ4262 deleted the fix/open_edge_zero_start branch March 30, 2022 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WB CAM Related to the CAM/Path Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants