[UI] Sketcher: icon overhaul for the sketcher workbench to unify the appearance

[maxwxyz]

This PR is only changing the .svg icons, no other WB code is touched.

The sketcher toolbar icons currently have different stroke widths, colors and point sizes. This PR unifies the appearance of the icons in the Sketcher WB. The icons were adjusted or redrawn to match the FreeCAD design guidelines and progress was discussed in the Design Working Group. The icons for the constraint group were made by @obelisk79

Here are a few screenshots before and after, GitHub seems to compress the copied screenshots.
If otherwise not specified, the theme in the screenshots is OpenDark. Please ignore the dropdown arrows on the toolbar icons.

There is another PR to change the cursor icons as well, to match the toolbar icons: #11749

Toolbar icons 24x24 and 32x32:

Dropdowns, right mouse menu and menu items plus elements:

24x24 px icons and constraints "in action":

Toolbar icons in FreeCAD Classic theme:

and for comparison the inconsistency of the current icons...

Current toolbars and menu items

[obelisk79]

@abdullahtahiriyo this also compliments this PR with cursor symbols:
#11749

[abdullahtahiriyo]

@obelisk79

This PR is missing the activation of the coincident unified icon. I will push a separate PR to fix that.

That other PR about cursors, I am going to let Paddle review first.

[maxwxyz]

@abdullahtahiriyo I did not push the activation of the new icon, as on the sketch canvas only the on point icon is displayed, never the coincident icon. I was not sure if the unified icon should be in the toolbar when the on point is only shown.

[obelisk79]

@abdullahtahiriyo

[abdullahtahiriyo]

@maxwxyz

diff --git a/src/Mod/Sketcher/Gui/CommandConstraints.cpp b/src/Mod/Sketcher/Gui/CommandConstraints.cpp
index 41070ff35a..7d0fc61245 100644
--- a/src/Mod/Sketcher/Gui/CommandConstraints.cpp
+++ b/src/Mod/Sketcher/Gui/CommandConstraints.cpp
@@ -3542,7 +3542,7 @@ CmdSketcherConstrainCoincidentUnified::CmdSketcherConstrainCoincidentUnified(con
         "or a concentric constraint between circles, arcs, and ellipses");
     sWhatsThis = "Sketcher_ConstrainCoincidentUnified";
     sStatusTip = sToolTipText;
-    sPixmap = "Constraint_PointOnObject";
+    sPixmap = "Constraint_Coincident";
 
     ParameterGrp::handle hGrp = App::GetApplication().GetParameterGroupByPath(
         "User parameter:BaseApp/Preferences/Mod/Sketcher/Constraints");

When the merge of the PoO and Coincident is enabled in preferences, the icon that is shown after this PR for this merged tool is the one of PointOnObject (curve with one point on it). I think you want the one that @obelisk79 is showing. Then, you need to change this line above.

My internet connection, for some reason, is not working properly for GH, so I cannot push this ATM. I will try tomorrow...

[maxwxyz]

@abdullahtahiriyo I know and did this for my test builds but I was not sure if the new unified icon from @obelisk79 is good for the toolbar.
I like the look but when I apply this constraint, always the OnPoint constraint is shown on the canvas above the geometry because the coincident is never shown on the geometry. Therefore this would be the only constraint icon which is never visible on the sketch plane on top of geometry

[abdullahtahiriyo]

Regarding a PR that is not draft, the expectation of a maintainer is that there have been enough discussion and that what is proposed has substantial consensus and is final. If for any reason, it is not the case, the PR should be draft and any issue should be raised right away. This is not aimed as criticism, but as improve future PRs.

From what is posted above, I understood that this PR is "just" so that: (a) the same visual principles (thicknesses,...) are applied to all icons to remove an inconsistency of drawing design, (b) some clear errors about wrongly marked selection of elements of geometry icons are corrected, and (c) some icons that are difficult to tell apart each other are better differentiated. All those changes belong to the realm of "art", they do not change functionality, which is the reason why it was merged quick.

Now, let's try to see if I understand the matter ok and to resolve the open issue (also others are welcome here):

1. Yes, it is true that the coincident constraint does not have an icon in the 3D view. That is also the case for all internal alignment constraints (axes of ellipses, focus of parabola, ...), but those are automated. The coincident is still the only one where selection of geometry causes a selection of a constraint. This behaviour is very convenient and I do not think we should look into changing it. Let me know if you agree to this point.
2. The situation (1) is about the same for the current coincident icon (point with outcoming rays), in that this is not shown either.
3. The situation (2) is only similar, and not the same, for this "unified coincident" because sometimes it produces a coincident and others a point on object. Then, the point on object icon is shown in the 3D view (canvas), and it does not match the new coincident icon, but shows the point on object equivalent (arc with point).

