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

Unifies sac_model_normal_parallel_plane with sac_model_normal_plane, removes duplicated code #696

Merged
merged 1 commit into from
May 24, 2014

Conversation

fran6co
Copy link
Contributor

@fran6co fran6co commented May 23, 2014

The classes are very similar, sac_model_normal_parallel_plane changes isModelValid method to filter out planes that don't comply with the axis restrictions.

@fran6co
Copy link
Contributor Author

fran6co commented May 23, 2014

There is no error, the gcc build timedout

@taketwo
Copy link
Member

taketwo commented May 24, 2014

Looks good to me.

@@ -107,8 +105,7 @@ namespace pcl
*/
SampleConsensusModelNormalParallelPlane (const PointCloudConstPtr &cloud,
bool random = false)
: SampleConsensusModelPlane<PointT> (cloud, random)
, SampleConsensusModelFromNormals<PointT, PointNT> ()
: SampleConsensusModelNormalPlane<PointT, PointNT>(cloud, random)
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add a space in front of the bracket?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

taketwo added a commit that referenced this pull request May 24, 2014
Unifies sac_model_normal_parallel_plane with sac_model_normal_plane, removes duplicated code
@taketwo taketwo merged commit 512ff6e into PointCloudLibrary:master May 24, 2014
@fran6co fran6co deleted the fix-parallel-plane branch May 25, 2014 19: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.

3 participants