Skip to content

Commit aee1be1

Browse files
committed
Move linestring splitting into geom.cpp and fix it
Moves the linestring splitting into its own function split_linestring and add tests. They show that the original function created invalid linestrings in some cases, so this also fixes the algorithm.
1 parent 0567c96 commit aee1be1

File tree

4 files changed

+238
-66
lines changed

4 files changed

+238
-66
lines changed

src/geom.cpp

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,77 @@ linestring_t::linestring_t(osmium::NodeRefList const &nodes,
5353
}
5454
}
5555

56+
void split_linestring(linestring_t const &line, double split_at,
57+
std::vector<linestring_t> *out)
58+
{
59+
double dist = 0;
60+
osmium::geom::Coordinates prev_pt{};
61+
out->emplace_back();
62+
63+
for (auto const this_pt : line) {
64+
if (prev_pt.valid()) {
65+
double const delta = distance(prev_pt, this_pt);
66+
67+
// figure out if the addition of this point would take the total
68+
// length of the line in `segment` over the `split_at` distance.
69+
70+
if (dist + delta > split_at) {
71+
auto const splits =
72+
(size_t)std::floor((dist + delta) / split_at);
73+
// use the splitting distance to split the current segment up
74+
// into as many parts as necessary to keep each part below
75+
// the `split_at` distance.
76+
osmium::geom::Coordinates ipoint;
77+
for (size_t j = 0; j < splits; ++j) {
78+
double const frac =
79+
((double)(j + 1) * split_at - dist) / delta;
80+
ipoint = interpolate(this_pt, prev_pt, frac);
81+
if (frac != 0.0) {
82+
out->back().add_point(ipoint);
83+
}
84+
// start a new segment
85+
out->emplace_back();
86+
out->back().add_point(ipoint);
87+
}
88+
// reset the distance based on the final splitting point for
89+
// the next iteration.
90+
if (this_pt == ipoint) {
91+
dist = 0;
92+
prev_pt = this_pt;
93+
continue;
94+
} else {
95+
dist = distance(this_pt, ipoint);
96+
}
97+
} else {
98+
dist += delta;
99+
}
100+
}
101+
102+
out->back().add_point(this_pt);
103+
104+
prev_pt = this_pt;
105+
}
106+
107+
if (out->back().size() <= 1) {
108+
out->pop_back();
109+
}
110+
}
111+
112+
void make_line(osmium::NodeRefList const &nodes, reprojection const &proj,
113+
double split_at, std::vector<linestring_t> *out)
114+
{
115+
linestring_t line{nodes, proj};
116+
117+
if (line.empty()) {
118+
return;
119+
}
120+
121+
if (split_at > 0.0) {
122+
split_linestring(line, split_at, out);
123+
} else {
124+
out->emplace_back(std::move(line));
125+
}
126+
}
127+
56128
} // namespace geom
57129

src/geom.hpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,21 @@ operator<<(std::basic_ostream<TChar, TTraits> &out, const linestring_t &line)
118118
return out << ')';
119119
}
120120

121+
/**
122+
* Possibly split linestring into several linestrings making sure each one
123+
* is no longer than split_at.
124+
*
125+
* \param line The input line string.
126+
* \param split_at The maximum length (using Euclidean distance) of each
127+
* resulting linestring.
128+
* \param out Add resulting linestrings to this vector.
129+
*/
130+
void split_linestring(linestring_t const &line, double split_at,
131+
std::vector<linestring_t> *out);
132+
133+
void make_line(osmium::NodeRefList const &nodes, reprojection const &proj,
134+
double split_at, std::vector<linestring_t> *out);
135+
121136
} // namespace geom
122137

123138
#endif // OSM2PGSQL_GEOM_HPP

src/osmium-builder.cpp

