PR #3456 - Adding chamfer angle field to PartDesign

[armandas]

Hi everyone.

Contributing to FreeCAD for the first time. I was designing a part and found it frustrating that the chamfer feature was fixed at 45 degrees.

Well, it looks like adding an angle field was not too difficult, so I implemented it. Please let me know your comments.

https://github.com/FreeCAD/FreeCAD/pull/3456

P.S.

I would like to add distance-distance and length options, but I think I need to study the Qt first to be able to implement a nice UI. However even this feature alone is a huge improvement, IMO.

[abdullah]

Hi everyone.

Contributing to FreeCAD for the first time. I was designing a part and found it frustrating that the chamfer feature was fixed at 45 degrees.

Well, it looks like adding an angle field was not too difficult, so I implemented it. Please let me know your comments.

https://github.com/FreeCAD/FreeCAD/pull/3456

P.S.

I would like to add distance-distance and length options, but I think I need to study the Qt first to be able to implement a nice UI. However even this feature alone is a huge improvement, IMO.

That's the attitude!!

Code looks good. Compiles. Works as advertised. Loads legacy. Legacy loads the output (the only drawback being that angle defaults to 45 degrees upon recompute). Merged.

[adrianinsaval]

Great news! Thanks for your contribution, I hope you manage to implement distance-distance too

[adrianinsaval]

Some weird behavior:

Screenshot_20200510_170609.png (12.09 KiB) Viewed 3721 times

Code: Select all

Size.setUnit(Base::Unit::Angle);

In line 64 should probably be:

Code: Select all

Anlge.setUnit(Base::Unit::Angle);

Additionally, I was wondering if the 89.99 deg limit really should be there? I know 90 deg would fail for a 90 deg intersection, bu take this example:

Screenshot_20200510_171314.png (22.64 KiB) Viewed 3721 times

There's no reason why 90 deg or more should be dissallowed. Or this example:

Screenshot_20200510_171946.png (28.99 KiB) Viewed 3721 times

Which fails much earlier than 90 deg. Or is 90 deg the limit imposed by OCCT?

[armandas]

Thank you for your feedback, guys!

the only drawback being that angle defaults to 45 degrees upon recompute

I don't quite understand what this means, but if there is a solution, I would be happy to implement it.

In line 64 should probably be:

Code: Select all

Anlge.setUnit(Base::Unit::Angle);

Ah, the infamous copy-paste bug. I should have reviewed my code better...

Additionally, I was wondering if the 89.99 deg limit really should be there?

You are right. I was using Draft as a reference and they have this limit there. I suppose the logical limit is 180 degrees, but, like you said, it can fail before 90 degrees too, so maybe just set the limit to FLT_MAX and let OCC deal with it?

I will create another PR with the fixes.

[adrianinsaval]

I don't quite understand what this means, but if there is a solution, I would be happy to implement it.

It means if you save a file with a chamfer with a different angle and you open in 0.18 or early and recompute, freecad will use the 45 deg angle because its all it knows, that can't be fixed.

so maybe just set the limit to FLT_MAX and let OCC deal with it?

I think that is better, this way it will be the same wether it fails at 60 deg or 120 deg or whatever.

Since you're working with chamfer, would you mind adding variable length too? I think that is an option in part workbench.

[armandas]

Since you're working with chamfer, would you mind adding variable length too? I think that is an option in part workbench.

I'll look into the Part workbench code for inspiration.

On an unrelated note, it looks to me that what is called variable-length in FreeCAD is what I called distance-distance setting. I imagined variable length would look something like this:

[armandas]

I have created a draft PR with the fixes: https://github.com/FreeCAD/FreeCAD/pull/3460

Any comments would be much appreciated.

[abdullah]

Thank you for your feedback, guys!

the only drawback being that angle defaults to 45 degrees upon recompute

I don't quite understand what this means, but if there is a solution, I would be happy to implement it.

Adrian got it right.

What I meant is that if you open a file saved with your branch in an older version of FreeCAD, and the angle of the chamfer is not 45 degrees, on recompute it will be made 45 degrees. This is only logical because the older version does not have angled support. It is not fixable (other than backporting the commit). It is just ok.

In line 64 should probably be:

Code: Select all

Anlge.setUnit(Base::Unit::Angle);

Ah, the infamous copy-paste bug. I should have reviewed my code better...

I missed it too. However, this one is easy to fix.

You are right. I was using Draft as a reference and they have this limit there. I suppose the logical limit is 180 degrees, but, like you said, it can fail before 90 degrees too, so maybe just set the limit to FLT_MAX and let OCC deal with it?

I will create another PR with the fixes.

Edit box limitation shall be the maximum possible value. Whether OCC fails or not is a different problem. OCC may fail because it is requested an impossible champfer or because of the algorithm used fails.

Opencascade documentation does not set bounds for the angle parameter.

[abdullah]

I have created a draft PR with the fixes: https://github.com/FreeCAD/FreeCAD/pull/3460

Any comments would be much appreciated.

I am not sure FLT_MAX is the right option. The reason is there should be maximum limits for an angle. 3600 degrees does not sound reasonable (but it is accepted as input). If there is a hard limit, my opinion is that the input should be limited to this hard limit (but I am as always open to expert opinion ).

Sure higher than 90 degress (e.g. 100 degrees) is possible in some (acute) corners. But, I am not myself technically knowledgeable enough about champfers. It looks to me that 180 degrees should be the hard limit, but let's try to get somebody who knows...

...

Sorry to disturb, can a champfer have an angle higher than 180 degrees? If yes, what is the limit?