[To be reworked] Sketcher Tool settings : testers welcome!

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!
User avatar
Kunda1
Veteran
Posts: 13434
Joined: Thu Jan 05, 2017 9:03 pm

Re: Sketcher Tool settings : testers welcome!

Post by Kunda1 »

Hey @abdullah! Great to hear from you! Thanks for the elaborate check-in. JFYI, give me a heads up which PRs you want me to generate test snap builds for. You can tag me in the PR ;)
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by abdullah »

Kunda1 wrote: Sat Jun 25, 2022 12:34 pm Hey @abdullah! Great to hear from you! Thanks for the elaborate check-in. JFYI, give me a heads up which PRs you want me to generate test snap builds for. You can tag me in the PR ;)
Hi there! Thank you.

I have just updated the testing branch (the same we used some months ago):
https://github.com/abdullahtahiriyo/Fre ... et_testing

Some snaps were created at the time, so probably it is not too difficult to create new snaps from there.

Many of the feedback of the first round has been implemented. Hopefully, I will have some time later on to comment on it.

What should work fine:
DSH Line
DSH Rectangle
DSH CIrcle
DSH Ellipse
DSH Polygon

The toolbars have been "reorganised". This is still up for discussion. There are some ideas I do like, for example merging of conics. It is not final.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by abdullah »

Previous feedback:
BrandonGene wrote: Wed Apr 20, 2022 7:28 pm My only suggestion for an implementation change is to avoid bouncing UI features around by showing the Corner Radius and Thickness boxes greyed out underneath their respective checkmarks.
Granted:
Screenshot_20220625_181605.png
Screenshot_20220625_181605.png (26.39 KiB) Viewed 967 times
BrandonGene wrote: Wed Apr 20, 2022 7:28 pm But! One major con to merging the tools: This could mess with shortcuts/workflow in a big way.
All the legacy tools have now a menu entry with an associated shortcut (the legacy one). Of course, it is also possible to use one of the shortcuts of the tool and then change the mode, or select the checkboxes. It depends on how each person works.

BrandonGene wrote: Wed Apr 20, 2022 7:28 pm M is not an ideal mode switching key as it requires leaving home row with the non-mousing hand. If this is going to be such an important and oft-used shortcut after the merge, it would be awesome if it was customizable. Probably a conversation for a different thread, though. :)
Currently 'M' is fixed and no customizable. This may change in the future. So 'M' will change the construction method of the tool. In addition, 'U' and 'J' are mapped to toggle the first and second checkboxes (for tools having them). In polygon, these two keys allow to increment/decrement the number of sides of the polygon.
cadcam wrote: Sat Apr 23, 2022 1:43 am 6) Primarily related to polygons. Users often swap between diameter and across-flats
dimensioning. Could the option be included in the menu and the last setting remembered
between function uses?
This we have not done it, but I might do it. I just did not remember about this comment when revisiting the polygon DSH.
Autoconstraint redundancy
This has been fully reworked (in the supported tools). It should not be possible that autoconstraints create redundant or conflicting constraints. Basically, each parameter of the element is checked before adding autoconstraints and before enforcing widget mandated constraints (dimensional constraints do to fixed values). Any autoconstraint that will lead to redundancy/conflict is not added. Any widget mandated constraint that would lead to redundancy/conflict is not added either.
Toolbar simplification
We set as goal to simplify the toolbars. With the introduction of modes, some icons are gone (to simply it). Icons and shortcuts are still in the menu.
Toolbar reordering
Grouping of some tools together in the toolbar has been debated and I think we should continue to debate it (with a larger group of users).
Widget only entry
The ability to introduce an element only using the keyboard (by entering dimensions in the widget) has improved considerably. Now, autoconstraints are detected (even if the mouse does not hover the autoconstraint places), redundants/conflicts are avoided.
User avatar
jnxd
Posts: 951
Joined: Mon Mar 30, 2015 2:30 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by jnxd »

abdullah wrote: Sat Jun 25, 2022 7:39 am I have added some PRs to master, which are of functionality independent from the tool widget and the new DSH architecture we have been working on, but that are a precondition and support it.
...
I do not have the time to review/test/correct all new tools and intended changes of DSH architecture and tool widget effort at a single time.
...
When I refer to new DSH architecture, mostly I refer to a new architecture hanging on the existing one (which is not changed, save for bug fixing a minor refactors), which should be an improvement, at least for the subclass of geometry creation.
...
Summary: The new architecture is not a drastic change of the existing one, but a specialisation useful for certain use cases. Any tool written in the existing architecture continues to work without modification.
...
While I will have some code availability during July, I hope to have more coding time in August (where I plan for holidays, so hopefully more free time for FreeCad).
Hi Abdullah.

I would suggest you spend some time documenting the planned workings (in a high level fashion) of the DSH and any other changes you plan to make. I realize it takes precious time away from coding, but it adds much more clarity to an outsider trying to read through this code. It might also help you in that other people can contribute code to your current effort.
My latest (or last) project: B-spline Construction Project.
User avatar
jnxd
Posts: 951
Joined: Mon Mar 30, 2015 2:30 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by jnxd »

