Added regex filter field for TF display #1744
Conversation
| @@ -40,6 +41,7 @@ | |||
| #include "rviz/ogre_helpers/axes.h" | |||
| #include "rviz/ogre_helpers/movable_text.h" | |||
| #include "rviz/properties/bool_property.h" | |||
| #include "rviz/properties/color_property.h" | |||
There was a problem hiding this comment.
Why do you need to add ColorProperty?
| filter_property_ = new StringProperty("Filter", ".*", "Regex filter", this); | ||
|
|
||
| filter_blacklist_property_ = new StringProperty("Filter (blacklist)", "", "Regex filter", this); |
There was a problem hiding this comment.
I suggest moving these properties as children of frames_category_. This makes it clear, where the filters are applied to. However, it makes the property harder to discover. What do you think?
Please rename "Filter" -> "Filter (whitelist)".
| @@ -372,22 +377,34 @@ void TFDisplay::updateFrames() | |||
| #pragma GCC diagnostic ignored "-Wdeprecated-declarations" | |||
| #endif | |||
|
|
|||
| context_->getTFClient()->getFrameStrings(frames); | |||
| context_->getTFClient()->getFrameStrings(frames); | |||
|
|
|||
| #ifndef _WIN32 | |||
| #pragma GCC diagnostic pop | |||
| #endif | |||
| std::sort(frames.begin(), frames.end()); | |||
There was a problem hiding this comment.
I suggest filtering frame names before sorting them (for efficiency).
| if (!blacklist_regex_string.empty()) | ||
| if (std::regex_match(frame, blacklist_regex)) |
There was a problem hiding this comment.
Prefer logical AND over nested IF:
| if (!blacklist_regex_string.empty()) | |
| if (std::regex_match(frame, blacklist_regex)) | |
| if (!blacklist_regex_string.empty()) && std::regex_match(frame, blacklist_regex)) |
| delete frame->tree_property_; | ||
| // delete frame->tree_property_; // Here happens segmentation fault. With skipping deletinion, memory leak | ||
| // appears but is not significant and only happens when user changes fields. |
src/rviz/default_plugin/tf_display.h
Outdated
| @@ -119,7 +119,8 @@ private Q_SLOTS: | |||
| BoolProperty* all_enabled_property_; | |||
|
|
|||
| FloatProperty* scale_property_; | |||
|
|
|||
| StringProperty* filter_property_; | |||
There was a problem hiding this comment.
Rename:
| StringProperty* filter_property_; | |
| StringProperty* filter_whitelist_property_; |
|
@BlazPotokar, do you plan to fix the remaining issues and finish this PR? |
Move items to the end instead.
The segfault occured due to an attempt to delete frame->tree_property_ twice, because it was deleted previously with a parent frame.
|
I will handle remaining issues in separate commits: #1789 |
|
Thank you so much for resolving the remaining issues! |
Description
When there is a lot of frames on
/tfit can be hard to properly visualize them in RViz especially if frames overlap. Usually solution to this is to enable and disable desired frames inFramesfield of theTF display. But it quickly become very annoying task if there is a lot of frames (e.g. 100 +) and each one needs to be found in a long list first and then disabled/enabled.Because of that I suggest adding regex field which filters frames and displays only the ones matching entered expression. For convenience I also suggest adding additional field for blacklist filter.
Benefit of this approach is that user can quickly setup one or more displays, each showing specific group of frames and he can turn each one on/off as needed.
Scene with a lot of frames ...
... can be easily filtered so only interesting frames are displayed
Example when multiple TF displays are added. Each one has its own settings and frame filter.
