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

Replace circle heading handle with triangle #527

Merged
merged 8 commits into from
Jul 15, 2024

Conversation

Nsl106
Copy link
Contributor

@Nsl106 Nsl106 commented Jul 11, 2024

Intended to fix #341, just a simple change to make the heading handle on waypoints a triangle so the front of the robot is more clear.
347597606-8313d2f9-c9e3-48a2-8fa5-275f67db9b0c

Copy link
Member

@calcmogul calcmogul left a comment

Choose a reason for hiding this comment

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

The clickable area feels too small with a side length of 0.2. A side length of 0.26935 gives equivalent area:

triangle area = circle area
1/2 bh = πr²
1/2 (l)(√3/2 l) = πr²
√3/4 l² = πr²
l² = 4πr²/√3
l = √(4πr²/√3)
l = 2r√(π/√3)
l = 2 * 0.1 √(π/√3)
l ≈ 0.26935

Copy link
Member

@calcmogul calcmogul left a comment

Choose a reason for hiding this comment

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

The triangle point math in render() checks out.

The white circle on the playback robot needs to be changed to a triangle. I think it's rendered in InterpolatedRobot.tsx.
Screenshot_20240711_124026

@shueja shueja closed this Jul 11, 2024
@shueja shueja reopened this Jul 11, 2024
@shueja
Copy link
Collaborator

shueja commented Jul 11, 2024

The playback robot is InterpolatedRobot.tsx.

@Nsl106
Copy link
Contributor Author

Nsl106 commented Jul 11, 2024

Where should the shared code between them go? Or would it be better to just copy-paste the same math from OverlayWaypoint.tsx? Also thanks for cleaning up the math comments :)

@calcmogul
Copy link
Member

calcmogul commented Jul 11, 2024

It's not that much code, and it's clear what it does, so I think it's fine to copy-paste.

@Nsl106
Copy link
Contributor Author

Nsl106 commented Jul 13, 2024

What would you think about making the center circle a triangle or arrow (similar to the advantagescope odometry page), and making the heading handle only be visible when the waypoint is selected?

@calcmogul
Copy link
Member

calcmogul commented Jul 13, 2024

making the heading handle only be visible when the waypoint is selected

That makes it less clear how you're supposed to interact with a pose waypoint, and I don't think it helps avoid visual clutter. I do like the idea of a central arrow though. Not sure how waypoint dragging would work.

@Nsl106
Copy link
Contributor Author

Nsl106 commented Jul 13, 2024

It could just be if you grab it anywhere but the heading handle, not sure if that would be clear enough though.

@calcmogul
Copy link
Member

calcmogul commented Jul 13, 2024

Having the waypoint translate if you missed the rotation handle sounds frustrating. The clickable areas for translation and rotation are currently far apart, and missing one of them results in nothing happening.

@calcmogul calcmogul changed the title Triangle waypoint handle Replace circle heading handle with triangle Jul 15, 2024
@calcmogul calcmogul merged commit 575a74c into SleipnirGroup:main Jul 15, 2024
25 checks passed
@Nsl106 Nsl106 deleted the triangle-pose-handle branch August 24, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace the dot handle on the robot UI element with a triangle
3 participants