I think that:
A. I do not see a problem from (1) and (2) above.
B. I see some degree of inconsistency in (3). The different with other inconsistencies is that you cannot remove it without removing functionality (of the merged coincident and point on object constraint). There are three options: (a) to leave the inconsistency, (b) to remove the functionality, or (c) to change the definition of inconsistent.
C. We have more constraint "buttons" which generate something that is not the same constraint as the icon (e.g. the lock constraint, which generates vertical and horizontal dimensional constraints, the dimensioning tool which creates many different ones). At least from a nomenclature point of view, I think we should differentiate "constraint" from "tool for generating constraints". The former shows the "consistent" behaviour (icon on button, icon in 3D view, user gets what he sees) and the latter does not (and will not, because functionality is intended not to).
D. I do not think we should venture into "removing all toolbar icons which are "tools for generating constraints", that would lead to a less powerful sketcher [so we should discard (b) as a solution].
E. The difference between (a) and (c) is only if we are going to do something about the inconsistency. The former is ignoring its existence and move on. The latter is to see if the UI/UX should be different to account for it (for example by different grouping, or location at different toolbars).
F. What I think needs to be different for these "tools for generating constraints", is that they will have an icon (tool icon) that is not the one icon that will appear in the 3D view. The icons (or labels if dimensional) in the 3D view will necessarily be different.

What I wrote above, I think applies to any change relating to "tools for generating constraints". So if we accept that they will have a different icon (and then I would introduce the patch of my previous post), what is left is how we are going to (re)arrange them, or if we are going to do anything different, because they are not the same. That I think is part of another different PR.

In that other PR, we do need to address the fact that we still have users who want "concise toolbars" and others who are not at lack of screen space, or are learning CAD and prefer to have a more "traditional" arrangement. When we accept that the requirements of both groups cannot be reconciled, I think we should still aim for the higher amount of common elements.

So, coming back to this PR, let me know if we want the behaviour as the patch above, or if we want anything else.

[maxwxyz]

Ok then I am fine with the patch, as the unified tool can be toggled in the preferences and people who like individual toolbar buttons and that the icon is reflected on the 3D view could change it anyways.
On a side note there are additional ideas around for displaying coincident points, also to indicate open wires (display something around two points, hollow points, different colors, subtle highlighting of closed islands) which will help the UX as well.

[PaddleStroke]

Yes, it is true that the coincident constraint does not have an icon in the 3D view.

Regarding this, I made a proposition while you were away that you may have missed. It is to draw un-coincident points with the construction geo color (blue now). And when points are coincident with another point they would become red. This way it would be very easy to see open wires.

And to the question, what if more than 3 points are stack, we just draw blue points on top of red ones.
Any thoughts on this?

[maxwxyz]

I like this idea but what if the geometry is fully constrained? Then all points become green:

I personally like the idea of closed islands / wires #11482 or some hollow point (stoke only) versos a filled point.

[PaddleStroke]

I don't think both ideas are incompatible actually. They would be nice together. Because you can have a closed shape without the points being coincident.

[maxwxyz]

Not sure if you actually can have a closed shape without a closed wire

[obelisk79]

Isn't this a bit out of scope of this pr?

I agree with paddle about combining the two could be complimentary. But also there are no closed shapes that don't have all segment points coincident.

[PaddleStroke]

Yes it's out of scope. I was just commenting about it because abdullah mentioned this topic.

[abdullahtahiriyo]

I understand the convenience of islands for the 3D building process. However a sketch does not build 3D geometry on its own (it could be a path for a milling bit, laser, or a sewing machine). Somehow, what could become a positive or negative space rather depends on the tool which uses a sketch as input. That is not even necessarily a pad.

I think I would prefer a system which does not impose restrictions on what the user expects for the 3D tool at sketch level, but rely on objective information of what is happening at sketch level.

Regarding Paddle's proposal, I think it is a better way. There are no blue points in the sketcher (as a construction point is red by convention, and hardly ever used sketch-defining points are white, as any defining geometry). The drawbacks I see are:
i. There is an argument that it would be attributing yet another function to colour in the sketcher, as colour is already coding type of geometry, construction state and constrained state.
ii. I have some concern that some points will be red and others blue, without this meaning anything is inherently wrong or different (all are sketcher construction points with equal behaviour). I have some fear this will be misleading users, who would like to understand what is wrong, when nothing is inherently wrong with it.

On a separate point, moving construction points to blue by default would likely mean changing all art in the sketcher for consistency. Not a technical argument. Just a note.

I think some time ago we talked about adding some material around coincident constraints (as the rays of the coincident icon). Nobody ever implemented it and I am unsure if there were arguments against.

In any case, when/if we address that, we need to have a separate GH issue open.

[obelisk79]

This conversation should move to a GitHub issue. I agree.

[PaddleStroke]

Issue : #11799