Feature/add arrow strip marker

[rr-tom-noble]

Description

Fixes https://github.com/ros-visualization/rviz/issues/1785 Depends on: https://github.com/ros/common_msgs/pull/190

This PR adds a new ARROW_STRIP Marker type for more efficient / reliable rendering of large batches of arrows.

When using ARROW_STRIP, The points field of the Marker message should be populated with N points, where each pair of consecutive points defines the start and end of an arrow, connected tip to tail. Every other property has the same semantics as ARROW, and is applied to all arrows in the strip.

This PR also adds a new test script, arrow_strip_marker_test.py, which renders an ARROW_STRIP around the circumference of a circle, and modifies the orientation over time to test that the pose field applies to all arrows in the strip.

Since https://github.com/ros/common_msgs/pull/190 is unmerged, I hijacked the LINE_STRIP Marker type in marker_utils.cpp createMarker() to return an ArrowStripMarker in order to get the script working for the video. These changes have not been comitted, and the lines of code using the new ARROW_STRIP msg constant are left commented out for until that PR is merged.

Video

The following video shows test_arrow_strip_marker.py in action:

arrow_strip_marker.webm

Documentation

• ROS Wiki Marker page to update after merging: Wiki access granted

• Let me know if there are any other docs that need updating.

[rr-tom-noble]

@rhaschke Should I be concerned about the build failing? It's not super clear to me what the cause is :sweat_smile:

[rhaschke]

Since ros/common_msgs#190 is unmerged, I hijacked the LINE_STRIP Marker type in marker_utils.cpp createMarker() to return an ArrowStripMarker in order to get the script working for the video. These changes have not been comitted, and the lines of code using the new ARROW_STRIP msg constant are left commented out for until that PR is merged.

To enable your msg changes, just add your fork+branch of common_msgs as an upstream dependency in ci.yaml. How do you handle arrows of different length? Do they get different thicknesses, i.e. are they simply scaled?

[rhaschke]

Should I be concerned about the build failing?

Don't be concerned about the ROS build farm failure, which just reports on warnings. However, the GHA jobs should pass.

[rr-tom-noble]

How do you handle arrows of different length? Do they get different thicknesses, i.e. are they simply scaled?

Only the length is changed, all of the arrows have the same head and shaft thickness, controlled through scale e.g.

To enable your msg changes, just add your fork+branch of common_msgs as an upstream dependency in ci.yaml.

Thanks, I'll do that now. How exactly does it work? Does UPSTREAM_WORKSPACE take a list of values, and shall I uncomment the lines using the ARROW_STRIP constant throughout my changes?

[rhaschke]

UPSTREAM_WORKSPACE takes a list of values: UPSTREAM_WORKSPACE: github:rhaschke/python_qt_binding#silent-external-warnings github:rr-tom-noble/common_msgs#feature/add-arrow-strip-marker-type Yes, please uncomment your final code. That's the idea of using the correct upstream - to test the final code.

[rr-tom-noble]

UPSTREAM_WORKSPACE takes a list of values: UPSTREAM_WORKSPACE: github:rhaschke/python_qt_binding#silent-external-warnings github:rr-tom-noble/common_msgs#feature/add-arrow-strip-marker-type Yes, please uncomment your final code. That's the idea of using the correct upstream - to test the final code.

Thanks. I've made those changes now :+1:

[rr-tom-noble]

@rhaschke The ABI build seems to be failing due to missing the ARROW_STRIP constant. Am I right in thinking that adding github:rr-tom-noble/common_msgs#feature/add-arrow-strip-marker-type to .github/workflows/ci.yaml should result in that version of common_msgs being built against? Any ideas why it might not be working?

[rr-tom-noble]

Run rhaschke/industrial_ci@master
  with:
  env:
    UPSTREAM_WORKSPACE: github:rhaschke/python_qt_binding#silent-external-warnings
    CCACHE_DIR: /home/runner/work/rviz/rviz/.ccache
    BASEDIR: /home/runner/work
    DOCKER_IMAGE: rhaschke/ici:rviz-noetic-ros
    CACHE_PREFIX: noetic
    ABICHECK_URL: github:ros-visualization/rviz#1.1[4](https://github.com/ros-visualization/rviz/actions/runs/4499797469/jobs/7928292247?pr=1786#step:5:4).1[5](https://github.com/ros-visualization/rviz/actions/runs/4499797469/jobs/7928292247?pr=1786#step:5:5)
    GHA_CACHE_SAVE: always

Ah, I think I forgot to add it somewhere

[rr-tom-noble]

Okay, I've added that dependency to abi.yaml too

[rhaschke]

Please merge https://github.com/rivelinrobotics/rviz/pull/1

[rr-tom-noble]

Thanks for the changes. Done :+1:

[rr-tom-noble]

Hey @rhaschke, I noticed the ABI workflow failed here: https://github.com/ros-visualization/rviz/actions/runs/4510995093/jobs/7945031975

Is that something your changes should've fixed, or shall I investigate that?

[rhaschke]

The ABI failure is unrelated and stems from a previous PR. Don't care about that one. The major hurdle for this PR is now to get the message changes merged.

[rr-tom-noble]

@tfoote Has recommended merging the visualization_msgs changes into ROS2 first. We're planning to migrate to ROS2 in April, so a ROS1 backport would not be super useful to us by then. I'll leave the PR open in case it's of interest to anyone else, but feel free to close if not. I'll raise an equivalent PR with rviz2 next month.