[bug] Cant add reference constraint to already defined circle/arc

Post here for help on using FreeCAD's graphical user interface (GUI).
Forum rules
and Helpful information
IMPORTANT: Please click here and read this first, before asking for help

Also, be nice to others! Read the FreeCAD code of conduct!
openBrain
Veteran
Posts: 9041
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by openBrain »

abdullah wrote: Fri Jun 04, 2021 5:55 am I am unhappy with myself that it took so long for me to merge this PR. However, my comment is not to be construed as a critic to the code or the functionality, rather it reflects my lack of own confidence when reviewing a commit after several months without coding and reviewing. This generally auto-corrects as I start coding and reviewing again. ;)
I know. Welcome back. ;)
That's all right. I had the tool locked to diameter (you know this "new" feature that remembers what you last used, radius or diameter). No surprise I could not see the bug fixed. Baby steps is ok.

Now that I know why you were removing the old behaviour and inverting the order of the equality and radius constraints, merging the diameter one should be faster.
PR for fixing diameter as well is available here
chrisb
Veteran
Posts: 54288
Joined: Tue Mar 17, 2015 9:14 am

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by chrisb »

Thank you, guys!
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by abdullah »

openBrain wrote: Fri Jun 04, 2021 8:53 am PR for fixing diameter as well is available here
Merged! Thanks!
openBrain
Veteran
Posts: 9041
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by openBrain »

abdullah wrote: Sun Jun 06, 2021 5:23 am Merged! Thanks!
Great thanks. I will mention this in the release notes.
What would you think to tag the 2 commits to be backported in 0.19 branch?

I'll make another post later on about some fine tuning we may do.
openBrain
Veteran
Posts: 9041
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by openBrain »

OK, now it's merged (and hopefully right :) ), some questions arose while tweaking this :

1) Is the behavior when both external and internal (real + construction) geometries are selected right ? Actually, it will set each external with a reference radius/diameter, then makes internal geometries equal and set a driving constraint on the first selected one. It's the best it can do if it has to do something, but should we keep it or just reject the selection and warn user ?

Some other not directly related questions :
2) Should we offer the same multi-selection ability for lines with horizontal/vertical/natural dimension ?
3) What would you think of a feature that will automatically constraint with diameter on complete circles, and radius on arcs -- I think it's some kind of good practice for dimensioning -- ?
4) For some geometrical constraints (equality, vertical, horizontal, parallel, ...), it's possible to select multiple geometries. In this case, sketcher creates chained constraint (geo1-cons-geo2, then geo2-cons-geo3, then geo3-cons-geo4, and so on...). What would you think of an option that user can choose a to-first mode instead (geo1-cons-geo2, geo1-cons-geo3, geo1-cons-geo4, and so on...) ?

Thoughts ?
chrisb
Veteran
Posts: 54288
Joined: Tue Mar 17, 2015 9:14 am

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by chrisb »

openBrain wrote: Sun Jun 06, 2021 5:58 pm) Should we offer the same multi-selection ability for lines with horizontal/vertical/natural dimension
I don’t use this often but for the sake of consistency it seems sensible
3) What would you think of a feature that will automatically constraint with diameter on complete circles, and radius on arcs -- I think it's some kind of good practice for dimensioning -- ?
Very good idea. It could be a third entry „auto“ in the drop down menu. For B-splines control points it would be always radius.
4) For some geometrical constraints (equality, vertical, horizontal, parallel, ...), it's possible to select multiple geometries. In this case, sketcher creates chained constraint (geo1-cons-geo2, then geo2-cons-geo3, then geo3-cons-geo4, and so on...). What would you think of an option that user can choose a to-first mode instead (geo1-cons-geo2, geo1-cons-geo3, geo1-cons-geo4, and so on...) ?
No objection, but am afraid that I don’t need it.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
openBrain
Veteran
Posts: 9041
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by openBrain »

chrisb wrote: Sun Jun 06, 2021 7:41 pm Very good idea. It could be a third entry „auto“ in the drop down menu. For B-splines control points it would be always radius.
Only one to reply, first to be served. :lol:
I was also thinking to a third entry. B-splines aren't even radius, they always are Weight (unitless). ;)
Pull request is here => https://github.com/FreeCAD/FreeCAD/pull/4855
The tool supports multi-selection exactly in the way radius & diameter do.
Radiam.gif
Radiam.gif (366.47 KiB) Viewed 1506 times
One thing that we probably have to look (no urgency) is that Sketcher currently has an option "Use diameter as default constraint for arcs & circles" which is a boolean (disabled -> default radius ; enabled -> default diameter).
it can't take into account the new tool. I see several solutions :
1) Make this a list with the 3 tools where one can choose its default
2) Remove this setting and make the new tool to be the default
3) Remove this setting and instead remember the last used tool for next session
4) Other that I didn't think about
Thought ?
chrisb
Veteran
Posts: 54288
Joined: Tue Mar 17, 2015 9:14 am

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by chrisb »

Great addition. For me all three options are an improvement. For keeping as much as possible in its current state I would go for option 1 or 3.

You are right with the B-spline control points, they have a weight. What I meant was, that currently this weight can only be changed if the radius variant is chosen and not the diameter.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by abdullah »

openBrain wrote: Fri Jun 11, 2021 2:55 pm One thing that we probably have to look (no urgency) is that Sketcher currently has an option "Use diameter as default constraint for arcs & circles" which is a boolean (disabled -> default radius ; enabled -> default diameter).
it can't take into account the new tool. I see several solutions :
1) Make this a list with the 3 tools where one can choose its default
2) Remove this setting and make the new tool to be the default
3) Remove this setting and instead remember the last used tool for next session
4) Other that I didn't think about
Thought ?
I like it. Merged. Thanks. :D

1 or 3 is ok with me. In addition, you may make the new tool the initial default (before the new parameter has a given value).
openBrain
Veteran
Posts: 9041
Joined: Fri Nov 09, 2018 5:38 pm
Contact:

Re: [bug] Cant add reference constraint to already defined circle/arc

Post by openBrain »

chrisb wrote: Fri Jun 11, 2021 10:13 pm Great addition. For me all three options are an improvement. For keeping as much as possible in its current state I would go for option 1 or 3.
abdullah wrote: Sat Jun 12, 2021 5:14 am I like it. Merged. Thanks. :D

1 or 3 is ok with me. In addition, you may make the new tool the initial default (before the new parameter has a given value).
Thx guys. I implemented option 3. ;) Pull request is here along with a bugfix for the previous PR => https://github.com/FreeCAD/FreeCAD/pull/4861
Post Reply