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

Sketcher: Chamfer tool (and fillet refactor) #12898

Merged
merged 2 commits into from Mar 18, 2024

Conversation

PaddleStroke
Copy link
Contributor

@PaddleStroke PaddleStroke commented Mar 12, 2024

@github-actions github-actions bot added Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD WB Sketcher Related to the Sketcher Workbench labels Mar 12, 2024
@FEA-eng
Copy link
Contributor

FEA-eng commented Mar 12, 2024

  • Introduce Chamfer tool.

Awesome to see this pretty much last major missing tool in the sketcher being added. Can I help with testing or is it too early as the PR is still in draft?

@PaddleStroke
Copy link
Contributor Author

If you can test it would be great. It's already finished.

@PaddleStroke PaddleStroke marked this pull request as ready for review March 12, 2024 13:07
@FEA-eng
Copy link
Contributor

FEA-eng commented Mar 12, 2024

Very good decision to turn the separate corner-preserving fillet tool into a checkbox. The new chamfer tool works very well too. Here are some minor comments:

  1. Fillet and chamfer are separate tools but when one of them is activated, the mode can be changed to the other tool. Doesn't it mean that they should be unified into a single fillet/chamfer tool?
  2. The preserve corner checkbox is shown also for the chamfer mode but it has no use there, right? Moreover, it becomes unchecked automatically after the first use of the tool but it seems to be still in effect.
  3. Do you plan to add the on-view parameters for those tools so that the user is asked for the fillet radius or chamfer size before the feature is created?

@PaddleStroke
Copy link
Contributor Author

PaddleStroke commented Mar 12, 2024

  1. I think it's best to keep them separate as they are usually separate tools in other cads.
  2. Preserve corner works with chamfer as well. This needs to be fixed.
  3. It's hard to implement OVP for fillet because of how the internal works. It would need much more work and I'm kind of swamped right now so if it comes it will be later.

@FEA-eng
Copy link
Contributor

FEA-eng commented Mar 12, 2024

  1. I think it's best to keep them separate as they are usually separate tools in other cads.

Sure, I don't mind it at all. I just thought that it was a bit different than the other tools but then I realized that it's already like this with some shape (circle/rectangle) drawing tools so never mind. Let's keep them separated.

  1. It's hard to implement OVP for fillet because of how the internal works. It would need much more work and I'm kind of swamped right now so if it comes it will be later.

Sure, it's nothing urgent. Just a minor suggestion for the future.

@maxwxyz
Copy link
Collaborator

maxwxyz commented Mar 13, 2024

Nice to see this! A few questions:

  • Is preserve corners checked by default?
  • Does it work with 2 edges (lines/conics) and also just with 1 point?
  • Does it work with a preselection?
  • If OVP is too much work for now, would a radius constraint be feasible or should this be postponed to later as well?

@PaddleStroke
Copy link
Contributor Author

  • yes
  • yes
  • No, as fillet current doesn't.
  • You mean create a radius constraint automatically? But then it can be annoying, say if you want to make 5 fillet and make them equal. If it creates a radius on every use it's bad

@maxwxyz
Copy link
Collaborator

maxwxyz commented Mar 13, 2024

OK thanks, that's good.

@PaddleStroke
Copy link
Contributor Author

Small issue with the checkbox reseting itself to false fixed.

@FEA-eng
Copy link
Contributor

FEA-eng commented Mar 16, 2024

I tested this again, the only small issue is indeed gone and it all works as expected.

@WandererFan WandererFan merged commit b3fe5bb into FreeCAD:main Mar 18, 2024
9 checks passed
@PaddleStroke PaddleStroke deleted the chamfer branch May 2, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD WB Sketcher Related to the Sketcher Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Problem] Sketcher: No OVP/Widget for Fillet Tool [Feature Request] Add Chamfer Edge-Tool
4 participants