Element widget for Sketcher WB

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!
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Element widget for Sketcher WB

Post by abdullah »

Hi Folks!

I want to show you a widget I did for the Sketcher WB, in order to allow editing of sketches when they have (partially) overlapping lines of any type (construction/external/normal). Just take a look and drop some lines with your impressions. I am very much open to criticism from any point, usability, functionality, (poor) c++ coding skills, inefficient code... Just shoot, I am willing to learn!!

Needless to say, if you think that any of this has a place in FreeCAD, in this or in a modified state, so be it (merge it, sorry rebase it, or modify it as you wish). Just if you think it is useful ;-)

How does it work?

1. A QListWidget presents a list of elements, element is here a geometric element
formed of Types: edges, starting points, end points and midpoints (the last for circles and arcs)
2. On hovering/Entering one item in the list, it is preselected in the Inventor view for easy identification.
3. The selection of Type can be switched from one type to another using the SHIFT key
(press and release) in a round-robin fashion (or you can select it using the droplist)
4. Multiple selection is possible by holding the Ctrl key.
5. Selection of different types is possible (e.g. two points and a line).
6. The contextual menu accessible by right click allow to apply a constraint to the
selection.
7. The constraints in the contextual menu have shortcuts, so you can actually use the
shortcut directly from the QListWidget.

How does it look like?
TaskSketcherElements.jpeg
TaskSketcherElements.jpeg (148.43 KiB) Viewed 4676 times
Other features (improvements?) introduced to other parts of FreeCAD
- New color introduced for Types that are selected and preselected at the same time
- Edges is a SoLineSet. You can not apply many So modifiers (width,offset) to parts of a SoLineSet.
An edge part of a SoLineSet that is drawn after another one, covers it unless it is closer to the
camera. The system of manual offsets already in place for points has been extended to properly
handle (i.e. show) overlapping lines part of a SoLineSet.


Download

This first working version is available in my github, https://github.com/abdullahtahiriyo/Fre ... master.git, in the sketch_element_list branch.

Yes, everything is in a single commit, I am also learning to squash with git (thanks Shoogen!).

History

In the last weeks I have been busy learning about the internals of FreeCAD, more particularly, the Sketcher WB.

I was having a problem with not being able to select overlapping lines (construction lines on construction lines, handy for example for making factors of the pitch of a thread). I remember someone (Ulrich1a?) opened a ticket for this in Mantis.

Well, I was feeling lucky so I decided to start coding an Element widget for the Sketcher WB, mainly thought to solve the aforementioned problem, but also to accelerate some mouse intensive tasks (i.e. making several line segments equal to each other...). Once I got the widget running (I heavily based myself on the Constraints widget, as I did not know anything about QT), I got into some visualization problems (i.e. an edge in a SoLineSet that is drawn by Inventor after another edge of the same SoLineSet, totally covers the previous segment, not only for selection, but also for visualisation). I tried adding transparency, but it is disabled for sketcher, then I though about making thicker the line selected, but Inventor applies the Drawing Style to the whole SoLineSet, not to only parts of it. Then I saw (Inventor is also new to me) that it is possible to define a camera offset for a line (depthoffset), but this also applies to the whole SoLineSet. Well, the solution that at the end I found is inspired in the depthoffset and following the same philosophy that it was already implemented for visualizing the points (vertex) over the lines (edges). In the way I introduced a new color for elements that are selected and preselected at the same time, which is handy when you do not have the mouse on the line and you want to see which element is actually preselected and selected. To finish everything I also learned about creating icons (great page in the documentation) and building the QT resources. Well, finally today I have finished it. Hope you like it and better if you find it useful, it would be a small contribution from me to FreeCAD in exchange of the tons of fun I get from it!!
jmaustpc
Veteran
Posts: 11207
Joined: Tue Jul 26, 2011 6:28 am
Location: Australia

Re: Element widget for Sketcher WB

Post by jmaustpc »

Hi Abdullah

I'm a bit short of time just now so my testing has only been very brief, but I can confirm it compiled for me and seems to work on Linux, :)

It seems to crash sometimes when I set a line to construction mode. I found an example that consistently crashes for me.

1)start your version of FreeCAD, go to sketcher wb, create new document, go to profiles menu, enter a hexagon, zoom fit all
2)set the hex sketch into edit mode, select a line by clicking on the line in the sketch window, then click on construction mode....FreeCAD will crash.


I did get a couple of other crashes with setting a line to construction mode but have not yet found another consistent example.

Jim

OS: Ubuntu 14.04 LTS
Platform: 64-bit
Version: 0.14.3672 (Git)
Branch: sketch_element_list
Hash: 6caa965a70c4fd6690cd99700abefc1956b1a623
Python version: 2.7.6
Qt version: 4.8.6
Coin version: 4.0.0a
SoQt version: 1.6.0a
OCC version: 6.7.1
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Element widget for Sketcher WB

