Extended selection tools by 'RestrictSelectionToSubset' which deselec… #7
Conversation
…ts all elements from the selection that are not in the specified subset.
sreiter
left a comment
sreiter
left a comment
There was a problem hiding this comment.
Thank's for providing the new functionality. I have some small comments. Summarizing: It would be great if you could remove the dynamic memory allocations by simply deselecting elements and if you could reduce code duplication by performing that deselection in an additional template helper function.
register_selection_tools.cpp
Outdated
| "volumes || value=true", TOOLTIP_SELECT_UNASSIGNED_ELEMENTS) | ||
| .add_function("RestrictSelectionToSubset", &RestrictSelectionToSubset, grp, "", | ||
| "mesh #" | ||
| "subset index#", TOOLTIP_RESTRICT_SELECTION_TO_SUBSET); |
There was a problem hiding this comment.
I think the '#' separator behind "subset index" is unnecessary and should probably be removed.
tools/selection_tools.cpp
Outdated
| Selector& sel = obj->selector(); | ||
|
|
||
| // Store selected elements that are contained in the specified subset | ||
| std::vector<Vertex*> vVertices; |
There was a problem hiding this comment.
While of course a matter of taste, The 'v' prefix is unnecessary, I'd say, since the type is clear. There are no prefixes on many other variables, so this is probably slightly inconsistent.
tools/selection_tools.cpp
Outdated
| CloseSelection (obj->selector()); | ||
| } | ||
|
|
||
| void RestrictSelectionToSubset(Mesh* obj, int si) |
There was a problem hiding this comment.
There is a lot of code duplication in that method. There could be an additional template method which performs the restriction for one element type. You'd then call that method here for 'Vertex', 'Edge', ...
Also - why do you store a list of selected elements, deselect all and reselect those elements. Instead you should simply deselect the ones which are not in the specified subset.
…dure introducing helper procedure DeselectUnassignedElementsHelper.
| @@ -194,17 +194,17 @@ static void SelectUnassignedElementsHelper( | |||
|
|
|||
| template <class TGeomObj> | |||
| static void DeselectUnassignedElementsHelper( | |||
There was a problem hiding this comment.
Do you actually still use this method? If not I would rather remove it.
There was a problem hiding this comment.
You are right, I do not use it anymore, it's just left over from the previous implementation. Shall I remove it or will you do it?
There was a problem hiding this comment.
Would be great if you could do it. Thanks!
|
Thanks! |
2 participants
…ts all elements from the selection that are not in the specified subset.