[Pull requested] 0002093: Additional check for over-constrained sketch (External geo and Coincidents)

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
triplus
Veteran
Posts: 9471
Joined: Mon Dec 12, 2011 4:45 pm

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by triplus »

sgrogan wrote:
triplus wrote: I see it's only affecting the production release. But i am still curious. If you remove that redundant vertical constraint do you get the x-axis back?
No. Saving the file. It doesn't comeback in release or current master. This is on Win.
EDIT: I new sketch the x-axis i back. In the original sketch the x-axis is gone forever. I'm crossing into OCD here sorry.
I tested production release from stable PPA and i can confirm the behaviour. I don't know when this got fixed in master. One way to get the axis back is to draw a line randomly and the axis will appear again. If i remember correctly something like that was discussed in discussion about improving the sketcher grid behaviour. For grid to be visible always in "full size" instead of following the sketcher geometry boundary each time it changes in size.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by abdullah »

triplus wrote: abdullah wrote:

triplus wrote:
How about when auto constraints mode is set to ON horizontal/vertical constraints aren’t added to the line if both points are added to horizontal/vertical coordinate system axis?



I do not understand. How can a line be horizontal or vertical and have both points in one axis? :shock:

Give me an example...:)

;)
Ah, ok. I was think of a horizontal line and vertical axis, or a vertical line and a horizontal axis. That makes much more sense. I will take a look...

I heard nothing from the functionality yet :( Is it behaving as expected?

If it is then I will:
1. Implement this way for the creation of vertical/horizontal constraint.
2. See how to handle Triplus' both points on axis case.
jmaustpc
Veteran
Posts: 11207
Joined: Tue Jul 26, 2011 6:28 am
Location: Australia

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by jmaustpc »

abdullah wrote:
jmaustpc wrote:3)when the hor/vert auto constraint is not going to be applied, the red mouse pointer icon still appears
Well I intended this, as it gives a good indication to the user that he is making a vertical/horizontal line. If I were to remove it, the user would probably miss it (and keep moving the mouse to get the horiz/vert icon). Additionally I do not think there is a feasible implementation to do that while dragging the line.
OK
abdullah wrote:I heard nothing from the functionality yet :( Is it behaving as expected?
Time zone difference, mate! :-) I have tested your update now and it works really well. This is a good improvement.

Jim
triplus
Veteran
Posts: 9471
Joined: Mon Dec 12, 2011 4:45 pm

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by triplus »