Lines changed: 8 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -71,75 +71,17 @@ osmium_builder_t::wkbs_t
7171
osmium_builder_t::get_wkb_line(osmium::WayNodeList const &nodes,
7272
double split_at)
7373
{
74-
wkbs_t ret;
75-
76-
bool const do_split = split_at > 0.0;
74+
std::vector<linestring_t> linestrings;
75+
geom::make_line(nodes, *m_proj, split_at, &linestrings);
7776

78-
double dist = 0;
79-
osmium::geom::Coordinates prev_pt;
80-
m_writer.linestring_start();
81-
size_t curlen = 0;
77+
wkbs_t ret;
8278

83-
for (auto const &node : nodes) {
84-
if (!node.location().valid()) {
85-
continue;
79+
for (auto const &line : linestrings) {
80+
m_writer.linestring_start();
81+
for (auto const &coord : line) {
82+
m_writer.add_location(coord);
8683
}
87-
88-
auto const this_pt = m_proj->reproject(node.location());
89-
if (prev_pt.valid()) {
90-
if (prev_pt == this_pt) {
91-
continue;
92-
}
93-
94-
if (do_split) {
95-
double const delta = distance(prev_pt, this_pt);
96-
97-
// figure out if the addition of this point would take the total
98-
// length of the line in `segment` over the `split_at` distance.
99-
100-
if (dist + delta > split_at) {
101-
auto const splits =
102-
(size_t)std::floor((dist + delta) / split_at);
103-
// use the splitting distance to split the current segment up
104-
// into as many parts as necessary to keep each part below
105-
// the `split_at` distance.
106-
osmium::geom::Coordinates ipoint;
107-
for (size_t j = 0; j < splits; ++j) {
108-
double const frac =
109-
((double)(j + 1) * split_at - dist) / delta;
110-
ipoint = interpolate(this_pt, prev_pt, frac);
111-
m_writer.add_location(ipoint);
112-
ret.push_back(m_writer.linestring_finish(curlen + 1));
113-
// start a new segment
114-
m_writer.linestring_start();
115-
m_writer.add_location(ipoint);
116-
curlen = 1;
117-
}
118-
// reset the distance based on the final splitting point for
119-
// the next iteration.
120-
if (this_pt == ipoint) {
121-
dist = 0;
122-
m_writer.linestring_finish(0);
123-
m_writer.linestring_start();
124-
curlen = 0;
125-
} else {
126-
dist = distance(this_pt, ipoint);
127-
}
128-
} else {
129-
dist += delta;
130-
}
131-
}
132-
}
133-
134-
m_writer.add_location(this_pt);
135-
++curlen;
136-
137-
prev_pt = this_pt;
138-
}
139-
140-
auto const wkb = m_writer.linestring_finish(curlen);
141-
if (curlen > 1) {
142-
ret.push_back(wkb);
84+
ret.push_back(m_writer.linestring_finish(line.size()));
14385
}
14486

14587
return ret;

tests/test-geom.cpp

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,146 @@ TEST_CASE("geom::linestring_t", "[NoDB]")
6262
REQUIRE(it == ls1.cend());
6363
}
6464

65+
TEST_CASE("geom::split_linestring w/o split", "[NoDB]")
66+
{
67+
geom::linestring_t const line{Coordinates{0, 0}, Coordinates{1, 2},
68+
Coordinates{2, 2}};
69+
70+
std::vector<geom::linestring_t> result;
71+
72+
geom::split_linestring(line, 10.0, &result);
73+
74+
REQUIRE(result.size() == 1);
75+
76+
REQUIRE(result[0].size() == 3);
77+
REQUIRE(result[0][0] == Coordinates{0, 0});
78+
REQUIRE(result[0][1] == Coordinates{1, 2});
79+
REQUIRE(result[0][2] == Coordinates{2, 2});
80+
}
81+
82+
TEST_CASE("geom::split_linestring with split 0.5", "[NoDB]")
83+
{
84+
geom::linestring_t const line{Coordinates{0, 0}, Coordinates{1, 0}};
85+
86+
std::vector<geom::linestring_t> result;
87+
88+
geom::split_linestring(line, 0.5, &result);
89+
90+
REQUIRE(result.size() == 2);
91+
92+
REQUIRE(result[0].size() == 2);
93+
REQUIRE(result[0][0] == Coordinates{0, 0});
94+
REQUIRE(result[0][1] == Coordinates{0.5, 0});
95+
96+
REQUIRE(result[1].size() == 2);
97+
REQUIRE(result[1][0] == Coordinates{0.5, 0});
98+
REQUIRE(result[1][1] == Coordinates{1, 0});
99+
}
100+
101+
TEST_CASE("geom::split_linestring with split 0.4", "[NoDB]")
102+
{
103+
geom::linestring_t const line{Coordinates{0, 0}, Coordinates{1, 0}};
104+
105+
std::vector<geom::linestring_t> result;
106+
107+
geom::split_linestring(line, 0.4, &result);
108+
109+
REQUIRE(result.size() == 3);
110+
111+
REQUIRE(result[0].size() == 2);
112+
REQUIRE(result[0][0] == Coordinates{0, 0});
113+
REQUIRE(result[0][1] == Coordinates{0.4, 0});
114+
115+
REQUIRE(result[1].size() == 2);
116+
REQUIRE(result[1][0] == Coordinates{0.4, 0});
117+
REQUIRE(result[1][1] == Coordinates{0.8, 0});
118+
119+
REQUIRE(result[2].size() == 2);
120+
REQUIRE(result[2][0] == Coordinates{0.8, 0});
121+
REQUIRE(result[2][1] == Coordinates{1, 0});
122+
}
123+
124+
TEST_CASE("geom::split_linestring with split 1.0 at start", "[NoDB]")
125+
{
126+
geom::linestring_t const line{Coordinates{0, 0}, Coordinates{2, 0},
127+
Coordinates{3, 0}, Coordinates{4, 0}};
128+
129+
std::vector<geom::linestring_t> result;
130+
131+
geom::split_linestring(line, 1.0, &result);
132+
133+
REQUIRE(result.size() == 4);
134+
135+
REQUIRE(result[0].size() == 2);
136+
REQUIRE(result[0][0] == Coordinates{0, 0});
137+
REQUIRE(result[0][1] == Coordinates{1, 0});
138+
139+
REQUIRE(result[1].size() == 2);
140+
REQUIRE(result[1][0] == Coordinates{1, 0});
141+
REQUIRE(result[1][1] == Coordinates{2, 0});
142+
143+
REQUIRE(result[2].size() == 2);
144+
REQUIRE(result[2][0] == Coordinates{2, 0});
145+
REQUIRE(result[2][1] == Coordinates{3, 0});
146+
147+
REQUIRE(result[3].size() == 2);
148+
REQUIRE(result[3][0] == Coordinates{3, 0});
149+
REQUIRE(result[3][1] == Coordinates{4, 0});
150+
}
151+
152+
TEST_CASE("geom::split_linestring with split 1.0 in middle", "[NoDB]")
153+
{
154+
geom::linestring_t const line{Coordinates{0, 0}, Coordinates{1, 0},
155+
Coordinates{3, 0}, Coordinates{4, 0}};
156+
157+
std::vector<geom::linestring_t> result;
158+
159+
geom::split_linestring(line, 1.0, &result);
160+
161+
REQUIRE(result.size() == 4);
162+
163+
REQUIRE(result[0].size() == 2);
164+
REQUIRE(result[0][0] == Coordinates{0, 0});
165+
REQUIRE(result[0][1] == Coordinates{1, 0});
166+
167+
REQUIRE(result[1].size() == 2);
168+
REQUIRE(result[1][0] == Coordinates{1, 0});
169+
REQUIRE(result[1][1] == Coordinates{2, 0});
170+
171+
REQUIRE(result[2].size() == 2);
172+
REQUIRE(result[2][0] == Coordinates{2, 0});
173+
REQUIRE(result[2][1] == Coordinates{3, 0});
174+
175+
REQUIRE(result[3].size() == 2);
176+
REQUIRE(result[3][0] == Coordinates{3, 0});
177+
REQUIRE(result[3][1] == Coordinates{4, 0});
178+
}
179+
180+
TEST_CASE("geom::split_linestring with split 1.0 at end", "[NoDB]")
181+
{
182+
geom::linestring_t const line{Coordinates{0, 0}, Coordinates{1, 0},
183+
Coordinates{2, 0}, Coordinates{4, 0}};
184+
185+
std::vector<geom::linestring_t> result;
186+
187+
geom::split_linestring(line, 1.0, &result);
188+
189+
REQUIRE(result.size() == 4);
190+
191+
REQUIRE(result[0].size() == 2);
192+
REQUIRE(result[0][0] == Coordinates{0, 0});
193+
REQUIRE(result[0][1] == Coordinates{1, 0});
194+
195+
REQUIRE(result[1].size() == 2);
196+
REQUIRE(result[1][0] == Coordinates{1, 0});
197+
REQUIRE(result[1][1] == Coordinates{2, 0});
198+
199+
REQUIRE(result[2].size() == 2);
200+
REQUIRE(result[2][0] == Coordinates{2, 0});
201+
REQUIRE(result[2][1] == Coordinates{3, 0});
202+
203+
REQUIRE(result[3].size() == 2);
204+
REQUIRE(result[3][0] == Coordinates{3, 0});
205+
REQUIRE(result[3][1] == Coordinates{4, 0});
206+
}
207+

0 commit comments

Comments
 (0)