Unexpected Chamfer behaviour when face selected

About the development of the Part Design module/workbench. PLEASE DO NOT POST HELP REQUESTS HERE!
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Unexpected Chamfer behaviour when face selected

Post by openBrain »

troyp76 wrote: Mon Sep 13, 2021 3:48 pm Finding my way around the code and now have an initial concept working. :D

Still a lot to do but it's a start.
Looks like a great start. Feel free to push a draft PR when ready so you can get early feedbacks. ;)
troyp76
Posts: 32
Joined: Thu May 06, 2021 8:05 am

Re: Unexpected Chamfer behaviour when face selected

Post by troyp76 »

Draft Pull Request created, appreciate any feedback.
https://github.com/FreeCAD/FreeCAD/pull/5029

Tested the change against a number of different shapes, alphabet letters, selection orders etc.
So far it has performed as I expected or the issues that I did see seem to be related to the underlying chamfer function itself.
(similar issues happen using an unmodified 0.19.2)
003424.png
003424.png (43.07 KiB) Viewed 4586 times
openBrain
Veteran
Posts: 9031
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: Unexpected Chamfer behaviour when face selected

Post by openBrain »

troyp76 wrote: Tue Sep 14, 2021 2:47 pm Draft Pull Request created, appreciate any feedback.
https://github.com/FreeCAD/FreeCAD/pull/5029
I had a quick look and this looks pretty fine to me. We'll try to have a deeper look at it later.
As we now have C++11 as a minimum requirement, could you use range-for loops in the code to make it easier to read ?
troyp76
Posts: 32
Joined: Thu May 06, 2021 8:05 am

Re: Unexpected Chamfer behaviour when face selected

Post by troyp76 »

openBrain wrote: Wed Sep 15, 2021 12:52 pm As we now have C++11 as a minimum requirement, could you use range-for loops in the code to make it easier to read ?
Done.
troyp76
Posts: 32
Joined: Thu May 06, 2021 8:05 am

Re: Unexpected Chamfer behaviour when face selected

Post by troyp76 »

A new PR with clean commit history has been submitted.
https://github.com/FreeCAD/FreeCAD/pull/5039

The following picture shows further testing by selecting only the top faces of the letters/numbers and then applying a distance angle chamfer.
It demonstrates that the changes work as expected over a range of face geometries.

Test file can be found here:
https://github.com/troyp76/FreeCAD_test ... %20Chamfer

Screenshot 2021-09-18 011602.png
Screenshot 2021-09-18 011602.png (217.91 KiB) Viewed 4306 times
User avatar
chennes
Veteran
Posts: 3868
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: Inconsistent Chamfer behaviour when face selected

Post by chennes »

GeneFC wrote: Thu Sep 09, 2021 3:34 pm If you are proposing a PR then it is important to thoroughly test this on various cases.
@GeneFC, do you have time to test this PR? I tried it on some strange U shapes with random holes in them and that sort of thing, but maybe you can come up with some more edge cases that it might break.
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
GeneFC
Veteran
Posts: 5373
Joined: Sat Mar 19, 2016 3:36 pm
Location: Punta Gorda, FL

Re: Unexpected Chamfer behaviour when face selected

Post by GeneFC »

I compiled and tested the new chamfer stuff. Seems to work fine.

Ironically when I was comparing to the existing version I discovered an error I had not seen previously. When I made a simple block with a hole though a face, but with the hole not touching any edges, I saw an angle-flipping error. Three sides of the face chamfered one way and the fourth side chamfered the opposite. I expected and saw that sort of error for a u-shaped cutout.

Capture1.PNG
Capture1.PNG (22.58 KiB) Viewed 4225 times
Capture2.PNG
Capture2.PNG (23.29 KiB) Viewed 4225 times
Capture3.PNG
Capture3.PNG (23.72 KiB) Viewed 4225 times

I saw similar sorts of quirks with the Chamfertest.FCStd file, but I did not see any problems with the Alphabet file.

Again, I saw zero problems with the new version.

Gene
Post Reply