Post by abdullah »

Thanks a lot for reporting!!
I did test creating normal lines and applying constraints, and also that it was working with contruction lines of a sketch created with a previous version, but never actually created a construction line...How silly of me!! I'll look into it tomorrow. Thanks!
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Element widget for Sketcher WB

Post by abdullah »

I (think I have) fixed the bug generating the crash. I have pushed the fix to my github branch.

I have been playing with it for a while and I suffered no crash. Let me know if you discover more bugs or unexpected behaviour.
mrlukeparry
Posts: 655
Joined: Fri Jul 22, 2011 8:37 pm
Contact:

Re: Element widget for Sketcher WB

Post by mrlukeparry »

Hi abdullah,

I'm yet to try it, but firstly a good effort taking up a long needed task on the Sketcher which has been fairly dormant for a year now. It's a great way to learn/practice c++ coding and is how I first began with alot of advice and critique from logari.
abdullah wrote:SoLineSet, totally covers the previous segment, not only for selection, but also for visualisation). I tried adding transparency, but it is disabled for sketcher,
Yes this is a problem I found. We would have to create our own implementation to allow per edge changes.

Code quality looks fine.

I'm trying to see what you've done in ViewProviderSketch. Im gathering you iterate through the edges and any that are selected has their z-value raised? I suppose this in practice is fine, but doesn't fix our z fighting issue.

Also on ViewProviderSketch: line 2623 I think you need to use *it->x?

In TaskSketcherElements.cpp you use a macro. I'm just wondering generally in FreeCAD if we are fine to use macros? I just don't tend to see them often. Not that it makes much difference, instead of a static_cast because ElementItem is a QListWidgetItem it's derived from a QObject so you can use qobject_cast.

To help improve readability can you try to use spaces throughout? For example in TaskSketcherElements.cpp line 566.

Code: Select all

ui->listWidgetElements->item(i)->setIcon(element==0?edge:element==1?sp:element==2?ep:mp);

Code: Select all

ui->listWidgetElements->item(i)->setIcon(element == 0 ? edge : element == 1 ? sp : element == 2 ? ep : mp);
I find it's just easier to read. I haven't yet tested it but it's very good start!
jmaustpc
Veteran
Posts: 11207
Joined: Tue Jul 26, 2011 6:28 am
Location: Australia

Re: Element widget for Sketcher WB

Post by jmaustpc »

1) Icons
Just a little thing, the three different icons are, in my opinion, too difficult to differentiate at the low resolution at which they are displayed in the taskview. Is there any reason why you have changed the colour scheme so that they do not match the "red dots" on the end of the lines in other sketcher icons? Off the top of my head, I would have thought the green (selection colour - a good idea) would be much clearer in contrast with the standard red. I can create some alternatives, .....if you want me to. :) By the way, it might also be a good idea for consistency to use the same green I used in the sketch profile Hexagon icon. The idea behind that green was to imply the default "selection" green, but to differentiate it from the green we use to for Mesh item icons.

2) Element types
I am not sure exactly what to suggest here, but the way it works at the moment is that items are still offered in the list even when the selection/element "type" is invalid. E.g. circle start and end point, point end or centre point, line centre point. I wonder if only elements with a valid selection of that type should be shown? Although that might also get confusing, I am not sure.

Another thing currently the appropriate item is set to preselect colour as you hover the mouse over the list....that seems like a good idea, however the pre-selection is not cleared if the you hover the mouse over the next item in the list if it is invalid for the "type" selected, which can be potentially confusing.


I like that this allows one to select an item that previously we could not select, like a line on another line. :)

Jim
User avatar
bejant
Veteran
Posts: 6075
Joined: Thu Jul 11, 2013 3:06 pm

Re: Element widget for Sketcher WB

Post by bejant »

abdullah wrote:I want to show you a widget I did for the Sketcher WB, in order to allow editing of sketches when they have (partially) overlapping lines of any type (construction/external/normal).
abdullah wrote:I was having a problem with not being able to select overlapping lines (construction lines on construction lines, handy for example for making factors of the pitch of a thread). I remember someone (Ulrich1a?) opened a ticket for this in Mantis.
Just thought I'd mention that maybe this resolves Mantis issue #0693 and / or issue #0930 before I forget...
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Element widget for Sketcher WB

Post by abdullah »

