From cdf014e35df61d2fbaeacfe446e8e21df77064f3 Mon Sep 17 00:00:00 2001 From: barendgehrels Date: Wed, 27 May 2015 10:56:56 +0200 Subject: [PATCH] [buffer] propagate error information from strategy to buffer inserter --- doc/release_notes.qbk | 5 ++ .../detail/buffer/buffer_inserter.hpp | 62 +++++++++++-------- include/boost/geometry/strategies/buffer.hpp | 11 ++++ .../cartesian/buffer_side_straight.hpp | 34 +++++++--- test/algorithms/buffer/linestring_buffer.cpp | 17 +++-- .../buffer/multi_linestring_buffer.cpp | 12 ++-- test/algorithms/buffer/polygon_buffer.cpp | 6 +- 7 files changed, 96 insertions(+), 51 deletions(-) diff --git a/doc/release_notes.qbk b/doc/release_notes.qbk index df3a5fe49..5acb97380 100644 --- a/doc/release_notes.qbk +++ b/doc/release_notes.qbk @@ -35,11 +35,16 @@ * Upgraded rtree const_query_iterator category to ForwardIterator. +[*Breaking changes] + +* buffer side strategy now returns error_code instead of bool. If you have your own custom side strategy, it should be adapted + [*Solved tickets] * [@https://svn.boost.org/trac/boost/ticket/11113 11113] Support easy enumeration of all elements with BOOST_FOREACH * [@https://svn.boost.org/trac/boost/ticket/11236 11236] Invalid result of centroid() for integer coordinate type * [@https://svn.boost.org/trac/boost/ticket/11232 11232] Feature request - relate() +* [@https://svn.boost.org/trac/boost/ticket/11332 11332] Assertion failure in buffer for extreme coordinate differences [/=================] [heading Boost 1.58] diff --git a/include/boost/geometry/algorithms/detail/buffer/buffer_inserter.hpp b/include/boost/geometry/algorithms/detail/buffer/buffer_inserter.hpp index 0419533e1..45995e817 100644 --- a/include/boost/geometry/algorithms/detail/buffer/buffer_inserter.hpp +++ b/include/boost/geometry/algorithms/detail/buffer/buffer_inserter.hpp @@ -12,9 +12,9 @@ #include #include +#include #include #include - #include #include @@ -227,7 +227,7 @@ struct buffer_range typename EndStrategy, typename RobustPolicy > - static inline bool iterate(Collection& collection, + static inline strategy::buffer::result_code iterate(Collection& collection, Iterator begin, Iterator end, strategy::buffer::buffer_side_selector side, DistanceStrategy const& distance_strategy, @@ -266,7 +266,7 @@ struct buffer_range * pup: penultimate_point */ - bool result = false; + strategy::buffer::result_code result = strategy::buffer::result_no_output; bool first = true; Iterator it = begin; @@ -277,18 +277,25 @@ struct buffer_range for (Iterator prev = it++; it != end; ++it) { generated_side.clear(); - side_strategy.apply(*prev, *it, side, + strategy::buffer::result_code error_code + = side_strategy.apply(*prev, *it, side, distance_strategy, generated_side); - if (generated_side.empty()) + if (error_code == strategy::buffer::result_no_output) { // Because input is simplified, this is improbable, // but it can happen for degenerate geometries // Further handling of this side is skipped continue; } + else if (error_code == strategy::buffer::result_error_numerical) + { + return error_code; + } - result = true; + BOOST_ASSERT(! generated_side.empty()); + + result = strategy::buffer::result_normal; if (! first) { @@ -459,7 +466,7 @@ struct buffer_inserter typename EndStrategy, typename RobustPolicy > - static inline bool iterate(Collection& collection, + static inline strategy::buffer::result_code iterate(Collection& collection, Iterator begin, Iterator end, strategy::buffer::buffer_side_selector side, DistanceStrategy const& distance_strategy, @@ -472,13 +479,14 @@ struct buffer_inserter typedef detail::buffer::buffer_range buffer_range; - bool result = buffer_range::iterate(collection, begin, end, + strategy::buffer::result_code result + = buffer_range::iterate(collection, begin, end, side, distance_strategy, side_strategy, join_strategy, end_strategy, robust_policy, first_p1, first_p2, last_p1, last_p2); // Generate closing join - if (result) + if (result == strategy::buffer::result_normal) { buffer_range::add_join(collection, *(end - 2), @@ -515,7 +523,7 @@ struct buffer_inserter RingInput simplified; detail::buffer::simplify_input(ring, distance, simplified); - bool has_output = false; + strategy::buffer::result_code code = strategy::buffer::result_no_output; std::size_t n = boost::size(simplified); std::size_t const min_points = core_detail::closure::minimum_ring_size @@ -529,21 +537,19 @@ struct buffer_inserter if (distance.negative()) { // Walk backwards (rings will be reversed afterwards) - // It might be that this will be changed later. - // TODO: decide this. - has_output = iterate(collection, boost::rbegin(view), boost::rend(view), + code = iterate(collection, boost::rbegin(view), boost::rend(view), strategy::buffer::buffer_side_right, distance, side_strategy, join_strategy, end_strategy, robust_policy); } else { - has_output = iterate(collection, boost::begin(view), boost::end(view), + code = iterate(collection, boost::begin(view), boost::end(view), strategy::buffer::buffer_side_left, distance, side_strategy, join_strategy, end_strategy, robust_policy); } } - if (! has_output && n >= 1) + if (code == strategy::buffer::result_no_output && n >= 1) { // Use point_strategy to buffer degenerated ring detail::buffer::buffer_point @@ -577,7 +583,7 @@ struct buffer_inserter typename EndStrategy, typename RobustPolicy > - static inline bool iterate(Collection& collection, + static inline strategy::buffer::result_code iterate(Collection& collection, Iterator begin, Iterator end, strategy::buffer::buffer_side_selector side, DistanceStrategy const& distance_strategy, @@ -602,24 +608,27 @@ struct buffer_inserter else { std::vector generated_side; - side_strategy.apply(ultimate_point, penultimate_point, + strategy::buffer::result_code code + = side_strategy.apply(ultimate_point, penultimate_point, strategy::buffer::buffer_side_right, distance_strategy, generated_side); - if (generated_side.empty()) + if (code != strategy::buffer::result_normal) { - return false; + // No output or numerical error + return code; } reverse_p1 = generated_side.front(); } output_point_type first_p2, last_p1, last_p2; - bool result = detail::buffer::buffer_range::iterate(collection, + strategy::buffer::result_code result + = detail::buffer::buffer_range::iterate(collection, begin, end, side, distance_strategy, side_strategy, join_strategy, end_strategy, robust_policy, first_p1, first_p2, last_p1, last_p2); - if (result) + if (result == strategy::buffer::result_normal) { std::vector range_out; end_strategy.apply(penultimate_point, last_p2, ultimate_point, reverse_p1, side, distance_strategy, range_out); @@ -649,28 +658,29 @@ struct buffer_inserter Linestring simplified; detail::buffer::simplify_input(linestring, distance, simplified); - bool has_output = false; + strategy::buffer::result_code code = strategy::buffer::result_no_output; std::size_t n = boost::size(simplified); if (n > 1) { collection.start_new_ring(); output_point_type first_p1; - has_output = iterate(collection, + code = iterate(collection, boost::begin(simplified), boost::end(simplified), strategy::buffer::buffer_side_left, distance, side_strategy, join_strategy, end_strategy, robust_policy, first_p1); - if (has_output) + if (code == strategy::buffer::result_normal) { - iterate(collection, boost::rbegin(simplified), boost::rend(simplified), + code = iterate(collection, + boost::rbegin(simplified), boost::rend(simplified), strategy::buffer::buffer_side_right, distance, side_strategy, join_strategy, end_strategy, robust_policy, first_p1); } collection.finish_ring(); } - if (! has_output && n >= 1) + if (code == strategy::buffer::result_no_output && n >= 1) { // Use point_strategy to buffer degenerated linestring detail::buffer::buffer_point diff --git a/include/boost/geometry/strategies/buffer.hpp b/include/boost/geometry/strategies/buffer.hpp index be3daefec..86bdcc37b 100644 --- a/include/boost/geometry/strategies/buffer.hpp +++ b/include/boost/geometry/strategies/buffer.hpp @@ -83,6 +83,17 @@ enum join_selector join_spike // collinear, with overlap, next segment goes back }; +/*! +\brief Enumerates types of result codes from buffer strategies +\ingroup enum +*/ +enum result_code +{ + result_normal, + result_error_numerical, + result_no_output +}; + }} // namespace strategy::buffer diff --git a/include/boost/geometry/strategies/cartesian/buffer_side_straight.hpp b/include/boost/geometry/strategies/cartesian/buffer_side_straight.hpp index 221dbe030..75247377f 100644 --- a/include/boost/geometry/strategies/cartesian/buffer_side_straight.hpp +++ b/include/boost/geometry/strategies/cartesian/buffer_side_straight.hpp @@ -9,6 +9,8 @@ #include +#include + #include #include #include @@ -51,9 +53,9 @@ public : typename OutputRange, typename DistanceStrategy > - static inline void apply( + static inline result_code apply( Point const& input_p1, Point const& input_p2, - strategy::buffer::buffer_side_selector side, + buffer_side_selector side, DistanceStrategy const& distance, OutputRange& output_range) { @@ -78,10 +80,11 @@ public : // In case of coordinates differences of e.g. 1e300, length // will overflow and we should not generate output #ifdef BOOST_GEOMETRY_DEBUG_BUFFER_WARN - std::cout << "Length not calculated for points " << geometry::wkt(input_p1) - << " " << geometry::wkt(input_p2) << " " << length << std::endl; + std::cout << "Error in length calculation for points " + << geometry::wkt(input_p1) << " " << geometry::wkt(input_p2) + << " length: " << length << std::endl; #endif - return; + return result_error_numerical; } if (geometry::math::equals(length, 0)) @@ -89,14 +92,29 @@ public : // Coordinates are simplified and therefore most often not equal. // But if simplify is skipped, or for lines with two // equal points, length is 0 and we cannot generate output. - return; + return result_no_output; } + promoted_type const d = distance.apply(input_p1, input_p2, side); + // Generate the normalized perpendicular p, to the left (ccw) promoted_type const px = -dy / length; promoted_type const py = dx / length; - promoted_type const d = distance.apply(input_p1, input_p2, side); + if (geometry::math::equals(px, 0) + && geometry::math::equals(py, 0)) + { + // This basically should not occur - because of the checks above. + // There are no unit tests triggering this condition +#ifdef BOOST_GEOMETRY_DEBUG_BUFFER_WARN + std::cout << "Error in perpendicular calculation for points " + << geometry::wkt(input_p1) << " " << geometry::wkt(input_p2) + << " length: " << length + << " distance: " << d + << std::endl; +#endif + return result_no_output; + } output_range.resize(2); @@ -104,6 +122,8 @@ public : set<1>(output_range.front(), get<1>(input_p1) + py * d); set<0>(output_range.back(), get<0>(input_p2) + px * d); set<1>(output_range.back(), get<1>(input_p2) + py * d); + + return result_normal; } #endif // DOXYGEN_SHOULD_SKIP_THIS }; diff --git a/test/algorithms/buffer/linestring_buffer.cpp b/test/algorithms/buffer/linestring_buffer.cpp index 1b5ed0354..cb3c11cfc 100644 --- a/test/algorithms/buffer/linestring_buffer.cpp +++ b/test/algorithms/buffer/linestring_buffer.cpp @@ -258,16 +258,13 @@ void test_invalid() bg::strategy::buffer::end_round end_round32(32); bg::strategy::buffer::join_round join_round32(32); -#if defined(BOOST_GEOMETRY_BUFFER_INCLUDE_FAILING_TESTS) - // Though the generated buffer of this linestring, containing extreme differences, is not correct, it resembles - // the equivalent (where the coordinates are set to 0). Both SQL Server and POSTGIS do not output anything for this case - test_one("mysql_report_2015_04_10a", mysql_report_2015_04_10a, join_round32, end_round32, 86496.5005, 100.0); - test_one("mysql_report_2015_04_10b", mysql_report_2015_04_10b, join_round32, end_round32, 86496.5005, 100.0); - test_one("mysql_report_2015_04_10c", mysql_report_2015_04_10c, join_round32, end_round32, 86496.5005, 100.0); - test_one("mysql_report_2015_04_10d", mysql_report_2015_04_10d, join_round32, end_round32, 86496.5005, 100.0); - test_one("mysql_report_2015_04_10e", mysql_report_2015_04_10e, join_round32, end_round32, 86496.5005, 100.0); - test_one("mysql_report_2015_04_10f", mysql_report_2015_04_10f, join_round32, end_round32, 86496.5005, 100.0); -#endif + // Linestring contains extreme differences causing numeric errors. Empty geometry is returned + test_one("mysql_report_2015_04_10a", mysql_report_2015_04_10a, join_round32, end_round32, 0.0, 100.0); + test_one("mysql_report_2015_04_10b", mysql_report_2015_04_10b, join_round32, end_round32, 0.0, 100.0); + test_one("mysql_report_2015_04_10c", mysql_report_2015_04_10c, join_round32, end_round32, 0.0, 100.0); + test_one("mysql_report_2015_04_10d", mysql_report_2015_04_10d, join_round32, end_round32, 0.0, 100.0); + test_one("mysql_report_2015_04_10e", mysql_report_2015_04_10e, join_round32, end_round32, 0.0, 100.0); + test_one("mysql_report_2015_04_10f", mysql_report_2015_04_10f, join_round32, end_round32, 0.0, 100.0); // The equivalent, valid, case test_one("mysql_report_2015_04_10g", mysql_report_2015_04_10g, join_round32, end_round32, 86527.871, 100.0); diff --git a/test/algorithms/buffer/multi_linestring_buffer.cpp b/test/algorithms/buffer/multi_linestring_buffer.cpp index c48127164..3ff7858e8 100644 --- a/test/algorithms/buffer/multi_linestring_buffer.cpp +++ b/test/algorithms/buffer/multi_linestring_buffer.cpp @@ -29,7 +29,8 @@ static std::string const mikado2 = "MULTILINESTRING((-6.117647058823528993798390 static std::string const mikado3 = "MULTILINESTRING((1 18,4.0559006211180124168436122999992 7.8136645962732922399140989000443),(6.7243816254416959310447055031545 -1.0812720848056533995418249105569,7 -2,7 -8,14 3.6666666666666669627261399000417),(15.297872340425531234586742357351 5.8297872340425538340014099958353,16 7,15.214285714285713524418497399893 5.8445378151260509724806979647838),(13.685863874345550073030608473346 3.5968586387434555717845796607435,-1 -18,-3.7900797165633304253162805252941 -11.117803365810452476125647081062),(-11.540540540540540348501963308081 8,-16 19,8 14),(1 -10,6.5999999999999996447286321199499 -1.200000000000000177635683940025),(11.5 6.5,15 12),(19 10,11.564231738035264385189293534495 6.4886649874055422060337150469422),(-13.438785504407443127661281323526 -5.3183153770812925387190261972137,-17 -7,-12.970074812967581578959652688354 -7.7556109725685784539450651209336),(-2.3532338308457703135445626685396 -9.7462686567164187323442092747428,-1 -10,12.285714285714286475581502600107 3.2857142857142864755815026001073),(14.90000000000000035527136788005 5.9000000000000003552713678800501,15 6,14.893004115226338157640384451952 5.9012345679012341292946075554937),(11.987804878048780921062643756159 3.2195121951219514144781896902714,-11 -18),(-12.210826210826210669324609625619 -11.703703703703702387883822666481,-15 -15,-11.463576158940396609864365018439 -15.589403973509934786534358863719),(-8.9189189189189193029960733838379 -16.013513513513512265262761502527,-3 -17,-7.0297239915074314353660156484693 -14.210191082802548834251865628175),(-12.450511945392491952588898129761 -10.457337883959045399251408525743,-16 -8,-12.923076923076923350208744523115 -8),(-0.52380952380952372493538859998807 -8,18 -8),(2 -19,-2.2961165048543685784920853620861 -9.6917475728155331182733789319173),(6.0463576158940393057150686217938 -1.7284768211920527036795647291001,7 -3,6.4482758620689653028534848999698 -1.3448275862068967967388744000345),(-1.3333333333333339254522798000835 8,4 16,2.9090909090909091716525836091023 8),(0.64705882352941168633719826175366 -6.8823529411764710062016092706472,-3 -16))"; static std::string const mikado4 = "MULTILINESTRING((-15 2,-15 -17,-6 11,-1.9358288770053475591481628725887 10.572192513368984023713892383967),(2.1545064377682408007785852532834 10.14163090128755406738036981551,6.87603305785123986026974307606 9.6446280991735537924114396446384),(8.4810810810810810522752944962122 9.475675675675674369813350494951,13 9),(-15 0,-8 9,-2.9850746268656713766631582984701 4.4865671641791049495395782287233),(-1.8235294117647056211239942058455 3.4411764705882355031008046353236,-1.1428571428571423496123315999284 2.8285714285714291804652020800859),(1.2307692307692308375521861307789 0.69230769230769229061195346730528,1.2857142857142858094476878250134 0.64285714285714290472384391250671,2 0,1.9459459459459460539676456392044 0.51351351351351348650808859019889),(1.908127208480565384363103476062 0.87279151943462895957281943992712,1.9078014184397162900097555393586 0.87588652482269502286271745106205),(1.4685990338164249813246442499803 5.0483091787439615671928550000302,0.63551401869158885560295857430901 12.962616822429906093816498469096,0 19,2.9565217391304345895264304999728 8.6521739130434784925682834000327),(0 19,3.4942528735632185643567027000245 6.770114942528735468840750399977),(4.75 2.375,5.2427184466019420838733822165523 0.65048543689320226235395239200443),(5.5384615384615383248956277384423 -0.38461538461538458122390693461057,5.7358490566037731994697423942853 -1.0754716981132084185901476303115),(5.9777777777777778567269706400111 -1.9222222222222207221875578397885,6.867052023121386739035187929403 -5.0346820809248553629799971531611,10 -16,-14 -19,-12 -12),(0 10,1.9476439790575916788384347455576 5.4554973821989527493769855936989),(-4 1,-4.2790697674418600726653494348284 0.16279069767441856075862460784265))"; -static std::string const mysql_2015_04_10 = "MULTILINESTRING((-58 19, 61 88),(1.922421e+307 1.520384e+308, 15 42, 89 -93,-89 -22),(-63 -5, -262141 -536870908, -3 87, 77 -69))"; +static std::string const mysql_2015_04_10a = "MULTILINESTRING((-58 19, 61 88),(1.922421e+307 1.520384e+308, 15 42, 89 -93,-89 -22),(-63 -5, -262141 -536870908, -3 87, 77 -69))"; +static std::string const mysql_2015_04_10b = "MULTILINESTRING((-58 19, 61 88),(-63 -5, -262141 -536870908, -3 87, 77 -69))"; template void test_all() @@ -99,12 +100,11 @@ void test_all() test_one("mikado4_small", mikado4, join_round32, end_round32, 2103.686, 10.0); test_one("mikado4_small", mikado4, join_round32, end_flat, 1930.785, 10.0); -#if defined(BOOST_GEOMETRY_BUFFER_INCLUDE_FAILING_TESTS) - // Coordinates vary so much that + // Coordinates in one linestring vary so much that // length = geometry::math::sqrt(dx * dx + dy * dy); returns a value of inf for length - // Buffer is still generated though (see also linestring.cpp) - test_one("mysql_2015_04_10", mysql_2015_04_10, join_round32, end_round32, 15471237017.3871, 0.98); -#endif + // That geometry is skipped for the buffer + test_one("mysql_2015_04_10a", mysql_2015_04_10a, join_round32, end_round32, 927681870.1710, 0.98); + test_one("mysql_2015_04_10b", mysql_2015_04_10b, join_round32, end_round32, 927681870.1710, 0.98); } diff --git a/test/algorithms/buffer/polygon_buffer.cpp b/test/algorithms/buffer/polygon_buffer.cpp index b45e0f59f..adc89de8b 100644 --- a/test/algorithms/buffer/polygon_buffer.cpp +++ b/test/algorithms/buffer/polygon_buffer.cpp @@ -157,7 +157,7 @@ public : typename OutputRange, typename DistanceStrategy > - static inline void apply( + static inline bg::strategy::buffer::result_code apply( Point const& input_p1, Point const& input_p2, bg::strategy::buffer::buffer_side_selector side, DistanceStrategy const& distance, @@ -173,7 +173,7 @@ public : if (bg::math::equals(length, 0)) { - return; + return bg::strategy::buffer::result_no_output; } // Generate the perpendicular p, to the left (ccw), and use an adapted distance @@ -187,6 +187,8 @@ public : bg::set<1>(output_range.front(), bg::get<1>(input_p1) + py); bg::set<0>(output_range.back(), bg::get<0>(input_p2) + px); bg::set<1>(output_range.back(), bg::get<1>(input_p2) + py); + + return bg::strategy::buffer::result_normal; } };