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

Rotating Caliper in Hull2DHelpers #1052

Merged
merged 21 commits into from
Oct 17, 2015
Merged

Conversation

kerautret
Copy link
Member

As discussed with Jaco last week, to simplify the alphaThickSegment I add the implementation of the rotating caliper (hamos , 1978) working this horizontal/vertical width or from the euclidean one.

@kerautret kerautret added this to the 0.9.1 milestone Oct 12, 2015
@dcoeurjo
Copy link
Member

Thanks.
What are the differences/dependencies between this PR and alpha thick DSS one ?

@kerautret
Copy link
Member Author

In fact the PR on alpha thick DSS will use it for a better definition and simpler integration (but almost equivalent form the initial version). It test the integration actually on another branch.

@kerautret
Copy link
Member Author

The width definition was given from the axis diagonal projection, but here we just have two classic def.

@kerautret
Copy link
Member Author

For the review, you can wait I am removing the std::pair<> which is not very convenient to pass in ref...

@kerautret
Copy link
Member Author

@dcoeurjo that ready for review, I tested the integration with the AlphaThickSegment and it works with the two different definition. It could be nice if you have time to review before finishing the AlphaThickSegment update that I start to modify.

## New Features / Critical Changes
- *Geometry Package*
- Hull2DHelpers: implementation of the rotating caliper algorithm to compute
the width (vertical/horizontal or euclidean) of a convex hull.
Copy link
Member

Choose a reason for hiding this comment

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

euclidean -> Euclidean

@@ -0,0 +1,271 @@
#FIG 3.2 Produced by xfig version 3.2.5c
Portrait
Copy link
Member

Choose a reason for hiding this comment

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

are you sure you want to include this file to DGtal ?

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure very useful ;)

double awaitedThHV = DGtal::functions::Hull2D::getThicknessAntipodalPair(Point(0,0), Point(11,1), Point(2,6), DGtal::functions::Hull2D::HorizontalVerticalThickness );
double awaitedThE = DGtal::functions::Hull2D::getThicknessAntipodalPair(Point(0,0), Point(11,1), Point(2,6), DGtal::functions::Hull2D::EuclideanThickness );
trace.info() << "Thickness HV = " << thicknessHV << std::endl;
trace.info() << "Thickness HV awaited = " << awaitedThHV << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

awaited-> expected ?

Copy link
Member

Choose a reason for hiding this comment

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

"Expected Thickness HV = " ...

Copy link
Member Author

Choose a reason for hiding this comment

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

yes thanks

@dcoeurjo
Copy link
Member

very nice PR, though.

@dcoeurjo
Copy link
Member

If you want to try Catch based unit test file, it would be nice to have a test file (catch based) only testing the thickness computation..

@kerautret
Copy link
Member Author

Thanks for all the remarks.
ok I look for the catch.

@kerautret
Copy link
Member Author

done with the catch ;)

@dcoeurjo
Copy link
Member

Great !
Just a minor thing: the "-catch" extension to test filename is not something I want to promote. Could you please rename this file to testConvexHullThickness for instance ?

(catch is cool, isn't it ? 😄)

@kerautret
Copy link
Member Author

yes done, I was wondering ;)
Yes thanks, catch is nice and simple to apply, direct with the script of DGTalScript ;)

@dcoeurjo
Copy link
Member

great merging

dcoeurjo added a commit that referenced this pull request Oct 17, 2015
Rotating Caliper in Hull2DHelpers
@dcoeurjo dcoeurjo merged commit e60d843 into DGtal-team:master Oct 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants