Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set per-point color on markers #318

Merged
merged 1 commit into from
Dec 8, 2021
Merged

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Nov 20, 2021

🎉 New feature

Part of

Requires

Builds on top of

Summary

If a message specifies multiple colors, use different colors for each point. Otherwise, fallback to current behaviour.

The branch is currently on ign-gui6 while I work on the implementation, but the goal is to merge this into main after gazebo-tooling/release-tools#554.

Test it

Run the marker example and see that both the single material and the per-point material work:

pts_color

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

@chapulina chapulina added needs upstream release Blocked by a release of an upstream library 🌱 garden Ignition Garden labels Nov 20, 2021
@chapulina chapulina force-pushed the chapulina/7/marker_point_color branch from c40f51a to b7e3a24 Compare November 25, 2021 00:19
@chapulina chapulina marked this pull request as ready for review November 30, 2021 21:57
@chapulina chapulina marked this pull request as draft November 30, 2021 21:57
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina force-pushed the chapulina/7/marker_point_color branch from 652dc0b to 26125a8 Compare December 6, 2021 23:03
@chapulina chapulina marked this pull request as ready for review December 6, 2021 23:04
@iche033
Copy link
Contributor

iche033 commented Dec 7, 2021

hmm not sure what's changed, the points look like they are more spread out? I'm now testing with all Garden branches
marker_point_colors

@iche033
Copy link
Contributor

iche033 commented Dec 7, 2021

oh I think it just needs #321 to be ported forward

Copy link
Contributor

@caguero caguero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me as expected.

@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #318 (26125a8) into main (4b3ca69) will decrease coverage by 0.11%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #318      +/-   ##
==========================================
- Coverage   65.74%   65.62%   -0.12%     
==========================================
  Files          34       34              
  Lines        4752     4751       -1     
==========================================
- Hits         3124     3118       -6     
- Misses       1628     1633       +5     
Impacted Files Coverage Δ
src/plugins/marker_manager/MarkerManager.cc 59.45% <25.00%> (-1.15%) ⬇️
src/plugins/scene3d/Scene3D.cc 48.83% <0.00%> (-0.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b3ca69...26125a8. Read the comment docs.

@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Dec 8, 2021
@chapulina chapulina merged commit 67a78e4 into main Dec 8, 2021
@chapulina chapulina deleted the chapulina/7/marker_point_color branch December 8, 2021 17:36
@chapulina chapulina mentioned this pull request Jan 7, 2022
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants