Skip to content

Geographic buffer line #1045

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

Merged

Conversation

barendgehrels
Copy link
Collaborator

@barendgehrels barendgehrels commented Jul 27, 2022

This is a fairly large PR adding two strategies for geographic buffers:

  • side_straight
  • join_round

They are now tested and working for linestrings.
Next phase will be: end_round
After that I will test it with multi linestrings and polygons.

It works pretty well in general, for example:
image

This picture is created with QGis and .csv, a next PR will add (optional) possibility to add .csv in testcode. Earlier I often used .svg but (especially for geographic), .csv or .geojson is quite convenient.

{
out << id++ << ";3;" << bg::wkt(p) << std::endl;
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This small .csv part is already there, and looks like:
image

@barendgehrels barendgehrels force-pushed the geographic-buffer-line branch from a117766 to caa4000 Compare July 27, 2022 13:16
@vissarion vissarion added this to the 1.81 milestone Jul 28, 2022
Copy link
Member

@vissarion vissarion left a comment

Choose a reason for hiding this comment

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

Thanks, I am ok, only minor comments.

typename RangeOut
>
inline bool apply(Point const& ip, Point const& vertex,
Point const& perp1, Point const& perp2,
Copy link
Member

Choose a reason for hiding this comment

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

something seems wrong with indentation here?

Copy link
Member

Choose a reason for hiding this comment

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

similar issues in apply() below

// If angle is zero, shift angle a tiny bit to avoid spikes.
calculation_type const eps = angle == 0 ? 1.0e-10 : 0.0;
auto const dir_rad = direct_t::apply(lon_rad, lat_rad,
buffer_distance, angle + eps,
Copy link
Member

Choose a reason for hiding this comment

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

I think is better to indent below lon_rad or use one tab

typename direct_t::result_type
dir_r = direct_t::apply(get_as_radian<0>(point), get_as_radian<1>(point),
buffer_distance, angle,
// If angle is zero, shift angle a tiny bit to avoid spikes.
Copy link
Member

Choose a reason for hiding this comment

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

is this some fix on buffer point geographic strategy to avoid spikes or is another issue introduced by the new buffer line strategies?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was already the case in earlier code. It's now less but not yet 100% disappeared.
We have to come back to it later. If I encounter it, I will create an issue.

typename DistanceStrategy
>
inline result_code apply(Point const& input_p1, Point const& input_p2,
buffer_side_selector side,
Copy link
Member

Choose a reason for hiding this comment

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

should that be passed as const&?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is an enumerator. We normally don't do that.

// For linestrings with flat ends, it's not necessarily the case, there may be points
// too close, especially on artefacts in heavily curved input with flat ends.
// Therefore the default expectation can be modified. Inspect the SVG visually before doing this.
std::size_t too_close = 0, too_far = 0, total = 0;
Copy link
Member

Choose a reason for hiding this comment

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

we use one declaration per line style, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, thanks (and I applied also your other indentation comments)

@barendgehrels barendgehrels force-pushed the geographic-buffer-line branch 3 times, most recently from a4a13a4 to 227c35f Compare August 3, 2022 08:45
@barendgehrels barendgehrels force-pushed the geographic-buffer-line branch from 227c35f to 839a4ed Compare August 3, 2022 09:07
@barendgehrels barendgehrels merged commit 49004c5 into boostorg:develop Aug 3, 2022
@barendgehrels barendgehrels deleted the geographic-buffer-line branch August 3, 2022 09:19
@vissarion vissarion modified the milestones: 1.81, 1.82 Nov 17, 2022
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