Skip to content

Conversation

@vissarion
Copy link
Member

@vissarion vissarion commented Dec 5, 2017

This PR adds support for point-box and box-box distance for spherical and geographic CS. It uses the meridian distance to compute distances from the horizontal line of a box and point-segment distance for distances to the vertical line.

vissarion added 19 commits July 19, 2017 13:06
…dd meridian distance computation as special case
@awulkiew awulkiew added this to the 1.67 milestone Dec 5, 2017
@vissarion vissarion force-pushed the feature/distance_box branch from a2fc360 to a0b7839 Compare January 8, 2018 10:07
@awulkiew
Copy link
Member

Thanks! I'm ok with merging however I have 2 remarks:

  1. I'm not sure if strategies is a good directory for generic P/B and B/B strategies. I'd rather put them in strategies/spherical. Even in the same files as spherical P/B and B/B strategies, in the strategy::distance::details namespace to hide it from the users.
  2. Using this opportunity I think we should rename distance strategies in a consistent way. Throwing cross_track part would be a good start. Then we'd have e.g.: strategy::distance::geographic_box_box. But we should take into account all geometries combinations and design consistent scheme. I'm ok with doing it in a different PR.

@vissarion
Copy link
Member Author

  1. I'm not sure if strategies is a good directory for generic P/B and B/B strategies. I'd rather put them in strategies/spherical. Even in the same files as spherical P/B and B/B strategies, in the strategy::distance::details namespace to hide it from the users.

I did the change following your suggestion. I am ok for now but in the future it is not very clear to have generic algorithms (i.e. for both geographic and spherical) hidden in spherical or at least be consistent in similar cases.

  1. Using this opportunity I think we should rename distance strategies in a consistent way. Throwing cross_track part would be a good start. Then we'd have e.g.: strategy::distance::geographic_box_box. But we should take into account all geometries combinations and design consistent scheme. I'm ok with doing it in a different PR.

I agree, there are many issues with the naming of distance strategies. Maybe better in a different PR.

@awulkiew
Copy link
Member

Thanks.

I noticed that you commented out #include <boost/geometry/core/srs.hpp> in test file which was included for sphere or spheroid. FYI, projections PR (#394) moved them from geometry/core directory to geometry/srs directory. geometry/srs/srs.hpp includes both models but also projections and transformations. So if you only need sphere or spheroid it's better to include <boost/geometry/srs/sphere.hpp> or <boost/geometry/srs/spheroid.hpp> respectively. But if the code works as it is right now (without the includes) it probably means that these models are included in somewhere else, probably in some strategy files.

So I'll merge this PR now and if needed tweak the includes afterwards.

@awulkiew awulkiew merged commit 18bc911 into boostorg:develop Jan 22, 2018
@vissarion
Copy link
Member Author

I noticed that you commented out #include <boost/geometry/core/srs.hpp> in test file which was included for sphere or spheroid. FYI, projections PR (#394) moved them from geometry/core directory to geometry/srs directory. geometry/srs/srs.hpp includes both models but also projections and transformations. So if you only need sphere or spheroid it's better to include <boost/geometry/srs/sphere.hpp> or <boost/geometry/srs/spheroid.hpp> respectively. But if the code works as it is right now (without the includes) it probably means that these models are included in somewhere else, probably in some strategy files.

The code compiles as it is, that's why I commented out. Thanks.

@vissarion vissarion deleted the feature/distance_box branch April 12, 2018 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants