-
Notifications
You must be signed in to change notification settings - Fork 118
improved computation of interactive marker size #613
Conversation
double s = 1.01 * ext.norm(); | ||
|
||
// clip scale to 5cm | ||
return std::max(0.05, s); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how did you choose 5cm here? i've seen some robots whose model does not properly follow metric sizing, i.e. they could be a couple of orders of magnitude larger or smaller than real life meters. Seems this could be error prone hard coding 0.05
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree, that the code should simply rely on its measurements, regardless of magnitude.
However, I didn't invent the 5cm limit. It was there before (see below) and I didn't want to break existing code.
Probably it was there for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah indeed! seems like that should have at least be a static const
added the proposed static const variable |
This makes sense to me, but I am not able to test it (away from office) so am hesitant to merge. Not sure how to verify this won't cause unexpected visual change for others. |
There is no need to hurry. I'm at IROS conference this week too. Of course, this PR will have some visual effects. |
I just tried this - the markers are much smaller than they used to be. You can try it with the same robot - https://github.com/sachinchitta/moveit_examples |
- use parent_link if group == parent_group - scale smaller than 5cm is clipped to 5cm instead of using default - clarified size computation, using diameter of AABB
compute size such that the marker sphere will cover - a spherical link geometry -> AABB.maxCoeff - a cubical link geometry -> AABB.norm -> use average of both values Virtual links (without any shape) will have a size of AABB of zero dims. In this case use the dimensions of the closest parent link instead.
@sachinchitta Indeed your example robot has a virtual end-effector link (resulting in size zero), which I didn't considered during implementation yet. To handle this case gracefully (instead of returning the |
This looks better now but would be good to have the size still be a little bigger - I tried it with 3.0 * size and it was easier to interact with. |
- drop largest extension dimension (-> use cross-section size of elongated link) - for an end-effector group, consider the sizes of individual links instead of the overall size of all links (which becomes huge very fast) - enlarge marker size by factor of 1.5 when there is only a single eef marker
LGTM. |
improved computation of interactive marker size
…_template/package.xml.template (moveit#613)
If there is a really small end-effector group / link, we should rely on the computation too, shouldn't we?