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

feat: update trafficlight roi styling #6985

Merged
merged 22 commits into from
Jun 28, 2024
Merged

Conversation

KhalilSelyan
Copy link
Contributor

@KhalilSelyan KhalilSelyan commented May 12, 2024

Description

This PR introduces several enhancements to the traffic light ROI visualization in RViz to improve its accuracy and usability. Key changes include fixing the ROI colors to better match real-world traffic lights, replacing textual overlays with intuitive graphical shapes that represent the traffic light status, and adjusting the positioning and sizing of the probability of detection display. These updates aim to provide a cleaner, more informative visualization that aids users in better decision-making and data analysis.

Effects on System Behavior

These changes are visual and configurational; they do not affect the underlying system behavior of RViz or the traffic light detection functionalities. The visual enhancements are designed to improve user interaction without impacting system performance.

Pre-review Checklist for the PR Author

The PR author must check the checkboxes below when creating the PR.

In-review Checklist for the PR Reviewers

The PR reviewers must check the checkboxes below before approval.

Post-review Checklist for the PR Author

The PR author must check the checkboxes below before merging.

  • All discussions have been resolved or are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@KhalilSelyan KhalilSelyan added component:perception Advanced sensor data processing and environment understanding. (auto-assigned) type:ui-ux User interface, user experience, graphical user interfaces. labels May 12, 2024
@KhalilSelyan KhalilSelyan requested a review from xmfcx May 12, 2024 00:46
@KhalilSelyan KhalilSelyan self-assigned this May 12, 2024
@KhalilSelyan KhalilSelyan linked an issue May 12, 2024 that may be closed by this pull request
5 tasks
Signed-off-by: KhalilSelyan <[email protected]>
@yukkysaito
Copy link
Contributor

@KhalilSelyan Can you share a screenshot of what the visualization will look like, it might be nice to have it in the README.

@xmfcx xmfcx added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label May 12, 2024
@KhalilSelyan
Copy link
Contributor Author

@yukkysaito

Screenshot from 2024-05-12 03-10-41

image

these are all the shapes would be drawn on top of the rectangle, and the probability would be on the side and out of 100 instead of 0.xxxxx

@github-actions github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label May 13, 2024
@KhalilSelyan
Copy link
Contributor Author

@xmfcx I made the update as requested, using the icons instead of cv draw

Screenshot from 2024-05-13 04-44-36

@kminoda kminoda requested a review from tzhong518 as a code owner May 17, 2024 01:18
@KhalilSelyan
Copy link
Contributor Author

using the icons instead of cv:draw and more contained icon + probability
image

@KhalilSelyan
Copy link
Contributor Author

@xmfcx Other shapes for reference

Screenshot from 2024-05-27 17-40-25
Screenshot from 2024-05-27 17-51-57
Screenshot from 2024-05-27 17-58-26
Screenshot from 2024-05-27 18-09-06

@xmfcx
Copy link
Contributor

xmfcx commented May 27, 2024

The text is not readable, could you make it larger?

Also the text box should be connected to the bounding box rectangle.

@KhalilSelyan
Copy link
Contributor Author

@xmfcx is it better with thicker or less thick text (i think thicker)
image
image

@xmfcx
Copy link
Contributor

xmfcx commented May 31, 2024

is it better with thicker or less thick text (i think thicker)

Yes, thicker is better 👍

image

The filled label box should be adjacent but not under the detection box.

image

Unknown box should follow the same style, ? part should be in the filled box.

image

The filled box should be as wide as needed, currently it has a fixed width.

Once applying these changes, could you also show it with the other symbols too?

@KhalilSelyan
Copy link
Contributor Author

Screenshot from 2024-05-31 15-03-21
Screenshot from 2024-05-31 15-07-03
Screenshot from 2024-05-31 15-10-51
Screenshot from 2024-05-31 15-12-46
Screenshot from 2024-05-31 15-18-00
Screenshot from 2024-05-31 15-21-36
Screenshot from 2024-05-31 15-28-02

@xmfcx i did all the necessary changes, you might notice the % text has extra padding on the right, i reduced it in the "x" sign, you can let me know if you think its better with the extra padding or without and we keep whichever version of them

@xmfcx
Copy link
Contributor

xmfcx commented May 31, 2024

let me know if you think its better with the extra padding or without

It's better without the padding.

Could you increase the font and symbol sizes a bit? Some are not legible.

Also FONT_HERSHEY_PLAIN might be better as a small font but I'm not sure.

@KhalilSelyan
Copy link
Contributor Author

KhalilSelyan commented May 31, 2024

@xmfcx pushed the font scale from 0.7 to 0.75
font hershey simplex
image

font hershey plain with same scale looks terrible sadly much less readable
image

Also just upped the size of the symbol as well it would look like this
image

Copy link
Contributor

@tzhong518 tzhong518 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@kminoda
Copy link
Contributor

kminoda commented Jun 28, 2024

@xmfcx Hi, would you check this PR? Your review request is being a blocker.

@xmfcx xmfcx merged commit 58d45d6 into main Jun 28, 2024
22 of 24 checks passed
@xmfcx xmfcx deleted the style/trafficlight-roi-styling branch June 28, 2024 08:22
mitukou1109 pushed a commit to mitukou1109/autoware.universe that referenced this pull request Jul 2, 2024
palas21 pushed a commit to palas21/autoware.universe that referenced this pull request Jul 12, 2024
tby-udel pushed a commit to tby-udel/autoware.universe that referenced this pull request Jul 14, 2024
KhalilSelyan added a commit that referenced this pull request Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:perception Advanced sensor data processing and environment understanding. (auto-assigned) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:documentation Creating or refining documentation. (auto-assigned) type:ui-ux User interface, user experience, graphical user interfaces.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Enhance Styling for Traffic Light ROI Visualization in RViz
5 participants