Thanks to all for your reactions!
I'm yet to try it, but firstly a good effort taking up a long needed task on the Sketcher which has been fairly dormant for a year now. It's a great way to learn/practice c++ coding and is how I first began with alot of advice and critique from logari.
Thanks. I really have no problem with critiques. Just make all the remarks that come to your mind.
Yes this is a problem I found. We would have to create our own implementation to allow per edge changes. I'm trying to see what you've done in ViewProviderSketch. Im gathering you iterate through the edges and any that are selected has their z-value raised? I suppose this in practice is fine, but doesn't fix our z fighting issue.
Yes you gather well, that is exactly what the code does. IMHO, that a line in a SoLineSet covers a previous line at the same distance/position from the camera is not a bug. The Inventor implementation would have to choose one of the lines, and choosing the last one is just one possible solution.

It would be nice that the linedepthoffset would be implemented as a MF value that we could apply like the material (i.e. color/transparency). It is simply not part of the Inventor functionality/standard. The "z rising" on our code is actually what linedepthoffset does for a whole SoLineSet in Inventor.

Another solution that I also considered and rejected was to create a second SoLineSet object for selected items only, so we could apply one modifier LineDepthoffset (though I think I remember that LineDepthoffset was not available in Coin3D implementation). Anyway, this would involve moving edges back and forth between the two SoLineSets, so not a wonderful solution either.

Chosing to rise the lines in updateColor function is just for the reason that updateColor is called in several different functions and color and rising are interrelated. There is a repetition of code in the cases where updateColor is called from draw (reiterating an already interating loop for assigning the z height). However, changing this would certainly involve major changes in ViewProviderSketch that at the time I wanted to avoid. Nevertheless, I would aggree there is room for improvement there.
Also on ViewProviderSketch: line 2623 I think you need to use *it->x?
Sorry I do not understand what you are referring to, that line is blank on my ViewProviderSketch.cpp and nothing around it refers to an object

Code: Select all

it
. Is it another file/line?
In TaskSketcherElements.cpp you use a macro. I'm just wondering generally in FreeCAD if we are fine to use macros? I just don't tend to see them often.
There are three macros, motivated mainly to be consistent with the first of them, which must generate diffently named objects for each qaction. If you (plural) think I should not use macros for this, please let me know about FreeCADs macro policy. I will use the shell to expand it to normal code.
Not that it makes much difference, instead of a static_cast because ElementItem is a QListWidgetItem it's derived from a QObject so you can use qobject_cast.
To be honest with you, I did not know qobject_cast existed, and to be even more honest, I had to look up static and dynamic cast macro templates myself, as before that I only knew the standard C style cast (). The reason I used static_cast is because in the constraints part is used so.

As far as I understood from Stackoverflow static_cast is faster than standard c style cast because of the need to check for lower amount of casts, as the c style one works for all types of casts. I have just googled it and came with the result that qobject_cast additionally checks for the object being casted to be a QObject, which I guess would be less efficient.

In fact, for a static cast I do not quite understand the advantage of using qobject_cast. If you know it please enlighten me!!
To help improve readability can you try to use spaces throughout?
Certainly. I know I have this problem with that specific operator. It is a long standing mispractice and I am trying to correct it, but because I am used to see it together I do not always realise. Another one I have is just putting the opening curly brace on a next empty line. I try to correct them. I general if you see in commited FreeCAD code one of those made by me do not hesitate to correct it for readibility.
1) Icons
Just a little thing, the three different icons are, in my opinion, too difficult to differentiate at the low resolution at which they are displayed in the taskview. Is there any reason why you have changed the colour scheme so that they do not match the "red dots" on the end of the lines in other sketcher icons? Off the top of my head, I would have thought the green (selection colour - a good idea) would be much clearer in contrast with the standard red. I can create some alternatives, .....if you want me to. :)
Yes and no. So, yes there is a reason, but not a valid one, I wanted them not be mixed up with constraints (generally in red). However, the whole theme of sketcher is redish and I can only agree with you that red would make a much clearer constrast. You are totally right.
By the way, it might also be a good idea for consistency to use the same green I used in the sketch profile Hexagon icon. The idea behind that green was to imply the default "selection" green, but to differentiate it from the green we use to for Mesh item icons.
I was totally unaware of that remark on the type of green. I tried to do my best by selecting the same palette that is suggested in the art documentation page. I also took care that the light comes from the top left. But I really agree with you. Making icons (or modifying icons) is not something that I do right.
I can create some alternatives, .....if you want me to
Please bring forward some nice ones!!
2) Element types I am not sure exactly what to suggest here, but the way it works at the moment is that items are still offered in the list even when the selection/element "type" is invalid. E.g. circle start and end point, point end or centre point, line centre point. I wonder if only elements with a valid selection of that type should be shown? Although that might also get confusing, I am not sure.
I understand it can be misleading to a certain point, but that was at least the initially intended behaviour. The reason is that this way, when you press shift you do not get "transported" to another object, but just iterate through the elements of the same object that was preselected.

