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

Detect orthogonal lines using RANSAC #1314

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

julianschuler
Copy link
Member

@julianschuler julianschuler commented Jul 18, 2024

Why? What?

This PR implements detecting orthogonal lines in the field using RANSAC. This improves the placement of line segments near corners.

Depends on #1334.

Since our image_to_ground is currently resulting in orthogonal lines not always being orthogonal, it may currently result in worse line fittings.

This feature is currently gated behind the ransac_fit_two_lines parameter, which is false by default.

ToDo / Known Issues

  • Evaluate line detection parameters, e.g. if ransac_iterations should be increased.

Ideas for Next Iterations (Not This PR)

  • Extract type of junction from orthogonal lines and use it directly within localization

How to Test

Enable recording and upload to a NAO. Watch the replay and have a look at the image panel with the line detection overlay enabled. The green lines show the fitted orthogonal lines, the red ones the fitted lines. Note that only the blue lines are the extracted line segments used for localization.

@julianschuler julianschuler force-pushed the in-the-corner branch 4 times, most recently from de0a1d2 to cf101c5 Compare July 19, 2024 07:22
@julianschuler julianschuler changed the title Detect corners using RANSAC Detect orthogonal lines using RANSAC Jul 19, 2024
@pejotejo
Copy link
Contributor

einmal mit kalibrierter Kamera testen

let clockwise_normal_vector = Direction::Clockwise
.rotate_vector_90_degrees(direction_vector)
.normalize();
Copy link
Member

Choose a reason for hiding this comment

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

Not your change, but I'd expect the rotate_vector_90_degrees to be a function of a vector taking the Direction as argument...

Resolve when read, or maybe even open an issue for that? What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, i will open an Issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 25 to 26
pub direction1: Vector2<Frame>,
pub direction2: Vector2<Frame>,
Copy link
Member

Choose a reason for hiding this comment

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

This naming has 90% overlap in the field names. This is easy to mistype...

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, changed to first_direction and second_direction.

Comment on lines 55 to 57
line1
.squared_distance_to(point)
.min(line2.squared_distance_to(point))
Copy link
Member

Choose a reason for hiding this comment

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

extract to variable to increase readability

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, implemented.

PathIntrospect,
PathDeserialize,
)]
pub struct TwoLines<Frame> {
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this called Corner if the docstring calls it corner?

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially the geometry just matched corners (i.e. two rays starting at the same point), which is now changed to two intersecting lines. I now updated the docstring accordingly.

#[default]
None,
Line(Line2<Frame>),
TwoLines(TwoLines<Frame>),
Copy link
Member

Choose a reason for hiding this comment

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

same here: maybe call it Corner?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

Comment on lines 27 to 31
if two_lines.direction2.norm() > f32::EPSILON {
Self::TwoLines(two_lines)
} else {
Self::Line(line)
}
Copy link
Member

Choose a reason for hiding this comment

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

What situation do you want to capture here? Having the same point twice?!

Copy link
Member Author

Choose a reason for hiding this comment

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

The case that the three points are (nearly) colinear.

{
unused_points
.into_iter()
.filter(|point| self.squared_distance_to(**point) <= maximum_score_distance_squared)
Copy link
Member

Choose a reason for hiding this comment

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

You might be able to do something like

Suggested change
.filter(|point| self.squared_distance_to(**point) <= maximum_score_distance_squared)
.filter(|&point| self.squared_distance_to(point) <= maximum_score_distance_squared)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, implemented.

#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq)]
pub enum RansacFeature<Frame> {
#[default]
None,
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a None feature type?! This sounds weird to me...

Copy link
Member Author

Choose a reason for hiding this comment

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

This is intended to cover the case that e.g. not enough points remain for describing a line. I will have a look at whether Option<RansacFeature<Frame>> might be more appropriate here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I replaced this now with Option::None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Request for Review
Development

Successfully merging this pull request may close these issues.

3 participants