abdullah wrote: Sat Jun 25, 2022 4:55 pm Currently 'M' is fixed and no customizable. This may change in the future. So 'M' will change the construction method of the tool.
I recall we had a private chat about using "M" as a shortcut and you had mentioned that this could cause problems for tool settings if someone wants to specify units. How did you go around it this time?
My latest (or last) project: B-spline Construction Project.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by abdullah »

jnxd wrote: Sun Jun 26, 2022 1:03 pm
abdullah wrote: Sat Jun 25, 2022 4:55 pm Currently 'M' is fixed and no customizable. This may change in the future. So 'M' will change the construction method of the tool.
I recall we had a private chat about using "M" as a shortcut and you had mentioned that this could cause problems for tool settings if someone wants to specify units. How did you go around it this time?
When the widget has the focus (which in practice every time the tool is activated), it decides whether to consume the events or not. Some events it will always consume, mostly signs and numbers (small subset). When a character of this subset is typed, it will consume any character over a period of 2 seconds (timer). This enables to enter the units "m" or "mm" without any problem. When the 2 seconds lapse it comes to the normal mode. In normal mode key strokes (other than the ones in the subset) get redirected to normal FC for handling. This mechanism allows to jump from one tool to other using the shortcuts.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by abdullah »

jnxd wrote: Sun Jun 26, 2022 12:53 pm I would suggest you spend some time documenting the planned workings (in a high level fashion) of the DSH and any other changes you plan to make. I realize it takes precious time away from coding, but it adds much more clarity to an outsider trying to read through this code. It might also help you in that other people can contribute code to your current effort.
There is a self-documentation in the form of working examples to understand how to do it. It should not be a problem to write a new handler using the new DSH architecture (though it is always possible to use the old one). The new handlers provide relatively simple basic functions (e.g. drawing).

Then, internally within the DSH, some interactions are much more complex. The reason is that tools may be changed via mouse, widget or the combination thereof, different combinations result in the state machine jumping to the next state, or reapplying the result of previous states. In the most general case, this involves different calls to the basic functions with different states. For this, I have opted for using plantuml to document the exchanges via message sequence charts.

I did a now partly outdated hierarchy in inkscape, this I will redo in plantuml.

I will write more on documentation a little bit later on.
User avatar
jnxd
Posts: 951
Joined: Mon Mar 30, 2015 2:30 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by jnxd »

abdullah wrote: Sun Jun 26, 2022 2:29 pm When a character of this subset is typed, it will consume any character over a period of 2 seconds (timer). This enables to enter the units "m" or "mm" without any problem. When the 2 seconds lapse it comes to the normal mode. In normal mode key strokes (other than the ones in the subset) get redirected to normal FC for handling.
I see. Thanks for the explanation. Couple suggestions off the top of my head (unsolicited though they be):
1. This timeout should be configurable, with a certain keystroke dedicated to saying we're done (maybe TAB).
2. There should probably be some Indication that the keystrokes are going towards the text box.
My latest (or last) project: B-spline Construction Project.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by abdullah »

jnxd wrote: Sun Jun 26, 2022 2:50 pm I see. Thanks for the explanation. Couple suggestions off the top of my head (unsolicited though they be):
1. This timeout should be configurable, with a certain keystroke dedicated to saying we're done (maybe TAB).
2. There should probably be some Indication that the keystrokes are going towards the text box.
Unsolicited suggestions are more than welcome ;)

Indeed, the timeout should be made configurable for accessibility reasons (it is straightforward to do it). However, I would like to get usability feedback first.

TAB, as in any widget, causes the widget to move to the next control (often next parameter). It is one in the special subset that triggers the widget (shift+tab goes back as expected too). In my tests, I did not feel the need for a stop key. But this may come back as feedback.

It would be nice to have some indication that any key will be consumed by the text box. Feel free to make suggestions (or even code them if you wish, you have the skill ;) ).

When snaps are available, I guess there will be lots of feedback to work with...
User avatar
jnxd
Posts: 951
Joined: Mon Mar 30, 2015 2:30 pm
Contact:

Re: Sketcher Tool settings : testers welcome!

Post by jnxd »

abdullah wrote: Sun Jun 26, 2022 5:27 pm TAB, as in any widget, causes the widget to move to the next control (often next parameter). It is one in the special subset that triggers the widget (shift+tab goes back as expected too). In my tests, I did not feel the need for a stop key. But this may come back as feedback.
OK, we can table the discussion for stop key. But let me rephrase in the case of tab/Shift tab: whenever we move to the next parameter, the stop should be activated, even if timeout hasn't passed. If I am correct that will be enough functionality.
abdullah wrote: Sun Jun 26, 2022 5:27 pm It would be nice to have some indication that any key will be consumed by the text box. Feel free to make suggestions (or even code them if you wish, you have the skill ;) ).
I was imagining disabling some elements, but that might cut into performance. Some kind of highlighting would be better.
abdullah wrote: Sun Jun 26, 2022 5:27 pm When snaps are available, I guess there will be lots of feedback to work with...
This is a big change, so that's expected.
My latest (or last) project: B-spline Construction Project.
Post Reply