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

Fix behavior of SACMODEL_PARALLEL_LINE #969

Merged
merged 6 commits into from
Nov 22, 2014

Conversation

taketwo
Copy link
Member

@taketwo taketwo commented Oct 21, 2014

This pull request fixes the behavior of SACMODEL_PARALLEL_LINE and makes some clean up in the related files.

Despite the name and in contradiction with the documentation, currently the model searches for lines perpendicular to the given axis. This issue has been brought up on the mailing list a long while ago, but no action was taken at that time.

The additional commits in this pull request:

  • remove trailing spaces;
  • fix documentation;
  • put instantiations of SACMODEL_LINE and SACMODEL_PARALLEL_LINE in separate cpp files;
  • add const qualifiers where appropriate;
  • add a unit test for SACMODEL_PARALLEL_LINE with the new, correct behavior.

@VictorLamoine
Copy link
Contributor

Hello, does this pull request fixes #918? I'll take a look at it later today.

@taketwo
Copy link
Member Author

taketwo commented Oct 22, 2014

No, that issue is about planes, this one is about lines.

The old condition checked if the line model is perpendicular to the axis
provided by user, while according to the name of the class and
documentation it should rather check if the line model is parallel to
the axis.
Additionaly rename test_sample_consensus_line_model to reflect the fact
that now it tests multiple line models.
@taketwo
Copy link
Member Author

taketwo commented Nov 22, 2014

@jspricke Can we merge this as well?

jspricke added a commit that referenced this pull request Nov 22, 2014
Fix behavior of SACMODEL_PARALLEL_LINE
@jspricke jspricke merged commit 52de1a6 into PointCloudLibrary:master Nov 22, 2014
@jspricke
Copy link
Member

Sure, thanks for the ping :).

@taketwo taketwo deleted the fix-parallel-line branch November 22, 2014 17:35
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.

3 participants