I heard nothing from the functionality yet :( Is it behaving as expected?
I had less time over the week to compile stuff but will do that today.
triplus
Veteran
Posts: 9471
Joined: Mon Dec 12, 2011 4:45 pm

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by triplus »

abdullah wrote:A line whose start and endpoints are coincident with external geometry can not get a vertical or horizontal constraint.

As a bonus (another commit) during geometry creation if a horizontal or vertical constraints are suggested and the endpoints belong to external geometry, the constrain is not created. This one was really annoying.
I did some tests and when line end points are created at external geometry end points horizontal/vertical constraint isn't and can't be added. I think this is how it should be yes as external geometry controls sketcher geometry in this use case and not the other way around.

Regarding the implementation i am not sure if pop-up should be introduced or could this be added to the Solver messages? Therefore to allow the user to add the constraint but to mark it as conflicting at the same time? But then again if the line is vertical and vertical constraint would be added solver probably should say redundant constraint? If this does not belong in Solver messages section maybe pop-up could be a bit more informative. For example renaming the Impossible constraint to Conflicting constraint. And in the description field to tell the user why it can't be added. For example: Vertical constraint can't be added as external geometry defines sketcher geometry.

Feel free to improve the message. This was quick example.

There are two more things to implement from perspective of the issue report and they are quite similar to "points on axis case":
  • Horizontal/vertical constraint should not be added when line end points are created with added Fix a point onto an object constraints on a (single) external geometry.
  • Horizontal/vertical constraint should not be added when combination of end point and Fix a point onto an object constraint is used on a (single) external geometry.
P.S. If the last part is not understood feel free to ask and i will attach images.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by abdullah »

jmaustpc wrote: abdullah wrote:
I heard nothing from the functionality yet :( Is it behaving as expected?



Time zone difference, mate! :-) I have tested your update now and it works really well. This is a good improvement.
triplus wrote:I had less time over the week to compile stuff but will do that today.
oh... this came out in a totally different way as I intended it. I did not mean to be pushy at all. I just intended to bring back the discussion to the functionality as opposed to the bug. Sorry :oops:
triplus wrote:I did some tests and when line end points are created at external geometry end points horizontal/vertical constraint isn't and can't be added. I think this is how it should be yes as external geometry controls sketcher geometry in this use case and not the other way around.

Regarding the implementation i am not sure if pop-up should be introduced or could this be added to the Solver messages? Therefore to allow the user to add the constraint but to mark it as conflicting at the same time? But then again if the line is vertical and vertical constraint would be added solver probably should say redundant constraint? If this does not belong in Solver messages section maybe pop-up could be a bit more informative. For example renaming the Impossible constraint to Conflicting constraint. And in the description field to tell the user why it can't be added. For example: Vertical constraint can't be added as external geometry defines sketcher geometry.

Feel free to improve the message. This was quick example.

There are two more things to implement from perspective of the issue report and they are quite similar to "points on axis case":

Horizontal/vertical constraint should not be added when line end points are created with added Fix a point onto an object constraints on a (single) external geometry.
Horizontal/vertical constraint should not be added when combination of end point and Fix a point onto an object constraint is used on a (single) external geometry.


P.S. If the last part is not understood feel free to ask and i will attach images.
I will keep it in mind for the next coding slot...
triplus
Veteran
Posts: 9471
Joined: Mon Dec 12, 2011 4:45 pm

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by triplus »

Actually after some additional thinking i do believe you solved the issue reported the best you could:
Issue.png
Issue.png (74.98 KiB) Viewed 973 times
  • User creates a sketch and adds sketcher geometry.
  • Pad it.
  • Add new Sketch with external and sketcher geometry.
  • Users goes back and changes the angle constraint.
Therefore this issue shouldn't happen anymore as you prevented the user to add horizontal/vertical constraint to such sketcher geometry constrained to external geometry datum points. I guess in the PartDesign NEXT when more linking will be allowed issues like that could be observed more often and hopefully this solution will reduce that.

As for my further suggestions i think they can't be done as it probably could work for lines but not for arcs:
UseCases.png
UseCases.png (28.99 KiB) Viewed 973 times
Preventing the user to add horizontal/vertical constraint in first use case could probably work fine but in second use case it must be allowed to add it. Therefore i am thinking you already solved the issue the best you could (limited to datum points) and that is probably it for now.

As for optimizing the behaviour and not adding redundant constraint by default:

Image

That would be nice and if possible focus should therefore go on this.

P.S. To avoid any confusion. On the first image (3 out of 5) vertical line with vertical constraint is missing.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by abdullah »

Thanks for the use cases. It really helps me understand what we are talking about.

I have also been thinking that... sometimes I wish to add that vertical constraint when dealing with external geometry. Well I will explain myself and please let me know if I could do it in another way.

My use case is the following:
1. I have part A finished which on a face looks like sketch M (assume for the sake of the discussion that M is rotated around the X,Y,Z axis so it is not vertical or horizontal).
2. I want to design part B which will sit on the face defined by the sketch M. Part B will have a face, N, substantially similar to that of Part A. As a simplicity, the attachment means will be a 3 mm bolt, so a hole is in both faces, M and N.
3. Because it is easy to design face N from the geometry of face (sketch) M, what I do is I create sketch N using face M as a substrate. I import external geometry, do the common hole and shapes. Then I overconstrain the sketch by adding constraints that would only be necessary if the external geometry would not exist. Then I delete the external geometry, thereby arriving to a fully constrained sketch (if I were good enough).
4. I may pad at this stage the sketch or not (irrelevant)
5. Then I hit "reorient sketch" for sketch N. It will ask me that it will detach it from the substrate, so I hit "ok". Then the reorientation dialog appears, but I do not really want to change the orientation, so I hit "cancel".

The new limitations to adding external geometry will interfere with this way of working.

If such a use case makes sense, a possibility would be, for example, to add a checkbox to the solver messages, just under "Auto-Update". Something named like "Prevent overconstraining", but nicer. This would create a new "modus".

If checked, the "logic" behind the sketch will try to prevent the user from adding constraints that would lead to overconstraining sketch (not claiming that it will not be possible to overconstrain it, as the possibilities for overconstraining are endless, only that the logic will prevent some situations, that could be extended at any time). This means autoconstrains and constrain addition will enable these checks (the functionality above) and effectively prevent the addition.

If not checked, the sketcher will allow the same as today.

I say "these functionalities", because with this implemented (in a more centralised fashion, with this functionality as a member of SketchObject), it may be feasible to implement the box selection coincident creation that we discussed some time ago and probably others.

Please, you guys that know much better than me the ways of using FC, please comment on this.
triplus
Veteran
Posts: 9471
Joined: Mon Dec 12, 2011 4:45 pm

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by triplus »

I think i do understand what you are trying to say but user can delete external geometry first and after add horizontal/vertical constraint to the sketcher geometry? Reorienting sketch will brake the external geometry anyway? Therefore current implementation doesn't prevent the end user to get to the end goal in your use case?

I am in general against implementing limitations to "help the end user" as usually they only get in a way but in this specific use case preventing the end user to add horizontal/vertical constraint to a line constrained to 2 external geometry points seems like a good idea to me. As that line can never control sketcher geometry in the first place by itself as the line itself is controlled by changes made to the external geometry? If valid use case would be found in the future where it would be needed to be able to add horizontal/vertical constraint i would vote for reverting the changes to current state.

As for current implementation it doesn't work reliably:
Issue.png
Issue.png (4.82 KiB) Viewed 925 times
2 external geometry lines + 2 sketcher geometry lines (fully constrained) and vertical constraint was allowed on sketcher geometry line.

P.S. Notice the sketch not reporting over-constrained sketch as that only happened after leaving and entering the edit mode again.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: [testing, user input] 0002093: Additional check for over-constrained sketch (external geometry)

Post by abdullah »

triplus wrote:I think i do understand what you are trying to say but user can delete external geometry first and after add horizontal/vertical constraint to the sketcher geometry? Reorienting sketch will brake the external geometry anyway? Therefore current implementation doesn't prevent the end user to get to the end goal in your use case?

If valid use case would be found in the future where it would be needed to be able to add horizontal/vertical constraint i would vote for reverting the changes to current state.

2 external geometry lines + 2 sketcher geometry lines (fully constrained) and vertical constraint was allowed on sketcher geometry line.

P.S. Notice the sketch not reporting over-constrained sketch as that only happened after leaving and entering the edit mode again.
Deleting before constraining runs the risk of the sketch moving.

Yes reorienting breaks external geo links (actually detaching from the substrate does)... if there is any left. That is why they must be deleted before detaching.

I find it very useful to have this prevention mechanism in 90% of the cases (it will speed me up). I think it will piss me off in the remaining 10%. That is why I was suggesting a modal behaviour with the setting very much at hand. I want have it generally on, byt be able to disable temporarely.

I guess that in your example, you added the contraint after geometry creation. The current branch only has the more reliable method implemented for autocontraints. This is temporary until we agree in the actual implementation. Then I will code it.
Post Reply