On alternative solution that comes to my mind now would be to "jump" in the round-robin selection to the next type if the currently selected object does not have that kind of "type" (in the case of a line, a mid-point), so pressing the SHIFT key in Endpoint in the case of a preselected line would jump to "Line" instead of "Midpoint".

This would however make not deterministic the number of SHIFT presses needed to jump among types, which could also be annoying. Give it an extra thought and let me know what you think...
Another thing currently the appropriate item is set to preselect colour as you hover the mouse over the list....that seems like a good idea, however the pre-selection is not cleared if the you hover the mouse over the next item in the list if it is invalid for the "type" selected, which can be potentially confusing.
Ohh!!... Let me check... yes definitely!!, that is not the intended behaviour, you found another bug!!! I will try to fix it today.
I like that this allows one to select an item that previously we could not select, like a line on another line. :)
That was the whole point of it...;)
Just thought I'd mention that maybe this resolves Mantis issue #0693 and / or issue #0930 before I forget...
Well, I do not know to what extend it really "solves" them. I mean, you still can not select the overlapped lines in the Inventor view.

It certainly provides a work-around that allows to select them. I personally would feel no need to have the
"bugs" "fixed" in other way if this widget were to be introduced in FreeCAD.

...I'll try to fix that bug...
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Element widget for Sketcher WB

Post by abdullah »

A new commit in the my github branch fixes the latest bug reported by Jim.
jmaustpc
Veteran
Posts: 11207
Joined: Tue Jul 26, 2011 6:28 am
Location: Australia

Re: Element widget for Sketcher WB

Post by jmaustpc »

abdullah wrote:I was totally unaware of that remark on the type of green. I tried to do my best by selecting the same palette that is suggested in the art documentation page.
I only came up with that colour scheme idea about 2 weeks ago, when I suggested some icons for the sketcher profiles that Jurgen and Werner made. :)


abdullah wrote:
jmaustpc wrote: I can create some alternatives, .....if you want me to


Please bring forward some nice ones!!
I had already made some...here they are... :) But really you don't have to accept these they are just my first thoughts.

I have not had time yet to compile these in to your branch to see what they look like, but I was thinking that something along these lines might be good?

Line element
at 16 px
Sketcher_Element_Line1_16px.png
Sketcher_Element_Line1_16px.png (630 Bytes) Viewed 4488 times
at 32opx
Sketcher_Element_Line1_32px.png
Sketcher_Element_Line1_32px.png (1.41 KiB) Viewed 4488 times
the svg file
Sketcher_Element_Line1.svg
(11.62 KiB) Downloaded 169 times



starting point
at 16px
Sketcher_Element_StartingPoint1_16px.png
Sketcher_Element_StartingPoint1_16px.png (660 Bytes) Viewed 4488 times
at 32 px
Sketcher_Element_StartingPoint1_32px.png
Sketcher_Element_StartingPoint1_32px.png (1.46 KiB) Viewed 4488 times
the svg file
Sketcher_Element_StartingPoint1.svg
(12.22 KiB) Downloaded 158 times

Ending point
at 16 px
Sketcher_Element_EndingPoint1_16px.png
Sketcher_Element_EndingPoint1_16px.png (649 Bytes) Viewed 4488 times
at 32px
Sketcher_Element_EndingPoint1_32px.png
Sketcher_Element_EndingPoint1_32px.png (1.42 KiB) Viewed 4488 times

the svg file
Sketcher_Element_EndingPoint1.svg
(11.42 KiB) Downloaded 109 times

For mid point I was not quite so sure...I think you really mean the centre point for an arc rather than mid point? So I came up with two

firstly a mod of your icon
at 16 px
Sketcher_Element_MidPoint1_16px.png
Sketcher_Element_MidPoint1_16px.png (656 Bytes) Viewed 4488 times
at 32px
Sketcher_Element_MidPoint1_32px.png
Sketcher_Element_MidPoint1_32px.png (1.45 KiB) Viewed 4488 times
the svg file
Sketcher_Element_MidPoint1.svg
(11.65 KiB) Downloaded 174 times
if you really mean the centre of the arc then perhaps one more like this? Although if you want this arc one, then I will re-do it the other way around, so that it matches the arc in sketcher

at 16 px
Sketcher_Element_MidPoint1_arc_16px.png
Sketcher_Element_MidPoint1_arc_16px.png (727 Bytes) Viewed 4488 times
at 32 px
Sketcher_Element_MidPoint1_arc_32px.png
Sketcher_Element_MidPoint1_arc_32px.png (1.56 KiB) Viewed 4488 times
the svg file
Sketcher_Element_MidPoint1_arc.svg
(11.62 KiB) Downloaded 177 times

Jim
Post Reply